From b106de50ac624469e0ed403f6b17b4ae267a42db Mon Sep 17 00:00:00 2001 From: OneTony Date: Mon, 20 Nov 2023 15:29:58 +0200 Subject: [PATCH] feat: needs-update-guard --- solidity/contracts/NeedsUpdateGuard.sol | 81 ++++++++++++++++++++ solidity/interfaces/IVerifierModule.sol | 15 ++++ solidity/test/unit/NeedsUpdateGuard.t.sol | 90 +++++++++++++++++++++++ 3 files changed, 186 insertions(+) create mode 100644 solidity/contracts/NeedsUpdateGuard.sol create mode 100644 solidity/interfaces/IVerifierModule.sol create mode 100644 solidity/test/unit/NeedsUpdateGuard.t.sol diff --git a/solidity/contracts/NeedsUpdateGuard.sol b/solidity/contracts/NeedsUpdateGuard.sol new file mode 100644 index 0000000..9335818 --- /dev/null +++ b/solidity/contracts/NeedsUpdateGuard.sol @@ -0,0 +1,81 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity =0.8.19; + +import {BaseGuard} from 'safe-contracts/base/GuardManager.sol'; +import {Enum} from 'safe-contracts/common/Enum.sol'; +import {IVerifierModule} from 'interfaces/IVerifierModule.sol'; + +/** + * @title NeedsUpdateGuard + * @notice This guard should prevent the safe from executing any transaction if an update is needed. + * @notice An update is needed based on the safe owner's security settings. + */ +contract NeedsUpdateGuard is BaseGuard { + /** + * @notice Emits when the owner changes the update security settings + * @param _safe The address of the safe + * @param _newSecuritySettings The new security settings, how many seconds the safe trusts the last update + */ + event SecuritySettingsChanged(address indexed _safe, uint256 _newSecuritySettings); + + /** + * @notice Throws if the safe needs an update + */ + error NeedsUpdateGuard_NeedsUpdate(); + + /** + * @notice The verifier module + */ + IVerifierModule public immutable VERIFIER_MODULE; + + /** + * @notice The mapping of a safe's address to their security settings, how many seconds the safe trusts the last update + */ + mapping(address => uint256) public safeSecuritySettings; + + constructor(IVerifierModule _verifierModule) { + VERIFIER_MODULE = _verifierModule; + } + + /** + * @notice Guard hook that is called before a Safe transaction is executed + * @dev This function should revert if the safe needs an update + * @dev WARNING: This can brick the safe if the owner doesn't update the settings or change the security settings + */ + // solhint-disable no-unused-vars + 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 { + uint256 _lastVerifiedUpdateTimestamp = VERIFIER_MODULE.latestVerifiedSettingsTimestamp(msg.sender); + uint256 _securitySettings = safeSecuritySettings[msg.sender]; + + if (_lastVerifiedUpdateTimestamp + _securitySettings < block.timestamp) { + revert NeedsUpdateGuard_NeedsUpdate(); + } + } + + /** + * @notice Guard hook that is called after a Safe transaction is executed + */ + function checkAfterExecution(bytes32 _txHash, bool _success) external {} + + /** + * @notice Should update the safe's security settings + * @dev This function should be called by the safe + * @param _securitySettings The new security settings, how many seconds the safe trusts the last verified update + */ + function updateSecuritySettings(uint256 _securitySettings) external { + safeSecuritySettings[msg.sender] = _securitySettings; + emit SecuritySettingsChanged(msg.sender, _securitySettings); + } +} diff --git a/solidity/interfaces/IVerifierModule.sol b/solidity/interfaces/IVerifierModule.sol new file mode 100644 index 0000000..08d4509 --- /dev/null +++ b/solidity/interfaces/IVerifierModule.sol @@ -0,0 +1,15 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity =0.8.19; + +interface IVerifierModule { + /*/////////////////////////////////////////////////////////////// + VARIABLES + //////////////////////////////////////////////////////////////*/ + + /** + * @notice The timestamp when the latest settings were verified + * @param _safe The address of the safe + * @return _timestamp The timestamp + */ + function latestVerifiedSettingsTimestamp(address _safe) external view returns (uint256 _timestamp); +} diff --git a/solidity/test/unit/NeedsUpdateGuard.t.sol b/solidity/test/unit/NeedsUpdateGuard.t.sol new file mode 100644 index 0000000..bbcfab3 --- /dev/null +++ b/solidity/test/unit/NeedsUpdateGuard.t.sol @@ -0,0 +1,90 @@ +// SPDX-License-Identifier: UNLICENSED +pragma solidity >=0.8.4 <0.9.0; + +import {Test} from 'forge-std/Test.sol'; +import {Enum} from 'safe-contracts/common/Enum.sol'; +import {NeedsUpdateGuard} from 'contracts/NeedsUpdateGuard.sol'; +import {IVerifierModule} from 'interfaces/IVerifierModule.sol'; + +abstract contract Base is Test { + event SecuritySettingsChanged(address indexed _safe, uint256 _newSecuritySettings); + + error NeedsUpdateGuard_NeedsUpdate(); + + address public safe; + IVerifierModule public verifierModule; + NeedsUpdateGuard public needsUpdateGuard; + + function setUp() public { + safe = makeAddr('safe'); + verifierModule = IVerifierModule(makeAddr('verifierModule')); + needsUpdateGuard = new NeedsUpdateGuard(verifierModule); + // Warp to 2022-01-01 00:00:00 UTC + vm.warp(1_641_070_800); + } +} + +contract UnitNeedsUpdateGuard is Base { + function testCheckTransaction(address _to, uint256 _value, bytes memory _data) public { + // Set security settings + vm.prank(safe); + needsUpdateGuard.updateSecuritySettings(200); + + // Mock latest verified settings timestamp to current timestamp - 100 + uint256 _currentTimeStamp = block.timestamp; + vm.mockCall( + address(verifierModule), + abi.encodeWithSelector(IVerifierModule.latestVerifiedSettingsTimestamp.selector, safe), + abi.encode(_currentTimeStamp - 100) + ); + + vm.expectCall( + address(verifierModule), abi.encodeWithSelector(IVerifierModule.latestVerifiedSettingsTimestamp.selector, safe) + ); + + vm.prank(safe); + needsUpdateGuard.checkTransaction( + _to, _value, _data, Enum.Operation.Call, 0, 0, 0, address(0), payable(0), '', address(0) + ); + } + + function testCheckTransactionReverts(address _to, uint256 _value, bytes memory _data) public { + // Set security settings + vm.prank(safe); + needsUpdateGuard.updateSecuritySettings(200); + + // Mock latest verified settings timestamp to current timestamp - 1_000 to make the check fail + uint256 _currentTimeStamp = block.timestamp; + vm.mockCall( + address(verifierModule), + abi.encodeWithSelector(IVerifierModule.latestVerifiedSettingsTimestamp.selector, safe), + abi.encode(_currentTimeStamp - 1000) + ); + + vm.expectCall( + address(verifierModule), abi.encodeWithSelector(IVerifierModule.latestVerifiedSettingsTimestamp.selector, safe) + ); + + vm.expectRevert(abi.encodeWithSelector(NeedsUpdateGuard.NeedsUpdateGuard_NeedsUpdate.selector)); + + vm.prank(safe); + needsUpdateGuard.checkTransaction( + _to, _value, _data, Enum.Operation.Call, 0, 0, 0, address(0), payable(0), '', address(0) + ); + } + + function testCheckAfterExecution(bytes32 _txHash) public { + vm.prank(safe); + needsUpdateGuard.checkAfterExecution(_txHash, true); + } + + function testUpdateSecuritySettings(uint256 _newSettings) public { + vm.expectEmit(true, true, true, true); + emit SecuritySettingsChanged(safe, _newSettings); + + vm.prank(safe); + needsUpdateGuard.updateSecuritySettings(_newSettings); + + assertEq(needsUpdateGuard.safeSecuritySettings(safe), _newSettings, 'Security settings should be updated'); + } +}