diff --git a/package.json b/package.json index 1325794..73b4170 100644 --- a/package.json +++ b/package.json @@ -35,12 +35,12 @@ "ds-test": "github:dapphub/ds-test#e282159", "forge-std": "github:foundry-rs/forge-std#v1.5.6", "isolmate": "github:defi-wonderland/isolmate#59e1804", - "prb/test": "github:paulrberg/prb-test#a245c71" + "prb/test": "github:paulrberg/prb-test#a245c71", + "safe-contracts": "github:safe-global/safe-contracts#v1.4.1" }, "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", diff --git a/remappings.txt b/remappings.txt index 184ca49..bcb55ae 100644 --- a/remappings.txt +++ b/remappings.txt @@ -2,7 +2,7 @@ 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/ +safe-contracts/=node_modules/safe-contracts/contracts @defi-wonderland/=node_modules/@defi-wonderland/ contracts/=solidity/contracts diff --git a/solidity/contracts/UpdateStorageMirrorGuard.sol b/solidity/contracts/UpdateStorageMirrorGuard.sol index b276c7d..087af43 100644 --- a/solidity/contracts/UpdateStorageMirrorGuard.sol +++ b/solidity/contracts/UpdateStorageMirrorGuard.sol @@ -1,18 +1,24 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity =0.8.19; -import {Guard} from 'safe-contracts/base/GuardManager.sol'; +import {BaseGuard} from 'safe-contracts/base/GuardManager.sol'; import {Enum} from 'safe-contracts/common/Enum.sol'; import {IGuardCallbackModule} from 'interfaces/IGuardCallbackModule.sol'; +import {IStorageMirror} from 'interfaces/IStorageMirror.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 { +contract UpdateStorageMirrorGuard is BaseGuard { + /** + * @notice Emits when a change in a safe's settings is observed + */ + event SettingsChanged(address indexed _safe, bytes32 indexed _settingsHash, IStorageMirror.SafeSettings _settings); /** * @notice The address of the guard callback module */ + IGuardCallbackModule public immutable GUARD_CALLBACK_MODULE; /** @@ -47,7 +53,13 @@ contract UpdateStorageMirrorGuard is Guard { address _msgSender ) external { didSettingsChange = true; - settingsHash = keccak256(abi.encodePacked('settings')); + // TODO: change these data with the decoded ones + address[] memory _owners = new address[](1); + IStorageMirror.SafeSettings memory _safeSettings = IStorageMirror.SafeSettings({owners: _owners, threshold: 1}); + + settingsHash = keccak256(abi.encode(_safeSettings)); + + emit SettingsChanged(msg.sender, settingsHash, _safeSettings); } /** diff --git a/solidity/test/unit/StorageMirror.t.sol b/solidity/test/unit/StorageMirror.t.sol index 9d8eabf..ee898be 100644 --- a/solidity/test/unit/StorageMirror.t.sol +++ b/solidity/test/unit/StorageMirror.t.sol @@ -1,9 +1,7 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity >=0.8.4 <0.9.0; -// solhint-disable-next-line -import 'forge-std/Test.sol'; - +import {Test} from 'forge-std/Test.sol'; import {StorageMirror} from 'contracts/StorageMirror.sol'; import {IStorageMirror} from 'interfaces/IStorageMirror.sol'; diff --git a/solidity/test/unit/UpdateStorageMirrorGuard.t.sol b/solidity/test/unit/UpdateStorageMirrorGuard.t.sol index fd48df9..70cd31e 100644 --- a/solidity/test/unit/UpdateStorageMirrorGuard.t.sol +++ b/solidity/test/unit/UpdateStorageMirrorGuard.t.sol @@ -1,19 +1,22 @@ // SPDX-License-Identifier: UNLICENSED pragma solidity >=0.8.4 <0.9.0; -// solhint-disable-next-line -import 'forge-std/Test.sol'; - +import {Test} from 'forge-std/Test.sol'; import {Enum} from 'safe-contracts/common/Enum.sol'; import {UpdateStorageMirrorGuard} from 'contracts/UpdateStorageMirrorGuard.sol'; import {IGuardCallbackModule} from 'interfaces/IGuardCallbackModule.sol'; +import {IStorageMirror} from 'interfaces/IStorageMirror.sol'; abstract contract Base is Test { + event SettingsChanged(address indexed _safe, bytes32 indexed _settingsHash, IStorageMirror.SafeSettings _settings); + address public safe; IGuardCallbackModule public guardCallbackModule; UpdateStorageMirrorGuard public updateStorageMirrorGuard; - bytes32 public immutable SETTINGS_HASH = keccak256(abi.encodePacked('settings')); + address[] public owners = new address[](1); + IStorageMirror.SafeSettings public safeSettings = IStorageMirror.SafeSettings({owners: owners, threshold: 1}); + bytes32 public settingsHash = keccak256(abi.encode(safeSettings)); function setUp() public { safe = makeAddr('safe'); @@ -27,13 +30,15 @@ contract UnitUpdateStorageMirrorGuard is Base { assertFalse(updateStorageMirrorGuard.didSettingsChange()); assertEq(updateStorageMirrorGuard.settingsHash(), bytes32('')); + vm.expectEmit(true, true, true, true); + emit SettingsChanged(safe, settingsHash, safeSettings); 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'))); + assertEq(updateStorageMirrorGuard.settingsHash(), settingsHash, 'Settings hash should be stored'); } function testCheckAfterExecution(bytes32 _txHash) public { @@ -45,17 +50,17 @@ contract UnitUpdateStorageMirrorGuard is Base { vm.mockCall( address(guardCallbackModule), - abi.encodeCall(IGuardCallbackModule.saveUpdatedSettings, (safe, SETTINGS_HASH)), + abi.encodeCall(IGuardCallbackModule.saveUpdatedSettings, (safe, settingsHash)), abi.encode() ); vm.expectCall( - address(guardCallbackModule), abi.encodeCall(IGuardCallbackModule.saveUpdatedSettings, (safe, SETTINGS_HASH)) + address(guardCallbackModule), abi.encodeCall(IGuardCallbackModule.saveUpdatedSettings, (safe, settingsHash)) ); vm.prank(safe); updateStorageMirrorGuard.checkAfterExecution(_txHash, true); assertFalse(updateStorageMirrorGuard.didSettingsChange()); - assertEq(updateStorageMirrorGuard.settingsHash(), keccak256(abi.encodePacked(''))); + assertEq(updateStorageMirrorGuard.settingsHash(), keccak256(abi.encodePacked('')), 'Settings hash should reset'); } function testCheckAfterExecutionNoSettingsChange(bytes32 _txHash) public { @@ -63,7 +68,7 @@ contract UnitUpdateStorageMirrorGuard is Base { updateStorageMirrorGuard.checkAfterExecution(_txHash, true); assertFalse(updateStorageMirrorGuard.didSettingsChange()); - assertEq(updateStorageMirrorGuard.settingsHash(), bytes32('')); + assertEq(updateStorageMirrorGuard.settingsHash(), bytes32(''), 'Settings hash should stay empty'); } function testCheckAfterExecutionTxFailed(bytes32 _txHash) public { @@ -78,6 +83,6 @@ contract UnitUpdateStorageMirrorGuard is Base { // 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'))); + assertEq(updateStorageMirrorGuard.settingsHash(), settingsHash, 'Settings hash should stay the same'); } } diff --git a/yarn.lock b/yarn.lock index 519ddc1..6974aff 100644 --- a/yarn.lock +++ b/yarn.lock @@ -200,11 +200,6 @@ ds-test "https://github.com/dapphub/ds-test" forge-std "https://github.com/foundry-rs/forge-std" -"@gnosis.pm/safe-contracts@1.3.0": - version "1.3.0" - resolved "https://registry.yarnpkg.com/@gnosis.pm/safe-contracts/-/safe-contracts-1.3.0.tgz#316741a7690d8751a1f701538cfc9ec80866eedc" - integrity sha512-1p+1HwGvxGUVzVkFjNzglwHrLNA67U/axP0Ct85FzzH8yhGJb4t9jDjPYocVMzLorDoWAfKicGy1akPY9jXRVw== - "@jridgewell/resolve-uri@^3.0.3": version "3.1.1" resolved "https://registry.yarnpkg.com/@jridgewell/resolve-uri/-/resolve-uri-3.1.1.tgz#c08679063f279615a3326583ba3a90d1d82cc721" @@ -806,14 +801,14 @@ dot-prop@^5.1.0: dependencies: is-obj "^2.0.0" -"ds-test@git+https://github.com/dapphub/ds-test.git": - version "1.0.0" - resolved "git+https://github.com/dapphub/ds-test.git#e282159d5170298eb2455a6c05280ab5a73a4ef0" - "ds-test@github:dapphub/ds-test#e282159": version "1.0.0" resolved "https://codeload.github.com/dapphub/ds-test/tar.gz/e282159d5170298eb2455a6c05280ab5a73a4ef0" +"ds-test@https://github.com/dapphub/ds-test": + version "1.0.0" + resolved "https://github.com/dapphub/ds-test#e282159d5170298eb2455a6c05280ab5a73a4ef0" + eastasianwidth@^0.2.0: version "0.2.0" resolved "https://registry.yarnpkg.com/eastasianwidth/-/eastasianwidth-0.2.0.tgz#696ce2ec0aa0e6ea93a397ffcf24aa7840c827cb" @@ -1084,14 +1079,14 @@ flatted@^2.0.0: resolved "https://registry.yarnpkg.com/flatted/-/flatted-2.0.2.tgz#4575b21e2bcee7434aa9be662f4b7b5f9c2b5138" integrity sha512-r5wGx7YeOwNWNlCA0wQ86zKyDLMQr+/RB8xy74M4hTphfmjlijTSSXGuH8rnvKZnfT9i+75zmd8jcKdMR4O6jA== -"forge-std@git+https://github.com/foundry-rs/forge-std.git": - version "1.7.1" - resolved "git+https://github.com/foundry-rs/forge-std.git#37a37ab73364d6644bfe11edf88a07880f99bd56" - "forge-std@github:foundry-rs/forge-std#v1.5.6": version "1.5.6" resolved "https://codeload.github.com/foundry-rs/forge-std/tar.gz/e8a047e3f40f13fa37af6fe14e6e06283d9a060e" +"forge-std@https://github.com/foundry-rs/forge-std": + version "1.7.2" + resolved "https://github.com/foundry-rs/forge-std#bdea49f9bb3c58c8c35850c3bdc17eaeea756e9a" + fs-extra@^11.0.0: version "11.1.1" resolved "https://registry.yarnpkg.com/fs-extra/-/fs-extra-11.1.1.tgz#da69f7c39f3b002378b0954bb6ae7efdc0876e2d" @@ -2114,6 +2109,10 @@ safe-buffer@~5.2.0: resolved "https://registry.yarnpkg.com/safe-buffer/-/safe-buffer-5.2.1.tgz#1eaf9fa9bdb1fdd4ec75f58f9cdb4e6b7827eec6" integrity sha512-rp3So07KcdmmKbGvgaNxQSJr7bGVSVk5S9Eq1F+ppbRo70+YeaDxkw5Dd8NPN+GD6bjnYm2VuPuCXmpuYvmCXQ== +"safe-contracts@github:safe-global/safe-contracts#v1.4.1": + version "1.4.1" + resolved "https://codeload.github.com/safe-global/safe-contracts/tar.gz/bf943f80fec5ac647159d26161446ac5d716a294" + "safer-buffer@>= 2.1.2 < 3": version "2.1.2" resolved "https://registry.yarnpkg.com/safer-buffer/-/safer-buffer-2.1.2.tgz#44fa161b0187b9549dd84bb91802f9bd8385cd6a"