From 83b5ff998f496b34b8b1172fe5606ff7e717afcb Mon Sep 17 00:00:00 2001 From: Michael de Hoog Date: Fri, 18 Oct 2024 13:40:33 -1000 Subject: [PATCH 01/18] Fix support for EIP-1271 --- script/universal/MultisigBase.sol | 70 ++++++++++++++++++---- script/universal/MultisigBuilder.sol | 2 +- script/universal/NestedMultisigBuilder.sol | 39 ++---------- 3 files changed, 65 insertions(+), 46 deletions(-) diff --git a/script/universal/MultisigBase.sol b/script/universal/MultisigBase.sol index 7bcbdb8..2e5604f 100644 --- a/script/universal/MultisigBase.sol +++ b/script/universal/MultisigBase.sol @@ -83,9 +83,7 @@ abstract contract MultisigBase is Simulator { IGnosisSafe safe = IGnosisSafe(payable(_safe)); bytes memory data = abi.encodeCall(IMulticall3.aggregate3, (_calls)); bytes32 hash = _getTransactionHash(_safe, data); - - // safe requires all signatures to be unique, and sorted ascending by public key - _signatures = sortUniqueSignatures(_signatures, hash, safe.getThreshold()); + _signatures = prepareSignatures(_safe, hash, _signatures); safe.checkSignatures({ dataHash: hash, @@ -101,9 +99,7 @@ abstract contract MultisigBase is Simulator { IGnosisSafe safe = IGnosisSafe(payable(_safe)); bytes memory data = abi.encodeCall(IMulticall3.aggregate3, (_calls)); bytes32 hash = _getTransactionHash(_safe, data); - - // safe requires all signatures to be unique, and sorted ascending by public key - _signatures = sortUniqueSignatures(_signatures, hash, safe.getThreshold()); + _signatures = prepareSignatures(_safe, hash, _signatures); logSimulationLink({ _to: _safe, @@ -170,24 +166,71 @@ abstract contract MultisigBase is Simulator { return calls; } - function prevalidatedSignatures(address[] memory _addresses) internal pure returns (bytes memory) { + function prepareSignatures(address _safe, bytes32 hash, bytes memory _signatures) internal view returns (bytes memory) { + // prepend the prevalidated signatures to the signatures + address[] memory approvers = _getApprovers(_safe, hash); + bytes memory prevalidatedSignatures = genPrevalidatedSignatures(approvers); + _signatures = bytes.concat(prevalidatedSignatures, _signatures); + + // safe requires all signatures to be unique, and sorted ascending by public key + return sortUniqueSignatures(_signatures, hash, IGnosisSafe(_safe).getThreshold(), prevalidatedSignatures.length); + } + + function genPrevalidatedSignatures(address[] memory _addresses) internal pure returns (bytes memory) { LibSort.sort(_addresses); bytes memory signatures; for (uint256 i; i < _addresses.length; i++) { - signatures = bytes.concat(signatures, prevalidatedSignature(_addresses[i])); + signatures = bytes.concat(signatures, genPrevalidatedSignature(_addresses[i])); } return signatures; } - function prevalidatedSignature(address _address) internal pure returns (bytes memory) { + function genPrevalidatedSignature(address _address) internal pure returns (bytes memory) { uint8 v = 1; bytes32 s = bytes32(0); bytes32 r = bytes32(uint256(uint160(_address))); return abi.encodePacked(r, s, v); } - // see https://github.com/safe-global/safe-smart-account/blob/1ed486bb148fe40c26be58d1b517cec163980027/contracts/Safe.sol#L265-L334 - function sortUniqueSignatures(bytes memory _signatures, bytes32 dataHash, uint256 threshold) internal pure returns (bytes memory) { + function _getApprovers(address _safe, bytes32 hash) internal view returns (address[] memory) { + IGnosisSafe safe = IGnosisSafe(payable(_safe)); + + // get a list of owners that have approved this transaction + uint256 threshold = safe.getThreshold(); + address[] memory owners = safe.getOwners(); + address[] memory approvers = new address[](threshold); + uint256 approverIndex; + for (uint256 i; i < owners.length; i++) { + address owner = owners[i]; + uint256 approved = safe.approvedHashes(owner, hash); + if (approved == 1) { + approvers[approverIndex] = owner; + approverIndex++; + if (approverIndex == threshold) { + return approvers; + } + } + } + address[] memory subset = new address[](approverIndex); + for (uint256 i; i < approverIndex; i++) { + subset[i] = approvers[i]; + } + return subset; + } + + /** + * @notice Sorts the signatures in ascending order of the signer's address, and removes any duplicates. + * @dev see https://github.com/safe-global/safe-smart-account/blob/1ed486bb148fe40c26be58d1b517cec163980027/contracts/Safe.sol#L265-L334 + * @param _signatures Signature data that should be verified. + * Can be packed ECDSA signature ({bytes32 r}{bytes32 s}{uint8 v}), contract signature (EIP-1271) or approved hash. + * Can be suffixed with EIP-1271 signatures after threshold*65 bytes. + * @param dataHash Hash that is signed. + * @param threshold Number of signatures required to approve the transaction. + * @param dynamicOffset Offset to add to the `s` value of any EIP-1271 signature. + * Can be used to accomodate any additional signatures prepended to the array. + * If prevalidated signatures were prepended, this should be the length of those signatures. + */ + function sortUniqueSignatures(bytes memory _signatures, bytes32 dataHash, uint256 threshold, uint256 dynamicOffset) internal pure returns (bytes memory) { bytes memory sorted; uint256 count = uint256(_signatures.length / 0x41); uint256[] memory addressesAndIndexes = new uint256[](threshold); @@ -228,6 +271,11 @@ abstract contract MultisigBase is Simulator { for (uint256 i; i < count; i++) { uint256 index = addressesAndIndexes[i] & 0xffffffff; (v, r, s) = signatureSplit(_signatures, index); + if (v == 0) { + // The `s` value is used by safe as a lookup into the signature bytes. + // Increment by the offset so that the lookup location remains correct. + s = bytes32(uint256(s) + dynamicOffset); + } sorted = bytes.concat(sorted, abi.encodePacked(r, s, v)); } diff --git a/script/universal/MultisigBuilder.sol b/script/universal/MultisigBuilder.sol index 211da89..c7dadde 100644 --- a/script/universal/MultisigBuilder.sol +++ b/script/universal/MultisigBuilder.sol @@ -134,7 +134,7 @@ abstract contract MultisigBuilder is MultisigBase { 0, address(0), payable(address(0)), - prevalidatedSignature(msg.sender) + genPrevalidatedSignature(msg.sender) ) ); diff --git a/script/universal/NestedMultisigBuilder.sol b/script/universal/NestedMultisigBuilder.sol index ec1bae2..10aeabf 100644 --- a/script/universal/NestedMultisigBuilder.sol +++ b/script/universal/NestedMultisigBuilder.sol @@ -111,9 +111,6 @@ abstract contract NestedMultisigBuilder is MultisigBase { IMulticall3.Call3[] memory nestedCalls = _buildCalls(); IMulticall3.Call3 memory call = _generateApproveCall(nestedSafeAddress, nestedCalls); - address[] memory approvers = _getApprovers(_signerSafe, toArray(call)); - _signatures = bytes.concat(_signatures, prevalidatedSignatures(approvers)); - vm.startBroadcast(); (Vm.AccountAccess[] memory accesses, SimulationPayload memory simPayload) = _executeTransaction(_signerSafe, toArray(call), _signatures); vm.stopBroadcast(); @@ -130,8 +127,9 @@ abstract contract NestedMultisigBuilder is MultisigBase { function run() public { address nestedSafeAddress = _ownerSafe(); IMulticall3.Call3[] memory nestedCalls = _buildCalls(); - address[] memory approvers = _getApprovers(nestedSafeAddress, nestedCalls); - bytes memory signatures = prevalidatedSignatures(approvers); + + // signatures is empty, because `_executeTransaction` internally collects all of the approvedHash addresses + bytes memory signatures; vm.startBroadcast(); (Vm.AccountAccess[] memory accesses, SimulationPayload memory simPayload) = _executeTransaction(nestedSafeAddress, nestedCalls, signatures); @@ -154,33 +152,6 @@ abstract contract NestedMultisigBuilder is MultisigBase { }); } - function _getApprovers(address _safe, IMulticall3.Call3[] memory _calls) internal view returns (address[] memory) { - IGnosisSafe safe = IGnosisSafe(payable(_safe)); - bytes32 hash = _getTransactionHash(_safe, _calls); - - // get a list of owners that have approved this transaction - uint256 threshold = safe.getThreshold(); - address[] memory owners = safe.getOwners(); - address[] memory approvers = new address[](threshold); - uint256 approverIndex; - for (uint256 i; i < owners.length; i++) { - address owner = owners[i]; - uint256 approved = safe.approvedHashes(owner, hash); - if (approved == 1) { - approvers[approverIndex] = owner; - approverIndex++; - if (approverIndex == threshold) { - return approvers; - } - } - } - address[] memory subset = new address[](approverIndex); - for (uint256 i; i < approverIndex; i++) { - subset[i] = approvers[i]; - } - return subset; - } - function _simulateForSigner(address _signerSafe, address _safe, IMulticall3.Call3[] memory _calls) internal returns (Vm.AccountAccess[] memory, SimulationPayload memory) @@ -211,7 +182,7 @@ abstract contract NestedMultisigBuilder is MultisigBase { 0, address(0), payable(address(0)), - prevalidatedSignature(address(multicall)) + genPrevalidatedSignature(address(multicall)) ) ); calls[0] = IMulticall3.Call3({ @@ -233,7 +204,7 @@ abstract contract NestedMultisigBuilder is MultisigBase { 0, address(0), payable(address(0)), - prevalidatedSignature(_signerSafe) + genPrevalidatedSignature(_signerSafe) ) ); calls[1] = IMulticall3.Call3({ From 3d1981d0b00e05f98025a729bd26c7c3a09e383c Mon Sep 17 00:00:00 2001 From: Michael de Hoog Date: Fri, 18 Oct 2024 19:38:05 -1000 Subject: [PATCH 02/18] Reduce stack and remove viaIR --- foundry.toml | 1 - script/universal/MultisigBase.sol | 107 +++++++++++---------- script/universal/NestedMultisigBuilder.sol | 89 +++++++---------- 3 files changed, 91 insertions(+), 106 deletions(-) diff --git a/foundry.toml b/foundry.toml index bdc785c..10799e9 100644 --- a/foundry.toml +++ b/foundry.toml @@ -4,6 +4,5 @@ fs_permissions = [ {access = "read-write", path = "./"} ] optimizer = true optimizer_runs = 999999 solc_version = "0.8.15" -viaIR = true # See more config options https://github.com/foundry-rs/foundry/tree/master/config \ No newline at end of file diff --git a/script/universal/MultisigBase.sol b/script/universal/MultisigBase.sol index 2e5604f..4005c22 100644 --- a/script/universal/MultisigBase.sol +++ b/script/universal/MultisigBase.sol @@ -92,6 +92,39 @@ abstract contract MultisigBase is Simulator { }); } + function _encodeCall(IGnosisSafe _safe, bytes memory _data, bytes memory _signatures) internal pure returns (bytes memory) { + return abi.encodeCall( + _safe.execTransaction, + ( + address(multicall), + 0, + _data, + Enum.Operation.DelegateCall, + 0, + 0, + 0, + address(0), + payable(address(0)), + _signatures + ) + ); + } + + function _execTransaction(IGnosisSafe _safe, bytes memory _data, bytes memory _signatures) internal returns (bool) { + return _safe.execTransaction({ + to: address(multicall), + value: 0, + data: _data, + operation: Enum.Operation.DelegateCall, + safeTxGas: 0, + baseGas: 0, + gasPrice: 0, + gasToken: address(0), + refundReceiver: payable(address(0)), + signatures: _signatures + }); + } + function _executeTransaction(address _safe, IMulticall3.Call3[] memory _calls, bytes memory _signatures) internal returns (Vm.AccountAccess[] memory, SimulationPayload memory) @@ -101,39 +134,15 @@ abstract contract MultisigBase is Simulator { bytes32 hash = _getTransactionHash(_safe, data); _signatures = prepareSignatures(_safe, hash, _signatures); + bytes memory simData = _encodeCall(safe, data, _signatures); logSimulationLink({ _to: _safe, _from: msg.sender, - _data: abi.encodeCall( - safe.execTransaction, - ( - address(multicall), - 0, - data, - Enum.Operation.DelegateCall, - 0, - 0, - 0, - address(0), - payable(address(0)), - _signatures - ) - ) + _data: simData }); vm.startStateDiffRecording(); - bool success = safe.execTransaction({ - to: address(multicall), - value: 0, - data: data, - operation: Enum.Operation.DelegateCall, - safeTxGas: 0, - baseGas: 0, - gasPrice: 0, - gasToken: address(0), - refundReceiver: payable(address(0)), - signatures: _signatures - }); + bool success = _execTransaction(safe, data, _signatures); Vm.AccountAccess[] memory accesses = vm.stopAndReturnStateDiff(); require(success, "MultisigBase::_executeTransaction: Transaction failed"); require(accesses.length > 0, "MultisigBase::_executeTransaction: No state changes"); @@ -143,18 +152,7 @@ abstract contract MultisigBase is Simulator { SimulationPayload memory simPayload = SimulationPayload({ from: msg.sender, to: address(safe), - data: abi.encodeCall(safe.execTransaction, ( - address(multicall), - 0, - data, - Enum.Operation.DelegateCall, - 0, - 0, - 0, - address(0), - payable(address(0)), - _signatures - )), + data: simData, stateOverrides: new SimulationStateOverride[](0) }); return (accesses, simPayload); @@ -241,15 +239,7 @@ abstract contract MultisigBase is Simulator { uint256 j; for (uint256 i; i < count; i++) { (v, r, s) = signatureSplit(_signatures, i); - address owner; - if (v <= 1) { - owner = address(uint160(uint256(r))); - } else if (v > 30) { - owner = - ecrecover(keccak256(abi.encodePacked("\x19Ethereum Signed Message:\n32", dataHash)), v - 4, r, s); - } else { - owner = ecrecover(dataHash, v, r, s); - } + address owner = extractOwner(dataHash, r, s, v); // skip duplicate owners uint256 k; @@ -281,13 +271,28 @@ abstract contract MultisigBase is Simulator { // append the non-static part of the signatures (can contain EIP-1271 signatures if contracts are signers) // if there were any duplicates detected above, they will be safely ignored by Safe's checkNSignatures method - if (_signatures.length > sorted.length) { - sorted = bytes.concat(sorted, Bytes.slice(_signatures, sorted.length, _signatures.length - sorted.length)); - } + sorted = appendRemainingBytes(sorted, _signatures); return sorted; } + function appendRemainingBytes(bytes memory a1, bytes memory a2) internal pure returns (bytes memory) { + if (a2.length > a1.length) { + a1 = bytes.concat(a1, Bytes.slice(a2, a1.length, a2.length - a1.length)); + } + return a1; + } + + function extractOwner(bytes32 dataHash, bytes32 r, bytes32 s, uint8 v) internal pure returns (address) { + if (v <= 1) { + return address(uint160(uint256(r))); + } + if (v > 30) { + return ecrecover(keccak256(abi.encodePacked("\x19Ethereum Signed Message:\n32", dataHash)), v - 4, r, s); + } + return ecrecover(dataHash, v, r, s); + } + // see https://github.com/safe-global/safe-contracts/blob/1ed486bb148fe40c26be58d1b517cec163980027/contracts/common/SignatureDecoder.sol function signatureSplit(bytes memory signatures, uint256 pos) internal diff --git a/script/universal/NestedMultisigBuilder.sol b/script/universal/NestedMultisigBuilder.sol index 10aeabf..51ec796 100644 --- a/script/universal/NestedMultisigBuilder.sol +++ b/script/universal/NestedMultisigBuilder.sol @@ -156,62 +156,10 @@ abstract contract NestedMultisigBuilder is MultisigBase { internal returns (Vm.AccountAccess[] memory, SimulationPayload memory) { - IGnosisSafe safe = IGnosisSafe(payable(_safe)); IGnosisSafe signerSafe = IGnosisSafe(payable(_signerSafe)); + IGnosisSafe safe = IGnosisSafe(payable(_safe)); bytes memory data = abi.encodeCall(IMulticall3.aggregate3, (_calls)); - bytes32 hash = _getTransactionHash(_safe, data); - IMulticall3.Call3[] memory calls = new IMulticall3.Call3[](2); - - // 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(safe.approveHash, (hash)) - }) - ))); - bytes memory approveHashExec = abi.encodeCall( - signerSafe.execTransaction, - ( - address(multicall), - 0, - approveHashData, - Enum.Operation.DelegateCall, - 0, - 0, - 0, - address(0), - payable(address(0)), - genPrevalidatedSignature(address(multicall)) - ) - ); - 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 = abi.encodeCall( - safe.execTransaction, - ( - address(multicall), - 0, - data, - Enum.Operation.DelegateCall, - 0, - 0, - 0, - address(0), - payable(address(0)), - genPrevalidatedSignature(_signerSafe) - ) - ); - calls[1] = IMulticall3.Call3({ - target: _safe, - allowFailure: false, - callData: finalExec - }); + IMulticall3.Call3[] memory calls = _simulateForSignerCalls(signerSafe, safe, data); // For each safe, determine if a nonce override is needed. At this point, // no state overrides (i.e. vm.store) have been applied to the Foundry VM, @@ -261,4 +209,37 @@ abstract contract NestedMultisigBuilder is MultisigBase { Vm.AccountAccess[] memory accesses = simulateFromSimPayload(simPayload); return (accesses, simPayload); } + + function _simulateForSignerCalls(IGnosisSafe _signerSafe, IGnosisSafe _safe, bytes memory _data) + internal view + returns (IMulticall3.Call3[] memory) + { + IMulticall3.Call3[] memory calls = new IMulticall3.Call3[](2); + bytes32 hash = _getTransactionHash(address(_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: address(_safe), + allowFailure: false, + callData: abi.encodeCall(_safe.approveHash, (hash)) + }) + ))); + bytes memory approveHashExec = _encodeCall(_signerSafe, approveHashData, genPrevalidatedSignature(address(multicall))); + calls[0] = IMulticall3.Call3({ + target: address(_signerSafe), + allowFailure: false, + callData: approveHashExec + }); + + // simulate the final state changes tx, so that signer can verify the final results + bytes memory finalExec = _encodeCall(_safe, _data, genPrevalidatedSignature(address(_signerSafe))); + calls[1] = IMulticall3.Call3({ + target: address(_safe), + allowFailure: false, + callData: finalExec + }); + + return calls; + } } From 467c2afcf653c3441e7b18e687fdd37953cb2511 Mon Sep 17 00:00:00 2001 From: Michael de Hoog Date: Sat, 19 Oct 2024 19:14:16 -1000 Subject: [PATCH 03/18] Simplify tenderly env var access --- script/universal/Simulator.sol | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/script/universal/Simulator.sol b/script/universal/Simulator.sol index beeaf96..39e7c32 100644 --- a/script/universal/Simulator.sol +++ b/script/universal/Simulator.sol @@ -127,15 +127,8 @@ abstract contract Simulator is CommonBase { } function logSimulationLink(address _to, bytes memory _data, address _from, SimulationStateOverride[] memory _overrides) public view { - (, bytes memory projData) = VM_ADDRESS.staticcall( - abi.encodeWithSignature("envOr(string,string)", "TENDERLY_PROJECT", "TENDERLY_PROJECT") - ); - string memory proj = abi.decode(projData, (string)); - - (, bytes memory userData) = VM_ADDRESS.staticcall( - abi.encodeWithSignature("envOr(string,string)", "TENDERLY_USERNAME", "TENDERLY_USERNAME") - ); - string memory username = abi.decode(userData, (string)); + string memory proj = vm.envOr("TENDERLY_PROJECT", string("TENDERLY_PROJECT")); + string memory username = vm.envOr("TENDERLY_USERNAME", string("TENDERLY_USERNAME")); // the following characters are url encoded: []{} string memory stateOverrides = "%5B"; From 0e5dac85e204120a56942a29b6fd214c6f10854a Mon Sep 17 00:00:00 2001 From: Michael de Hoog Date: Sat, 19 Oct 2024 19:31:16 -1000 Subject: [PATCH 04/18] Accept IGnosisSafe interface rather than address where appropriate --- script/deploy/l1/SetGasLimitBuilder.sol | 7 +-- script/universal/MultisigBase.sol | 56 ++++++++---------- script/universal/MultisigBuilder.sol | 41 +++++++------- script/universal/NestedMultisigBuilder.sol | 66 ++++++++++------------ 4 files changed, 78 insertions(+), 92 deletions(-) diff --git a/script/deploy/l1/SetGasLimitBuilder.sol b/script/deploy/l1/SetGasLimitBuilder.sol index 6337a59..51db536 100644 --- a/script/deploy/l1/SetGasLimitBuilder.sol +++ b/script/deploy/l1/SetGasLimitBuilder.sol @@ -55,10 +55,9 @@ abstract contract SetGasLimitBuilder is MultisigBuilder { nonce = safe.nonce() + _nonceOffset(); } - function _addOverrides(address _safe) internal view override returns (SimulationStateOverride memory) { - IGnosisSafe safe = IGnosisSafe(payable(_safe)); - uint256 _nonce = _getNonce(safe); - return overrideSafeThresholdOwnerAndNonce(_safe, DEFAULT_SENDER, _nonce); + function _addOverrides(IGnosisSafe _safe) internal view override returns (SimulationStateOverride memory) { + uint256 _nonce = _getNonce(_safe); + return overrideSafeThresholdOwnerAndNonce(address(_safe), DEFAULT_SENDER, _nonce); } // We need to expect that the gas limit will have been updated previously in our simulation diff --git a/script/universal/MultisigBase.sol b/script/universal/MultisigBase.sol index 4005c22..5ba9231 100644 --- a/script/universal/MultisigBase.sol +++ b/script/universal/MultisigBase.sol @@ -9,15 +9,14 @@ import {LibSort} from "@solady/utils/LibSort.sol"; import "./Simulator.sol"; abstract contract MultisigBase is Simulator { - IMulticall3 internal constant multicall = IMulticall3(MULTICALL3_ADDRESS); bytes32 internal constant SAFE_NONCE_SLOT = bytes32(uint256(5)); - function _getTransactionHash(address _safe, IMulticall3.Call3[] memory calls) internal view returns (bytes32) { + function _getTransactionHash(IGnosisSafe _safe, IMulticall3.Call3[] memory calls) internal view returns (bytes32) { bytes memory data = abi.encodeCall(IMulticall3.aggregate3, (calls)); return _getTransactionHash(_safe, data); } - function _getTransactionHash(address _safe, bytes memory _data) internal view returns (bytes32) { + function _getTransactionHash(IGnosisSafe _safe, bytes memory _data) internal view returns (bytes32) { return keccak256(_encodeTransactionData(_safe, _data)); } @@ -39,16 +38,15 @@ abstract contract MultisigBase is Simulator { catch {} } - function _encodeTransactionData(address _safe, bytes memory _data) internal view returns (bytes memory) { + function _encodeTransactionData(IGnosisSafe _safe, bytes memory _data) internal view returns (bytes memory) { // Ensure that the required contracts exist - require(address(multicall).code.length > 0, "multicall3 not deployed"); - require(_safe.code.length > 0, "no code at safe address"); + require(MULTICALL3_ADDRESS.code.length > 0, "multicall3 not deployed"); + require(address(_safe).code.length > 0, "no code at safe address"); - IGnosisSafe safe = IGnosisSafe(payable(_safe)); - uint256 nonce = _getNonce(safe); + uint256 nonce = _getNonce(_safe); - return safe.encodeTransactionData({ - to: address(multicall), + return _safe.encodeTransactionData({ + to: MULTICALL3_ADDRESS, value: 0, data: _data, operation: Enum.Operation.DelegateCall, @@ -61,7 +59,7 @@ abstract contract MultisigBase is Simulator { }); } - function _printDataToSign(address _safe, IMulticall3.Call3[] memory _calls) internal view { + function _printDataToSign(IGnosisSafe _safe, IMulticall3.Call3[] memory _calls) internal view { bytes memory data = abi.encodeCall(IMulticall3.aggregate3, (_calls)); bytes memory txData = _encodeTransactionData(_safe, data); @@ -76,27 +74,26 @@ abstract contract MultisigBase is Simulator { console.log("###############################"); } - function _checkSignatures(address _safe, IMulticall3.Call3[] memory _calls, bytes memory _signatures) + function _checkSignatures(IGnosisSafe _safe, IMulticall3.Call3[] memory _calls, bytes memory _signatures) internal view { - IGnosisSafe safe = IGnosisSafe(payable(_safe)); bytes memory data = abi.encodeCall(IMulticall3.aggregate3, (_calls)); bytes32 hash = _getTransactionHash(_safe, data); _signatures = prepareSignatures(_safe, hash, _signatures); - safe.checkSignatures({ + _safe.checkSignatures({ dataHash: hash, data: data, signatures: _signatures }); } - function _encodeCall(IGnosisSafe _safe, bytes memory _data, bytes memory _signatures) internal pure returns (bytes memory) { + function _execTransationCalldata(IGnosisSafe _safe, bytes memory _data, bytes memory _signatures) internal pure returns (bytes memory) { return abi.encodeCall( _safe.execTransaction, ( - address(multicall), + MULTICALL3_ADDRESS, 0, _data, Enum.Operation.DelegateCall, @@ -112,7 +109,7 @@ abstract contract MultisigBase is Simulator { function _execTransaction(IGnosisSafe _safe, bytes memory _data, bytes memory _signatures) internal returns (bool) { return _safe.execTransaction({ - to: address(multicall), + to: MULTICALL3_ADDRESS, value: 0, data: _data, operation: Enum.Operation.DelegateCall, @@ -125,24 +122,23 @@ abstract contract MultisigBase is Simulator { }); } - function _executeTransaction(address _safe, IMulticall3.Call3[] memory _calls, bytes memory _signatures) + function _executeTransaction(IGnosisSafe _safe, IMulticall3.Call3[] memory _calls, bytes memory _signatures) internal returns (Vm.AccountAccess[] memory, SimulationPayload memory) { - IGnosisSafe safe = IGnosisSafe(payable(_safe)); bytes memory data = abi.encodeCall(IMulticall3.aggregate3, (_calls)); bytes32 hash = _getTransactionHash(_safe, data); _signatures = prepareSignatures(_safe, hash, _signatures); - bytes memory simData = _encodeCall(safe, data, _signatures); + bytes memory simData = _execTransationCalldata(_safe, data, _signatures); logSimulationLink({ - _to: _safe, + _to: address(_safe), _from: msg.sender, _data: simData }); vm.startStateDiffRecording(); - bool success = _execTransaction(safe, data, _signatures); + bool success = _execTransaction(_safe, data, _signatures); Vm.AccountAccess[] memory accesses = vm.stopAndReturnStateDiff(); require(success, "MultisigBase::_executeTransaction: Transaction failed"); require(accesses.length > 0, "MultisigBase::_executeTransaction: No state changes"); @@ -151,7 +147,7 @@ abstract contract MultisigBase is Simulator { // data about the state diff before broadcasting the transaction. SimulationPayload memory simPayload = SimulationPayload({ from: msg.sender, - to: address(safe), + to: address(_safe), data: simData, stateOverrides: new SimulationStateOverride[](0) }); @@ -164,14 +160,14 @@ abstract contract MultisigBase is Simulator { return calls; } - function prepareSignatures(address _safe, bytes32 hash, bytes memory _signatures) internal view returns (bytes memory) { + function prepareSignatures(IGnosisSafe _safe, bytes32 hash, bytes memory _signatures) internal view returns (bytes memory) { // prepend the prevalidated signatures to the signatures address[] memory approvers = _getApprovers(_safe, hash); bytes memory prevalidatedSignatures = genPrevalidatedSignatures(approvers); _signatures = bytes.concat(prevalidatedSignatures, _signatures); // safe requires all signatures to be unique, and sorted ascending by public key - return sortUniqueSignatures(_signatures, hash, IGnosisSafe(_safe).getThreshold(), prevalidatedSignatures.length); + return sortUniqueSignatures(_signatures, hash, _safe.getThreshold(), prevalidatedSignatures.length); } function genPrevalidatedSignatures(address[] memory _addresses) internal pure returns (bytes memory) { @@ -190,17 +186,15 @@ abstract contract MultisigBase is Simulator { return abi.encodePacked(r, s, v); } - function _getApprovers(address _safe, bytes32 hash) internal view returns (address[] memory) { - IGnosisSafe safe = IGnosisSafe(payable(_safe)); - + function _getApprovers(IGnosisSafe _safe, bytes32 hash) internal view returns (address[] memory) { // get a list of owners that have approved this transaction - uint256 threshold = safe.getThreshold(); - address[] memory owners = safe.getOwners(); + uint256 threshold = _safe.getThreshold(); + address[] memory owners = _safe.getOwners(); address[] memory approvers = new address[](threshold); uint256 approverIndex; for (uint256 i; i < owners.length; i++) { address owner = owners[i]; - uint256 approved = safe.approvedHashes(owner, hash); + uint256 approved = _safe.approvedHashes(owner, hash); if (approved == 1) { approvers[approverIndex] = owner; approverIndex++; diff --git a/script/universal/MultisigBuilder.sol b/script/universal/MultisigBuilder.sol index c7dadde..733d4a4 100644 --- a/script/universal/MultisigBuilder.sol +++ b/script/universal/MultisigBuilder.sol @@ -50,19 +50,19 @@ abstract contract MultisigBuilder is MultisigBase { * used by a separate tx executor address in step 2, which doesn't have to be a signer. */ function sign() public { - address safe = _ownerSafe(); + IGnosisSafe safe = IGnosisSafe(_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(IGnosisSafe(safe)); + uint256 originalNonce = _getNonce(safe); IMulticall3.Call3[] memory calls = _buildCalls(); (Vm.AccountAccess[] memory accesses, SimulationPayload memory simPayload) = _simulateForSigner(safe, calls); _postCheck(accesses, simPayload); // Restore the original nonce. - vm.store(safe, SAFE_NONCE_SLOT, bytes32(uint256(originalNonce))); + vm.store(address(safe), SAFE_NONCE_SLOT, bytes32(uint256(originalNonce))); _printDataToSign(safe, calls); } @@ -74,11 +74,11 @@ abstract contract MultisigBuilder is MultisigBase { * This allow transactions to be pre-signed and stored safely before execution. */ function verify(bytes memory _signatures) public view { - _checkSignatures(_ownerSafe(), _buildCalls(), _signatures); + _checkSignatures(IGnosisSafe(_ownerSafe()), _buildCalls(), _signatures); } function nonce() public view { - IGnosisSafe safe = IGnosisSafe(payable(_ownerSafe())); + IGnosisSafe safe = IGnosisSafe(_ownerSafe()); console.log("Nonce:", safe.nonce()); } @@ -88,12 +88,11 @@ abstract contract MultisigBuilder is MultisigBase { * Simulate the transaction. This method should be called by the final member of the multisig * that will execute the transaction. Signatures from step 1 are required. */ - function simulateSigned(bytes memory _signatures) public { - address _safe = _ownerSafe(); - IGnosisSafe safe = IGnosisSafe(payable(_safe)); + function simulate(bytes memory _signatures) public { + IGnosisSafe safe = IGnosisSafe(_ownerSafe()); uint256 _nonce = _getNonce(safe); - vm.store(_safe, SAFE_NONCE_SLOT, bytes32(uint256(_nonce))); - (Vm.AccountAccess[] memory accesses, SimulationPayload memory simPayload) = _executeTransaction(_safe, _buildCalls(), _signatures); + vm.store(address(safe), SAFE_NONCE_SLOT, bytes32(uint256(_nonce))); + (Vm.AccountAccess[] memory accesses, SimulationPayload memory simPayload) = _executeTransaction(safe, _buildCalls(), _signatures); _postCheck(accesses, simPayload); } @@ -108,24 +107,23 @@ abstract contract MultisigBuilder is MultisigBase { */ function run(bytes memory _signatures) public { vm.startBroadcast(); - (Vm.AccountAccess[] memory accesses, SimulationPayload memory simPayload) = _executeTransaction(_ownerSafe(), _buildCalls(), _signatures); + (Vm.AccountAccess[] memory accesses, SimulationPayload memory simPayload) = _executeTransaction(IGnosisSafe(_ownerSafe()), _buildCalls(), _signatures); vm.stopBroadcast(); _postCheck(accesses, simPayload); } - function _simulateForSigner(address _safe, IMulticall3.Call3[] memory _calls) + function _simulateForSigner(IGnosisSafe _safe, IMulticall3.Call3[] memory _calls) internal returns (Vm.AccountAccess[] memory, SimulationPayload memory) { - IGnosisSafe safe = IGnosisSafe(payable(_safe)); bytes memory data = abi.encodeCall(IMulticall3.aggregate3, (_calls)); SimulationStateOverride[] memory overrides = _setOverrides(_safe); - bytes memory txData = abi.encodeCall(safe.execTransaction, + bytes memory txData = abi.encodeCall(_safe.execTransaction, ( - address(multicall), + MULTICALL3_ADDRESS, 0, data, Enum.Operation.DelegateCall, @@ -139,7 +137,7 @@ abstract contract MultisigBuilder is MultisigBase { ); logSimulationLink({ - _to: _safe, + _to: address(_safe), _data: txData, _from: msg.sender, _overrides: overrides @@ -148,7 +146,7 @@ abstract contract MultisigBuilder is MultisigBase { // Forge simulation of the data logged in the link. If the simulation fails // we revert to make it explicit that the simulation failed. SimulationPayload memory simPayload = SimulationPayload({ - to: _safe, + to: address(_safe), data: txData, from: msg.sender, stateOverrides: overrides @@ -163,10 +161,9 @@ abstract contract MultisigBuilder is MultisigBase { // will not be reflected in the prod execution. // This particular implementation can be overwritten by an inheriting script. The // default logic is vestigial for backwards compatibility. - function _addOverrides(address _safe) internal virtual view returns (SimulationStateOverride memory) { - IGnosisSafe safe = IGnosisSafe(payable(_safe)); - uint256 _nonce = _getNonce(safe); - return overrideSafeThresholdAndNonce(_safe, _nonce); + function _addOverrides(IGnosisSafe _safe) internal virtual view returns (SimulationStateOverride memory) { + uint256 _nonce = _getNonce(_safe); + return overrideSafeThresholdAndNonce(address(_safe), _nonce); } // Tenderly simulations can accept generic state overrides. This hook enables this functionality. @@ -180,7 +177,7 @@ abstract contract MultisigBuilder is MultisigBase { returns (SimulationStateOverride[] memory overrides_) {} - function _setOverrides(address _safe) internal virtual returns (SimulationStateOverride[] memory) { + function _setOverrides(IGnosisSafe _safe) internal virtual returns (SimulationStateOverride[] memory) { SimulationStateOverride[] memory extraOverrides = _addMultipleGenericOverrides(); SimulationStateOverride[] memory overrides = new SimulationStateOverride[](2 + extraOverrides.length); overrides[0] = _addOverrides(_safe); diff --git a/script/universal/NestedMultisigBuilder.sol b/script/universal/NestedMultisigBuilder.sol index 51ec796..4131546 100644 --- a/script/universal/NestedMultisigBuilder.sol +++ b/script/universal/NestedMultisigBuilder.sol @@ -74,27 +74,28 @@ abstract contract NestedMultisigBuilder 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 nestedSafeAddress = _ownerSafe(); + function sign(IGnosisSafe _signerSafe) public { + IGnosisSafe nestedSafe = IGnosisSafe(_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(IGnosisSafe(nestedSafeAddress)); - uint256 originalSignerNonce = _getNonce(IGnosisSafe(_signerSafe)); + uint256 originalNonce = _getNonce(nestedSafe); + uint256 originalSignerNonce = _getNonce(_signerSafe); IMulticall3.Call3[] memory nestedCalls = _buildCalls(); - IMulticall3.Call3 memory call = _generateApproveCall(nestedSafeAddress, nestedCalls); + IMulticall3.Call3 memory call = _generateApproveCall(nestedSafe, nestedCalls); bytes32 hash = _getTransactionHash(_signerSafe, toArray(call)); - console.log("---\nIf submitting onchain, call Safe.approveHash on %s with the following hash:", _signerSafe); + console.log("---\nIf submitting onchain, call Safe.approveHash on %s with the following hash:", address(_signerSafe)); console.logBytes32(hash); - (Vm.AccountAccess[] memory accesses, SimulationPayload memory simPayload) = _simulateForSigner(_signerSafe, nestedSafeAddress, nestedCalls); + + (Vm.AccountAccess[] memory accesses, SimulationPayload memory simPayload) = _simulateForSigner(_signerSafe, nestedSafe, nestedCalls); _postCheck(accesses, simPayload); // Restore the original nonce. - vm.store(nestedSafeAddress, SAFE_NONCE_SLOT, bytes32(uint256(originalNonce))); - vm.store(_signerSafe, SAFE_NONCE_SLOT, bytes32(uint256(originalSignerNonce))); + vm.store(address(nestedSafe), SAFE_NONCE_SLOT, bytes32(uint256(originalNonce))); + vm.store(address(_signerSafe), SAFE_NONCE_SLOT, bytes32(uint256(originalSignerNonce))); _printDataToSign(_signerSafe, toArray(call)); } @@ -106,16 +107,14 @@ abstract contract NestedMultisigBuilder 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 { - address nestedSafeAddress = _ownerSafe(); + function approve(IGnosisSafe _signerSafe, bytes memory _signatures) public { + IGnosisSafe nestedSafe = IGnosisSafe(_ownerSafe()); IMulticall3.Call3[] memory nestedCalls = _buildCalls(); - IMulticall3.Call3 memory call = _generateApproveCall(nestedSafeAddress, nestedCalls); + IMulticall3.Call3 memory call = _generateApproveCall(nestedSafe, nestedCalls); vm.startBroadcast(); (Vm.AccountAccess[] memory accesses, SimulationPayload memory simPayload) = _executeTransaction(_signerSafe, toArray(call), _signatures); vm.stopBroadcast(); - - _postCheck(accesses, simPayload); } /** @@ -125,48 +124,45 @@ abstract contract NestedMultisigBuilder is MultisigBase { * all of the approval transactions have been submitted onchain (see step 2). */ function run() public { - address nestedSafeAddress = _ownerSafe(); + IGnosisSafe nestedSafe = IGnosisSafe(_ownerSafe()); IMulticall3.Call3[] memory nestedCalls = _buildCalls(); // signatures is empty, because `_executeTransaction` internally collects all of the approvedHash addresses bytes memory signatures; vm.startBroadcast(); - (Vm.AccountAccess[] memory accesses, SimulationPayload memory simPayload) = _executeTransaction(nestedSafeAddress, nestedCalls, signatures); + (Vm.AccountAccess[] memory accesses, SimulationPayload memory simPayload) = _executeTransaction(nestedSafe, nestedCalls, signatures); vm.stopBroadcast(); _postCheck(accesses, simPayload); } - function _generateApproveCall(address _safe, IMulticall3.Call3[] memory _calls) internal view returns (IMulticall3.Call3 memory) { - IGnosisSafe safe = IGnosisSafe(payable(_safe)); + function _generateApproveCall(IGnosisSafe _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, + target: address(_safe), allowFailure: false, - callData: abi.encodeCall(safe.approveHash, (hash)) + callData: abi.encodeCall(_safe.approveHash, (hash)) }); } - function _simulateForSigner(address _signerSafe, address _safe, IMulticall3.Call3[] memory _calls) + function _simulateForSigner(IGnosisSafe _signerSafe, IGnosisSafe _safe, IMulticall3.Call3[] memory _calls) internal returns (Vm.AccountAccess[] memory, SimulationPayload memory) { - IGnosisSafe signerSafe = IGnosisSafe(payable(_signerSafe)); - IGnosisSafe safe = IGnosisSafe(payable(_safe)); bytes memory data = abi.encodeCall(IMulticall3.aggregate3, (_calls)); - IMulticall3.Call3[] memory calls = _simulateForSignerCalls(signerSafe, safe, data); + IMulticall3.Call3[] memory calls = _simulateForSignerCalls(_signerSafe, _safe, data); // For each safe, determine if a nonce override is needed. At this point, // no state overrides (i.e. vm.store) have been applied to the Foundry VM, // meaning the nonce is not yet overriden. Therefore these calls to // `safe.nonce()` will correctly return the current nonce of the safe. - bool safeNonceOverride = _getNonce(safe) != safe.nonce(); - bool signerSafeNonceOverride = _getNonce(signerSafe) != signerSafe.nonce(); + bool safeNonceOverride = _getNonce(_safe) != _safe.nonce(); + bool signerSafeNonceOverride = _getNonce(_signerSafe) != _signerSafe.nonce(); // Now define the state overrides for the simulation. SimulationStateOverride[] memory overrides = new SimulationStateOverride[](2); @@ -175,24 +171,24 @@ abstract contract NestedMultisigBuilder is MultisigBase { // will look like upon transaction execution. The multisig threshold // will not actually change in the transaction execution. if (safeNonceOverride) { - overrides[0] = overrideSafeThresholdAndNonce(_safe, _getNonce(safe)); + overrides[0] = overrideSafeThresholdAndNonce(address(_safe), _getNonce(_safe)); } else { - overrides[0] = overrideSafeThreshold(_safe); + overrides[0] = overrideSafeThreshold(address(_safe)); } // Set the signer safe threshold to 1, and set the owner to multicall. // This is a little hacky; reason is to simulate both the approve hash // and the final tx in a single Tenderly tx, using multicall. Given an // EOA cannot DELEGATECALL, multicall needs to own the signer safe. if (signerSafeNonceOverride) { - overrides[1] = overrideSafeThresholdOwnerAndNonce(_signerSafe, address(multicall), _getNonce(signerSafe)); + overrides[1] = overrideSafeThresholdOwnerAndNonce(address(_signerSafe), MULTICALL3_ADDRESS, _getNonce(_signerSafe)); } else { - overrides[1] = overrideSafeThresholdAndOwner(_signerSafe, address(multicall)); + overrides[1] = overrideSafeThresholdAndOwner(address(_signerSafe), MULTICALL3_ADDRESS); } bytes memory txData = abi.encodeCall(IMulticall3.aggregate3, (calls)); console.log("---\nSimulation link:"); logSimulationLink({ - _to: address(multicall), + _to: MULTICALL3_ADDRESS, _data: txData, _from: msg.sender, _overrides: overrides @@ -201,7 +197,7 @@ abstract contract NestedMultisigBuilder is MultisigBase { // Forge simulation of the data logged in the link. If the simulation fails // we revert to make it explicit that the simulation failed. SimulationPayload memory simPayload = SimulationPayload({ - to: address(multicall), + to: MULTICALL3_ADDRESS, data: txData, from: msg.sender, stateOverrides: overrides @@ -215,7 +211,7 @@ abstract contract NestedMultisigBuilder is MultisigBase { returns (IMulticall3.Call3[] memory) { IMulticall3.Call3[] memory calls = new IMulticall3.Call3[](2); - bytes32 hash = _getTransactionHash(address(_safe), _data); + 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( @@ -225,7 +221,7 @@ abstract contract NestedMultisigBuilder is MultisigBase { callData: abi.encodeCall(_safe.approveHash, (hash)) }) ))); - bytes memory approveHashExec = _encodeCall(_signerSafe, approveHashData, genPrevalidatedSignature(address(multicall))); + bytes memory approveHashExec = _execTransationCalldata(_signerSafe, approveHashData, genPrevalidatedSignature(MULTICALL3_ADDRESS)); calls[0] = IMulticall3.Call3({ target: address(_signerSafe), allowFailure: false, @@ -233,7 +229,7 @@ abstract contract NestedMultisigBuilder is MultisigBase { }); // simulate the final state changes tx, so that signer can verify the final results - bytes memory finalExec = _encodeCall(_safe, _data, genPrevalidatedSignature(address(_signerSafe))); + bytes memory finalExec = _execTransationCalldata(_safe, _data, genPrevalidatedSignature(address(_signerSafe))); calls[1] = IMulticall3.Call3({ target: address(_safe), allowFailure: false, From 259263fa6658937a94f51c75d266c902e1eb6d83 Mon Sep 17 00:00:00 2001 From: Michael de Hoog Date: Sat, 19 Oct 2024 19:40:34 -1000 Subject: [PATCH 05/18] Pull _getNonce complexity into MultisigBase --- script/universal/MultisigBase.sol | 52 ++++++++++++++++------ script/universal/NestedMultisigBuilder.sol | 32 ++----------- 2 files changed, 43 insertions(+), 41 deletions(-) diff --git a/script/universal/MultisigBase.sol b/script/universal/MultisigBase.sol index 5ba9231..2be16be 100644 --- a/script/universal/MultisigBase.sol +++ b/script/universal/MultisigBase.sol @@ -20,22 +20,48 @@ abstract contract MultisigBase is Simulator { return keccak256(_encodeTransactionData(_safe, _data)); } - // Virtual method which can be overwritten - // Default logic here is vestigial for backwards compatibility - // IMPORTANT: this method is used in the sign, simulate, AND execution contexts - // If you override it, ensure that the behavior is correct for all contexts - // As an example, if you are pre-signing a message that needs safe.nonce+1 (before safe.nonce is executed), - // you should explicitly set the nonce value with an env var. - // Overwriting this method with safe.nonce + 1 will cause issues upon execution because the transaction - // hash will differ from the one signed. + // Subclasses that use nested safes should return `false` to force use of the + // explicit SAFE_NONCE_{UPPERCASE_SAFE_ADDRESS} env var. + function _readFrom_SAFE_NONCE() internal pure virtual returns (bool); + + // Get the nonce to use for the given safe, for signing and simulations. + // + // If you override it, ensure that the behavior is correct for all contexts. + // As an example, if you are pre-signing a message that needs safe.nonce+1 (before + // safe.nonce is executed), you should explicitly set the nonce value with an env var. + // Overriding this method with safe.nonce+1 will cause issues upon execution because + // the transaction hash will differ from the one signed. + // + // The process for determining a nonce override is as follows: + // 1. We look for an env var of the name SAFE_NONCE_{UPPERCASE_SAFE_ADDRESS}. For example, + // SAFE_NONCE_0X6DF4742A3C28790E63FE933F7D108FE9FCE51EA4. + // 2. If it exists, we use it as the nonce override for the safe. + // 3. If it does not exist and _readFrom_SAFE_NONCE() returns true, we do the same for the + // SAFE_NONCE env var. + // 4. Otherwise we fallback to the safe's current nonce (no override). function _getNonce(IGnosisSafe safe) internal view virtual returns (uint256 nonce) { - nonce = safe.nonce(); - console.log("Safe current nonce:", nonce); - try vm.envUint("SAFE_NONCE") { - nonce = vm.envUint("SAFE_NONCE"); - console.log("Creating transaction with nonce:", nonce); + uint256 safeNonce = safe.nonce(); + nonce = safeNonce; + + // first try SAFE_NONCE + if (_readFrom_SAFE_NONCE()) { + try vm.envUint("SAFE_NONCE") { + nonce = vm.envUint("SAFE_NONCE"); + } + catch {} + } + + // then try SAFE_NONCE_{UPPERCASE_SAFE_ADDRESS} + string memory envVarName = string.concat("SAFE_NONCE_", vm.toUppercase(vm.toString(address(safe)))); + try vm.envUint(envVarName) { + nonce = vm.envUint(envVarName); } catch {} + + // print if any override + if (nonce != safeNonce) { + console.log("Overriding nonce for safe %s: %d -> %d", address(safe), safeNonce, nonce); + } } function _encodeTransactionData(IGnosisSafe _safe, bytes memory _data) internal view returns (bytes memory) { diff --git a/script/universal/NestedMultisigBuilder.sol b/script/universal/NestedMultisigBuilder.sol index 4131546..8c22eaa 100644 --- a/script/universal/NestedMultisigBuilder.sol +++ b/script/universal/NestedMultisigBuilder.sol @@ -38,34 +38,6 @@ abstract contract NestedMultisigBuilder is MultisigBase { * ----------------------------------------------------------- */ - // Virtual method which can be overwritten. - // This allows different nonce overrides for each safe in the nested multisig case. - // IMPORTANT: this method is used in the sign, simulate, AND execution contexts - // If you override it, ensure that the behavior is correct for all contexts - // As an example, if you are pre-signing a message that needs safe.nonce+1 (before safe.nonce is executed), - // you should explicitly set the nonce value with an env var. - // Overwriting this method with safe.nonce + 1 will cause issues upon execution because the transaction - // hash will differ from the one signed. - function _getNonce(IGnosisSafe safe) internal view override virtual returns (uint256 nonce) { - string memory safeAddrStr = vm.toString(address(safe)); - nonce = safe.nonce(); - console.log("Safe", safeAddrStr, "current nonce:", nonce); - - // In this overridden method, the process for determining the nonce is as follows: - // 1. We look for an env var of the name SAFE_NONCE_{UPPERCASE_SAFE_ADDRESS}. For example, - // SAFE_NONCE_0X6DF4742A3C28790E63FE933F7D108FE9FCE51EA4. - // 2. If it exists, we use it as the nonce override for the safe. - // 3. If it does not exist, we use the current nonce of the safe. - // 4. We explicitly do not use SAFE_NONCE as a fallback, because in the nested case it is - // ambiguous which safe it refers to. - string memory safeNonceEnvVarName = string.concat("SAFE_NONCE_", vm.toUppercase(safeAddrStr)); - try vm.envUint(safeNonceEnvVarName) { - nonce = vm.envUint(safeNonceEnvVarName); - console.log("Creating transaction with nonce:", nonce); - } - catch {} - } - /** * Step 1 * ====== @@ -137,6 +109,10 @@ abstract contract NestedMultisigBuilder is MultisigBase { _postCheck(accesses, simPayload); } + function _readFrom_SAFE_NONCE() internal pure override returns (bool) { + return false; + } + function _generateApproveCall(IGnosisSafe _safe, IMulticall3.Call3[] memory _calls) internal view returns (IMulticall3.Call3 memory) { bytes32 hash = _getTransactionHash(_safe, _calls); From 2fdc7c6efe8609d9f6dafad7ae12ff8de62c52e1 Mon Sep 17 00:00:00 2001 From: Michael de Hoog Date: Sat, 19 Oct 2024 19:43:46 -1000 Subject: [PATCH 06/18] Emit event from _printDataToSign --- script/universal/MultisigBase.sol | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/script/universal/MultisigBase.sol b/script/universal/MultisigBase.sol index 2be16be..61b1a21 100644 --- a/script/universal/MultisigBase.sol +++ b/script/universal/MultisigBase.sol @@ -11,6 +11,8 @@ import "./Simulator.sol"; abstract contract MultisigBase is Simulator { bytes32 internal constant SAFE_NONCE_SLOT = bytes32(uint256(5)); + event DataToSign(bytes); + function _getTransactionHash(IGnosisSafe _safe, IMulticall3.Call3[] memory calls) internal view returns (bytes32) { bytes memory data = abi.encodeCall(IMulticall3.aggregate3, (calls)); return _getTransactionHash(_safe, data); @@ -85,10 +87,12 @@ abstract contract MultisigBase is Simulator { }); } - function _printDataToSign(IGnosisSafe _safe, IMulticall3.Call3[] memory _calls) internal view { + function _printDataToSign(IGnosisSafe _safe, IMulticall3.Call3[] memory _calls) internal { bytes memory data = abi.encodeCall(IMulticall3.aggregate3, (_calls)); bytes memory txData = _encodeTransactionData(_safe, data); + emit DataToSign(txData); + console.log("---\nData to sign:"); console.log("vvvvvvvv"); console.logBytes(txData); From 9120f065f61c5664a73e5a1415d4617753ce22c4 Mon Sep 17 00:00:00 2001 From: Michael de Hoog Date: Sat, 19 Oct 2024 19:52:45 -1000 Subject: [PATCH 07/18] Split out state/simulator post checks from the actual postCheck --- script/deploy/l1/SetGasLimitBuilder.sol | 2 +- script/universal/MultisigBuilder.sol | 39 +++++++++++++++++----- script/universal/NestedMultisigBuilder.sol | 37 ++++++++++++++++---- 3 files changed, 62 insertions(+), 16 deletions(-) diff --git a/script/deploy/l1/SetGasLimitBuilder.sol b/script/deploy/l1/SetGasLimitBuilder.sol index 51db536..dde82f5 100644 --- a/script/deploy/l1/SetGasLimitBuilder.sol +++ b/script/deploy/l1/SetGasLimitBuilder.sol @@ -31,7 +31,7 @@ abstract contract SetGasLimitBuilder is MultisigBuilder { * ----------------------------------------------------------- */ - function _postCheck(Vm.AccountAccess[] memory, SimulationPayload memory) internal override view { + function _postCheck() internal override view { assert(SystemConfig(L1_SYSTEM_CONFIG).gasLimit() == _toGasLimit()); } diff --git a/script/universal/MultisigBuilder.sol b/script/universal/MultisigBuilder.sol index 733d4a4..f621478 100644 --- a/script/universal/MultisigBuilder.sol +++ b/script/universal/MultisigBuilder.sol @@ -19,19 +19,31 @@ abstract contract MultisigBuilder is MultisigBase { */ /** - * @notice Follow up assertions to ensure that the script ran to completion. + * @notice Returns the safe address to execute the transaction from */ - function _postCheck(Vm.AccountAccess[] memory accesses, SimulationPayload memory simPayload) internal virtual; + function _ownerSafe() internal virtual view returns (address); /** - * @notice Creates the calldata + * @notice Creates the calldata for both signatures (`sign`) and execution (`run`) */ function _buildCalls() internal virtual view returns (IMulticall3.Call3[] memory); /** - * @notice Returns the safe address to execute the transaction from + * @notice Follow up assertions to ensure that the script ran to completion. */ - function _ownerSafe() internal virtual view returns (address); + function _postCheck() internal virtual; + + /** + * @notice Follow up assertions on state and simulation after a `sign` call. + */ + function _postSign(Vm.AccountAccess[] memory accesses, SimulationPayload memory simPayload) internal virtual { + } + + /** + * @notice Follow up assertions on state and simulation after a `run` call. + */ + function _postRun(Vm.AccountAccess[] memory accesses, SimulationPayload memory simPayload) internal virtual { + } /** * ----------------------------------------------------------- @@ -59,7 +71,8 @@ abstract contract MultisigBuilder is MultisigBase { IMulticall3.Call3[] memory calls = _buildCalls(); (Vm.AccountAccess[] memory accesses, SimulationPayload memory simPayload) = _simulateForSigner(safe, calls); - _postCheck(accesses, simPayload); + _postSign(accesses, simPayload); + _postCheck(); // Restore the original nonce. vm.store(address(safe), SAFE_NONCE_SLOT, bytes32(uint256(originalNonce))); @@ -87,13 +100,18 @@ abstract contract MultisigBuilder is MultisigBase { * ====== * Simulate the transaction. This method should be called by the final member of the multisig * that will execute the transaction. Signatures from step 1 are required. + * + * Differs from `run` in that you can override the safe nonce for simulation purposes. */ function simulate(bytes memory _signatures) public { IGnosisSafe safe = IGnosisSafe(_ownerSafe()); uint256 _nonce = _getNonce(safe); vm.store(address(safe), SAFE_NONCE_SLOT, bytes32(uint256(_nonce))); + (Vm.AccountAccess[] memory accesses, SimulationPayload memory simPayload) = _executeTransaction(safe, _buildCalls(), _signatures); - _postCheck(accesses, simPayload); + + _postRun(accesses, simPayload); + _postCheck(); } /** @@ -110,7 +128,12 @@ abstract contract MultisigBuilder is MultisigBase { (Vm.AccountAccess[] memory accesses, SimulationPayload memory simPayload) = _executeTransaction(IGnosisSafe(_ownerSafe()), _buildCalls(), _signatures); vm.stopBroadcast(); - _postCheck(accesses, simPayload); + _postRun(accesses, simPayload); + _postCheck(); + } + + function _readFrom_SAFE_NONCE() internal pure override returns (bool) { + return true; } function _simulateForSigner(IGnosisSafe _safe, IMulticall3.Call3[] memory _calls) diff --git a/script/universal/NestedMultisigBuilder.sol b/script/universal/NestedMultisigBuilder.sol index 8c22eaa..c75d8da 100644 --- a/script/universal/NestedMultisigBuilder.sol +++ b/script/universal/NestedMultisigBuilder.sol @@ -18,19 +18,38 @@ abstract contract NestedMultisigBuilder is MultisigBase { */ /** - * @notice Follow up assertions to ensure that the script ran to completion + * @notice Returns the nested safe address to execute the final transaction from */ - function _postCheck(Vm.AccountAccess[] memory accesses, SimulationPayload memory simPayload) internal virtual; + function _ownerSafe() internal virtual view returns (address); /** - * @notice Creates the calldata + * @notice Creates the calldata for both signatures (`sign`) and execution (`run`) */ function _buildCalls() internal virtual view returns (IMulticall3.Call3[] memory); /** - * @notice Returns the nested safe address to execute the final transaction from + * @notice Follow up assertions to ensure that the script ran to completion. + * @dev Called after `sign` and `run`, but not `approve`. */ - function _ownerSafe() internal virtual view returns (address); + function _postCheck() internal virtual; + + /** + * @notice Follow up assertions on state and simulation after a `sign` call. + */ + function _postSign(Vm.AccountAccess[] memory accesses, SimulationPayload memory simPayload) internal virtual { + } + + /** + * @notice Follow up assertions on state and simulation after a `approve` call. + */ + function _postApprove(Vm.AccountAccess[] memory accesses, SimulationPayload memory simPayload) internal virtual { + } + + /** + * @notice Follow up assertions on state and simulation after a `run` call. + */ + function _postRun(Vm.AccountAccess[] memory accesses, SimulationPayload memory simPayload) internal virtual { + } /** * ----------------------------------------------------------- @@ -63,7 +82,8 @@ abstract contract NestedMultisigBuilder is MultisigBase { console.logBytes32(hash); (Vm.AccountAccess[] memory accesses, SimulationPayload memory simPayload) = _simulateForSigner(_signerSafe, nestedSafe, nestedCalls); - _postCheck(accesses, simPayload); + _postSign(accesses, simPayload); + _postCheck(); // Restore the original nonce. vm.store(address(nestedSafe), SAFE_NONCE_SLOT, bytes32(uint256(originalNonce))); @@ -87,6 +107,8 @@ abstract contract NestedMultisigBuilder is MultisigBase { vm.startBroadcast(); (Vm.AccountAccess[] memory accesses, SimulationPayload memory simPayload) = _executeTransaction(_signerSafe, toArray(call), _signatures); vm.stopBroadcast(); + + _postApprove(accesses, simPayload); } /** @@ -106,7 +128,8 @@ abstract contract NestedMultisigBuilder is MultisigBase { (Vm.AccountAccess[] memory accesses, SimulationPayload memory simPayload) = _executeTransaction(nestedSafe, nestedCalls, signatures); vm.stopBroadcast(); - _postCheck(accesses, simPayload); + _postRun(accesses, simPayload); + _postCheck(); } function _readFrom_SAFE_NONCE() internal pure override returns (bool) { From 13242bd7a3f89e9081601c308442021971e67858 Mon Sep 17 00:00:00 2001 From: Michael de Hoog Date: Sat, 19 Oct 2024 20:01:25 -1000 Subject: [PATCH 08/18] Move `Safe.approveHash` log to MultisigBase._printDataToSign --- script/universal/MultisigBase.sol | 4 ++++ script/universal/NestedMultisigBuilder.sol | 4 ---- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/script/universal/MultisigBase.sol b/script/universal/MultisigBase.sol index 61b1a21..121aee6 100644 --- a/script/universal/MultisigBase.sol +++ b/script/universal/MultisigBase.sol @@ -90,9 +90,13 @@ abstract contract MultisigBase is Simulator { function _printDataToSign(IGnosisSafe _safe, IMulticall3.Call3[] memory _calls) internal { bytes memory data = abi.encodeCall(IMulticall3.aggregate3, (_calls)); bytes memory txData = _encodeTransactionData(_safe, data); + bytes32 hash = _getTransactionHash(_safe, data); emit DataToSign(txData); + console.log("---\nIf submitting onchain, call Safe.approveHash on %s with the following hash:", address(_safe)); + console.logBytes32(hash); + console.log("---\nData to sign:"); console.log("vvvvvvvv"); console.logBytes(txData); diff --git a/script/universal/NestedMultisigBuilder.sol b/script/universal/NestedMultisigBuilder.sol index c75d8da..4de27e4 100644 --- a/script/universal/NestedMultisigBuilder.sol +++ b/script/universal/NestedMultisigBuilder.sol @@ -76,10 +76,6 @@ abstract contract NestedMultisigBuilder is MultisigBase { IMulticall3.Call3[] memory nestedCalls = _buildCalls(); IMulticall3.Call3 memory call = _generateApproveCall(nestedSafe, nestedCalls); - bytes32 hash = _getTransactionHash(_signerSafe, toArray(call)); - - console.log("---\nIf submitting onchain, call Safe.approveHash on %s with the following hash:", address(_signerSafe)); - console.logBytes32(hash); (Vm.AccountAccess[] memory accesses, SimulationPayload memory simPayload) = _simulateForSigner(_signerSafe, nestedSafe, nestedCalls); _postSign(accesses, simPayload); From 9b82271d92ff4b81897373deb63b9b4370aac956 Mon Sep 17 00:00:00 2001 From: Michael de Hoog Date: Sat, 19 Oct 2024 20:01:42 -1000 Subject: [PATCH 09/18] abi.encodeCall simplification --- script/universal/MultisigBuilder.sol | 16 +--------------- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/script/universal/MultisigBuilder.sol b/script/universal/MultisigBuilder.sol index f621478..c0ea657 100644 --- a/script/universal/MultisigBuilder.sol +++ b/script/universal/MultisigBuilder.sol @@ -144,21 +144,7 @@ abstract contract MultisigBuilder is MultisigBase { SimulationStateOverride[] memory overrides = _setOverrides(_safe); - bytes memory txData = abi.encodeCall(_safe.execTransaction, - ( - MULTICALL3_ADDRESS, - 0, - data, - Enum.Operation.DelegateCall, - 0, - 0, - 0, - address(0), - payable(address(0)), - genPrevalidatedSignature(msg.sender) - ) - ); - + bytes memory txData = _execTransationCalldata(_safe, data, genPrevalidatedSignature(msg.sender)); logSimulationLink({ _to: address(_safe), _data: txData, From e41a6bf8fd521831d62143fea32a8883350f7374 Mon Sep 17 00:00:00 2001 From: Michael de Hoog Date: Sat, 19 Oct 2024 20:03:57 -1000 Subject: [PATCH 10/18] Remove code.length checks --- script/universal/MultisigBase.sol | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/script/universal/MultisigBase.sol b/script/universal/MultisigBase.sol index 121aee6..f5a9023 100644 --- a/script/universal/MultisigBase.sol +++ b/script/universal/MultisigBase.sol @@ -67,12 +67,6 @@ abstract contract MultisigBase is Simulator { } function _encodeTransactionData(IGnosisSafe _safe, bytes memory _data) internal view returns (bytes memory) { - // Ensure that the required contracts exist - require(MULTICALL3_ADDRESS.code.length > 0, "multicall3 not deployed"); - require(address(_safe).code.length > 0, "no code at safe address"); - - uint256 nonce = _getNonce(_safe); - return _safe.encodeTransactionData({ to: MULTICALL3_ADDRESS, value: 0, @@ -83,7 +77,7 @@ abstract contract MultisigBase is Simulator { gasPrice: 0, gasToken: address(0), refundReceiver: address(0), - _nonce: nonce + _nonce: _getNonce(_safe) }); } From bad79ae15d3a5f4f338d387ab1779b4899fe5cfd Mon Sep 17 00:00:00 2001 From: Michael de Hoog Date: Sat, 19 Oct 2024 20:06:36 -1000 Subject: [PATCH 11/18] Reorder functions for clarity --- script/universal/MultisigBase.sol | 136 +++++++++++++++--------------- 1 file changed, 68 insertions(+), 68 deletions(-) diff --git a/script/universal/MultisigBase.sol b/script/universal/MultisigBase.sol index f5a9023..d56585c 100644 --- a/script/universal/MultisigBase.sol +++ b/script/universal/MultisigBase.sol @@ -13,15 +13,6 @@ abstract contract MultisigBase is Simulator { event DataToSign(bytes); - function _getTransactionHash(IGnosisSafe _safe, IMulticall3.Call3[] memory calls) internal view returns (bytes32) { - bytes memory data = abi.encodeCall(IMulticall3.aggregate3, (calls)); - return _getTransactionHash(_safe, data); - } - - function _getTransactionHash(IGnosisSafe _safe, bytes memory _data) internal view returns (bytes32) { - return keccak256(_encodeTransactionData(_safe, _data)); - } - // Subclasses that use nested safes should return `false` to force use of the // explicit SAFE_NONCE_{UPPERCASE_SAFE_ADDRESS} env var. function _readFrom_SAFE_NONCE() internal pure virtual returns (bool); @@ -66,21 +57,6 @@ abstract contract MultisigBase is Simulator { } } - function _encodeTransactionData(IGnosisSafe _safe, bytes memory _data) internal view returns (bytes memory) { - return _safe.encodeTransactionData({ - to: MULTICALL3_ADDRESS, - value: 0, - data: _data, - operation: Enum.Operation.DelegateCall, - safeTxGas: 0, - baseGas: 0, - gasPrice: 0, - gasToken: address(0), - refundReceiver: address(0), - _nonce: _getNonce(_safe) - }); - } - function _printDataToSign(IGnosisSafe _safe, IMulticall3.Call3[] memory _calls) internal { bytes memory data = abi.encodeCall(IMulticall3.aggregate3, (_calls)); bytes memory txData = _encodeTransactionData(_safe, data); @@ -117,39 +93,6 @@ abstract contract MultisigBase is Simulator { }); } - function _execTransationCalldata(IGnosisSafe _safe, bytes memory _data, bytes memory _signatures) internal pure returns (bytes memory) { - return abi.encodeCall( - _safe.execTransaction, - ( - MULTICALL3_ADDRESS, - 0, - _data, - Enum.Operation.DelegateCall, - 0, - 0, - 0, - address(0), - payable(address(0)), - _signatures - ) - ); - } - - function _execTransaction(IGnosisSafe _safe, bytes memory _data, bytes memory _signatures) internal returns (bool) { - return _safe.execTransaction({ - to: MULTICALL3_ADDRESS, - value: 0, - data: _data, - operation: Enum.Operation.DelegateCall, - safeTxGas: 0, - baseGas: 0, - gasPrice: 0, - gasToken: address(0), - refundReceiver: payable(address(0)), - signatures: _signatures - }); - } - function _executeTransaction(IGnosisSafe _safe, IMulticall3.Call3[] memory _calls, bytes memory _signatures) internal returns (Vm.AccountAccess[] memory, SimulationPayload memory) @@ -182,10 +125,61 @@ abstract contract MultisigBase is Simulator { return (accesses, simPayload); } - 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; + function _getTransactionHash(IGnosisSafe _safe, IMulticall3.Call3[] memory calls) internal view returns (bytes32) { + bytes memory data = abi.encodeCall(IMulticall3.aggregate3, (calls)); + return _getTransactionHash(_safe, data); + } + + function _getTransactionHash(IGnosisSafe _safe, bytes memory _data) internal view returns (bytes32) { + return keccak256(_encodeTransactionData(_safe, _data)); + } + + function _encodeTransactionData(IGnosisSafe _safe, bytes memory _data) internal view returns (bytes memory) { + return _safe.encodeTransactionData({ + to: MULTICALL3_ADDRESS, + value: 0, + data: _data, + operation: Enum.Operation.DelegateCall, + safeTxGas: 0, + baseGas: 0, + gasPrice: 0, + gasToken: address(0), + refundReceiver: address(0), + _nonce: _getNonce(_safe) + }); + } + + function _execTransationCalldata(IGnosisSafe _safe, bytes memory _data, bytes memory _signatures) internal pure returns (bytes memory) { + return abi.encodeCall( + _safe.execTransaction, + ( + MULTICALL3_ADDRESS, + 0, + _data, + Enum.Operation.DelegateCall, + 0, + 0, + 0, + address(0), + payable(address(0)), + _signatures + ) + ); + } + + function _execTransaction(IGnosisSafe _safe, bytes memory _data, bytes memory _signatures) internal returns (bool) { + return _safe.execTransaction({ + to: MULTICALL3_ADDRESS, + value: 0, + data: _data, + operation: Enum.Operation.DelegateCall, + safeTxGas: 0, + baseGas: 0, + gasPrice: 0, + gasToken: address(0), + refundReceiver: payable(address(0)), + signatures: _signatures + }); } function prepareSignatures(IGnosisSafe _safe, bytes32 hash, bytes memory _signatures) internal view returns (bytes memory) { @@ -298,13 +292,6 @@ abstract contract MultisigBase is Simulator { return sorted; } - function appendRemainingBytes(bytes memory a1, bytes memory a2) internal pure returns (bytes memory) { - if (a2.length > a1.length) { - a1 = bytes.concat(a1, Bytes.slice(a2, a1.length, a2.length - a1.length)); - } - return a1; - } - function extractOwner(bytes32 dataHash, bytes32 r, bytes32 s, uint8 v) internal pure returns (address) { if (v <= 1) { return address(uint160(uint256(r))); @@ -328,4 +315,17 @@ abstract contract MultisigBase is Simulator { v := and(mload(add(signatures, add(signaturePos, 0x41))), 0xff) } } + + function appendRemainingBytes(bytes memory a1, bytes memory a2) internal pure returns (bytes memory) { + if (a2.length > a1.length) { + a1 = bytes.concat(a1, Bytes.slice(a2, a1.length, a2.length - a1.length)); + } + return a1; + } + + 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 e817916ca2ceab3b9382cc077089a34cd2c83e21 Mon Sep 17 00:00:00 2001 From: Michael de Hoog Date: Sat, 19 Oct 2024 20:53:38 -1000 Subject: [PATCH 12/18] Clean up the overrides --- script/deploy/l1/SetGasLimitBuilder.sol | 17 ++++------ script/universal/MultisigBase.sol | 13 ++++++++ script/universal/MultisigBuilder.sol | 37 ++++---------------- script/universal/NestedMultisigBuilder.sol | 38 +++++++-------------- script/universal/Simulator.sol | 39 +++++++++++----------- 5 files changed, 58 insertions(+), 86 deletions(-) diff --git a/script/deploy/l1/SetGasLimitBuilder.sol b/script/deploy/l1/SetGasLimitBuilder.sol index dde82f5..7614ab0 100644 --- a/script/deploy/l1/SetGasLimitBuilder.sol +++ b/script/deploy/l1/SetGasLimitBuilder.sol @@ -55,19 +55,16 @@ abstract contract SetGasLimitBuilder is MultisigBuilder { nonce = safe.nonce() + _nonceOffset(); } - function _addOverrides(IGnosisSafe _safe) internal view override returns (SimulationStateOverride memory) { - uint256 _nonce = _getNonce(_safe); - return overrideSafeThresholdOwnerAndNonce(address(_safe), DEFAULT_SENDER, _nonce); - } - // We need to expect that the gas limit will have been updated previously in our simulation // Use this override to specifically set the gas limit to the expected update value. - function _addGenericOverrides() internal view override returns (SimulationStateOverride memory) { - SimulationStorageOverride[] memory _stateOverrides = new SimulationStorageOverride[](1); - _stateOverrides[0] = SimulationStorageOverride({ + function _simulationOverrides() internal view override returns (SimulationStateOverride[] memory) { + SimulationStateOverride[] memory _stateOverrides = new SimulationStateOverride[](1); + SimulationStorageOverride[] memory _storageOverrides = new SimulationStorageOverride[](1); + _storageOverrides[0] = SimulationStorageOverride({ key: 0x0000000000000000000000000000000000000000000000000000000000000068, // slot of gas limit value: bytes32(uint(_fromGasLimit())) }); - return SimulationStateOverride({contractAddress: L1_SYSTEM_CONFIG, overrides: _stateOverrides}); + _stateOverrides[0] = SimulationStateOverride({contractAddress: L1_SYSTEM_CONFIG, overrides: _storageOverrides}); + return _stateOverrides; } -} \ No newline at end of file +} diff --git a/script/universal/MultisigBase.sol b/script/universal/MultisigBase.sol index d56585c..edf0318 100644 --- a/script/universal/MultisigBase.sol +++ b/script/universal/MultisigBase.sol @@ -328,4 +328,17 @@ abstract contract MultisigBase is Simulator { calls[0] = call; return calls; } + + // The state change simulation can set the threshold, owner address and/or nonce. + // This allows simulation of the final transaction by overriding the threshold to 1. + // State changes reflected in the simulation as a result of these overrides will + // not be reflected in the prod execution. + function _safeOverrides(IGnosisSafe _safe, address _owner) internal virtual view returns (SimulationStateOverride memory) { + uint256 _nonce = _getNonce(_safe); + return overrideSafeThresholdOwnerAndNonce(address(_safe), _owner, _nonce); + } + + // Tenderly simulations can accept generic state overrides. This hook enables this functionality. + // By default, an empty (no-op) override is returned. + function _simulationOverrides() internal virtual view returns (SimulationStateOverride[] memory overrides_) {} } diff --git a/script/universal/MultisigBuilder.sol b/script/universal/MultisigBuilder.sol index c0ea657..ef98ade 100644 --- a/script/universal/MultisigBuilder.sol +++ b/script/universal/MultisigBuilder.sol @@ -142,7 +142,7 @@ abstract contract MultisigBuilder is MultisigBase { { bytes memory data = abi.encodeCall(IMulticall3.aggregate3, (_calls)); - SimulationStateOverride[] memory overrides = _setOverrides(_safe); + SimulationStateOverride[] memory overrides = _overrides(_safe); bytes memory txData = _execTransationCalldata(_safe, data, genPrevalidatedSignature(msg.sender)); logSimulationLink({ @@ -164,35 +164,12 @@ abstract contract MultisigBuilder is MultisigBase { return (accesses, simPayload); } - // The state change simulation can set the threshold, owner address and/or nonce. - // This allows a non-signing owner to simulate the transaction - // State changes reflected in the simulation as a result of these overrides - // will not be reflected in the prod execution. - // This particular implementation can be overwritten by an inheriting script. The - // default logic is vestigial for backwards compatibility. - function _addOverrides(IGnosisSafe _safe) internal virtual view returns (SimulationStateOverride memory) { - uint256 _nonce = _getNonce(_safe); - return overrideSafeThresholdAndNonce(address(_safe), _nonce); - } - - // Tenderly simulations can accept generic state overrides. This hook enables this functionality. - // By default, an empty (no-op) override is returned - function _addGenericOverrides() internal virtual view returns (SimulationStateOverride memory override_) {} - - function _addMultipleGenericOverrides() - internal - view - virtual - returns (SimulationStateOverride[] memory overrides_) - {} - - function _setOverrides(IGnosisSafe _safe) internal virtual returns (SimulationStateOverride[] memory) { - SimulationStateOverride[] memory extraOverrides = _addMultipleGenericOverrides(); - SimulationStateOverride[] memory overrides = new SimulationStateOverride[](2 + extraOverrides.length); - overrides[0] = _addOverrides(_safe); - overrides[1] = _addGenericOverrides(); - for (uint256 i = 0; i < extraOverrides.length; i++) { - overrides[i + 2] = extraOverrides[i]; + function _overrides(IGnosisSafe _safe) internal returns (SimulationStateOverride[] memory) { + SimulationStateOverride[] memory simOverrides = _simulationOverrides(); + SimulationStateOverride[] memory overrides = new SimulationStateOverride[](1 + simOverrides.length); + overrides[0] = _safeOverrides(_safe, msg.sender); + for (uint256 i = 0; i < simOverrides.length; i++) { + overrides[i + 1] = simOverrides[i]; } return overrides; } diff --git a/script/universal/NestedMultisigBuilder.sol b/script/universal/NestedMultisigBuilder.sol index 4de27e4..9058f2b 100644 --- a/script/universal/NestedMultisigBuilder.sol +++ b/script/universal/NestedMultisigBuilder.sol @@ -152,33 +152,8 @@ abstract contract NestedMultisigBuilder is MultisigBase { bytes memory data = abi.encodeCall(IMulticall3.aggregate3, (_calls)); IMulticall3.Call3[] memory calls = _simulateForSignerCalls(_signerSafe, _safe, data); - // For each safe, determine if a nonce override is needed. At this point, - // no state overrides (i.e. vm.store) have been applied to the Foundry VM, - // meaning the nonce is not yet overriden. Therefore these calls to - // `safe.nonce()` will correctly return the current nonce of the safe. - bool safeNonceOverride = _getNonce(_safe) != _safe.nonce(); - bool signerSafeNonceOverride = _getNonce(_signerSafe) != _signerSafe.nonce(); - // Now define the state overrides for the simulation. - SimulationStateOverride[] memory overrides = new SimulationStateOverride[](2); - // The state change simulation sets the multisig threshold to 1 in the - // simulation to enable an approver to see what the final state change - // will look like upon transaction execution. The multisig threshold - // will not actually change in the transaction execution. - if (safeNonceOverride) { - overrides[0] = overrideSafeThresholdAndNonce(address(_safe), _getNonce(_safe)); - } else { - overrides[0] = overrideSafeThreshold(address(_safe)); - } - // Set the signer safe threshold to 1, and set the owner to multicall. - // This is a little hacky; reason is to simulate both the approve hash - // and the final tx in a single Tenderly tx, using multicall. Given an - // EOA cannot DELEGATECALL, multicall needs to own the signer safe. - if (signerSafeNonceOverride) { - overrides[1] = overrideSafeThresholdOwnerAndNonce(address(_signerSafe), MULTICALL3_ADDRESS, _getNonce(_signerSafe)); - } else { - overrides[1] = overrideSafeThresholdAndOwner(address(_signerSafe), MULTICALL3_ADDRESS); - } + SimulationStateOverride[] memory overrides = _overrides(_signerSafe, _safe); bytes memory txData = abi.encodeCall(IMulticall3.aggregate3, (calls)); console.log("---\nSimulation link:"); @@ -233,4 +208,15 @@ abstract contract NestedMultisigBuilder is MultisigBase { return calls; } + + function _overrides(IGnosisSafe _signerSafe, IGnosisSafe _safe) internal view returns (SimulationStateOverride[] memory) { + SimulationStateOverride[] memory simOverrides = _simulationOverrides(); + SimulationStateOverride[] memory overrides = new SimulationStateOverride[](2 + simOverrides.length); + overrides[0] = _safeOverrides(_signerSafe, MULTICALL3_ADDRESS); + overrides[1] = _safeOverrides(_safe, msg.sender); + for (uint256 i = 0; i < simOverrides.length; i++) { + overrides[i + 2] = simOverrides[i]; + } + return overrides; + } } diff --git a/script/universal/Simulator.sol b/script/universal/Simulator.sol index 39e7c32..d10c095 100644 --- a/script/universal/Simulator.sol +++ b/script/universal/Simulator.sol @@ -48,32 +48,23 @@ abstract contract Simulator is CommonBase { return accesses; } - function overrideSafeThreshold(address _safe) public pure returns (SimulationStateOverride memory) { - return addThresholdOverride(SimulationStateOverride({ + function overrideSafeThresholdOwnerAndNonce(address _safe, address _owner, uint256 _nonce) public view returns (SimulationStateOverride memory) { + SimulationStateOverride memory state = SimulationStateOverride({ contractAddress: _safe, overrides: new SimulationStorageOverride[](0) - })); - } - - function overrideSafeThresholdAndNonce(address _safe, uint256 _nonce) public view returns (SimulationStateOverride memory) { - SimulationStateOverride memory state = overrideSafeThreshold(_safe); + }); + state = addThresholdOverride(_safe, state); + state = addOwnerOverride(_safe, state, _owner); state = addNonceOverride(_safe, state, _nonce); return state; } - function overrideSafeThresholdAndOwner(address _safe, address _owner) public pure returns (SimulationStateOverride memory) { - SimulationStateOverride memory state = overrideSafeThreshold(_safe); - state = addOwnerOverride(state, _owner); - return state; - } + function addThresholdOverride(address _safe, SimulationStateOverride memory _state) internal view returns (SimulationStateOverride memory) { + // get the threshold and check if we need to override it + (, bytes memory thresholdBytes) = _safe.staticcall(abi.encodeWithSignature("getThreshold()")); + uint256 threshold = abi.decode(thresholdBytes, (uint256)); + if (threshold == 1) return _state; - function overrideSafeThresholdOwnerAndNonce(address _safe, address _owner, uint256 _nonce) public view returns (SimulationStateOverride memory) { - SimulationStateOverride memory state = overrideSafeThresholdAndOwner(_safe, _owner); - state = addNonceOverride(_safe, state, _nonce); - return state; - } - - function addThresholdOverride(SimulationStateOverride memory _state) internal pure returns (SimulationStateOverride memory) { // set the threshold (slot 4) to 1 return addOverride(_state, SimulationStorageOverride({ key: bytes32(uint256(0x4)), @@ -81,7 +72,14 @@ abstract contract Simulator is CommonBase { })); } - function addOwnerOverride(SimulationStateOverride memory _state, address _owner) internal pure returns (SimulationStateOverride memory) { + function addOwnerOverride(address _safe, SimulationStateOverride memory _state, address _owner) internal view returns (SimulationStateOverride memory) { + // get the owners and check if _owner is an owner + (, bytes memory ownersBytes) = _safe.staticcall(abi.encodeWithSignature("getOwners()")); + address[] memory owners = abi.decode(ownersBytes, (address[])); + for (uint256 i; i < owners.length; i++) { + if (owners[i] == _owner) return _state; + } + // set the ownerCount (slot 3) to 1 _state = addOverride(_state, SimulationStorageOverride({ key: bytes32(uint256(0x3)), @@ -103,6 +101,7 @@ abstract contract Simulator is CommonBase { (, bytes memory nonceBytes) = _safe.staticcall(abi.encodeWithSignature("nonce()")); uint256 nonce = abi.decode(nonceBytes, (uint256)); if (nonce == _nonce) return _state; + // set the nonce (slot 5) to the desired value return addOverride(_state, SimulationStorageOverride({ key: bytes32(uint256(0x5)), From 2eb1634ebe94009571feb491276a5b51408f981d Mon Sep 17 00:00:00 2001 From: Michael de Hoog Date: Sat, 19 Oct 2024 21:08:12 -1000 Subject: [PATCH 13/18] Consistency cleanup --- script/universal/MultisigBuilder.sol | 30 ++++++++++++---------- script/universal/NestedMultisigBuilder.sol | 17 ++++++++++-- 2 files changed, 31 insertions(+), 16 deletions(-) diff --git a/script/universal/MultisigBuilder.sol b/script/universal/MultisigBuilder.sol index ef98ade..30e907f 100644 --- a/script/universal/MultisigBuilder.sol +++ b/script/universal/MultisigBuilder.sol @@ -67,7 +67,7 @@ abstract contract MultisigBuilder is MultisigBase { // 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(safe); + uint256 _nonce = _getNonce(safe); IMulticall3.Call3[] memory calls = _buildCalls(); (Vm.AccountAccess[] memory accesses, SimulationPayload memory simPayload) = _simulateForSigner(safe, calls); @@ -75,13 +75,13 @@ abstract contract MultisigBuilder is MultisigBase { _postCheck(); // Restore the original nonce. - vm.store(address(safe), SAFE_NONCE_SLOT, bytes32(uint256(originalNonce))); + vm.store(address(safe), SAFE_NONCE_SLOT, bytes32(_nonce)); _printDataToSign(safe, calls); } /** - * Step 2 + * 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. @@ -90,23 +90,17 @@ abstract contract MultisigBuilder is MultisigBase { _checkSignatures(IGnosisSafe(_ownerSafe()), _buildCalls(), _signatures); } - function nonce() public view { - IGnosisSafe safe = IGnosisSafe(_ownerSafe()); - console.log("Nonce:", safe.nonce()); - } - /** - * Step 3 + * Step 1.2 (optional) * ====== - * Simulate the transaction. This method should be called by the final member of the multisig + * Simulate the transaction. This method can be called by the final member of the multisig * that will execute the transaction. Signatures from step 1 are required. * * Differs from `run` in that you can override the safe nonce for simulation purposes. */ function simulate(bytes memory _signatures) public { IGnosisSafe safe = IGnosisSafe(_ownerSafe()); - uint256 _nonce = _getNonce(safe); - vm.store(address(safe), SAFE_NONCE_SLOT, bytes32(uint256(_nonce))); + vm.store(address(safe), SAFE_NONCE_SLOT, bytes32(_getNonce(safe))); (Vm.AccountAccess[] memory accesses, SimulationPayload memory simPayload) = _executeTransaction(safe, _buildCalls(), _signatures); @@ -115,7 +109,7 @@ abstract contract MultisigBuilder is MultisigBase { } /** - * Step 4 + * Step 2 * ====== * Execute the transaction. This method should be called by the final member of the multisig * that will execute the transaction. Signatures from step 1 are required. @@ -132,6 +126,14 @@ abstract contract MultisigBuilder is MultisigBase { _postCheck(); } + /** + * Print the current safe nonce. + */ + function nonce() public view { + IGnosisSafe safe = IGnosisSafe(_ownerSafe()); + console.log("Nonce:", safe.nonce()); + } + function _readFrom_SAFE_NONCE() internal pure override returns (bool) { return true; } @@ -164,7 +166,7 @@ abstract contract MultisigBuilder is MultisigBase { return (accesses, simPayload); } - function _overrides(IGnosisSafe _safe) internal returns (SimulationStateOverride[] memory) { + function _overrides(IGnosisSafe _safe) internal view returns (SimulationStateOverride[] memory) { SimulationStateOverride[] memory simOverrides = _simulationOverrides(); SimulationStateOverride[] memory overrides = new SimulationStateOverride[](1 + simOverrides.length); overrides[0] = _safeOverrides(_safe, msg.sender); diff --git a/script/universal/NestedMultisigBuilder.sol b/script/universal/NestedMultisigBuilder.sol index 9058f2b..e4d9b79 100644 --- a/script/universal/NestedMultisigBuilder.sol +++ b/script/universal/NestedMultisigBuilder.sol @@ -82,12 +82,25 @@ abstract contract NestedMultisigBuilder is MultisigBase { _postCheck(); // Restore the original nonce. - vm.store(address(nestedSafe), SAFE_NONCE_SLOT, bytes32(uint256(originalNonce))); - vm.store(address(_signerSafe), SAFE_NONCE_SLOT, bytes32(uint256(originalSignerNonce))); + vm.store(address(nestedSafe), SAFE_NONCE_SLOT, bytes32(originalNonce)); + vm.store(address(_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(IGnosisSafe _signerSafe, bytes memory _signatures) public view { + IGnosisSafe nestedSafe = IGnosisSafe(_ownerSafe()); + IMulticall3.Call3[] memory nestedCalls = _buildCalls(); + IMulticall3.Call3 memory call = _generateApproveCall(nestedSafe, nestedCalls); + _checkSignatures(_signerSafe, toArray(call), _signatures); + } + /** * Step 2 * ====== From e999eb776b292f7e60d147fb2f364b4f51e91117 Mon Sep 17 00:00:00 2001 From: Michael de Hoog Date: Sat, 19 Oct 2024 21:23:55 -1000 Subject: [PATCH 14/18] Extract Signatures code to separate library --- script/universal/MultisigBase.sol | 151 +-------------------- script/universal/MultisigBuilder.sol | 2 +- script/universal/NestedMultisigBuilder.sol | 4 +- script/universal/Signatures.sol | 145 ++++++++++++++++++++ 4 files changed, 153 insertions(+), 149 deletions(-) create mode 100644 script/universal/Signatures.sol diff --git a/script/universal/MultisigBase.sol b/script/universal/MultisigBase.sol index edf0318..55f14fa 100644 --- a/script/universal/MultisigBase.sol +++ b/script/universal/MultisigBase.sol @@ -1,12 +1,12 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.15; +import { Vm } from "forge-std/Vm.sol"; import {console} from "forge-std/console.sol"; import {IMulticall3} from "forge-std/interfaces/IMulticall3.sol"; import {IGnosisSafe, Enum} from "./IGnosisSafe.sol"; -import {Bytes} from "@eth-optimism-bedrock/src/libraries/Bytes.sol"; -import {LibSort} from "@solady/utils/LibSort.sol"; -import "./Simulator.sol"; +import {Simulator} from "./Simulator.sol"; +import {Signatures} from "./Signatures.sol"; abstract contract MultisigBase is Simulator { bytes32 internal constant SAFE_NONCE_SLOT = bytes32(uint256(5)); @@ -84,7 +84,7 @@ abstract contract MultisigBase is Simulator { { bytes memory data = abi.encodeCall(IMulticall3.aggregate3, (_calls)); bytes32 hash = _getTransactionHash(_safe, data); - _signatures = prepareSignatures(_safe, hash, _signatures); + _signatures = Signatures.prepareSignatures(_safe, hash, _signatures); _safe.checkSignatures({ dataHash: hash, @@ -99,7 +99,7 @@ abstract contract MultisigBase is Simulator { { bytes memory data = abi.encodeCall(IMulticall3.aggregate3, (_calls)); bytes32 hash = _getTransactionHash(_safe, data); - _signatures = prepareSignatures(_safe, hash, _signatures); + _signatures = Signatures.prepareSignatures(_safe, hash, _signatures); bytes memory simData = _execTransationCalldata(_safe, data, _signatures); logSimulationLink({ @@ -182,147 +182,6 @@ abstract contract MultisigBase is Simulator { }); } - function prepareSignatures(IGnosisSafe _safe, bytes32 hash, bytes memory _signatures) internal view returns (bytes memory) { - // prepend the prevalidated signatures to the signatures - address[] memory approvers = _getApprovers(_safe, hash); - bytes memory prevalidatedSignatures = genPrevalidatedSignatures(approvers); - _signatures = bytes.concat(prevalidatedSignatures, _signatures); - - // safe requires all signatures to be unique, and sorted ascending by public key - return sortUniqueSignatures(_signatures, hash, _safe.getThreshold(), prevalidatedSignatures.length); - } - - function genPrevalidatedSignatures(address[] memory _addresses) internal pure returns (bytes memory) { - LibSort.sort(_addresses); - bytes memory signatures; - for (uint256 i; i < _addresses.length; i++) { - signatures = bytes.concat(signatures, genPrevalidatedSignature(_addresses[i])); - } - return signatures; - } - - function genPrevalidatedSignature(address _address) internal pure returns (bytes memory) { - uint8 v = 1; - bytes32 s = bytes32(0); - bytes32 r = bytes32(uint256(uint160(_address))); - return abi.encodePacked(r, s, v); - } - - function _getApprovers(IGnosisSafe _safe, bytes32 hash) internal view returns (address[] memory) { - // get a list of owners that have approved this transaction - uint256 threshold = _safe.getThreshold(); - address[] memory owners = _safe.getOwners(); - address[] memory approvers = new address[](threshold); - uint256 approverIndex; - for (uint256 i; i < owners.length; i++) { - address owner = owners[i]; - uint256 approved = _safe.approvedHashes(owner, hash); - if (approved == 1) { - approvers[approverIndex] = owner; - approverIndex++; - if (approverIndex == threshold) { - return approvers; - } - } - } - address[] memory subset = new address[](approverIndex); - for (uint256 i; i < approverIndex; i++) { - subset[i] = approvers[i]; - } - return subset; - } - - /** - * @notice Sorts the signatures in ascending order of the signer's address, and removes any duplicates. - * @dev see https://github.com/safe-global/safe-smart-account/blob/1ed486bb148fe40c26be58d1b517cec163980027/contracts/Safe.sol#L265-L334 - * @param _signatures Signature data that should be verified. - * Can be packed ECDSA signature ({bytes32 r}{bytes32 s}{uint8 v}), contract signature (EIP-1271) or approved hash. - * Can be suffixed with EIP-1271 signatures after threshold*65 bytes. - * @param dataHash Hash that is signed. - * @param threshold Number of signatures required to approve the transaction. - * @param dynamicOffset Offset to add to the `s` value of any EIP-1271 signature. - * Can be used to accomodate any additional signatures prepended to the array. - * If prevalidated signatures were prepended, this should be the length of those signatures. - */ - function sortUniqueSignatures(bytes memory _signatures, bytes32 dataHash, uint256 threshold, uint256 dynamicOffset) internal pure returns (bytes memory) { - bytes memory sorted; - uint256 count = uint256(_signatures.length / 0x41); - uint256[] memory addressesAndIndexes = new uint256[](threshold); - address[] memory uniqueAddresses = new address[](threshold); - uint8 v; - bytes32 r; - bytes32 s; - uint256 j; - for (uint256 i; i < count; i++) { - (v, r, s) = signatureSplit(_signatures, i); - address owner = extractOwner(dataHash, r, s, v); - - // skip duplicate owners - uint256 k; - for (; k < j; k++) { - if (uniqueAddresses[k] == owner) break; - } - if (k < j) continue; - - uniqueAddresses[j] = owner; - addressesAndIndexes[j] = uint256(uint256(uint160(owner)) << 0x60 | i); // address in first 160 bits, index in second 96 bits - j++; - - // we have enough signatures to reach the threshold - if (j == threshold) break; - } - require(j == threshold, "not enough signatures"); - - LibSort.sort(addressesAndIndexes); - for (uint256 i; i < count; i++) { - uint256 index = addressesAndIndexes[i] & 0xffffffff; - (v, r, s) = signatureSplit(_signatures, index); - if (v == 0) { - // The `s` value is used by safe as a lookup into the signature bytes. - // Increment by the offset so that the lookup location remains correct. - s = bytes32(uint256(s) + dynamicOffset); - } - sorted = bytes.concat(sorted, abi.encodePacked(r, s, v)); - } - - // append the non-static part of the signatures (can contain EIP-1271 signatures if contracts are signers) - // if there were any duplicates detected above, they will be safely ignored by Safe's checkNSignatures method - sorted = appendRemainingBytes(sorted, _signatures); - - return sorted; - } - - function extractOwner(bytes32 dataHash, bytes32 r, bytes32 s, uint8 v) internal pure returns (address) { - if (v <= 1) { - return address(uint160(uint256(r))); - } - if (v > 30) { - return ecrecover(keccak256(abi.encodePacked("\x19Ethereum Signed Message:\n32", dataHash)), v - 4, r, s); - } - return ecrecover(dataHash, v, r, s); - } - - // see https://github.com/safe-global/safe-contracts/blob/1ed486bb148fe40c26be58d1b517cec163980027/contracts/common/SignatureDecoder.sol - function signatureSplit(bytes memory signatures, uint256 pos) - internal - pure - returns (uint8 v, bytes32 r, bytes32 s) - { - assembly { - let signaturePos := mul(0x41, pos) - r := mload(add(signatures, add(signaturePos, 0x20))) - s := mload(add(signatures, add(signaturePos, 0x40))) - v := and(mload(add(signatures, add(signaturePos, 0x41))), 0xff) - } - } - - function appendRemainingBytes(bytes memory a1, bytes memory a2) internal pure returns (bytes memory) { - if (a2.length > a1.length) { - a1 = bytes.concat(a1, Bytes.slice(a2, a1.length, a2.length - a1.length)); - } - return a1; - } - function toArray(IMulticall3.Call3 memory call) internal pure returns (IMulticall3.Call3[] memory) { IMulticall3.Call3[] memory calls = new IMulticall3.Call3[](1); calls[0] = call; diff --git a/script/universal/MultisigBuilder.sol b/script/universal/MultisigBuilder.sol index 30e907f..c834c41 100644 --- a/script/universal/MultisigBuilder.sol +++ b/script/universal/MultisigBuilder.sol @@ -146,7 +146,7 @@ abstract contract MultisigBuilder is MultisigBase { SimulationStateOverride[] memory overrides = _overrides(_safe); - bytes memory txData = _execTransationCalldata(_safe, data, genPrevalidatedSignature(msg.sender)); + bytes memory txData = _execTransationCalldata(_safe, data, Signatures.genPrevalidatedSignature(msg.sender)); logSimulationLink({ _to: address(_safe), _data: txData, diff --git a/script/universal/NestedMultisigBuilder.sol b/script/universal/NestedMultisigBuilder.sol index e4d9b79..514e46b 100644 --- a/script/universal/NestedMultisigBuilder.sol +++ b/script/universal/NestedMultisigBuilder.sol @@ -204,7 +204,7 @@ abstract contract NestedMultisigBuilder is MultisigBase { callData: abi.encodeCall(_safe.approveHash, (hash)) }) ))); - bytes memory approveHashExec = _execTransationCalldata(_signerSafe, approveHashData, genPrevalidatedSignature(MULTICALL3_ADDRESS)); + bytes memory approveHashExec = _execTransationCalldata(_signerSafe, approveHashData, Signatures.genPrevalidatedSignature(MULTICALL3_ADDRESS)); calls[0] = IMulticall3.Call3({ target: address(_signerSafe), allowFailure: false, @@ -212,7 +212,7 @@ abstract contract NestedMultisigBuilder is MultisigBase { }); // simulate the final state changes tx, so that signer can verify the final results - bytes memory finalExec = _execTransationCalldata(_safe, _data, genPrevalidatedSignature(address(_signerSafe))); + bytes memory finalExec = _execTransationCalldata(_safe, _data, Signatures.genPrevalidatedSignature(address(_signerSafe))); calls[1] = IMulticall3.Call3({ target: address(_safe), allowFailure: false, diff --git a/script/universal/Signatures.sol b/script/universal/Signatures.sol new file mode 100644 index 0000000..7d1b2e9 --- /dev/null +++ b/script/universal/Signatures.sol @@ -0,0 +1,145 @@ +// SPDX-License-Identifier: MIT +pragma solidity ^0.8.15; + +import {Bytes} from "@eth-optimism-bedrock/src/libraries/Bytes.sol"; +import {LibSort} from "@solady/utils/LibSort.sol"; +import {IGnosisSafe} from "./IGnosisSafe.sol"; + +library Signatures { + function prepareSignatures(IGnosisSafe _safe, bytes32 hash, bytes memory _signatures) internal view returns (bytes memory) { + // prepend the prevalidated signatures to the signatures + address[] memory approvers = getApprovers(_safe, hash); + bytes memory prevalidatedSignatures = genPrevalidatedSignatures(approvers); + _signatures = bytes.concat(prevalidatedSignatures, _signatures); + + // safe requires all signatures to be unique, and sorted ascending by public key + return sortUniqueSignatures(_signatures, hash, _safe.getThreshold(), prevalidatedSignatures.length); + } + + function genPrevalidatedSignatures(address[] memory _addresses) internal pure returns (bytes memory) { + LibSort.sort(_addresses); + bytes memory signatures; + for (uint256 i; i < _addresses.length; i++) { + signatures = bytes.concat(signatures, genPrevalidatedSignature(_addresses[i])); + } + return signatures; + } + + function genPrevalidatedSignature(address _address) internal pure returns (bytes memory) { + uint8 v = 1; + bytes32 s = bytes32(0); + bytes32 r = bytes32(uint256(uint160(_address))); + return abi.encodePacked(r, s, v); + } + + function getApprovers(IGnosisSafe _safe, bytes32 hash) internal view returns (address[] memory) { + // get a list of owners that have approved this transaction + uint256 threshold = _safe.getThreshold(); + address[] memory owners = _safe.getOwners(); + address[] memory approvers = new address[](threshold); + uint256 approverIndex; + for (uint256 i; i < owners.length; i++) { + address owner = owners[i]; + uint256 approved = _safe.approvedHashes(owner, hash); + if (approved == 1) { + approvers[approverIndex] = owner; + approverIndex++; + if (approverIndex == threshold) { + return approvers; + } + } + } + address[] memory subset = new address[](approverIndex); + for (uint256 i; i < approverIndex; i++) { + subset[i] = approvers[i]; + } + return subset; + } + + /** + * @notice Sorts the signatures in ascending order of the signer's address, and removes any duplicates. + * @dev see https://github.com/safe-global/safe-smart-account/blob/1ed486bb148fe40c26be58d1b517cec163980027/contracts/Safe.sol#L265-L334 + * @param _signatures Signature data that should be verified. + * Can be packed ECDSA signature ({bytes32 r}{bytes32 s}{uint8 v}), contract signature (EIP-1271) or approved hash. + * Can be suffixed with EIP-1271 signatures after threshold*65 bytes. + * @param dataHash Hash that is signed. + * @param threshold Number of signatures required to approve the transaction. + * @param dynamicOffset Offset to add to the `s` value of any EIP-1271 signature. + * Can be used to accomodate any additional signatures prepended to the array. + * If prevalidated signatures were prepended, this should be the length of those signatures. + */ + function sortUniqueSignatures(bytes memory _signatures, bytes32 dataHash, uint256 threshold, uint256 dynamicOffset) internal pure returns (bytes memory) { + bytes memory sorted; + uint256 count = uint256(_signatures.length / 0x41); + uint256[] memory addressesAndIndexes = new uint256[](threshold); + address[] memory uniqueAddresses = new address[](threshold); + uint8 v; + bytes32 r; + bytes32 s; + uint256 j; + for (uint256 i; i < count; i++) { + (v, r, s) = signatureSplit(_signatures, i); + address owner = extractOwner(dataHash, r, s, v); + + // skip duplicate owners + uint256 k; + for (; k < j; k++) { + if (uniqueAddresses[k] == owner) break; + } + if (k < j) continue; + + uniqueAddresses[j] = owner; + addressesAndIndexes[j] = uint256(uint256(uint160(owner)) << 0x60 | i); // address in first 160 bits, index in second 96 bits + j++; + + // we have enough signatures to reach the threshold + if (j == threshold) break; + } + require(j == threshold, "not enough signatures"); + + LibSort.sort(addressesAndIndexes); + for (uint256 i; i < count; i++) { + uint256 index = addressesAndIndexes[i] & 0xffffffff; + (v, r, s) = signatureSplit(_signatures, index); + if (v == 0) { + // The `s` value is used by safe as a lookup into the signature bytes. + // Increment by the offset so that the lookup location remains correct. + s = bytes32(uint256(s) + dynamicOffset); + } + sorted = bytes.concat(sorted, abi.encodePacked(r, s, v)); + } + + // append the non-static part of the signatures (can contain EIP-1271 signatures if contracts are signers) + // if there were any duplicates detected above, they will be safely ignored by Safe's checkNSignatures method + sorted = appendRemainingBytes(sorted, _signatures); + + return sorted; + } + + function extractOwner(bytes32 dataHash, bytes32 r, bytes32 s, uint8 v) internal pure returns (address) { + if (v <= 1) { + return address(uint160(uint256(r))); + } + if (v > 30) { + return ecrecover(keccak256(abi.encodePacked("\x19Ethereum Signed Message:\n32", dataHash)), v - 4, r, s); + } + return ecrecover(dataHash, v, r, s); + } + + // see https://github.com/safe-global/safe-contracts/blob/1ed486bb148fe40c26be58d1b517cec163980027/contracts/common/SignatureDecoder.sol + function signatureSplit(bytes memory signatures, uint256 pos) internal pure returns (uint8 v, bytes32 r, bytes32 s) { + assembly { + let signaturePos := mul(0x41, pos) + r := mload(add(signatures, add(signaturePos, 0x20))) + s := mload(add(signatures, add(signaturePos, 0x40))) + v := and(mload(add(signatures, add(signaturePos, 0x41))), 0xff) + } + } + + function appendRemainingBytes(bytes memory a1, bytes memory a2) internal pure returns (bytes memory) { + if (a2.length > a1.length) { + a1 = bytes.concat(a1, Bytes.slice(a2, a1.length, a2.length - a1.length)); + } + return a1; + } +} From 24fbcffd1c3b3135541807edeca192becc245066 Mon Sep 17 00:00:00 2001 From: Michael de Hoog Date: Sat, 19 Oct 2024 21:38:14 -1000 Subject: [PATCH 15/18] Rename Simlator to Simulation, and switch to a library --- script/deploy/l1/SetGasLimitBuilder.sol | 13 ++-- script/universal/MultisigBase.sol | 21 +++---- script/universal/MultisigBuilder.sol | 26 ++++---- script/universal/NestedMultisigBuilder.sol | 28 ++++----- .../{Simulator.sol => Simulation.sol} | 60 ++++++++++--------- 5 files changed, 76 insertions(+), 72 deletions(-) rename script/universal/{Simulator.sol => Simulation.sol} (72%) diff --git a/script/deploy/l1/SetGasLimitBuilder.sol b/script/deploy/l1/SetGasLimitBuilder.sol index 7614ab0..4012a9f 100644 --- a/script/deploy/l1/SetGasLimitBuilder.sol +++ b/script/deploy/l1/SetGasLimitBuilder.sol @@ -5,7 +5,8 @@ import {SystemConfig} from "@eth-optimism-bedrock/src/L1/SystemConfig.sol"; import { MultisigBuilder, IMulticall3, - IGnosisSafe + IGnosisSafe, + Simulation } from "../../universal/MultisigBuilder.sol"; import { Vm } from "forge-std/Vm.sol"; @@ -57,14 +58,14 @@ abstract contract SetGasLimitBuilder is MultisigBuilder { // We need to expect that the gas limit will have been updated previously in our simulation // Use this override to specifically set the gas limit to the expected update value. - function _simulationOverrides() internal view override returns (SimulationStateOverride[] memory) { - SimulationStateOverride[] memory _stateOverrides = new SimulationStateOverride[](1); - SimulationStorageOverride[] memory _storageOverrides = new SimulationStorageOverride[](1); - _storageOverrides[0] = SimulationStorageOverride({ + function _simulationOverrides() internal view override returns (Simulation.StateOverride[] memory) { + Simulation.StateOverride[] memory _stateOverrides = new Simulation.StateOverride[](1); + Simulation.StorageOverride[] memory _storageOverrides = new Simulation.StorageOverride[](1); + _storageOverrides[0] = Simulation.StorageOverride({ key: 0x0000000000000000000000000000000000000000000000000000000000000068, // slot of gas limit value: bytes32(uint(_fromGasLimit())) }); - _stateOverrides[0] = SimulationStateOverride({contractAddress: L1_SYSTEM_CONFIG, overrides: _storageOverrides}); + _stateOverrides[0] = Simulation.StateOverride({contractAddress: L1_SYSTEM_CONFIG, overrides: _storageOverrides}); return _stateOverrides; } } diff --git a/script/universal/MultisigBase.sol b/script/universal/MultisigBase.sol index 55f14fa..5b230e4 100644 --- a/script/universal/MultisigBase.sol +++ b/script/universal/MultisigBase.sol @@ -1,14 +1,15 @@ // SPDX-License-Identifier: MIT pragma solidity ^0.8.15; -import { Vm } from "forge-std/Vm.sol"; +import {Vm} from "forge-std/Vm.sol"; import {console} from "forge-std/console.sol"; +import {CommonBase} from "forge-std/Base.sol"; import {IMulticall3} from "forge-std/interfaces/IMulticall3.sol"; import {IGnosisSafe, Enum} from "./IGnosisSafe.sol"; -import {Simulator} from "./Simulator.sol"; +import {Simulation} from "./Simulation.sol"; import {Signatures} from "./Signatures.sol"; -abstract contract MultisigBase is Simulator { +abstract contract MultisigBase is CommonBase { bytes32 internal constant SAFE_NONCE_SLOT = bytes32(uint256(5)); event DataToSign(bytes); @@ -95,14 +96,14 @@ abstract contract MultisigBase is Simulator { function _executeTransaction(IGnosisSafe _safe, IMulticall3.Call3[] memory _calls, bytes memory _signatures) internal - returns (Vm.AccountAccess[] memory, SimulationPayload memory) + returns (Vm.AccountAccess[] memory, Simulation.Payload memory) { bytes memory data = abi.encodeCall(IMulticall3.aggregate3, (_calls)); bytes32 hash = _getTransactionHash(_safe, data); _signatures = Signatures.prepareSignatures(_safe, hash, _signatures); bytes memory simData = _execTransationCalldata(_safe, data, _signatures); - logSimulationLink({ + Simulation.logSimulationLink({ _to: address(_safe), _from: msg.sender, _data: simData @@ -116,11 +117,11 @@ abstract contract MultisigBase is Simulator { // This can be used to e.g. call out to the Tenderly API and get additional // data about the state diff before broadcasting the transaction. - SimulationPayload memory simPayload = SimulationPayload({ + Simulation.Payload memory simPayload = Simulation.Payload({ from: msg.sender, to: address(_safe), data: simData, - stateOverrides: new SimulationStateOverride[](0) + stateOverrides: new Simulation.StateOverride[](0) }); return (accesses, simPayload); } @@ -192,12 +193,12 @@ abstract contract MultisigBase is Simulator { // This allows simulation of the final transaction by overriding the threshold to 1. // State changes reflected in the simulation as a result of these overrides will // not be reflected in the prod execution. - function _safeOverrides(IGnosisSafe _safe, address _owner) internal virtual view returns (SimulationStateOverride memory) { + function _safeOverrides(IGnosisSafe _safe, address _owner) internal virtual view returns (Simulation.StateOverride memory) { uint256 _nonce = _getNonce(_safe); - return overrideSafeThresholdOwnerAndNonce(address(_safe), _owner, _nonce); + return Simulation.overrideSafeThresholdOwnerAndNonce(address(_safe), _owner, _nonce); } // Tenderly simulations can accept generic state overrides. This hook enables this functionality. // By default, an empty (no-op) override is returned. - function _simulationOverrides() internal virtual view returns (SimulationStateOverride[] memory overrides_) {} + function _simulationOverrides() internal virtual view returns (Simulation.StateOverride[] memory overrides_) {} } diff --git a/script/universal/MultisigBuilder.sol b/script/universal/MultisigBuilder.sol index c834c41..2540aa0 100644 --- a/script/universal/MultisigBuilder.sol +++ b/script/universal/MultisigBuilder.sol @@ -36,13 +36,13 @@ abstract contract MultisigBuilder is MultisigBase { /** * @notice Follow up assertions on state and simulation after a `sign` call. */ - function _postSign(Vm.AccountAccess[] memory accesses, SimulationPayload memory simPayload) internal virtual { + 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, SimulationPayload memory simPayload) internal virtual { + function _postRun(Vm.AccountAccess[] memory accesses, Simulation.Payload memory simPayload) internal virtual { } /** @@ -70,7 +70,7 @@ abstract contract MultisigBuilder is MultisigBase { uint256 _nonce = _getNonce(safe); IMulticall3.Call3[] memory calls = _buildCalls(); - (Vm.AccountAccess[] memory accesses, SimulationPayload memory simPayload) = _simulateForSigner(safe, calls); + (Vm.AccountAccess[] memory accesses, Simulation.Payload memory simPayload) = _simulateForSigner(safe, calls); _postSign(accesses, simPayload); _postCheck(); @@ -102,7 +102,7 @@ abstract contract MultisigBuilder is MultisigBase { IGnosisSafe safe = IGnosisSafe(_ownerSafe()); vm.store(address(safe), SAFE_NONCE_SLOT, bytes32(_getNonce(safe))); - (Vm.AccountAccess[] memory accesses, SimulationPayload memory simPayload) = _executeTransaction(safe, _buildCalls(), _signatures); + (Vm.AccountAccess[] memory accesses, Simulation.Payload memory simPayload) = _executeTransaction(safe, _buildCalls(), _signatures); _postRun(accesses, simPayload); _postCheck(); @@ -119,7 +119,7 @@ abstract contract MultisigBuilder is MultisigBase { */ function run(bytes memory _signatures) public { vm.startBroadcast(); - (Vm.AccountAccess[] memory accesses, SimulationPayload memory simPayload) = _executeTransaction(IGnosisSafe(_ownerSafe()), _buildCalls(), _signatures); + (Vm.AccountAccess[] memory accesses, Simulation.Payload memory simPayload) = _executeTransaction(IGnosisSafe(_ownerSafe()), _buildCalls(), _signatures); vm.stopBroadcast(); _postRun(accesses, simPayload); @@ -140,14 +140,14 @@ abstract contract MultisigBuilder is MultisigBase { function _simulateForSigner(IGnosisSafe _safe, IMulticall3.Call3[] memory _calls) internal - returns (Vm.AccountAccess[] memory, SimulationPayload memory) + returns (Vm.AccountAccess[] memory, Simulation.Payload memory) { bytes memory data = abi.encodeCall(IMulticall3.aggregate3, (_calls)); - SimulationStateOverride[] memory overrides = _overrides(_safe); + Simulation.StateOverride[] memory overrides = _overrides(_safe); bytes memory txData = _execTransationCalldata(_safe, data, Signatures.genPrevalidatedSignature(msg.sender)); - logSimulationLink({ + Simulation.logSimulationLink({ _to: address(_safe), _data: txData, _from: msg.sender, @@ -156,19 +156,19 @@ abstract contract MultisigBuilder is MultisigBase { // Forge simulation of the data logged in the link. If the simulation fails // we revert to make it explicit that the simulation failed. - SimulationPayload memory simPayload = SimulationPayload({ + Simulation.Payload memory simPayload = Simulation.Payload({ to: address(_safe), data: txData, from: msg.sender, stateOverrides: overrides }); - Vm.AccountAccess[] memory accesses = simulateFromSimPayload(simPayload); + Vm.AccountAccess[] memory accesses = Simulation.simulateFromSimPayload(simPayload); return (accesses, simPayload); } - function _overrides(IGnosisSafe _safe) internal view returns (SimulationStateOverride[] memory) { - SimulationStateOverride[] memory simOverrides = _simulationOverrides(); - SimulationStateOverride[] memory overrides = new SimulationStateOverride[](1 + simOverrides.length); + function _overrides(IGnosisSafe _safe) internal view returns (Simulation.StateOverride[] memory) { + Simulation.StateOverride[] memory simOverrides = _simulationOverrides(); + Simulation.StateOverride[] memory overrides = new Simulation.StateOverride[](1 + simOverrides.length); overrides[0] = _safeOverrides(_safe, msg.sender); for (uint256 i = 0; i < simOverrides.length; i++) { overrides[i + 1] = simOverrides[i]; diff --git a/script/universal/NestedMultisigBuilder.sol b/script/universal/NestedMultisigBuilder.sol index 514e46b..b3e1b7c 100644 --- a/script/universal/NestedMultisigBuilder.sol +++ b/script/universal/NestedMultisigBuilder.sol @@ -36,19 +36,19 @@ abstract contract NestedMultisigBuilder is MultisigBase { /** * @notice Follow up assertions on state and simulation after a `sign` call. */ - function _postSign(Vm.AccountAccess[] memory accesses, SimulationPayload memory simPayload) internal virtual { + 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, SimulationPayload memory simPayload) internal virtual { + 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, SimulationPayload memory simPayload) internal virtual { + function _postRun(Vm.AccountAccess[] memory accesses, Simulation.Payload memory simPayload) internal virtual { } /** @@ -77,7 +77,7 @@ abstract contract NestedMultisigBuilder is MultisigBase { IMulticall3.Call3[] memory nestedCalls = _buildCalls(); IMulticall3.Call3 memory call = _generateApproveCall(nestedSafe, nestedCalls); - (Vm.AccountAccess[] memory accesses, SimulationPayload memory simPayload) = _simulateForSigner(_signerSafe, nestedSafe, nestedCalls); + (Vm.AccountAccess[] memory accesses, Simulation.Payload memory simPayload) = _simulateForSigner(_signerSafe, nestedSafe, nestedCalls); _postSign(accesses, simPayload); _postCheck(); @@ -114,7 +114,7 @@ abstract contract NestedMultisigBuilder is MultisigBase { IMulticall3.Call3 memory call = _generateApproveCall(nestedSafe, nestedCalls); vm.startBroadcast(); - (Vm.AccountAccess[] memory accesses, SimulationPayload memory simPayload) = _executeTransaction(_signerSafe, toArray(call), _signatures); + (Vm.AccountAccess[] memory accesses, Simulation.Payload memory simPayload) = _executeTransaction(_signerSafe, toArray(call), _signatures); vm.stopBroadcast(); _postApprove(accesses, simPayload); @@ -134,7 +134,7 @@ abstract contract NestedMultisigBuilder is MultisigBase { bytes memory signatures; vm.startBroadcast(); - (Vm.AccountAccess[] memory accesses, SimulationPayload memory simPayload) = _executeTransaction(nestedSafe, nestedCalls, signatures); + (Vm.AccountAccess[] memory accesses, Simulation.Payload memory simPayload) = _executeTransaction(nestedSafe, nestedCalls, signatures); vm.stopBroadcast(); _postRun(accesses, simPayload); @@ -160,17 +160,17 @@ abstract contract NestedMultisigBuilder is MultisigBase { function _simulateForSigner(IGnosisSafe _signerSafe, IGnosisSafe _safe, IMulticall3.Call3[] memory _calls) internal - returns (Vm.AccountAccess[] memory, SimulationPayload memory) + 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. - SimulationStateOverride[] memory overrides = _overrides(_signerSafe, _safe); + Simulation.StateOverride[] memory overrides = _overrides(_signerSafe, _safe); bytes memory txData = abi.encodeCall(IMulticall3.aggregate3, (calls)); console.log("---\nSimulation link:"); - logSimulationLink({ + Simulation.logSimulationLink({ _to: MULTICALL3_ADDRESS, _data: txData, _from: msg.sender, @@ -179,13 +179,13 @@ abstract contract NestedMultisigBuilder is MultisigBase { // Forge simulation of the data logged in the link. If the simulation fails // we revert to make it explicit that the simulation failed. - SimulationPayload memory simPayload = SimulationPayload({ + Simulation.Payload memory simPayload = Simulation.Payload({ to: MULTICALL3_ADDRESS, data: txData, from: msg.sender, stateOverrides: overrides }); - Vm.AccountAccess[] memory accesses = simulateFromSimPayload(simPayload); + Vm.AccountAccess[] memory accesses = Simulation.simulateFromSimPayload(simPayload); return (accesses, simPayload); } @@ -222,9 +222,9 @@ abstract contract NestedMultisigBuilder is MultisigBase { return calls; } - function _overrides(IGnosisSafe _signerSafe, IGnosisSafe _safe) internal view returns (SimulationStateOverride[] memory) { - SimulationStateOverride[] memory simOverrides = _simulationOverrides(); - SimulationStateOverride[] memory overrides = new SimulationStateOverride[](2 + simOverrides.length); + function _overrides(IGnosisSafe _signerSafe, IGnosisSafe _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, msg.sender); for (uint256 i = 0; i < simOverrides.length; i++) { diff --git a/script/universal/Simulator.sol b/script/universal/Simulation.sol similarity index 72% rename from script/universal/Simulator.sol rename to script/universal/Simulation.sol index d10c095..10ff69b 100644 --- a/script/universal/Simulator.sol +++ b/script/universal/Simulation.sol @@ -2,38 +2,40 @@ pragma solidity ^0.8.15; import { console } from "forge-std/console.sol"; -import { CommonBase } from "forge-std/Base.sol"; import { Vm } from "forge-std/Vm.sol"; -abstract contract Simulator is CommonBase { - struct SimulationStateOverride { +library Simulation { + address internal constant VM_ADDRESS = address(uint160(uint256(keccak256("hevm cheat code")))); + Vm internal constant vm = Vm(VM_ADDRESS); + + struct StateOverride { address contractAddress; - SimulationStorageOverride[] overrides; + StorageOverride[] overrides; } - struct SimulationStorageOverride { + struct StorageOverride { bytes32 key; bytes32 value; } - struct SimulationPayload { + struct Payload { address from; address to; bytes data; - SimulationStateOverride[] stateOverrides; + StateOverride[] stateOverrides; } - function simulateFromSimPayload(SimulationPayload memory simPayload) internal returns (Vm.AccountAccess[] memory) { + function simulateFromSimPayload(Payload memory simPayload) internal returns (Vm.AccountAccess[] memory) { require(simPayload.from != address(0), "Simulator::simulateFromSimPayload: from address cannot be zero address"); require(simPayload.to != address(0), "Simulator::simulateFromSimPayload: to address cannot be zero address"); // Apply state overrides. - SimulationStateOverride[] memory stateOverrides = simPayload.stateOverrides; + StateOverride[] memory stateOverrides = simPayload.stateOverrides; for (uint256 i; i < stateOverrides.length; i++) { - SimulationStateOverride memory stateOverride = stateOverrides[i]; - SimulationStorageOverride[] memory storageOverrides = stateOverride.overrides; + StateOverride memory stateOverride = stateOverrides[i]; + StorageOverride[] memory storageOverrides = stateOverride.overrides; for (uint256 j; j < storageOverrides.length; j++) { - SimulationStorageOverride memory storageOverride = storageOverrides[j]; + StorageOverride memory storageOverride = storageOverrides[j]; vm.store(stateOverride.contractAddress, storageOverride.key, storageOverride.value); } } @@ -48,10 +50,10 @@ abstract contract Simulator is CommonBase { return accesses; } - function overrideSafeThresholdOwnerAndNonce(address _safe, address _owner, uint256 _nonce) public view returns (SimulationStateOverride memory) { - SimulationStateOverride memory state = SimulationStateOverride({ + function overrideSafeThresholdOwnerAndNonce(address _safe, address _owner, uint256 _nonce) public view returns (StateOverride memory) { + StateOverride memory state = StateOverride({ contractAddress: _safe, - overrides: new SimulationStorageOverride[](0) + overrides: new StorageOverride[](0) }); state = addThresholdOverride(_safe, state); state = addOwnerOverride(_safe, state, _owner); @@ -59,20 +61,20 @@ abstract contract Simulator is CommonBase { return state; } - function addThresholdOverride(address _safe, SimulationStateOverride memory _state) internal view returns (SimulationStateOverride memory) { + function addThresholdOverride(address _safe, StateOverride memory _state) internal view returns (StateOverride memory) { // get the threshold and check if we need to override it (, bytes memory thresholdBytes) = _safe.staticcall(abi.encodeWithSignature("getThreshold()")); uint256 threshold = abi.decode(thresholdBytes, (uint256)); if (threshold == 1) return _state; // set the threshold (slot 4) to 1 - return addOverride(_state, SimulationStorageOverride({ + return addOverride(_state, StorageOverride({ key: bytes32(uint256(0x4)), value: bytes32(uint256(0x1)) })); } - function addOwnerOverride(address _safe, SimulationStateOverride memory _state, address _owner) internal view returns (SimulationStateOverride memory) { + function addOwnerOverride(address _safe, StateOverride memory _state, address _owner) internal view returns (StateOverride memory) { // get the owners and check if _owner is an owner (, bytes memory ownersBytes) = _safe.staticcall(abi.encodeWithSignature("getOwners()")); address[] memory owners = abi.decode(ownersBytes, (address[])); @@ -81,58 +83,58 @@ abstract contract Simulator is CommonBase { } // set the ownerCount (slot 3) to 1 - _state = addOverride(_state, SimulationStorageOverride({ + _state = addOverride(_state, StorageOverride({ key: bytes32(uint256(0x3)), value: bytes32(uint256(0x1)) })); // override the owner mapping (slot 2), which requires two key/value pairs: { 0x1: _owner, _owner: 0x1 } - _state = addOverride(_state, SimulationStorageOverride({ + _state = addOverride(_state, StorageOverride({ key: bytes32(0xe90b7bceb6e7df5418fb78d8ee546e97c83a08bbccc01a0644d599ccd2a7c2e0), // keccak256(1 || 2) value: bytes32(uint256(uint160(_owner))) })); - return addOverride(_state, SimulationStorageOverride({ + return addOverride(_state, StorageOverride({ key: keccak256(abi.encode(_owner, uint256(2))), value: bytes32(uint256(0x1)) })); } - function addNonceOverride(address _safe, SimulationStateOverride memory _state, uint256 _nonce) internal view returns (SimulationStateOverride memory) { + function addNonceOverride(address _safe, StateOverride memory _state, uint256 _nonce) internal view returns (StateOverride memory) { // get the nonce and check if we need to override it (, bytes memory nonceBytes) = _safe.staticcall(abi.encodeWithSignature("nonce()")); uint256 nonce = abi.decode(nonceBytes, (uint256)); if (nonce == _nonce) return _state; // set the nonce (slot 5) to the desired value - return addOverride(_state, SimulationStorageOverride({ + return addOverride(_state, StorageOverride({ key: bytes32(uint256(0x5)), value: bytes32(_nonce) })); } - function addOverride(SimulationStateOverride memory _state, SimulationStorageOverride memory _override) internal pure returns (SimulationStateOverride memory) { - SimulationStorageOverride[] memory overrides = new SimulationStorageOverride[](_state.overrides.length + 1); + function addOverride(StateOverride memory _state, StorageOverride memory _override) internal pure returns (StateOverride memory) { + StorageOverride[] memory overrides = new StorageOverride[](_state.overrides.length + 1); for (uint256 i; i < _state.overrides.length; i++) { overrides[i] = _state.overrides[i]; } overrides[_state.overrides.length] = _override; - return SimulationStateOverride({ + return StateOverride({ contractAddress: _state.contractAddress, overrides: overrides }); } function logSimulationLink(address _to, bytes memory _data, address _from) public view { - logSimulationLink(_to, _data, _from, new SimulationStateOverride[](0)); + logSimulationLink(_to, _data, _from, new StateOverride[](0)); } - function logSimulationLink(address _to, bytes memory _data, address _from, SimulationStateOverride[] memory _overrides) public view { + function logSimulationLink(address _to, bytes memory _data, address _from, StateOverride[] memory _overrides) public view { string memory proj = vm.envOr("TENDERLY_PROJECT", string("TENDERLY_PROJECT")); string memory username = vm.envOr("TENDERLY_USERNAME", string("TENDERLY_USERNAME")); // the following characters are url encoded: []{} string memory stateOverrides = "%5B"; for (uint256 i; i < _overrides.length; i++) { - SimulationStateOverride memory _override = _overrides[i]; + StateOverride memory _override = _overrides[i]; if (i > 0) stateOverrides = string.concat(stateOverrides, ","); stateOverrides = string.concat( stateOverrides, From df78b0e5e8153ce582fbc75a2b8562216e03c4b3 Mon Sep 17 00:00:00 2001 From: Michael de Hoog Date: Sat, 19 Oct 2024 21:38:39 -1000 Subject: [PATCH 16/18] Add some simple tests for the *MultisigBuilder classes --- test/universal/Counter.sol | 16 +++ test/universal/MultisigBuilder.t.sol | 67 ++++++++++++ test/universal/NestedMultisigBuilder.t.sol | 116 +++++++++++++++++++++ 3 files changed, 199 insertions(+) create mode 100644 test/universal/Counter.sol create mode 100644 test/universal/MultisigBuilder.t.sol create mode 100644 test/universal/NestedMultisigBuilder.t.sol diff --git a/test/universal/Counter.sol b/test/universal/Counter.sol new file mode 100644 index 0000000..d75e054 --- /dev/null +++ b/test/universal/Counter.sol @@ -0,0 +1,16 @@ +// SPDX-License-Identifier: MIT +pragma solidity 0.8.15; + +contract Counter { + address internal immutable OWNER; + uint256 public count = 0; + + constructor(address owner) { + OWNER = owner; + } + + function increment() external { + require(msg.sender == OWNER, "only owner can increment"); + count += 1; + } +} diff --git a/test/universal/MultisigBuilder.t.sol b/test/universal/MultisigBuilder.t.sol new file mode 100644 index 0000000..4618881 --- /dev/null +++ b/test/universal/MultisigBuilder.t.sol @@ -0,0 +1,67 @@ +// 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 {MultisigBuilder} from "../../script/universal/MultisigBuilder.sol"; +import {IGnosisSafe} from "../../script/universal/IGnosisSafe.sol"; +import {Counter} from "./Counter.sol"; + +contract MultisigBuilderTest is Test, MultisigBuilder { + Vm.Wallet internal wallet1 = vm.createWallet("1"); + Vm.Wallet internal wallet2 = vm.createWallet("2"); + + IGnosisSafe internal safe = IGnosisSafe(address(1001)); + Counter internal counter = new Counter(address(safe)); + + bytes internal dataToSign = hex"1901d4bb33110137810c444c1d9617abe97df097d587ecde64e6fcb38d7f49e1280c41dcff2c17a271265df60d1612a7387110475b6fc5178add5518196db5dba6bd"; + + function setUp() public { + vm.etch(address(safe), Preinstalls.getDeployedCode(Preinstalls.Safe_v130, block.chainid)); + vm.etch(Preinstalls.MultiCall3, Preinstalls.getDeployedCode(Preinstalls.MultiCall3, block.chainid)); + + address[] memory owners = new address[](2); + owners[0] = wallet1.addr; + owners[1] = wallet2.addr; + safe.setup(owners, 2, address(0), "", address(0), address(0), 0, address(0)); + } + + function _postCheck() 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 override view 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 override view returns (address) { + return address(safe); + } + + function test_sign() external { + vm.recordLogs(); + sign(); + Vm.Log[] memory logs = vm.getRecordedLogs(); + assertEq(keccak256(logs[logs.length-1].data), keccak256(abi.encode(dataToSign))); + } + + function test_run() external { + (uint8 v1, bytes32 r1, bytes32 s1) = vm.sign(wallet1, keccak256(dataToSign)); + (uint8 v2, bytes32 r2, bytes32 s2) = vm.sign(wallet2, keccak256(dataToSign)); + bytes memory signatures = abi.encodePacked(r1, s1, v1, r2, s2, v2); + run(signatures); + } +} diff --git a/test/universal/NestedMultisigBuilder.t.sol b/test/universal/NestedMultisigBuilder.t.sol new file mode 100644 index 0000000..f88cdf7 --- /dev/null +++ b/test/universal/NestedMultisigBuilder.t.sol @@ -0,0 +1,116 @@ +// 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 {NestedMultisigBuilder} from "../../script/universal/NestedMultisigBuilder.sol"; +import {IGnosisSafe} from "../../script/universal/IGnosisSafe.sol"; +import {Counter} from "./Counter.sol"; + +contract NestedMultisigBuilderTest is Test, NestedMultisigBuilder { + Vm.Wallet internal wallet1 = vm.createWallet("1"); + Vm.Wallet internal wallet2 = vm.createWallet("2"); + + IGnosisSafe internal safe1 = IGnosisSafe(address(1001)); + IGnosisSafe internal safe2 = IGnosisSafe(address(1002)); + IGnosisSafe internal safe3 = IGnosisSafe(address(1003)); + Counter internal counter = new Counter(address(safe3)); + + bytes internal dataToSign1 = hex"1901d4bb33110137810c444c1d9617abe97df097d587ecde64e6fcb38d7f49e1280c3afd48ea8b0056e1028951ba44695d612396f4a1c3851f4b8a262c53ee1f2503"; + bytes internal dataToSign2 = hex"190132640243d7aade8c72f3d90d2dbf359e9897feba5fce1453bc8d9e7ba10d17153afd48ea8b0056e1028951ba44695d612396f4a1c3851f4b8a262c53ee1f2503"; + + function setUp() public { + bytes memory safeCode = Preinstalls.getDeployedCode(Preinstalls.Safe_v130, block.chainid); + vm.etch(address(safe1), safeCode); + vm.etch(address(safe2), safeCode); + vm.etch(address(safe3), safeCode); + vm.etch(Preinstalls.MultiCall3, Preinstalls.getDeployedCode(Preinstalls.MultiCall3, block.chainid)); + + address[] memory owners1 = new address[](1); + owners1[0] = wallet1.addr; + safe1.setup(owners1, 1, address(0), "", address(0), address(0), 0, address(0)); + + address[] memory owners2 = new address[](1); + owners2[0] = wallet2.addr; + safe2.setup(owners2, 1, address(0), "", address(0), address(0), 0, address(0)); + + address[] memory owners3 = new address[](2); + owners3[0] = address(safe1); + owners3[1] = address(safe2); + safe3.setup(owners3, 2, address(0), "", address(0), address(0), 0, address(0)); + } + + function _postCheck() 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 override view 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 override view returns (address) { + return address(safe3); + } + + function test_sign_safe1() external { + vm.recordLogs(); + sign(safe1); + Vm.Log[] memory logs = vm.getRecordedLogs(); + assertEq(keccak256(logs[logs.length-1].data), keccak256(abi.encode(dataToSign1))); + } + + function test_sign_safe2() external { + vm.recordLogs(); + sign(safe2); + Vm.Log[] memory logs = vm.getRecordedLogs(); + assertEq(keccak256(logs[logs.length-1].data), keccak256(abi.encode(dataToSign2))); + } + + function test_approve_safe1() external { + (uint8 v, bytes32 r, bytes32 s) = vm.sign(wallet1, keccak256(dataToSign1)); + approve(safe1, abi.encodePacked(r, s, v)); + } + + function test_approve_safe2() external { + (uint8 v, bytes32 r, bytes32 s) = vm.sign(wallet2, keccak256(dataToSign2)); + approve(safe2, abi.encodePacked(r, s, v)); + } + + function test_approve_notOwner() external { + (uint8 v, bytes32 r, bytes32 s) = vm.sign(wallet1, keccak256(dataToSign1)); + bytes memory data = abi.encodeCall(this.approve, (safe2, abi.encodePacked(r, s, v))); + (bool success, bytes memory result) = address(this).call(data); + assertFalse(success); + assertEq(result, abi.encodeWithSignature("Error(string)", "GS026")); + } + + function test_run() external { + (uint8 v1, bytes32 r1, bytes32 s1) = vm.sign(wallet1, keccak256(dataToSign1)); + (uint8 v2, bytes32 r2, bytes32 s2) = vm.sign(wallet2, keccak256(dataToSign2)); + approve(safe1, abi.encodePacked(r1, s1, v1)); + approve(safe2, abi.encodePacked(r2, s2, v2)); + run(); + } + + function test_run_notApproved() external { + (uint8 v1, bytes32 r1, bytes32 s1) = vm.sign(wallet1, keccak256(dataToSign1)); + approve(safe1, abi.encodePacked(r1, s1, v1)); + bytes memory data = abi.encodeCall(this.run, ()); + (bool success, bytes memory result) = address(this).call(data); + assertFalse(success); + assertEq(result, abi.encodeWithSignature("Error(string)", "not enough signatures")); + } +} From 1168e6d62dd44be2103ed199d718401acf62c4a8 Mon Sep 17 00:00:00 2001 From: Michael de Hoog Date: Sat, 19 Oct 2024 22:01:15 -1000 Subject: [PATCH 17/18] Pass IGnosisSafe to Simulation library methods --- script/universal/MultisigBase.sol | 2 +- script/universal/Simulation.sol | 22 +++++++++------------- 2 files changed, 10 insertions(+), 14 deletions(-) diff --git a/script/universal/MultisigBase.sol b/script/universal/MultisigBase.sol index 5b230e4..cc46694 100644 --- a/script/universal/MultisigBase.sol +++ b/script/universal/MultisigBase.sol @@ -195,7 +195,7 @@ abstract contract MultisigBase is CommonBase { // not be reflected in the prod execution. function _safeOverrides(IGnosisSafe _safe, address _owner) internal virtual view returns (Simulation.StateOverride memory) { uint256 _nonce = _getNonce(_safe); - return Simulation.overrideSafeThresholdOwnerAndNonce(address(_safe), _owner, _nonce); + return Simulation.overrideSafeThresholdOwnerAndNonce(_safe, _owner, _nonce); } // Tenderly simulations can accept generic state overrides. This hook enables this functionality. diff --git a/script/universal/Simulation.sol b/script/universal/Simulation.sol index 10ff69b..77445b3 100644 --- a/script/universal/Simulation.sol +++ b/script/universal/Simulation.sol @@ -3,6 +3,7 @@ pragma solidity ^0.8.15; import { console } from "forge-std/console.sol"; import { Vm } from "forge-std/Vm.sol"; +import { IGnosisSafe } from "./IGnosisSafe.sol"; library Simulation { address internal constant VM_ADDRESS = address(uint160(uint256(keccak256("hevm cheat code")))); @@ -50,9 +51,9 @@ library Simulation { return accesses; } - function overrideSafeThresholdOwnerAndNonce(address _safe, address _owner, uint256 _nonce) public view returns (StateOverride memory) { + function overrideSafeThresholdOwnerAndNonce(IGnosisSafe _safe, address _owner, uint256 _nonce) public view returns (StateOverride memory) { StateOverride memory state = StateOverride({ - contractAddress: _safe, + contractAddress: address(_safe), overrides: new StorageOverride[](0) }); state = addThresholdOverride(_safe, state); @@ -61,11 +62,9 @@ library Simulation { return state; } - function addThresholdOverride(address _safe, StateOverride memory _state) internal view returns (StateOverride memory) { + function addThresholdOverride(IGnosisSafe _safe, StateOverride memory _state) internal view returns (StateOverride memory) { // get the threshold and check if we need to override it - (, bytes memory thresholdBytes) = _safe.staticcall(abi.encodeWithSignature("getThreshold()")); - uint256 threshold = abi.decode(thresholdBytes, (uint256)); - if (threshold == 1) return _state; + if (_safe.getThreshold() == 1) return _state; // set the threshold (slot 4) to 1 return addOverride(_state, StorageOverride({ @@ -74,10 +73,9 @@ library Simulation { })); } - function addOwnerOverride(address _safe, StateOverride memory _state, address _owner) internal view returns (StateOverride memory) { + function addOwnerOverride(IGnosisSafe _safe, StateOverride memory _state, address _owner) internal view returns (StateOverride memory) { // get the owners and check if _owner is an owner - (, bytes memory ownersBytes) = _safe.staticcall(abi.encodeWithSignature("getOwners()")); - address[] memory owners = abi.decode(ownersBytes, (address[])); + address[] memory owners = _safe.getOwners(); for (uint256 i; i < owners.length; i++) { if (owners[i] == _owner) return _state; } @@ -98,11 +96,9 @@ library Simulation { })); } - function addNonceOverride(address _safe, StateOverride memory _state, uint256 _nonce) internal view returns (StateOverride memory) { + function addNonceOverride(IGnosisSafe _safe, StateOverride memory _state, uint256 _nonce) internal view returns (StateOverride memory) { // get the nonce and check if we need to override it - (, bytes memory nonceBytes) = _safe.staticcall(abi.encodeWithSignature("nonce()")); - uint256 nonce = abi.decode(nonceBytes, (uint256)); - if (nonce == _nonce) return _state; + if (_safe.nonce() == _nonce) return _state; // set the nonce (slot 5) to the desired value return addOverride(_state, StorageOverride({ From 582e4feaa5fb2592c00d4d9b8895e3f832a1c583 Mon Sep 17 00:00:00 2001 From: Michael de Hoog Date: Sat, 19 Oct 2024 22:32:29 -1000 Subject: [PATCH 18/18] Move _toArray helper to NestedMultisigBuilder --- script/universal/MultisigBase.sol | 6 ------ script/universal/NestedMultisigBuilder.sol | 14 ++++++++++---- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/script/universal/MultisigBase.sol b/script/universal/MultisigBase.sol index cc46694..792df56 100644 --- a/script/universal/MultisigBase.sol +++ b/script/universal/MultisigBase.sol @@ -183,12 +183,6 @@ abstract contract MultisigBase is CommonBase { }); } - 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; - } - // The state change simulation can set the threshold, owner address and/or nonce. // This allows simulation of the final transaction by overriding the threshold to 1. // State changes reflected in the simulation as a result of these overrides will diff --git a/script/universal/NestedMultisigBuilder.sol b/script/universal/NestedMultisigBuilder.sol index b3e1b7c..b0b0c1f 100644 --- a/script/universal/NestedMultisigBuilder.sol +++ b/script/universal/NestedMultisigBuilder.sol @@ -85,7 +85,7 @@ abstract contract NestedMultisigBuilder is MultisigBase { vm.store(address(nestedSafe), SAFE_NONCE_SLOT, bytes32(originalNonce)); vm.store(address(_signerSafe), SAFE_NONCE_SLOT, bytes32(originalSignerNonce)); - _printDataToSign(_signerSafe, toArray(call)); + _printDataToSign(_signerSafe, _toArray(call)); } /** @@ -98,7 +98,7 @@ abstract contract NestedMultisigBuilder is MultisigBase { IGnosisSafe nestedSafe = IGnosisSafe(_ownerSafe()); IMulticall3.Call3[] memory nestedCalls = _buildCalls(); IMulticall3.Call3 memory call = _generateApproveCall(nestedSafe, nestedCalls); - _checkSignatures(_signerSafe, toArray(call), _signatures); + _checkSignatures(_signerSafe, _toArray(call), _signatures); } /** @@ -114,7 +114,7 @@ abstract contract NestedMultisigBuilder is MultisigBase { IMulticall3.Call3 memory call = _generateApproveCall(nestedSafe, nestedCalls); vm.startBroadcast(); - (Vm.AccountAccess[] memory accesses, Simulation.Payload memory simPayload) = _executeTransaction(_signerSafe, toArray(call), _signatures); + (Vm.AccountAccess[] memory accesses, Simulation.Payload memory simPayload) = _executeTransaction(_signerSafe, _toArray(call), _signatures); vm.stopBroadcast(); _postApprove(accesses, simPayload); @@ -197,7 +197,7 @@ abstract contract NestedMultisigBuilder is MultisigBase { 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( + bytes memory approveHashData = abi.encodeCall(IMulticall3.aggregate3, (_toArray( IMulticall3.Call3({ target: address(_safe), allowFailure: false, @@ -232,4 +232,10 @@ abstract contract NestedMultisigBuilder is MultisigBase { } 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; + } }