contracts-bedrock: Fix enforcement of delay on DelayedVetoable (#9050)

* contracts-bedrock: Fix enforcement of delay on DelayedVetoable

Prior to this change, a bug existed in the DelayedVetoable contract,
which resulted in anyone being able to execute a transaction as soon as
it had been initiated.

A factor in this was an edge case behavior in foundry's vm.expectRevert
which flips the success value of a low level call. This commit also
introduces a new convention when expectRevert is used on low level calls
to make this edge case behavior more evident.

* contracts-bedrock: Add revertsAsExpected to Style Guide

* contracts: Fix compiler warnings

* bindings and semver

op-bindings/bindings/delayedvetoable_more.go

@@ -13,7 +13,7 @@ const DelayedVetoableStorageLayoutJSON = "{\"storage\":[{\"astId\":1000,\"contra
 
 var DelayedVetoableStorageLayout = new(solc.StorageLayout)
 
-var DelayedVetoableDeployedBin = "0x608060405234801561001057600080fd5b50600436106100725760003560e01c8063b912de5d11610050578063b912de5d14610111578063d4b8399214610124578063d8bff4401461012c57610072565b806354fd4d501461007c5780635c39fcc1146100ce5780636a42b8f8146100fb575b61007a610134565b005b6100b86040518060400160405280600581526020017f312e302e3000000000000000000000000000000000000000000000000000000081525081565b6040516100c591906106a7565b60405180910390f35b6100d66104fb565b60405173ffffffffffffffffffffffffffffffffffffffff90911681526020016100c5565b610103610532565b6040519081526020016100c5565b61010361011f36600461071a565b610540565b6100d6610567565b6100d6610593565b361580156101425750600054155b15610298573373ffffffffffffffffffffffffffffffffffffffff7f000000000000000000000000000000000000000000000000000000000000000016148015906101c357503373ffffffffffffffffffffffffffffffffffffffff7f00000000000000000000000000000000000000000000000000000000000000001614155b1561023d576040517f295a81c100000000000000000000000000000000000000000000000000000000815273ffffffffffffffffffffffffffffffffffffffff7f00000000000000000000000000000000000000000000000000000000000000001660048201523360248201526044015b60405180910390fd5b7f000000000000000000000000000000000000000000000000000000000000000060008190556040519081527febf28bfb587e28dfffd9173cf71c32ba5d3f0544a0117b5539c9b274a5bba2a89060200160405180910390a1565b600080366040516102aa929190610733565b60405190819003902090503373ffffffffffffffffffffffffffffffffffffffff7f0000000000000000000000000000000000000000000000000000000000000000161480156103065750600081815260016020526040902054155b1561036c5760005460000361031e5761031e816105bf565b6000818152600160205260408082204290555182917f87a332a414acbc7da074543639ce7ae02ff1ea72e88379da9f261b080beb5a139161036191903690610743565b60405180910390a250565b3373ffffffffffffffffffffffffffffffffffffffff7f0000000000000000000000000000000000000000000000000000000000000000161480156103be575060008181526001602052604090205415155b15610406576000818152600160205260408082208290555182917fbede6852c1d97d93ff557f676de76670cd0dec861e7fe8beb13aa0ba2b0ab0409161036191903690610743565b600081815260016020526040812054900361048b576040517f295a81c100000000000000000000000000000000000000000000000000000000815273ffffffffffffffffffffffffffffffffffffffff7f0000000000000000000000000000000000000000000000000000000000000000166004820152336024820152604401610234565b60008054828252600160205260409091205442916104a891610790565b10156104e0576040517f43dc986d00000000000000000000000000000000000000000000000000000000815260040160405180910390fd5b6000818152600160205260408120556104f8816105bf565b50565b60003361052757507f000000000000000000000000000000000000000000000000000000000000000090565b61052f610134565b90565b600033610527575060005490565b60003361055a575060009081526001602052604090205490565b610562610134565b919050565b60003361052757507f000000000000000000000000000000000000000000000000000000000000000090565b60003361052757507f000000000000000000000000000000000000000000000000000000000000000090565b807f4c109d85bcd0bb5c735b4be850953d652afe4cd9aa2e0b1426a65a4dcb2e12296000366040516105f2929190610743565b60405180910390a26000807f000000000000000000000000000000000000000000000000000000000000000073ffffffffffffffffffffffffffffffffffffffff16600036604051610645929190610733565b6000604051808303816000865af19150503d8060008114610682576040519150601f19603f3d011682016040523d82523d6000602084013e610687565b606091505b50909250905081151560010361069f57805160208201f35b805160208201fd5b600060208083528351808285015260005b818110156106d4578581018301518582016040015282016106b8565b818111156106e6576000604083870101525b50601f017fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffe016929092016040019392505050565b60006020828403121561072c57600080fd5b5035919050565b8183823760009101908152919050565b60208152816020820152818360408301376000818301604090810191909152601f9092017fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffe0160101919050565b600082198211156107ca577f4e487b7100000000000000000000000000000000000000000000000000000000600052601160045260246000fd5b50019056fea164736f6c634300080f000a"
+var DelayedVetoableDeployedBin = "0x608060405234801561001057600080fd5b50600436106100725760003560e01c8063b912de5d11610050578063b912de5d14610111578063d4b8399214610124578063d8bff4401461012c57610072565b806354fd4d501461007c5780635c39fcc1146100ce5780636a42b8f8146100fb575b61007a610134565b005b6100b86040518060400160405280600581526020017f312e302e3000000000000000000000000000000000000000000000000000000081525081565b6040516100c591906106a7565b60405180910390f35b6100d66104fb565b60405173ffffffffffffffffffffffffffffffffffffffff90911681526020016100c5565b610103610532565b6040519081526020016100c5565b61010361011f36600461071a565b610540565b6100d6610567565b6100d6610593565b361580156101425750600054155b15610298573373ffffffffffffffffffffffffffffffffffffffff7f000000000000000000000000000000000000000000000000000000000000000016148015906101c357503373ffffffffffffffffffffffffffffffffffffffff7f00000000000000000000000000000000000000000000000000000000000000001614155b1561023d576040517f295a81c100000000000000000000000000000000000000000000000000000000815273ffffffffffffffffffffffffffffffffffffffff7f00000000000000000000000000000000000000000000000000000000000000001660048201523360248201526044015b60405180910390fd5b7f000000000000000000000000000000000000000000000000000000000000000060008190556040519081527febf28bfb587e28dfffd9173cf71c32ba5d3f0544a0117b5539c9b274a5bba2a89060200160405180910390a1565b600080366040516102aa929190610733565b60405190819003902090503373ffffffffffffffffffffffffffffffffffffffff7f0000000000000000000000000000000000000000000000000000000000000000161480156103065750600081815260016020526040902054155b1561036c5760005460000361031e5761031e816105bf565b6000818152600160205260408082204290555182917f87a332a414acbc7da074543639ce7ae02ff1ea72e88379da9f261b080beb5a139161036191903690610743565b60405180910390a250565b3373ffffffffffffffffffffffffffffffffffffffff7f0000000000000000000000000000000000000000000000000000000000000000161480156103be575060008181526001602052604090205415155b15610406576000818152600160205260408082208290555182917fbede6852c1d97d93ff557f676de76670cd0dec861e7fe8beb13aa0ba2b0ab0409161036191903690610743565b600081815260016020526040812054900361048b576040517f295a81c100000000000000000000000000000000000000000000000000000000815273ffffffffffffffffffffffffffffffffffffffff7f0000000000000000000000000000000000000000000000000000000000000000166004820152336024820152604401610234565b60008054828252600160205260409091205442916104a891610790565b11156104e0576040517f43dc986d00000000000000000000000000000000000000000000000000000000815260040160405180910390fd5b6000818152600160205260408120556104f8816105bf565b50565b60003361052757507f000000000000000000000000000000000000000000000000000000000000000090565b61052f610134565b90565b600033610527575060005490565b60003361055a575060009081526001602052604090205490565b610562610134565b919050565b60003361052757507f000000000000000000000000000000000000000000000000000000000000000090565b60003361052757507f000000000000000000000000000000000000000000000000000000000000000090565b807f4c109d85bcd0bb5c735b4be850953d652afe4cd9aa2e0b1426a65a4dcb2e12296000366040516105f2929190610743565b60405180910390a26000807f000000000000000000000000000000000000000000000000000000000000000073ffffffffffffffffffffffffffffffffffffffff16600036604051610645929190610733565b6000604051808303816000865af19150503d8060008114610682576040519150601f19603f3d011682016040523d82523d6000602084013e610687565b606091505b50909250905081151560010361069f57805160208201f35b805160208201fd5b600060208083528351808285015260005b818110156106d4578581018301518582016040015282016106b8565b818111156106e6576000604083870101525b50601f017fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffe016929092016040019392505050565b60006020828403121561072c57600080fd5b5035919050565b8183823760009101908152919050565b60208152816020820152818360408301376000818301604090810191909152601f9092017fffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffe0160101919050565b600082198211156107ca577f4e487b7100000000000000000000000000000000000000000000000000000000600052601160045260246000fd5b50019056fea164736f6c634300080f000a"
 
 
 func init() {

packages/contracts-bedrock/STYLE_GUIDE.md

@@ -118,6 +118,11 @@ These guidelines are also encoded in a script which can be run with:
 tsx scripts/forge-test-names.ts
 ```
 
+#### Expect Revert with Low Level Calls
+
+There is a non-intuitive behavior in foundry tests, which is documented [here](https://book.getfoundry.sh/cheatcodes/expect-revert?highlight=expectrevert#expectrevert).
+When testing for a revert on a low-level call, please use the `revertsAsExpected` pattern suggested there.
+
 _Note: This is a work in progress, not all test files are compliant with these guidelines._
 
 #### Organizing Principles

packages/contracts-bedrock/semver-lock.json

@@ -8,8 +8,8 @@
     "sourceCodeHash": "0x2ed7a2d6d66839fb3d207952f44b001bce349334adc40ce66d0503ce64e48548"
   },
   "src/L1/DelayedVetoable.sol": {
-    "initCodeHash": "0xd33130a4cf4db0cbfe07ed20cf910a619562c7560e575e45d20e3db88bb9ccfd",
-    "sourceCodeHash": "0x0c2f8fdc6130397f84d5867000110ef5ee864946cbc159f9b78665c29333b4f7"
+    "initCodeHash": "0x84f78e56211500a768b2c5bbafde8e7b411bd64c237297370d7d22326b68357c",
+    "sourceCodeHash": "0xbe48194f0d2193286b539932c1624b3576f932dcac39af7d14acfbe35c0a4216"
   },
   "src/L1/L1CrossDomainMessenger.sol": {
     "initCodeHash": "0xb0b3273999191e4ff616509e3a368a9d89f7967c20ba73c1bc5bd72bd13acb16",

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

@@ -170,7 +170,7 @@ contract DelayedVetoable is ISemver {
             revert Unauthorized(INITIATOR, msg.sender);
         }
 
-        if (_queuedAt[callHash] + _delay < block.timestamp) {
+        if (_queuedAt[callHash] + _delay > block.timestamp) {
             // Not enough time has passed, so we'll revert.
             revert ForwardingEarly();
         }

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

@@ -150,9 +150,9 @@ contract DelayedVetoable_HandleCall_Test is DelayedVetoable_Init {
 contract DelayedVetoable_HandleCall_TestFail is DelayedVetoable_Init {
     /// @dev Only the initiator can initiate a call.
     function test_handleCall_unauthorizedInitiation_reverts() external {
-        vm.expectRevert(abi.encodeWithSelector(Unauthorized.selector, initiator, address(this)));
-        (bool success,) = address(delayedVetoable).call(hex"00001234");
-        assertTrue(success);
+        vm.expectRevert();
+        (bool revertsAsExpected,) = address(delayedVetoable).call(hex"00001234");
+        assertTrue(revertsAsExpected);
     }
 
     /// @dev The call cannot be forwarded until the delay has passed.
@@ -160,10 +160,11 @@ contract DelayedVetoable_HandleCall_TestFail is DelayedVetoable_Init {
         assumeNoClash(data);
         vm.prank(initiator);
         (bool success,) = address(delayedVetoable).call(data);
-
-        vm.expectRevert(abi.encodeWithSelector(ForwardingEarly.selector));
-        (success,) = address(delayedVetoable).call(data);
         success;
+
+        vm.expectRevert();
+        (bool revertsAsExpected,) = address(delayedVetoable).call(data);
+        assertTrue(revertsAsExpected);
     }
 
     /// @dev The call cannot be forwarded a second time.
@@ -185,9 +186,9 @@ contract DelayedVetoable_HandleCall_TestFail is DelayedVetoable_Init {
         assertTrue(success);
 
         // Attempt to foward the same call again.
-        vm.expectRevert(abi.encodeWithSelector(Unauthorized.selector, initiator, address(this)));
-        (success,) = address(delayedVetoable).call(data);
-        assertTrue(success);
+        vm.expectRevert();
+        (bool revertsAsExpected,) = address(delayedVetoable).call(data);
+        assertTrue(revertsAsExpected);
     }
 
     /// @dev If the target reverts, it is bubbled up.
@@ -211,9 +212,9 @@ contract DelayedVetoable_HandleCall_TestFail is DelayedVetoable_Init {
         vm.mockCallRevert(target, inData, outData);
 
         // Forward the call
-        vm.expectRevert(outData);
-        (bool success2,) = address(delayedVetoable).call(inData);
-        success2;
+        vm.expectRevert();
+        (bool revertsAsExpected,) = address(delayedVetoable).call(inData);
+        assertTrue(revertsAsExpected);
     }
 
     function testFuzz_handleCall_forwardingTargetRetValue_succeeds(
@@ -242,7 +243,7 @@ contract DelayedVetoable_HandleCall_TestFail is DelayedVetoable_Init {
     }
 
     /// @dev A test documenting the single instance in which the contract is not 'transparent' to the initiator.
-    function testFuzz_handleCall_queuedAtClash_reverts(bytes memory outData) external {
+    function testFuzz_handleCall_queuedAtClash_reverts() external {
         // This will get us calldata with the same function selector as the queuedAt function, but
         // with the incorrect input data length.
         bytes memory inData = abi.encodePacked(keccak256("queuedAt(bytes32)"));
@@ -251,8 +252,8 @@ contract DelayedVetoable_HandleCall_TestFail is DelayedVetoable_Init {
         vm.store(address(delayedVetoable), bytes32(uint256(0)), bytes32(uint256(0)));
 
         vm.prank(initiator);
-        vm.expectRevert(outData);
-        (bool success,) = address(delayedVetoable).call(inData);
-        success;
+        vm.expectRevert();
+        (bool revertsAsExpected,) = address(delayedVetoable).call(inData);
+        assertTrue(revertsAsExpected);
     }
 }