feat(ctb): Start with a delay of zero and forward instantly

This allows us to include this contract in the initial deployment without having to

swap it in later.

feat(ctb): Add missing annotations

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

@@ -4,6 +4,9 @@ pragma solidity 0.8.15;
 import { ISemver } from "src/universal/ISemver.sol";
 
 contract DelayedVetoable is ISemver {
+    /// @notice Error for when the delay has already been set.
+    error AlreadyDelayed();
+
     /// @notice Error for when attempting to forward too early.
     error ForwardingEarly();
 
@@ -13,6 +16,10 @@ contract DelayedVetoable is ISemver {
     /// @notice Error for unauthorized calls.
     error Unauthorized(address expected, address actual);
 
+    /// @notice An event that is emitted when the delay is activated.
+    /// @param delay The delay that was activated.
+    event DelayActivated(uint256 delay);
+
     /// @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.
@@ -37,12 +44,15 @@ contract DelayedVetoable is ISemver {
     /// @notice The address that can initiate a call.
     address internal immutable _initiator;
 
+    /// @notice The current amount of time to wait before forwarding a call.
+    uint256 internal _delay;
+
+    /// @notice The delay which will be set after the initial system deployment is completed.
+    uint256 internal immutable _operatingDelay;
+
     /// @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
@@ -64,12 +74,13 @@ contract DelayedVetoable is ISemver {
     /// @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_) {
+    /// @param operatingDelay_ Time to delay when the system is operational.
+    constructor(address vetoer_, address initiator_, address target_, uint256 operatingDelay_) {
         _vetoer = vetoer_;
         _initiator = initiator_;
         _target = target_;
-        _delay = delay_;
+        _delay = 0;
+        _operatingDelay = operatingDelay_;
     }
 
     /// @notice Gets the initiator
@@ -96,6 +107,13 @@ contract DelayedVetoable is ISemver {
         return _delay;
     }
 
+    /// @notice Gets entries in the _queuedAt mapping.
+    /// @param callHash The hash of the call data.
+    /// @return The time the callHash was recorded.
+    function queuedAt(bytes32 callHash) external readOrHandle returns (uint256) {
+        return _queuedAt[callHash];
+    }
+
     /// @notice Used for all calls that pass data to the contract.
     fallback() external {
         _handleCall();
@@ -105,10 +123,24 @@ contract DelayedVetoable is ISemver {
     ///         This enables transparent initiation and forwarding of calls to the target and avoids
     ///         the need for additional layers of abi encoding.
     function _handleCall() internal {
+        // The initiator and vetoer activate the delay by passing in null data.
+        if (msg.data.length == 0) {
+            if (msg.sender != _initiator && msg.sender != _vetoer) {
+                // todo(maurelian): make this error have an expected array.
+                revert Unauthorized(_initiator, msg.sender);
+            }
+            _activateDelay();
+            return;
+        }
+
         bytes32 callHash = keccak256(msg.data);
 
         // Case 1: The initiator is calling the contract to initiate a call.
         if (msg.sender == _initiator && _queuedAt[callHash] == 0) {
+            if (_delay == 0) {
+                // This forward function will halt the call frame on completion.
+                _forwardAndHalt(callHash, msg.data);
+            }
             _queuedAt[callHash] = block.timestamp;
             emit Initiated(callHash, msg.data);
             return;
@@ -135,7 +167,11 @@ contract DelayedVetoable is ISemver {
 
         // Delete the call to prevent replays
         delete _queuedAt[callHash];
+        _forwardAndHalt(callHash, msg.data);
+    }
 
+    /// @notice Forwards the call to the target and halts the call frame.
+    function _forwardAndHalt(bytes32 callHash, bytes memory data) internal {
         // Forward the call
         emit Forwarded(callHash, msg.data);
         (bool success,) = _target.call(msg.data);
@@ -147,4 +183,11 @@ contract DelayedVetoable is ISemver {
             return(0x0, returndatasize())
         }
     }
+
+    /// @notice Sets the delay to the operating delay.
+    function _activateDelay() internal {
+        if (_delay != 0) revert AlreadyDelayed();
+        _delay = _operatingDelay;
+        emit DelayActivated(_delay);
+    }
 }

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

@@ -15,7 +15,7 @@ contract DelayedVetoable_Init is CommonTest {
     address target = address(0xabba);
     address initiator = alice;
     address vetoer = bob;
-    uint256 delay = 14 days;
+    uint256 operatingDelay = 14 days;
     DelayedVetoable delayedVetoable;
     Reverter reverter;
 
@@ -25,8 +25,14 @@ contract DelayedVetoable_Init is CommonTest {
             initiator_: alice,
             vetoer_: bob,
             target_: address(target),
-            delay_: delay
+            operatingDelay_: operatingDelay
         });
+        // Most tests will use the operating delay, so we call as the initiator with null data
+        // to set the delay. For tests that need to use the initial zero delay, we'll modify the
+        // value in storage.
+        vm.prank(initiator);
+        (bool success,) = address(delayedVetoable).call(hex"");
+
         reverter = new Reverter();
     }
 }
@@ -37,7 +43,7 @@ contract DelayedVetoable_Getters_Test is DelayedVetoable_Init {
         assertEq(delayedVetoable.initiator(), initiator);
         assertEq(delayedVetoable.vetoer(), vetoer);
         assertEq(delayedVetoable.target(), target);
-        assertEq(delayedVetoable.delay(), delay);
+        assertEq(delayedVetoable.delay(), operatingDelay);
     }
 }
 
@@ -57,7 +63,12 @@ contract DelayedVetoable_Getters_TestFail is DelayedVetoable_Init {
 }
 
 contract DelayedVetoable_HandleCall_Test is DelayedVetoable_Init {
+    /// @dev A call can be initiated by the initiator.
     function testFuzz_handleCall_initiation_succeeds(bytes memory data) external {
+        // Calls with no data are used to activate the delay and are not valid otherwise,
+        // so we assume a non-zero data value.
+        vm.assume(data.length > 0);
+
         vm.expectEmit(true, false, false, true, address(delayedVetoable));
         emit Initiated(keccak256(data), data);
 
@@ -66,12 +77,32 @@ contract DelayedVetoable_HandleCall_Test is DelayedVetoable_Init {
         assert(success);
     }
 
-    function testFuzz_handleCall_forwarding_succeeds(bytes memory data) external {
-        // Initiate the call
+    /// @dev The delay is inititially set to zero and the call is immediately forwarded.
+    function testFuzz_handleCall_initialForwardingImmediately_succeeds(bytes memory data) external {
+        // Calls with no data are used to activate the delay and are not valid otherwise,
+        // so we assume a non-zero data value.
+        vm.assume(data.length > 0);
+
+        // Reset the delay to zero
+        vm.store(address(delayedVetoable), bytes32(uint256(0)), bytes32(uint256(0)));
+
+        vm.prank(initiator);
+        vm.expectEmit(true, false, false, true, address(delayedVetoable));
+        vm.expectCall({ callee: target, data: data });
+        emit Forwarded(keccak256(data), data);
+        (bool success,) = address(delayedVetoable).call(data);
+        assert(success);
+    }
+
+    /// @dev The delay can be activated by the vetoer or initiator, and are not forwarded until the delay has passed
+    ///      once activated.
+    function testFuzz_handleCall_forwardingWithDelay_succeeds(bytes memory data) external {
+        vm.assume(data.length > 0);
         vm.prank(initiator);
+        // it's immediately forwarding for some reason.
         (bool success,) = address(delayedVetoable).call(data);
 
-        vm.warp(block.timestamp + delay);
+        vm.warp(block.timestamp + operatingDelay);
         vm.expectEmit(true, false, false, true, address(delayedVetoable));
         emit Forwarded(keccak256(data), data);
 
@@ -82,12 +113,14 @@ contract DelayedVetoable_HandleCall_Test is DelayedVetoable_Init {
 }
 
 contract DelayedVetoable_HandleCall_TestFail is DelayedVetoable_Init {
+    /// @dev The delay is inititially set to zero and the call is immediately forwarded.
     function test_handleCall_unauthorizedInitiation_reverts() external {
         vm.expectRevert(abi.encodeWithSelector(Unauthorized.selector, initiator, address(this)));
-        (bool success,) = address(delayedVetoable).call(hex"");
+        (bool success,) = address(delayedVetoable).call(NON_ZERO_DATA);
         assert(success);
     }
 
+    /// @dev The call cannot be forewarded until the delay has passed.
     function test_handleCall_forwardingTooSoon_reverts(bytes memory data) external {
         vm.prank(initiator);
         (bool success,) = address(delayedVetoable).call(data);
@@ -97,15 +130,17 @@ contract DelayedVetoable_HandleCall_TestFail is DelayedVetoable_Init {
         assertFalse(success);
     }
 
+    /// @dev The call cannot be forwarded a second time.
     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.warp(block.timestamp + operatingDelay);
         vm.expectEmit(true, false, false, true, address(delayedVetoable));
         emit Forwarded(keccak256(data), data);
 
+        // Forward the call
         vm.expectCall({ callee: target, data: data });
         (success,) = address(delayedVetoable).call(data);
         assert(success);
@@ -116,13 +151,14 @@ contract DelayedVetoable_HandleCall_TestFail is DelayedVetoable_Init {
         assert(success);
     }
 
+    /// @dev If the target reverts, it is bubbled up.
     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.warp(block.timestamp + operatingDelay);
         vm.expectEmit(true, false, false, true, address(delayedVetoable));
         emit Forwarded(keccak256(data), data);