Remove unnecessary guard migration related code

safe-global · May 16, 2024 · ff20341 · ff20341
1 parent 72182da
commit ff20341
Show file tree

Hide file tree

Showing 2 changed files with 7 additions and 111 deletions.
diff --git a/contracts/libraries/Safe150Migration.sol b/contracts/libraries/Safe150Migration.sol
@@ -3,8 +3,8 @@
 pragma solidity >=0.7.0 <0.9.0;
 
 import {SafeStorage} from "../libraries/SafeStorage.sol";
-import {ITransactionGuard} from "../base/GuardManager.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.
@@ -25,9 +25,11 @@ contract Safe150Migration is SafeStorage {
     // TODO: Update this address when the Safe 1.5.0 Compatibility Fallback Handler is deployed
     address public constant SAFE_150_FALLBACK_HANDLER = address(0x8aa755cB169991fEDC3E306751dCb71964A041c7);
 
-    // the slot is defined as "keccak256("guard_manager.guard.address")" in the GuardManager contract
-    // reference: https://github.com/safe-global/safe-smart-account/blob/8ffae95faa815acf86ec8b50021ebe9f96abde10/contracts/base/GuardManager.sol#L76-L77
-    bytes32 internal constant GUARD_STORAGE_SLOT = 0x4a204f620c8c5ccdca3fd54d003badd85ba500436a431f0cbda4f558c93c34c8;
+    /**
+     * @notice Event indicating a change of master copy address.
+     * @param singleton New master copy address
+     */
+    event ChangedMasterCopy(address singleton);
 
     /**
      * @notice Constructor
@@ -39,23 +41,6 @@ contract Safe150Migration is SafeStorage {
         require(isContract(SAFE_150_FALLBACK_HANDLER), "Safe 1.4.1 Fallback Handler is not deployed");
     }
 
-    /**
-     * @notice Event indicating a change of master copy address.
-     * @param singleton New master copy address
-     */
-    event ChangedMasterCopy(address singleton);
-
-    /**
-     * @dev Private function to check if a guard is supported.
-     */
-    function checkGuard() private view {
-        address guard = getGuard();
-
-        if (guard != address(0)) {
-            require(ITransactionGuard(guard).supportsInterface(type(ITransactionGuard).interfaceId), "GS300");
-        }
-    }
-
     function checkCurrentSingleton() internal view {
         require(isContract(singleton), "Trying to migrate an invalid Safe");
     }
@@ -70,8 +55,6 @@ contract Safe150Migration is SafeStorage {
      * @dev This function should only be called via a delegatecall to perform the upgrade.
      */
     function migrateSingleton() public validSingletonOnly {
-        checkGuard();
-
         singleton = SAFE_150_SINGLETON;
         emit ChangedMasterCopy(singleton);
     }
@@ -90,8 +73,6 @@ contract Safe150Migration is SafeStorage {
      * @dev This function should only be called via a delegatecall to perform the upgrade.
      */
     function migrateL2Singleton() public validSingletonOnly {
-        checkGuard();
-
         singleton = SAFE_150_SINGLETON_L2;
         emit ChangedMasterCopy(singleton);
     }
@@ -105,20 +86,6 @@ contract Safe150Migration is SafeStorage {
         ISafe(address(this)).setFallbackHandler(SAFE_150_FALLBACK_HANDLER);
     }
 
-    /**
-     * @notice Get the address of the current guard.
-     * @return guard The address of the current guard contract.
-     */
-    function getGuard() internal view returns (address guard) {
-        bytes32 slot = GUARD_STORAGE_SLOT;
-        /* solhint-disable no-inline-assembly */
-        /// @solidity memory-safe-assembly
-        assembly {
-            guard := sload(slot)
-        }
-        /* solhint-enable no-inline-assembly */
-    }
-
     /**
      * @notice Checks whether an Ethereum address corresponds to a contract or an externally owned account (EOA).
      * @param account The Ethereum address to be checked.

diff --git a/test/libraries/Safe150Migration.spec.ts b/test/libraries/Safe150Migration.spec.ts
@@ -1,7 +1,7 @@
 import { expect } from "chai";
 import hre, { ethers, deployments } from "hardhat";
 import { AddressZero } from "@ethersproject/constants";
-import { getSafeWithSingleton, migrationContractTo150, getSafeSingletonAt, getMock } from "../utils/setup";
+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";
@@ -41,12 +41,6 @@ describe("Safe150Migration library", () => {
         const singleton130 = await getSafeSingletonAt(singleton130Address);
         const singleton130L2 = await getSafeSingletonAt(singleton130L2Address);
 
-        const guardContract = await hre.ethers.getContractAt("ITransactionGuard", AddressZero);
-        const guardEip165Calldata = guardContract.interface.encodeFunctionData("supportsInterface", ["0xe6d7a83a"]);
-
-        const invalidGuardMock = await getMock();
-        await invalidGuardMock.givenCalldataReturnBool(guardEip165Calldata, false);
-
         const safeWith1967Proxy = await getSafeSingletonAt(
             await hre.ethers
                 .getContractFactory("UpgradeableProxy")
@@ -74,7 +68,6 @@ describe("Safe150Migration library", () => {
             safeWith1967Proxy,
             migration,
             signers,
-            invalidGuardMock,
         };
     });
 
@@ -109,22 +102,6 @@ describe("Safe150Migration library", () => {
             await expect(migratedInterface.decodeFunctionResult("masterCopy", singletonResp)[0]).to.eq(SAFE_SINGLETON_150_ADDRESS);
         });
 
-        it("reverts when trying to migrate with a guard incompatible with 1.5.0 guard interface", async () => {
-            const {
-                safe130,
-                migration,
-                signers: [user1],
-                invalidGuardMock,
-            } = await setupTests();
-            const invalidGuardMockAddress = await invalidGuardMock.getAddress();
-
-            await executeContractCallWithSigners(safe130, safe130, "setGuard", [invalidGuardMockAddress], [user1]);
-
-            await expect(executeContractCallWithSigners(safe130, migration, "migrateSingleton", [], [user1], true)).to.be.revertedWith(
-                "GS013",
-            );
-        });
-
         it("doesn't touch important storage slots", async () => {
             const {
                 safe130,
@@ -182,22 +159,6 @@ describe("Safe150Migration library", () => {
             await expect(migratedInterface.decodeFunctionResult("masterCopy", singletonResp)[0]).to.eq(SAFE_SINGLETON_150_L2_ADDRESS);
         });
 
-        it("reverts when trying to migrate with a guard incompatible with 1.5.0 guard interface", async () => {
-            const {
-                safe130l2,
-                migration,
-                signers: [user1],
-                invalidGuardMock,
-            } = await setupTests();
-            const invalidGuardMockAddress = await invalidGuardMock.getAddress();
-
-            await executeContractCallWithSigners(safe130l2, safe130l2, "setGuard", [invalidGuardMockAddress], [user1]);
-
-            await expect(executeContractCallWithSigners(safe130l2, migration, "migrateL2Singleton", [], [user1], true)).to.be.revertedWith(
-                "GS013",
-            );
-        });
-
         it("doesn't touch important storage slots", async () => {
             const {
                 safe130l2,
@@ -257,22 +218,6 @@ describe("Safe150Migration library", () => {
             );
         });
 
-        it("reverts when trying to migrate with a guard incompatible with 1.5.0 guard interface", async () => {
-            const {
-                safe130,
-                migration,
-                signers: [user1],
-                invalidGuardMock,
-            } = await setupTests();
-            const invalidGuardMockAddress = await invalidGuardMock.getAddress();
-
-            await executeContractCallWithSigners(safe130, safe130, "setGuard", [invalidGuardMockAddress], [user1]);
-
-            await expect(
-                executeContractCallWithSigners(safe130, migration, "migrateWithFallbackHandler", [], [user1], true),
-            ).to.be.revertedWith("GS013");
-        });
-
         it("doesn't touch important storage slots", async () => {
             const {
                 safe130,
@@ -332,22 +277,6 @@ describe("Safe150Migration library", () => {
             );
         });
 
-        it("reverts when trying to migrate with a guard incompatible with 1.5.0 guard interface", async () => {
-            const {
-                safe130l2,
-                migration,
-                signers: [user1],
-                invalidGuardMock,
-            } = await setupTests();
-            const invalidGuardMockAddress = await invalidGuardMock.getAddress();
-
-            await executeContractCallWithSigners(safe130l2, safe130l2, "setGuard", [invalidGuardMockAddress], [user1]);
-
-            await expect(
-                executeContractCallWithSigners(safe130l2, migration, "migrateL2WithFallbackHandler", [], [user1], true),
-            ).to.be.revertedWith("GS013");
-        });
-
         it("doesn't touch important storage slots", async () => {
             const {
                 safe130l2,