From ae92b9f1eacc81b446b6a3f124b06a2a0bfbb60f Mon Sep 17 00:00:00 2001 From: Jack Chuma Date: Tue, 7 Jan 2025 18:33:49 -0500 Subject: [PATCH 1/4] add double nested multisig builder script --- .../universal/DoubleNestedMultisigBuilder.sol | 226 ++++++++++++++++++ 1 file changed, 226 insertions(+) create mode 100644 script/universal/DoubleNestedMultisigBuilder.sol diff --git a/script/universal/DoubleNestedMultisigBuilder.sol b/script/universal/DoubleNestedMultisigBuilder.sol new file mode 100644 index 0000000..f459d1d --- /dev/null +++ b/script/universal/DoubleNestedMultisigBuilder.sol @@ -0,0 +1,226 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.15; + +import "./MultisigBase.sol"; + +/** + * @title DoubleNestedMultisigBuilder + * @notice Modeled from Optimism's SafeBuilder, but built for double nested safes (Safes where the signers are other Safes with signers that are other Safes). + */ +abstract contract DoubleNestedMultisigBuilder is MultisigBase { + /** + * ----------------------------------------------------------- + * Virtual Functions + * ----------------------------------------------------------- + */ + + /** + * @notice Returns the nested safe address to execute the final transaction from + */ + function _ownerSafe() internal view virtual returns (address); + + /** + * @notice Creates the calldata for both signatures (`sign`) and execution (`run`) + */ + function _buildCalls() internal view virtual returns (IMulticall3.Call3[] memory); + + /** + * @notice Follow up assertions to ensure that the script ran to completion. + * @dev Called after `sign` and `run`, but not `approve`. + */ + function _postCheck(Vm.AccountAccess[] memory accesses, Simulation.Payload memory simPayload) internal virtual; + + /** + * @notice Follow up assertions on state and simulation after a `sign` call. + */ + function _postSign(Vm.AccountAccess[] memory accesses, Simulation.Payload memory simPayload) internal virtual {} + + /** + * @notice Follow up assertions on state and simulation after a `approve` call. + */ + function _postApprove(Vm.AccountAccess[] memory accesses, Simulation.Payload memory simPayload) internal virtual {} + + /** + * @notice Follow up assertions on state and simulation after a `run` call. + */ + function _postRun(Vm.AccountAccess[] memory accesses, Simulation.Payload memory simPayload) internal virtual {} + + /** + * ----------------------------------------------------------- + * Implemented Functions + * ----------------------------------------------------------- + */ + + /** + * Step 1 + * ====== + * Generate a transaction approval data to sign. This method should be called by a threshold + * of members of each of the multisigs involved in the nested multisig. Signers will pass + * their signature to a facilitator, who will execute the approval transaction for each + * multisig (see step 2). + */ + function sign(address _signerSafe) public { + address nestedSafe = _ownerSafe(); + + // Snapshot and restore Safe nonce after simulation, otherwise the data logged to sign + // would not match the actual data we need to sign, because the simulation + // would increment the nonce. + uint256 originalNonce = _getNonce(nestedSafe); + uint256 originalSignerNonce = _getNonce(_signerSafe); + + IMulticall3.Call3[] memory nestedCalls = _buildCalls(); + IMulticall3.Call3 memory call = _generateApproveCall(nestedSafe, nestedCalls); + + (Vm.AccountAccess[] memory accesses, Simulation.Payload memory simPayload) = + _simulateForSigner(_signerSafe, nestedSafe, nestedCalls); + _postSign(accesses, simPayload); + _postCheck(accesses, simPayload); + + // Restore the original nonce. + vm.store(nestedSafe, SAFE_NONCE_SLOT, bytes32(originalNonce)); + vm.store(_signerSafe, SAFE_NONCE_SLOT, bytes32(originalSignerNonce)); + + _printDataToSign(_signerSafe, _toArray(call)); + } + + /** + * Step 1.1 (optional) + * ====== + * Verify the signatures generated from step 1 are valid. + * This allow transactions to be pre-signed and stored safely before execution. + */ + function verify(address _signerSafe, bytes memory _signatures) public view { + IMulticall3.Call3[] memory nestedCalls = _buildCalls(); + IMulticall3.Call3 memory call = _generateApproveCall(_ownerSafe(), nestedCalls); + _checkSignatures(_signerSafe, _toArray(call), _signatures); + } + + /** + * Step 2 + * ====== + * Execute an approval transaction. This method should be called by a facilitator + * (non-signer), once for each of the multisigs involved in the nested multisig, + * after collecting a threshold of signatures for each multisig (see step 1). + */ + function approve(address _signerSafe, bytes memory _signatures) public { + IMulticall3.Call3[] memory nestedCalls = _buildCalls(); + IMulticall3.Call3 memory call = _generateApproveCall(_ownerSafe(), nestedCalls); + + (Vm.AccountAccess[] memory accesses, Simulation.Payload memory simPayload) = + _executeTransaction(_signerSafe, _toArray(call), _signatures, true); + + _postApprove(accesses, simPayload); + } + + /** + * Step 3 + * ====== + * Execute the transaction. This method should be called by a facilitator (non-signer), after + * all of the approval transactions have been submitted onchain (see step 2). + */ + function run() public { + IMulticall3.Call3[] memory nestedCalls = _buildCalls(); + + // signatures is empty, because `_executeTransaction` internally collects all of the approvedHash addresses + bytes memory signatures; + + (Vm.AccountAccess[] memory accesses, Simulation.Payload memory simPayload) = + _executeTransaction(_ownerSafe(), nestedCalls, signatures, true); + + _postRun(accesses, simPayload); + _postCheck(accesses, simPayload); + } + + function _readFrom_SAFE_NONCE() internal pure override returns (bool) { + return false; + } + + function _generateApproveCall(address _safe, IMulticall3.Call3[] memory _calls) + internal + view + returns (IMulticall3.Call3 memory) + { + bytes32 hash = _getTransactionHash(_safe, _calls); + + console.log("---\nNested hash:"); + console.logBytes32(hash); + + return IMulticall3.Call3({ + target: _safe, + allowFailure: false, + callData: abi.encodeCall(IGnosisSafe(_safe).approveHash, (hash)) + }); + } + + function _simulateForSigner(address _signerSafe, address _safe, IMulticall3.Call3[] memory _calls) + internal + returns (Vm.AccountAccess[] memory, Simulation.Payload memory) + { + bytes memory data = abi.encodeCall(IMulticall3.aggregate3, (_calls)); + IMulticall3.Call3[] memory calls = _simulateForSignerCalls(_signerSafe, _safe, data); + + // Now define the state overrides for the simulation. + Simulation.StateOverride[] memory overrides = _overrides(_signerSafe, _safe); + + bytes memory txData = abi.encodeCall(IMulticall3.aggregate3, (calls)); + console.log("---\nSimulation link:"); + Simulation.logSimulationLink({_to: MULTICALL3_ADDRESS, _data: txData, _from: msg.sender, _overrides: overrides}); + + // Forge simulation of the data logged in the link. If the simulation fails + // we revert to make it explicit that the simulation failed. + Simulation.Payload memory simPayload = + Simulation.Payload({to: MULTICALL3_ADDRESS, data: txData, from: msg.sender, stateOverrides: overrides}); + Vm.AccountAccess[] memory accesses = Simulation.simulateFromSimPayload(simPayload); + return (accesses, simPayload); + } + + function _simulateForSignerCalls(address _signerSafe, address _safe, bytes memory _data) + internal + view + returns (IMulticall3.Call3[] memory) + { + IMulticall3.Call3[] memory calls = new IMulticall3.Call3[](2); + bytes32 hash = _getTransactionHash(_safe, _data); + + // simulate an approveHash, so that signer can verify the data they are signing + bytes memory approveHashData = abi.encodeCall( + IMulticall3.aggregate3, + ( + _toArray( + IMulticall3.Call3({ + target: _safe, + allowFailure: false, + callData: abi.encodeCall(IGnosisSafe(_safe).approveHash, (hash)) + }) + ) + ) + ); + bytes memory approveHashExec = _execTransationCalldata( + _signerSafe, approveHashData, Signatures.genPrevalidatedSignature(MULTICALL3_ADDRESS) + ); + calls[0] = IMulticall3.Call3({target: _signerSafe, allowFailure: false, callData: approveHashExec}); + + // simulate the final state changes tx, so that signer can verify the final results + bytes memory finalExec = _execTransationCalldata(_safe, _data, Signatures.genPrevalidatedSignature(_signerSafe)); + calls[1] = IMulticall3.Call3({target: _safe, allowFailure: false, callData: finalExec}); + + return calls; + } + + function _overrides(address _signerSafe, address _safe) internal view returns (Simulation.StateOverride[] memory) { + Simulation.StateOverride[] memory simOverrides = _simulationOverrides(); + Simulation.StateOverride[] memory overrides = new Simulation.StateOverride[](2 + simOverrides.length); + overrides[0] = _safeOverrides(_signerSafe, MULTICALL3_ADDRESS); + overrides[1] = _safeOverrides(_safe, address(0)); + for (uint256 i = 0; i < simOverrides.length; i++) { + overrides[i + 2] = simOverrides[i]; + } + return overrides; + } + + function _toArray(IMulticall3.Call3 memory call) internal pure returns (IMulticall3.Call3[] memory) { + IMulticall3.Call3[] memory calls = new IMulticall3.Call3[](1); + calls[0] = call; + return calls; + } +} From e28229590c7affea879e7f0602190c16a2f4a4b2 Mon Sep 17 00:00:00 2001 From: Jack Chuma Date: Tue, 7 Jan 2025 21:47:58 -0500 Subject: [PATCH 2/4] implement necessary changes to support additional layer of multisig nesting --- .../universal/DoubleNestedMultisigBuilder.sol | 213 ++++++------------ script/universal/NestedMultisigBase.sol | 131 +++++++++++ .../DoubleNestedMultisigBuilder.t.sol | 149 ++++++++++++ 3 files changed, 346 insertions(+), 147 deletions(-) create mode 100644 script/universal/NestedMultisigBase.sol create mode 100644 test/universal/DoubleNestedMultisigBuilder.t.sol diff --git a/script/universal/DoubleNestedMultisigBuilder.sol b/script/universal/DoubleNestedMultisigBuilder.sol index f459d1d..b62a395 100644 --- a/script/universal/DoubleNestedMultisigBuilder.sol +++ b/script/universal/DoubleNestedMultisigBuilder.sol @@ -1,55 +1,22 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.15; -import "./MultisigBase.sol"; +import "./NestedMultisigBase.sol"; /** * @title DoubleNestedMultisigBuilder * @notice Modeled from Optimism's SafeBuilder, but built for double nested safes (Safes where the signers are other Safes with signers that are other Safes). */ -abstract contract DoubleNestedMultisigBuilder is MultisigBase { +abstract contract DoubleNestedMultisigBuilder is NestedMultisigBase { /** - * ----------------------------------------------------------- - * Virtual Functions - * ----------------------------------------------------------- + * @notice Returns the intermediate safe address to execute the final approval transaction from */ + function _intermediateSafe() internal view virtual returns (address); /** - * @notice Returns the nested safe address to execute the final transaction from - */ - function _ownerSafe() internal view virtual returns (address); - - /** - * @notice Creates the calldata for both signatures (`sign`) and execution (`run`) - */ - function _buildCalls() internal view virtual returns (IMulticall3.Call3[] memory); - - /** - * @notice Follow up assertions to ensure that the script ran to completion. - * @dev Called after `sign` and `run`, but not `approve`. - */ - function _postCheck(Vm.AccountAccess[] memory accesses, Simulation.Payload memory simPayload) internal virtual; - - /** - * @notice Follow up assertions on state and simulation after a `sign` call. - */ - function _postSign(Vm.AccountAccess[] memory accesses, Simulation.Payload memory simPayload) internal virtual {} - - /** - * @notice Follow up assertions on state and simulation after a `approve` call. - */ - function _postApprove(Vm.AccountAccess[] memory accesses, Simulation.Payload memory simPayload) internal virtual {} - - /** - * @notice Follow up assertions on state and simulation after a `run` call. - */ - function _postRun(Vm.AccountAccess[] memory accesses, Simulation.Payload memory simPayload) internal virtual {} - - /** - * ----------------------------------------------------------- - * Implemented Functions - * ----------------------------------------------------------- + * @notice Returns the signer safe address to execute the initial approval transaction from */ + function _signerSafe() internal view virtual returns (address); /** * Step 1 @@ -59,28 +26,34 @@ abstract contract DoubleNestedMultisigBuilder is MultisigBase { * their signature to a facilitator, who will execute the approval transaction for each * multisig (see step 2). */ - function sign(address _signerSafe) public { - address nestedSafe = _ownerSafe(); + function sign() public { + address signerSafe = _signerSafe(); + address intermediateSafe = _intermediateSafe(); + address topSafe = _ownerSafe(); // Snapshot and restore Safe nonce after simulation, otherwise the data logged to sign // would not match the actual data we need to sign, because the simulation // would increment the nonce. - uint256 originalNonce = _getNonce(nestedSafe); - uint256 originalSignerNonce = _getNonce(_signerSafe); + uint256 originalTopNonce = _getNonce(topSafe); + uint256 originalIntermediateNonce = _getNonce(intermediateSafe); + uint256 originalSignerNonce = _getNonce(signerSafe); - IMulticall3.Call3[] memory nestedCalls = _buildCalls(); - IMulticall3.Call3 memory call = _generateApproveCall(nestedSafe, nestedCalls); + IMulticall3.Call3[] memory dstCalls = _buildCalls(); + IMulticall3.Call3 memory topSafeApprovalCall = _generateApproveCall(topSafe, dstCalls); + IMulticall3.Call3 memory intermediateSafeApprovalCall = + _generateApproveCall(intermediateSafe, _toArray(topSafeApprovalCall)); (Vm.AccountAccess[] memory accesses, Simulation.Payload memory simPayload) = - _simulateForSigner(_signerSafe, nestedSafe, nestedCalls); + _simulateForSigner(intermediateSafe, topSafe, dstCalls); _postSign(accesses, simPayload); _postCheck(accesses, simPayload); // Restore the original nonce. - vm.store(nestedSafe, SAFE_NONCE_SLOT, bytes32(originalNonce)); - vm.store(_signerSafe, SAFE_NONCE_SLOT, bytes32(originalSignerNonce)); + vm.store(topSafe, SAFE_NONCE_SLOT, bytes32(originalTopNonce)); + vm.store(intermediateSafe, SAFE_NONCE_SLOT, bytes32(originalIntermediateNonce)); + vm.store(signerSafe, SAFE_NONCE_SLOT, bytes32(originalSignerNonce)); - _printDataToSign(_signerSafe, _toArray(call)); + _printDataToSign(signerSafe, _toArray(intermediateSafeApprovalCall)); } /** @@ -89,10 +62,11 @@ abstract contract DoubleNestedMultisigBuilder is MultisigBase { * Verify the signatures generated from step 1 are valid. * This allow transactions to be pre-signed and stored safely before execution. */ - function verify(address _signerSafe, bytes memory _signatures) public view { - IMulticall3.Call3[] memory nestedCalls = _buildCalls(); - IMulticall3.Call3 memory call = _generateApproveCall(_ownerSafe(), nestedCalls); - _checkSignatures(_signerSafe, _toArray(call), _signatures); + function verify(bytes memory _signatures) public view { + IMulticall3.Call3 memory topSafeApprovalCall = _generateApproveCall(_ownerSafe(), _buildCalls()); + IMulticall3.Call3 memory intermediateSafeApprovalCall = + _generateApproveCall(_intermediateSafe(), _toArray(topSafeApprovalCall)); + _checkSignatures(_signerSafe(), _toArray(intermediateSafeApprovalCall), _signatures); } /** @@ -102,14 +76,16 @@ abstract contract DoubleNestedMultisigBuilder is MultisigBase { * (non-signer), once for each of the multisigs involved in the nested multisig, * after collecting a threshold of signatures for each multisig (see step 1). */ - function approve(address _signerSafe, bytes memory _signatures) public { - IMulticall3.Call3[] memory nestedCalls = _buildCalls(); - IMulticall3.Call3 memory call = _generateApproveCall(_ownerSafe(), nestedCalls); + function approveInit(bytes memory _signatures) public { + IMulticall3.Call3[] memory dstCalls = _buildCalls(); + IMulticall3.Call3 memory topSafeApprovalCall = _generateApproveCall(_ownerSafe(), dstCalls); + IMulticall3.Call3 memory intermediateSafeApprovalCall = + _generateApproveCall(_intermediateSafe(), _toArray(topSafeApprovalCall)); (Vm.AccountAccess[] memory accesses, Simulation.Payload memory simPayload) = - _executeTransaction(_signerSafe, _toArray(call), _signatures, true); + _executeTransaction(_signerSafe(), _toArray(intermediateSafeApprovalCall), _signatures, true); - _postApprove(accesses, simPayload); + _postApprove(); } /** @@ -118,109 +94,52 @@ abstract contract DoubleNestedMultisigBuilder is MultisigBase { * Execute the transaction. This method should be called by a facilitator (non-signer), after * all of the approval transactions have been submitted onchain (see step 2). */ - function run() public { - IMulticall3.Call3[] memory nestedCalls = _buildCalls(); + function runInit() public { + IMulticall3.Call3[] memory dstCalls = _buildCalls(); + IMulticall3.Call3 memory topSafeApprovalCall = _generateApproveCall(_ownerSafe(), dstCalls); // signatures is empty, because `_executeTransaction` internally collects all of the approvedHash addresses bytes memory signatures; (Vm.AccountAccess[] memory accesses, Simulation.Payload memory simPayload) = - _executeTransaction(_ownerSafe(), nestedCalls, signatures, true); + _executeTransaction(_intermediateSafe(), _toArray(topSafeApprovalCall), signatures, true); - _postRun(accesses, simPayload); - _postCheck(accesses, simPayload); + _postRunInit(); } - function _readFrom_SAFE_NONCE() internal pure override returns (bool) { - return false; - } + function run() public { + IMulticall3.Call3[] memory dstCalls = _buildCalls(); - function _generateApproveCall(address _safe, IMulticall3.Call3[] memory _calls) - internal - view - returns (IMulticall3.Call3 memory) - { - bytes32 hash = _getTransactionHash(_safe, _calls); - - console.log("---\nNested hash:"); - console.logBytes32(hash); - - return IMulticall3.Call3({ - target: _safe, - allowFailure: false, - callData: abi.encodeCall(IGnosisSafe(_safe).approveHash, (hash)) - }); - } + // signatures is empty, because `_executeTransaction` internally collects all of the approvedHash addresses + bytes memory signatures; - function _simulateForSigner(address _signerSafe, address _safe, IMulticall3.Call3[] memory _calls) - internal - returns (Vm.AccountAccess[] memory, Simulation.Payload memory) - { - bytes memory data = abi.encodeCall(IMulticall3.aggregate3, (_calls)); - IMulticall3.Call3[] memory calls = _simulateForSignerCalls(_signerSafe, _safe, data); - - // Now define the state overrides for the simulation. - Simulation.StateOverride[] memory overrides = _overrides(_signerSafe, _safe); - - bytes memory txData = abi.encodeCall(IMulticall3.aggregate3, (calls)); - console.log("---\nSimulation link:"); - Simulation.logSimulationLink({_to: MULTICALL3_ADDRESS, _data: txData, _from: msg.sender, _overrides: overrides}); - - // Forge simulation of the data logged in the link. If the simulation fails - // we revert to make it explicit that the simulation failed. - Simulation.Payload memory simPayload = - Simulation.Payload({to: MULTICALL3_ADDRESS, data: txData, from: msg.sender, stateOverrides: overrides}); - Vm.AccountAccess[] memory accesses = Simulation.simulateFromSimPayload(simPayload); - return (accesses, simPayload); - } + (Vm.AccountAccess[] memory accesses, Simulation.Payload memory simPayload) = + _executeTransaction(_ownerSafe(), dstCalls, signatures, true); - function _simulateForSignerCalls(address _signerSafe, address _safe, bytes memory _data) - internal - view - returns (IMulticall3.Call3[] memory) - { - IMulticall3.Call3[] memory calls = new IMulticall3.Call3[](2); - bytes32 hash = _getTransactionHash(_safe, _data); - - // simulate an approveHash, so that signer can verify the data they are signing - bytes memory approveHashData = abi.encodeCall( - IMulticall3.aggregate3, - ( - _toArray( - IMulticall3.Call3({ - target: _safe, - allowFailure: false, - callData: abi.encodeCall(IGnosisSafe(_safe).approveHash, (hash)) - }) - ) - ) - ); - bytes memory approveHashExec = _execTransationCalldata( - _signerSafe, approveHashData, Signatures.genPrevalidatedSignature(MULTICALL3_ADDRESS) - ); - calls[0] = IMulticall3.Call3({target: _signerSafe, allowFailure: false, callData: approveHashExec}); - - // simulate the final state changes tx, so that signer can verify the final results - bytes memory finalExec = _execTransationCalldata(_safe, _data, Signatures.genPrevalidatedSignature(_signerSafe)); - calls[1] = IMulticall3.Call3({target: _safe, allowFailure: false, callData: finalExec}); - - return calls; + _postRun(accesses, simPayload); + _postCheck(accesses, simPayload); } - function _overrides(address _signerSafe, address _safe) internal view returns (Simulation.StateOverride[] memory) { - Simulation.StateOverride[] memory simOverrides = _simulationOverrides(); - Simulation.StateOverride[] memory overrides = new Simulation.StateOverride[](2 + simOverrides.length); - overrides[0] = _safeOverrides(_signerSafe, MULTICALL3_ADDRESS); - overrides[1] = _safeOverrides(_safe, address(0)); - for (uint256 i = 0; i < simOverrides.length; i++) { - overrides[i + 2] = simOverrides[i]; - } - return overrides; + /** + * @dev Follow up assertions on state and simulation after an `approve` call. + */ + function _postApprove() private view { + IMulticall3.Call3 memory topSafeApprovalCall = _generateApproveCall(_ownerSafe(), _buildCalls()); + bytes memory data = abi.encodeCall(IMulticall3.aggregate3, _toArray(topSafeApprovalCall)); + bytes32 approvedHash = _getTransactionHash(_intermediateSafe(), data); + + uint256 isApproved = IGnosisSafe(_intermediateSafe()).approvedHashes(_signerSafe(), approvedHash); + require(isApproved == 1, "DoubleNestedMultisigBuilder::_postApprove: Approval failed"); } - function _toArray(IMulticall3.Call3 memory call) internal pure returns (IMulticall3.Call3[] memory) { - IMulticall3.Call3[] memory calls = new IMulticall3.Call3[](1); - calls[0] = call; - return calls; + /** + * @dev Follow up assertions on state and simulation after an `approve` call. + */ + function _postRunInit() private view { + bytes memory data = abi.encodeCall(IMulticall3.aggregate3, _buildCalls()); + bytes32 approvedHash = _getTransactionHash(_ownerSafe(), data); + + uint256 isApproved = IGnosisSafe(_ownerSafe()).approvedHashes(_intermediateSafe(), approvedHash); + require(isApproved == 1, "DoubleNestedMultisigBuilder::_postApprove: Approval failed"); } } diff --git a/script/universal/NestedMultisigBase.sol b/script/universal/NestedMultisigBase.sol new file mode 100644 index 0000000..369368a --- /dev/null +++ b/script/universal/NestedMultisigBase.sol @@ -0,0 +1,131 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.15; + +import "./MultisigBase.sol"; + +abstract contract NestedMultisigBase is MultisigBase { + /** + * ----------------------------------------------------------- + * Virtual Functions + * ----------------------------------------------------------- + */ + + /** + * @notice Returns the nested safe address to execute the final transaction from + */ + function _ownerSafe() internal view virtual returns (address); + + /** + * @notice Creates the calldata for both signatures (`sign`) and execution (`run`) + */ + function _buildCalls() internal view virtual returns (IMulticall3.Call3[] memory); + + /** + * @notice Follow up assertions to ensure that the script ran to completion. + * @dev Called after `sign` and `run`, but not `approve`. + */ + function _postCheck(Vm.AccountAccess[] memory accesses, Simulation.Payload memory simPayload) internal virtual; + + /** + * @notice Follow up assertions on state and simulation after a `sign` call. + */ + function _postSign(Vm.AccountAccess[] memory accesses, Simulation.Payload memory simPayload) internal virtual {} + + /** + * @notice Follow up assertions on state and simulation after a `run` call. + */ + function _postRun(Vm.AccountAccess[] memory accesses, Simulation.Payload memory simPayload) internal virtual {} + + function _readFrom_SAFE_NONCE() internal pure override returns (bool) { + return false; + } + + function _generateApproveCall(address _safe, IMulticall3.Call3[] memory _calls) + internal + view + returns (IMulticall3.Call3 memory) + { + bytes32 hash = _getTransactionHash(_safe, _calls); + + console.log("---\nNested hash:"); + console.logBytes32(hash); + + return IMulticall3.Call3({ + target: _safe, + allowFailure: false, + callData: abi.encodeCall(IGnosisSafe(_safe).approveHash, (hash)) + }); + } + + function _simulateForSigner(address _signerSafe, address _safe, IMulticall3.Call3[] memory _calls) + internal + returns (Vm.AccountAccess[] memory, Simulation.Payload memory) + { + bytes memory data = abi.encodeCall(IMulticall3.aggregate3, (_calls)); + IMulticall3.Call3[] memory calls = _simulateForSignerCalls(_signerSafe, _safe, data); + + // Now define the state overrides for the simulation. + Simulation.StateOverride[] memory overrides = _overrides(_signerSafe, _safe); + + bytes memory txData = abi.encodeCall(IMulticall3.aggregate3, (calls)); + console.log("---\nSimulation link:"); + Simulation.logSimulationLink({_to: MULTICALL3_ADDRESS, _data: txData, _from: msg.sender, _overrides: overrides}); + + // Forge simulation of the data logged in the link. If the simulation fails + // we revert to make it explicit that the simulation failed. + Simulation.Payload memory simPayload = + Simulation.Payload({to: MULTICALL3_ADDRESS, data: txData, from: msg.sender, stateOverrides: overrides}); + Vm.AccountAccess[] memory accesses = Simulation.simulateFromSimPayload(simPayload); + return (accesses, simPayload); + } + + function _simulateForSignerCalls(address _signerSafe, address _safe, bytes memory _data) + internal + view + returns (IMulticall3.Call3[] memory) + { + IMulticall3.Call3[] memory calls = new IMulticall3.Call3[](2); + bytes32 hash = _getTransactionHash(_safe, _data); + + // simulate an approveHash, so that signer can verify the data they are signing + bytes memory approveHashData = abi.encodeCall( + IMulticall3.aggregate3, + ( + _toArray( + IMulticall3.Call3({ + target: _safe, + allowFailure: false, + callData: abi.encodeCall(IGnosisSafe(_safe).approveHash, (hash)) + }) + ) + ) + ); + bytes memory approveHashExec = _execTransationCalldata( + _signerSafe, approveHashData, Signatures.genPrevalidatedSignature(MULTICALL3_ADDRESS) + ); + calls[0] = IMulticall3.Call3({target: _signerSafe, allowFailure: false, callData: approveHashExec}); + + // simulate the final state changes tx, so that signer can verify the final results + bytes memory finalExec = _execTransationCalldata(_safe, _data, Signatures.genPrevalidatedSignature(_signerSafe)); + calls[1] = IMulticall3.Call3({target: _safe, allowFailure: false, callData: finalExec}); + + return calls; + } + + function _overrides(address _signerSafe, address _safe) internal view returns (Simulation.StateOverride[] memory) { + Simulation.StateOverride[] memory simOverrides = _simulationOverrides(); + Simulation.StateOverride[] memory overrides = new Simulation.StateOverride[](2 + simOverrides.length); + overrides[0] = _safeOverrides(_signerSafe, MULTICALL3_ADDRESS); + overrides[1] = _safeOverrides(_safe, address(0)); + for (uint256 i = 0; i < simOverrides.length; i++) { + overrides[i + 2] = simOverrides[i]; + } + return overrides; + } + + function _toArray(IMulticall3.Call3 memory call) internal pure returns (IMulticall3.Call3[] memory) { + IMulticall3.Call3[] memory calls = new IMulticall3.Call3[](1); + calls[0] = call; + return calls; + } +} diff --git a/test/universal/DoubleNestedMultisigBuilder.t.sol b/test/universal/DoubleNestedMultisigBuilder.t.sol new file mode 100644 index 0000000..b3de61c --- /dev/null +++ b/test/universal/DoubleNestedMultisigBuilder.t.sol @@ -0,0 +1,149 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.15; + +import {Test} from "forge-std/Test.sol"; +import {Vm} from "forge-std/Vm.sol"; +import {console} from "forge-std/console.sol"; +import {IMulticall3} from "forge-std/interfaces/IMulticall3.sol"; +import {Preinstalls} from "@eth-optimism-bedrock/src/libraries/Preinstalls.sol"; +import {DoubleNestedMultisigBuilder} from "../../script/universal/DoubleNestedMultisigBuilder.sol"; +import {Simulation} from "../../script/universal/Simulation.sol"; +import {IGnosisSafe} from "../../script/universal/IGnosisSafe.sol"; +import {Counter} from "./Counter.sol"; + +contract DoubleNestedMultisigBuilderTest is Test, DoubleNestedMultisigBuilder { + Vm.Wallet internal wallet1 = vm.createWallet("1"); + Vm.Wallet internal wallet2 = vm.createWallet("2"); + + address internal safe1 = address(1001); + address internal safe2 = address(1002); + address internal safe3 = address(1003); + address internal safe4 = address(1004); + address internal signerSafe = safe1; + Counter internal counter = new Counter(address(safe4)); + + bytes internal dataToSign1 = + hex"1901d4bb33110137810c444c1d9617abe97df097d587ecde64e6fcb38d7f49e1280c32a807b9689901dd0dbb7352e9e6c5265e3f6a68667de4be988f03f6a88511f7"; + bytes internal dataToSign2 = + hex"190132640243d7aade8c72f3d90d2dbf359e9897feba5fce1453bc8d9e7ba10d171532a807b9689901dd0dbb7352e9e6c5265e3f6a68667de4be988f03f6a88511f7"; + + function setUp() public { + bytes memory safeCode = Preinstalls.getDeployedCode(Preinstalls.Safe_v130, block.chainid); + vm.etch(safe1, safeCode); + vm.etch(safe2, safeCode); + vm.etch(safe3, safeCode); + vm.etch(safe4, safeCode); + vm.etch(Preinstalls.MultiCall3, Preinstalls.getDeployedCode(Preinstalls.MultiCall3, block.chainid)); + + address[] memory owners1 = new address[](1); + owners1[0] = wallet1.addr; + IGnosisSafe(safe1).setup(owners1, 1, address(0), "", address(0), address(0), 0, address(0)); + + address[] memory owners2 = new address[](1); + owners2[0] = wallet2.addr; + IGnosisSafe(safe2).setup(owners2, 1, address(0), "", address(0), address(0), 0, address(0)); + + address[] memory owners3 = new address[](2); + owners3[0] = safe1; + owners3[1] = safe2; + IGnosisSafe(safe3).setup(owners3, 2, address(0), "", address(0), address(0), 0, address(0)); + + address[] memory owners4 = new address[](1); + owners4[0] = safe3; + IGnosisSafe(safe4).setup(owners4, 1, address(0), "", address(0), address(0), 0, address(0)); + } + + function _postCheck(Vm.AccountAccess[] memory, Simulation.Payload memory) internal view override { + // Check that the counter has been incremented + uint256 counterValue = counter.count(); + require(counterValue == 1, "Counter value is not 1"); + } + + function _buildCalls() internal view override returns (IMulticall3.Call3[] memory) { + IMulticall3.Call3[] memory calls = new IMulticall3.Call3[](1); + + calls[0] = IMulticall3.Call3({ + target: address(counter), + allowFailure: false, + callData: abi.encodeCall(Counter.increment, ()) + }); + + return calls; + } + + function _ownerSafe() internal view override returns (address) { + return safe4; + } + + function _intermediateSafe() internal view override returns (address) { + return safe3; + } + + function _signerSafe() internal view override returns (address) { + return signerSafe; + } + + function test_sign_double_nested_safe1() external { + vm.recordLogs(); + sign(); + Vm.Log[] memory logs = vm.getRecordedLogs(); + assertEq(keccak256(logs[logs.length - 1].data), keccak256(abi.encode(dataToSign1))); + } + + function test_sign_double_nested_safe2() external { + signerSafe = safe2; + vm.recordLogs(); + sign(); + Vm.Log[] memory logs = vm.getRecordedLogs(); + assertEq(keccak256(logs[logs.length - 1].data), keccak256(abi.encode(dataToSign2))); + } + + function test_approveInit_double_nested_safe1() external { + (uint8 v, bytes32 r, bytes32 s) = vm.sign(wallet1, keccak256(dataToSign1)); + approveInit(abi.encodePacked(r, s, v)); + } + + function test_approveInit_double_nested_safe2() external { + signerSafe = safe2; + (uint8 v, bytes32 r, bytes32 s) = vm.sign(wallet2, keccak256(dataToSign2)); + approveInit(abi.encodePacked(r, s, v)); + } + + function test_approveInit_double_nested_notOwner() external { + signerSafe = safe2; + (uint8 v, bytes32 r, bytes32 s) = vm.sign(wallet1, keccak256(dataToSign1)); + bytes memory data = abi.encodeCall(this.approveInit, (abi.encodePacked(r, s, v))); + (bool success, bytes memory result) = address(this).call(data); + assertFalse(success); + assertEq(result, abi.encodeWithSignature("Error(string)", "not enough signatures")); + } + + function test_runInit_double_nested() external { + (uint8 v1, bytes32 r1, bytes32 s1) = vm.sign(wallet1, keccak256(dataToSign1)); + (uint8 v2, bytes32 r2, bytes32 s2) = vm.sign(wallet2, keccak256(dataToSign2)); + approveInit(abi.encodePacked(r1, s1, v1)); + signerSafe = safe2; + approveInit(abi.encodePacked(r2, s2, v2)); + runInit(); + } + + function test_runInit_double_nested_notApproved() external { + (uint8 v1, bytes32 r1, bytes32 s1) = vm.sign(wallet1, keccak256(dataToSign1)); + approveInit(abi.encodePacked(r1, s1, v1)); + bytes memory data = abi.encodeCall(this.runInit, ()); + (bool success, bytes memory result) = address(this).call(data); + assertFalse(success); + assertEq(result, abi.encodeWithSignature("Error(string)", "not enough signatures")); + } + + function test_run_double_nested() external { + (uint8 v1, bytes32 r1, bytes32 s1) = vm.sign(wallet1, keccak256(dataToSign1)); + (uint8 v2, bytes32 r2, bytes32 s2) = vm.sign(wallet2, keccak256(dataToSign2)); + approveInit(abi.encodePacked(r1, s1, v1)); + signerSafe = safe2; + approveInit(abi.encodePacked(r2, s2, v2)); + runInit(); + + run(); + } +} From c2c8b17da41c332c4c33ee5991e08dcdae150f33 Mon Sep 17 00:00:00 2001 From: Jack Chuma Date: Wed, 8 Jan 2025 21:27:04 -0500 Subject: [PATCH 3/4] refactor double nested multisig builder and update comments --- .../universal/DoubleNestedMultisigBuilder.sol | 115 ++++++++++-------- .../DoubleNestedMultisigBuilder.t.sol | 40 ++---- 2 files changed, 74 insertions(+), 81 deletions(-) diff --git a/script/universal/DoubleNestedMultisigBuilder.sol b/script/universal/DoubleNestedMultisigBuilder.sol index b62a395..9062f4e 100644 --- a/script/universal/DoubleNestedMultisigBuilder.sol +++ b/script/universal/DoubleNestedMultisigBuilder.sol @@ -5,30 +5,24 @@ import "./NestedMultisigBase.sol"; /** * @title DoubleNestedMultisigBuilder - * @notice Modeled from Optimism's SafeBuilder, but built for double nested safes (Safes where the signers are other Safes with signers that are other Safes). + * @notice Modeled from Optimism's SafeBuilder, but built for double nested safes (Safes where + * the signers are other Safes with signers that are other Safes). + * + * There are three safes involved in a double nested multisig: + * 1. The top-level safe, which should be returned by `_ownerSafe()`. + * 2. One or more intermediate safes, which are signers for the top-level safe. + * 3. Signer safes, which are signers for the intermediate safes. There should be at least one signer safe per intermediate safe. */ abstract contract DoubleNestedMultisigBuilder is NestedMultisigBase { - /** - * @notice Returns the intermediate safe address to execute the final approval transaction from - */ - function _intermediateSafe() internal view virtual returns (address); - - /** - * @notice Returns the signer safe address to execute the initial approval transaction from - */ - function _signerSafe() internal view virtual returns (address); - /** * Step 1 * ====== * Generate a transaction approval data to sign. This method should be called by a threshold - * of members of each of the multisigs involved in the nested multisig. Signers will pass + * of members of each of the signer safes involved in the nested multisig. Signers will pass * their signature to a facilitator, who will execute the approval transaction for each - * multisig (see step 2). + * signer safe (see step 2). */ - function sign() public { - address signerSafe = _signerSafe(); - address intermediateSafe = _intermediateSafe(); + function sign(address signerSafe, address intermediateSafe) public { address topSafe = _ownerSafe(); // Snapshot and restore Safe nonce after simulation, otherwise the data logged to sign @@ -38,13 +32,9 @@ abstract contract DoubleNestedMultisigBuilder is NestedMultisigBase { uint256 originalIntermediateNonce = _getNonce(intermediateSafe); uint256 originalSignerNonce = _getNonce(signerSafe); - IMulticall3.Call3[] memory dstCalls = _buildCalls(); - IMulticall3.Call3 memory topSafeApprovalCall = _generateApproveCall(topSafe, dstCalls); - IMulticall3.Call3 memory intermediateSafeApprovalCall = - _generateApproveCall(intermediateSafe, _toArray(topSafeApprovalCall)); - (Vm.AccountAccess[] memory accesses, Simulation.Payload memory simPayload) = - _simulateForSigner(intermediateSafe, topSafe, dstCalls); + _simulateForSigner(intermediateSafe, topSafe, _buildCalls()); + _postSign(accesses, simPayload); _postCheck(accesses, simPayload); @@ -53,7 +43,7 @@ abstract contract DoubleNestedMultisigBuilder is NestedMultisigBase { vm.store(intermediateSafe, SAFE_NONCE_SLOT, bytes32(originalIntermediateNonce)); vm.store(signerSafe, SAFE_NONCE_SLOT, bytes32(originalSignerNonce)); - _printDataToSign(signerSafe, _toArray(intermediateSafeApprovalCall)); + _printDataToSign(signerSafe, _generateIntermediateSafeApprovalCall(intermediateSafe)); } /** @@ -62,59 +52,60 @@ abstract contract DoubleNestedMultisigBuilder is NestedMultisigBase { * Verify the signatures generated from step 1 are valid. * This allow transactions to be pre-signed and stored safely before execution. */ - function verify(bytes memory _signatures) public view { - IMulticall3.Call3 memory topSafeApprovalCall = _generateApproveCall(_ownerSafe(), _buildCalls()); - IMulticall3.Call3 memory intermediateSafeApprovalCall = - _generateApproveCall(_intermediateSafe(), _toArray(topSafeApprovalCall)); - _checkSignatures(_signerSafe(), _toArray(intermediateSafeApprovalCall), _signatures); + function verify(address signerSafe, address intermediateSafe, bytes memory signatures) public view { + IMulticall3.Call3[] memory calls = _generateIntermediateSafeApprovalCall(intermediateSafe); + _checkSignatures(signerSafe, calls, signatures); } /** * Step 2 * ====== - * Execute an approval transaction. This method should be called by a facilitator - * (non-signer), once for each of the multisigs involved in the nested multisig, - * after collecting a threshold of signatures for each multisig (see step 1). + * Execute an approval transaction for a signer safe. This method should be called by a facilitator + * (non-signer), once for each of the signer safes involved in the nested multisig, + * after collecting a threshold of signatures for each signer safe (see step 1). */ - function approveInit(bytes memory _signatures) public { - IMulticall3.Call3[] memory dstCalls = _buildCalls(); - IMulticall3.Call3 memory topSafeApprovalCall = _generateApproveCall(_ownerSafe(), dstCalls); - IMulticall3.Call3 memory intermediateSafeApprovalCall = - _generateApproveCall(_intermediateSafe(), _toArray(topSafeApprovalCall)); + function approveInit(address signerSafe, address intermediateSafe, bytes memory signatures) public { + IMulticall3.Call3[] memory calls = _generateIntermediateSafeApprovalCall(intermediateSafe); (Vm.AccountAccess[] memory accesses, Simulation.Payload memory simPayload) = - _executeTransaction(_signerSafe(), _toArray(intermediateSafeApprovalCall), _signatures, true); + _executeTransaction(signerSafe, calls, signatures, true); - _postApprove(); + _postApprove(signerSafe, intermediateSafe); } /** * Step 3 * ====== - * Execute the transaction. This method should be called by a facilitator (non-signer), after - * all of the approval transactions have been submitted onchain (see step 2). + * Execute an approval transaction for an intermediate safe. This method should be called by a + * facilitator (non-signer), for each of the intermediate safes after all of their approval + * transactions have been submitted onchain by their signer safes (see step 2). */ - function runInit() public { - IMulticall3.Call3[] memory dstCalls = _buildCalls(); - IMulticall3.Call3 memory topSafeApprovalCall = _generateApproveCall(_ownerSafe(), dstCalls); + function runInit(address intermediateSafe) public { + IMulticall3.Call3[] memory calls = _generateTopSafeApprovalCall(); // signatures is empty, because `_executeTransaction` internally collects all of the approvedHash addresses bytes memory signatures; (Vm.AccountAccess[] memory accesses, Simulation.Payload memory simPayload) = - _executeTransaction(_intermediateSafe(), _toArray(topSafeApprovalCall), signatures, true); + _executeTransaction(intermediateSafe, calls, signatures, true); - _postRunInit(); + _postRunInit(intermediateSafe); } + /** + * Step 4 + * ====== + * Execute the final transaction. This method should be called by a facilitator (non-signer), after + * all of the intermediate safe approval transactions have been submitted onchain (see step 3). + */ function run() public { - IMulticall3.Call3[] memory dstCalls = _buildCalls(); + IMulticall3.Call3[] memory calls = _buildCalls(); // signatures is empty, because `_executeTransaction` internally collects all of the approvedHash addresses bytes memory signatures; (Vm.AccountAccess[] memory accesses, Simulation.Payload memory simPayload) = - _executeTransaction(_ownerSafe(), dstCalls, signatures, true); + _executeTransaction(_ownerSafe(), calls, signatures, true); _postRun(accesses, simPayload); _postCheck(accesses, simPayload); @@ -123,23 +114,39 @@ abstract contract DoubleNestedMultisigBuilder is NestedMultisigBase { /** * @dev Follow up assertions on state and simulation after an `approve` call. */ - function _postApprove() private view { + function _postApprove(address signerSafe, address intermediateSafe) private view { IMulticall3.Call3 memory topSafeApprovalCall = _generateApproveCall(_ownerSafe(), _buildCalls()); bytes memory data = abi.encodeCall(IMulticall3.aggregate3, _toArray(topSafeApprovalCall)); - bytes32 approvedHash = _getTransactionHash(_intermediateSafe(), data); + bytes32 approvedHash = _getTransactionHash(intermediateSafe, data); - uint256 isApproved = IGnosisSafe(_intermediateSafe()).approvedHashes(_signerSafe(), approvedHash); + uint256 isApproved = IGnosisSafe(intermediateSafe).approvedHashes(signerSafe, approvedHash); require(isApproved == 1, "DoubleNestedMultisigBuilder::_postApprove: Approval failed"); } /** - * @dev Follow up assertions on state and simulation after an `approve` call. + * @dev Follow up assertions on state and simulation after an `init` call. */ - function _postRunInit() private view { + function _postRunInit(address intermediateSafe) private view { bytes memory data = abi.encodeCall(IMulticall3.aggregate3, _buildCalls()); bytes32 approvedHash = _getTransactionHash(_ownerSafe(), data); - uint256 isApproved = IGnosisSafe(_ownerSafe()).approvedHashes(_intermediateSafe(), approvedHash); - require(isApproved == 1, "DoubleNestedMultisigBuilder::_postApprove: Approval failed"); + uint256 isApproved = IGnosisSafe(_ownerSafe()).approvedHashes(intermediateSafe, approvedHash); + require(isApproved == 1, "DoubleNestedMultisigBuilder::_postRunInit: Init transaction failed"); + } + + function _generateIntermediateSafeApprovalCall(address intermediateSafe) + private + view + returns (IMulticall3.Call3[] memory) + { + IMulticall3.Call3[] memory topCalls = _generateTopSafeApprovalCall(); + IMulticall3.Call3 memory intermediateCall = _generateApproveCall(intermediateSafe, topCalls); + return _toArray(intermediateCall); + } + + function _generateTopSafeApprovalCall() private view returns (IMulticall3.Call3[] memory) { + IMulticall3.Call3[] memory dstCalls = _buildCalls(); + IMulticall3.Call3 memory topSafeApprovalCall = _generateApproveCall(_ownerSafe(), dstCalls); + return _toArray(topSafeApprovalCall); } } diff --git a/test/universal/DoubleNestedMultisigBuilder.t.sol b/test/universal/DoubleNestedMultisigBuilder.t.sol index b3de61c..cb316b8 100644 --- a/test/universal/DoubleNestedMultisigBuilder.t.sol +++ b/test/universal/DoubleNestedMultisigBuilder.t.sol @@ -19,7 +19,6 @@ contract DoubleNestedMultisigBuilderTest is Test, DoubleNestedMultisigBuilder { address internal safe2 = address(1002); address internal safe3 = address(1003); address internal safe4 = address(1004); - address internal signerSafe = safe1; Counter internal counter = new Counter(address(safe4)); bytes internal dataToSign1 = @@ -75,44 +74,33 @@ contract DoubleNestedMultisigBuilderTest is Test, DoubleNestedMultisigBuilder { return safe4; } - function _intermediateSafe() internal view override returns (address) { - return safe3; - } - - function _signerSafe() internal view override returns (address) { - return signerSafe; - } - function test_sign_double_nested_safe1() external { vm.recordLogs(); - sign(); + sign(safe1, safe3); Vm.Log[] memory logs = vm.getRecordedLogs(); assertEq(keccak256(logs[logs.length - 1].data), keccak256(abi.encode(dataToSign1))); } function test_sign_double_nested_safe2() external { - signerSafe = safe2; vm.recordLogs(); - sign(); + sign(safe2, safe3); Vm.Log[] memory logs = vm.getRecordedLogs(); assertEq(keccak256(logs[logs.length - 1].data), keccak256(abi.encode(dataToSign2))); } function test_approveInit_double_nested_safe1() external { (uint8 v, bytes32 r, bytes32 s) = vm.sign(wallet1, keccak256(dataToSign1)); - approveInit(abi.encodePacked(r, s, v)); + approveInit(safe1, safe3, abi.encodePacked(r, s, v)); } function test_approveInit_double_nested_safe2() external { - signerSafe = safe2; (uint8 v, bytes32 r, bytes32 s) = vm.sign(wallet2, keccak256(dataToSign2)); - approveInit(abi.encodePacked(r, s, v)); + approveInit(safe2, safe3, abi.encodePacked(r, s, v)); } function test_approveInit_double_nested_notOwner() external { - signerSafe = safe2; (uint8 v, bytes32 r, bytes32 s) = vm.sign(wallet1, keccak256(dataToSign1)); - bytes memory data = abi.encodeCall(this.approveInit, (abi.encodePacked(r, s, v))); + bytes memory data = abi.encodeCall(this.approveInit, (safe2, safe3, abi.encodePacked(r, s, v))); (bool success, bytes memory result) = address(this).call(data); assertFalse(success); assertEq(result, abi.encodeWithSignature("Error(string)", "not enough signatures")); @@ -121,16 +109,15 @@ contract DoubleNestedMultisigBuilderTest is Test, DoubleNestedMultisigBuilder { function test_runInit_double_nested() external { (uint8 v1, bytes32 r1, bytes32 s1) = vm.sign(wallet1, keccak256(dataToSign1)); (uint8 v2, bytes32 r2, bytes32 s2) = vm.sign(wallet2, keccak256(dataToSign2)); - approveInit(abi.encodePacked(r1, s1, v1)); - signerSafe = safe2; - approveInit(abi.encodePacked(r2, s2, v2)); - runInit(); + approveInit(safe1, safe3, abi.encodePacked(r1, s1, v1)); + approveInit(safe2, safe3, abi.encodePacked(r2, s2, v2)); + runInit(safe3); } function test_runInit_double_nested_notApproved() external { (uint8 v1, bytes32 r1, bytes32 s1) = vm.sign(wallet1, keccak256(dataToSign1)); - approveInit(abi.encodePacked(r1, s1, v1)); - bytes memory data = abi.encodeCall(this.runInit, ()); + approveInit(safe1, safe3, abi.encodePacked(r1, s1, v1)); + bytes memory data = abi.encodeCall(this.runInit, (safe3)); (bool success, bytes memory result) = address(this).call(data); assertFalse(success); assertEq(result, abi.encodeWithSignature("Error(string)", "not enough signatures")); @@ -139,10 +126,9 @@ contract DoubleNestedMultisigBuilderTest is Test, DoubleNestedMultisigBuilder { function test_run_double_nested() external { (uint8 v1, bytes32 r1, bytes32 s1) = vm.sign(wallet1, keccak256(dataToSign1)); (uint8 v2, bytes32 r2, bytes32 s2) = vm.sign(wallet2, keccak256(dataToSign2)); - approveInit(abi.encodePacked(r1, s1, v1)); - signerSafe = safe2; - approveInit(abi.encodePacked(r2, s2, v2)); - runInit(); + approveInit(safe1, safe3, abi.encodePacked(r1, s1, v1)); + approveInit(safe2, safe3, abi.encodePacked(r2, s2, v2)); + runInit(safe3); run(); } From c4f3ae4d21fd5e3cdf361dd51aeb803a63263064 Mon Sep 17 00:00:00 2001 From: Jack Chuma Date: Thu, 9 Jan 2025 15:01:13 -0500 Subject: [PATCH 4/4] improve double nested multisig builder function names --- .../universal/DoubleNestedMultisigBuilder.sol | 20 +++++++---------- .../DoubleNestedMultisigBuilder.t.sol | 22 +++++++++---------- 2 files changed, 19 insertions(+), 23 deletions(-) diff --git a/script/universal/DoubleNestedMultisigBuilder.sol b/script/universal/DoubleNestedMultisigBuilder.sol index 9062f4e..aa1149e 100644 --- a/script/universal/DoubleNestedMultisigBuilder.sol +++ b/script/universal/DoubleNestedMultisigBuilder.sol @@ -11,7 +11,8 @@ import "./NestedMultisigBase.sol"; * There are three safes involved in a double nested multisig: * 1. The top-level safe, which should be returned by `_ownerSafe()`. * 2. One or more intermediate safes, which are signers for the top-level safe. - * 3. Signer safes, which are signers for the intermediate safes. There should be at least one signer safe per intermediate safe. + * 3. Signer safes, which are signers for the intermediate safes. + * There should be at least one signer safe per intermediate safe. */ abstract contract DoubleNestedMultisigBuilder is NestedMultisigBase { /** @@ -64,12 +65,11 @@ abstract contract DoubleNestedMultisigBuilder is NestedMultisigBase { * (non-signer), once for each of the signer safes involved in the nested multisig, * after collecting a threshold of signatures for each signer safe (see step 1). */ - function approveInit(address signerSafe, address intermediateSafe, bytes memory signatures) public { + function approveOnBehalfOfSignerSafe(address signerSafe, address intermediateSafe, bytes memory signatures) + public + { IMulticall3.Call3[] memory calls = _generateIntermediateSafeApprovalCall(intermediateSafe); - - (Vm.AccountAccess[] memory accesses, Simulation.Payload memory simPayload) = - _executeTransaction(signerSafe, calls, signatures, true); - + _executeTransaction(signerSafe, calls, signatures, true); _postApprove(signerSafe, intermediateSafe); } @@ -80,15 +80,11 @@ abstract contract DoubleNestedMultisigBuilder is NestedMultisigBase { * facilitator (non-signer), for each of the intermediate safes after all of their approval * transactions have been submitted onchain by their signer safes (see step 2). */ - function runInit(address intermediateSafe) public { + function approveOnBehalfOfIntermediateSafe(address intermediateSafe) public { IMulticall3.Call3[] memory calls = _generateTopSafeApprovalCall(); - // signatures is empty, because `_executeTransaction` internally collects all of the approvedHash addresses bytes memory signatures; - - (Vm.AccountAccess[] memory accesses, Simulation.Payload memory simPayload) = - _executeTransaction(intermediateSafe, calls, signatures, true); - + _executeTransaction(intermediateSafe, calls, signatures, true); _postRunInit(intermediateSafe); } diff --git a/test/universal/DoubleNestedMultisigBuilder.t.sol b/test/universal/DoubleNestedMultisigBuilder.t.sol index cb316b8..77e4f0a 100644 --- a/test/universal/DoubleNestedMultisigBuilder.t.sol +++ b/test/universal/DoubleNestedMultisigBuilder.t.sol @@ -90,17 +90,17 @@ contract DoubleNestedMultisigBuilderTest is Test, DoubleNestedMultisigBuilder { function test_approveInit_double_nested_safe1() external { (uint8 v, bytes32 r, bytes32 s) = vm.sign(wallet1, keccak256(dataToSign1)); - approveInit(safe1, safe3, abi.encodePacked(r, s, v)); + approveOnBehalfOfSignerSafe(safe1, safe3, abi.encodePacked(r, s, v)); } function test_approveInit_double_nested_safe2() external { (uint8 v, bytes32 r, bytes32 s) = vm.sign(wallet2, keccak256(dataToSign2)); - approveInit(safe2, safe3, abi.encodePacked(r, s, v)); + approveOnBehalfOfSignerSafe(safe2, safe3, abi.encodePacked(r, s, v)); } function test_approveInit_double_nested_notOwner() external { (uint8 v, bytes32 r, bytes32 s) = vm.sign(wallet1, keccak256(dataToSign1)); - bytes memory data = abi.encodeCall(this.approveInit, (safe2, safe3, abi.encodePacked(r, s, v))); + bytes memory data = abi.encodeCall(this.approveOnBehalfOfSignerSafe, (safe2, safe3, abi.encodePacked(r, s, v))); (bool success, bytes memory result) = address(this).call(data); assertFalse(success); assertEq(result, abi.encodeWithSignature("Error(string)", "not enough signatures")); @@ -109,15 +109,15 @@ contract DoubleNestedMultisigBuilderTest is Test, DoubleNestedMultisigBuilder { function test_runInit_double_nested() external { (uint8 v1, bytes32 r1, bytes32 s1) = vm.sign(wallet1, keccak256(dataToSign1)); (uint8 v2, bytes32 r2, bytes32 s2) = vm.sign(wallet2, keccak256(dataToSign2)); - approveInit(safe1, safe3, abi.encodePacked(r1, s1, v1)); - approveInit(safe2, safe3, abi.encodePacked(r2, s2, v2)); - runInit(safe3); + approveOnBehalfOfSignerSafe(safe1, safe3, abi.encodePacked(r1, s1, v1)); + approveOnBehalfOfSignerSafe(safe2, safe3, abi.encodePacked(r2, s2, v2)); + approveOnBehalfOfIntermediateSafe(safe3); } function test_runInit_double_nested_notApproved() external { (uint8 v1, bytes32 r1, bytes32 s1) = vm.sign(wallet1, keccak256(dataToSign1)); - approveInit(safe1, safe3, abi.encodePacked(r1, s1, v1)); - bytes memory data = abi.encodeCall(this.runInit, (safe3)); + approveOnBehalfOfSignerSafe(safe1, safe3, abi.encodePacked(r1, s1, v1)); + bytes memory data = abi.encodeCall(this.approveOnBehalfOfIntermediateSafe, (safe3)); (bool success, bytes memory result) = address(this).call(data); assertFalse(success); assertEq(result, abi.encodeWithSignature("Error(string)", "not enough signatures")); @@ -126,9 +126,9 @@ contract DoubleNestedMultisigBuilderTest is Test, DoubleNestedMultisigBuilder { function test_run_double_nested() external { (uint8 v1, bytes32 r1, bytes32 s1) = vm.sign(wallet1, keccak256(dataToSign1)); (uint8 v2, bytes32 r2, bytes32 s2) = vm.sign(wallet2, keccak256(dataToSign2)); - approveInit(safe1, safe3, abi.encodePacked(r1, s1, v1)); - approveInit(safe2, safe3, abi.encodePacked(r2, s2, v2)); - runInit(safe3); + approveOnBehalfOfSignerSafe(safe1, safe3, abi.encodePacked(r1, s1, v1)); + approveOnBehalfOfSignerSafe(safe2, safe3, abi.encodePacked(r2, s2, v2)); + approveOnBehalfOfIntermediateSafe(safe3); run(); }