diff --git a/solidity/core/contracts/validator-manager/InboxMultisigValidatorManager.sol b/solidity/core/contracts/validator-manager/InboxMultisigValidatorManager.sol new file mode 100644 index 000000000..5fa72104d --- /dev/null +++ b/solidity/core/contracts/validator-manager/InboxMultisigValidatorManager.sol @@ -0,0 +1,40 @@ +// SPDX-License-Identifier: MIT OR Apache-2.0 +pragma solidity >=0.6.11; +pragma abicoder v2; + +// ============ Internal Imports ============ +import {MultisigValidatorManager} from "./MultisigValidatorManager.sol"; +import {Inbox} from "../Inbox.sol"; + +contract InboxMultisigValidatorManager is MultisigValidatorManager { + // ============ Constructor ============ + + /** + * @param _remoteDomain The remote domain of the outbox chain. + */ + // solhint-disable-next-line no-empty-blocks + constructor(uint32 _remoteDomain) MultisigValidatorManager(_remoteDomain) {} + + // ============ External Functions ============ + + /** + * @notice Submits a checkpoint signed by a quorum of validators to 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. + * @param _inbox The inbox to submit the checkpoint to. + * @param _root The merkle root of the checkpoint. + * @param _index The index of the checkpoint. + * @param _signatures Signatures over the checkpoint to be checked for a validator + * quorum. Must be sorted in ascending order by signer address. + */ + function checkpoint( + Inbox _inbox, + bytes32 _root, + uint256 _index, + bytes[] calldata _signatures + ) external { + require(isQuorum(_root, _index, _signatures), "!quorum"); + _inbox.checkpoint(_root, _index); + } +} diff --git a/solidity/core/contracts/validator-manager/CommonMultisigValidatorManager.sol b/solidity/core/contracts/validator-manager/MultisigValidatorManager.sol similarity index 51% rename from solidity/core/contracts/validator-manager/CommonMultisigValidatorManager.sol rename to solidity/core/contracts/validator-manager/MultisigValidatorManager.sol index 4ba837b9a..bf17d59cf 100644 --- a/solidity/core/contracts/validator-manager/CommonMultisigValidatorManager.sol +++ b/solidity/core/contracts/validator-manager/MultisigValidatorManager.sol @@ -2,33 +2,34 @@ 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 { +/** + * @notice Manages an ownable validator set that sign checkpoints with + * a basic ECDSA multi-signature. + */ +abstract contract MultisigValidatorManager is Ownable { // ============ Libraries ============ using EnumerableSet for EnumerableSet.AddressSet; // ============ Immutables ============ - // The domain of the outbox the set of validators this validator manager - // tracks is for. + // The domain of the validator set's outbox chain. uint32 public immutable outboxDomain; + // The domain hash of the validator set's outbox chain. bytes32 public immutable outboxDomainHash; // ============ Mutable Storage ============ - // The minimum threshold of validator signatures to constitute a quorum - uint256 public threshold; + // The minimum threshold of validator signatures to constitute a quorum. + uint256 public quorumThreshold; - // The set of validators + // The set of validators. EnumerableSet.AddressSet private validators; // ============ Events ============ @@ -47,22 +48,9 @@ contract CommonMultisigValidatorManager is Ownable { /** * @notice Emitted when the quorum threshold is set. - * @param threshold The quorum threshold. + * @param quorumThreshold 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 - ); + event SetQuorumThreshold(uint256 quorumThreshold); // ============ Constructor ============ @@ -77,92 +65,68 @@ contract CommonMultisigValidatorManager is Ownable { // ============ External Functions ============ - // Adds _validator to validators + /** + * @notice Enrolls a validator into the validator set. + * @dev Reverts if _validator is already in the validator set. + * @param _validator The validator to add to the validator set. + */ function enrollValidator(address _validator) external onlyOwner { - // Revert if _validator is already an enrolled validator. - require(validators.add(_validator), "!unenrolled"); + require(validators.add(_validator), "enrolled"); emit EnrollValidator(_validator); } - // Removes _validator from validators + /** + * @notice Unenrolls a validator from the validator set. + * @dev Reverts if _validator is not in the validator set. + * @param _validator The validator to remove from the validator set. + */ 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; + /** + * @notice Sets the quorum threshold. + * @param _quorumThreshold The new quorum threshold. + */ + function setQuorumThreshold(uint256 _quorumThreshold) external onlyOwner { + require( + _quorumThreshold > 0 && _quorumThreshold <= validators.length(), + "!range" + ); + quorumThreshold = _quorumThreshold; + emit SetQuorumThreshold(_quorumThreshold); } // ============ 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. + /** + * @notice Returns whether provided signatures over a checkpoint constitute + * 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. + * @dev Does not revert if a signature's signer is not in the validator set. + * @param _root The merkle root of the checkpoint. + * @param _index The index of the 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 _signatures constitute a quorum of validator signatures over + * the checkpoint. + */ 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, + // If there are less signatures provided than the required quorum threshold, // this is not a quorum. - if (_signaturesLength < threshold) { + if (_signaturesLength < quorumThreshold) { return false; } // To identify duplicates, the signers recovered from _signatures // must be sorted in ascending order. previousSigner is used to - // enforce sort order. + // enforce ordering. address _previousSigner = address(0); uint256 _validatorSignatureCount = 0; for (uint256 i = 0; i < _signaturesLength; i++) { @@ -177,8 +141,9 @@ contract CommonMultisigValidatorManager is Ownable { if (validators.contains(_signer)) { _validatorSignatureCount++; } + _previousSigner = _signer; } - return _validatorSignatureCount >= threshold; + return _validatorSignatureCount >= quorumThreshold; } // ============ Internal Functions ============ diff --git a/solidity/core/contracts/validator-manager/OutboxMultisigValidatorManager.sol b/solidity/core/contracts/validator-manager/OutboxMultisigValidatorManager.sol new file mode 100644 index 000000000..d7bba02a6 --- /dev/null +++ b/solidity/core/contracts/validator-manager/OutboxMultisigValidatorManager.sol @@ -0,0 +1,56 @@ +// SPDX-License-Identifier: MIT OR Apache-2.0 +pragma solidity >=0.6.11; +pragma abicoder v2; + +// ============ Internal Imports ============ +import {MultisigValidatorManager} from "./MultisigValidatorManager.sol"; +import {Outbox} from "../Outbox.sol"; + +contract OutboxMultisigValidatorManager is MultisigValidatorManager { + // ============ Events ============ + + /** + * @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 _localDomain The local domain. + */ + // solhint-disable-next-line no-empty-blocks + constructor(uint32 _localDomain) MultisigValidatorManager(_localDomain) {} + + // ============ External Functions ============ + + // 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); + } +}