Setting multisig guards via Guard factory

contracts/multisigs/SafeGuard.sol

@@ -1,6 +1,10 @@
 // SPDX-License-Identifier: MIT
 pragma solidity ^0.8.19;
 
+/// @dev The contract instance is already initialized.
+/// @param owner Contract instance owner address.
+error Initialized(address owner);
+
 /// @dev Only `owner` has a privilege, but the `sender` was provided.
 /// @param sender Sender address.
 /// @param owner Required sender address as an owner.
@@ -14,27 +18,32 @@ error Guarded();
 
 /// @title SafeGuard - Smart contract for Gnosis Safe Guard functionality
 /// @author Aleksandr Kuperman - <[email protected]>
+/// @author AL
 contract SafeGuard {
     event OwnerUpdated(address indexed owner);
-    event ChangedGuard(address guard);
-    // keccak256("guard_manager.guard.address")
-    bytes32 internal constant GUARD_STORAGE_SLOT = 0x4a204f620c8c5ccdca3fd54d003badd85ba500436a431f0cbda4f558c93c34c8;
 
     // Safe enum
     enum Operation {Call, DelegateCall}
-
     // Contract owner
     address public owner;
 
-    /// @dev Guard constructor.
-    /// @param _owner Guard owner address.
-    constructor (address _owner) {
+    /// @dev Initializes the Safe Guard instance by setting its owner.
+    /// @param _owner Safe Guard owner address.
+    function initialize (address _owner) external {
+        if (owner != address(0)) {
+            revert Initialized(owner);
+        }
+
+        // Check for the zero address
+        if (_owner == address(0)) {
+            revert ZeroAddress();
+        }
         owner = _owner;
     }
 
     /// @dev Changes the owner address.
     /// @param newOwner Address of a new owner.
-    function changeOwner(address newOwner) external virtual {
+    function changeOwner(address newOwner) external {
         // Check for the ownership
         if (msg.sender != owner) {
             revert OwnerOnly(msg.sender, owner);
@@ -73,16 +82,4 @@ contract SafeGuard {
     function checkAfterExecution(bytes32 txHash, bool success) external {
 
     }
-
-    /// @dev Sets a guard that checks transactions before and execution.
-    /// @notice This function copies the corresponding Safe function such that it is correctly initialized.
-    /// @param guard The address of the guard to be used or the 0 address to disable the guard.
-    function setGuardForSafe(address guard) external {
-        bytes32 slot = GUARD_STORAGE_SLOT;
-        // solhint-disable-next-line no-inline-assembly
-        assembly {
-            sstore(slot, guard)
-        }
-        emit ChangedGuard(guard);
-    }
 }

contracts/multisigs/SafeGuardFactory.sol

@@ -0,0 +1,144 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.19;
+
+import "./SafeGuard.sol";
+
+// Safe Guard interface
+interface ISafeGuard {
+    /// @dev Initializes the Safe Guard instance by setting its owner.
+    /// @param _owner Safe Guard owner address.
+    function initialize (address _owner) external;
+}
+
+// Gnosis Safe interface
+interface IGnosisSafe {
+    /// @dev Returns array of owners.
+    /// @return Array of Safe owners.
+    function getOwners() external view returns (address[] memory);
+}
+
+/// @dev Safe Guard already exists.
+/// @param guard Safe Guard address.
+error SafeGuardALreadyExists(address guard);
+
+/// @dev The call must be strictly made via a delegatecall.
+/// @param instance Guard Factory instance.
+error DelegateCallOnly(address instance);
+
+/// @dev Number of owners is insufficient.
+/// @param provided Provided number of owners.
+/// @param expected Minimum expected number of owners.
+error InsufficientNumberOfOwners(uint256 provided, uint256 expected);
+
+/// @dev Provided incorrect data length.
+/// @param expected Expected minimum data length.
+/// @param provided Provided data length.
+error IncorrectPayloadLength(uint256 expected, uint256 provided);
+
+/// @dev Payload delegatecall transaction failed.
+/// @param payload Payload data.
+error PayloadExecFailed(bytes payload);
+
+/// @dev Failed to create a Safe Guard contract for the caller.
+/// @param caller Caller address.
+error FailedCreateSafeGuard(address caller);
+
+/// @title SafeGuardFactory - Smart contract for deployment of Safe Guard contracts via the Factory functionality
+/// @author Aleksandr Kuperman - <[email protected]>
+contract SafeGuardFactory {
+    event ChangedGuard(address indexed guard);
+
+    // keccak256("guard_manager.guard.address")
+    bytes32 internal constant GUARD_STORAGE_SLOT = 0x4a204f620c8c5ccdca3fd54d003badd85ba500436a431f0cbda4f558c93c34c8;
+    // Default payload length to be processed via a delegatecall
+    // If not empty, it has to contain at least an address (20 bytes) and a call signature (4 bytes)
+    uint256 public constant DEFAULT_PAYLOAD_LENGTH = 24;
+    // The deployed factory instance address
+    address public immutable factoryInstance;
+
+    /// @dev Guard Factory constructor.
+    constructor () {
+        factoryInstance = address(this);
+    }
+
+    /// @dev Creates a safe guard that checks transactions before and after safe transaction executions.
+    /// @notice This function must be called via a delegatecall only.
+    /// @notice The function follows the standard Safe Guard creation routine where it sets the guard address
+    ///         into its dedicated guard slot followed by the event emit defined in the GuardManager contract.
+    /// @notice A multisig has to have at least three owners to set up a safe guard.
+    /// @param payload Custom payload containing at least address and a function signature for a delegatecall.
+    function createSafeGuard(bytes memory payload) external {
+        // Check for the delegatecall
+        if (address(this) == factoryInstance) {
+            revert DelegateCallOnly(factoryInstance);
+        }
+
+        // Get the number of Safe owners
+        address[] memory owners = IGnosisSafe(address(this)).getOwners();
+        // Check for the number of owners
+        uint256 numOwners = owners.length;
+        if (numOwners < 3) {
+            revert InsufficientNumberOfOwners(numOwners, 3);
+        }
+
+        // Check for the payload length
+        uint256 payloadLength = payload.length;
+        if (payloadLength > 0 && payloadLength < DEFAULT_PAYLOAD_LENGTH) {
+            revert IncorrectPayloadLength(DEFAULT_PAYLOAD_LENGTH, payloadLength);
+        }
+
+        // Check that the guard slot is empty
+        bytes32 slot = GUARD_STORAGE_SLOT;
+        address guard;
+        // solhint-disable-next-line no-inline-assembly
+        assembly {
+            guard := sload(slot)
+        }
+        if (guard != address(0)) {
+            revert SafeGuardALreadyExists(guard);
+        }
+
+        // Execute the payload, if provided
+        // The payload must be executed before the safe guard is set, as the payload could potentially break the guard
+        if (payloadLength > 0) {
+            address target;
+            bool success;
+            // solhint-disable-next-line no-inline-assembly
+            assembly {
+                // First 20 bytes of the payload is address
+                target := mload(add(payload, 20))
+                // Offset 52 bytes from the payload start: initial 32 bytes plus the target address 20 bytes
+                success := delegatecall(gas(), target, add(payload, 52), mload(payload), 0, 0)
+            }
+            if (!success) {
+                revert PayloadExecFailed(payload);
+            }
+        }
+
+        // Bytecode of the Safe Guard contract
+        bytes memory bytecode = abi.encodePacked(type(SafeGuard).creationCode);
+        // Supposedly the address(this) is the unique address of a safe contract that calls this guard creation function
+        // Supposedly the same Safe contract is not going to create a guard, delete it and try to create again
+        // in the same block. Thus, the combination of the Safe address and the block timestamp will be unique
+        bytes32 salt = keccak256(abi.encodePacked(address(this), block.timestamp));
+
+        // Create Safe Guard instance and write it into its designated slot
+        // solhint-disable-next-line no-inline-assembly
+        assembly {
+            guard := create2(0, add(bytecode, 0x20), mload(bytecode), salt)
+        }
+        // Check for the guard creation
+        if(guard == address(0)) {
+            revert FailedCreateSafeGuard(address(this));
+        }
+
+        // Initialize the guard by setting its owner as the last one from the list of owners
+        ISafeGuard(guard).initialize(owners[numOwners - 1]);
+        // Write guard address into the guard slot
+        // solhint-disable-next-line no-inline-assembly
+        assembly {
+            sstore(slot, guard)
+        }
+        emit ChangedGuard(guard);
+    }
+}

contracts/test/DelegateCall.sol

@@ -0,0 +1,19 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.19;
+
+contract DelegateCall {
+    event SetValue(uint256 value);
+
+    // keccak256("delegatecall.test.slot")
+    bytes32 internal constant DELEGATE_CALL_SLOT = 0xa8636cd0c6eb2f8df5b1a59f8747fe760deace77ea3ee8215e95b2d8962a700e;
+
+    function setSlotValue(uint256 value) external {
+        bytes32 slot = DELEGATE_CALL_SLOT;
+        // Write guard value into the guard slot
+        // solhint-disable-next-line no-inline-assembly
+        assembly {
+            sstore(slot, value)
+        }
+        emit SetValue(value);
+    }
+}

test/GnosisSafeMultisig.js

@@ -7,6 +7,8 @@ describe("GnosisSafeMultisig", function () {
     let gnosisSafe;
     let gnosisSafeProxyFactory;
     let gnosisSafeMultisig;
+    let guardFactory;
+    let delegateCall;
     let signers;
     let defaultOwnerAddresses;
     const threshold = 2;
@@ -26,6 +28,14 @@ describe("GnosisSafeMultisig", function () {
         gnosisSafeMultisig = await GnosisSafeMultisig.deploy(gnosisSafe.address, gnosisSafeProxyFactory.address);
         await gnosisSafeMultisig.deployed();
 
+        const GuardFactory = await ethers.getContractFactory("SafeGuardFactory");
+        guardFactory = await GuardFactory.deploy();
+        await guardFactory.deployed();
+
+        const DelegateCall = await ethers.getContractFactory("DelegateCall");
+        delegateCall = await DelegateCall.deploy();
+        await delegateCall.deployed();
+
         signers = await ethers.getSigners();
         defaultOwnerAddresses = [signers[1].address, signers[2].address, signers[3].address];
     });
@@ -133,19 +143,14 @@ describe("GnosisSafeMultisig", function () {
             expect(await multisig.getThreshold()).to.equal(threshold);
         });
 
-        it("Setting a guard contract address that guards transactions for not being able to transfer ETH funds", async function () {
-            // Set up a guard contract with the last Safe owner being the guard owner
-            const SafeGuard = await ethers.getContractFactory("SafeGuard");
-            const safeGuard = await SafeGuard.deploy(signers[2].address);
-            await safeGuard.deployed();
-
-            const to = safeGuard.address;
+        it("Setting a safe guard (no additional payload) that guards transactions for not being able to transfer ETH funds", async function () {
+            const to = guardFactory.address;
             const fallbackHandler = AddressZero;
             const paymentToken = AddressZero;
             const paymentReceiver = AddressZero;
             const payment = 0;
             let nonce = 0;
-            const payload = safeGuard.interface.encodeFunctionData("setGuardForSafe", [safeGuard.address]);
+            const payload = guardFactory.interface.encodeFunctionData("createSafeGuard", ["0x"]);
             // Pack the data
             const data = ethers.utils.solidityPack(["address", "address", "address", "address", "uint256", "uint256", "bytes"],
                 [to, fallbackHandler, paymentToken, paymentReceiver, payment, nonce, payload]);
@@ -172,8 +177,66 @@ describe("GnosisSafeMultisig", function () {
             const guardSlot = "0x4a204f620c8c5ccdca3fd54d003badd85ba500436a431f0cbda4f558c93c34c8";
             let slotValue = await ethers.provider.getStorageAt(multisig.address, guardSlot);
             slotValue = ethers.utils.hexlify(ethers.utils.stripZeros(slotValue));
-            const safeGuardAddress = safeGuard.address.toLowerCase();
-            expect(slotValue).to.equal(safeGuardAddress);
+            // TODO Check the correct guard address
+            expect(slotValue).to.not.equal(AddressZero);
+            //const guardFactoryAddress = guardFactory.address.toLowerCase();
+            //expect(slotValue).to.equal(guardFactoryAddress);
+
+            // Send funds to the multisig address
+            await signers[0].sendTransaction({to: multisig.address, value: ethers.utils.parseEther("1000")});
+
+            // Try to send ETH funds from the multisig using first two Safe owners while facing the guard check
+            const safeContracts = require("@gnosis.pm/safe-contracts");
+            nonce = await multisig.nonce();
+            // The function call is irrelevant, we just need to pass the value
+            let txHashData = await safeContracts.buildContractCall(multisig, "nonce", [], nonce, 0, 0);
+            txHashData.value = ethers.utils.parseEther("10");
+            const signMessageData = [await safeContracts.safeSignMessage(signers[1], multisig, txHashData, 0),
+                await safeContracts.safeSignMessage(signers[2], multisig, txHashData, 0)];
+            await expect(
+                safeContracts.executeTx(multisig, txHashData, signMessageData, 0)
+            ).to.be.revertedWith("Guarded");
+        });
+
+        it("Setting a safe guard (with additional payload) that guards transactions for not being able to transfer ETH funds", async function () {
+            const to = guardFactory.address;
+            const fallbackHandler = AddressZero;
+            const paymentToken = AddressZero;
+            const paymentReceiver = AddressZero;
+            const payment = 0;
+            let nonce = 0;
+            // Pass a custom payload to a safe guard contract for the delegatecall from the DelegateCall contract
+            const customValue = 100;
+            const customData = delegateCall.interface.encodeFunctionData("setSlotValue", [customValue]);
+            const customPayload = ethers.utils.solidityPack(["address", "bytes"], [delegateCall.address, customData]);
+            // Assemble a payload from the customPayload to pass to the createSafeGuard function
+            const payload = guardFactory.interface.encodeFunctionData("createSafeGuard", [customPayload]);
+            // Pack the data
+            const data = ethers.utils.solidityPack(["address", "address", "address", "address", "uint256", "uint256", "bytes"],
+                [to, fallbackHandler, paymentToken, paymentReceiver, payment, nonce, payload]);
+
+            // Create a multisig with the packed data
+            const tx = await gnosisSafeMultisig.create(defaultOwnerAddresses, threshold, data);
+            const result = await tx.wait();
+
+            // Check that the obtained address is not zero
+            expect(result.events[0].address).to.not.equal(AddressZero);
+            const proxyAddress = result.events[0].address;
+
+            // Get the safe multisig instance
+            const multisig = await ethers.getContractAt("GnosisSafe", proxyAddress);
+
+            // Check the multisig owners and threshold
+            const owners = await multisig.getOwners();
+            for (let i = 0; i < defaultOwnerAddresses.length; i++) {
+                expect(owners[i]).to.equal(defaultOwnerAddresses[i]);
+            }
+            expect(await multisig.getThreshold()).to.equal(threshold);
+
+            // Check the delegatecall slot
+            const delegateCallSlot = "0xa8636cd0c6eb2f8df5b1a59f8747fe760deace77ea3ee8215e95b2d8962a700e";
+            let slotValue = await ethers.provider.getStorageAt(multisig.address, delegateCallSlot);
+            expect(Number(slotValue)).to.equal(customValue);
 
             // Send funds to the multisig address
             await signers[0].sendTransaction({to: multisig.address, value: ethers.utils.parseEther("1000")});