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); + } }