From d6bcd32034abeb640270f685baab8719a21b363c Mon Sep 17 00:00:00 2001 From: Akshay Date: Tue, 16 Jul 2024 22:10:10 +0200 Subject: [PATCH] [#787] Remove Safe150Migration --- contracts/libraries/Safe150Migration.sol | 109 -------- test/libraries/Safe150Migration.spec.ts | 309 ----------------------- test/utils/setup.ts | 4 - 3 files changed, 422 deletions(-) delete mode 100644 contracts/libraries/Safe150Migration.sol delete mode 100644 test/libraries/Safe150Migration.spec.ts diff --git a/contracts/libraries/Safe150Migration.sol b/contracts/libraries/Safe150Migration.sol deleted file mode 100644 index 83e080046..000000000 --- a/contracts/libraries/Safe150Migration.sol +++ /dev/null @@ -1,109 +0,0 @@ -// SPDX-License-Identifier: LGPL-3.0-only -/* solhint-disable one-contract-per-file */ -pragma solidity >=0.7.0 <0.9.0; - -import {SafeStorage} from "../libraries/SafeStorage.sol"; -import {ISafe} from "../interfaces/ISafe.sol"; - -/** - * @title Migration Contract for Safe Upgrade - * @notice This contract facilitates the migration of a Safe contract from version 1.3.0/1.4.1 to 1.5.0. - * @dev IMPORTANT: The library is intended to be used with the Safe standard proxy that stores the singleton address - * at the storage slot 0. Use at your own risk with custom proxy implementations. The library will block calls - * if the address stored at the storage slot 0 is not a contract. - */ -contract Safe150Migration is SafeStorage { - // Address of Safe contract version 1.5.0 Singleton (L1) - // TODO: Update this address when the Safe 1.5.0 Singleton is deployed - address public constant SAFE_150_SINGLETON = address(0x477C3fb2D564349E2F95a2EF1091bF9657b26145); - - // Address of Safe contract version 1.5.0 Singleton (L2) - // TODO: Update this address when the Safe 1.5.0 Singleton (L2) is deployed - address public constant SAFE_150_SINGLETON_L2 = address(0x551A2F9a71bF88cDBef3CBe60E95722f38eE0eAA); - - // Address of Safe contract version 1.5.0 Compatibility Fallback Handler - // TODO: Update this address when the Safe 1.5.0 Compatibility Fallback Handler is deployed - address public constant SAFE_150_FALLBACK_HANDLER = address(0x4c95c836D31d329d80d696cb679f3dEa028Ad4e5); - - /** - * @notice Event indicating a change of master copy address. - * @param singleton New master copy address - */ - event ChangedMasterCopy(address singleton); - - /** - * @notice Constructor - * @dev Initializes the migrationSingleton with the contract's own address. - */ - constructor() { - require(isContract(SAFE_150_SINGLETON), "Safe 1.4.1 Singleton is not deployed"); - require(isContract(SAFE_150_SINGLETON_L2), "Safe 1.4.1 Singleton (L2) is not deployed"); - require(isContract(SAFE_150_FALLBACK_HANDLER), "Safe 1.4.1 Fallback Handler is not deployed"); - } - - function checkCurrentSingleton() internal view { - require(isContract(singleton), "Trying to migrate an invalid Safe"); - } - - modifier validSingletonOnly() { - checkCurrentSingleton(); - _; - } - - /** - * @notice Migrate to Safe 1.5.0 Singleton (L1) at `SAFE_150_SINGLETON` - * @dev This function should only be called via a delegatecall to perform the upgrade. - */ - function migrateSingleton() public validSingletonOnly { - singleton = SAFE_150_SINGLETON; - emit ChangedMasterCopy(singleton); - } - - /** - * @notice Migrate and set the fallback handler to Safe 1.5.0 Compatibility Fallback Handler. - */ - function migrateWithFallbackHandler() public validSingletonOnly { - migrateSingleton(); - - ISafe(address(this)).setFallbackHandler(SAFE_150_FALLBACK_HANDLER); - } - - /** - * @notice Migrate to Safe 1.5.0 Singleton (L2) at `SAFE_150_SINGLETON_L2` - * @dev This function should only be called via a delegatecall to perform the upgrade. - */ - function migrateL2Singleton() public validSingletonOnly { - singleton = SAFE_150_SINGLETON_L2; - emit ChangedMasterCopy(singleton); - } - - /** - * @notice Migrate to Safe 1.5.0 Singleton (L2) and set the fallback handler to Safe 1.5.0 Compatibility Fallback Handler. - */ - function migrateL2WithFallbackHandler() public validSingletonOnly { - migrateL2Singleton(); - - ISafe(address(this)).setFallbackHandler(SAFE_150_FALLBACK_HANDLER); - } - - /** - * @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 no-inline-assembly */ - /// @solidity memory-safe-assembly - assembly { - size := extcodesize(account) - } - /* solhint-enable no-inline-assembly */ - - // 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/Safe150Migration.spec.ts b/test/libraries/Safe150Migration.spec.ts deleted file mode 100644 index bad2f3d45..000000000 --- a/test/libraries/Safe150Migration.spec.ts +++ /dev/null @@ -1,309 +0,0 @@ -import { expect } from "chai"; -import hre, { ethers, deployments } from "hardhat"; -import { AddressZero } from "@ethersproject/constants"; -import { getSafeWithSingleton, migrationContractTo150, getSafeSingletonAt } from "../utils/setup"; -import deploymentData from "../json/safeDeployment.json"; -import safeRuntimeBytecode from "../json/safeRuntimeBytecode.json"; -import { executeContractCallWithSigners } from "../../src/utils/execution"; - -const SAFE_SINGLETON_150_ADDRESS = "0x477C3fb2D564349E2F95a2EF1091bF9657b26145"; - -const SAFE_SINGLETON_150_L2_ADDRESS = "0x551A2F9a71bF88cDBef3CBe60E95722f38eE0eAA"; - -const COMPATIBILITY_FALLBACK_HANDLER_150 = "0x4c95c836D31d329d80d696cb679f3dEa028Ad4e5"; - -const FALLBACK_HANDLER_STORAGE_SLOT = "0x6c9a6c4a39284e37ed1cf53d337577d14212a4870fb976a4366c693b939918d5"; - -const GUARD_STORAGE_SLOT = "0x4a204f620c8c5ccdca3fd54d003badd85ba500436a431f0cbda4f558c93c34c8"; - -describe("Safe150Migration library", () => { - const migratedInterface = new ethers.Interface(["function masterCopy() view returns(address)"]); - - const setupTests = deployments.createFixture(async ({ deployments }) => { - await deployments.fixture(); - - // Set the runtime code for hardcoded addresses, so the expected events are emitted - await hre.network.provider.send("hardhat_setCode", [SAFE_SINGLETON_150_ADDRESS, safeRuntimeBytecode.safe150]); - 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 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"); - } - const singleton130 = await getSafeSingletonAt(singleton130Address); - const singleton130L2 = await getSafeSingletonAt(singleton130L2Address); - - const safeWith1967Proxy = await getSafeSingletonAt( - await hre.ethers - .getContractFactory("UpgradeableProxy") - .then((factory) => - factory.deploy( - singleton130Address, - singleton130.interface.encodeFunctionData("setup", [ - [user1.address], - 1, - AddressZero, - "0x", - AddressZero, - AddressZero, - 0, - AddressZero, - ]), - ), - ) - .then((proxy) => proxy.getAddress()), - ); - const migration = await (await migrationContractTo150()).deploy(); - return { - safe130: await getSafeWithSingleton(singleton130, [user1.address]), - safe130l2: await getSafeWithSingleton(singleton130L2, [user1.address]), - safeWith1967Proxy, - migration, - signers, - }; - }); - - describe("migrateSingleton", () => { - it("reverts if the singleton in storage at slot 0 is not a contract", async () => { - const { - migration, - safeWith1967Proxy, - signers: [user1], - } = await setupTests(); - - await expect( - executeContractCallWithSigners(safeWith1967Proxy, migration, "migrateSingleton", [], [user1], true), - ).to.be.revertedWith("GS013"); - }); - - it("migrates the singleton", async () => { - const { - safe130, - migration, - signers: [user1], - } = await setupTests(); - const safeAddress = await safe130.getAddress(); - // The emit matcher checks the address, which is the Safe as delegatecall is used - const migrationSafe = migration.attach(safeAddress); - - await expect(executeContractCallWithSigners(safe130, migration, "migrateSingleton", [], [user1], true)) - .to.emit(migrationSafe, "ChangedMasterCopy") - .withArgs(SAFE_SINGLETON_150_ADDRESS); - - const singletonResp = await user1.call({ to: safeAddress, data: migratedInterface.encodeFunctionData("masterCopy") }); - await expect(migratedInterface.decodeFunctionResult("masterCopy", singletonResp)[0]).to.eq(SAFE_SINGLETON_150_ADDRESS); - }); - - it("doesn't touch important storage slots", async () => { - const { - safe130, - migration, - signers: [user1], - } = await setupTests(); - const safeAddress = await safe130.getAddress(); - - const ownerCountBeforeMigration = await hre.ethers.provider.getStorage(safeAddress, 3); - const thresholdBeforeMigration = await hre.ethers.provider.getStorage(safeAddress, 4); - const nonceBeforeMigration = await hre.ethers.provider.getStorage(safeAddress, 5); - const guardBeforeMigration = await hre.ethers.provider.getStorage(safeAddress, GUARD_STORAGE_SLOT); - const fallbackHandlerBeforeMigration = await hre.ethers.provider.getStorage(safeAddress, FALLBACK_HANDLER_STORAGE_SLOT); - - expect(await executeContractCallWithSigners(safe130, migration, "migrateSingleton", [], [user1], true)); - - expect(await hre.ethers.provider.getStorage(safeAddress, 3)).to.be.eq(ownerCountBeforeMigration); - expect(await hre.ethers.provider.getStorage(safeAddress, 4)).to.be.eq(thresholdBeforeMigration); - expect(await hre.ethers.provider.getStorage(safeAddress, 5)).to.be.eq( - ethers.toBigInt(nonceBeforeMigration) + ethers.toBigInt(1), - ); - expect(await hre.ethers.provider.getStorage(safeAddress, GUARD_STORAGE_SLOT)).to.be.eq(guardBeforeMigration); - expect(await hre.ethers.provider.getStorage(safeAddress, FALLBACK_HANDLER_STORAGE_SLOT)).to.be.eq( - fallbackHandlerBeforeMigration, - ); - }); - }); - - describe("migrateL2Singleton", () => { - it("reverts if the singleton in storage at slot 0 is not a contract", async () => { - const { - migration, - safeWith1967Proxy, - signers: [user1], - } = await setupTests(); - - await expect( - executeContractCallWithSigners(safeWith1967Proxy, migration, "migrateL2Singleton", [], [user1], true), - ).to.be.revertedWith("GS013"); - }); - - it("migrates the singleton", async () => { - const { - safe130l2, - migration, - signers: [user1], - } = await setupTests(); - const safeAddress = await safe130l2.getAddress(); - // The emit matcher checks the address, which is the Safe as delegatecall is used - const migrationSafe = migration.attach(safeAddress); - - await expect(executeContractCallWithSigners(safe130l2, migration, "migrateL2Singleton", [], [user1], true)) - .to.emit(migrationSafe, "ChangedMasterCopy") - .withArgs(SAFE_SINGLETON_150_L2_ADDRESS); - - const singletonResp = await user1.call({ to: safeAddress, data: migratedInterface.encodeFunctionData("masterCopy") }); - await expect(migratedInterface.decodeFunctionResult("masterCopy", singletonResp)[0]).to.eq(SAFE_SINGLETON_150_L2_ADDRESS); - }); - - it("doesn't touch important storage slots", async () => { - const { - safe130l2, - migration, - signers: [user1], - } = await setupTests(); - const safeAddress = await safe130l2.getAddress(); - - const ownerCountBeforeMigration = await hre.ethers.provider.getStorage(safeAddress, 3); - const thresholdBeforeMigration = await hre.ethers.provider.getStorage(safeAddress, 4); - const nonceBeforeMigration = await hre.ethers.provider.getStorage(safeAddress, 5); - const guardBeforeMigration = await hre.ethers.provider.getStorage(safeAddress, GUARD_STORAGE_SLOT); - - expect(await executeContractCallWithSigners(safe130l2, migration, "migrateL2Singleton", [], [user1], true)); - - expect(await hre.ethers.provider.getStorage(safeAddress, 3)).to.be.eq(ownerCountBeforeMigration); - expect(await hre.ethers.provider.getStorage(safeAddress, 4)).to.be.eq(thresholdBeforeMigration); - expect(await hre.ethers.provider.getStorage(safeAddress, 5)).to.be.eq( - ethers.toBigInt(nonceBeforeMigration) + ethers.toBigInt(1), - ); - expect(await hre.ethers.provider.getStorage(safeAddress, GUARD_STORAGE_SLOT)).to.be.eq(guardBeforeMigration); - }); - }); - - describe("migrateWithFallbackHandler", () => { - it("reverts if the singleton in storage at slot 0 is not a contract", async () => { - const { - migration, - safeWith1967Proxy, - signers: [user1], - } = await setupTests(); - - await expect( - executeContractCallWithSigners(safeWith1967Proxy, migration, "migrateWithFallbackHandler", [], [user1], true), - ).to.be.revertedWith("GS013"); - }); - - it("migrates the singleton and the fallback handler", async () => { - const { - safe130, - migration, - signers: [user1], - } = await setupTests(); - const safeAddress = await safe130.getAddress(); - // The emit matcher checks the address, which is the Safe as delegatecall is used - const migrationSafe = migration.attach(safeAddress); - - await expect(executeContractCallWithSigners(safe130, migration, "migrateWithFallbackHandler", [], [user1], true)) - .to.emit(migrationSafe, "ChangedMasterCopy") - .withArgs(SAFE_SINGLETON_150_ADDRESS) - .and.to.emit(safe130, "ChangedFallbackHandler") - .withArgs(COMPATIBILITY_FALLBACK_HANDLER_150); - - const singletonResp = await user1.call({ to: safeAddress, data: migratedInterface.encodeFunctionData("masterCopy") }); - await expect(migratedInterface.decodeFunctionResult("masterCopy", singletonResp)[0]).to.eq(SAFE_SINGLETON_150_ADDRESS); - - expect(await safe130.getStorageAt(FALLBACK_HANDLER_STORAGE_SLOT, 1)).to.eq( - "0x" + COMPATIBILITY_FALLBACK_HANDLER_150.slice(2).toLowerCase().padStart(64, "0"), - ); - }); - - it("doesn't touch important storage slots", async () => { - const { - safe130, - migration, - signers: [user1], - } = await setupTests(); - const safeAddress = await safe130.getAddress(); - - const ownerCountBeforeMigration = await hre.ethers.provider.getStorage(safeAddress, 3); - const thresholdBeforeMigration = await hre.ethers.provider.getStorage(safeAddress, 4); - const nonceBeforeMigration = await hre.ethers.provider.getStorage(safeAddress, 5); - const guardBeforeMigration = await hre.ethers.provider.getStorage(safeAddress, GUARD_STORAGE_SLOT); - - expect(await executeContractCallWithSigners(safe130, migration, "migrateWithFallbackHandler", [], [user1], true)); - - expect(await hre.ethers.provider.getStorage(safeAddress, 3)).to.be.eq(ownerCountBeforeMigration); - expect(await hre.ethers.provider.getStorage(safeAddress, 4)).to.be.eq(thresholdBeforeMigration); - expect(await hre.ethers.provider.getStorage(safeAddress, 5)).to.be.eq( - ethers.toBigInt(nonceBeforeMigration) + ethers.toBigInt(1), - ); - expect(await hre.ethers.provider.getStorage(safeAddress, GUARD_STORAGE_SLOT)).to.be.eq(guardBeforeMigration); - }); - }); - - describe("migrateL2WithFallbackHandler", () => { - it("reverts if the singleton in storage at slot 0 is not a contract", async () => { - const { - migration, - safeWith1967Proxy, - signers: [user1], - } = await setupTests(); - - await expect( - executeContractCallWithSigners(safeWith1967Proxy, migration, "migrateL2WithFallbackHandler", [], [user1], true), - ).to.be.revertedWith("GS013"); - }); - - it("migrates the singleton and the fallback handler", async () => { - const { - safe130l2, - migration, - signers: [user1], - } = await setupTests(); - const safeAddress = await safe130l2.getAddress(); - // The emit matcher checks the address, which is the Safe as delegatecall is used - const migrationSafe = migration.attach(safeAddress); - - await expect(executeContractCallWithSigners(safe130l2, migration, "migrateL2WithFallbackHandler", [], [user1], true)) - .to.emit(migrationSafe, "ChangedMasterCopy") - .withArgs(SAFE_SINGLETON_150_L2_ADDRESS) - .and.to.emit(safe130l2, "ChangedFallbackHandler") - .withArgs(COMPATIBILITY_FALLBACK_HANDLER_150); - - const singletonResp = await user1.call({ to: safeAddress, data: migratedInterface.encodeFunctionData("masterCopy") }); - await expect(migratedInterface.decodeFunctionResult("masterCopy", singletonResp)[0]).to.eq(SAFE_SINGLETON_150_L2_ADDRESS); - - expect(await safe130l2.getStorageAt(FALLBACK_HANDLER_STORAGE_SLOT, 1)).to.eq( - "0x" + COMPATIBILITY_FALLBACK_HANDLER_150.slice(2).toLowerCase().padStart(64, "0"), - ); - }); - - it("doesn't touch important storage slots", async () => { - const { - safe130l2, - migration, - signers: [user1], - } = await setupTests(); - const safeAddress = await safe130l2.getAddress(); - - const ownerCountBeforeMigration = await hre.ethers.provider.getStorage(safeAddress, 3); - const thresholdBeforeMigration = await hre.ethers.provider.getStorage(safeAddress, 4); - const nonceBeforeMigration = await hre.ethers.provider.getStorage(safeAddress, 5); - const guardBeforeMigration = await hre.ethers.provider.getStorage(safeAddress, GUARD_STORAGE_SLOT); - - expect(await executeContractCallWithSigners(safe130l2, migration, "migrateL2WithFallbackHandler", [], [user1], true)); - - expect(await hre.ethers.provider.getStorage(safeAddress, 3)).to.be.eq(ownerCountBeforeMigration); - expect(await hre.ethers.provider.getStorage(safeAddress, 4)).to.be.eq(thresholdBeforeMigration); - expect(await hre.ethers.provider.getStorage(safeAddress, 5)).to.be.eq( - ethers.toBigInt(nonceBeforeMigration) + ethers.toBigInt(1), - ); - expect(await hre.ethers.provider.getStorage(safeAddress, GUARD_STORAGE_SLOT)).to.be.eq(guardBeforeMigration); - }); - }); -}); diff --git a/test/utils/setup.ts b/test/utils/setup.ts index e464b6448..b5281134d 100644 --- a/test/utils/setup.ts +++ b/test/utils/setup.ts @@ -111,10 +111,6 @@ export const migrationContract = async () => { return await hre.ethers.getContractFactory("Migration"); }; -export const migrationContractTo150 = async () => { - return await hre.ethers.getContractFactory("Safe150Migration"); -}; - export const migrationContractFrom130To141 = async () => { return await hre.ethers.getContractFactory("Safe130To141Migration"); };