From 6eb48a4c3bfaebf096fecadfe3b1ea853e83a5b9 Mon Sep 17 00:00:00 2001 From: Uxio Fuentefria Date: Tue, 24 Oct 2023 17:23:55 +0200 Subject: [PATCH] WIP Update from 1.1.1 --- contracts/libraries/SafeToL2Migration.sol | 89 ++++++++++++++++++----- test/libraries/SafeToL2Migration.spec.ts | 53 +++++++++++++- 2 files changed, 120 insertions(+), 22 deletions(-) diff --git a/contracts/libraries/SafeToL2Migration.sol b/contracts/libraries/SafeToL2Migration.sol index 608d282cd..566b2e01e 100644 --- a/contracts/libraries/SafeToL2Migration.sol +++ b/contracts/libraries/SafeToL2Migration.sol @@ -8,6 +8,12 @@ import {Enum} from "../common/Enum.sol"; interface ISafe { // solhint-disable-next-line function VERSION() external view returns (string memory); + + function setFallbackHandler(address handler) external; + + function getOwners() external view returns (address[] memory); + + function getThreshold() external view returns (uint256); } /** @@ -34,6 +40,8 @@ contract SafeToL2Migration is SafeStorage { */ event ChangedMasterCopy(address singleton); + event SafeSetup(address indexed initiator, address[] owners, uint256 threshold, address initializer, address fallbackHandler); + event SafeMultiSigTransaction( address to, uint256 value, @@ -50,6 +58,30 @@ contract SafeToL2Migration is SafeStorage { bytes additionalInfo ); + function migrate(address l2Singleton) private { + singleton = l2Singleton; + + // Simulate a L2 transaction so indexer picks up the Safe + // 0xef2624ae - keccak("migrateToL2(address)") + bytes memory data = abi.encodeWithSelector(0xef2624ae, l2Singleton); + // nonce, sender, threshold + bytes memory additionalInfo = abi.encode(nonce - 1, msg.sender, threshold); + emit SafeMultiSigTransaction( + MIGRATION_SINGLETON, + 0, + data, + Enum.Operation.DelegateCall, + 0, + 0, + 0, + address(0), + address(0), + "", // We cannot detect signatures + additionalInfo + ); + emit ChangedMasterCopy(singleton); + } + /** * @notice Migrate from Safe 1.3.0/1.4.1 Singleton (L1) to the same version provided L2 singleton * Safe is required to have nonce 0 so backend can support it after the migration @@ -58,6 +90,7 @@ contract SafeToL2Migration is SafeStorage { */ function migrateToL2(address l2Singleton) public { require(address(this) != MIGRATION_SINGLETON, "Migration should only be called via delegatecall"); + require(address(singleton) != l2Singleton, "Safe is already using the singleton"); // Nonce is increased before executing a tx, so first executed tx will have nonce=1 require(nonce == 1, "Safe must have not executed any tx"); @@ -71,26 +104,44 @@ contract SafeToL2Migration is SafeStorage { "Provided singleton version is not supported" ); - singleton = l2Singleton; + migrate(l2Singleton); + } - // Simulate a L2 transaction so indexer picks up the Safe - // 0xef2624ae - keccak("migrateToL2(address)") - bytes memory data = abi.encodeWithSelector(0xef2624ae, l2Singleton); - // nonce, sender, threshold - bytes memory additionalInfo = abi.encode(nonce - 1, msg.sender, threshold); - emit SafeMultiSigTransaction( - MIGRATION_SINGLETON, - 0, - data, - Enum.Operation.DelegateCall, - 0, - 0, - 0, - address(0), - address(0), - "", // We cannot detect signatures - additionalInfo + function migrateFromV111(address l2Singleton, address fallbackHandler) public { + require(address(this) != MIGRATION_SINGLETON, "Migration should only be called via delegatecall"); + + bytes32 oldSingletonVersion = keccak256(abi.encodePacked(ISafe(singleton).VERSION())); + require(oldSingletonVersion == keccak256(abi.encodePacked("1.1.1")), "Provided singleton version is not supported"); + + bytes32 newSingletonVersion = keccak256(abi.encodePacked(ISafe(l2Singleton).VERSION())); + require( + newSingletonVersion == keccak256(abi.encodePacked("1.3.0")) || newSingletonVersion == keccak256(abi.encodePacked("1.4.1")), + "Provided singleton version is not supported" ); - emit ChangedMasterCopy(singleton); + + require(isContract(fallbackHandler), "fallbackHandler is not a contract"); + ISafe safe = ISafe(address(this)); + safe.setFallbackHandler(fallbackHandler); + emit SafeSetup(MIGRATION_SINGLETON, safe.getOwners(), safe.getThreshold(), address(0), fallbackHandler); + migrate(l2Singleton); + } + + /** + * @notice Checks whether an Ethereum address corresponds to a contract or an externally owned account (EOA). + * @param account The Ethereum address to be checked. + * @return A boolean value indicating whether the address is associated with a contract (true) or an EOA (false). + * @dev This function relies on the `extcodesize` assembly opcode to determine whether an address is a contract. + * It may return incorrect results in some edge cases (see documentation for details). + * Developers should use caution when relying on the results of this function for critical decision-making. + */ + function isContract(address account) internal view returns (bool) { + uint256 size; + // solhint-disable-next-line no-inline-assembly + assembly { + size := extcodesize(account) + } + + // If the code size is greater than 0, it is a contract; otherwise, it is an EOA. + return size > 0; } } diff --git a/test/libraries/SafeToL2Migration.spec.ts b/test/libraries/SafeToL2Migration.spec.ts index cee5196ac..ccf6e246e 100644 --- a/test/libraries/SafeToL2Migration.spec.ts +++ b/test/libraries/SafeToL2Migration.spec.ts @@ -4,7 +4,13 @@ import { AddressZero } from "@ethersproject/constants"; import { getSafeWithSingleton, getSafeSingletonAt, getMock } from "../utils/setup"; import deploymentData from "../json/safeDeployment.json"; import safeRuntimeBytecode from "../json/safeRuntimeBytecode.json"; -import { buildSafeTransaction, executeContractCallWithSigners, executeTxWithSigners } from "../../src/utils/execution"; +import { + buildSafeTransaction, + executeContractCallWithSigners, + executeTx, + executeTxWithSigners, + safeApproveHash, +} from "../../src/utils/execution"; const SAFE_SINGLETON_141_ADDRESS = "0x3E5c63644E683549055b9Be8653de26E0B4CD36E"; @@ -12,6 +18,8 @@ const SAFE_SINGLETON_141_L2_ADDRESS = "0xfb1bffC9d739B8D520DaF37dF666da4C687191E const SAFE_SINGLETON_150_L2_ADDRESS = "0x0Ee37514644683f7EB9745a5726C722DeBa77e52"; +const COMPATIBILITY_FALLBACK_HANDLER_150 = "0x8aa755cB169991fEDC3E306751dCb71964A041c7"; + const FALLBACK_HANDLER_STORAGE_SLOT = "0x6c9a6c4a39284e37ed1cf53d337577d14212a4870fb976a4366c693b939918d5"; const GUARD_STORAGE_SLOT = "0x4a204f620c8c5ccdca3fd54d003badd85ba500436a431f0cbda4f558c93c34c8"; @@ -26,15 +34,21 @@ describe("SafeToL2Migration library", () => { await hre.network.provider.send("hardhat_setCode", [SAFE_SINGLETON_141_ADDRESS, safeRuntimeBytecode.safe141]); await hre.network.provider.send("hardhat_setCode", [SAFE_SINGLETON_141_L2_ADDRESS, safeRuntimeBytecode.safe141l2]); await hre.network.provider.send("hardhat_setCode", [SAFE_SINGLETON_150_L2_ADDRESS, safeRuntimeBytecode.safe150l2]); + await hre.network.provider.send("hardhat_setCode", [ + COMPATIBILITY_FALLBACK_HANDLER_150, + safeRuntimeBytecode.safe150CompatibilityFallbackHandler, + ]); const signers = await ethers.getSigners(); const [user1] = signers; + const singleton111Address = (await (await user1.sendTransaction({ data: deploymentData.safe111 })).wait())?.contractAddress; const singleton130Address = (await (await user1.sendTransaction({ data: deploymentData.safe130 })).wait())?.contractAddress; const singleton130L2Address = (await (await user1.sendTransaction({ data: deploymentData.safe130l2 })).wait())?.contractAddress; - if (!singleton130Address || !singleton130L2Address) { - throw new Error("Could not deploy Safe130 or Safe130L2"); + if (!singleton111Address || !singleton130Address || !singleton130L2Address) { + throw new Error("Could not deploy Safe111, Safe130 or Safe130L2"); } + const singleton111 = await getSafeSingletonAt(singleton111Address); const singleton130 = await getSafeSingletonAt(singleton130Address); const singleton141 = await getSafeSingletonAt(SAFE_SINGLETON_141_ADDRESS); @@ -69,6 +83,7 @@ describe("SafeToL2Migration library", () => { const safeToL2MigrationContract = await hre.ethers.getContractFactory("SafeToL2Migration"); const migration = await safeToL2MigrationContract.deploy(); return { + safe111: await getSafeWithSingleton(singleton111, [user1.address]), safe130: await getSafeWithSingleton(singleton130, [user1.address]), safe141: await getSafeWithSingleton(singleton141, [user1.address]), safeWith1967Proxy, @@ -227,6 +242,38 @@ describe("SafeToL2Migration library", () => { expect(await safe141.nonce()).to.be.eq(1); }); + it("migrates from singleton 1.1.1 to 1.4.1L2", async () => { + const { + safe111, + migration, + signers: [user1], + } = await setupTests(); + expect(await safe111.VERSION()).eq("1.1.1"); + const safeAddress = await safe111.getAddress(); + + // The emit matcher checks the address, which is the Safe as delegatecall is used + const migrationSafe = migration.attach(safeAddress); + const migrationAddress = await migration.getAddress(); + + const functionName = "migrateFromV111"; + const data = migration.interface.encodeFunctionData(functionName, [ + SAFE_SINGLETON_141_L2_ADDRESS, + COMPATIBILITY_FALLBACK_HANDLER_150, + ]); + const nonce = await safe111.nonce(); + expect(nonce).to.be.eq(0); + const tx = buildSafeTransaction({ to: migrationAddress, data, operation: 1, nonce }); + + expect(await executeTx(safe111, tx, [await safeApproveHash(user1, safe111, tx, true)])) + .to.emit(migrationSafe, "ChangedMasterCopy") + .withArgs(SAFE_SINGLETON_141_L2_ADDRESS); + + expect(await safe111.nonce()).to.be.eq(1); + expect(await safe111.VERSION()).to.be.eq("1.4.1"); + const singletonResp = await user1.call({ to: safeAddress, data: migratedInterface.encodeFunctionData("masterCopy") }); + expect(migratedInterface.decodeFunctionResult("masterCopy", singletonResp)[0]).to.eq(SAFE_SINGLETON_141_L2_ADDRESS); + }); + it("doesn't touch important storage slots", async () => { const { safe130,