Skip to content

Commit

Permalink
chore: conflicts
Browse files Browse the repository at this point in the history
  • Loading branch information
excaliborr committed Nov 21, 2023
2 parents 7e8fe64 + 0210608 commit 711f823
Show file tree
Hide file tree
Showing 5 changed files with 242 additions and 6 deletions.
23 changes: 23 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,29 @@ Make sure to set `MAINNET_RPC` environment variable before running end-to-end te
| `yarn test:unit` | Run unit tests. |
| `yarn test:e2e` | Run e2e tests. |


## Smart Contracts

### Home Chain
- `UpdateStorageMirrorGuard`: This guard is responsible for calling the GuardCallbackModule when a change in the settings of a safe is executed.
- `GuardCallbackModule`: This contract is a module that is used to save the updated settings to the StorageMirror.
- `StorageMirror`: This contract is a storage of information about the safe’s settings. All safe’s settings changes should be mirrored in this contract and be saved. In the end, this contract’s storage root is gonna be used to see if a proposed update on the non-home chain is valid.

### Non-Home Chain
- `BlockHeaderOracle`: This contract's purpose is to return the latest stored L1 block header and timestamp. Every X minutes a "magical" off-chain agent provides the latest block header and timestamp.
- `NeedsUpdateGuard`: This guard should prevent the safe from executing any transaction if an update is needed. An update is needed based on the owner's security settings that was inputed.
- `VerifierModule`: This contract is the verifier module that verifies the settings of a safe against the StorageMirror on the home chain.
- `StorageMirrorRootRegistry`: This contract should accept and store storageRoots of the StorageMirror contract in L1.


## ⚠️ Warnings

The project is a PoC implementation and should be treated with caution. Bellow we describe some cases that should be taken into account before using the modules/guard.

- `UpdateStorageMirrorGuard` for the PoC this guard is calling the `GuardCallbackModule` in every call. A possible improvement would be to decode the txData, on the guard `checkTransaction` pre-execute hook, and filter against certain function signatures that change the settings of a Safe to accurately catch the change.
- `NeedsUpdateGuard` this guard on the non-home chain can brick the user's safe, since it will block every tx, if their security settings expire. Also it's worth mentioning that before using the guard the safe owner must verify at least 1 set of settings using the VerifierModule in order for the guard to have a point of reference for the latest verified update.
- `VerifierModule` is executing a safeTx after the verification and update of their settings. This safeTx can become invalid since the signatures passed were created before the change of the settings, in this case the user(s) will need to re-sign the tx manually outside of the UI. A possible improvement would be to have a custom safe app that let's you sign even if you are not a "current owner" but are a "potential future owner" of the "soon-to-be-updated" settings

## Contributors

