From 6a2afdcffcafa8c902ff4337f657c62cc535bffc Mon Sep 17 00:00:00 2001 From: Asa Oines Date: Tue, 31 May 2022 13:04:29 -0400 Subject: [PATCH] Only cache Outbox checkpoints for fraud proofs (#475) --- package.json | 3 +- solidity/app/contracts/Router.sol | 95 +---- solidity/app/contracts/test/TestRouter.sol | 18 - solidity/app/test/router.test.ts | 48 --- solidity/core/contracts/Common.sol | 43 ++- solidity/core/contracts/Inbox.sol | 42 +- .../{Merkle.sol => MerkleTreeManager.sol} | 7 - solidity/core/contracts/Outbox.sol | 51 +-- solidity/core/contracts/test/TestCommon.sol | 4 + solidity/core/contracts/test/TestInbox.sol | 4 +- solidity/core/contracts/test/TestMerkle.sol | 9 +- solidity/core/contracts/test/TestOutbox.sol | 37 +- .../contracts/test/TestValidatorManager.sol | 4 +- .../InboxValidatorManager.sol | 19 +- .../OutboxValidatorManager.sol | 212 +++++++++- solidity/core/interfaces/ICommon.sol | 4 +- solidity/core/interfaces/IInbox.sol | 2 +- solidity/core/interfaces/IOutbox.sol | 11 +- .../core/test/abacusConnectionManager.test.ts | 17 +- solidity/core/test/common.test.ts | 21 + solidity/core/test/inbox.test.ts | 40 +- solidity/core/test/outbox.test.ts | 27 +- .../inboxValidatorManager.test.ts | 23 +- .../outboxValidatorManager.test.ts | 364 ++++++++++++++++-- typescript/deploy/src/core/deploy.ts | 14 +- typescript/hardhat/src/TestCoreApp.ts | 5 - typescript/hardhat/src/TestCoreDeploy.ts | 30 +- typescript/infra/hardhat.config.ts | 11 +- typescript/sdk/CHANGELOG.md | 2 +- typescript/sdk/src/core/events.ts | 7 +- typescript/sdk/src/core/message.ts | 73 +--- vectors/destinationNonce.json | 27 -- 32 files changed, 727 insertions(+), 547 deletions(-) rename solidity/core/contracts/{Merkle.sol => MerkleTreeManager.sol} (80%) delete mode 100644 vectors/destinationNonce.json diff --git a/package.json b/package.json index 688b721f9..2a7b3434c 100644 --- a/package.json +++ b/package.json @@ -21,7 +21,8 @@ "build": "yarn workspaces foreach --parallel --topological run build", "postinstall": "husky install", "prettier": "yarn workspaces foreach --parallel run prettier", - "lint-ts": "eslint . --ext .ts" + "lint-ts": "eslint . --ext .ts", + "test": "yarn workspaces foreach --parallel --topological run test" }, "workspaces": [ "solidity/*", diff --git a/solidity/app/contracts/Router.sol b/solidity/app/contracts/Router.sol index 2accf0c2e..9a2a2e470 100644 --- a/solidity/app/contracts/Router.sol +++ b/solidity/app/contracts/Router.sol @@ -118,7 +118,7 @@ abstract contract Router is AbacusConnectionClient, IMessageRecipient { /** * @notice Dispatches a message to an enrolled router via the local router's Outbox. - * @notice Does not pay interchain gas or create a checkpoint. + * @notice Does not pay interchain gas. * @dev Reverts if there is no enrolled router for _destinationDomain. * @param _destinationDomain The domain of the chain to which to send the message. * @param _msg The message to dispatch. @@ -130,28 +130,9 @@ abstract contract Router is AbacusConnectionClient, IMessageRecipient { return _dispatch(_outbox(), _destinationDomain, _msg); } - /** - * @notice Dispatches a message to an enrolled router via the local router's Outbox - * and creates a checkpoint. - * @dev Does not pay interchain gas. - * @dev Reverts if there is no enrolled router for _destinationDomain. - * @param _destinationDomain The domain of the chain to which to send the message. - * @param _msg The message to dispatch. - */ - function _dispatchAndCheckpoint( - uint32 _destinationDomain, - bytes memory _msg - ) internal { - // Gets the outbox once to avoid multiple storage reads and calls. - IOutbox _outbox = _outbox(); - _dispatch(_outbox, _destinationDomain, _msg); - _outbox.checkpoint(); - } - /** * @notice Dispatches a message to an enrolled router via the local router's Outbox * and pays interchain gas for the dispatched message. - * @dev Does not create a checkpoint on the Outbox. * @dev Reverts if there is no enrolled router for _destinationDomain. * @param _destinationDomain The domain of the chain to which to send the message. * @param _msg The message to dispatch. @@ -165,58 +146,23 @@ abstract contract Router is AbacusConnectionClient, IMessageRecipient { ) internal { // Gets the abacusConnectionManager from storage once to avoid multiple reads. IAbacusConnectionManager _abacusConnectionManager = abacusConnectionManager; - _dispatchWithGas( + uint256 _leafIndex = _dispatch( _abacusConnectionManager.outbox(), - _abacusConnectionManager.interchainGasPaymaster(), - _destinationDomain, - _msg, - _gasPayment - ); - } - - /** - * @notice Dispatches a message to an enrolled router via the local router's Outbox, - * pays interchain gas for the dispatched message, and creates a checkpoint. - * @dev Reverts if there is no enrolled router for _destinationDomain. - * @param _destinationDomain The domain of the chain to which to send the message. - * @param _msg The message to dispatch. - * @param _gasPayment The amount of native tokens to pay the Interchain Gas - * Paymaster to process the dispatched message. - */ - function _dispatchWithGasAndCheckpoint( - uint32 _destinationDomain, - bytes memory _msg, - uint256 _gasPayment - ) internal { - // Gets the abacusConnectionManager and outbox once to avoid multiple storage reads - // and calls. - IAbacusConnectionManager _abacusConnectionManager = abacusConnectionManager; - IOutbox _outbox = _abacusConnectionManager.outbox(); - _dispatchWithGas( - _outbox, - _abacusConnectionManager.interchainGasPaymaster(), _destinationDomain, - _msg, - _gasPayment + _msg ); - _outbox.checkpoint(); - } - - /** - * @notice Creates a checkpoint on the local router's Outbox. - * @dev If dispatching a single message and immediately checkpointing, - * `_dispatchAndCheckpoint` or `_dispatchWithGasAndCheckpoint` should be preferred, - * as they will consume less gas than calling `_dispatch` and this function. - */ - function _checkpoint() internal { - _outbox().checkpoint(); + if (_gasPayment > 0) { + _abacusConnectionManager.interchainGasPaymaster().payGasFor{ + value: _gasPayment + }(_leafIndex); + } } // ============ Private functions ============ /** * @notice Dispatches a message to an enrolled router via the provided Outbox. - * @dev Does not pay interchain gas or create a checkpoint. + * @dev Does not pay interchain gas. * @dev Reverts if there is no enrolled router for _destinationDomain. * @param _outbox The outbox contract to dispatch the message through. * @param _destinationDomain The domain of the chain to which to send the message. @@ -231,27 +177,4 @@ abstract contract Router is AbacusConnectionClient, IMessageRecipient { bytes32 _router = _mustHaveRemoteRouter(_destinationDomain); return _outbox.dispatch(_destinationDomain, _router, _msg); } - - /** - * @notice Dispatches a message to an enrolled router via the provided Outbox - * and pays interchain gas for the dispatched message via the provided InterchainGasPaymaster. - * @dev Does not create a checkpoint. - * @dev Reverts if there is no enrolled router for _destinationDomain. - * @param _outbox The outbox contract to dispatch the message through. - * @param _interchainGasPaymaster The InterchainGasPaymaster contract to pay for interchain gas. - * @param _destinationDomain The domain of the chain to which to send the message. - * @param _msg The message to dispatch. - */ - function _dispatchWithGas( - IOutbox _outbox, - IInterchainGasPaymaster _interchainGasPaymaster, - uint32 _destinationDomain, - bytes memory _msg, - uint256 _gasPayment - ) private { - uint256 _leafIndex = _dispatch(_outbox, _destinationDomain, _msg); - if (_gasPayment > 0) { - _interchainGasPaymaster.payGasFor{value: _gasPayment}(_leafIndex); - } - } } diff --git a/solidity/app/contracts/test/TestRouter.sol b/solidity/app/contracts/test/TestRouter.sol index 2239a5dae..b20d5f05e 100644 --- a/solidity/app/contracts/test/TestRouter.sol +++ b/solidity/app/contracts/test/TestRouter.sol @@ -41,22 +41,4 @@ contract TestRouter is Router { ) external payable { _dispatchWithGas(_destination, _msg, _gasPayment); } - - function dispatchAndCheckpoint(uint32 _destination, bytes memory _msg) - external - { - _dispatchAndCheckpoint(_destination, _msg); - } - - function dispatchWithGasAndCheckpoint( - uint32 _destination, - bytes memory _msg, - uint256 _gasPayment - ) external payable { - _dispatchWithGasAndCheckpoint(_destination, _msg, _gasPayment); - } - - function checkpoint() external { - _checkpoint(); - } } diff --git a/solidity/app/test/router.test.ts b/solidity/app/test/router.test.ts index 832f91cf0..a9ddcd6ed 100644 --- a/solidity/app/test/router.test.ts +++ b/solidity/app/test/router.test.ts @@ -126,7 +126,6 @@ describe('Router', async () => { destinationDomain: number, interchainGasPayment?: number, ) => Promise, - expectCheckpoint: boolean, expectGasPayment: boolean, ) => { // Allows a Chai Assertion to be programmatically negated @@ -155,16 +154,6 @@ describe('Router', async () => { .withArgs(leafIndex, testInterchainGasPayment); }); - it(`${ - expectCheckpoint ? 'creates' : 'does not create' - } a checkpoint`, async () => { - const assertion = expectAssertion( - expect(dispatchFunction(destination)).to, - expectCheckpoint, - ); - await assertion.emit(outbox, 'Checkpoint'); - }); - it('reverts when dispatching a message to an unenrolled remote router', async () => { await expect( dispatchFunction(destinationWithoutRouter), @@ -176,16 +165,6 @@ describe('Router', async () => { runDispatchFunctionTests( (destinationDomain) => router.dispatch(destinationDomain, '0x'), false, - false, - ); - }); - - describe('#dispatchAndCheckpoint', () => { - runDispatchFunctionTests( - (destinationDomain) => - router.dispatchAndCheckpoint(destinationDomain, '0x'), - true, - false, ); }); @@ -200,35 +179,8 @@ describe('Router', async () => { value: interchainGasPayment, }, ), - false, true, ); }); - - describe('#dispatchWithGasAndCheckpoint', () => { - runDispatchFunctionTests( - (destinationDomain, interchainGasPayment = 0) => - router.dispatchWithGasAndCheckpoint( - destinationDomain, - '0x', - interchainGasPayment, - { value: interchainGasPayment }, - ), - true, - true, - ); - }); - }); - - describe('#checkpoint', () => { - it('creates a checkpoint', async () => { - // dispatch dummy message - await outbox.dispatch( - destination, - utils.addressToBytes32(outbox.address), - '0x', - ); - await expect(router.checkpoint()).to.emit(outbox, 'Checkpoint'); - }); }); }); diff --git a/solidity/core/contracts/Common.sol b/solidity/core/contracts/Common.sol index 7a7fdbee4..c044ad8ee 100644 --- a/solidity/core/contracts/Common.sol +++ b/solidity/core/contracts/Common.sol @@ -20,12 +20,12 @@ abstract contract Common is ICommon, OwnableUpgradeable { // ============ Public Variables ============ - // Checkpoints of root => leaf index - // Checkpoints of index 0 have to be disallowed as the existence of such - // a checkpoint cannot be distinguished from their non-existence - mapping(bytes32 => uint256) public checkpoints; - // The latest checkpointed root - bytes32 public checkpointedRoot; + // Cached checkpoints, mapping root => leaf index. + // Cached checkpoints must have index > 0 as the presence of such + // a checkpoint cannot be distinguished from its absence. + mapping(bytes32 => uint256) public cachedCheckpoints; + // The latest cached root + bytes32 public latestCachedRoot; // Address of the validator manager contract. address public validatorManager; @@ -37,12 +37,11 @@ abstract contract Common is ICommon, OwnableUpgradeable { // ============ Events ============ /** - * @notice Emitted when a root is checkpointed on Outbox or a signed - * checkpoint is relayed to a Inbox. + * @notice Emitted when a checkpoint is cached. * @param root Merkle root * @param index Leaf index */ - event Checkpoint(bytes32 indexed root, uint256 indexed index); + event CheckpointCached(bytes32 indexed root, uint256 indexed index); /** * @notice Emitted when the validator manager contract is changed @@ -91,18 +90,18 @@ abstract contract Common is ICommon, OwnableUpgradeable { } /** - * @notice Returns the latest checkpoint for the Validators to sign. - * @return root Latest checkpointed root - * @return index Latest checkpointed index + * @notice Returns the latest entry in the checkpoint cache. + * @return root Latest cached root + * @return index Latest cached index */ - function latestCheckpoint() + function latestCachedCheckpoint() external view override returns (bytes32 root, uint256 index) { - root = checkpointedRoot; - index = checkpoints[root]; + root = latestCachedRoot; + index = cachedCheckpoints[root]; } // ============ Internal Functions ============ @@ -121,13 +120,15 @@ abstract contract Common is ICommon, OwnableUpgradeable { } /** - * @notice Store the provided checkpoint. - * @param _root The merkle root + * @notice Caches the provided checkpoint. + * Caching checkpoints with index == 0 are disallowed. + * @param _root The merkle root to cache. * @param _index The leaf index of the latest message in the merkle tree. */ - function _checkpoint(bytes32 _root, uint256 _index) internal { - checkpoints[_root] = _index; - checkpointedRoot = _root; - emit Checkpoint(_root, _index); + function _cacheCheckpoint(bytes32 _root, uint256 _index) internal { + require(_index > 0, "!index"); + cachedCheckpoints[_root] = _index; + latestCachedRoot = _root; + emit CheckpointCached(_root, _index); } } diff --git a/solidity/core/contracts/Inbox.sol b/solidity/core/contracts/Inbox.sol index a9b77591d..32e52b4c9 100644 --- a/solidity/core/contracts/Inbox.sol +++ b/solidity/core/contracts/Inbox.sol @@ -51,9 +51,17 @@ contract Inbox is IInbox, Version0, Common { /** * @notice Emitted when message is processed - * @param messageHash Hash of message that failed to process + * @dev This event allows watchers to observe the merkle proof they need + * to prove fraud on the Outbox. + * @param messageHash Hash of message that was processed. + * @param leafIndex The leaf index of the message that was processed. + * @param proof A merkle proof of inclusion of `messageHash` at `leafIndex`. */ - event Process(bytes32 indexed messageHash); + event Process( + bytes32 indexed messageHash, + uint256 indexed leafIndex, + bytes32[32] proof + ); // ============ Constructor ============ @@ -62,36 +70,33 @@ contract Inbox is IInbox, Version0, Common { // ============ Initializer ============ - function initialize( - uint32 _remoteDomain, - address _validatorManager, - bytes32 _checkpointedRoot, - uint256 _checkpointedIndex - ) public initializer { + function initialize(uint32 _remoteDomain, address _validatorManager) + public + initializer + { __Common_initialize(_validatorManager); entered = 1; remoteDomain = _remoteDomain; - _checkpoint(_checkpointedRoot, _checkpointedIndex); } // ============ External Functions ============ /** - * @notice Checkpoints the provided root and index. + * @notice Caches the provided merkle root and index. * @dev Called by the validator manager, which is responsible for verifying a * quorum of validator signatures on the checkpoint. - * @dev Reverts if checkpoints's index is not greater than our latest index. + * @dev Reverts if the checkpoint's index is not greater than the index of the latest checkpoint in the cache. * @param _root Checkpoint's merkle root. * @param _index Checkpoint's index. */ - function checkpoint(bytes32 _root, uint256 _index) + function cacheCheckpoint(bytes32 _root, uint256 _index) external override onlyValidatorManager { - // Ensure that the checkpoint is more recent than the latest we've seen. - require(_index > checkpoints[checkpointedRoot], "old checkpoint"); - _checkpoint(_root, _index); + // Ensure that the checkpoint is newer than the latest we've cached. + require(_index > cachedCheckpoints[latestCachedRoot], "!newer"); + _cacheCheckpoint(_root, _index); } /** @@ -126,9 +131,10 @@ contract Inbox is IInbox, Version0, Common { _proof, _index ); - // ensure that the root has been checkpointed - require(checkpoints[_calculatedRoot] > 0, "!checkpointed root"); + // ensure that the root has been cached + require(cachedCheckpoints[_calculatedRoot] >= _index, "!cache"); _process(_message, _messageHash); + emit Process(_messageHash, _index, _proof); // reset re-entrancy guard entered = 1; } @@ -161,7 +167,5 @@ contract Inbox is IInbox, Version0, Common { sender, body ); - // emit process results - emit Process(_messageHash); } } diff --git a/solidity/core/contracts/Merkle.sol b/solidity/core/contracts/MerkleTreeManager.sol similarity index 80% rename from solidity/core/contracts/Merkle.sol rename to solidity/core/contracts/MerkleTreeManager.sol index c1bce6ca7..f0a31f420 100644 --- a/solidity/core/contracts/Merkle.sol +++ b/solidity/core/contracts/MerkleTreeManager.sol @@ -29,11 +29,4 @@ contract MerkleTreeManager { function root() public view returns (bytes32) { return tree.root(); } - - /** - * @notice Returns the number of inserted leaves in the tree (current index) - */ - function count() public view returns (uint256) { - return tree.count; - } } diff --git a/solidity/core/contracts/Outbox.sol b/solidity/core/contracts/Outbox.sol index fb267db12..e75a78665 100644 --- a/solidity/core/contracts/Outbox.sol +++ b/solidity/core/contracts/Outbox.sol @@ -7,7 +7,7 @@ import {Common} from "./Common.sol"; import {MerkleLib} from "../libs/Merkle.sol"; import {Message} from "../libs/Message.sol"; import {TypeCasts} from "../libs/TypeCasts.sol"; -import {MerkleTreeManager} from "./Merkle.sol"; +import {MerkleTreeManager} from "./MerkleTreeManager.sol"; import {IOutbox} from "../interfaces/IOutbox.sol"; /** @@ -133,18 +133,12 @@ contract Outbox is IOutbox, Version0, MerkleTreeManager, Common { } /** - * @notice Checkpoints the latest root and index. - * Validators are expected to sign this checkpoint so that it can be - * relayed to the Inbox contracts. Checkpoints for a single message (i.e. - * count = 1) are disallowed since they make checkpoint tracking more - * difficult. + * @notice Caches the current merkle root and index. * @dev emits Checkpoint event */ - function checkpoint() external override notFailed { - uint256 count = count(); - require(count > 1, "!count"); - bytes32 root = root(); - _checkpoint(root, count - 1); + function cacheCheckpoint() external override notFailed { + (bytes32 root, uint256 index) = latestCheckpoint(); + _cacheCheckpoint(root, index); } /** @@ -158,37 +152,18 @@ contract Outbox is IOutbox, Version0, MerkleTreeManager, Common { } /** - * @notice Returns whether the provided root and index are a known - * checkpoint. - * @param _root The merkle root. - * @param _index The index. - * @return TRUE iff `_root` and `_index` are a known checkpoint. + * @notice Returns the number of inserted leaves in the tree */ - function isCheckpoint(bytes32 _root, uint256 _index) - external - view - override - returns (bool) - { - // Checkpoints are zero-indexed, but checkpoints of index 0 are disallowed - return _index > 0 && checkpoints[_root] == _index; + function count() public view returns (uint256) { + return tree.count; } - // ============ Internal Functions ============ - /** - * @notice Internal utility function that combines - * `_destination` and `_nonce`. - * @dev Both destination and nonce should be less than 2^32 - 1 - * @param _destination Domain of destination chain - * @param _nonce Current nonce for given destination chain - * @return Returns (`_destination` << 32) & `_nonce` + * @notice Returns a checkpoint representing the current merkle tree. + * @return root The root of the Outbox's merkle tree. + * @return index The index of the last element in the tree. */ - function _destinationAndNonce(uint32 _destination, uint32 _nonce) - internal - pure - returns (uint64) - { - return (uint64(_destination) << 32) | _nonce; + function latestCheckpoint() public view returns (bytes32, uint256) { + return (root(), count() - 1); } } diff --git a/solidity/core/contracts/test/TestCommon.sol b/solidity/core/contracts/test/TestCommon.sol index d52a3888b..cb2d65492 100644 --- a/solidity/core/contracts/test/TestCommon.sol +++ b/solidity/core/contracts/test/TestCommon.sol @@ -9,4 +9,8 @@ contract TestCommon is Common { function initialize(address _validatorManager) external initializer { __Common_initialize(_validatorManager); } + + function cacheCheckpoint(bytes32 _root, uint256 _index) external { + _cacheCheckpoint(_root, _index); + } } diff --git a/solidity/core/contracts/test/TestInbox.sol b/solidity/core/contracts/test/TestInbox.sol index dead5db28..950716f44 100644 --- a/solidity/core/contracts/test/TestInbox.sol +++ b/solidity/core/contracts/test/TestInbox.sol @@ -8,8 +8,8 @@ contract TestInbox is Inbox { constructor(uint32 _localDomain) Inbox(_localDomain) {} // solhint-disable-line no-empty-blocks - function setCheckpoint(bytes32 _root, uint256 _index) external { - checkpoints[_root] = _index; + function setCachedCheckpoint(bytes32 _root, uint256 _index) external { + cachedCheckpoints[_root] = _index; } function testBranchRoot( diff --git a/solidity/core/contracts/test/TestMerkle.sol b/solidity/core/contracts/test/TestMerkle.sol index e50c99768..e3c87e64d 100644 --- a/solidity/core/contracts/test/TestMerkle.sol +++ b/solidity/core/contracts/test/TestMerkle.sol @@ -1,7 +1,7 @@ // SPDX-License-Identifier: MIT OR Apache-2.0 pragma solidity >=0.8.0; -import "../Merkle.sol"; +import "../MerkleTreeManager.sol"; contract TestMerkle is MerkleTreeManager { using MerkleLib for MerkleLib.Tree; @@ -20,4 +20,11 @@ contract TestMerkle is MerkleTreeManager { ) external pure returns (bytes32 _node) { return MerkleLib.branchRoot(_leaf, _proof, _index); } + + /** + * @notice Returns the number of inserted leaves in the tree + */ + function count() public view returns (uint256) { + return tree.count; + } } diff --git a/solidity/core/contracts/test/TestOutbox.sol b/solidity/core/contracts/test/TestOutbox.sol index 0c7cc1ac9..457321b5d 100644 --- a/solidity/core/contracts/test/TestOutbox.sol +++ b/solidity/core/contracts/test/TestOutbox.sol @@ -3,18 +3,11 @@ pragma solidity >=0.8.0; // ============ Internal Imports ============ import "../Outbox.sol"; +import {MerkleLib} from "../../libs/Merkle.sol"; contract TestOutbox is Outbox { constructor(uint32 _localDomain) Outbox(_localDomain) {} // solhint-disable-line no-empty-blocks - function destinationAndNonce(uint32 _destination, uint32 _nonce) - external - pure - returns (uint64) - { - return _destinationAndNonce(_destination, _nonce); - } - /** * @notice Set the validator manager * @param _validatorManager Address of the validator manager @@ -22,4 +15,32 @@ contract TestOutbox is Outbox { function testSetValidatorManager(address _validatorManager) external { validatorManager = _validatorManager; } + + function proof() external view returns (bytes32[32] memory) { + bytes32[32] memory _zeroes = MerkleLib.zeroHashes(); + uint256 _index = tree.count - 1; + bytes32[32] memory _proof; + + for (uint256 i = 0; i < 32; i++) { + uint256 _ithBit = (_index >> i) & 0x01; + if (_ithBit == 1) { + _proof[i] = tree.branch[i]; + } else { + _proof[i] = _zeroes[i]; + } + } + return _proof; + } + + function branch() external view returns (bytes32[32] memory) { + return tree.branch; + } + + function branchRoot( + bytes32 _item, + bytes32[32] memory _branch, + uint256 _index + ) external pure returns (bytes32) { + return MerkleLib.branchRoot(_item, _branch, _index); + } } diff --git a/solidity/core/contracts/test/TestValidatorManager.sol b/solidity/core/contracts/test/TestValidatorManager.sol index a32b12b71..8810173ac 100644 --- a/solidity/core/contracts/test/TestValidatorManager.sol +++ b/solidity/core/contracts/test/TestValidatorManager.sol @@ -8,11 +8,11 @@ import {IInbox} from "../../interfaces/IInbox.sol"; * to be a contract. */ contract TestValidatorManager { - function checkpoint( + function cacheCheckpoint( IInbox _inbox, bytes32 _root, uint256 _index ) external { - _inbox.checkpoint(_root, _index); + _inbox.cacheCheckpoint(_root, _index); } } diff --git a/solidity/core/contracts/validator-manager/InboxValidatorManager.sol b/solidity/core/contracts/validator-manager/InboxValidatorManager.sol index 270063714..672d2d8d4 100644 --- a/solidity/core/contracts/validator-manager/InboxValidatorManager.sol +++ b/solidity/core/contracts/validator-manager/InboxValidatorManager.sol @@ -12,6 +12,18 @@ import {MultisigValidatorManager} from "./MultisigValidatorManager.sol"; * them to an Inbox. */ contract InboxValidatorManager is MultisigValidatorManager { + // ============ Events ============ + + /** + * @notice Emitted when a checkpoint has been signed by a quorum + * of validators and cached on an Inbox. + * @dev This event allows watchers to observe the signatures they need + * to prove fraud on the Outbox. + * @param signatures The signatures by a quorum of validators on the + * checkpoint. + */ + event Quorum(bytes[] signatures); + // ============ Constructor ============ /** @@ -31,7 +43,7 @@ contract InboxValidatorManager is MultisigValidatorManager { // ============ External Functions ============ /** - * @notice Submits a checkpoint signed by a quorum of validators to an Inbox. + * @notice Submits a checkpoint signed by a quorum of validators to be cached by an Inbox. * @dev Reverts if `_signatures` is not a quorum of validator signatures. * @dev Reverts if `_signatures` is not sorted in ascending order by the signer * address, which is required for duplicate detection. @@ -41,13 +53,14 @@ contract InboxValidatorManager is MultisigValidatorManager { * @param _signatures Signatures over the checkpoint to be checked for a validator * quorum. Must be sorted in ascending order by signer address. */ - function checkpoint( + function cacheCheckpoint( IInbox _inbox, bytes32 _root, uint256 _index, bytes[] calldata _signatures ) external { require(isQuorum(_root, _index, _signatures), "!quorum"); - _inbox.checkpoint(_root, _index); + emit Quorum(_signatures); + _inbox.cacheCheckpoint(_root, _index); } } diff --git a/solidity/core/contracts/validator-manager/OutboxValidatorManager.sol b/solidity/core/contracts/validator-manager/OutboxValidatorManager.sol index a4068a59b..12db8166e 100644 --- a/solidity/core/contracts/validator-manager/OutboxValidatorManager.sol +++ b/solidity/core/contracts/validator-manager/OutboxValidatorManager.sol @@ -4,30 +4,59 @@ pragma abicoder v2; // ============ Internal Imports ============ import {IOutbox} from "../../interfaces/IOutbox.sol"; +import {MerkleLib} from "../../libs/Merkle.sol"; import {MultisigValidatorManager} from "./MultisigValidatorManager.sol"; /** * @title OutboxValidatorManager - * @notice Verifies if an improper checkpoint has been signed by a quorum of + * @notice Verifies if an premature or fraudulent checkpoint has been signed by a quorum of * validators and reports it to an Outbox. */ contract OutboxValidatorManager is MultisigValidatorManager { // ============ Events ============ /** - * @notice Emitted when proof of an improper checkpoint is submitted. + * @notice Emitted when a checkpoint is proven premature. * @dev Observers of this event should filter by the outbox address. * @param outbox The outbox. - * @param root Root of the improper checkpoint. - * @param index Index of the improper checkpoint. - * @param signatures A quorum of signatures on the improper checkpoint. + * @param signedRoot Root of the premature checkpoint. + * @param signedIndex Index of the premature checkpoint. + * @param signatures A quorum of signatures on the premature checkpoint. * May include non-validator signatures. + * @param count The number of messages in the Outbox. */ - event ImproperCheckpoint( + event PrematureCheckpoint( address indexed outbox, - bytes32 root, - uint256 index, - bytes[] signatures + bytes32 signedRoot, + uint256 signedIndex, + bytes[] signatures, + uint256 count + ); + + /** + * @notice Emitted when a checkpoint is proven fraudulent. + * @dev Observers of this event should filter by the outbox address. + * @param outbox The outbox. + * @param signedRoot Root of the fraudulent checkpoint. + * @param signedIndex Index of the fraudulent checkpoint. + * @param signatures A quorum of signatures on the fraudulent checkpoint. + * May include non-validator signatures. + * @param fraudulentLeaf The leaf in the fraudulent tree. + * @param fraudulentProof Proof of inclusion of fraudulentLeaf. + * @param actualLeaf The leaf in the Outbox's tree. + * @param actualProof Proof of inclusion of actualLeaf. + * @param leafIndex The index of the leaves that are being proved. + */ + event FraudulentCheckpoint( + address indexed outbox, + bytes32 signedRoot, + uint256 signedIndex, + bytes[] signatures, + bytes32 fraudulentLeaf, + bytes32[32] fraudulentProof, + bytes32 actualLeaf, + bytes32[32] actualProof, + uint256 leafIndex ); // ============ Constructor ============ @@ -49,25 +78,168 @@ contract OutboxValidatorManager is MultisigValidatorManager { // ============ External Functions ============ /** - * @notice Determines if a quorum of validators have signed an improper checkpoint, + * @notice Determines if a quorum of validators have signed a premature checkpoint, * failing the Outbox if so. - * @dev Improper checkpoints signed by individual validators are not handled to prevent + * A checkpoint is premature if it commits to more messages than are present in the + * Outbox's merkle tree. + * @dev Premature checkpoints signed by individual validators are not handled to prevent * a single byzantine validator from failing the Outbox. * @param _outbox The outbox. - * @param _root The merkle root of the checkpoint. - * @param _index The index of the checkpoint. + * @param _signedRoot The root of the signed checkpoint. + * @param _signedIndex The index of the signed checkpoint. * @param _signatures Signatures over the checkpoint to be checked for a validator * quorum. Must be sorted in ascending order by signer address. + * @return True iff prematurity was proved. */ - function improperCheckpoint( + function prematureCheckpoint( IOutbox _outbox, - bytes32 _root, - uint256 _index, + bytes32 _signedRoot, + uint256 _signedIndex, bytes[] calldata _signatures - ) external { - require(isQuorum(_root, _index, _signatures), "!quorum"); - require(!_outbox.isCheckpoint(_root, _index), "!improper checkpoint"); + ) external returns (bool) { + require(isQuorum(_signedRoot, _signedIndex, _signatures), "!quorum"); + // Checkpoints are premature if the checkpoint commits to more messages + // than the Outbox has in its merkle tree. + uint256 count = _outbox.count(); + require(_signedIndex >= count, "!premature"); + _outbox.fail(); + emit PrematureCheckpoint( + address(_outbox), + _signedRoot, + _signedIndex, + _signatures, + count + ); + return true; + } + + /** + * @notice Determines if a quorum of validators have signed a fraudulent checkpoint, + * failing the Outbox if so. + * A checkpoint is fraudulent if the leaf it commits to at index I differs + * from the leaf the Outbox committed to at index I, where I is less than or equal + * to the index of the checkpoint. + * This difference can be proved by comparing two merkle proofs for leaf + * index J >= I. One against the fraudulent checkpoint, and one against a + * checkpoint cached on the Outbox. + * @dev Fraudulent checkpoints signed by individual validators are not handled to prevent + * a single byzantine validator from failing the Outbox. + * @param _outbox The outbox. + * @param _signedRoot The root of the signed checkpoint. + * @param _signedIndex The index of the signed checkpoint. + * @param _signatures Signatures over the checkpoint to be checked for a validator + * quorum. Must be sorted in ascending order by signer address. + * @param _fraudulentLeaf The leaf in the fraudulent tree. + * @param _fraudulentProof Proof of inclusion of `_fraudulentLeaf`. + * @param _actualLeaf The leaf in the Outbox's tree. + * @param _actualProof Proof of inclusion of `_actualLeaf`. + * @param _leafIndex The index of the leaves that are being proved. + * @return True iff fraud was proved. + */ + function fraudulentCheckpoint( + IOutbox _outbox, + bytes32 _signedRoot, + uint256 _signedIndex, + bytes[] calldata _signatures, + bytes32 _fraudulentLeaf, + bytes32[32] calldata _fraudulentProof, + bytes32 _actualLeaf, + bytes32[32] calldata _actualProof, + uint256 _leafIndex + ) external returns (bool) { + // Check the signed checkpoint commits to _fraudulentLeaf at _leafIndex. + require(isQuorum(_signedRoot, _signedIndex, _signatures), "!quorum"); + bytes32 _fraudulentRoot = MerkleLib.branchRoot( + _fraudulentLeaf, + _fraudulentProof, + _leafIndex + ); + require(_fraudulentRoot == _signedRoot, "!root"); + require(_signedIndex >= _leafIndex, "!index"); + + // Check the cached checkpoint commits to _actualLeaf at _leafIndex. + bytes32 _cachedRoot = MerkleLib.branchRoot( + _actualLeaf, + _actualProof, + _leafIndex + ); + uint256 _cachedIndex = _outbox.cachedCheckpoints(_cachedRoot); + require(_cachedIndex > 0 && _cachedIndex >= _leafIndex, "!cache"); + + // Check that the two roots commit to at least one differing leaf + // with index <= _leafIndex. + require( + impliesDifferingLeaf( + _fraudulentLeaf, + _fraudulentProof, + _actualLeaf, + _actualProof, + _leafIndex + ), + "!fraud" + ); + + // Fail the Outbox. _outbox.fail(); - emit ImproperCheckpoint(address(_outbox), _root, _index, _signatures); + emit FraudulentCheckpoint( + address(_outbox), + _signedRoot, + _signedIndex, + _signatures, + _fraudulentLeaf, + _fraudulentProof, + _actualLeaf, + _actualProof, + _leafIndex + ); + return true; + } + + /** + * @notice Returns true if the implied merkle roots commit to at least one + * differing leaf with index <= `_leafIndex`. + * Given a merkle proof for leaf index J, we can determine whether an + * element in the proof is an internal node whose terminal children are leaves + * with index <= J. + * Given two merkle proofs for leaf index J, if such elements do not match, + * these two proofs necessarily commit to at least one differing leaf with + * index I <= J. + * @param _leafA The leaf in tree A. + * @param _proofA Proof of inclusion of `_leafA` in tree A. + * @param _leafB The leaf in tree B. + * @param _proofB Proof of inclusion of `_leafB` in tree B. + * @param _leafIndex The index of `_leafA` and `_leafB`. + * @return differ True if the implied trees differ, false if not. + */ + function impliesDifferingLeaf( + bytes32 _leafA, + bytes32[32] calldata _proofA, + bytes32 _leafB, + bytes32[32] calldata _proofB, + uint256 _leafIndex + ) public pure returns (bool) { + // The implied merkle roots commit to at least one differing leaf + // with index <= _leafIndex, if either: + + // 1. If the provided leaves differ. + if (_leafA != _leafB) { + return true; + } + + // 2. If the branches contain internal nodes whose subtrees are full + // (as implied by _leafIndex) that differ from one another. + for (uint8 i = 0; i < 32; i++) { + uint256 _ithBit = (_leafIndex >> i) & 0x01; + // If the i'th is 1, the i'th element in the proof is an internal + // node whose subtree is full. + // If these nodes differ, at least one leaf that they commit to + // must differ as well. + if (_ithBit == 1) { + if (_proofA[i] != _proofB[i]) { + return true; + } + } + } + return false; } } diff --git a/solidity/core/interfaces/ICommon.sol b/solidity/core/interfaces/ICommon.sol index f2840ccc9..fb9db357a 100644 --- a/solidity/core/interfaces/ICommon.sol +++ b/solidity/core/interfaces/ICommon.sol @@ -3,8 +3,8 @@ pragma solidity >=0.6.11; interface ICommon { function localDomain() external view returns (uint32); - - function latestCheckpoint() + function cachedCheckpoints(bytes32) external view returns (uint256); + function latestCachedCheckpoint() external view returns (bytes32 root, uint256 index); diff --git a/solidity/core/interfaces/IInbox.sol b/solidity/core/interfaces/IInbox.sol index 6193caa12..b85a3b1da 100644 --- a/solidity/core/interfaces/IInbox.sol +++ b/solidity/core/interfaces/IInbox.sol @@ -4,7 +4,7 @@ pragma solidity >=0.6.11; import {ICommon} from "./ICommon.sol"; interface IInbox is ICommon { - function checkpoint( + function cacheCheckpoint( bytes32 _root, uint256 _index ) external; diff --git a/solidity/core/interfaces/IOutbox.sol b/solidity/core/interfaces/IOutbox.sol index 73ec618bb..a69ebb626 100644 --- a/solidity/core/interfaces/IOutbox.sol +++ b/solidity/core/interfaces/IOutbox.sol @@ -9,13 +9,8 @@ interface IOutbox is ICommon { bytes32 _recipientAddress, bytes calldata _messageBody ) external returns (uint256); - - function checkpoint() external; - - function isCheckpoint( - bytes32 _root, - uint256 _index - ) external returns (bool); - + function cacheCheckpoint() external; + function latestCheckpoint() external view returns (bytes32, uint256); + function count() external returns (uint256); function fail() external; } diff --git a/solidity/core/test/abacusConnectionManager.test.ts b/solidity/core/test/abacusConnectionManager.test.ts index bcd4a7f6e..0ea0e4e58 100644 --- a/solidity/core/test/abacusConnectionManager.test.ts +++ b/solidity/core/test/abacusConnectionManager.test.ts @@ -8,10 +8,10 @@ import { AbacusConnectionManager__factory, InterchainGasPaymaster, InterchainGasPaymaster__factory, + Outbox, + Outbox__factory, TestInbox, TestInbox__factory, - TestOutbox, - TestOutbox__factory, } from '../types'; const ONLY_OWNER_REVERT_MSG = 'Ownable: caller is not the owner'; @@ -29,19 +29,14 @@ describe('AbacusConnectionManager', async () => { }); beforeEach(async () => { - const outboxFactory = new TestOutbox__factory(signer); + const outboxFactory = new Outbox__factory(signer); const outbox = await outboxFactory.deploy(localDomain); const inboxFactory = new TestInbox__factory(signer); enrolledInbox = await inboxFactory.deploy(localDomain); // The ValidatorManager is unused in these tests *but* needs to be a // contract. - await enrolledInbox.initialize( - remoteDomain, - outbox.address, - ethers.constants.HashZero, - 0, - ); + await enrolledInbox.initialize(remoteDomain, outbox.address); const connectionManagerFactory = new AbacusConnectionManager__factory( signer, @@ -63,10 +58,10 @@ describe('AbacusConnectionManager', async () => { signer?: SignerWithAddress, ) => Promise, ) => { - let newOutbox: TestOutbox; + let newOutbox: Outbox; beforeEach(async () => { - const outboxFactory = new TestOutbox__factory(signer); + const outboxFactory = new Outbox__factory(signer); newOutbox = await outboxFactory.deploy(localDomain); }); diff --git a/solidity/core/test/common.test.ts b/solidity/core/test/common.test.ts index 8bf8e0881..4cc9d1655 100644 --- a/solidity/core/test/common.test.ts +++ b/solidity/core/test/common.test.ts @@ -43,4 +43,25 @@ describe('Common', async () => { common.connect(nonowner).setValidatorManager(common.address), ).to.be.revertedWith(ONLY_OWNER_REVERT_MSG); }); + + it('Caches a checkpoint', async () => { + const root = + '0x9c7a007113f829cfd019a91e4ca5e7f6760589fd6bc7925c877f6971ffee1647'; + const index = 1; + await common.cacheCheckpoint(root, index); + expect(await common.latestCachedRoot()).to.equal(root); + expect(await common.cachedCheckpoints(root)).to.equal(index); + const [actualRoot, actualIndex] = await common.latestCachedCheckpoint(); + expect(actualRoot).to.equal(root); + expect(actualIndex).to.equal(index); + }); + + it('Reverts when caching a checkpoint with index zero', async () => { + const root = + '0x9c7a007113f829cfd019a91e4ca5e7f6760589fd6bc7925c877f6971ffee1647'; + const index = 0; + await expect(common.cacheCheckpoint(root, index)).to.be.revertedWith( + '!index', + ); + }); }); diff --git a/solidity/core/test/inbox.test.ts b/solidity/core/test/inbox.test.ts index e8c756cb7..de44564f3 100644 --- a/solidity/core/test/inbox.test.ts +++ b/solidity/core/test/inbox.test.ts @@ -52,30 +52,20 @@ describe('Inbox', async () => { beforeEach(async () => { const inboxFactory = new TestInbox__factory(signer); inbox = await inboxFactory.deploy(localDomain); - await inbox.initialize( - remoteDomain, - validatorManager.address, - ethers.constants.HashZero, - 0, - ); + await inbox.initialize(remoteDomain, validatorManager.address); }); it('Cannot be initialized twice', async () => { await expect( - inbox.initialize( - remoteDomain, - validatorManager.address, - ethers.constants.HashZero, - 0, - ), + inbox.initialize(remoteDomain, validatorManager.address), ).to.be.revertedWith('Initializable: contract is already initialized'); }); - it('Accepts checkpoint from validator manager', async () => { + it('Caches checkpoint from validator manager', async () => { const root = ethers.utils.formatBytes32String('first new root'); const index = 1; - await validatorManager.checkpoint(inbox.address, root, index); - const [croot, cindex] = await inbox.latestCheckpoint(); + await validatorManager.cacheCheckpoint(inbox.address, root, index); + const [croot, cindex] = await inbox.latestCachedCheckpoint(); expect(croot).to.equal(root); expect(cindex).to.equal(index); }); @@ -83,7 +73,7 @@ describe('Inbox', async () => { it('Rejects checkpoint from non-validator manager', async () => { const root = ethers.utils.formatBytes32String('first new root'); const index = 1; - await expect(inbox.checkpoint(root, index)).to.be.revertedWith( + await expect(inbox.cacheCheckpoint(root, index)).to.be.revertedWith( '!validatorManager', ); }); @@ -91,16 +81,16 @@ describe('Inbox', async () => { it('Rejects old checkpoint from validator manager', async () => { let root = ethers.utils.formatBytes32String('first new root'); let index = 10; - await validatorManager.checkpoint(inbox.address, root, index); - const [croot, cindex] = await inbox.latestCheckpoint(); + await validatorManager.cacheCheckpoint(inbox.address, root, index); + const [croot, cindex] = await inbox.latestCachedCheckpoint(); expect(croot).to.equal(root); expect(cindex).to.equal(index); root = ethers.utils.formatBytes32String('second new root'); index = 9; await expect( - validatorManager.checkpoint(inbox.address, root, index), - ).to.be.revertedWith('old checkpoint'); + validatorManager.cacheCheckpoint(inbox.address, root, index), + ).to.be.revertedWith('!newer'); }); it('Processes a valid message', async () => { @@ -110,7 +100,7 @@ describe('Inbox', async () => { await recipient.deployTransaction.wait(); const { index, proof, root, message } = messageWithProof; - await inbox.setCheckpoint(root, 1); + await inbox.setCachedCheckpoint(root, 1); await inbox.process(message, proof, index, '0x'); const hash = utils.messageHash(message, index); @@ -120,7 +110,7 @@ describe('Inbox', async () => { it('Rejects an already-processed message', async () => { const { leaf, index, proof, root, message } = messageWithProof; - await inbox.setCheckpoint(root, 1); + await inbox.setCachedCheckpoint(root, 1); // Set message status as MessageStatus.Processed await inbox.setMessageStatus(leaf, MessageStatus.PROCESSED); @@ -140,11 +130,11 @@ describe('Inbox', async () => { newProof[0] = proof[1]; newProof[1] = proof[0]; - await inbox.setCheckpoint(root, 1); + await inbox.setCachedCheckpoint(root, 1); expect( inbox.process(message, newProof as types.BytesArray, index, '0x'), - ).to.be.revertedWith('!checkpointed root'); + ).to.be.revertedWith('!cache'); expect(await inbox.messages(leaf)).to.equal(types.MessageStatus.NONE); }); @@ -274,7 +264,7 @@ describe('Inbox', async () => { path as types.BytesArray, index, ); - await inbox.setCheckpoint(proofRoot, 1); + await inbox.setCachedCheckpoint(proofRoot, 1); await inbox.process(abacusMessage, path as types.BytesArray, index, '0x'); diff --git a/solidity/core/test/outbox.test.ts b/solidity/core/test/outbox.test.ts index e1f1361c7..7d04c5c20 100644 --- a/solidity/core/test/outbox.test.ts +++ b/solidity/core/test/outbox.test.ts @@ -6,8 +6,6 @@ import { types, utils } from '@abacus-network/utils'; import { TestOutbox, TestOutbox__factory } from '../types'; -const destinationNonceTestCases = require('../../../vectors/destinationNonce.json'); - const localDomain = 1000; const destDomain = 2000; @@ -123,7 +121,7 @@ describe('Outbox', async () => { }); }); - it('Checkpoints the latest root', async () => { + it('Caches a checkpoint', async () => { const message = ethers.utils.formatBytes32String('message'); const count = 2; for (let i = 0; i < count; i++) { @@ -133,32 +131,19 @@ describe('Outbox', async () => { message, ); } - await outbox.checkpoint(); - const [root, index] = await outbox.latestCheckpoint(); + await outbox.cacheCheckpoint(); + const root = await outbox.latestCachedRoot(); expect(root).to.not.equal(ethers.constants.HashZero); - expect(index).to.equal(count - 1); - - expect(await outbox.isCheckpoint(root, index)).to.be.true; + expect(await outbox.cachedCheckpoints(root)).to.equal(count - 1); }); - it('does not allow a checkpoint of index 0', async () => { + it('does not allow caching a checkpoint with index 0', async () => { const message = ethers.utils.formatBytes32String('message'); await outbox.dispatch( destDomain, utils.addressToBytes32(recipient.address), message, ); - await expect(outbox.checkpoint()).to.be.revertedWith('!count'); - }); - - it('Correctly calculates destinationAndNonce', async () => { - for (const testCase of destinationNonceTestCases) { - const { destination, nonce, expectedDestinationAndNonce } = testCase; - const solidityDestinationAndNonce = await outbox.destinationAndNonce( - destination, - nonce, - ); - expect(solidityDestinationAndNonce).to.equal(expectedDestinationAndNonce); - } + await expect(outbox.cacheCheckpoint()).to.be.revertedWith('!index'); }); }); diff --git a/solidity/core/test/validator-manager/inboxValidatorManager.test.ts b/solidity/core/test/validator-manager/inboxValidatorManager.test.ts index b7f9815d6..2ac65a610 100644 --- a/solidity/core/test/validator-manager/inboxValidatorManager.test.ts +++ b/solidity/core/test/validator-manager/inboxValidatorManager.test.ts @@ -41,12 +41,7 @@ describe('InboxValidatorManager', () => { const inboxFactory = new Inbox__factory(signer); inbox = await inboxFactory.deploy(INBOX_DOMAIN); - await inbox.initialize( - OUTBOX_DOMAIN, - validatorManager.address, - ethers.constants.HashZero, - 0, - ); + await inbox.initialize(OUTBOX_DOMAIN, validatorManager.address); }); describe('#checkpoint', () => { @@ -60,9 +55,14 @@ describe('InboxValidatorManager', () => { [validator0, validator1], // 2/2 signers, making a quorum ); - await validatorManager.checkpoint(inbox.address, root, index, signatures); + await validatorManager.cacheCheckpoint( + inbox.address, + root, + index, + signatures, + ); - expect(await inbox.checkpoints(root)).to.equal(index); + expect(await inbox.cachedCheckpoints(root)).to.equal(index); }); it('reverts if there is not a quorum', async () => { @@ -73,7 +73,12 @@ describe('InboxValidatorManager', () => { ); await expect( - validatorManager.checkpoint(inbox.address, root, index, signatures), + validatorManager.cacheCheckpoint( + inbox.address, + root, + index, + signatures, + ), ).to.be.revertedWith('!quorum'); }); }); diff --git a/solidity/core/test/validator-manager/outboxValidatorManager.test.ts b/solidity/core/test/validator-manager/outboxValidatorManager.test.ts index a8a1c1a65..279ab4697 100644 --- a/solidity/core/test/validator-manager/outboxValidatorManager.test.ts +++ b/solidity/core/test/validator-manager/outboxValidatorManager.test.ts @@ -3,12 +3,13 @@ import { expect } from 'chai'; import { ethers } from 'hardhat'; import { Validator, types, utils } from '@abacus-network/utils'; +import { BytesArray } from '@abacus-network/utils/dist/src/types'; import { - Outbox, OutboxValidatorManager, OutboxValidatorManager__factory, - Outbox__factory, + TestOutbox, + TestOutbox__factory, } from '../../types'; import { signCheckpoint } from './utils'; @@ -17,13 +18,53 @@ const OUTBOX_DOMAIN = 1234; const INBOX_DOMAIN = 4321; const QUORUM_THRESHOLD = 2; +interface MerkleProof { + root: string; + proof: BytesArray; + leaf: string; + index: number; +} + describe('OutboxValidatorManager', () => { let validatorManager: OutboxValidatorManager, - outbox: Outbox, + outbox: TestOutbox, + helperOutbox: TestOutbox, signer: SignerWithAddress, validator0: Validator, validator1: Validator; + const dispatchMessage = async (outbox: TestOutbox, message: string) => { + const recipient = utils.addressToBytes32(validator0.address); + const destination = INBOX_DOMAIN; + const tx = await outbox.dispatch( + destination, + recipient, + ethers.utils.formatBytes32String(message), + ); + const receipt = await tx.wait(); + const dispatch = receipt.events![0]; + expect(dispatch.event).to.equal('Dispatch'); + return dispatch.args!; + }; + + const dispatchMessageAndReturnProof = async ( + outbox: TestOutbox, + messageStr: string, + ) => { + const { messageHash, leafIndex } = await dispatchMessage( + outbox, + messageStr, + ); + const root = await outbox.root(); + const proof = await outbox.proof(); + return { + root, + proof, + leaf: messageHash, + index: leafIndex, + }; + }; + before(async () => { const signers = await ethers.getSigners(); signer = signers[0]; @@ -39,82 +80,331 @@ describe('OutboxValidatorManager', () => { QUORUM_THRESHOLD, ); - const outboxFactory = new Outbox__factory(signer); + const outboxFactory = new TestOutbox__factory(signer); outbox = await outboxFactory.deploy(OUTBOX_DOMAIN); await outbox.initialize(validatorManager.address); + + // Deploy a second Outbox for convenience. We push a fraudulent message to this Outbox + // and use it to generate a fraudulent merkle proof. + helperOutbox = await outboxFactory.deploy(OUTBOX_DOMAIN); + await helperOutbox.initialize(validatorManager.address); }); - describe('#improperCheckpoint', () => { + describe('#prematureCheckpoint', () => { + const messageCount = 1; + // An premature checkpoint is one that has index greater than the latest index + // in the Outbox. + const prematureIndex = messageCount; const root = ethers.utils.formatBytes32String('test root'); - const index = 1; - it('accepts an improper checkpoint if there is a quorum', async () => { + beforeEach(async () => { + await dispatchMessage(outbox, 'message'); + }); + + it('accepts a premature checkpoint if it has been signed by a quorum of validators', async () => { const signatures = await signCheckpoint( root, - index, - [validator0, validator1], // 2/2 signers, making a quorum + prematureIndex, + [validator0, validator1], // 2/2 signers is a quorum ); await expect( - validatorManager.improperCheckpoint( + validatorManager.prematureCheckpoint( outbox.address, root, - index, + prematureIndex, signatures, ), ) - .to.emit(validatorManager, 'ImproperCheckpoint') - .withArgs(outbox.address, root, index, signatures); + .to.emit(validatorManager, 'PrematureCheckpoint') + .withArgs( + outbox.address, + root, + prematureIndex, + signatures, + messageCount, + ); expect(await outbox.state()).to.equal(types.AbacusState.FAILED); }); - it('reverts if there is not a quorum', async () => { + it('reverts if a premature checkpoint has not been signed a quorum of validators', async () => { const signatures = await signCheckpoint( root, - index, + prematureIndex, [validator0], // 1/2 signers is not a quorum ); await expect( - validatorManager.improperCheckpoint( + validatorManager.prematureCheckpoint( outbox.address, root, - index, + prematureIndex, signatures, ), ).to.be.revertedWith('!quorum'); }); - it('reverts if the checkpoint is not improper', async () => { - const message = `0x${Buffer.alloc(10).toString('hex')}`; - // Send two messages to allow checkpointing - await outbox.dispatch( - INBOX_DOMAIN, - utils.addressToBytes32(signer.address), - message, + it('reverts if a non-premature checkpoint has been signed by a quorum of validators', async () => { + const validIndex = messageCount - 1; + const signatures = await signCheckpoint( + root, + validIndex, + [validator0, validator1], // 2/2 signers is a quorum ); - await outbox.dispatch( - INBOX_DOMAIN, - utils.addressToBytes32(signer.address), - message, + + await expect( + validatorManager.prematureCheckpoint( + outbox.address, + root, + validIndex, + signatures, + ), + ).to.be.revertedWith('!premature'); + }); + }); + + const dispatchMessagesAndReturnProofs = async (args: { + differingIndex: number; + proofIndex: number; + messageCount: number; + }) => { + const { differingIndex, proofIndex, messageCount } = args; + const actualMessage = 'message'; + const fraudulentMessage = 'fraud'; + let index = 0; + const helperMessage = (j: number) => + j === differingIndex ? fraudulentMessage : actualMessage; + for (; index < proofIndex; index++) { + await dispatchMessage(outbox, actualMessage); + await dispatchMessage(helperOutbox, helperMessage(index)); + } + const proofA = await dispatchMessageAndReturnProof(outbox, actualMessage); + const proofB = await dispatchMessageAndReturnProof( + helperOutbox, + helperMessage(proofIndex), + ); + for (index = proofIndex + 1; index < messageCount; index++) { + await dispatchMessage(outbox, actualMessage); + await dispatchMessage(helperOutbox, helperMessage(index)); + } + + return { proofA: proofA, proofB: proofB }; + }; + + describe('#impliesDifferingLeaf', async () => { + it('returns true when proving a leaf with index greater than the differing leaf', async () => { + const { proofA, proofB } = await dispatchMessagesAndReturnProofs({ + differingIndex: 3, + proofIndex: 4, + messageCount: 5, + }); + expect( + await validatorManager.impliesDifferingLeaf( + proofA.leaf, + proofA.proof, + proofB.leaf, + proofB.proof, + proofA.index, + ), + ).to.be.true; + }); + + it('returns true when proving a leaf with index equal to the differing leaf', async () => { + const { proofA, proofB } = await dispatchMessagesAndReturnProofs({ + differingIndex: 4, + proofIndex: 4, + messageCount: 5, + }); + expect( + await validatorManager.impliesDifferingLeaf( + proofA.leaf, + proofA.proof, + proofB.leaf, + proofB.proof, + proofA.index, + ), + ).to.be.true; + }); + + it('returns false when proving a leaf with index less than the differing leaf', async () => { + const { proofA, proofB } = await dispatchMessagesAndReturnProofs({ + differingIndex: 4, + proofIndex: 3, + messageCount: 5, + }); + expect( + await validatorManager.impliesDifferingLeaf( + proofA.leaf, + proofA.proof, + proofB.leaf, + proofB.proof, + proofA.index, + ), + ).to.be.false; + }); + }); + + describe('#fraudulentCheckpoint', async () => { + let actual: MerkleProof, fraudulent: MerkleProof; + + beforeEach(async () => { + const { proofA, proofB } = await dispatchMessagesAndReturnProofs({ + differingIndex: 3, + proofIndex: 4, + messageCount: 5, + }); + actual = proofA; + fraudulent = proofB; + }); + + it('accepts a fraud proof signed by a quorum', async () => { + await outbox.cacheCheckpoint(); + const signatures = await signCheckpoint( + fraudulent.root, + fraudulent.index, + [validator0, validator1], // 2/2 signers is a quorum ); - await outbox.checkpoint(); - const [root, index] = await outbox.latestCheckpoint(); + await expect( + validatorManager.fraudulentCheckpoint( + outbox.address, + fraudulent.root, + fraudulent.index, + signatures, + fraudulent.leaf, + fraudulent.proof, + actual.leaf, + actual.proof, + fraudulent.index, + ), + ) + .to.emit(validatorManager, 'FraudulentCheckpoint') + .withArgs( + outbox.address, + fraudulent.root, + fraudulent.index, + signatures, + fraudulent.leaf, + fraudulent.proof, + actual.leaf, + actual.proof, + fraudulent.index, + ); + expect(await outbox.state()).to.equal(types.AbacusState.FAILED); + }); + + it('reverts if a fraud proof is not signed by a quorum', async () => { + await outbox.cacheCheckpoint(); const signatures = await signCheckpoint( - root, - index.toNumber(), - [validator0, validator1], // 2/2 signers, making a quorum + fraudulent.root, + fraudulent.index, + [validator0], // 1/2 signers is not a quorum ); await expect( - validatorManager.improperCheckpoint( + validatorManager.fraudulentCheckpoint( outbox.address, - root, - index, + fraudulent.root, + fraudulent.index, + signatures, + fraudulent.leaf, + fraudulent.proof, + actual.leaf, + actual.proof, + fraudulent.index, + ), + ).to.be.revertedWith('!quorum'); + }); + + it('reverts if the signed root is not fraudulent', async () => { + await outbox.cacheCheckpoint(); + const signatures = await signCheckpoint( + actual.root, + actual.index, + [validator0, validator1], // 2/2 signers is a quorum + ); + + await expect( + validatorManager.fraudulentCheckpoint( + outbox.address, + actual.root, + actual.index, + signatures, + fraudulent.leaf, + fraudulent.proof, + actual.leaf, + actual.proof, + fraudulent.index, + ), + ).to.be.revertedWith('!root'); + }); + + it('reverts if the disputed leaf is not committed to by the signed checkpoint', async () => { + await outbox.cacheCheckpoint(); + const signatures = await signCheckpoint( + fraudulent.root, + fraudulent.index - 1, + [validator0, validator1], // 2/2 signers is a quorum + ); + + await expect( + validatorManager.fraudulentCheckpoint( + outbox.address, + fraudulent.root, + fraudulent.index - 1, + signatures, + fraudulent.leaf, + fraudulent.proof, + actual.leaf, + actual.proof, + fraudulent.index, + ), + ).to.be.revertedWith('!index'); + }); + + it('reverts if the actual root is not cached', async () => { + const signatures = await signCheckpoint( + fraudulent.root, + fraudulent.index, + [validator0, validator1], // 2/2 signers is a quorum + ); + + await expect( + validatorManager.fraudulentCheckpoint( + outbox.address, + fraudulent.root, + fraudulent.index, + signatures, + fraudulent.leaf, + fraudulent.proof, + actual.leaf, + actual.proof, + fraudulent.index, + ), + ).to.be.revertedWith('!cache'); + }); + + it('reverts if the root is not fraudulent', async () => { + await outbox.cacheCheckpoint(); + const signatures = await signCheckpoint( + actual.root, + actual.index, + [validator0, validator1], // 2/2 signers is a quorum + ); + + await expect( + validatorManager.fraudulentCheckpoint( + outbox.address, + actual.root, + actual.index, signatures, + actual.leaf, + actual.proof, + actual.leaf, + actual.proof, + actual.index, ), - ).to.be.revertedWith('!improper'); + ).to.be.revertedWith('!fraud'); }); }); }); diff --git a/typescript/deploy/src/core/deploy.ts b/typescript/deploy/src/core/deploy.ts index 5ca1e555a..142ac563b 100644 --- a/typescript/deploy/src/core/deploy.ts +++ b/typescript/deploy/src/core/deploy.ts @@ -154,12 +154,7 @@ export class AbacusCoreDeployer< this.inboxFactoryBuilder(signer), [domain], upgradeBeaconController.address, - [ - chainMetadata[firstRemote].id, - firstValidatorManager.address, - ethers.constants.HashZero, - 0, - ], + [chainMetadata[firstRemote].id, firstValidatorManager.address], ); const getMailbox = ( @@ -184,12 +179,7 @@ export class AbacusCoreDeployer< chain, 'Inbox', firstInbox, - [ - chainMetadata[remote].id, - validatorManager.address, - ethers.constants.HashZero, - 0, - ], + [chainMetadata[remote].id, validatorManager.address], ); return [remote, getMailbox(validatorManager, inbox)]; diff --git a/typescript/hardhat/src/TestCoreApp.ts b/typescript/hardhat/src/TestCoreApp.ts index a47dce99b..d95847700 100644 --- a/typescript/hardhat/src/TestCoreApp.ts +++ b/typescript/hardhat/src/TestCoreApp.ts @@ -71,11 +71,6 @@ export class TestCoreApp extends AbacusCore { this.getContracts(destinationChain).inboxes[origin].inbox; const status = await inbox.messages(dispatch.args.messageHash); if (status !== types.MessageStatus.PROCESSED) { - if (dispatch.args.leafIndex.toNumber() == 0) { - // disregard the dummy message - continue; - } - const response = await inbox.testProcess( dispatch.args.message, dispatch.args.leafIndex.toNumber(), diff --git a/typescript/hardhat/src/TestCoreDeploy.ts b/typescript/hardhat/src/TestCoreDeploy.ts index 037380190..7c190a390 100644 --- a/typescript/hardhat/src/TestCoreDeploy.ts +++ b/typescript/hardhat/src/TestCoreDeploy.ts @@ -1,13 +1,7 @@ import { TestCoreApp } from './TestCoreApp'; import { TestInbox__factory, TestOutbox__factory } from '@abacus-network/core'; import { AbacusCoreDeployer, CoreConfig } from '@abacus-network/deploy'; -import { - chainMetadata, - CoreContractAddresses, - MultiProvider, - TestChainNames, -} from '@abacus-network/sdk'; -import { utils } from '@abacus-network/utils'; +import { MultiProvider, TestChainNames } from '@abacus-network/sdk'; import { ethers } from 'ethers'; // dummy config as TestInbox and TestOutbox do not use deployed ValidatorManager @@ -32,28 +26,6 @@ export class TestCoreDeploy extends AbacusCoreDeployer { outboxFactoryBuilder = (signer: ethers.Signer) => new TestOutbox__factory(signer); - async deployContracts( - local: LocalChain, - config: CoreConfig, - ): Promise> { - const addresses = await super.deployContracts(local, config); - - const signer = this.multiProvider.getChainConnection(local).signer!; - const outbox = this.outboxFactoryBuilder(signer).attach( - addresses.outbox.proxy, - ); - const remote = this.multiProvider.remoteChains(local)[0]; - - // dispatch a dummy event to allow a consumer to checkpoint/process a single message - await outbox.dispatch( - chainMetadata[remote].id, - utils.addressToBytes32(ethers.constants.AddressZero), - '0x', - ); - - return addresses; - } - async deployCore() { const result = await super.deploy(); return new TestCoreApp(result, this.multiProvider); diff --git a/typescript/infra/hardhat.config.ts b/typescript/infra/hardhat.config.ts index 9d954815f..65539aa76 100644 --- a/typescript/infra/hardhat.config.ts +++ b/typescript/infra/hardhat.config.ts @@ -22,15 +22,13 @@ const chainSummary = async ( ) => { const coreContracts = core.getContracts(chain); const outbox = coreContracts.outbox.outbox; - const [outboxCheckpointRoot, outboxCheckpointIndex] = - await outbox.latestCheckpoint(); const count = (await outbox.tree()).toNumber(); const inboxSummary = async (remote: Chain) => { const remoteContracts = core.getContracts(remote); const inbox = remoteContracts.inboxes[chain as Exclude].inbox; const [inboxCheckpointRoot, inboxCheckpointIndex] = - await inbox.latestCheckpoint(); + await inbox.latestCachedCheckpoint(); const processFilter = inbox.filters.Process(); const processes = await inbox.queryFilter(processFilter); return { @@ -45,10 +43,6 @@ const chainSummary = async ( chain, outbox: { count, - checkpoint: { - root: outboxCheckpointRoot, - index: outboxCheckpointIndex.toNumber(), - }, }, inboxes: await Promise.all( core.remoteChains(chain).map((remote) => inboxSummary(remote)), @@ -91,9 +85,6 @@ task('kathy', 'Dispatches random abacus messages').setAction( utils.addressToBytes32(recipient.address), '0x1234', ); - if ((await outbox.count()).gt(1)) { - await outbox.checkpoint(); - } console.log( `send to ${recipient.address} on ${remote} at index ${ (await outbox.count()).toNumber() - 1 diff --git a/typescript/sdk/CHANGELOG.md b/typescript/sdk/CHANGELOG.md index 858428a15..b41a238f3 100644 --- a/typescript/sdk/CHANGELOG.md +++ b/typescript/sdk/CHANGELOG.md @@ -10,4 +10,4 @@ ## 0.1.1 -Initial Alpha Release of the SDK \ No newline at end of file +Initial Alpha Release of the SDK diff --git a/typescript/sdk/src/core/events.ts b/typescript/sdk/src/core/events.ts index 704c29fa9..9f3871778 100644 --- a/typescript/sdk/src/core/events.ts +++ b/typescript/sdk/src/core/events.ts @@ -5,17 +5,16 @@ import { TypedEvent } from '@abacus-network/core/dist/commons'; import { Annotated } from '../events'; // copied from the Outbox.d.ts -export type DispatchTypes = [string, BigNumber, BigNumber, string, string]; +export type DispatchTypes = [string, BigNumber, number, string]; export type DispatchArgs = { messageHash: string; leafIndex: BigNumber; - destinationAndNonce: BigNumber; - committedRoot: string; + destination: number; message: string; }; export type DispatchEvent = TypedEvent; -// copied from the Outbox.d.ts +// copied from the Inbox.d.ts export type CheckpointTypes = [string, BigNumber]; export type CheckpointArgs = { root: string; index: BigNumber }; export type CheckpointEvent = TypedEvent; diff --git a/typescript/sdk/src/core/message.ts b/typescript/sdk/src/core/message.ts index 55d712697..e92e716e1 100644 --- a/typescript/sdk/src/core/message.ts +++ b/typescript/sdk/src/core/message.ts @@ -76,7 +76,6 @@ export enum InboxMessageStatus { } export type EventCache = { - outboxCheckpoint?: AnnotatedCheckpoint; inboxCheckpoint?: AnnotatedCheckpoint; process?: AnnotatedProcess; }; @@ -276,51 +275,6 @@ export class AbacusMessage { ); } - /** - * Get the Outbox `Checkpoint` event associated with this message (if any) - * - * @returns An {@link AnnotatedCheckpoint} (if any) - */ - async getOutboxCheckpoint(): Promise { - // if we have already gotten the event, - // return it without re-querying - if (this.cache.outboxCheckpoint) { - return this.cache.outboxCheckpoint; - } - - const leafIndex = this.dispatch.event.args.leafIndex; - const [checkpointRoot, checkpointIndex] = - await this.outbox.latestCheckpoint(); - // The checkpoint index needs to be at least leafIndex + 1 to include - // the message. - if (checkpointIndex.lte(leafIndex)) { - return undefined; - } - - // Query the latest checkpoint event. - const checkpointFilter = this.outbox.filters.Checkpoint( - checkpointRoot, - checkpointIndex, - ); - - const checkpointLogs: AnnotatedCheckpoint[] = - await findAnnotatedSingleEvent( - this.multiProvider, - this.originName, - this.outbox, - checkpointFilter, - ); - - if (checkpointLogs.length === 1) { - // if event is returned, store it to the object - this.cache.outboxCheckpoint = checkpointLogs[0]; - } else if (checkpointLogs.length > 1) { - throw new Error('multiple outbox checkpoints for same root and index'); - } - // return the event or undefined if it doesn't exist - return this.cache.outboxCheckpoint; - } - /** * Get the Inbox `Checkpoint` event associated with this message (if any) * @@ -335,7 +289,7 @@ export class AbacusMessage { const leafIndex = this.dispatch.event.args.leafIndex; const [checkpointRoot, checkpointIndex] = - await this.inbox.latestCheckpoint(); + await this.inbox.latestCachedCheckpoint(); // The checkpoint index needs to be at least leafIndex + 1 to include // the message. if (checkpointIndex.lte(leafIndex)) { @@ -343,7 +297,7 @@ export class AbacusMessage { } // if not, attempt to query the event - const checkpointFilter = this.inbox.filters.Checkpoint( + const checkpointFilter = this.inbox.filters.CheckpointCached( checkpointRoot, checkpointIndex, ); @@ -404,15 +358,6 @@ export class AbacusMessage { */ async events(): Promise { const events: AnnotatedLifecycleEvent[] = [this.dispatch]; - // attempt to get Outbox checkpoint - const outboxCheckpoint = await this.getOutboxCheckpoint(); - if (!outboxCheckpoint) { - return { - status: MessageStatus.Dispatched, // the message has been sent; nothing more - events, - }; - } - events.push(outboxCheckpoint); // attempt to get Inbox checkpoint const inboxCheckpoint = await this.getInboxCheckpoint(); if (!inboxCheckpoint) { @@ -549,18 +494,4 @@ export class AbacusMessage { get leafIndex(): BigNumber { return this.dispatch.event.args.leafIndex; } - - /** - * The destination and nonceof this message. - */ - get destinationAndNonce(): BigNumber { - return this.dispatch.event.args.destinationAndNonce; - } - - /** - * The committed root when this message was dispatched. - */ - get committedRoot(): string { - return this.dispatch.event.args.committedRoot; - } } diff --git a/vectors/destinationNonce.json b/vectors/destinationNonce.json deleted file mode 100644 index 7687339dc..000000000 --- a/vectors/destinationNonce.json +++ /dev/null @@ -1,27 +0,0 @@ -[ - { - "destination": 1, - "expectedDestinationAndNonce": 4294967298, - "nonce": 2 - }, - { - "destination": 2, - "expectedDestinationAndNonce": 8589934595, - "nonce": 3 - }, - { - "destination": 3, - "expectedDestinationAndNonce": 12884901892, - "nonce": 4 - }, - { - "destination": 4, - "expectedDestinationAndNonce": 17179869189, - "nonce": 5 - }, - { - "destination": 5, - "expectedDestinationAndNonce": 21474836486, - "nonce": 6 - } -]