feat(ctb): Add vetoer and initiator roles

feat(ctb): Add getters to DelayedVetoable

packages/contracts-bedrock/.gas-snapshot

@@ -46,7 +46,14 @@ CrossDomainOwnable3_Test:test_transferOwnership_zeroAddress_reverts() (gas: 1208
 CrossDomainOwnableThroughPortal_Test:test_depositTransaction_crossDomainOwner_succeeds() (gas: 81417)
 CrossDomainOwnable_Test:test_onlyOwner_notOwner_reverts() (gas: 10597)
 CrossDomainOwnable_Test:test_onlyOwner_succeeds() (gas: 34883)
-DelayedVetoable_HandleCall_TestFail:test_handleCall_reverts() (gas: 24969)
+DelayedVetoable_Getters_Test:test_getters() (gas: 28128)
+DelayedVetoable_Getters_TestFail:test_getters_notVetoer() (gas: 52018)
+DelayedVetoable_HandleCall_TestFail:test_handleCall_forwardingTargetReverts_reverts(bytes) (runs: 256, μ: 59701, ~: 59067)
+DelayedVetoable_HandleCall_TestFail:test_handleCall_forwardingTooSoon_reverts(bytes) (runs: 256, μ: 49589, ~: 49170)
+DelayedVetoable_HandleCall_TestFail:test_handleCall_targetIsZero_reverts() (gas: 17599)
+DelayedVetoable_HandleCall_TestFail:test_handleCall_unAuthorizedInitiation_reverts() (gas: 6627)
+DelayedVetoable_Veto_Test:test_veto_succeeds(bytes) (runs: 256, μ: 21411, ~: 20927)
+DelayedVetoable_Veto_TestFail:test_veto_notVetoer_reverts() (gas: 17552)
 DeleteOutput:test_script_succeeds() (gas: 3100)
 DeployerWhitelist_Test:test_owner_succeeds() (gas: 7582)
 DeployerWhitelist_Test:test_storageSlots_succeeds() (gas: 33395)

packages/contracts-bedrock/src/universal/DelayedVetoable.sol

@@ -11,6 +11,9 @@ contract DelayedVetoable {
     /// @notice Error for the target is not set.
     error TargetUnitialized();
 
+    /// @notice Error for unauthorized calls.
+    error Unauthorized(address expected, address actual);
+
     /// @notice An event that is emitted when a call is initiated.
     /// @param callHash The hash of the call data.
     /// @param data The data of the initiated call.
@@ -21,22 +24,82 @@ contract DelayedVetoable {
     /// @param data The data forwarded to the target.
     event Forwarded(bytes32 indexed callHash, bytes data);
 
+    /// @notice An event that is emitted each time a call is vetoed.
+    /// @param callHash The hash of the call data.
+    /// @param data The data forwarded to the target.
+    event Vetoed(bytes32 indexed callHash, bytes data);
+
     /// @notice The address that all calls are forwarded to after the delay.
     address internal _target;
 
+    // TODO(maurelian): move this to the new SuperChainConfig contract
+    /// @notice The address that can veto a call.
+    address internal _vetoer;
+
+    // TODO(maurelian): move this to the new SuperChainConfig contract
+    /// @notice The address that can initiate a call.
+    address internal _initiator;
+
     /// @notice The time that a call was initiated.
     mapping(bytes32 => uint256) internal _queuedAt;
 
     /// @notice The time to wait before forwarding a call.
     uint256 internal _delay;
 
+    /// @notice A modifier that reverts if not called by the vetoer or by address(0) to allow
+    ///         eth_call to interact with this proxy without needing to use low-level storage
+    ///         inspection. We assume that nobody is able to trigger calls from address(0) during
+    ///         normal EVM execution.
+    modifier handleCallIfNotVetoer() {
+        if (msg.sender == _vetoer || msg.sender == address(0)) {
+            _;
+        } else {
+            // This WILL halt the call frame on completion.
+            _handleCall();
+        }
+    }
+
     /// @notice Sets the target admin during contract deployment.
-    /// @param target Address of the target.
-    constructor(address target, uint256 delay) {
-        _target = target;
-        _delay = delay;
+    /// @param vetoer_ Address of the vetoer.
+    /// @param initiator_ Address of the initiator.
+    /// @param target_ Address of the target.
+    /// @param delay_ Address of the delay.
+    constructor(address vetoer_, address initiator_, address target_, uint256 delay_) {
+        _vetoer = vetoer_;
+        _initiator = initiator_;
+        _target = target_;
+        _delay = delay_;
+    }
+
+    /// @notice Gets the initiator
+    /// @return Initiator address.
+    function initiator() external handleCallIfNotVetoer returns (address) {
+        return _initiator;
     }
 
+    //// @notice Queries the vetoer address.
+    /// @return Vetoer address.
+    function vetoer() external handleCallIfNotVetoer returns (address) {
+        return _vetoer;
+    }
+
+    //// @notice Queries the target address.
+    /// @return Target address.
+    function target() external handleCallIfNotVetoer returns (address) {
+        return _target;
+    }
+
+    /// @notice Gets the delay
+    /// @return Delay address.
+    function delay() external handleCallIfNotVetoer returns (uint256) {
+        return _delay;
+    }
+
+    // TODO(maurelian): Remove this? The contract currently cannot handle forwarding ETH and I'm
+    //   not sure the complexity is warranted.
+    //   If we do allow it:
+    //      1. the callHash will need to include the value
+    //      2. forwarding will need to be done by passing the callHash, rather than the unhashed data
     /// @notice Used when no data is passed to the contract.
     receive() external payable {
         _handleCall();
@@ -47,7 +110,18 @@ contract DelayedVetoable {
         _handleCall();
     }
 
-    /// @notice Handles forwards the call to the target.
+    /// @notice Vetoes a call. This method can only be called by the vetoer. If called by another
+    ///         address, execution will be redirected to _handleCall()
+    function veto(bytes memory data) external handleCallIfNotVetoer {
+        bytes32 callHash = keccak256(data);
+
+        delete _queuedAt[callHash];
+        emit Vetoed(callHash, data);
+    }
+
+    /// @notice Receives all calls other than those made by the vetoer.
+    ///         This enables transparent initiation and forwarding of calls to the target and avoids
+    ///         the need for additional layers of abi encoding.
     function _handleCall() internal {
         if (_target == address(0)) {
             revert TargetUnitialized();
@@ -55,13 +129,18 @@ contract DelayedVetoable {
 
         bytes32 callHash = keccak256(msg.data);
         if (_queuedAt[callHash] == 0) {
+            if (msg.sender != _initiator) {
+                revert Unauthorized(_initiator, msg.sender);
+            }
             _queuedAt[callHash] = block.timestamp;
             emit Initiated(callHash, msg.data);
         } else if (_queuedAt[callHash] + _delay < block.timestamp) {
             // Not enough time has passed, so we'll revert.
             revert ForwardingEarly();
         } else {
-            // sufficient time has passed.
+            // The ability to finalize the call after sufficient time has passed does not require
+            // authorization.
+
             // Delete the call to prevent replays
             delete _queuedAt[callHash];
 

packages/contracts-bedrock/test/DelayedVetoable.t.sol

@@ -5,10 +5,17 @@ import { CommonTest, Reverter } from "./CommonTest.t.sol";
 import { DelayedVetoable } from "../src/universal/DelayedVetoable.sol";
 
 contract DelayedVetoable_Init is CommonTest {
+    error Unauthorized(address expected, address actual);
+    error ForwardingEarly();
+    error TargetUnitialized();
+
     event Initiated(bytes32 indexed callHash, bytes data);
     event Forwarded(bytes32 indexed callHash, bytes data);
+    event Vetoed(bytes32 indexed callHash, bytes data);
 
     address target = address(0xabba);
+    address initiator = alice;
+    address vetoer = bob;
     uint256 delay = 14 days;
     DelayedVetoable delayedVetoable;
     Reverter reverter;
@@ -16,24 +23,53 @@ contract DelayedVetoable_Init is CommonTest {
     function setUp() public override {
         super.setUp();
         delayedVetoable = new DelayedVetoable({
-            target: address(target),
-            delay: delay
+            initiator_: alice,
+            vetoer_: bob,
+            target_: address(target),
+            delay_: delay
         });
         reverter = new Reverter();
     }
 }
 
+contract DelayedVetoable_Getters_Test is DelayedVetoable_Init {
+    function test_getters() external {
+        vm.startPrank(address(0));
+        assertEq(delayedVetoable.initiator(), initiator);
+        assertEq(delayedVetoable.vetoer(), vetoer);
+        assertEq(delayedVetoable.target(), target);
+        assertEq(delayedVetoable.delay(), delay);
+    }
+}
+
+contract DelayedVetoable_Getters_TestFail is DelayedVetoable_Init {
+    function test_getters_notVetoer() external {
+        // getter calls from addresses other than the vetoer or zero address will revert in the
+        // initiation branch of the proxy.
+        vm.expectRevert(abi.encodeWithSelector(Unauthorized.selector, initiator, address(this)));
+        delayedVetoable.initiator();
+        vm.expectRevert(abi.encodeWithSelector(Unauthorized.selector, initiator, address(this)));
+        delayedVetoable.vetoer();
+        vm.expectRevert(abi.encodeWithSelector(Unauthorized.selector, initiator, address(this)));
+        delayedVetoable.target();
+        vm.expectRevert(abi.encodeWithSelector(Unauthorized.selector, initiator, address(this)));
+        delayedVetoable.delay();
+    }
+}
+
 contract DelayedVetoable_HandleCall_Test is DelayedVetoable_Init {
     function testFuzz_handleCall_initiation_succeeds(bytes memory data) external {
         vm.expectEmit(true, false, false, true, address(delayedVetoable));
         emit Initiated(keccak256(data), data);
 
+        vm.prank(initiator);
         (bool success,) = address(delayedVetoable).call(data);
         assert(success);
     }
 
     function testFuzz_handleCall_forwarding_succeeds(bytes memory data) external {
         // Initiate the call
+        vm.prank(initiator);
         (bool success,) = address(delayedVetoable).call(data);
 
         vm.warp(block.timestamp + delay);
@@ -47,25 +83,75 @@ contract DelayedVetoable_HandleCall_Test is DelayedVetoable_Init {
 }
 
 contract DelayedVetoable_HandleCall_TestFail is DelayedVetoable_Init {
+    function test_handleCall_unAuthorizedInitiation_reverts() external {
+        vm.store(address(delayedVetoable), bytes32(0), bytes32(0));
+        vm.expectRevert(abi.encodeWithSelector(TargetUnitialized.selector));
+        (bool success,) = address(delayedVetoable).call(hex"");
+        assert(success);
+    }
+
+    function test_handleCall_targetIsZero_reverts() external {
+        vm.expectRevert(abi.encodeWithSelector(Unauthorized.selector, initiator, address(this)));
+        (bool success,) = address(delayedVetoable).call(hex"");
+        assert(success);
+    }
+
     function test_handleCall_forwardingTooSoon_reverts(bytes memory data) external {
+        vm.prank(initiator);
         (bool success,) = address(delayedVetoable).call(data);
 
-        vm.expectRevert();
+        vm.expectRevert(abi.encodeWithSelector(ForwardingEarly.selector));
         (success,) = address(delayedVetoable).call(data);
         assertFalse(success);
     }
 
+    function test_handleCall_forwardingTwice_reverts(bytes memory data) external {
+        // Initiate the call
+        vm.prank(initiator);
+        (bool success,) = address(delayedVetoable).call(data);
+
+        vm.warp(block.timestamp + delay);
+        vm.expectEmit(true, false, false, true, address(delayedVetoable));
+        emit Forwarded(keccak256(data), data);
+
+        vm.expectCall({ callee: target, data: data });
+        (success,) = address(delayedVetoable).call(data);
+        assert(success);
+
+        // Attempt to foward the same call again.
+        vm.expectRevert(abi.encodeWithSelector(Unauthorized.selector, initiator, address(this)));
+        (success,) = address(delayedVetoable).call(data);
+        assert(success);
+    }
+
     function test_handleCall_forwardingTargetReverts_reverts(bytes memory data) external {
         vm.etch(target, address(reverter).code);
 
+        vm.prank(initiator);
         (bool success,) = address(delayedVetoable).call(data);
 
         vm.warp(block.timestamp + delay);
         vm.expectEmit(true, false, false, true, address(delayedVetoable));
         emit Forwarded(keccak256(data), data);
 
-        vm.expectCall({ callee: target, data: data });
         (success,) = address(delayedVetoable).call(data);
         assertFalse(success);
     }
 }
+
+contract DelayedVetoable_Veto_Test is DelayedVetoable_Init {
+    function test_veto_succeeds(bytes memory data) external {
+        vm.expectEmit(true, false, false, true, address(delayedVetoable));
+        emit Vetoed(keccak256(data), data);
+
+        vm.prank(vetoer);
+        delayedVetoable.veto(data);
+    }
+}
+
+contract DelayedVetoable_Veto_TestFail is DelayedVetoable_Init {
+    function test_veto_notVetoer_reverts() external {
+        vm.expectRevert();
+        delayedVetoable.veto(hex"");
+    }
+}