feat(validator): modified validator timelock to allow more than 1 val…

…idator (#211)

l1-contracts/contracts/zksync/ValidatorTimelock.sol

@@ -26,33 +26,56 @@ contract ValidatorTimelock is IExecutor, Ownable2Step {
     /// @notice The delay between committing and executing batches is changed.
     event NewExecutionDelay(uint256 _newExecutionDelay);
 
-    /// @notice The validator address is changed.
-    event NewValidator(address _oldValidator, address _newValidator);
+    /// @notice A new validator has been added.
+    event ValidatorAdded(address _addedValidator);
+
+    /// @notice A validator has been removed.
+    event ValidatorRemoved(address _removedValidator);
+
+    /// @notice Error for when an address is already a validator.
+    error AddressAlreadyValidator();
+
+    /// @notice Error for when an address is not a validator.
+    error ValidatorDoesNotExist();
 
     /// @dev The main zkSync smart contract.
     address public immutable zkSyncContract;
 
     /// @dev The mapping of L2 batch number => timestamp when it was committed.
     LibMap.Uint32Map internal committedBatchTimestamp;
 
-    /// @dev The address that can commit/revert/validate/execute batches.
-    address public validator;
-
-    /// @dev The delay between committing and executing batches.
+    /// @dev The delay between committing and executing batches.ValidatorTimelock
     uint32 public executionDelay;
 
-    constructor(address _initialOwner, address _zkSyncContract, uint32 _executionDelay, address _validator) {
+    /// @dev Mapping denoting if an address is also a validator
+    mapping(address => bool) public validators;
+
+    constructor(address _initialOwner, address _zkSyncContract, uint32 _executionDelay, address[] memory _validators) {
         _transferOwnership(_initialOwner);
         zkSyncContract = _zkSyncContract;
         executionDelay = _executionDelay;
-        validator = _validator;
+
+        for (uint256 i = 0; i < _validators.length; i++) {
+            validators[_validators[i]] = true;
+        }
     }
 
-    /// @dev Set new validator address.
-    function setValidator(address _newValidator) external onlyOwner {
-        address oldValidator = validator;
-        validator = _newValidator;
-        emit NewValidator(oldValidator, _newValidator);
+    /// @dev Sets an address as a validator.
+    function addValidator(address _newValidator) external onlyOwner {
+        if (validators[_newValidator]) {
+            revert AddressAlreadyValidator();
+        }
+        validators[_newValidator] = true;
+        emit ValidatorAdded(_newValidator);
+    }
+
+    /// @dev Removes an address as a validator.
+    function removeValidator(address _validator) external onlyOwner {
+        if (!validators[_validator]) {
+            revert ValidatorDoesNotExist();
+        }
+        validators[_validator] = false;
+        emit ValidatorRemoved(_validator);
     }
 
     /// @dev Set the delay between committing and executing batches.
@@ -63,7 +86,7 @@ contract ValidatorTimelock is IExecutor, Ownable2Step {
 
     /// @notice Checks if the caller is a validator.
     modifier onlyValidator() {
-        require(msg.sender == validator, "8h");
+        require(validators[msg.sender] == true, "8h");
         _;
     }
 

l1-contracts/src.ts/deploy.ts

@@ -417,7 +417,7 @@ export class Deployer {
     const validatorAddress = getAddressFromEnv("ETH_SENDER_SENDER_OPERATOR_COMMIT_ETH_ADDR");
     const contractAddress = await this.deployViaCreate2(
       "ValidatorTimelock",
-      [this.ownerAddress, this.addresses.ZkSync.DiamondProxy, executionDelay, validatorAddress],
+      [this.ownerAddress, this.addresses.ZkSync.DiamondProxy, executionDelay, [validatorAddress]],
       create2Salt,
       ethTxOptions
     );

l1-contracts/test/foundry/unit/concrete/Utils/Utils.sol

@@ -3,7 +3,7 @@ pragma solidity 0.8.20;
 
 import {GettersFacet} from "../../../../../cache/solpp-generated-contracts/zksync/facets/Getters.sol";
 import {MailboxFacet} from "../../../../../cache/solpp-generated-contracts/zksync/facets/Mailbox.sol";
-import {SystemLogKey} from "solpp/zksync/interfaces/IExecutor.sol";
+import {IExecutor, SystemLogKey} from "solpp/zksync/interfaces/IExecutor.sol";
 
 bytes32 constant DEFAULT_L2_LOGS_TREE_ROOT_HASH = 0x0000000000000000000000000000000000000000000000000000000000000000;
 address constant L2_SYSTEM_CONTEXT_ADDRESS = 0x000000000000000000000000000000000000800B;
@@ -83,6 +83,36 @@ library Utils {
         return logs;
     }
 
+    function createStoredBatchInfo() public pure returns (IExecutor.StoredBatchInfo memory) {
+        return
+            IExecutor.StoredBatchInfo({
+                batchNumber: 0,
+                batchHash: bytes32(""),
+                indexRepeatedStorageChanges: 0,
+                numberOfLayer1Txs: 0,
+                priorityOperationsHash: keccak256(""),
+                l2LogsTreeRoot: DEFAULT_L2_LOGS_TREE_ROOT_HASH,
+                timestamp: 0,
+                commitment: bytes32("")
+            });
+    }
+
+    function createCommitBatchInfo() public view returns (IExecutor.CommitBatchInfo memory) {
+        return
+            IExecutor.CommitBatchInfo({
+                batchNumber: 1,
+                timestamp: uint64(uint256(randomBytes32("timestamp"))),
+                indexRepeatedStorageChanges: 0,
+                newStateRoot: randomBytes32("newStateRoot"),
+                numberOfLayer1Txs: 0,
+                priorityOperationsHash: keccak256(""),
+                bootloaderHeapInitialContentsHash: randomBytes32("bootloaderHeapInitialContentsHash"),
+                eventsQueueStateHash: randomBytes32("eventsQueueStateHash"),
+                systemLogs: abi.encode(randomBytes32("systemLogs")),
+                pubdataCommitments: abi.encodePacked(uint256(0))
+            });
+    }
+
     function encodePacked(bytes[] memory data) public pure returns (bytes memory) {
         bytes memory result;
         for (uint256 i = 0; i < data.length; i++) {

l1-contracts/test/foundry/unit/concrete/ValidatorTimelock.t.sol

@@ -0,0 +1,123 @@
+// SPDX-License-Identifier: MIT
+pragma solidity 0.8.20;
+
+import {Test} from "forge-std/Test.sol";
+import {Utils} from "./Utils/Utils.sol";
+import {ValidatorTimelock, IExecutor} from "solpp/zksync/ValidatorTimelock.sol";
+
+contract ValidatorTimelockTest is Test {
+    /// @notice Event emitted from ValidatorTimelock when new validator is added
+    event ValidatorAdded(address _addedValidator);
+
+    /// @notice Event emitted from ValidatorTimelock when new validator is removed
+    event ValidatorRemoved(address _removedValidator);
+
+    /// @notice Error for when an address is already a validator.
+    error AddressAlreadyValidator();
+
+    /// @notice Error for when an address is not a validator.
+    error ValidatorDoesNotExist();
+
+    ValidatorTimelock validator;
+
+    address owner;
+    address zkSync;
+    address alice;
+    address bob;
+
+    function setUp() public {
+        owner = makeAddr("owner");
+        zkSync = makeAddr("zkSync");
+        alice = makeAddr("alice");
+        bob = makeAddr("bob");
+
+        address[] memory initValidators = new address[](1);
+        initValidators[0] = alice;
+
+        validator = new ValidatorTimelock(owner, zkSync, 10, initValidators);
+    }
+
+    function test_addValidator() public {
+        assert(validator.validators(bob) == false);
+
+        vm.prank(owner);
+        vm.expectEmit(true, true, true, true, address(validator));
+        emit ValidatorAdded(bob);
+        validator.addValidator(bob);
+
+        assert(validator.validators(bob) == true);
+    }
+
+    function test_removeValidator() public {
+        vm.prank(owner);
+        validator.addValidator(bob);
+        assert(validator.validators(bob) == true);
+
+        vm.prank(owner);
+        vm.expectEmit(true, true, true, true, address(validator));
+        emit ValidatorRemoved(bob);
+        validator.removeValidator(bob);
+
+        assert(validator.validators(bob) == false);
+    }
+
+    function test_validatorCanMakeCall() public {
+        // Setup Mock call to executor
+        vm.mockCall(zkSync, abi.encodeWithSelector(IExecutor.commitBatches.selector), "");
+
+        IExecutor.StoredBatchInfo memory storedBatch = Utils.createStoredBatchInfo();
+        IExecutor.CommitBatchInfo memory batchToCommit = Utils.createCommitBatchInfo();
+
+        IExecutor.CommitBatchInfo[] memory batchesToCommit = new IExecutor.CommitBatchInfo[](1);
+        batchesToCommit[0] = batchToCommit;
+
+        vm.prank(alice);
+        validator.commitBatches(storedBatch, batchesToCommit);
+    }
+
+    function test_addValidator_revertWhenNotOwner() public {
+        assert(validator.validators(bob) == false);
+
+        vm.expectRevert("Ownable: caller is not the owner");
+        validator.addValidator(bob);
+
+        assert(validator.validators(bob) == false);
+    }
+
+    function test_removeValidator_revertWhenNotOwner() public {
+        assert(validator.validators(alice) == true);
+
+        vm.expectRevert("Ownable: caller is not the owner");
+        validator.removeValidator(alice);
+
+        assert(validator.validators(alice) == true);
+    }
+
+    function test_addValidator_revertWhenAddressAlreadyValidator() public {
+        assert(validator.validators(alice) == true);
+
+        vm.prank(owner);
+        vm.expectRevert(AddressAlreadyValidator.selector);
+        validator.addValidator(alice);
+    }
+
+    function test_removeValidator_revertWhenAddressNotValidator() public {
+        assert(validator.validators(bob) == false);
+
+        vm.prank(owner);
+        vm.expectRevert(ValidatorDoesNotExist.selector);
+        validator.removeValidator(bob);
+    }
+
+    function test_validatorCanMakeCall_revertWhenNotValidator() public {
+        IExecutor.StoredBatchInfo memory storedBatch = Utils.createStoredBatchInfo();
+        IExecutor.CommitBatchInfo memory batchToCommit = Utils.createCommitBatchInfo();
+
+        IExecutor.CommitBatchInfo[] memory batchesToCommit = new IExecutor.CommitBatchInfo[](1);
+        batchesToCommit[0] = batchToCommit;
+
+        vm.prank(bob);
+        vm.expectRevert(bytes("8h"));
+        validator.commitBatches(storedBatch, batchesToCommit);
+    }
+}

l1-contracts/test/unit_tests/validator_timelock_test.spec.ts

@@ -57,7 +57,7 @@ describe("ValidatorTimelock tests", function () {
       await owner.getAddress(),
       dummyExecutor.address,
       0,
-      ethers.constants.AddressZero
+      [ethers.constants.AddressZero]
     );
     validatorTimelock = ValidatorTimelockFactory.connect(
       validatorTimelockContract.address,
@@ -99,7 +99,7 @@ describe("ValidatorTimelock tests", function () {
 
   it("Should revert if non-owner sets validator", async () => {
     const revertReason = await getCallRevertReason(
-      validatorTimelock.connect(randomSigner).setValidator(await randomSigner.getAddress())
+      validatorTimelock.connect(randomSigner).addValidator(await randomSigner.getAddress())
     );
 
     expect(revertReason).equal("Ownable: caller is not the owner");
@@ -113,9 +113,9 @@ describe("ValidatorTimelock tests", function () {
 
   it("Should successfully set the validator", async () => {
     const validatorAddress = await validator.getAddress();
-    await validatorTimelock.connect(owner).setValidator(validatorAddress);
+    await validatorTimelock.connect(owner).addValidator(validatorAddress);
 
-    expect(await validatorTimelock.validator()).equal(validatorAddress);
+    expect(await validatorTimelock.validators(validatorAddress)).equal(true);
   });
 
   it("Should successfully set the execution delay", async () => {