refactor and test: update same address multisig implementation

contracts/multisigs/GnosisSafeSameAddressMultisig.sol

@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-pragma solidity ^0.8.15;
+pragma solidity ^0.8.21;
 
 // Gnosis Safe Master Copy interface extracted from the mainnet: https://etherscan.io/address/0xd9db270c1b5e3bd161e8c8503c55ceabee709552#code#F6#L126
 interface IGnosisSafe {
@@ -12,6 +12,16 @@ interface IGnosisSafe {
     function getThreshold() external view returns (uint256);
 }
 
+/// @dev Zero value when it has to be different from zero.
+error ZeroValue();
+
+/// @dev Provided zero address.
+error ZeroAddress();
+
+/// @dev Multisig proxy bytecode is not whitelisted.
+/// @param multisig Address of a multisig proxy.
+error UnauthorizedMultisig(address multisig);
+
 /// @dev Provided incorrect data length.
 /// @param expected Expected minimum data length.
 /// @param provided Provided data length.
@@ -42,6 +52,31 @@ contract GnosisSafeSameAddressMultisig {
     // This exact size suggests that all the changes to the multisig have been performed and only validation is needed
     uint256 public constant DEFAULT_DATA_LENGTH = 20;
 
+    // Map of approved multisig proxy hashes
+    mapping(bytes32 => bool) public mapMultisigHashes;
+
+    /// @dev GnosisSafeSameAddressMultisig constructor.
+    /// @param _multisigProxyHashes Multisig proxy hashes.
+    constructor(bytes32[] memory _multisigProxyHashes) {
+        // There must be at least one multisig proxy hash
+        uint256 size = _multisigProxyHashes.length;
+        if (size == 0) {
+            revert ZeroValue();
+        }
+
+        // Record provided multisig proxy bytecode hashes
+        for (uint256 i = 0; i < size; ++i) {
+            bytes32 proxyHash = _multisigProxyHashes[i];
+            // Check for the zero hash
+            if (proxyHash == bytes32(0)) {
+                revert ZeroValue();
+            }
+
+            // Hash the proxy bytecode
+            mapMultisigHashes[proxyHash] = true;
+        }
+    }
+
     /// @dev Updates and/or verifies the existent gnosis safe multisig for changed owners and threshold.
     /// @notice This function operates with existent multisig proxy that is requested to be updated in terms of
     ///         the set of owners' addresses and the threshold. There are two scenarios possible:
@@ -75,6 +110,12 @@ contract GnosisSafeSameAddressMultisig {
             multisig := mload(add(data, DEFAULT_DATA_LENGTH))
         }
 
+        // Check that the multisig address corresponds to the authorized multisig proxy
+        bytes32 proxyHash = keccak256(multisig.code);
+        if (!mapMultisigHashes[proxyHash]) {
+            revert UnauthorizedMultisig(multisig);
+        }
+
         // If provided, read the payload that is going to change the multisig ownership and threshold
         // The payload is expected to be the `execTransaction()` function call with all its arguments and signature(s)
         if (dataLength > DEFAULT_DATA_LENGTH) {

test/GnosisSafeSameAddressMultisig.js

@@ -15,6 +15,7 @@ describe("GnosisSafeSameAddressMultisig", function () {
     const newThreshold = 3;
     const AddressZero = "0x" + "0".repeat(40);
     const maxUint256 = "115792089237316195423570985008687907853269984665640564039457584007913129639935";
+    let bytecodeHash;
 
     beforeEach(async function () {
         const GnosisSafe = await ethers.getContractFactory("GnosisSafe");
@@ -29,8 +30,14 @@ describe("GnosisSafeSameAddressMultisig", function () {
         multiSend = await MultiSend.deploy();
         await multiSend.deployed();
 
+        const GnosisSafeProxy = await ethers.getContractFactory("GnosisSafeProxy");
+        const gnosisSafeProxy = await GnosisSafeProxy.deploy(gnosisSafe.address);
+        await gnosisSafeProxy.deployed();
+        const bytecode = await ethers.provider.getCode(gnosisSafeProxy.address);
+        bytecodeHash = ethers.utils.keccak256(bytecode);
+
         const GnosisSafeSameAddressMultisig = await ethers.getContractFactory("GnosisSafeSameAddressMultisig");
-        gnosisSafeSameAddressMultisig = await GnosisSafeSameAddressMultisig.deploy();
+        gnosisSafeSameAddressMultisig = await GnosisSafeSameAddressMultisig.deploy([bytecodeHash]);
         await gnosisSafeSameAddressMultisig.deployed();
 
         signers = await ethers.getSigners();
@@ -39,6 +46,19 @@ describe("GnosisSafeSameAddressMultisig", function () {
     });
 
     context("Verifying multisigs", async function () {
+        it("Try to deploy a contract without specified proxies or zero hashes", async function () {
+            const GnosisSafeSameAddressMultisig = await ethers.getContractFactory("GnosisSafeSameAddressMultisig");
+
+            await expect(
+                GnosisSafeSameAddressMultisig.deploy([])
+            ).to.be.revertedWithCustomError(gnosisSafeSameAddressMultisig, "ZeroValue");
+
+            const bytes32Zero = "0x" + "0".repeat(64);
+            await expect(
+                GnosisSafeSameAddressMultisig.deploy([bytes32Zero])
+            ).to.be.revertedWithCustomError(gnosisSafeSameAddressMultisig, "ZeroValue");
+        });
+
         it("Should fail when passing the non-zero multisig data with the incorrect number of bytes", async function () {
             await expect(
                 gnosisSafeSameAddressMultisig.create(newOwnerAddresses, initialThreshold, "0x55")

test/ServiceManagementWithOperatorSignatures.js

@@ -30,6 +30,7 @@ describe("ServiceManagementWithOperatorSignatures", function () {
     const AddressZero = "0x" + "0".repeat(40);
     const ETHAddress = "0xEeeeeEeeeEeEeeEeEeEeeEEEeeeeEeeeeeeeEEeE";
     const payload = "0x";
+    let bytecodeHash;
 
     beforeEach(async function () {
         const ComponentRegistry = await ethers.getContractFactory("ComponentRegistry");
@@ -62,8 +63,14 @@ describe("ServiceManagementWithOperatorSignatures", function () {
         multiSend = await MultiSend.deploy();
         await multiSend.deployed();
 
+        const GnosisSafeProxy = await ethers.getContractFactory("GnosisSafeProxy");
+        const gnosisSafeProxy = await GnosisSafeProxy.deploy(gnosisSafe.address);
+        await gnosisSafeProxy.deployed();
+        const bytecode = await ethers.provider.getCode(gnosisSafeProxy.address);
+        bytecodeHash = ethers.utils.keccak256(bytecode);
+
         const GnosisSafeSameAddressMultisig = await ethers.getContractFactory("GnosisSafeSameAddressMultisig");
-        gnosisSafeSameAddressMultisig = await GnosisSafeSameAddressMultisig.deploy();
+        gnosisSafeSameAddressMultisig = await GnosisSafeSameAddressMultisig.deploy([bytecodeHash]);
         await gnosisSafeSameAddressMultisig.deployed();
 
         const ServiceRegistry = await ethers.getContractFactory("ServiceRegistry");

test/ServiceRegistry.js

@@ -40,6 +40,7 @@ describe("ServiceRegistry", function () {
     const AddressZero = "0x" + "0".repeat(40);
     const payload = "0x";
     const serviceRegistryImplementations = ["l1", "l2", "l1an"];
+    let bytecodeHash;
 
     beforeEach(async function () {
         const ComponentRegistry = await ethers.getContractFactory("ComponentRegistry");
@@ -72,8 +73,14 @@ describe("ServiceRegistry", function () {
         multiSend = await MultiSend.deploy();
         await multiSend.deployed();
 
+        const GnosisSafeProxy = await ethers.getContractFactory("GnosisSafeProxy");
+        const gnosisSafeProxy = await GnosisSafeProxy.deploy(gnosisSafe.address);
+        await gnosisSafeProxy.deployed();
+        const bytecode = await ethers.provider.getCode(gnosisSafeProxy.address);
+        bytecodeHash = ethers.utils.keccak256(bytecode);
+
         const GnosisSafeSameAddressMultisig = await ethers.getContractFactory("GnosisSafeSameAddressMultisig");
-        gnosisSafeSameAddressMultisig = await GnosisSafeSameAddressMultisig.deploy();
+        gnosisSafeSameAddressMultisig = await GnosisSafeSameAddressMultisig.deploy([bytecodeHash]);
         await gnosisSafeSameAddressMultisig.deployed();
 
         const ServiceRegistry = await ethers.getContractFactory("ServiceRegistry");

test/ServiceStaking.js

@@ -93,7 +93,7 @@ describe("ServiceStakingNativeToken", function () {
         bytecodeHash = ethers.utils.keccak256(bytecode);
 
         const GnosisSafeSameAddressMultisig = await ethers.getContractFactory("GnosisSafeSameAddressMultisig");
-        gnosisSafeSameAddressMultisig = await GnosisSafeSameAddressMultisig.deploy();
+        gnosisSafeSameAddressMultisig = await GnosisSafeSameAddressMultisig.deploy([bytecodeHash]);
         await gnosisSafeSameAddressMultisig.deployed();
 
         const MultiSend = await ethers.getContractFactory("MultiSendCallOnly");
@@ -1025,15 +1025,9 @@ describe("ServiceStakingNativeToken", function () {
 
             // Prepare the payload to redeploy with the attacker address
             const data = ethers.utils.solidityPack(["address"], [attacker.address]);
-            await serviceRegistry.deploy(deployer.address, serviceId, gnosisSafeSameAddressMultisig.address, data);
-
-            // Approve service
-            await serviceRegistry.approve(serviceStaking.address, serviceId);
-
-            // Try to stake the service
             await expect(
-                serviceStaking.stake(serviceId)
-            ).to.be.revertedWithCustomError(serviceStaking, "UnauthorizedMultisig");
+                serviceRegistry.deploy(deployer.address, serviceId, gnosisSafeSameAddressMultisig.address, data)
+            ).to.be.revertedWithCustomError(gnosisSafeSameAddressMultisig, "UnauthorizedMultisig");
 
             // Restore a previous state of blockchain
             snapshot.restore();

test/ServiceStaking.t.sol

@@ -12,7 +12,6 @@ import {ServiceRegistryTokenUtility} from "../contracts/ServiceRegistryTokenUtil
 import {ServiceManagerToken} from "../contracts/ServiceManagerToken.sol";
 import {OperatorWhitelist} from "../contracts/utils/OperatorWhitelist.sol";
 import {GnosisSafeMultisig} from "../contracts/multisigs/GnosisSafeMultisig.sol";
-import {GnosisSafeSameAddressMultisig} from "../contracts/multisigs/GnosisSafeSameAddressMultisig.sol";
 import "../contracts/staking/ServiceStakingNativeToken.sol";
 import {ServiceStakingToken} from "../contracts/staking/ServiceStakingToken.sol";
 import {SafeNonceLib} from "../contracts/test/SafeNonceLib.sol";
@@ -28,7 +27,6 @@ contract BaseSetup is Test {
     GnosisSafeProxy internal gnosisSafeProxy;
     GnosisSafeProxyFactory internal gnosisSafeProxyFactory;
     GnosisSafeMultisig internal gnosisSafeMultisig;
-    GnosisSafeSameAddressMultisig internal gnosisSafeSameAddressMultisig;
     ServiceStakingNativeToken internal serviceStakingNativeToken;
     ServiceStakingToken internal serviceStakingToken;
     SafeNonceLib internal safeNonceLib;
@@ -93,7 +91,6 @@ contract BaseSetup is Test {
         gnosisSafeProxy = new GnosisSafeProxy(address(gnosisSafe));
         gnosisSafeProxyFactory = new GnosisSafeProxyFactory();
         gnosisSafeMultisig = new GnosisSafeMultisig(payable(address(gnosisSafe)), address(gnosisSafeProxyFactory));
-        gnosisSafeSameAddressMultisig = new GnosisSafeSameAddressMultisig();
         safeNonceLib = new SafeNonceLib();
 
         // Deploying a token contract and minting to deployer, operator and a current contract
@@ -102,19 +99,20 @@ contract BaseSetup is Test {
         token.mint(operator, initialMint);
         token.mint(address(this), initialMint);
 
-        // Deploy service staking native token and arbitraty token
+        // Get the multisig proxy bytecode hash
+        bytes32[] memory multisigProxyHashes = new bytes32[](1);
+        multisigProxyHashes[0] = keccak256(address(gnosisSafeProxy).code);
+
+        // Deploy service staking native token and arbitrary ERC20 token
         ServiceStakingBase.StakingParams memory stakingParams = ServiceStakingBase.StakingParams(maxNumServices,
             rewardsPerSecond, minStakingDeposit, livenessPeriod, livenessRatio, numAgentInstances, emptyArray, 0, bytes32(0));
-        bytes32[] memory multisigProxyAddresses = new bytes32[](1);
-        multisigProxyAddresses[0] = keccak256(address(gnosisSafeProxy).code);
         serviceStakingNativeToken = new ServiceStakingNativeToken(stakingParams, address(serviceRegistry),
-            multisigProxyAddresses);
+            multisigProxyHashes);
         serviceStakingToken = new ServiceStakingToken(stakingParams, address(serviceRegistry), address(serviceRegistryTokenUtility),
-            address(token), multisigProxyAddresses);
+            address(token), multisigProxyHashes);
 
         // Whitelist multisig implementations
         serviceRegistry.changeMultisigPermission(address(gnosisSafeMultisig), true);
-        serviceRegistry.changeMultisigPermission(address(gnosisSafeSameAddressMultisig), true);
 
         IService.AgentParams[] memory agentParams = new IService.AgentParams[](1);
         agentParams[0].slots = 1;