feat: update storage mirror guard

package.json

@@ -31,6 +31,7 @@
     "package.json": "sort-package-json"
   },
   "dependencies": {
+    "@defi-wonderland/solidity-utils": "0.0.0-3e9c8e8b",
     "ds-test": "github:dapphub/ds-test#e282159",
     "forge-std": "github:foundry-rs/forge-std#v1.5.6",
     "isolmate": "github:defi-wonderland/isolmate#59e1804",
@@ -39,6 +40,7 @@
   "devDependencies": {
     "@commitlint/cli": "17.0.3",
     "@commitlint/config-conventional": "17.0.3",
+    "@gnosis.pm/safe-contracts": "1.3.0",
     "husky": ">=8",
     "lint-staged": ">=10",
     "solhint": "3.3.6",

remappings.txt

@@ -2,6 +2,8 @@ ds-test/=node_modules/ds-test/src
 prb/test/=node_modules/prb/test/src/
 forge-std/=node_modules/forge-std/src
 isolmate/=node_modules/isolmate/src
+safe-contracts/=node_modules/@gnosis.pm/safe-contracts/contracts/
+@defi-wonderland/=node_modules/@defi-wonderland/
 
 contracts/=solidity/contracts
 interfaces/=solidity/interfaces

solidity/contracts/StorageMirror.sol

@@ -12,15 +12,13 @@ contract StorageMirror is IStorageMirror {
   /**
    * @notice The mapping of the safe to the keccak256(abi.encode(SafeSettings))
    */
-
   mapping(address => bytes32) public latestSettingsHash;
 
   /**
    * @notice Updates a safe's settings hash
    * @dev The safe should always be msg.sender
    * @param _safeSettings The settings we are going to update to
    */
-
   function update(SafeSettings memory _safeSettings) external {
     bytes32 _settingsHash = keccak256(abi.encode(_safeSettings));
     latestSettingsHash[msg.sender] = _settingsHash;

solidity/contracts/UpdateStorageMirrorGuard.sol

@@ -0,0 +1,64 @@
+// SPDX-License-Identifier: UNLICENSED
+pragma solidity =0.8.19;
+
+import {Guard} from 'safe-contracts/base/GuardManager.sol';
+import {Enum} from 'safe-contracts/common/Enum.sol';
+import {IGuardCallbackModule} from 'interfaces/IGuardCallbackModule.sol';
+
+/**
+ * @title UpdateStorageMirrorGuard
+ * @notice This guard is responsible for calling the GuardCallbackModule when a change in settings of a safe is executed.
+ */
+contract UpdateStorageMirrorGuard is Guard {
+  /**
+   * @notice The address of the guard callback module
+   */
+  IGuardCallbackModule public immutable GUARD_CALLBACK_MODULE;
+
+  /**
+   * @notice A boolean that returns true if a tx is changing the safe's settings
+   */
+  bool public didSettingsChange;
+
+  /**
+   * @notice The hash of the new settings
+   */
+  bytes32 public settingsHash;
+
+  constructor(IGuardCallbackModule _guardCallbackModule) {
+    GUARD_CALLBACK_MODULE = _guardCallbackModule;
+  }
+
+  /**
+   * @notice Guard hook that is called before a Safe transaction is executed
+   */
+  function checkTransaction(
+    address to,
+    uint256 value,
+    bytes memory data,
+    Enum.Operation operation,
+    uint256 safeTxGas,
+    uint256 baseGas,
+    uint256 gasPrice,
+    address gasToken,
+    address payable refundReceiver,
+    bytes memory signatures,
+    address msgSender
+  ) external {
+    didSettingsChange = true;
+    settingsHash = keccak256(abi.encodePacked('settings'));
+  }
+
+  /**
+   * @notice Guard hook that is called after a Safe transaction is executed
+   * @dev It should call the GuardCallbackModule
+   * @dev The msg.sender should be the safe
+   */
+  function checkAfterExecution(bytes32 _txHash, bool _success) external {
+    if (didSettingsChange && _success) {
+      GUARD_CALLBACK_MODULE.saveUpdatedSettings(msg.sender, settingsHash);
+      didSettingsChange = false;
+      settingsHash = keccak256(abi.encodePacked(''));
+    }
+  }
+}

solidity/interfaces/IGuardCallbackModule.sol

@@ -0,0 +1,6 @@
+// SPDX-License-Identifier: UNLICENSED
+pragma solidity =0.8.19;
+
+interface IGuardCallbackModule {
+  function saveUpdatedSettings(address _safe, bytes32 _settingsHash) external;
+}

solidity/interfaces/IStorageMirror.sol

@@ -13,7 +13,6 @@ interface IStorageMirror {
    * @param _settingsHash The hash of the settings
    * @param _safeSettings The plaintext of the settings
    */
-
   event SettingsUpdated(address indexed _safe, bytes32 indexed _settingsHash, SafeSettings _safeSettings);
 
   /*///////////////////////////////////////////////////////////////

solidity/test/e2e/Common.sol

@@ -1,15 +1,14 @@
 // SPDX-License-Identifier: MIT
 pragma solidity =0.8.19;
 
-import {DSTestFull} from 'test/utils/DSTestFull.sol';
-import {console} from 'forge-std/console.sol';
+import {DSTestPlus} from '@defi-wonderland/solidity-utils/solidity/test/DSTestPlus.sol';
 import {IERC20} from 'isolmate/interfaces/tokens/IERC20.sol';
 
-contract CommonE2EBase is DSTestFull {
+contract CommonE2EBase is DSTestPlus {
   uint256 internal constant _FORK_BLOCK = 15_452_788;
 
-  address internal _user = _label('user');
-  address internal _owner = _label('owner');
+  address internal _user = makeAddr('user');
+  address internal _owner = makeAddr('owner');
 
   function setUp() public {
     vm.createSelectFork(vm.rpcUrl('mainnet'), _FORK_BLOCK);

solidity/test/unit/StorageMirror.t.sol

@@ -1,18 +1,22 @@
 // SPDX-License-Identifier: UNLICENSED
 pragma solidity >=0.8.4 <0.9.0;
 
-import {DSTestFull} from '../utils/DSTestFull.sol';
+import 'forge-std/Test.sol';
 import {StorageMirror} from 'contracts/StorageMirror.sol';
 import {IStorageMirror} from 'interfaces/IStorageMirror.sol';
 
-abstract contract Base is DSTestFull {
-  address internal _safe = _label('safe');
-
-  StorageMirror internal _storageMirror = new StorageMirror();
-
+abstract contract Base is Test {
   event SettingsUpdated(
     address indexed _safe, bytes32 indexed _settingsHash, IStorageMirror.SafeSettings _safeSettings
   );
+
+  address public safe;
+  StorageMirror public storageMirror;
+
+  function setUp() public {
+    safe = makeAddr('safe');
+    storageMirror = new StorageMirror();
+  }
 }
 
 contract UnitStorageMirror is Base {
@@ -23,11 +27,11 @@ contract UnitStorageMirror is Base {
     IStorageMirror.SafeSettings memory _safeSettings =
       IStorageMirror.SafeSettings({owners: _owners, threshold: _threshold});
 
-    vm.prank(_safe);
-    _storageMirror.update(_safeSettings);
+    vm.prank(safe);
+    storageMirror.update(_safeSettings);
 
     bytes32 _settingsHash = keccak256(abi.encode(_safeSettings));
-    bytes32 _savedHash = _storageMirror.latestSettingsHash(_safe);
+    bytes32 _savedHash = storageMirror.latestSettingsHash(safe);
 
     assertEq(_settingsHash, _savedHash, 'Settings hash should be saved');
   }
@@ -41,9 +45,9 @@ contract UnitStorageMirror is Base {
 
     bytes32 _expectedSettingsHash = keccak256(abi.encode(_safeSettings));
 
-    vm.prank(_safe);
+    vm.prank(safe);
     vm.expectEmit(true, true, true, true);
-    emit SettingsUpdated(_safe, _expectedSettingsHash, _safeSettings);
-    _storageMirror.update(_safeSettings);
+    emit SettingsUpdated(safe, _expectedSettingsHash, _safeSettings);
+    storageMirror.update(_safeSettings);
   }
 }

solidity/test/unit/UpdateStorageMirrorGuard.t.sol

@@ -0,0 +1,81 @@
+// SPDX-License-Identifier: UNLICENSED
+pragma solidity >=0.8.4 <0.9.0;
+
+import 'forge-std/Test.sol';
+import {Enum} from 'safe-contracts/common/Enum.sol';
+import {UpdateStorageMirrorGuard} from 'contracts/UpdateStorageMirrorGuard.sol';
+import {IGuardCallbackModule} from 'interfaces/IGuardCallbackModule.sol';
+
+abstract contract Base is Test {
+  address public safe;
+  IGuardCallbackModule public guardCallbackModule;
+  UpdateStorageMirrorGuard public updateStorageMirrorGuard;
+
+  bytes32 public immutable SETTINGS_HASH = keccak256(abi.encodePacked('settings'));
+
+  function setUp() public {
+    safe = makeAddr('safe');
+    guardCallbackModule = IGuardCallbackModule(makeAddr('guardCallbackModule'));
+    updateStorageMirrorGuard = new UpdateStorageMirrorGuard(guardCallbackModule);
+  }
+}
+
+contract UnitUpdateStorageMirrorGuard is Base {
+  function testCheckTransaction() public {
+    assertFalse(updateStorageMirrorGuard.didSettingsChange());
+    assertEq(updateStorageMirrorGuard.settingsHash(), bytes32(''));
+
+    vm.prank(safe);
+    updateStorageMirrorGuard.checkTransaction(
+      address(0), 0, '', Enum.Operation.Call, 0, 0, 0, address(0), payable(0), '', address(0)
+    );
+
+    assertTrue(updateStorageMirrorGuard.didSettingsChange());
+    assertEq(updateStorageMirrorGuard.settingsHash(), keccak256(abi.encodePacked('settings')));
+  }
+
+  function testCheckAfterExecution(bytes32 _txHash) public {
+    // Call checkTransaction to change didSettingsChange to true
+    vm.prank(safe);
+    updateStorageMirrorGuard.checkTransaction(
+      address(0), 0, '', Enum.Operation.Call, 0, 0, 0, address(0), payable(0), '', address(0)
+    );
+
+    vm.mockCall(
+      address(guardCallbackModule),
+      abi.encodeCall(IGuardCallbackModule.saveUpdatedSettings, (safe, SETTINGS_HASH)),
+      abi.encode()
+    );
+    vm.expectCall(
+      address(guardCallbackModule), abi.encodeCall(IGuardCallbackModule.saveUpdatedSettings, (safe, SETTINGS_HASH))
+    );
+    vm.prank(safe);
+    updateStorageMirrorGuard.checkAfterExecution(_txHash, true);
+
+    assertFalse(updateStorageMirrorGuard.didSettingsChange());
+    assertEq(updateStorageMirrorGuard.settingsHash(), keccak256(abi.encodePacked('')));
+  }
+
+  function testCheckAfterExecutionNoSettingsChange(bytes32 _txHash) public {
+    vm.prank(safe);
+    updateStorageMirrorGuard.checkAfterExecution(_txHash, true);
+
+    assertFalse(updateStorageMirrorGuard.didSettingsChange());
+    assertEq(updateStorageMirrorGuard.settingsHash(), bytes32(''));
+  }
+
+  function testCheckAfterExecutionTxFailed(bytes32 _txHash) public {
+    // Call checkTransaction to change didSettingsChange to true
+    vm.prank(safe);
+    updateStorageMirrorGuard.checkTransaction(
+      address(0), 0, '', Enum.Operation.Call, 0, 0, 0, address(0), payable(0), '', address(0)
+    );
+
+    vm.prank(safe);
+    updateStorageMirrorGuard.checkAfterExecution(_txHash, false);
+
+    // Should be true since the tx failed to execute and thus didnt make it to reset
+    assertTrue(updateStorageMirrorGuard.didSettingsChange());
+    assertEq(updateStorageMirrorGuard.settingsHash(), keccak256(abi.encodePacked('settings')));
+  }
+}