From dc0a64b82b97976cfb469d5581df34db1bbb5f5a Mon Sep 17 00:00:00 2001 From: Trevor Porter Date: Thu, 7 Apr 2022 15:37:57 +0100 Subject: [PATCH] All contracts working together, need to separate local vs remote, clean up natspec, and add tests --- solidity/core/contracts/Common.sol | 8 + solidity/core/contracts/Inbox.sol | 30 +-- solidity/core/contracts/Outbox.sol | 24 ++- .../CommonMultisigValidatorManager.sol | 204 ++++++++++++++++++ 4 files changed, 238 insertions(+), 28 deletions(-) create mode 100644 solidity/core/contracts/validator-manager/CommonMultisigValidatorManager.sol diff --git a/solidity/core/contracts/Common.sol b/solidity/core/contracts/Common.sol index 46857504d..3c8a7279b 100644 --- a/solidity/core/contracts/Common.sol +++ b/solidity/core/contracts/Common.sol @@ -50,6 +50,14 @@ abstract contract Common is OwnableUpgradeable { // ============ Modifiers ============ + /** + * @notice Ensures that function is called by the ValidatorManager contract + */ + modifier onlyValidatorManager() { + require(msg.sender == address(validatorManager), "!validatorManager"); + _; + } + // ============ Constructor ============ constructor(uint32 _localDomain) { diff --git a/solidity/core/contracts/Inbox.sol b/solidity/core/contracts/Inbox.sol index 585fd1c14..2aef48b64 100644 --- a/solidity/core/contracts/Inbox.sol +++ b/solidity/core/contracts/Inbox.sol @@ -80,29 +80,19 @@ contract Inbox is Version0, Common { // ============ External Functions ============ /** - * @notice Checkpoints the provided root and index given a signature. + * @notice Checkpoints the provided root and index. + * @dev Called by the ValidatorManager, which is responsible for verifying + * quorum of validator signatures on the checkpoint. * @dev Reverts if checkpoints's index is not greater than our latest index. - * @param _root Checkpoint's merkle root - * @param _index Checkpoint's index - * @param _signature Validator's signature on `_root` and `_index` + * @param _root Checkpoint's merkle root. + * @param _index Checkpoint's index. */ - function checkpoint( - bytes32 _root, - uint256 _index, - bytes memory _signature - ) external { - // ensure that update is more recent than the latest we've seen + function checkpoint(bytes32 _root, uint256 _index) + external + onlyValidatorManager + { + // Ensure that the checkpoint is more recent than the latest we've seen. require(_index > checkpoints[checkpointedRoot], "old checkpoint"); - // validate validator signature - require( - validatorManager.isValidatorSignature( - remoteDomain, - _root, - _index, - _signature - ), - "!validator sig" - ); _checkpoint(_root, _index); } diff --git a/solidity/core/contracts/Outbox.sol b/solidity/core/contracts/Outbox.sol index a234a9bfa..527fea0a5 100644 --- a/solidity/core/contracts/Outbox.sol +++ b/solidity/core/contracts/Outbox.sol @@ -98,14 +98,6 @@ contract Outbox is Version0, MerkleTreeManager, Common { _; } - /** - * @notice Ensures that function is called by the ValidatorManager contract - */ - modifier onlyValidatorManager() { - require(msg.sender == address(validatorManager), "!validatorManager"); - _; - } - // ============ External Functions ============ /** @@ -174,6 +166,22 @@ contract Outbox is Version0, MerkleTreeManager, Common { emit Fail(); } + /** + * @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. + */ + function isCheckpoint(bytes32 _root, uint256 _index) + external + view + returns (bool) + { + // Checkpoint indices are one-indexed. + return _index > 0 && checkpoints[_root] == _index; + } + // ============ Internal Functions ============ /** diff --git a/solidity/core/contracts/validator-manager/CommonMultisigValidatorManager.sol b/solidity/core/contracts/validator-manager/CommonMultisigValidatorManager.sol new file mode 100644 index 000000000..4ba837b9a --- /dev/null +++ b/solidity/core/contracts/validator-manager/CommonMultisigValidatorManager.sol @@ -0,0 +1,204 @@ +// SPDX-License-Identifier: MIT OR Apache-2.0 +pragma solidity >=0.6.11; +pragma abicoder v2; + +// ============ Internal Imports ============ +import {Inbox} from "../Inbox.sol"; +import {Outbox} from "../Outbox.sol"; +// ============ External Imports ============ +import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol"; +import {ECDSA} from "@openzeppelin/contracts/cryptography/ECDSA.sol"; +import {EnumerableSet} from "@openzeppelin/contracts/utils/EnumerableSet.sol"; + +contract CommonMultisigValidatorManager is Ownable { + // ============ Libraries ============ + + using EnumerableSet for EnumerableSet.AddressSet; + + // ============ Immutables ============ + + // The domain of the outbox the set of validators this validator manager + // tracks is for. + uint32 public immutable outboxDomain; + + bytes32 public immutable outboxDomainHash; + + // ============ Mutable Storage ============ + + // The minimum threshold of validator signatures to constitute a quorum + uint256 public threshold; + + // The set of validators + EnumerableSet.AddressSet private validators; + + // ============ Events ============ + + /** + * @notice Emitted when a validator is enrolled in the validator set. + * @param validator The address of the validator. + */ + event EnrollValidator(address indexed validator); + + /** + * @notice Emitted when a validator is unenrolled in the validator set. + * @param validator The address of the validator. + */ + event UnenrollValidator(address indexed validator); + + /** + * @notice Emitted when the quorum threshold is set. + * @param threshold The quorum threshold. + */ + event SetThreshold(uint256 threshold); + + /** + * @notice Emitted when proof of an improper checkpoint is submitted. + * @param root Root of the improper checkpoint. + * @param index Index of the improper checkpoint. + * @param signatures A quorum of signatures on the improper checkpoint. + */ + event ImproperCheckpoint( + address indexed outbox, + bytes32 indexed root, + uint256 index, + bytes[] signatures + ); + + // ============ Constructor ============ + + /** + * @param _outboxDomain The domain of the outbox this validator manager + * tracks the validator set for. + */ + constructor(uint32 _outboxDomain) Ownable() { + outboxDomain = _outboxDomain; + outboxDomainHash = keccak256(abi.encodePacked(_outboxDomain, "ABACUS")); + } + + // ============ External Functions ============ + + // Adds _validator to validators + function enrollValidator(address _validator) external onlyOwner { + // Revert if _validator is already an enrolled validator. + require(validators.add(_validator), "!unenrolled"); + emit EnrollValidator(_validator); + } + + // Removes _validator from validators + function unenrollValidator(address _validator) external onlyOwner { + // Revert if _validator is not an already enrolled validator. + require(validators.remove(_validator), "!enrolled"); + emit UnenrollValidator(_validator); + } + + function setThreshold(uint256 _threshold) external onlyOwner { + threshold = _threshold; + emit SetThreshold(_threshold); + } + + // Gets the domain from IInbox(_inbox).localDomain(), then + // requires isQuorum(domain, _root, _index, _signatures), + // and then calls IInbox(_inbox).checkpoint(_root, _index); + function checkpoint( + Inbox _inbox, + bytes32 _root, + uint256 _index, + bytes[] calldata _signatures + ) external { + require(isQuorum(_root, _index, _signatures), "!quorum"); + _inbox.checkpoint(_root, _index); + } + + // Determines if a quorum of signers have signed an improper checkpoint, + // and fails the Outbox if so. + // If staking / slashing existed, we'd want to check this for individual validator + // signatures. Because we don't care about that and we don't want a single byzantine + // validator to be able to fail the outbox, we require a quorum. + // + // Gets the domain from IOutbox(_outbox).localDomain(), then + // requires isQuorum(domain, _root, _index, _signatures), + // requires that the checkpoint is an improper checkpoint, + // and calls IOutbox(_outbox).fail(). (Similar behavior as existing improperCheckpoint) + function improperCheckpoint( + Outbox _outbox, + bytes32 _root, + uint256 _index, + bytes[] calldata _signatures + ) external { + require(isQuorum(_root, _index, _signatures), "!quorum"); + require(!_outbox.isCheckpoint(_root, _index), "!improper checkpoint"); + _outbox.fail(); + emit ImproperCheckpoint(address(_outbox), _root, _index, _signatures); + } + + // Just returns the addresses in the private enumerable set `validators`. + function validatorSet() external view returns (address[] memory) { + uint256 _length = validators.length(); + address[] memory _validatorSet = new address[](_length); + for (uint256 i = 0; i < _length; i++) { + _validatorSet[i] = validators.at(i); + } + return _validatorSet; + } + + // ============ Public Functions ============ + + // Returns whether the provided signatures over the checkpoint for the domain + // constitute a quorum of validator signatures. + // Requires each signature to be over the given domain, root, and index. + // Requires _signatures to be sorted by their recovered signer's address for duplicate detection. + // Requires each recovered signer to be in the `validators` set. + // Requires _signatures.length to be >= threshold. + function isQuorum( + bytes32 _root, + uint256 _index, + bytes[] calldata _signatures + ) public view returns (bool) { + uint256 _signaturesLength = _signatures.length; + // If there are less signatures provided than the required threshold, + // this is not a quorum. + if (_signaturesLength < threshold) { + return false; + } + // To identify duplicates, the signers recovered from _signatures + // must be sorted in ascending order. previousSigner is used to + // enforce sort order. + address _previousSigner = address(0); + uint256 _validatorSignatureCount = 0; + for (uint256 i = 0; i < _signaturesLength; i++) { + address _signer = recoverCheckpointSigner( + _root, + _index, + _signatures[i] + ); + // Revert if the signer violates the required sort order. + require(_previousSigner < _signer, "!sorted signers"); + // If the signer is a validator, increment _validatorSignatureCount. + if (validators.contains(_signer)) { + _validatorSignatureCount++; + } + } + return _validatorSignatureCount >= threshold; + } + + // ============ Internal Functions ============ + + /** + * @notice Recovers the signer from a signature of a checkpoint. + * @param _root The checkpoint's merkle root. + * @param _index The checkpoint's index. + * @param _signature Signature on the the checkpoint. + * @return The signer of the checkpoint signature. + **/ + function recoverCheckpointSigner( + bytes32 _root, + uint256 _index, + bytes calldata _signature + ) internal view returns (address) { + bytes32 _digest = keccak256( + abi.encodePacked(outboxDomainHash, _root, _index) + ); + _digest = ECDSA.toEthSignedMessageHash(_digest); + return ECDSA.recover(_digest, _signature); + } +}