Clean up the overrides

base-org · Oct 20, 2024 · e817916 · e817916
1 parent bad79ae
commit e817916
Show file tree

Hide file tree

Showing 5 changed files with 58 additions and 86 deletions.
diff --git 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;
     }
-}
+}
diff --git 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
@@ -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
@@ -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
@@ -48,40 +48,38 @@ 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)),
             value: bytes32(uint256(0x1))
         }));
     }
 
-    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)),