From d2f16e68de5e42830c4529ed20aa00df48144f81 Mon Sep 17 00:00:00 2001 From: aureliusbtc <82057759+aureliusbtc@users.noreply.github.com> Date: Sat, 4 Jun 2022 04:09:34 -0400 Subject: [PATCH 01/11] Get Replica set up for customizable latency --- .../contracts/contracts/ReplicaManager.sol | 22 +++++++++++----- packages/contracts/contracts/libs/Replica.sol | 2 +- packages/contracts/test/ReplicaManager.t.sol | 26 +++++++++---------- 3 files changed, 29 insertions(+), 21 deletions(-) diff --git a/packages/contracts/contracts/ReplicaManager.sol b/packages/contracts/contracts/ReplicaManager.sol index 5861fe8328..28dd759144 100644 --- a/packages/contracts/contracts/ReplicaManager.sol +++ b/packages/contracts/contracts/ReplicaManager.sol @@ -216,7 +216,7 @@ contract ReplicaManager is Version0, Initializable, OwnableUpgradeable { // Hook for future use _beforeUpdate(); // set the new root's confirmation timer - replica.setConfirmAt(_newRoot, block.timestamp + replica.optimisticSeconds); + replica.setConfirmAt(_newRoot, block.timestamp); // update committedRoot replica.setCommittedRoot(_newRoot); emit Update(_remoteDomain, _oldRoot, _newRoot, _signature); @@ -247,10 +247,11 @@ contract ReplicaManager is Version0, Initializable, OwnableUpgradeable { function proveAndProcess( uint32 _remoteDomain, bytes memory _message, + uint32 _optimisticSeconds, bytes32[32] calldata _proof, uint256 _index ) external { - require(prove(_remoteDomain, keccak256(_message), _proof, _index), "!prove"); + require(prove(_remoteDomain, _message, _optimisticSeconds, _proof, _index), "!prove"); process(_message); } @@ -383,12 +384,16 @@ contract ReplicaManager is Version0, Initializable, OwnableUpgradeable { * @param _root the Merkle root, submitted in an update, to check * @return TRUE iff root has been submitted & timeout has expired */ - function acceptableRoot(uint32 _remoteDomain, bytes32 _root) public view returns (bool) { + function acceptableRoot( + uint32 _remoteDomain, + uint32 _optimisticSeconds, + bytes32 _root + ) public view returns (bool) { uint256 _time = allReplicas[activeReplicas[_remoteDomain]].confirmAt[_root]; if (_time == 0) { return false; } - return block.timestamp >= _time; + return block.timestamp + _optimisticSeconds >= _time; } /** @@ -398,24 +403,27 @@ contract ReplicaManager is Version0, Initializable, OwnableUpgradeable { * already proven or processed) * @dev For convenience, we allow proving against any previous root. * This means that witnesses never need to be updated for the new root - * @param _leaf Leaf of message to prove + * @param _message Formatted message + * @param _optimisticSeconds Latency period requested by app on Home, included in part of Merkle proof * @param _proof Merkle proof of inclusion for leaf * @param _index Index of leaf in home's merkle tree * @return Returns true if proof was valid and `prove` call succeeded **/ function prove( uint32 _remoteDomain, - bytes32 _leaf, + bytes memory _message, + uint32 _optimisticSeconds, bytes32[32] calldata _proof, uint256 _index ) public returns (bool) { + bytes32 _leaf = keccak256(abi.encodePacked(_message, _optimisticSeconds)); ReplicaLib.Replica storage replica = allReplicas[activeReplicas[_remoteDomain]]; // ensure that message has not been proven or processed require(replica.messages[_leaf] == ReplicaLib.MessageStatus.None, "!MessageStatus.None"); // calculate the expected root based on the proof bytes32 _calculatedRoot = MerkleLib.branchRoot(_leaf, _proof, _index); // if the root is valid, change status to Proven - if (acceptableRoot(_remoteDomain, _calculatedRoot)) { + if (acceptableRoot(_remoteDomain, _optimisticSeconds, _calculatedRoot)) { replica.setMessageStatus(_leaf, ReplicaLib.MessageStatus.Processed); return true; } diff --git a/packages/contracts/contracts/libs/Replica.sol b/packages/contracts/contracts/libs/Replica.sol index afb23d2779..d2c2babe58 100644 --- a/packages/contracts/contracts/libs/Replica.sol +++ b/packages/contracts/contracts/libs/Replica.sol @@ -35,7 +35,7 @@ library ReplicaLib { uint32 remoteDomain; // 32 bits // Optimistic seconds per remote domain (E.g specifies optimistic seconds on a remote domain basis to wait) ReplicaStatus status; // 8 bits - // Mapping of roots to allowable confirmation times + // Mapping of roots to time at which Relayer submitted on-chain. Latency period begins here. // TODO: confirmAt doesn't need to be uint256 necessarily mapping(bytes32 => uint256) confirmAt; // Mapping of message leaves to MessageStatus diff --git a/packages/contracts/test/ReplicaManager.t.sol b/packages/contracts/test/ReplicaManager.t.sol index d7d7564513..bbb962b8de 100644 --- a/packages/contracts/test/ReplicaManager.t.sol +++ b/packages/contracts/test/ReplicaManager.t.sol @@ -150,17 +150,17 @@ contract ReplicaManagerTest is SynapseTest { replicaManager.update(remoteDomain, committedRoot, newRoot, sig); } - function test_acceptableRoot() public { - bytes32 newRoot = "new root"; - test_successfulUpdate(); - vm.warp(block.timestamp + optimisticSeconds + 1); - assertTrue(replicaManager.acceptableRoot(remoteDomain, newRoot)); - } - - function test_cannotAcceptableRoot() public { - bytes32 newRoot = "new root"; - test_successfulUpdate(); - vm.warp(block.timestamp + optimisticSeconds - 1); - assertFalse(replicaManager.acceptableRoot(remoteDomain, newRoot)); - } + // function test_acceptableRoot() public { + // bytes32 newRoot = "new root"; + // test_successfulUpdate(); + // vm.warp(block.timestamp + optimisticSeconds + 1); + // assertTrue(replicaManager.acceptableRoot(remoteDomain, newRoot)); + // } + + // function test_cannotAcceptableRoot() public { + // bytes32 newRoot = "new root"; + // test_successfulUpdate(); + // vm.warp(block.timestamp + optimisticSeconds - 1); + // assertFalse(replicaManager.acceptableRoot(remoteDomain, newRoot)); + // } } From 8665ea251e56a955038ee02a33d4cb60e047a199 Mon Sep 17 00:00:00 2001 From: aureliusbtc <82057759+aureliusbtc@users.noreply.github.com> Date: Sat, 4 Jun 2022 04:21:50 -0400 Subject: [PATCH 02/11] Fix all previous tests on Home + ReplicaManager to work with customizable latency --- packages/contracts/contracts/Home.sol | 5 +- .../contracts/contracts/ReplicaManager.sol | 9 +- .../governance/GovernanceMessage.sol | 171 ----- .../contracts/governance/GovernanceRouter.sol | 594 ------------------ packages/contracts/test/Home.t.sol | 14 +- packages/contracts/test/ReplicaManager.t.sol | 47 +- 6 files changed, 43 insertions(+), 797 deletions(-) delete mode 100644 packages/contracts/contracts/governance/GovernanceMessage.sol delete mode 100644 packages/contracts/contracts/governance/GovernanceRouter.sol diff --git a/packages/contracts/contracts/Home.sol b/packages/contracts/contracts/Home.sol index 525a12b359..84a3aeafab 100644 --- a/packages/contracts/contracts/Home.sol +++ b/packages/contracts/contracts/Home.sol @@ -65,6 +65,7 @@ contract Home is Version0, QueueManager, MerkleTreeManager, SynapseBase { uint256 indexed leafIndex, uint64 indexed destinationAndNonce, bytes32 committedRoot, + uint32 optimisticSeconds, bytes message ); @@ -170,6 +171,7 @@ contract Home is Version0, QueueManager, MerkleTreeManager, SynapseBase { function dispatch( uint32 _destinationDomain, bytes32 _recipientAddress, + uint32 _optimisticSeconds, bytes memory _messageBody ) external notFailed { require(_messageBody.length <= MAX_MESSAGE_BODY_BYTES, "msg too long"); @@ -186,7 +188,7 @@ contract Home is Version0, QueueManager, MerkleTreeManager, SynapseBase { _messageBody ); // insert the hashed message into the Merkle tree - bytes32 _messageHash = keccak256(_message); + bytes32 _messageHash = keccak256(abi.encodePacked(_message, _optimisticSeconds)); tree.insert(_messageHash); // enqueue the new Merkle root after inserting the message queue.enqueue(root()); @@ -197,6 +199,7 @@ contract Home is Version0, QueueManager, MerkleTreeManager, SynapseBase { count() - 1, _destinationAndNonce(_destinationDomain, _nonce), committedRoot, + _optimisticSeconds, _message ); } diff --git a/packages/contracts/contracts/ReplicaManager.sol b/packages/contracts/contracts/ReplicaManager.sol index 28dd759144..0d44447e1d 100644 --- a/packages/contracts/contracts/ReplicaManager.sol +++ b/packages/contracts/contracts/ReplicaManager.sol @@ -199,19 +199,22 @@ contract ReplicaManager is Version0, Initializable, OwnableUpgradeable { * @dev Reverts if update doesn't build off latest committedRoot * or if signature is invalid. * @param _oldRoot Old merkle root - * @param _newRoot New merkle root - * @param _signature Updater's signature on `_oldRoot` and `_newRoot` + * @param _message Formatted message + * @param _optimisticSeconds Latency period requested by app on Home, included in part of Merkle proof + * @param _signature Updater's signature on `_oldRoot` and keccak256(message, optimisticSeconds) (which forms a root) */ function update( uint32 _remoteDomain, bytes32 _oldRoot, - bytes32 _newRoot, + bytes memory _message, + uint32 _optimisticSeconds, bytes memory _signature ) external { ReplicaLib.Replica storage replica = allReplicas[activeReplicas[_remoteDomain]]; // ensure that update is building off the last submitted root require(_oldRoot == replica.committedRoot, "not current update"); // validate updater signature + bytes32 _newRoot = keccak256(abi.encodePacked(_message, _optimisticSeconds)); require(_isUpdaterSignature(_remoteDomain, _oldRoot, _newRoot, _signature), "!updater sig"); // Hook for future use _beforeUpdate(); diff --git a/packages/contracts/contracts/governance/GovernanceMessage.sol b/packages/contracts/contracts/governance/GovernanceMessage.sol deleted file mode 100644 index 3fd9e3e5a1..0000000000 --- a/packages/contracts/contracts/governance/GovernanceMessage.sol +++ /dev/null @@ -1,171 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity 0.8.13; - -// ============ External Imports ============ -import { TypedMemView } from "../libs/TypedMemView.sol"; - -library GovernanceMessage { - using TypedMemView for bytes; - using TypedMemView for bytes29; - - // Batch message characteristics - // * 1 item - the type - uint256 private constant BATCH_PREFIX_ITEMS = 1; - // * type is 1 byte long - uint256 private constant BATCH_PREFIX_LEN = 1; - // * Length of a Batch message - // * type + batch hash - uint256 private constant BATCH_MESSAGE_LEN = 1 + 32; - - // Serialized Call[] characteristics - // * 1 item - the type - uint256 private constant CALLS_PREFIX_ITEMS = 1; - // * type is 1 byte long - uint256 private constant CALLS_PREFIX_LEN = 1; - - // Serialized Call characteristics - // * Location of the data blob in a serialized call - // * address + length - uint256 private constant CALL_DATA_OFFSET = 32 + 4; - - // Transfer Governance message characteristics - // * Length of a Transfer Governance message - // * type + domain + address - uint256 private constant TRANSFER_GOV_MESSAGE_LEN = 1 + 4 + 32; - - struct Call { - bytes32 to; - bytes data; - } - - enum Types { - Invalid, // 0 - Batch, // 1 - A Batch message - TransferGovernor // 2 - A TransferGovernor message - } - - // Read the type of a message - function messageType(bytes29 _view) internal pure returns (Types) { - return Types(uint8(_view.typeOf())); - } - - // Read the message identifer (first byte) of a message - function identifier(bytes29 _view) internal pure returns (uint8) { - return uint8(_view.indexUint(0, 1)); - } - - /* - * Message Type: BATCH - * struct Call { - * identifier, // message ID -- 1 byte - * batchHash // Hash of serialized calls (see below) -- 32 bytes - * } - * - * struct Call { - * to, // address to call -- 32 bytes - * dataLen, // call data length -- 4 bytes, - * data // call data -- 0+ bytes (variable) - * } - * - * struct Calls - * numCalls, // number of calls -- 1 byte - * calls[] // serialized Call -- 0+ bytes - * } - */ - - // create a Batch message from a list of calls - function formatBatch(Call[] memory _calls) internal view returns (bytes memory) { - return abi.encodePacked(Types.Batch, getBatchHash(_calls)); - } - - // serialize a call to memory and return a reference - function serializeCall(Call memory _call) internal pure returns (bytes29) { - return abi.encodePacked(_call.to, uint32(_call.data.length), _call.data).ref(0); - } - - function getBatchHash(Call[] memory _calls) internal view returns (bytes32) { - // length prefix + 1 entry for each - bytes29[] memory _encodedCalls = new bytes29[](_calls.length + CALLS_PREFIX_ITEMS); - _encodedCalls[0] = abi.encodePacked(uint8(_calls.length)).ref(0); - for (uint256 i = 0; i < _calls.length; i++) { - _encodedCalls[i + CALLS_PREFIX_ITEMS] = serializeCall(_calls[i]); - } - return keccak256(TypedMemView.join(_encodedCalls)); - } - - function isValidBatch(bytes29 _view) internal pure returns (bool) { - return identifier(_view) == uint8(Types.Batch) && _view.len() == BATCH_MESSAGE_LEN; - } - - function isBatch(bytes29 _view) internal pure returns (bool) { - return isValidBatch(_view) && messageType(_view) == Types.Batch; - } - - function tryAsBatch(bytes29 _view) internal pure returns (bytes29) { - if (isValidBatch(_view)) { - return _view.castTo(uint40(Types.Batch)); - } - return TypedMemView.nullView(); - } - - function mustBeBatch(bytes29 _view) internal pure returns (bytes29) { - return tryAsBatch(_view).assertValid(); - } - - // Types.Batch - function batchHash(bytes29 _view) internal pure returns (bytes32) { - return _view.index(BATCH_PREFIX_LEN, 32); - } - - /* - * Message Type: TRANSFER GOVERNOR - * struct TransferGovernor { - * identifier, // message ID -- 1 byte - * domain, // domain of new governor -- 4 bytes - * addr // address of new governor -- 32 bytes - * } - */ - - function formatTransferGovernor(uint32 _domain, bytes32 _governor) - internal - view - returns (bytes memory _msg) - { - _msg = TypedMemView.clone( - mustBeTransferGovernor( - abi.encodePacked(Types.TransferGovernor, _domain, _governor).ref(0) - ) - ); - } - - function isValidTransferGovernor(bytes29 _view) internal pure returns (bool) { - return - identifier(_view) == uint8(Types.TransferGovernor) && - _view.len() == TRANSFER_GOV_MESSAGE_LEN; - } - - function isTransferGovernor(bytes29 _view) internal pure returns (bool) { - return isValidTransferGovernor(_view) && messageType(_view) == Types.TransferGovernor; - } - - function tryAsTransferGovernor(bytes29 _view) internal pure returns (bytes29) { - if (isValidTransferGovernor(_view)) { - return _view.castTo(uint40(Types.TransferGovernor)); - } - return TypedMemView.nullView(); - } - - function mustBeTransferGovernor(bytes29 _view) internal pure returns (bytes29) { - return tryAsTransferGovernor(_view).assertValid(); - } - - // Types.TransferGovernor - function domain(bytes29 _view) internal pure returns (uint32) { - return uint32(_view.indexUint(1, 4)); - } - - // Types.TransferGovernor - function governor(bytes29 _view) internal pure returns (bytes32) { - return _view.index(5, 32); - } -} diff --git a/packages/contracts/contracts/governance/GovernanceRouter.sol b/packages/contracts/contracts/governance/GovernanceRouter.sol deleted file mode 100644 index d605f3894e..0000000000 --- a/packages/contracts/contracts/governance/GovernanceRouter.sol +++ /dev/null @@ -1,594 +0,0 @@ -// SPDX-License-Identifier: MIT -pragma solidity 0.8.13; -pragma experimental ABIEncoderV2; - -// ============ Internal Imports ============ -import { Home } from "../Home.sol"; -import { Version0 } from "../Version0.sol"; -import { XAppConfig, TypeCasts } from "../XAppConfig.sol"; -import { IMessageRecipient } from "../interfaces/IMessageRecipient.sol"; -import { GovernanceMessage } from "./GovernanceMessage.sol"; -// ============ External Imports ============ -import { Initializable } from "@openzeppelin/contracts/proxy/utils/Initializable.sol"; -import { SafeMath } from "@openzeppelin/contracts/utils/math/SafeMath.sol"; -import { TypedMemView } from "../libs/TypedMemView.sol"; - -contract GovernanceRouter is Version0, Initializable, IMessageRecipient { - // ============ Libraries ============ - - using SafeMath for uint256; - using TypedMemView for bytes; - using TypedMemView for bytes29; - using GovernanceMessage for bytes29; - - // ============== Enums ============== - - // The status of a batch of governance calls - enum BatchStatus { - Unknown, // 0 - Pending, // 1 - Complete // 2 - } - - // ============ Immutables ============ - - uint32 public immutable localDomain; - // number of seconds before recovery can be activated - uint256 public immutable recoveryTimelock; - - // ============ Public Storage ============ - - // timestamp when recovery timelock expires; 0 if timelock has not been initiated - uint256 public recoveryActiveAt; - // the address of the recovery manager multisig - address public recoveryManager; - // the local entity empowered to call governance functions, set to 0x0 on non-Governor chains - address public governor; - // domain of Governor chain -- for accepting incoming messages from Governor - uint32 public governorDomain; - // xAppConnectionManager contract which stores Replica addresses - XAppConfig public xAppConfig; - // domain -> remote GovernanceRouter contract address - mapping(uint32 => bytes32) public routers; - // array of all domains with registered GovernanceRouter - uint32[] public domains; - // call hash -> call status - mapping(bytes32 => BatchStatus) public inboundCallBatches; - - // ============ Upgrade Gap ============ - - // gap for upgrade safety - uint256[42] private __GAP; - - // ============ Events ============ - - /** - * @notice Emitted a remote GovernanceRouter address is added, removed, or changed - * @param domain the domain of the remote Router - * @param previousRouter the previously registered router; 0 if router is being added - * @param newRouter the new registered router; 0 if router is being removed - */ - event SetRouter(uint32 indexed domain, bytes32 previousRouter, bytes32 newRouter); - - /** - * @notice Emitted when the Governor role is transferred - * @param previousGovernorDomain the domain of the previous Governor - * @param newGovernorDomain the domain of the new Governor - * @param previousGovernor the address of the previous Governor; 0 if the governor was remote - * @param newGovernor the address of the new Governor; 0 if the governor is remote - */ - event TransferGovernor( - uint32 previousGovernorDomain, - uint32 newGovernorDomain, - address indexed previousGovernor, - address indexed newGovernor - ); - - /** - * @notice Emitted when the RecoveryManager role is transferred - * @param previousRecoveryManager the address of the previous RecoveryManager - * @param newRecoveryManager the address of the new RecoveryManager - */ - event TransferRecoveryManager( - address indexed previousRecoveryManager, - address indexed newRecoveryManager - ); - - /** - * @notice Emitted when recovery state is initiated by the RecoveryManager - * @param recoveryManager the address of the current RecoveryManager who - * initiated the transition - * @param recoveryActiveAt the block at which recovery state will be active - */ - event InitiateRecovery(address indexed recoveryManager, uint256 recoveryActiveAt); - - /** - * @notice Emitted when recovery state is exited by the RecoveryManager - * @param recoveryManager the address of the current RecoveryManager who - * initiated the transition - */ - event ExitRecovery(address recoveryManager); - - /** - * @notice Emitted when a batch of governance instructions from the - * governing remote router is received and ready for execution - * @param batchHash A hash committing to the batch of calls to be executed - */ - event BatchReceived(bytes32 indexed batchHash); - - /** - * @notice Emitted when a batch of governance instructions from the - * governing remote router is executed - * @param batchHash A hash committing to the batch of calls to be executed - */ - event BatchExecuted(bytes32 indexed batchHash); - - modifier typeAssert(bytes29 _view, GovernanceMessage.Types _type) { - _view.assertType(uint40(_type)); - _; - } - - // ============ Modifiers ============ - - modifier onlyReplica() { - require(xAppConfig.isReplica(msg.sender), "!replica"); - _; - } - - modifier onlyGovernorRouter(uint32 _domain, bytes32 _address) { - require(_isGovernorRouter(_domain, _address), "!governorRouter"); - _; - } - - modifier onlyGovernor() { - require(msg.sender == governor || msg.sender == address(this), "! called by governor"); - _; - } - - modifier onlyRecoveryManager() { - require(msg.sender == recoveryManager, "! called by recovery manager"); - _; - } - - modifier onlyNotInRecovery() { - require(!inRecovery(), "in recovery"); - _; - } - - modifier onlyGovernorOrRecoveryManager() { - if (!inRecovery()) { - require(msg.sender == governor || msg.sender == address(this), "! called by governor"); - } else { - require( - msg.sender == recoveryManager || msg.sender == address(this), - "! called by recovery manager" - ); - } - _; - } - - // ============ Constructor ============ - - constructor(uint32 _localDomain, uint256 _recoveryTimelock) { - localDomain = _localDomain; - recoveryTimelock = _recoveryTimelock; - } - - // ============ Initializer ============ - - function initialize(address _xAppConnectionManager, address _recoveryManager) - public - initializer - { - // initialize governor - address _governorAddr = msg.sender; - bool _isLocalGovernor = true; - _transferGovernor(localDomain, _governorAddr, _isLocalGovernor); - // initialize recovery manager - recoveryManager = _recoveryManager; - // initialize XAppConnectionManager - setXAppConnectionManager(_xAppConnectionManager); - require(xAppConfig.localDomain() == localDomain, "XAppConnectionManager bad domain"); - } - - // ============ External Functions ============ - - /** - * @notice Handle Nomad messages - * For all non-Governor chains to handle messages - * sent from the Governor chain via Nomad. - * Governor chain should never receive messages, - * because non-Governor chains are not able to send them - * @param _origin The domain (of the Governor Router) - * @param _sender The message sender (must be the Governor Router) - * @param _message The message - */ - function handle( - uint32 _origin, - uint32, // _nonce (unused) - bytes32 _sender, - bytes memory _message - ) external override onlyReplica onlyGovernorRouter(_origin, _sender) { - bytes29 _msg = _message.ref(0); - bytes29 _view = _msg.tryAsBatch(); - if (_view.notNull()) { - _handleBatch(_view); - return; - } - _view = _msg.tryAsTransferGovernor(); - if (_view.notNull()) { - _handleTransferGovernor(_view); - return; - } - require(false, "!valid message type"); - } - - /** - * @notice Dispatch a set of local and remote calls - * Local calls are executed immediately. - * Remote calls are dispatched to the remote domain for processing and - * execution. - * @dev The contents of the _domains array at the same index - * will determine the destination of messages in that _remoteCalls array. - * As such, all messages in an array MUST have the same destination. - * Missing destinations or too many will result in reverts. - * @param _localCalls An array of local calls - * @param _remoteCalls An array of arrays of remote calls - */ - function executeGovernanceActions( - GovernanceMessage.Call[] calldata _localCalls, - uint32[] calldata _domains, - GovernanceMessage.Call[][] calldata _remoteCalls - ) external onlyGovernorOrRecoveryManager { - require(_domains.length == _remoteCalls.length, "!domains length matches calls length"); - // remote calls are disallowed while in recovery - require(_remoteCalls.length == 0 || !inRecovery(), "!remote calls in recovery mode"); - // _localCall loop - for (uint256 i = 0; i < _localCalls.length; i++) { - _callLocal(_localCalls[i]); - } - // remote calls loop - for (uint256 i = 0; i < _remoteCalls.length; i++) { - uint32 destination = _domains[i]; - _callRemote(destination, _remoteCalls[i]); - } - } - - /** - * @notice Dispatch calls on a remote chain via the remote GovernanceRouter - * @param _destination The domain of the remote chain - * @param _calls The calls - */ - function _callRemote(uint32 _destination, GovernanceMessage.Call[] calldata _calls) - internal - onlyGovernor - onlyNotInRecovery - { - // ensure that destination chain has enrolled router - bytes32 _router = _mustHaveRouter(_destination); - // format batch message - bytes memory _msg = GovernanceMessage.formatBatch(_calls); - // dispatch call message using Nomad - Home(xAppConfig.home()).dispatch(_destination, _router, _msg); - } - - /** - * @notice Transfer governorship - * @param _newDomain The domain of the new governor - * @param _newGovernor The address of the new governor - */ - function transferGovernor(uint32 _newDomain, address _newGovernor) - external - onlyGovernor - onlyNotInRecovery - { - bool _isLocalGovernor = _isLocalDomain(_newDomain); - // transfer the governor locally - _transferGovernor(_newDomain, _newGovernor, _isLocalGovernor); - // if the governor domain is local, we only need to change the governor address locally - // no need to message remote routers; they should already have the same domain set and governor = bytes32(0) - if (_isLocalGovernor) { - return; - } - // format transfer governor message - bytes memory _transferGovernorMessage = GovernanceMessage.formatTransferGovernor( - _newDomain, - TypeCasts.addressToBytes32(_newGovernor) - ); - // send transfer governor message to all remote routers - // note: this assumes that the Router is on the global GovernorDomain; - // this causes a process error when relinquishing governorship - // on a newly deployed domain which is not the GovernorDomain - _sendToAllRemoteRouters(_transferGovernorMessage); - } - - /** - * @notice Transfer recovery manager role - * @dev callable by the recoveryManager at any time to transfer the role - * @param _newRecoveryManager The address of the new recovery manager - */ - function transferRecoveryManager(address _newRecoveryManager) external onlyRecoveryManager { - emit TransferRecoveryManager(recoveryManager, _newRecoveryManager); - recoveryManager = _newRecoveryManager; - } - - /** - * @notice Set the router address for a given domain and - * dispatch the change to all remote routers - * @param _domain The domain - * @param _router The address of the new router - */ - function setRouterGlobal(uint32 _domain, bytes32 _router) - external - onlyGovernor - onlyNotInRecovery - { - _setRouterGlobal(_domain, _router); - } - - function _setRouterGlobal(uint32 _domain, bytes32 _router) internal { - Home _home = Home(xAppConfig.home()); - // Set up the call for use in the loop. - // Because each domain's governance router may be different, we cannot - // serialize the `Call` once and then reuse it. We have to re-serialize - // the call, adjusting its `to` value on each step of the loop. - GovernanceMessage.Call[] memory _calls = new GovernanceMessage.Call[](1); - _calls[0].data = abi.encodeWithSignature( - "setRouterLocal(uint32,bytes32)", - _domain, - _router - ); - for (uint256 i = 0; i < domains.length; i++) { - uint32 _destination = domains[i]; - if (_destination != uint32(0)) { - // set to, and dispatch - bytes32 _recipient = routers[_destination]; - _calls[0].to = _recipient; - bytes memory _msg = GovernanceMessage.formatBatch(_calls); - _home.dispatch(_destination, _recipient, _msg); - } - } - // set the router locally - _setRouter(_domain, _router); - } - - /** - * @notice Set the router address *locally only* - * @dev For use in deploy to setup the router mapping locally - * @param _domain The domain - * @param _router The new router - */ - function setRouterLocal(uint32 _domain, bytes32 _router) - external - onlyGovernorOrRecoveryManager - { - // set the router locally - _setRouter(_domain, _router); - } - - /** - * @notice Set the address of the XAppConnectionManager - * @dev Domain/address validation helper - * @param _xAppConnectionManager The address of the new xAppConnectionManager - */ - function setXAppConnectionManager(address _xAppConnectionManager) - public - onlyGovernorOrRecoveryManager - { - xAppConfig = XAppConfig(_xAppConnectionManager); - } - - /** - * @notice Initiate the recovery timelock - * @dev callable by the recovery manager - */ - function initiateRecoveryTimelock() external onlyNotInRecovery onlyRecoveryManager { - require(recoveryActiveAt == 0, "recovery already initiated"); - // set the time that recovery will be active - recoveryActiveAt = block.timestamp.add(recoveryTimelock); - emit InitiateRecovery(recoveryManager, recoveryActiveAt); - } - - /** - * @notice Exit recovery mode - * @dev callable by the recovery manager to end recovery mode - */ - function exitRecovery() external onlyRecoveryManager { - require(recoveryActiveAt != 0, "recovery not initiated"); - delete recoveryActiveAt; - emit ExitRecovery(recoveryManager); - } - - // ============ Public Functions ============ - - /** - * @notice Check if the contract is in recovery mode currently - * @return TRUE iff the contract is actively in recovery mode currently - */ - function inRecovery() public view returns (bool) { - uint256 _recoveryActiveAt = recoveryActiveAt; - bool _recoveryInitiated = _recoveryActiveAt != 0; - bool _recoveryActive = _recoveryActiveAt <= block.timestamp; - return _recoveryInitiated && _recoveryActive; - } - - // ============ Internal Functions ============ - - /** - * @notice Handle message dispatching calls locally - * @dev We considered requiring the batch was not previously known. - * However, this would prevent us from ever processing identical - * batches, which seems desirable in some cases. - * As a result, we simply set it to pending. - * @param _msg The message - */ - function _handleBatch(bytes29 _msg) internal typeAssert(_msg, GovernanceMessage.Types.Batch) { - bytes32 _batchHash = _msg.batchHash(); - // prevent accidental SSTORE and extra event if already pending - if (inboundCallBatches[_batchHash] == BatchStatus.Pending) return; - inboundCallBatches[_batchHash] = BatchStatus.Pending; - emit BatchReceived(_batchHash); - } - - /** - * @notice execute a pending batch of messages - */ - function executeCallBatch(GovernanceMessage.Call[] calldata _calls) external { - bytes32 _batchHash = GovernanceMessage.getBatchHash(_calls); - require(inboundCallBatches[_batchHash] == BatchStatus.Pending, "!batch pending"); - inboundCallBatches[_batchHash] = BatchStatus.Complete; - for (uint256 i = 0; i < _calls.length; i++) { - _callLocal(_calls[i]); - } - emit BatchExecuted(_batchHash); - } - - /** - * @notice Handle message transferring governorship to a new Governor - * @param _msg The message - */ - function _handleTransferGovernor(bytes29 _msg) - internal - typeAssert(_msg, GovernanceMessage.Types.TransferGovernor) - { - uint32 _newDomain = _msg.domain(); - address _newGovernor = TypeCasts.bytes32ToAddress(_msg.governor()); - bool _isLocalGovernor = _isLocalDomain(_newDomain); - _transferGovernor(_newDomain, _newGovernor, _isLocalGovernor); - } - - /** - * @notice Dispatch message to all remote routers - * @param _msg The message - */ - function _sendToAllRemoteRouters(bytes memory _msg) internal { - Home _home = Home(xAppConfig.home()); - - for (uint256 i = 0; i < domains.length; i++) { - if (domains[i] != uint32(0)) { - _home.dispatch(domains[i], routers[domains[i]], _msg); - } - } - } - - /** - * @notice Dispatch call locally - * @param _call The call - * @return _ret - */ - function _callLocal(GovernanceMessage.Call memory _call) internal returns (bytes memory _ret) { - address _toContract = TypeCasts.bytes32ToAddress(_call.to); - // attempt to dispatch using low-level call - bool _success; - (_success, _ret) = _toContract.call(_call.data); - // revert if the call failed - require(_success, "call failed"); - } - - /** - * @notice Transfer governorship within this contract's state - * @param _newDomain The domain of the new governor - * @param _newGovernor The address of the new governor - * @param _isLocalGovernor True if the newDomain is the localDomain - */ - function _transferGovernor( - uint32 _newDomain, - address _newGovernor, - bool _isLocalGovernor - ) internal { - // require that the new governor is not the zero address - require(_newGovernor != address(0), "cannot renounce governor"); - // require that the governor domain has a valid router - if (!_isLocalGovernor) { - _mustHaveRouter(_newDomain); - } - // Governor is 0x0 unless the governor is local - address _newGov = _isLocalGovernor ? _newGovernor : address(0); - // emit event before updating state variables - emit TransferGovernor(governorDomain, _newDomain, governor, _newGov); - // update state - governorDomain = _newDomain; - governor = _newGov; - } - - /** - * @notice Set the router for a given domain - * @param _domain The domain - * @param _newRouter The new router - */ - function _setRouter(uint32 _domain, bytes32 _newRouter) internal { - // ignore local domain in router mapping - require(!_isLocalDomain(_domain), "can't set local router"); - // store previous router in memory - bytes32 _previousRouter = routers[_domain]; - // if router is being removed, - if (_newRouter == bytes32(0)) { - // remove domain from array - _removeDomain(_domain); - // remove router from mapping - delete routers[_domain]; - } else { - // if router was not previously added, - if (_previousRouter == bytes32(0)) { - // add domain to array - _addDomain(_domain); - } - // set router in mapping (add or change) - routers[_domain] = _newRouter; - } - // emit event - emit SetRouter(_domain, _previousRouter, _newRouter); - } - - /** - * @notice Add a domain that has a router - * @param _domain The domain - */ - function _addDomain(uint32 _domain) internal { - domains.push(_domain); - } - - /** - * @notice Remove a domain from array - * @param _domain The domain - */ - function _removeDomain(uint32 _domain) internal { - // find the index of the domain to remove & delete it from domains[] - for (uint256 i = 0; i < domains.length; i++) { - if (domains[i] == _domain) { - delete domains[i]; - return; - } - } - } - - /** - * @notice Determine if a given domain and address is the Governor Router - * @param _domain The domain - * @param _address The address of the domain's router - * @return _ret True if the given domain/address is the - * Governor Router. - */ - function _isGovernorRouter(uint32 _domain, bytes32 _address) internal view returns (bool) { - return _domain == governorDomain && _address == routers[_domain]; - } - - /** - * @notice Determine if a given domain is the local domain - * @param _domain The domain - * @return _ret - True if the given domain is the local domain - */ - function _isLocalDomain(uint32 _domain) internal view returns (bool) { - return _domain == localDomain; - } - - /** - * @notice Require that a domain has a router and returns the router - * @param _domain The domain - * @return _router - The domain's router - */ - function _mustHaveRouter(uint32 _domain) internal view returns (bytes32 _router) { - _router = routers[_domain]; - require(_router != bytes32(0), "!router"); - } -} diff --git a/packages/contracts/test/Home.t.sol b/packages/contracts/test/Home.t.sol index 5a9f8e65f3..41615bf0a5 100644 --- a/packages/contracts/test/Home.t.sol +++ b/packages/contracts/test/Home.t.sol @@ -9,9 +9,11 @@ import { SynapseTestWithUpdaterManager } from "./utils/SynapseTest.sol"; contract HomeTest is SynapseTestWithUpdaterManager { HomeHarness home; + uint32 optimisticSeconds; function setUp() public override { super.setUp(); + optimisticSeconds = 10; home = new HomeHarness(localDomain); home.initialize(IUpdaterManager(updaterManager)); updaterManager.setHome(address(home)); @@ -65,7 +67,7 @@ contract HomeTest is SynapseTestWithUpdaterManager { function test_haltsOnFail() public { home.setFailed(); vm.expectRevert("failed state"); - home.dispatch(remoteDomain, addressToBytes32(address(1337)), bytes("")); + home.dispatch(remoteDomain, addressToBytes32(address(1337)), optimisticSeconds, bytes("")); } // TODO: testHashDomain against Go generated domains @@ -83,6 +85,7 @@ contract HomeTest is SynapseTestWithUpdaterManager { uint256 indexed leafIndex, uint64 indexed destinationAndNonce, bytes32 committedRoot, + uint32 optimisticSeconds, bytes message ); @@ -100,17 +103,18 @@ contract HomeTest is SynapseTestWithUpdaterManager { recipient, messageBody ); - bytes32 messageHash = keccak256(message); + bytes32 messageHash = keccak256(abi.encodePacked(message, optimisticSeconds)); vm.expectEmit(true, true, true, true); emit Dispatch( messageHash, home.count(), (uint64(remoteDomain) << 32) | nonce, home.committedRoot(), + optimisticSeconds, message ); vm.prank(sender); - home.dispatch(remoteDomain, recipient, messageBody); + home.dispatch(remoteDomain, recipient, optimisticSeconds, messageBody); assert(home.queueContains(home.root())); } @@ -130,7 +134,7 @@ contract HomeTest is SynapseTestWithUpdaterManager { ); vm.prank(sender); vm.expectRevert("msg too long"); - home.dispatch(remoteDomain, recipient, messageBody); + home.dispatch(remoteDomain, recipient, optimisticSeconds, messageBody); } // ============ UPDATING MESSAGES ============ @@ -148,7 +152,7 @@ contract HomeTest is SynapseTestWithUpdaterManager { home.improperUpdate(oldRoot, newRoot, sig); assertEq(uint256(home.state()), 2); vm.expectRevert("failed state"); - home.dispatch(0, bytes32(0), bytes("")); + home.dispatch(0, bytes32(0), optimisticSeconds, bytes("")); } // Tests signing new roots of queue, becoming committed root diff --git a/packages/contracts/test/ReplicaManager.t.sol b/packages/contracts/test/ReplicaManager.t.sol index bbb962b8de..fcf35a00b1 100644 --- a/packages/contracts/test/ReplicaManager.t.sol +++ b/packages/contracts/test/ReplicaManager.t.sol @@ -120,7 +120,8 @@ contract ReplicaManagerTest is SynapseTest { // Relayer relays a new root signed by updater on Home chain function test_successfulUpdate() public { - bytes32 newRoot = "new root"; + bytes memory newMessage = "new root"; + bytes32 newRoot = keccak256(abi.encodePacked(newMessage, optimisticSeconds)); assertEq(replicaManager.updater(), vm.addr(updaterPK)); bytes memory sig = signRemoteUpdate(updaterPK, committedRoot, newRoot); // Root doesn't exist yet @@ -128,39 +129,39 @@ contract ReplicaManagerTest is SynapseTest { // Relayer sends over a root signed by the updater on the Home chain vm.expectEmit(true, true, true, true); emit Update(remoteDomain, committedRoot, newRoot, sig); - replicaManager.update(remoteDomain, committedRoot, newRoot, sig); - // Root set with optimistic latency allowing it to be processed at T+10 - assertEq( - replicaManager.activeReplicaConfirmedAt(remoteDomain, newRoot), - block.timestamp + 10 - ); + replicaManager.update(remoteDomain, committedRoot, newMessage, optimisticSeconds, sig); + // Time at which root was confirmed is set, optimistic timeout starts now + assertEq(replicaManager.activeReplicaConfirmedAt(remoteDomain, newRoot), block.timestamp); assertEq(replicaManager.activeReplicaCommittedRoot(remoteDomain), newRoot); } function test_updateWithIncorrectRoot() public { + bytes memory newMessage = "new root"; bytes32 newRoot = "new root"; vm.expectRevert("not current update"); - replicaManager.update(remoteDomain, newRoot, newRoot, bytes("")); + replicaManager.update(remoteDomain, newRoot, newMessage, optimisticSeconds, bytes("")); } function test_updateWithIncorrectSig() public { - bytes32 newRoot = "new root"; + bytes memory newMessage = "new root"; + bytes32 newRoot = keccak256(abi.encodePacked(newMessage, optimisticSeconds)); bytes memory sig = signRemoteUpdate(fakeUpdaterPK, committedRoot, newRoot); vm.expectRevert("!updater sig"); - replicaManager.update(remoteDomain, committedRoot, newRoot, sig); + replicaManager.update(remoteDomain, committedRoot, newMessage, optimisticSeconds, sig); } - // function test_acceptableRoot() public { - // bytes32 newRoot = "new root"; - // test_successfulUpdate(); - // vm.warp(block.timestamp + optimisticSeconds + 1); - // assertTrue(replicaManager.acceptableRoot(remoteDomain, newRoot)); - // } - - // function test_cannotAcceptableRoot() public { - // bytes32 newRoot = "new root"; - // test_successfulUpdate(); - // vm.warp(block.timestamp + optimisticSeconds - 1); - // assertFalse(replicaManager.acceptableRoot(remoteDomain, newRoot)); - // } + function test_acceptableRoot() public { + bytes memory newMessage = "new root"; + bytes32 newRoot = keccak256(abi.encodePacked(newMessage, optimisticSeconds)); + test_successfulUpdate(); + vm.warp(block.timestamp + optimisticSeconds + 1); + assertTrue(replicaManager.acceptableRoot(remoteDomain, optimisticSeconds, newRoot)); + } + + function test_cannotAcceptableRoot() public { + bytes32 newRoot = "new root"; + test_successfulUpdate(); + vm.warp(block.timestamp + optimisticSeconds - 1); + assertFalse(replicaManager.acceptableRoot(remoteDomain, optimisticSeconds, newRoot)); + } } From 010c4886d656bbee41b27811ea85c8f4b15ade1d Mon Sep 17 00:00:00 2001 From: aureliusbtc <82057759+aureliusbtc@users.noreply.github.com> Date: Sat, 4 Jun 2022 04:37:24 -0400 Subject: [PATCH 03/11] Keep updater to only posting newRoot --- packages/contracts/contracts/ReplicaManager.sol | 9 +++------ packages/contracts/test/ReplicaManager.t.sol | 6 +++--- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/packages/contracts/contracts/ReplicaManager.sol b/packages/contracts/contracts/ReplicaManager.sol index 0d44447e1d..3095315146 100644 --- a/packages/contracts/contracts/ReplicaManager.sol +++ b/packages/contracts/contracts/ReplicaManager.sol @@ -199,22 +199,19 @@ contract ReplicaManager is Version0, Initializable, OwnableUpgradeable { * @dev Reverts if update doesn't build off latest committedRoot * or if signature is invalid. * @param _oldRoot Old merkle root - * @param _message Formatted message - * @param _optimisticSeconds Latency period requested by app on Home, included in part of Merkle proof - * @param _signature Updater's signature on `_oldRoot` and keccak256(message, optimisticSeconds) (which forms a root) + * @param _newRoot New merkle root + * @param _signature Updater's signature on `_oldRoot` and `_newRoot` = `keccak256(message, optimisticSeconds)` */ function update( uint32 _remoteDomain, bytes32 _oldRoot, - bytes memory _message, - uint32 _optimisticSeconds, + bytes32 _newRoot, bytes memory _signature ) external { ReplicaLib.Replica storage replica = allReplicas[activeReplicas[_remoteDomain]]; // ensure that update is building off the last submitted root require(_oldRoot == replica.committedRoot, "not current update"); // validate updater signature - bytes32 _newRoot = keccak256(abi.encodePacked(_message, _optimisticSeconds)); require(_isUpdaterSignature(_remoteDomain, _oldRoot, _newRoot, _signature), "!updater sig"); // Hook for future use _beforeUpdate(); diff --git a/packages/contracts/test/ReplicaManager.t.sol b/packages/contracts/test/ReplicaManager.t.sol index fcf35a00b1..0ad3c589f5 100644 --- a/packages/contracts/test/ReplicaManager.t.sol +++ b/packages/contracts/test/ReplicaManager.t.sol @@ -129,7 +129,7 @@ contract ReplicaManagerTest is SynapseTest { // Relayer sends over a root signed by the updater on the Home chain vm.expectEmit(true, true, true, true); emit Update(remoteDomain, committedRoot, newRoot, sig); - replicaManager.update(remoteDomain, committedRoot, newMessage, optimisticSeconds, sig); + replicaManager.update(remoteDomain, committedRoot, newRoot, sig); // Time at which root was confirmed is set, optimistic timeout starts now assertEq(replicaManager.activeReplicaConfirmedAt(remoteDomain, newRoot), block.timestamp); assertEq(replicaManager.activeReplicaCommittedRoot(remoteDomain), newRoot); @@ -139,7 +139,7 @@ contract ReplicaManagerTest is SynapseTest { bytes memory newMessage = "new root"; bytes32 newRoot = "new root"; vm.expectRevert("not current update"); - replicaManager.update(remoteDomain, newRoot, newMessage, optimisticSeconds, bytes("")); + replicaManager.update(remoteDomain, newRoot, newRoot, bytes("")); } function test_updateWithIncorrectSig() public { @@ -147,7 +147,7 @@ contract ReplicaManagerTest is SynapseTest { bytes32 newRoot = keccak256(abi.encodePacked(newMessage, optimisticSeconds)); bytes memory sig = signRemoteUpdate(fakeUpdaterPK, committedRoot, newRoot); vm.expectRevert("!updater sig"); - replicaManager.update(remoteDomain, committedRoot, newMessage, optimisticSeconds, sig); + replicaManager.update(remoteDomain, committedRoot, newRoot, sig); } function test_acceptableRoot() public { From bb89d4840d9d949a3f6c84750656362efbb0b3a8 Mon Sep 17 00:00:00 2001 From: Trajan0x Date: Sat, 4 Jun 2022 16:03:52 -0400 Subject: [PATCH 04/11] update contracts --- packages/contracts/contracts/Home.sol | 1 + packages/contracts/contracts/libs/Message.sol | 13 +++++++++++-- packages/contracts/test/Home.t.sol | 2 ++ packages/contracts/test/Message.t.sol | 6 ++++++ .../contracts/test/harnesses/MessageHarness.sol | 9 ++++++++- 5 files changed, 28 insertions(+), 3 deletions(-) diff --git a/packages/contracts/contracts/Home.sol b/packages/contracts/contracts/Home.sol index 84a3aeafab..951a98a048 100644 --- a/packages/contracts/contracts/Home.sol +++ b/packages/contracts/contracts/Home.sol @@ -185,6 +185,7 @@ contract Home is Version0, QueueManager, MerkleTreeManager, SynapseBase { _nonce, _destinationDomain, _recipientAddress, + _optimisticSeconds, _messageBody ); // insert the hashed message into the Merkle tree diff --git a/packages/contracts/contracts/libs/Message.sol b/packages/contracts/contracts/libs/Message.sol index d2c2f7b03d..c7ca2e241f 100644 --- a/packages/contracts/contracts/libs/Message.sol +++ b/packages/contracts/contracts/libs/Message.sol @@ -15,7 +15,7 @@ library Message { using TypedMemView for bytes29; // Number of bytes in formatted message before `body` field - uint256 internal constant PREFIX_LENGTH = 76; + uint256 internal constant PREFIX_LENGTH = 80; /** * @notice Returns formatted (packed) message with provided fields @@ -33,6 +33,7 @@ library Message { uint32 _nonce, uint32 _destinationDomain, bytes32 _recipient, + uint32 _optimisticSeconds, bytes memory _messageBody ) internal pure returns (bytes memory) { return @@ -42,6 +43,7 @@ library Message { _nonce, _destinationDomain, _recipient, + _optimisticSeconds, _messageBody ); } @@ -62,9 +64,10 @@ library Message { uint32 _nonce, uint32 _destination, bytes32 _recipient, + uint32 _optimisticSeconds, bytes memory _body ) internal pure returns (bytes32) { - return keccak256(formatMessage(_origin, _sender, _nonce, _destination, _recipient, _body)); + return keccak256(formatMessage(_origin, _sender, _nonce, _destination, _recipient, _optimisticSeconds, _body)); } /// @notice Returns message's origin field @@ -92,6 +95,11 @@ library Message { return _message.index(44, 32); } + /// @notice Returns the optimistic seconds from the message + function optimisticSeconds(bytes29 _message) internal pure returns (uint32){ + return uint32(_message.indexUint(76, 4)); + } + /// @notice Returns message's recipient field as an address function recipientAddress(bytes29 _message) internal pure returns (address) { return TypeCasts.bytes32ToAddress(recipient(_message)); @@ -110,6 +118,7 @@ library Message { nonce(_message), destination(_message), recipient(_message), + optimisticSeconds(_message), TypedMemView.clone(body(_message)) ); } diff --git a/packages/contracts/test/Home.t.sol b/packages/contracts/test/Home.t.sol index 41615bf0a5..54cc3c837b 100644 --- a/packages/contracts/test/Home.t.sol +++ b/packages/contracts/test/Home.t.sol @@ -101,6 +101,7 @@ contract HomeTest is SynapseTestWithUpdaterManager { nonce, remoteDomain, recipient, + optimisticSeconds, messageBody ); bytes32 messageHash = keccak256(abi.encodePacked(message, optimisticSeconds)); @@ -130,6 +131,7 @@ contract HomeTest is SynapseTestWithUpdaterManager { nonce, remoteDomain, recipient, + optimisticSeconds, messageBody ); vm.prank(sender); diff --git a/packages/contracts/test/Message.t.sol b/packages/contracts/test/Message.t.sol index ed460a13d9..183c89f20e 100644 --- a/packages/contracts/test/Message.t.sol +++ b/packages/contracts/test/Message.t.sol @@ -17,6 +17,7 @@ contract MessageTest is Test { bytes32 sender; uint32 nonce; uint32 destinationDomain; + uint32 optimisticSeconds; bytes32 recipient; bytes messageBody; @@ -26,6 +27,7 @@ contract MessageTest is Test { sender = bytes32("AAAA THE SENDOOOOOR"); nonce = 0; destinationDomain = 2000; + optimisticSeconds = 4; recipient = bytes32("AAAA THE RECEIVOOOR"); messageBody = bytes("Messagoooor"); } @@ -37,6 +39,7 @@ contract MessageTest is Test { nonce, destinationDomain, recipient, + optimisticSeconds, messageBody ); @@ -46,6 +49,7 @@ contract MessageTest is Test { assertEq(messageHarness.nonce(message), nonce); assertEq(messageHarness.destination(message), destinationDomain); assertEq(messageHarness.recipient(message), recipient); + assertEq(messageHarness.optimisticSeconds(message), optimisticSeconds); assertEq(messageHarness.body(message), (messageBody)); assertEq(messageHarness.leaf(message), keccak256(message)); } @@ -57,6 +61,7 @@ contract MessageTest is Test { nonce, destinationDomain, recipient, + optimisticSeconds, messageBody ); @@ -66,6 +71,7 @@ contract MessageTest is Test { nonce, destinationDomain, recipient, + optimisticSeconds, messageBody ); diff --git a/packages/contracts/test/harnesses/MessageHarness.sol b/packages/contracts/test/harnesses/MessageHarness.sol index 6e1c0cd8cc..93391d632d 100644 --- a/packages/contracts/test/harnesses/MessageHarness.sol +++ b/packages/contracts/test/harnesses/MessageHarness.sol @@ -16,6 +16,7 @@ contract MessageHarness { uint32 _nonce, uint32 _destinationDomain, bytes32 _recipient, + uint32 _optimisticSeconds, bytes memory _messageBody ) public pure returns (bytes memory) { return @@ -25,6 +26,7 @@ contract MessageHarness { _nonce, _destinationDomain, _recipient, + _optimisticSeconds, _messageBody ); } @@ -45,9 +47,10 @@ contract MessageHarness { uint32 _nonce, uint32 _destination, bytes32 _recipient, + uint32 _optimisticSeconds, bytes memory _body ) public pure returns (bytes32) { - return Message.messageHash(_origin, _sender, _nonce, _destination, _recipient, _body); + return Message.messageHash(_origin, _sender, _nonce, _destination, _recipient, _optimisticSeconds, _body); } function body(bytes memory _message) external view returns (bytes memory) { @@ -78,6 +81,10 @@ contract MessageHarness { return _message.ref(0).recipientAddress(); } + function optimisticSeconds(bytes memory _message) external pure returns (uint32) { + return _message.ref(0).optimisticSeconds(); + } + function leaf(bytes memory _message) external view returns (bytes32) { return _message.ref(0).leaf(); } From 7199ae155f4bdec4a224c3adca7a6194be782d7c Mon Sep 17 00:00:00 2001 From: aureliusbtc <82057759+aureliusbtc@users.noreply.github.com> Date: Sat, 4 Jun 2022 16:17:08 -0400 Subject: [PATCH 05/11] fix: change messageHash back to only message --- packages/contracts/contracts/Home.sol | 4 +--- packages/contracts/contracts/ReplicaManager.sol | 10 ++++------ packages/contracts/test/Home.t.sol | 4 +--- packages/contracts/test/ReplicaManager.t.sol | 6 +++--- 4 files changed, 9 insertions(+), 15 deletions(-) diff --git a/packages/contracts/contracts/Home.sol b/packages/contracts/contracts/Home.sol index 951a98a048..c1a3b5bcc2 100644 --- a/packages/contracts/contracts/Home.sol +++ b/packages/contracts/contracts/Home.sol @@ -65,7 +65,6 @@ contract Home is Version0, QueueManager, MerkleTreeManager, SynapseBase { uint256 indexed leafIndex, uint64 indexed destinationAndNonce, bytes32 committedRoot, - uint32 optimisticSeconds, bytes message ); @@ -189,7 +188,7 @@ contract Home is Version0, QueueManager, MerkleTreeManager, SynapseBase { _messageBody ); // insert the hashed message into the Merkle tree - bytes32 _messageHash = keccak256(abi.encodePacked(_message, _optimisticSeconds)); + bytes32 _messageHash = keccak256(_message); tree.insert(_messageHash); // enqueue the new Merkle root after inserting the message queue.enqueue(root()); @@ -200,7 +199,6 @@ contract Home is Version0, QueueManager, MerkleTreeManager, SynapseBase { count() - 1, _destinationAndNonce(_destinationDomain, _nonce), committedRoot, - _optimisticSeconds, _message ); } diff --git a/packages/contracts/contracts/ReplicaManager.sol b/packages/contracts/contracts/ReplicaManager.sol index 3095315146..4f325453c3 100644 --- a/packages/contracts/contracts/ReplicaManager.sol +++ b/packages/contracts/contracts/ReplicaManager.sol @@ -247,11 +247,10 @@ contract ReplicaManager is Version0, Initializable, OwnableUpgradeable { function proveAndProcess( uint32 _remoteDomain, bytes memory _message, - uint32 _optimisticSeconds, bytes32[32] calldata _proof, uint256 _index ) external { - require(prove(_remoteDomain, _message, _optimisticSeconds, _proof, _index), "!prove"); + require(prove(_remoteDomain, _message, _proof, _index), "!prove"); process(_message); } @@ -404,7 +403,6 @@ contract ReplicaManager is Version0, Initializable, OwnableUpgradeable { * @dev For convenience, we allow proving against any previous root. * This means that witnesses never need to be updated for the new root * @param _message Formatted message - * @param _optimisticSeconds Latency period requested by app on Home, included in part of Merkle proof * @param _proof Merkle proof of inclusion for leaf * @param _index Index of leaf in home's merkle tree * @return Returns true if proof was valid and `prove` call succeeded @@ -412,18 +410,18 @@ contract ReplicaManager is Version0, Initializable, OwnableUpgradeable { function prove( uint32 _remoteDomain, bytes memory _message, - uint32 _optimisticSeconds, bytes32[32] calldata _proof, uint256 _index ) public returns (bool) { - bytes32 _leaf = keccak256(abi.encodePacked(_message, _optimisticSeconds)); + uint32 optimisticSeconds = _message.ref(0).optimisticSeconds(); + bytes32 _leaf = keccak256(_message); ReplicaLib.Replica storage replica = allReplicas[activeReplicas[_remoteDomain]]; // ensure that message has not been proven or processed require(replica.messages[_leaf] == ReplicaLib.MessageStatus.None, "!MessageStatus.None"); // calculate the expected root based on the proof bytes32 _calculatedRoot = MerkleLib.branchRoot(_leaf, _proof, _index); // if the root is valid, change status to Proven - if (acceptableRoot(_remoteDomain, _optimisticSeconds, _calculatedRoot)) { + if (acceptableRoot(_remoteDomain, optimisticSeconds, _calculatedRoot)) { replica.setMessageStatus(_leaf, ReplicaLib.MessageStatus.Processed); return true; } diff --git a/packages/contracts/test/Home.t.sol b/packages/contracts/test/Home.t.sol index 54cc3c837b..517894985b 100644 --- a/packages/contracts/test/Home.t.sol +++ b/packages/contracts/test/Home.t.sol @@ -85,7 +85,6 @@ contract HomeTest is SynapseTestWithUpdaterManager { uint256 indexed leafIndex, uint64 indexed destinationAndNonce, bytes32 committedRoot, - uint32 optimisticSeconds, bytes message ); @@ -104,14 +103,13 @@ contract HomeTest is SynapseTestWithUpdaterManager { optimisticSeconds, messageBody ); - bytes32 messageHash = keccak256(abi.encodePacked(message, optimisticSeconds)); + bytes32 messageHash = keccak256(message); vm.expectEmit(true, true, true, true); emit Dispatch( messageHash, home.count(), (uint64(remoteDomain) << 32) | nonce, home.committedRoot(), - optimisticSeconds, message ); vm.prank(sender); diff --git a/packages/contracts/test/ReplicaManager.t.sol b/packages/contracts/test/ReplicaManager.t.sol index 0ad3c589f5..e42e943f14 100644 --- a/packages/contracts/test/ReplicaManager.t.sol +++ b/packages/contracts/test/ReplicaManager.t.sol @@ -121,7 +121,7 @@ contract ReplicaManagerTest is SynapseTest { // Relayer relays a new root signed by updater on Home chain function test_successfulUpdate() public { bytes memory newMessage = "new root"; - bytes32 newRoot = keccak256(abi.encodePacked(newMessage, optimisticSeconds)); + bytes32 newRoot = keccak256(newMessage); assertEq(replicaManager.updater(), vm.addr(updaterPK)); bytes memory sig = signRemoteUpdate(updaterPK, committedRoot, newRoot); // Root doesn't exist yet @@ -144,7 +144,7 @@ contract ReplicaManagerTest is SynapseTest { function test_updateWithIncorrectSig() public { bytes memory newMessage = "new root"; - bytes32 newRoot = keccak256(abi.encodePacked(newMessage, optimisticSeconds)); + bytes32 newRoot = keccak256(newMessage); bytes memory sig = signRemoteUpdate(fakeUpdaterPK, committedRoot, newRoot); vm.expectRevert("!updater sig"); replicaManager.update(remoteDomain, committedRoot, newRoot, sig); @@ -152,7 +152,7 @@ contract ReplicaManagerTest is SynapseTest { function test_acceptableRoot() public { bytes memory newMessage = "new root"; - bytes32 newRoot = keccak256(abi.encodePacked(newMessage, optimisticSeconds)); + bytes32 newRoot = keccak256(newMessage); test_successfulUpdate(); vm.warp(block.timestamp + optimisticSeconds + 1); assertTrue(replicaManager.acceptableRoot(remoteDomain, optimisticSeconds, newRoot)); From b224d353b54baacea5e3758ba709f6da06f544d8 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Sun, 5 Jun 2022 13:59:46 +0300 Subject: [PATCH 06/11] chore: make ReplicaManager inherit SynapseBase --- packages/contracts/contracts/Home.sol | 48 +++++++++-- .../contracts/contracts/ReplicaManager.sol | 86 +++---------------- packages/contracts/contracts/SynapseBase.sol | 42 ++------- 3 files changed, 63 insertions(+), 113 deletions(-) diff --git a/packages/contracts/contracts/Home.sol b/packages/contracts/contracts/Home.sol index 525a12b359..9af3c301a5 100644 --- a/packages/contracts/contracts/Home.sol +++ b/packages/contracts/contracts/Home.sol @@ -29,6 +29,20 @@ contract Home is Version0, QueueManager, MerkleTreeManager, SynapseBase { using QueueLib for QueueLib.Queue; using MerkleLib for MerkleLib.Tree; + // ============ Enums ============ + + // States: + // 0 - UnInitialized - before initialize function is called + // note: the contract is initialized at deploy time, so it should never be in this state + // 1 - Active - as long as the contract has not become fraudulent + // 2 - Failed - after a valid fraud proof has been submitted; + // contract will no longer accept updates or new messages + enum States { + UnInitialized, + Active, + Failed + } + // ============ Constants ============ // Maximum bytes per message = 2 KiB @@ -41,11 +55,15 @@ contract Home is Version0, QueueManager, MerkleTreeManager, SynapseBase { mapping(uint32 => uint32) public nonces; // contract responsible for Updater bonding, slashing and rotation IUpdaterManager public updaterManager; + // Current state of contract + States public state; + // The latest root that has been signed by the Updater + bytes32 public committedRoot; // ============ Upgrade Gap ============ // gap for upgrade safety - uint256[48] private __GAP; + uint256[46] private __GAP; // ============ Events ============ @@ -112,6 +130,7 @@ contract Home is Version0, QueueManager, MerkleTreeManager, SynapseBase { __QueueManager_initialize(); _setUpdaterManager(_updaterManager); __SynapseBase_initialize(updaterManager.updater()); + state = States.Active; } // ============ Modifiers ============ @@ -263,8 +282,8 @@ contract Home is Version0, QueueManager, MerkleTreeManager, SynapseBase { bytes calldata _signature2 ) external notFailed { if ( - SynapseBase._isUpdaterSignature(_oldRoot, _newRoot[0], _signature) && - SynapseBase._isUpdaterSignature(_oldRoot, _newRoot[1], _signature2) && + _isUpdaterSignature(_oldRoot, _newRoot[0], _signature) && + _isUpdaterSignature(_oldRoot, _newRoot[1], _signature2) && _newRoot[0] != _newRoot[1] ) { _fail(); @@ -272,15 +291,15 @@ contract Home is Version0, QueueManager, MerkleTreeManager, SynapseBase { } } - // ============ Public Functions ============ - /** * @notice Hash of Home domain concatenated with "SYN" */ - function homeDomainHash() public view override returns (bytes32) { - return _homeDomainHash(localDomain); + function homeDomainHash() external view returns (bytes32) { + return _domainHash(localDomain); } + // ============ Public Functions ============ + /** * @notice Check if an Update is an Improper Update; * if so, slash the Updater and set the contract to FAILED state. @@ -329,6 +348,21 @@ contract Home is Version0, QueueManager, MerkleTreeManager, SynapseBase { // ============ Internal Functions ============ + /** + * @notice Checks that signature was signed by Updater on the local chain + * @param _oldRoot Old merkle root + * @param _newRoot New merkle root + * @param _signature Signature on `_oldRoot` and `_newRoot` + * @return TRUE if signature is valid signed by updater + **/ + function _isUpdaterSignature( + bytes32 _oldRoot, + bytes32 _newRoot, + bytes memory _signature + ) internal view returns (bool) { + return _isUpdaterSignature(localDomain, _oldRoot, _newRoot, _signature); + } + /** * @notice Set the UpdaterManager * @param _updaterManager Address of the UpdaterManager diff --git a/packages/contracts/contracts/ReplicaManager.sol b/packages/contracts/contracts/ReplicaManager.sol index 5861fe8328..e354982711 100644 --- a/packages/contracts/contracts/ReplicaManager.sol +++ b/packages/contracts/contracts/ReplicaManager.sol @@ -2,6 +2,7 @@ pragma solidity 0.8.13; // ============ Internal Imports ============ +import { SynapseBase } from "./SynapseBase.sol"; import { Version0 } from "./Version0.sol"; import { ReplicaLib } from "./libs/Replica.sol"; import { MerkleLib } from "./libs/Merkle.sol"; @@ -19,7 +20,7 @@ import { * @notice Track root updates on Home, * prove and dispatch messages to end recipients. */ -contract ReplicaManager is Version0, Initializable, OwnableUpgradeable { +contract ReplicaManager is Version0, SynapseBase { // ============ Libraries ============ using ReplicaLib for ReplicaLib.Replica; @@ -30,9 +31,6 @@ contract ReplicaManager is Version0, Initializable, OwnableUpgradeable { // ============ Immutables ============ - // Domain of chain on which the contract is deployed - uint32 public immutable localDomain; - // Minimum gas for message processing uint256 public immutable PROCESS_GAS; // Reserved gas (to ensure tx completes in case message processing runs out) @@ -55,13 +53,10 @@ contract ReplicaManager is Version0, Initializable, OwnableUpgradeable { // (domain => [replica indexes]): array of indexes of archived replicas in allReplicas mapping(uint32 => uint256[]) internal archivedReplicas; - // Address of bonded Updater - address public updater; - // ============ Upgrade Gap ============ // gap for upgrade safety - uint256[44] private __GAP; + uint256[45] private __GAP; // ============ Events ============ @@ -98,36 +93,13 @@ contract ReplicaManager is Version0, Initializable, OwnableUpgradeable { uint256 newConfirmAt ); - /** - * @notice Emitted when update is made on Home - * or unconfirmed update root is submitted on Replica - * @param homeDomain Domain of home contract - * @param oldRoot Old merkle root - * @param newRoot New merkle root - * @param signature Updater's signature on `oldRoot` and `newRoot` - */ - event Update( - uint32 indexed homeDomain, - bytes32 indexed oldRoot, - bytes32 indexed newRoot, - bytes signature - ); - - /** - * @notice Emitted when Updater is rotated - * @param oldUpdater The address of the old updater - * @param newUpdater The address of the new updater - */ - event NewUpdater(address oldUpdater, address newUpdater); - // ============ Constructor ============ constructor( uint32 _localDomain, uint256 _processGas, uint256 _reserveGas - ) { - localDomain = _localDomain; + ) SynapseBase(_localDomain) { require(_processGas >= 850_000, "!process gas"); require(_reserveGas >= 15_000, "!reserve gas"); PROCESS_GAS = _processGas; @@ -153,8 +125,7 @@ contract ReplicaManager is Version0, Initializable, OwnableUpgradeable { address _updater, uint32 _optimisticSeconds ) public initializer { - __Ownable_init(); - _setUpdater(_updater); + __SynapseBase_initialize(_updater); // set storage variables entered = 1; activeReplicas[_remoteDomain] = _createReplica(_remoteDomain, _optimisticSeconds); @@ -222,19 +193,6 @@ contract ReplicaManager is Version0, Initializable, OwnableUpgradeable { emit Update(_remoteDomain, _oldRoot, _newRoot, _signature); } - function _isUpdaterSignature( - uint32 _remoteDomain, - bytes32 _oldRoot, - bytes32 _newRoot, - bytes memory _signature - ) internal view returns (bool) { - bytes32 _digest = keccak256( - abi.encodePacked(_homeDomainHash(_remoteDomain), _oldRoot, _newRoot) - ); - _digest = ECDSA.toEthSignedMessageHash(_digest); - return (ECDSA.recover(_digest, _signature) == updater); - } - /** * @notice First attempts to prove the validity of provided formatted * `message`. If the message is successfully proven, then tries to process @@ -331,6 +289,14 @@ contract ReplicaManager is Version0, Initializable, OwnableUpgradeable { entered = 1; } + /** + * @notice Hash of Home domain concatenated with "SYN" + * @param _homeDomain the Home domain to hash + */ + function homeDomainHash(uint32 _homeDomain) external pure returns (bytes32) { + return _domainHash(_homeDomain); + } + // ============ External Owner Functions ============ /** @@ -422,14 +388,6 @@ contract ReplicaManager is Version0, Initializable, OwnableUpgradeable { return false; } - /** - * @notice Hash of Home domain concatenated with "SYN" - * @param _homeDomain the Home domain to hash - */ - function homeDomainHash(uint32 _homeDomain) public pure returns (bytes32) { - return _homeDomainHash(_homeDomain); - } - // ============ Internal Functions ============ function _createReplica(uint32 _remoteDomain, uint32 _optimisticSeconds) @@ -443,24 +401,6 @@ contract ReplicaManager is Version0, Initializable, OwnableUpgradeable { } } - /** - * @notice Hash of Home domain concatenated with "SYN" - * @param _homeDomain the Home domain to hash - */ - function _homeDomainHash(uint32 _homeDomain) internal pure returns (bytes32) { - return keccak256(abi.encodePacked(_homeDomain, "SYN")); - } - - /** - * @notice Set the Updater - * @param _newUpdater Address of the new Updater - */ - function _setUpdater(address _newUpdater) internal { - address _oldUpdater = updater; - updater = _newUpdater; - emit NewUpdater(_oldUpdater, _newUpdater); - } - /// @notice Hook for potential future use // solhint-disable-next-line no-empty-blocks function _beforeUpdate() internal {} diff --git a/packages/contracts/contracts/SynapseBase.sol b/packages/contracts/contracts/SynapseBase.sol index 5fe9b5e68b..3f78c10293 100644 --- a/packages/contracts/contracts/SynapseBase.sol +++ b/packages/contracts/contracts/SynapseBase.sol @@ -16,20 +16,6 @@ import { * @notice Shared utilities between Home and Replica. */ abstract contract SynapseBase is Initializable, OwnableUpgradeable { - // ============ Enums ============ - - // States: - // 0 - UnInitialized - before initialize function is called - // note: the contract is initialized at deploy time, so it should never be in this state - // 1 - Active - as long as the contract has not become fraudulent - // 2 - Failed - after a valid fraud proof has been submitted; - // contract will no longer accept updates or new messages - enum States { - UnInitialized, - Active, - Failed - } - // ============ Immutable Variables ============ // Domain of chain on which the contract is deployed @@ -39,15 +25,11 @@ abstract contract SynapseBase is Initializable, OwnableUpgradeable { // Address of bonded Updater address public updater; - // Current state of contract - States public state; - // The latest root that has been signed by the Updater - bytes32 public committedRoot; // ============ Upgrade Gap ============ // gap for upgrade safety - uint256[47] private __GAP; + uint256[49] private __GAP; // ============ Events ============ @@ -84,24 +66,16 @@ abstract contract SynapseBase is Initializable, OwnableUpgradeable { function __SynapseBase_initialize(address _updater) internal onlyInitializing { __Ownable_init(); _setUpdater(_updater); - state = States.Active; } - // ============ Public Functions ============ - - /** - * @notice Hash of Home domain concatenated with "SYN" - */ - function homeDomainHash() public view virtual returns (bytes32); - // ============ Internal Functions ============ /** - * @notice Hash of Home domain concatenated with "SYN" - * @param _homeDomain the Home domain to hash + * @notice Hash of domain concatenated with "SYN" + * @param _domain The domain to hash */ - function _homeDomainHash(uint32 _homeDomain) internal pure returns (bytes32) { - return keccak256(abi.encodePacked(_homeDomain, "SYN")); + function _domainHash(uint32 _domain) internal pure returns (bytes32) { + return keccak256(abi.encodePacked(_domain, "SYN")); } /** @@ -116,17 +90,19 @@ abstract contract SynapseBase is Initializable, OwnableUpgradeable { /** * @notice Checks that signature was signed by Updater + * @param _homeDomain Domain of Home contract where the signing was done * @param _oldRoot Old merkle root * @param _newRoot New merkle root * @param _signature Signature on `_oldRoot` and `_newRoot` - * @return TRUE iff signature is valid signed by updater + * @return TRUE if signature is valid signed by updater **/ function _isUpdaterSignature( + uint32 _homeDomain, bytes32 _oldRoot, bytes32 _newRoot, bytes memory _signature ) internal view returns (bool) { - bytes32 _digest = keccak256(abi.encodePacked(homeDomainHash(), _oldRoot, _newRoot)); + bytes32 _digest = keccak256(abi.encodePacked(_domainHash(_homeDomain), _oldRoot, _newRoot)); _digest = ECDSA.toEthSignedMessageHash(_digest); return (ECDSA.recover(_digest, _signature) == updater); } From 614b2fcf05442ae4f12d2bd5bed4cea2c966bbdd Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Sun, 5 Jun 2022 14:08:30 +0300 Subject: [PATCH 07/11] chore: remove unused imports --- packages/contracts/contracts/ReplicaManager.sol | 5 ----- 1 file changed, 5 deletions(-) diff --git a/packages/contracts/contracts/ReplicaManager.sol b/packages/contracts/contracts/ReplicaManager.sol index e354982711..7ec3464c36 100644 --- a/packages/contracts/contracts/ReplicaManager.sol +++ b/packages/contracts/contracts/ReplicaManager.sol @@ -9,11 +9,6 @@ import { MerkleLib } from "./libs/Merkle.sol"; import { Message } from "./libs/Message.sol"; // ============ External Imports ============ import { TypedMemView } from "./libs/TypedMemView.sol"; -import { ECDSA } from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; -import { Initializable } from "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; -import { - OwnableUpgradeable -} from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol"; /** * @title ReplicaManager From 8d25ad09ad963c63db0a8349fa93990669fe894a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=CF=87=C2=B2?= <88190723+ChiTimesChi@users.noreply.github.com> Date: Mon, 20 Jun 2022 22:49:04 +0300 Subject: [PATCH 08/11] dapp optimistic period (#47) * chore: replica struct comments correct ordering * feat: remove optimisticSeconds from Replica * fix: wrong message status in prove() * feat: store generalised "message status" * fix: do not store failed messages as processed * fix: root optimistic period checking * test: replica.process() * add timeout Co-authored-by: Trajan0x --- .github/workflows/go.yml | 1 + .../contracts/contracts/ReplicaManager.sol | 93 ++++++++----------- .../interfaces/IMessageRecipient.sol | 1 + packages/contracts/contracts/libs/Message.sol | 15 ++- packages/contracts/contracts/libs/Replica.sol | 36 +++---- packages/contracts/test/Replica.t.sol | 16 +--- packages/contracts/test/ReplicaManager.t.sol | 89 +++++++++++++----- .../contracts/test/harnesses/AppHarness.sol | 44 +++++++++ .../test/harnesses/ReplicaManagerHarness.sol | 12 +++ 9 files changed, 199 insertions(+), 108 deletions(-) create mode 100644 packages/contracts/test/harnesses/AppHarness.sol diff --git a/.github/workflows/go.yml b/.github/workflows/go.yml index 1ca0e6b2e5..20057a4759 100644 --- a/.github/workflows/go.yml +++ b/.github/workflows/go.yml @@ -132,6 +132,7 @@ jobs: # Path to your GolangCI-Lint config within the repo (optional) config: ${{ env.GITHUB_WORKSPACE }}/.golangci.yml # see: https://github.com/golangci/golangci-lint/issues/2654 + args: --timeout=5m env: # GitHub token for annotations (optional) GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} diff --git a/packages/contracts/contracts/ReplicaManager.sol b/packages/contracts/contracts/ReplicaManager.sol index 4f325453c3..23361e6099 100644 --- a/packages/contracts/contracts/ReplicaManager.sol +++ b/packages/contracts/contracts/ReplicaManager.sol @@ -6,6 +6,7 @@ import { Version0 } from "./Version0.sol"; import { ReplicaLib } from "./libs/Replica.sol"; import { MerkleLib } from "./libs/Merkle.sol"; import { Message } from "./libs/Message.sol"; +import { IMessageRecipient } from "./interfaces/IMessageRecipient.sol"; // ============ External Imports ============ import { TypedMemView } from "./libs/TypedMemView.sol"; import { ECDSA } from "@openzeppelin/contracts/utils/cryptography/ECDSA.sol"; @@ -79,12 +80,6 @@ contract ReplicaManager is Version0, Initializable, OwnableUpgradeable { bytes returnData ); - /** - * @notice Emitted when the value for optimisticTimeout is set - * @param timeout The new value for optimistic timeout - */ - event SetOptimisticTimeout(uint32 indexed remoteDomain, uint32 timeout); - /** * @notice Emitted when a root's confirmation is modified by governance * @param root The root for which confirmAt has been set @@ -146,19 +141,13 @@ contract ReplicaManager is Version0, Initializable, OwnableUpgradeable { * - sets the optimistic timer * @param _remoteDomain The domain of the Home contract this follows * @param _updater The EVM id of the updater - * @param _optimisticSeconds The time a new root must wait to be confirmed */ - function initialize( - uint32 _remoteDomain, - address _updater, - uint32 _optimisticSeconds - ) public initializer { + function initialize(uint32 _remoteDomain, address _updater) public initializer { __Ownable_init(); _setUpdater(_updater); // set storage variables entered = 1; - activeReplicas[_remoteDomain] = _createReplica(_remoteDomain, _optimisticSeconds); - emit SetOptimisticTimeout(_remoteDomain, _optimisticSeconds); + activeReplicas[_remoteDomain] = _createReplica(_remoteDomain); } // ============ Active Replica Views ============ @@ -167,10 +156,6 @@ contract ReplicaManager is Version0, Initializable, OwnableUpgradeable { return allReplicas[activeReplicas[_remoteDomain]].committedRoot; } - function activeReplicaOptimisticSeconds(uint32 _remoteDomain) external view returns (uint32) { - return allReplicas[activeReplicas[_remoteDomain]].optimisticSeconds; - } - function activeReplicaConfirmedAt(uint32 _remoteDomain, bytes32 _root) external view @@ -182,9 +167,9 @@ contract ReplicaManager is Version0, Initializable, OwnableUpgradeable { function activeReplicaMessageStatus(uint32 _remoteDomain, bytes32 _messageId) external view - returns (ReplicaLib.MessageStatus) + returns (bytes32) { - return allReplicas[activeReplicas[_remoteDomain]].messages[_messageId]; + return allReplicas[activeReplicas[_remoteDomain]].messageStatus[_messageId]; } // ============ Archived Replica Views ============ @@ -272,12 +257,14 @@ contract ReplicaManager is Version0, Initializable, OwnableUpgradeable { require(_m.destination() == localDomain, "!destination"); // ensure message has been proven bytes32 _messageHash = _m.keccak(); - require(replica.messages[_messageHash] == ReplicaLib.MessageStatus.Proven, "!proven"); + bytes32 _root = replica.messageStatus[_messageHash]; + require(ReplicaLib.isPotentialRoot(_root), "!exists || processed"); + require(acceptableRoot(_remoteDomain, _m.optimisticSeconds(), _root), "!optimisticSeconds"); // check re-entrancy guard require(entered == 1, "!reentrant"); entered = 0; // update message status as processed - replica.setMessageStatus(_messageHash, ReplicaLib.MessageStatus.Processed); + replica.setMessageStatus(_messageHash, ReplicaLib.MESSAGE_STATUS_PROCESSED); // A call running out of gas TYPICALLY errors the whole tx. We want to // a) ensure the call has a sufficient amount of gas to make a // meaningful state change. @@ -286,6 +273,14 @@ contract ReplicaManager is Version0, Initializable, OwnableUpgradeable { // To do this, we require that we have enough gas to process // and still return. We then delegate only the minimum processing gas. require(gasleft() >= PROCESS_GAS + RESERVE_GAS, "!gas"); + bytes memory _calldata = abi.encodeWithSelector( + IMessageRecipient.handle.selector, + _remoteDomain, + _m.nonce(), + _m.sender(), + replica.confirmAt[_root], + _m.body().clone() + ); // get the message recipient address _recipient = _m.recipientAddress(); // set up for assembly call @@ -294,13 +289,7 @@ contract ReplicaManager is Version0, Initializable, OwnableUpgradeable { uint256 _gas = PROCESS_GAS; // allocate memory for returndata bytes memory _returnData = new bytes(_maxCopy); - bytes memory _calldata = abi.encodeWithSignature( - "handle(uint32,uint32,bytes32,bytes)", - _m.origin(), - _m.nonce(), - _m.sender(), - _m.body().clone() - ); + // dispatch message to recipient // by assembly calling "handle" function // we call via assembly to avoid memcopying a very large returndata @@ -325,6 +314,7 @@ contract ReplicaManager is Version0, Initializable, OwnableUpgradeable { // copy the bytes from returndata[0:_toCopy] returndatacopy(add(_returnData, 0x20), 0, _toCopy) } + if (!_success) revert(_getRevertMsg(_returnData)); // emit process results emit Process(_remoteDomain, _messageHash, _success, _returnData); // reset re-entrancy guard @@ -333,19 +323,6 @@ contract ReplicaManager is Version0, Initializable, OwnableUpgradeable { // ============ External Owner Functions ============ - /** - * @notice Set optimistic timeout period for new roots - * @dev Only callable by owner (Governance) - * @param _optimisticSeconds New optimistic timeout period - */ - function setOptimisticTimeout(uint32 _remoteDomain, uint32 _optimisticSeconds) - external - onlyOwner - { - allReplicas[activeReplicas[_remoteDomain]].setOptimisticTimeout(_optimisticSeconds); - emit SetOptimisticTimeout(_remoteDomain, _optimisticSeconds); - } - /** * @notice Set Updater role * @dev MUST ensure that all roots signed by previous Updater have @@ -392,7 +369,7 @@ contract ReplicaManager is Version0, Initializable, OwnableUpgradeable { if (_time == 0) { return false; } - return block.timestamp + _optimisticSeconds >= _time; + return block.timestamp >= _time + _optimisticSeconds; } /** @@ -413,16 +390,18 @@ contract ReplicaManager is Version0, Initializable, OwnableUpgradeable { bytes32[32] calldata _proof, uint256 _index ) public returns (bool) { - uint32 optimisticSeconds = _message.ref(0).optimisticSeconds(); bytes32 _leaf = keccak256(_message); ReplicaLib.Replica storage replica = allReplicas[activeReplicas[_remoteDomain]]; // ensure that message has not been proven or processed - require(replica.messages[_leaf] == ReplicaLib.MessageStatus.None, "!MessageStatus.None"); + require( + replica.messageStatus[_leaf] == ReplicaLib.MESSAGE_STATUS_NONE, + "!MessageStatus.None" + ); // calculate the expected root based on the proof bytes32 _calculatedRoot = MerkleLib.branchRoot(_leaf, _proof, _index); - // if the root is valid, change status to Proven - if (acceptableRoot(_remoteDomain, optimisticSeconds, _calculatedRoot)) { - replica.setMessageStatus(_leaf, ReplicaLib.MessageStatus.Processed); + // if the root is valid, save it for later optimistic period checking + if (replica.confirmAt[_calculatedRoot] != 0) { + replica.setMessageStatus(_leaf, _calculatedRoot); return true; } return false; @@ -438,12 +417,9 @@ contract ReplicaManager is Version0, Initializable, OwnableUpgradeable { // ============ Internal Functions ============ - function _createReplica(uint32 _remoteDomain, uint32 _optimisticSeconds) - internal - returns (uint256 replicaIndex) - { + function _createReplica(uint32 _remoteDomain) internal returns (uint256 replicaIndex) { replicaIndex = replicaCount; - allReplicas[replicaIndex].setupReplica(_remoteDomain, _optimisticSeconds); + allReplicas[replicaIndex].setupReplica(_remoteDomain); unchecked { replicaCount = replicaIndex + 1; } @@ -470,4 +446,15 @@ contract ReplicaManager is Version0, Initializable, OwnableUpgradeable { /// @notice Hook for potential future use // solhint-disable-next-line no-empty-blocks function _beforeUpdate() internal {} + + function _getRevertMsg(bytes memory _returnData) internal pure returns (string memory) { + // If the _res length is less than 68, then the transaction failed silently (without a revert message) + if (_returnData.length < 68) return "Transaction reverted silently"; + + assembly { + // Slice the sighash. + _returnData := add(_returnData, 0x04) + } + return abi.decode(_returnData, (string)); // All that remains is the revert string + } } diff --git a/packages/contracts/contracts/interfaces/IMessageRecipient.sol b/packages/contracts/contracts/interfaces/IMessageRecipient.sol index 0996eb6a1c..9ddd8e449c 100644 --- a/packages/contracts/contracts/interfaces/IMessageRecipient.sol +++ b/packages/contracts/contracts/interfaces/IMessageRecipient.sol @@ -6,6 +6,7 @@ interface IMessageRecipient { uint32 _origin, uint32 _nonce, bytes32 _sender, + uint256 _rootTimestamp, bytes memory _message ) external; } diff --git a/packages/contracts/contracts/libs/Message.sol b/packages/contracts/contracts/libs/Message.sol index c7ca2e241f..a5c22cd9ac 100644 --- a/packages/contracts/contracts/libs/Message.sol +++ b/packages/contracts/contracts/libs/Message.sol @@ -67,7 +67,18 @@ library Message { uint32 _optimisticSeconds, bytes memory _body ) internal pure returns (bytes32) { - return keccak256(formatMessage(_origin, _sender, _nonce, _destination, _recipient, _optimisticSeconds, _body)); + return + keccak256( + formatMessage( + _origin, + _sender, + _nonce, + _destination, + _recipient, + _optimisticSeconds, + _body + ) + ); } /// @notice Returns message's origin field @@ -96,7 +107,7 @@ library Message { } /// @notice Returns the optimistic seconds from the message - function optimisticSeconds(bytes29 _message) internal pure returns (uint32){ + function optimisticSeconds(bytes29 _message) internal pure returns (uint32) { return uint32(_message.indexUint(76, 4)); } diff --git a/packages/contracts/contracts/libs/Replica.sol b/packages/contracts/contracts/libs/Replica.sol index d2c2babe58..144dbf965a 100644 --- a/packages/contracts/contracts/libs/Replica.sol +++ b/packages/contracts/contracts/libs/Replica.sol @@ -25,30 +25,32 @@ library ReplicaLib { Failed } + // ============ Constants ============ + /// @dev Should not be possible to have 0x0 or 0x1 as valid Merkle root, + /// so it's safe to use those values as NONE/PROCESSED + bytes32 public constant MESSAGE_STATUS_NONE = bytes32(0); + bytes32 public constant MESSAGE_STATUS_PROCESSED = bytes32(uint256(1)); + // TODO: optimize read/writes by further packing? struct Replica { // The latest root that has been signed by the Updater for this given Replica bytes32 committedRoot; // 256 bits // Domain of home chain - uint32 optimisticSeconds; // 32 bits - // Status of Replica based on the Home remote domain uint32 remoteDomain; // 32 bits - // Optimistic seconds per remote domain (E.g specifies optimistic seconds on a remote domain basis to wait) + // Status of Replica based on the Home remote domain ReplicaStatus status; // 8 bits // Mapping of roots to time at which Relayer submitted on-chain. Latency period begins here. // TODO: confirmAt doesn't need to be uint256 necessarily mapping(bytes32 => uint256) confirmAt; - // Mapping of message leaves to MessageStatus - mapping(bytes32 => MessageStatus) messages; + // Mapping of message leaves to status: + // - NONE: message not yet submitted + // - PROCESSED: message was proven and processed + // bytes32 root: message was proven against `root`, but not yet processed + mapping(bytes32 => bytes32) messageStatus; } - function setupReplica( - Replica storage replica, - uint32 _remoteDomain, - uint32 _optimisticSeconds - ) internal { + function setupReplica(Replica storage replica, uint32 _remoteDomain) internal { replica.remoteDomain = _remoteDomain; - replica.optimisticSeconds = _optimisticSeconds; replica.status = ReplicaStatus.Active; } @@ -67,16 +69,16 @@ library ReplicaLib { function setMessageStatus( Replica storage replica, bytes32 _messageHash, - MessageStatus _status + bytes32 _status ) internal { - replica.messages[_messageHash] = _status; - } - - function setOptimisticTimeout(Replica storage replica, uint32 _optimisticSeconds) internal { - replica.optimisticSeconds = _optimisticSeconds; + replica.messageStatus[_messageHash] = _status; } function setStatus(Replica storage replica, ReplicaStatus _status) internal { replica.status = _status; } + + function isPotentialRoot(bytes32 messageStatus) internal pure returns (bool) { + return messageStatus != MESSAGE_STATUS_NONE && messageStatus != MESSAGE_STATUS_PROCESSED; + } } diff --git a/packages/contracts/test/Replica.t.sol b/packages/contracts/test/Replica.t.sol index c8fd74a6ae..5226f8baf1 100644 --- a/packages/contracts/test/Replica.t.sol +++ b/packages/contracts/test/Replica.t.sol @@ -11,18 +11,15 @@ contract ReplicaTest is SynapseTest { using ReplicaLib for ReplicaLib.Replica; ReplicaLib.Replica replica; - uint32 optimisticSeconds; function setUp() public override { super.setUp(); - optimisticSeconds = 10; - replica.setupReplica(remoteDomain, optimisticSeconds); + replica.setupReplica(remoteDomain); } function test_setup() public { assertEq(replica.committedRoot, bytes32("")); assertEq(replica.remoteDomain, remoteDomain); - assertEq(replica.optimisticSeconds, optimisticSeconds); assertEq(uint256(replica.status), 1); } @@ -36,14 +33,9 @@ contract ReplicaTest is SynapseTest { assertEq(replica.confirmAt[_committedRoot], _confirmAt); } - function test_setMessageStatus(bytes32 _messageHash) public { - replica.setMessageStatus(_messageHash, ReplicaLib.MessageStatus.Processed); - assertEq(uint256(replica.messages[_messageHash]), 2); - } - - function test_setOptimisticTimeout(uint32 _optimisticSeconds) public { - replica.setOptimisticTimeout(_optimisticSeconds); - assertEq(replica.optimisticSeconds, _optimisticSeconds); + function test_setMessageStatus(bytes32 _messageHash, bytes32 _status) public { + replica.setMessageStatus(_messageHash, _status); + assertEq(replica.messageStatus[_messageHash], _status); } function test_setStatus() public { diff --git a/packages/contracts/test/ReplicaManager.t.sol b/packages/contracts/test/ReplicaManager.t.sol index e42e943f14..214966cdc4 100644 --- a/packages/contracts/test/ReplicaManager.t.sol +++ b/packages/contracts/test/ReplicaManager.t.sol @@ -5,18 +5,24 @@ pragma solidity 0.8.13; import "forge-std/Test.sol"; import { TypedMemView } from "../contracts/libs/TypedMemView.sol"; +import { TypeCasts } from "../contracts/libs/TypeCasts.sol"; + import { Message } from "../contracts/libs/Message.sol"; import { ReplicaLib } from "../contracts/libs/Replica.sol"; import { ReplicaManagerHarness } from "./harnesses/ReplicaManagerHarness.sol"; +import { AppHarness } from "./harnesses/AppHarness.sol"; + import { SynapseTest } from "./utils/SynapseTest.sol"; contract ReplicaManagerTest is SynapseTest { ReplicaManagerHarness replicaManager; + AppHarness dApp; + + uint32 internal constant OPTIMISTIC_PERIOD = 10; - uint32 optimisticSeconds; bytes32 committedRoot; uint256 processGas; uint256 reserveGas; @@ -27,12 +33,12 @@ contract ReplicaManagerTest is SynapseTest { function setUp() public override { super.setUp(); - optimisticSeconds = 10; committedRoot = ""; processGas = 850_000; reserveGas = 15_000; replicaManager = new ReplicaManagerHarness(localDomain, processGas, reserveGas); - replicaManager.initialize(remoteDomain, updater, optimisticSeconds); + replicaManager.initialize(remoteDomain, updater); + dApp = new AppHarness(OPTIMISTIC_PERIOD); } // ============ INITIAL STATE ============ @@ -47,7 +53,7 @@ contract ReplicaManagerTest is SynapseTest { function test_cannotInitializeTwice() public { vm.expectRevert("Initializable: contract is already initialized"); - replicaManager.initialize(remoteDomain, updater, optimisticSeconds); + replicaManager.initialize(remoteDomain, updater); } // ============ STATE & PERMISSIONING ============ @@ -66,24 +72,6 @@ contract ReplicaManagerTest is SynapseTest { assertEq(replicaManager.updater(), _updater); } - function test_cannotSetOptimisticTimeoutAsNotOwner(address _notOwner) public { - vm.assume(_notOwner != replicaManager.owner()); - vm.prank(_notOwner); - vm.expectRevert("Ownable: caller is not the owner"); - replicaManager.setOptimisticTimeout(remoteDomain, 10); - } - - event SetOptimisticTimeout(uint32 indexed remoteDomain, uint32 timeout); - - function test_setOptimisticTimeout(uint32 _optimisticSeconds) public { - vm.startPrank(replicaManager.owner()); - assertEq(replicaManager.activeReplicaOptimisticSeconds(remoteDomain), 10); - vm.expectEmit(true, false, false, true); - emit SetOptimisticTimeout(remoteDomain, _optimisticSeconds); - replicaManager.setOptimisticTimeout(remoteDomain, _optimisticSeconds); - assertEq(replicaManager.activeReplicaOptimisticSeconds(remoteDomain), _optimisticSeconds); - } - function test_cannotSetConfirmationAsNotOwner(address _notOwner) public { vm.assume(_notOwner != replicaManager.owner()); vm.prank(_notOwner); @@ -136,7 +124,6 @@ contract ReplicaManagerTest is SynapseTest { } function test_updateWithIncorrectRoot() public { - bytes memory newMessage = "new root"; bytes32 newRoot = "new root"; vm.expectRevert("not current update"); replicaManager.update(remoteDomain, newRoot, newRoot, bytes("")); @@ -153,15 +140,69 @@ contract ReplicaManagerTest is SynapseTest { function test_acceptableRoot() public { bytes memory newMessage = "new root"; bytes32 newRoot = keccak256(newMessage); + uint32 optimisticSeconds = 69; test_successfulUpdate(); - vm.warp(block.timestamp + optimisticSeconds + 1); + vm.warp(block.timestamp + optimisticSeconds); assertTrue(replicaManager.acceptableRoot(remoteDomain, optimisticSeconds, newRoot)); } function test_cannotAcceptableRoot() public { bytes32 newRoot = "new root"; test_successfulUpdate(); + uint32 optimisticSeconds = 69; vm.warp(block.timestamp + optimisticSeconds - 1); assertFalse(replicaManager.acceptableRoot(remoteDomain, optimisticSeconds, newRoot)); } + + function test_process() public { + bytes memory message = _prepareProcessTest(OPTIMISTIC_PERIOD); + vm.warp(block.timestamp + OPTIMISTIC_PERIOD); + replicaManager.process(message); + } + + function test_processPeriodNotPassed() public { + bytes memory message = _prepareProcessTest(OPTIMISTIC_PERIOD); + vm.warp(block.timestamp + OPTIMISTIC_PERIOD - 1); + vm.expectRevert("!optimisticSeconds"); + replicaManager.process(message); + } + + function test_processForgedPeriodReduced() public { + bytes memory message = _prepareProcessTest(OPTIMISTIC_PERIOD - 1); + vm.warp(block.timestamp + OPTIMISTIC_PERIOD - 1); + vm.expectRevert("app: !optimisticSeconds"); + replicaManager.process(message); + } + + function test_processForgePeriodZero() public { + bytes memory message = _prepareProcessTest(0); + vm.expectRevert("app: !optimisticSeconds"); + replicaManager.process(message); + } + + function _prepareProcessTest(uint32 optimisticPeriod) internal returns (bytes memory message) { + test_successfulUpdate(); + + bytes32 root = replicaManager.activeReplicaCommittedRoot(remoteDomain); + assert(root != bytes32(0)); + + uint32 nonce = 1234; + bytes32 sender = "sender"; + bytes memory messageBody = "message body"; + dApp.prepare(remoteDomain, nonce, sender, messageBody); + bytes32 recipient = TypeCasts.addressToBytes32(address(dApp)); + + message = Message.formatMessage( + remoteDomain, + sender, + nonce, + localDomain, + recipient, + optimisticPeriod, + messageBody + ); + bytes32 messageHash = keccak256(message); + // Let's imagine message was proved against current root + replicaManager.setMessageStatus(remoteDomain, messageHash, root); + } } diff --git a/packages/contracts/test/harnesses/AppHarness.sol b/packages/contracts/test/harnesses/AppHarness.sol new file mode 100644 index 0000000000..8e21e6e311 --- /dev/null +++ b/packages/contracts/test/harnesses/AppHarness.sol @@ -0,0 +1,44 @@ +// SPDX-License-Identifier: MIT + +pragma solidity 0.8.13; + +import { IMessageRecipient } from "../../contracts/interfaces/IMessageRecipient.sol"; + +contract AppHarness is IMessageRecipient { + uint32 public optimisticSeconds; + + uint32 public expectedOrigin; + uint32 public expectedNonce; + bytes32 public expectedSender; + bytes32 expectedMessageBodyHash; + + constructor(uint32 _optimisticSeconds) { + optimisticSeconds = _optimisticSeconds; + } + + function prepare( + uint32 _origin, + uint32 _nonce, + bytes32 _sender, + bytes memory _message + ) external { + expectedOrigin = _origin; + expectedNonce = _nonce; + expectedSender = _sender; + expectedMessageBodyHash = keccak256(_message); + } + + function handle( + uint32 _origin, + uint32 _nonce, + bytes32 _sender, + uint256 _rootTimestamp, + bytes memory _message + ) external view { + require(block.timestamp >= _rootTimestamp + optimisticSeconds, "app: !optimisticSeconds"); + require(_origin == expectedOrigin, "app: !origin"); + require(_nonce == expectedNonce, "app: !nonce"); + require(_sender == expectedSender, "app: !sender"); + require(keccak256(_message) == expectedMessageBodyHash, "app: !message"); + } +} diff --git a/packages/contracts/test/harnesses/ReplicaManagerHarness.sol b/packages/contracts/test/harnesses/ReplicaManagerHarness.sol index a37b1c3b54..a3c498b6bd 100644 --- a/packages/contracts/test/harnesses/ReplicaManagerHarness.sol +++ b/packages/contracts/test/harnesses/ReplicaManagerHarness.sol @@ -4,10 +4,22 @@ pragma solidity 0.8.13; import { ReplicaManager } from "../../contracts/ReplicaManager.sol"; +import { ReplicaLib } from "../../contracts/libs/Replica.sol"; + contract ReplicaManagerHarness is ReplicaManager { + using ReplicaLib for ReplicaLib.Replica; + constructor( uint32 _localDomain, uint256 _processGas, uint256 _reserveGas ) ReplicaManager(_localDomain, _processGas, _reserveGas) {} + + function setMessageStatus( + uint32 _remoteDomain, + bytes32 _messageHash, + bytes32 _status + ) external { + allReplicas[activeReplicas[_remoteDomain]].setMessageStatus(_messageHash, _status); + } } From 787bb51a6fd36b6e6a75af67dbadcb6fefcbf484 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Tue, 28 Jun 2022 17:21:43 +0300 Subject: [PATCH 09/11] chore: SynapseBase -> UpdaterStorage --- packages/contracts/contracts/Home.sol | 6 +++--- packages/contracts/contracts/ReplicaManager.sol | 8 ++++---- .../contracts/{SynapseBase.sol => UpdaterStorage.sol} | 4 ++-- 3 files changed, 9 insertions(+), 9 deletions(-) rename packages/contracts/contracts/{SynapseBase.sol => UpdaterStorage.sol} (97%) diff --git a/packages/contracts/contracts/Home.sol b/packages/contracts/contracts/Home.sol index cefc0ffeca..1f0086332c 100644 --- a/packages/contracts/contracts/Home.sol +++ b/packages/contracts/contracts/Home.sol @@ -3,7 +3,7 @@ pragma solidity 0.8.13; // ============ Internal Imports ============ import { Version0 } from "./Version0.sol"; -import { SynapseBase } from "./SynapseBase.sol"; +import { UpdaterStorage } from "./UpdaterStorage.sol"; import { QueueLib } from "./libs/Queue.sol"; import { MerkleLib } from "./libs/Merkle.sol"; import { Message } from "./libs/Message.sol"; @@ -23,7 +23,7 @@ import { Address } from "@openzeppelin/contracts/utils/Address.sol"; * Accepts submissions of fraudulent signatures * by the Updater and slashes the Updater in this case. */ -contract Home is Version0, QueueManager, MerkleTreeManager, SynapseBase { +contract Home is Version0, QueueManager, MerkleTreeManager, UpdaterStorage { // ============ Libraries ============ using QueueLib for QueueLib.Queue; @@ -121,7 +121,7 @@ contract Home is Version0, QueueManager, MerkleTreeManager, SynapseBase { // ============ Constructor ============ - constructor(uint32 _localDomain) SynapseBase(_localDomain) {} // solhint-disable-line no-empty-blocks + constructor(uint32 _localDomain) UpdaterStorage(_localDomain) {} // solhint-disable-line no-empty-blocks // ============ Initializer ============ diff --git a/packages/contracts/contracts/ReplicaManager.sol b/packages/contracts/contracts/ReplicaManager.sol index b299564913..63e841f698 100644 --- a/packages/contracts/contracts/ReplicaManager.sol +++ b/packages/contracts/contracts/ReplicaManager.sol @@ -2,7 +2,7 @@ pragma solidity 0.8.13; // ============ Internal Imports ============ -import { SynapseBase } from "./SynapseBase.sol"; +import { UpdaterStorage } from "./UpdaterStorage.sol"; import { Version0 } from "./Version0.sol"; import { ReplicaLib } from "./libs/Replica.sol"; import { MerkleLib } from "./libs/Merkle.sol"; @@ -16,7 +16,7 @@ import { TypedMemView } from "./libs/TypedMemView.sol"; * @notice Track root updates on Home, * prove and dispatch messages to end recipients. */ -contract ReplicaManager is Version0, SynapseBase { +contract ReplicaManager is Version0, UpdaterStorage { // ============ Libraries ============ using ReplicaLib for ReplicaLib.Replica; @@ -89,7 +89,7 @@ contract ReplicaManager is Version0, SynapseBase { uint32 _localDomain, uint256 _processGas, uint256 _reserveGas - ) SynapseBase(_localDomain) { + ) UpdaterStorage(_localDomain) { require(_processGas >= 850_000, "!process gas"); require(_reserveGas >= 15_000, "!reserve gas"); PROCESS_GAS = _processGas; @@ -178,7 +178,7 @@ contract ReplicaManager is Version0, SynapseBase { * `message`. If the message is successfully proven, then tries to process * message. * @dev Reverts if `prove` call returns false - * @param _message Formatted message (refer to SynapseBase.sol Message library) + * @param _message Formatted message (refer to UpdaterStorage.sol Message library) * @param _proof Merkle proof of inclusion for message's leaf * @param _index Index of leaf in home's merkle tree */ diff --git a/packages/contracts/contracts/SynapseBase.sol b/packages/contracts/contracts/UpdaterStorage.sol similarity index 97% rename from packages/contracts/contracts/SynapseBase.sol rename to packages/contracts/contracts/UpdaterStorage.sol index 3f78c10293..4e21909c74 100644 --- a/packages/contracts/contracts/SynapseBase.sol +++ b/packages/contracts/contracts/UpdaterStorage.sol @@ -11,11 +11,11 @@ import { } from "@openzeppelin/contracts-upgradeable/access/OwnableUpgradeable.sol"; /** - * @title SynapseBase + * @title UpdaterStorage * @author Illusory Systems Inc. * @notice Shared utilities between Home and Replica. */ -abstract contract SynapseBase is Initializable, OwnableUpgradeable { +abstract contract UpdaterStorage is Initializable, OwnableUpgradeable { // ============ Immutable Variables ============ // Domain of chain on which the contract is deployed From 859ba8a9cb13414b72981ce43ca1c634a0625579 Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Thu, 30 Jun 2022 14:45:06 +0300 Subject: [PATCH 10/11] fix: add optimistic period check in SynapseClient --- .../contracts/contracts/client/Client.sol | 21 +++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/packages/contracts/contracts/client/Client.sol b/packages/contracts/contracts/client/Client.sol index c38b776285..2196d25c81 100644 --- a/packages/contracts/contracts/client/Client.sol +++ b/packages/contracts/contracts/client/Client.sol @@ -36,10 +36,19 @@ abstract contract Client is IMessageRecipient { uint32 _origin, uint32 _nonce, bytes32 _sender, + uint256 _rootTimestamp, bytes memory _message ) external { - require(msg.sender == replicaManager, "!replica"); - require(_sender == trustedSender(_origin) && _sender != bytes32(0), "!trustedSender"); + require(msg.sender == replicaManager, "Client: !replica"); + require( + _sender == trustedSender(_origin) && _sender != bytes32(0), + "Client: !trustedSender" + ); + // solhint-disable-next-line do-not-rely-on-time + require( + block.timestamp >= _rootTimestamp + optimisticSeconds(), + "Client: !optimisticSeconds" + ); _handle(_origin, _nonce, _sender, _message); } @@ -60,10 +69,14 @@ abstract contract Client is IMessageRecipient { */ function _send(uint32 _destination, bytes memory _message) internal { bytes32 recipient = trustedSender(_destination); - require(recipient != bytes32(0), "!recipient"); - Home(home).dispatch(_destination, recipient, _message); + require(recipient != bytes32(0), "Client: !recipient"); + Home(home).dispatch(_destination, recipient, optimisticSeconds(), _message); } + /// @dev Period of time since the root was submitted to Replica. Once this period is over, + /// root can be used for proving and executing a message though this Client. + function optimisticSeconds() public view virtual returns (uint32); + /** * @dev Address of the trusted sender on the destination chain. * The trusted sender will be able to: From 93503430ae86f341645555b782cfafd49b34b8fa Mon Sep 17 00:00:00 2001 From: ChiTimesChi <88190723+ChiTimesChi@users.noreply.github.com> Date: Thu, 30 Jun 2022 14:47:28 +0300 Subject: [PATCH 11/11] tests: optimisticSeconds in Client --- packages/contracts/test/HomeGasGolf.t.sol | 3 +- packages/contracts/test/SynapseClient.t.sol | 33 +++++++++++------ .../test/SynapseClientUpgradeable.t.sol | 37 ++++++++++++------- .../test/harnesses/SynapseClientHarness.sol | 4 ++ .../SynapseClientUpgradeableHarness.sol | 4 ++ 5 files changed, 54 insertions(+), 27 deletions(-) diff --git a/packages/contracts/test/HomeGasGolf.t.sol b/packages/contracts/test/HomeGasGolf.t.sol index 8c01a9e198..6b88912bc5 100644 --- a/packages/contracts/test/HomeGasGolf.t.sol +++ b/packages/contracts/test/HomeGasGolf.t.sol @@ -36,6 +36,7 @@ contract HomeGasGolfTest is SynapseTestWithUpdaterManager { nonce, remoteDomain, recipient, + 0, messageBody ); bytes32 messageHash = keccak256(message); @@ -48,7 +49,7 @@ contract HomeGasGolfTest is SynapseTestWithUpdaterManager { message ); vm.prank(sender); - home.dispatch(remoteDomain, recipient, messageBody); + home.dispatch(remoteDomain, recipient, 0, messageBody); newRoot = home.root(); } diff --git a/packages/contracts/test/SynapseClient.t.sol b/packages/contracts/test/SynapseClient.t.sol index 7db7d7819c..e0e54055a2 100644 --- a/packages/contracts/test/SynapseClient.t.sol +++ b/packages/contracts/test/SynapseClient.t.sol @@ -100,7 +100,7 @@ contract SynapseClientTest is SynapseTestWithUpdaterManager { test_setTrustedSender(); vm.prank(replicaManager); - client.handle(remoteDomain, 0, trustedSender, bytes("")); + client.handle(remoteDomain, 0, trustedSender, block.timestamp, bytes("")); } function test_handleNotReplica(address _notReplica) public { @@ -108,28 +108,28 @@ contract SynapseClientTest is SynapseTestWithUpdaterManager { test_setTrustedSender(); vm.prank(_notReplica); - vm.expectRevert("!replica"); - client.handle(remoteDomain, 0, trustedSender, bytes("")); + vm.expectRevert("Client: !replica"); + client.handle(remoteDomain, 0, trustedSender, block.timestamp, bytes("")); } - function test_handleWrongDomain(uint32 _notRemote) public { + function test_handleFakeDomain(uint32 _notRemote) public { vm.assume(_notRemote != remoteDomain); test_setTrustedSender(); vm.prank(replicaManager); - vm.expectRevert("!trustedSender"); - client.handle(_notRemote, 0, trustedSender, bytes("")); + vm.expectRevert("Client: !trustedSender"); + client.handle(_notRemote, 0, trustedSender, block.timestamp, bytes("")); } - function test_handleWrongSender(bytes32 _notSender) public { + function test_handleFakeSender(bytes32 _notSender) public { vm.assume(_notSender != trustedSender); test_setTrustedSender(); vm.prank(replicaManager); - vm.expectRevert("!trustedSender"); - client.handle(remoteDomain, 0, _notSender, bytes("")); + vm.expectRevert("Client: !trustedSender"); + client.handle(remoteDomain, 0, _notSender, block.timestamp, bytes("")); } function test_handleFakeDomainAndSender(uint32 _notRemote) public { @@ -138,10 +138,18 @@ contract SynapseClientTest is SynapseTestWithUpdaterManager { test_setTrustedSender(); vm.prank(replicaManager); - vm.expectRevert("!trustedSender"); + vm.expectRevert("Client: !trustedSender"); // trustedSender for unknown remote is bytes32(0), // but this still has to revert - client.handle(_notRemote, 0, bytes32(0), bytes("")); + client.handle(_notRemote, 0, bytes32(0), block.timestamp, bytes("")); + } + + function test_handleOptimisticSecondsNotPassed() public { + test_setTrustedSender(); + + vm.prank(replicaManager); + vm.expectRevert("Client: !optimisticSeconds"); + client.handle(remoteDomain, 0, trustedSender, block.timestamp + 1, bytes("")); } event Dispatch( @@ -161,6 +169,7 @@ contract SynapseClientTest is SynapseTestWithUpdaterManager { 0, remoteDomain, trustedSender, + 0, messageBody ); vm.expectEmit(true, true, true, true); @@ -170,7 +179,7 @@ contract SynapseClientTest is SynapseTestWithUpdaterManager { function test_sendNoRecipient() public { bytes memory messageBody = hex"01030307"; - vm.expectRevert("!recipient"); + vm.expectRevert("Client: !recipient"); client.send(remoteDomain, messageBody); } } diff --git a/packages/contracts/test/SynapseClientUpgradeable.t.sol b/packages/contracts/test/SynapseClientUpgradeable.t.sol index b499c03a1f..3d1d43e3bb 100644 --- a/packages/contracts/test/SynapseClientUpgradeable.t.sol +++ b/packages/contracts/test/SynapseClientUpgradeable.t.sol @@ -120,7 +120,7 @@ contract SynapseClientTest is SynapseTestWithUpdaterManager { test_setTrustedSender(); vm.prank(replicaManager); - client.handle(remoteDomain, 0, trustedSender, bytes("")); + client.handle(remoteDomain, 0, trustedSender, block.timestamp, bytes("")); } function test_handleNotReplica(address _notReplica) public { @@ -128,18 +128,28 @@ contract SynapseClientTest is SynapseTestWithUpdaterManager { test_setTrustedSender(); vm.prank(_notReplica); - vm.expectRevert("!replica"); - client.handle(remoteDomain, 0, trustedSender, bytes("")); + vm.expectRevert("Client: !replica"); + client.handle(remoteDomain, 0, trustedSender, block.timestamp, bytes("")); } - function test_handleWrongDomain(uint32 _notRemote) public { + function test_handleFakeDomain(uint32 _notRemote) public { vm.assume(_notRemote != remoteDomain); test_setTrustedSender(); vm.prank(replicaManager); - vm.expectRevert("!trustedSender"); - client.handle(_notRemote, 0, trustedSender, bytes("")); + vm.expectRevert("Client: !trustedSender"); + client.handle(_notRemote, 0, trustedSender, block.timestamp, bytes("")); + } + + function test_handleFakeSender(bytes32 _notSender) public { + vm.assume(_notSender != trustedSender); + + test_setTrustedSender(); + + vm.prank(replicaManager); + vm.expectRevert("Client: !trustedSender"); + client.handle(remoteDomain, 0, _notSender, block.timestamp, bytes("")); } function test_handleFakeDomainAndSender(uint32 _notRemote) public { @@ -148,20 +158,18 @@ contract SynapseClientTest is SynapseTestWithUpdaterManager { test_setTrustedSender(); vm.prank(replicaManager); - vm.expectRevert("!trustedSender"); + vm.expectRevert("Client: !trustedSender"); // trustedSender for unknown remote is bytes32(0), // but this still has to revert - client.handle(_notRemote, 0, bytes32(0), bytes("")); + client.handle(_notRemote, 0, bytes32(0), block.timestamp, bytes("")); } - function test_handleWrongSender(bytes32 _notSender) public { - vm.assume(_notSender != trustedSender); - + function test_handleOptimisticSecondsNotPassed() public { test_setTrustedSender(); vm.prank(replicaManager); - vm.expectRevert("!trustedSender"); - client.handle(remoteDomain, 0, _notSender, bytes("")); + vm.expectRevert("Client: !optimisticSeconds"); + client.handle(remoteDomain, 0, trustedSender, block.timestamp + 1, bytes("")); } event Dispatch( @@ -181,6 +189,7 @@ contract SynapseClientTest is SynapseTestWithUpdaterManager { 0, remoteDomain, trustedSender, + 0, messageBody ); vm.expectEmit(true, true, true, true); @@ -190,7 +199,7 @@ contract SynapseClientTest is SynapseTestWithUpdaterManager { function test_sendNoRecipient() public { bytes memory messageBody = hex"01030307"; - vm.expectRevert("!recipient"); + vm.expectRevert("Client: !recipient"); client.send(remoteDomain, messageBody); } } diff --git a/packages/contracts/test/harnesses/SynapseClientHarness.sol b/packages/contracts/test/harnesses/SynapseClientHarness.sol index 0083e6cf6f..85df803491 100644 --- a/packages/contracts/test/harnesses/SynapseClientHarness.sol +++ b/packages/contracts/test/harnesses/SynapseClientHarness.sol @@ -14,6 +14,10 @@ contract SynapseClientHarness is SynapseClient { bytes memory ) internal override {} + function optimisticSeconds() public pure override returns (uint32) { + return 0; + } + function send(uint32 _destination, bytes memory _message) external { _send(_destination, _message); } diff --git a/packages/contracts/test/harnesses/SynapseClientUpgradeableHarness.sol b/packages/contracts/test/harnesses/SynapseClientUpgradeableHarness.sol index 1a318f7a58..1877655e95 100644 --- a/packages/contracts/test/harnesses/SynapseClientUpgradeableHarness.sol +++ b/packages/contracts/test/harnesses/SynapseClientUpgradeableHarness.sol @@ -20,6 +20,10 @@ contract SynapseClientUpgradeableHarness is SynapseClientUpgradeable { bytes memory ) internal override {} + function optimisticSeconds() public pure override returns (uint32) { + return 0; + } + function send(uint32 _destination, bytes memory _message) external { _send(_destination, _message); }