Safe-Liveness was built with ❤️ by [Wonderland](https://defi.sucks).
Expand Down
81 changes: 81 additions & 0 deletions solidity/contracts/NeedsUpdateGuard.sol
Original file line number Diff line number Diff line change
@@ -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 time inputed on how long an update is trusted.
*/
contract NeedsUpdateGuard is BaseGuard {
/**
* @notice Emits when the owner changes the trustLatestUpdateForSeconds
* @param _safe The address of the safe
* @param _trustLatestUpdateForSeconds The new trustLatestUpdateForSeconds, how many seconds the safe trusts the last update
*/
event TrustLatestUpdateForSecondsChanged(address indexed _safe, uint256 _trustLatestUpdateForSeconds);

/**
* @notice Throws if the safe needs an update
*/
error NeedsUpdateGuard_NeedsUpdate();

/**
* @notice The verifier module
*/
IVerifierModule public immutable VERIFIER_MODULE;

/**
* @notice How many seconds the safe trusts the last update
*/
mapping(address => uint256) public trustLatestUpdateForSeconds;

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 trustLatestUpdateForSeconds
*/
// 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(_msgSender);
uint256 _trustLatestUpdateForSeconds = trustLatestUpdateForSeconds[_msgSender];

if (_lastVerifiedUpdateTimestamp + _trustLatestUpdateForSeconds < 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 trustLatestUpdateForSeconds
* @dev This function should be called by the safe
* @param _trustLatestUpdateForSeconds The new trustLatestUpdateForSeconds, how many seconds the safe trusts the last verified update
*/
function updateTrustLatestUpdateForSeconds(uint256 _trustLatestUpdateForSeconds) external {
trustLatestUpdateForSeconds[msg.sender] = _trustLatestUpdateForSeconds;
emit TrustLatestUpdateForSecondsChanged(msg.sender, _trustLatestUpdateForSeconds);
}
}
2 changes: 1 addition & 1 deletion solidity/contracts/UpdateStorageMirrorGuard.sol
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ 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.
* @notice This guard is responsible for calling the GuardCallbackModule when a change in the settings of a safe is executed.
*/
contract UpdateStorageMirrorGuard is BaseGuard {
/**
Expand Down
48 changes: 43 additions & 5 deletions solidity/test/e2e/Common.sol
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,11 @@ import {StorageMirror} from 'contracts/StorageMirror.sol';
import {UpdateStorageMirrorGuard} from 'contracts/UpdateStorageMirrorGuard.sol';
import {GuardCallbackModule} from 'contracts/GuardCallbackModule.sol';
import {BlockHeaderOracle} from 'contracts/BlockHeaderOracle.sol';
import {NeedsUpdateGuard} from 'contracts/NeedsUpdateGuard.sol';

import {IGuardCallbackModule} from 'interfaces/IGuardCallbackModule.sol';
import {ISafe} from 'interfaces/ISafe.sol';
import {IVerifierModule} from 'interfaces/IVerifierModule.sol';

import {IGnosisSafeProxyFactory} from 'test/e2e/IGnosisSafeProxyFactory.sol';
import {TestConstants} from 'test/utils/TestConstants.sol';
Expand All @@ -26,19 +28,28 @@ contract CommonE2EBase is DSTestPlus, TestConstants {
address public safeOwner;
uint256 public safeOwnerKey;

address public nonHomeChainSafeOwner;
uint256 public nonHomeChainSafeOwnerKey;

StorageMirror public storageMirror;
UpdateStorageMirrorGuard public updateStorageMirrorGuard;
GuardCallbackModule public guardCallbackModule;
BlockHeaderOracle public oracle;
NeedsUpdateGuard public needsUpdateGuard;
ISafe public safe;
ISafe public nonHomeChainSafe;
IVerifierModule public verifierModule = IVerifierModule(makeAddr('verifierModule'));
IGnosisSafeProxyFactory public gnosisSafeProxyFactory = IGnosisSafeProxyFactory(GNOSIS_SAFE_PROXY_FACTORY);

function setUp() public virtual {
vm.createSelectFork(vm.rpcUrl('mainnet'), _FORK_BLOCK);

// Make address and key of safe owner
(safeOwner, safeOwnerKey) = makeAddrAndKey('safeOwner');
// Make address and key of non home chain safe owner
(nonHomeChainSafeOwner, nonHomeChainSafeOwnerKey) = makeAddrAndKey('nonHomeChainSafeOwner');

/// =============== HOME CHAIN ===============
vm.prank(safeOwner);
safe = ISafe(address(gnosisSafeProxyFactory.createProxy(GNOSIS_SAFE_SINGLETON, ''))); // safeOwner nonce 0
label(address(safe), 'SafeProxy');
Expand All @@ -57,14 +68,10 @@ contract CommonE2EBase is DSTestPlus, TestConstants {
updateStorageMirrorGuard = new UpdateStorageMirrorGuard(guardCallbackModule); // deployer nonce 2
label(address(updateStorageMirrorGuard), 'UpdateStorageMirrorGuard');

vm.prank(deployer);
oracle = new BlockHeaderOracle(); // deployer nonce 3
label(address(oracle), 'MockOracle');

// Make sure the theoritical address was calculated correctly
assert(address(updateStorageMirrorGuard) == _updateStorageMirrorGuardTheoriticalAddress);

// Set up safe
// Set up owner home chain safe
address[] memory _owners = new address[](1);
_owners[0] = safeOwner;
vm.prank(safeOwner); // safeOwner nonce 1
Expand Down Expand Up @@ -97,6 +104,37 @@ contract CommonE2EBase is DSTestPlus, TestConstants {
payable(0),
_setGuardSignature
);

/// =============== NON HOME CHAIN ===============
// Set up non home chain safe
vm.prank(nonHomeChainSafeOwner);
nonHomeChainSafe = ISafe(address(gnosisSafeProxyFactory.createProxy(GNOSIS_SAFE_SINGLETON, ''))); // nonHomeChainSafeOwner nonce 0
label(address(nonHomeChainSafe), 'NonHomeChainSafeProxy');

// Deploy non home chain contracts
vm.prank(deployer);
oracle = new BlockHeaderOracle(); // deployer nonce 3
label(address(oracle), 'MockOracle');

// vm.prank(deployer);
// verifierModule = new VerifierModule(..); // deployer nonce 4
// label(address(verifierModule), 'VerifierModule');

vm.prank(deployer);
needsUpdateGuard = new NeedsUpdateGuard(verifierModule); // deployer nonce 5
label(address(needsUpdateGuard), 'NeedsUpdateGuard');

// set up non home chain safe
address[] memory _nonHomeChainSafeOwners = new address[](1);
_nonHomeChainSafeOwners[0] = nonHomeChainSafeOwner;
vm.prank(nonHomeChainSafeOwner); // nonHomeChainSafeOwner nonce 1
nonHomeChainSafe.setup(
_nonHomeChainSafeOwners, 1, address(nonHomeChainSafe), bytes(''), address(0), address(0), 0, payable(0)
);

// enable verifier module

// set needs update guard
}

/**
Expand Down
94 changes: 94 additions & 0 deletions solidity/test/unit/NeedsUpdateGuard.t.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
// 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 TrustLatestUpdateForSecondsChanged(address indexed _safe, uint256 _trustLatestUpdateForSeconds);

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 trustLatestUpdateForSeconds
vm.prank(safe);
needsUpdateGuard.updateTrustLatestUpdateForSeconds(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), '', safe
);
}

function testCheckTransactionReverts(address _to, uint256 _value, bytes memory _data) public {
// Set trustLatestUpdateForSeconds
vm.prank(safe);
needsUpdateGuard.updateTrustLatestUpdateForSeconds(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), '', safe
);
}

function testCheckAfterExecution(bytes32 _txHash) public {
vm.prank(safe);
needsUpdateGuard.checkAfterExecution(_txHash, true);
}

function testUpdateTrustLatestUpdateForSeconds(uint256 _newTrustLatestUpdateForSeconds) public {
vm.expectEmit(true, true, true, true);
emit TrustLatestUpdateForSecondsChanged(safe, _newTrustLatestUpdateForSeconds);

vm.prank(safe);
needsUpdateGuard.updateTrustLatestUpdateForSeconds(_newTrustLatestUpdateForSeconds);

assertEq(
needsUpdateGuard.trustLatestUpdateForSeconds(safe),
_newTrustLatestUpdateForSeconds,
'Should update trustLatestUpdateForSeconds'
);
}
}

0 comments on commit 711f823

Please sign in to comment.