Create Inbox/Outbox specific MultisigValidatorManagers

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

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
+    // The domain of the validator set's outbox chain.
-    // tracks is for.
     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
+    // The minimum threshold of validator signatures to constitute a quorum.
-    uint256 public threshold;
+    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);
+    event SetQuorumThreshold(uint256 quorumThreshold);
-
-    /**
-     * @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 ============
 
@@ -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), "enrolled");
-        require(validators.add(_validator), "!unenrolled");
         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;
+     * @notice Sets the quorum threshold.
-        emit SetThreshold(_threshold);
+     * @param _quorumThreshold The new quorum threshold.
-    }
+     */
-
+    function setQuorumThreshold(uint256 _quorumThreshold) external onlyOwner {
-    // Gets the domain from IInbox(_inbox).localDomain(), then
+        require(
-    // requires isQuorum(domain, _root, _index, _signatures),
+            _quorumThreshold > 0 && _quorumThreshold <= validators.length(),
-    // and then calls IInbox(_inbox).checkpoint(_root, _index);
+            "!range"
-    function checkpoint(
+        );
-        Inbox _inbox,
+        quorumThreshold = _quorumThreshold;
-        bytes32 _root,
+        emit SetQuorumThreshold(_quorumThreshold);
-        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.
+     * @notice Returns whether provided signatures over a checkpoint constitute
-    // Requires each signature to be over the given domain, root, and index.
+     * a quorum of validator signatures.
-    // Requires _signatures to be sorted by their recovered signer's address for duplicate detection.
+     * @dev Reverts if _signatures is not sorted in ascending order by the signer
-    // Requires each recovered signer to be in the `validators` set.
+     * address, which is required for duplicate detection.
-    // Requires _signatures.length to be >= threshold.
+     * @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 ============

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