Merge pull request #4746 from ethereum-optimism/fix_depositETHTo_test

Test for both legacy and new events in bridges

.circleci/config.yml

@@ -238,7 +238,7 @@ jobs:
       - run:
           name: test and generate coverage
           command: yarn coverage:lcov
-          no_output_timeout: 15m
+          no_output_timeout: 18m
           environment:
             FOUNDRY_PROFILE: ci
           working_directory: packages/contracts-bedrock

packages/contracts-bedrock/.gas-snapshot

@@ -92,23 +92,23 @@ L1ERC721Bridge_Test:test_finalizeBridgeERC721_notFromRemoteMessenger_reverts() (
 L1ERC721Bridge_Test:test_finalizeBridgeERC721_notViaLocalMessenger_reverts() (gas: 16093)
 L1ERC721Bridge_Test:test_finalizeBridgeERC721_selfToken_reverts() (gas: 17593)
 L1ERC721Bridge_Test:test_finalizeBridgeERC721_succeeds() (gas: 323814)
-L1StandardBridge_DepositERC20To_Test:test_depositERC20To_succeeds() (gas: 624279)
-L1StandardBridge_DepositERC20_Test:test_depositERC20_succeeds() (gas: 621958)
+L1StandardBridge_DepositERC20To_Test:test_depositERC20To_succeeds() (gas: 624908)
+L1StandardBridge_DepositERC20_Test:test_depositERC20_succeeds() (gas: 622587)
 L1StandardBridge_DepositERC20_TestFail:test_depositERC20_notEoa_reverts() (gas: 22320)
-L1StandardBridge_DepositETHTo_Test:test_depositETHTo_succeeds() (gas: 358590)
-L1StandardBridge_DepositETH_Test:test_depositETH_succeeds() (gas: 401413)
+L1StandardBridge_DepositETHTo_Test:test_depositETHTo_succeeds() (gas: 359219)
+L1StandardBridge_DepositETH_Test:test_depositETH_succeeds() (gas: 405236)
 L1StandardBridge_DepositETH_TestFail:test_depositETH_notEoa_reverts() (gas: 40780)
-L1StandardBridge_FinalizeBridgeETH_Test:test_finalizeBridgeETH_succeeds() (gas: 48661)
+L1StandardBridge_FinalizeBridgeETH_Test:test_finalizeBridgeETH_succeeds() (gas: 48788)
 L1StandardBridge_FinalizeBridgeETH_TestFail:test_finalizeBridgeETH_incorrectValue_reverts() (gas: 34207)
 L1StandardBridge_FinalizeBridgeETH_TestFail:test_finalizeBridgeETH_sendToMessenger_reverts() (gas: 34288)
 L1StandardBridge_FinalizeBridgeETH_TestFail:test_finalizeBridgeETH_sendToSelf_reverts() (gas: 34257)
-L1StandardBridge_FinalizeERC20Withdrawal_Test:test_finalizeERC20Withdrawal_succeeds() (gas: 495885)
+L1StandardBridge_FinalizeERC20Withdrawal_Test:test_finalizeERC20Withdrawal_succeeds() (gas: 496088)
 L1StandardBridge_FinalizeERC20Withdrawal_TestFail:test_finalizeERC20Withdrawal_notMessenger_reverts() (gas: 31148)
 L1StandardBridge_FinalizeERC20Withdrawal_TestFail:test_finalizeERC20Withdrawal_notOtherBridge_reverts() (gas: 31504)
-L1StandardBridge_FinalizeETHWithdrawal_Test:test_finalizeETHWithdrawal_succeeds() (gas: 58686)
+L1StandardBridge_FinalizeETHWithdrawal_Test:test_finalizeETHWithdrawal_succeeds() (gas: 61815)
 L1StandardBridge_Getter_Test:test_getters_succeeds() (gas: 32151)
 L1StandardBridge_Initialize_Test:test_initialize_succeeds() (gas: 22005)
-L1StandardBridge_Receive_Test:test_receive_succeeds() (gas: 519995)
+L1StandardBridge_Receive_Test:test_receive_succeeds() (gas: 520249)
 L2CrossDomainMessenger_Test:test_messageVersion_succeeds() (gas: 8389)
 L2CrossDomainMessenger_Test:test_pause_notOwner_reverts() (gas: 10837)
 L2CrossDomainMessenger_Test:test_pause_succeeds() (gas: 31846)
@@ -165,16 +165,17 @@ L2OutputOracleUpgradeable_Test:test_initializeImpl_alreadyInitialized_reverts()
 L2OutputOracleUpgradeable_Test:test_initializeProxy_alreadyInitialized_reverts() (gas: 20131)
 L2OutputOracleUpgradeable_Test:test_upgrading_succeeds() (gas: 180413)
 L2StandardBridge_FinalizeBridgeETH_Test:test_finalizeBridgeETH_succeeds() (gas: 36076)
-L2StandardBridge_Test:test_finalizeBridgeETH_incorrectValue_reverts() (gas: 23843)
-L2StandardBridge_Test:test_finalizeBridgeETH_sendToMessenger_reverts() (gas: 23982)
-L2StandardBridge_Test:test_finalizeBridgeETH_sendToSelf_reverts() (gas: 23893)
-L2StandardBridge_Test:test_finalizeDeposit_succeeds() (gas: 90641)
+L2StandardBridge_Test:test_finalizeBridgeETH_incorrectValue_reverts() (gas: 23865)
+L2StandardBridge_Test:test_finalizeBridgeETH_sendToMessenger_reverts() (gas: 23960)
+L2StandardBridge_Test:test_finalizeBridgeETH_sendToSelf_reverts() (gas: 23826)
+L2StandardBridge_Test:test_finalizeDeposit_depositingERC20_succeeds() (gas: 90619)
+L2StandardBridge_Test:test_finalizeDeposit_depositingETH_succeeds() (gas: 89451)
 L2StandardBridge_Test:test_initialize_succeeds() (gas: 24270)
-L2StandardBridge_Test:test_receive_succeeds() (gas: 176698)
-L2StandardBridge_Test:test_withdrawTo_succeeds() (gas: 384552)
-L2StandardBridge_Test:test_withdraw_insufficientValue_reverts() (gas: 19627)
-L2StandardBridge_Test:test_withdraw_notEOA_reverts() (gas: 251836)
-L2StandardBridge_Test:test_withdraw_succeeds() (gas: 382649)
+L2StandardBridge_Test:test_receive_succeeds() (gas: 177206)
+L2StandardBridge_Test:test_withdrawTo_withdrawingERC20_succeeds() (gas: 386492)
+L2StandardBridge_Test:test_withdraw_insufficientValue_reverts() (gas: 19649)
+L2StandardBridge_Test:test_withdraw_notEOA_reverts() (gas: 251799)
+L2StandardBridge_Test:test_withdraw_withdrawingERC20_succeeds() (gas: 384536)
 L2ToL1MessagePasserTest:test_burn_succeeds() (gas: 112572)
 L2ToL1MessagePasserTest:test_initiateWithdrawal_fromContract_succeeds() (gas: 70423)
 L2ToL1MessagePasserTest:test_initiateWithdrawal_fromEOA_succeeds() (gas: 75874)

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

@@ -40,10 +40,10 @@ contract L1StandardBridge_Receive_Test is Bridge_Initializer {
         assertEq(address(op).balance, 0);
 
         // The legacy event must be emitted for backwards compatibility
-        vm.expectEmit(true, true, true, true);
+        vm.expectEmit(true, true, true, true, address(L1Bridge));
         emit ETHDepositInitiated(alice, alice, 100, hex"");
 
-        vm.expectEmit(true, true, true, true);
+        vm.expectEmit(true, true, true, true, address(L1Bridge));
         emit ETHBridgeInitiated(alice, alice, 100, hex"");
 
         vm.expectCall(
@@ -83,7 +83,10 @@ contract L1StandardBridge_DepositETH_Test is Bridge_Initializer {
         uint256 version = 0; // Internal constant in the OptimismPortal: DEPOSIT_VERSION
         address l1MessengerAliased = AddressAliasHelper.applyL1ToL2Alias(address(L1Messenger));
 
-        vm.expectEmit(true, true, true, true);
+        vm.expectEmit(true, true, true, true, address(L1Bridge));
+        emit ETHDepositInitiated(alice, alice, 500, hex"ff");
+
+        vm.expectEmit(true, true, true, true, address(L1Bridge));
         emit ETHBridgeInitiated(alice, alice, 500, hex"ff");
 
         bytes memory message = abi.encodeWithSelector(
@@ -136,15 +139,15 @@ contract L1StandardBridge_DepositETH_Test is Bridge_Initializer {
         );
 
         // OptimismPortal emits a TransactionDeposited event on `depositTransaction` call
-        vm.expectEmit(true, true, true, true);
+        vm.expectEmit(true, true, true, true, address(op));
         emit TransactionDeposited(l1MessengerAliased, address(L2Messenger), version, opaqueData);
 
         // SentMessage event emitted by the CrossDomainMessenger
-        vm.expectEmit(true, true, true, true);
+        vm.expectEmit(true, true, true, true, address(L1Messenger));
         emit SentMessage(address(L2Bridge), address(L1Bridge), message, nonce, 50000);
 
         // SentMessageExtension1 event emitted by the CrossDomainMessenger
-        vm.expectEmit(true, true, true, true);
+        vm.expectEmit(true, true, true, true, address(L1Messenger));
         emit SentMessageExtension1(address(L1Bridge), 500);
 
         vm.prank(alice, alice);
@@ -176,10 +179,10 @@ contract L1StandardBridge_DepositETHTo_Test is Bridge_Initializer {
         uint256 version = 0; // Internal constant in the OptimismPortal: DEPOSIT_VERSION
         address l1MessengerAliased = AddressAliasHelper.applyL1ToL2Alias(address(L1Messenger));
 
-        vm.expectEmit(true, true, true, true);
+        vm.expectEmit(true, true, true, true, address(L1Bridge));
         emit ETHDepositInitiated(alice, bob, 600, hex"dead");
 
-        vm.expectEmit(true, true, true, true);
+        vm.expectEmit(true, true, true, true, address(L1Bridge));
         emit ETHBridgeInitiated(alice, bob, 600, hex"dead");
 
         // depositETHTo on the L1 bridge should be called
@@ -240,15 +243,15 @@ contract L1StandardBridge_DepositETHTo_Test is Bridge_Initializer {
         );
 
         // OptimismPortal emits a TransactionDeposited event on `depositTransaction` call
-        vm.expectEmit(true, true, true, true);
+        vm.expectEmit(true, true, true, true, address(op));
         emit TransactionDeposited(l1MessengerAliased, address(L2Messenger), version, opaqueData);
 
         // SentMessage event emitted by the CrossDomainMessenger
-        vm.expectEmit(true, true, true, true);
+        vm.expectEmit(true, true, true, true, address(L1Messenger));
         emit SentMessage(address(L2Bridge), address(L1Bridge), message, nonce, 1000);
 
         // SentMessageExtension1 event emitted by the CrossDomainMessenger
-        vm.expectEmit(true, true, true, true);
+        vm.expectEmit(true, true, true, true, address(L1Messenger));
         emit SentMessageExtension1(address(L1Bridge), 600);
 
         // deposit eth to bob
@@ -272,9 +275,6 @@ contract L1StandardBridge_DepositERC20_Test is Bridge_Initializer {
         uint256 version = 0; // Internal constant in the OptimismPortal: DEPOSIT_VERSION
         address l1MessengerAliased = AddressAliasHelper.applyL1ToL2Alias(address(L1Messenger));
 
-        vm.expectEmit(true, true, true, true);
-        emit ERC20DepositInitiated(address(L1Token), address(L2Token), alice, alice, 100, hex"");
-
         // Deal Alice's ERC20 State
         deal(address(L1Token), alice, 100000, true);
         vm.prank(alice);
@@ -338,20 +338,23 @@ contract L1StandardBridge_DepositERC20_Test is Bridge_Initializer {
             innerMessage
         );
 
-        // ERC20 Deposit Initiated event emitted by the StandardBridge Contract
-        vm.expectEmit(true, true, true, true);
+        // Should emit both the bedrock and legacy events
+        vm.expectEmit(true, true, true, true, address(L1Bridge));
+        emit ERC20DepositInitiated(address(L1Token), address(L2Token), alice, alice, 100, hex"");
+
+        vm.expectEmit(true, true, true, true, address(L1Bridge));
         emit ERC20BridgeInitiated(address(L1Token), address(L2Token), alice, alice, 100, hex"");
 
         // OptimismPortal emits a TransactionDeposited event on `depositTransaction` call
-        vm.expectEmit(true, true, true, true);
+        vm.expectEmit(true, true, true, true, address(op));
         emit TransactionDeposited(l1MessengerAliased, address(L2Messenger), version, opaqueData);
 
         // SentMessage event emitted by the CrossDomainMessenger
-        vm.expectEmit(true, true, true, true);
+        vm.expectEmit(true, true, true, true, address(L1Messenger));
         emit SentMessage(address(L2Bridge), address(L1Bridge), message, nonce, 10000);
 
         // SentMessageExtension1 event emitted by the CrossDomainMessenger
-        vm.expectEmit(true, true, true, true);
+        vm.expectEmit(true, true, true, true, address(L1Messenger));
         emit SentMessageExtension1(address(L1Bridge), 0);
 
         vm.prank(alice);
@@ -434,22 +437,23 @@ contract L1StandardBridge_DepositERC20To_Test is Bridge_Initializer {
             innerMessage
         );
 
-        vm.expectEmit(true, true, true, true);
+        // Should emit both the bedrock and legacy events
+        vm.expectEmit(true, true, true, true, address(L1Bridge));
         emit ERC20DepositInitiated(address(L1Token), address(L2Token), alice, bob, 1000, hex"");
 
-        vm.expectEmit(true, true, true, true);
+        vm.expectEmit(true, true, true, true, address(L1Bridge));
         emit ERC20BridgeInitiated(address(L1Token), address(L2Token), alice, bob, 1000, hex"");
 
         // OptimismPortal emits a TransactionDeposited event on `depositTransaction` call
-        vm.expectEmit(true, true, true, true);
+        vm.expectEmit(true, true, true, true, address(op));
         emit TransactionDeposited(l1MessengerAliased, address(L2Messenger), version, opaqueData);
 
         // SentMessage event emitted by the CrossDomainMessenger
-        vm.expectEmit(true, true, true, true);
+        vm.expectEmit(true, true, true, true, address(L1Messenger));
         emit SentMessage(address(L2Bridge), address(L1Bridge), message, nonce, 10000);
 
         // SentMessageExtension1 event emitted by the CrossDomainMessenger
-        vm.expectEmit(true, true, true, true);
+        vm.expectEmit(true, true, true, true, address(L1Messenger));
         emit SentMessageExtension1(address(L1Bridge), 0);
 
         deal(address(L1Token), alice, 100000, true);
@@ -478,9 +482,12 @@ contract L1StandardBridge_FinalizeETHWithdrawal_Test is Bridge_Initializer {
     function test_finalizeETHWithdrawal_succeeds() external {
         uint256 aliceBalance = alice.balance;
 
-        vm.expectEmit(true, true, true, true);
+        vm.expectEmit(true, true, true, true, address(L1Bridge));
         emit ETHWithdrawalFinalized(alice, alice, 100, hex"");
 
+        vm.expectEmit(true, true, true, true, address(L1Bridge));
+        emit ETHBridgeFinalized(alice, alice, 100, hex"");
+
         vm.expectCall(alice, hex"");
 
         vm.mockCall(
@@ -521,10 +528,10 @@ contract L1StandardBridge_FinalizeERC20Withdrawal_Test is Bridge_Initializer {
         vm.store(address(L1Bridge), bytes32(slot), bytes32(uint256(100)));
         assertEq(L1Bridge.deposits(address(L1Token), address(L2Token)), 100);
 
-        vm.expectEmit(true, true, true, true);
+        vm.expectEmit(true, true, true, true, address(L1Bridge));
         emit ERC20WithdrawalFinalized(address(L1Token), address(L2Token), alice, alice, 100, hex"");
 
-        vm.expectEmit(true, true, true, true);
+        vm.expectEmit(true, true, true, true, address(L1Bridge));
         emit ERC20BridgeFinalized(address(L1Token), address(L2Token), alice, alice, 100, hex"");
 
         vm.expectCall(
@@ -601,7 +608,7 @@ contract L1StandardBridge_FinalizeBridgeETH_Test is Bridge_Initializer {
         vm.deal(messenger, 100);
         vm.prank(messenger);
 
-        vm.expectEmit(true, true, true, true);
+        vm.expectEmit(true, true, true, true, address(L1Bridge));
         emit ETHBridgeFinalized(alice, alice, 100, hex"");
 
         L1Bridge.finalizeBridgeETH{ value: 100 }(alice, alice, 100, hex"");

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

@@ -4,6 +4,7 @@ pragma solidity 0.8.15;
 import { Bridge_Initializer } from "./CommonTest.t.sol";
 import { stdStorage, StdStorage } from "forge-std/Test.sol";
 import { CrossDomainMessenger } from "../universal/CrossDomainMessenger.sol";
+import { OptimismMintableERC20 } from "../universal/OptimismMintableERC20.sol";
 import { Predeploys } from "../libraries/Predeploys.sol";
 import { console } from "forge-std/console.sol";
 import { StandardBridge } from "../universal/StandardBridge.sol";
@@ -16,10 +17,6 @@ import { OptimismMintableERC20 } from "../universal/OptimismMintableERC20.sol";
 contract L2StandardBridge_Test is Bridge_Initializer {
     using stdStorage for StdStorage;
 
-    function setUp() public override {
-        super.setUp();
-    }
-
     function test_initialize_succeeds() external {
         assertEq(address(L2Bridge.messenger()), address(L2Messenger));
         assertEq(L1Bridge.l2TokenBridge(), address(L2Bridge));
@@ -60,11 +57,11 @@ contract L2StandardBridge_Test is Bridge_Initializer {
             })
         );
 
-        vm.expectEmit(true, true, true, true);
+        vm.expectEmit(true, true, true, true, address(L2Bridge));
         emit ETHBridgeInitiated(alice, alice, 100, hex"");
 
         // L2ToL1MessagePasser will emit a MessagePassed event
-        vm.expectEmit(true, true, true, true);
+        vm.expectEmit(true, true, true, true, address(messagePasser));
         emit MessagePassed(
             nonce,
             address(L2Messenger),
@@ -76,11 +73,11 @@ contract L2StandardBridge_Test is Bridge_Initializer {
         );
 
         // SentMessage event emitted by the CrossDomainMessenger
-        vm.expectEmit(true, true, true, true);
+        vm.expectEmit(true, true, true, true, address(L2Messenger));
         emit SentMessage(address(L1Bridge), address(L2Bridge), message, nonce, 200_000);
 
         // SentMessageExtension1 event emitted by the CrossDomainMessenger
-        vm.expectEmit(true, true, true, true);
+        vm.expectEmit(true, true, true, true, address(L2Messenger));
         emit SentMessageExtension1(address(L2Bridge), 100);
 
         vm.expectCall(
@@ -126,7 +123,7 @@ contract L2StandardBridge_Test is Bridge_Initializer {
     // - token is burned
     // - emits WithdrawalInitiated
     // - calls Withdrawer.initiateWithdrawal
-    function test_withdraw_succeeds() external {
+    function test_withdraw_withdrawingERC20_succeeds() external {
         // Alice has 100 L2Token
         deal(address(L2Token), alice, 100, true);
         assertEq(L2Token.balanceOf(alice), 100);
@@ -164,7 +161,7 @@ contract L2StandardBridge_Test is Bridge_Initializer {
         vm.expectEmit(true, true, true, true);
         emit ERC20BridgeInitiated(address(L2Token), address(L1Token), alice, alice, 100, hex"");
 
-        vm.expectEmit(true, true, true, true);
+        vm.expectEmit(true, true, true, true, address(messagePasser));
         emit MessagePassed(
             nonce,
             address(L2Messenger),
@@ -176,11 +173,11 @@ contract L2StandardBridge_Test is Bridge_Initializer {
         );
 
         // SentMessage event emitted by the CrossDomainMessenger
-        vm.expectEmit(true, true, true, true);
+        vm.expectEmit(true, true, true, true, address(L2Messenger));
         emit SentMessage(address(L1Bridge), address(L2Bridge), message, nonce, 1000);
 
         // SentMessageExtension1 event emitted by the CrossDomainMessenger
-        vm.expectEmit(true, true, true, true);
+        vm.expectEmit(true, true, true, true, address(L2Messenger));
         emit SentMessageExtension1(address(L2Bridge), 0);
 
         vm.expectEmit(true, true, true, true);
@@ -230,7 +227,7 @@ contract L2StandardBridge_Test is Bridge_Initializer {
     // - token is burned
     // - emits WithdrawalInitiated w/ correct recipient
     // - calls Withdrawer.initiateWithdrawal
-    function test_withdrawTo_succeeds() external {
+    function test_withdrawTo_withdrawingERC20_succeeds() external {
         deal(address(L2Token), alice, 100, true);
         assertEq(L2Token.balanceOf(alice), 100);
         uint256 nonce = L2Messenger.messageNonce();
@@ -267,7 +264,7 @@ contract L2StandardBridge_Test is Bridge_Initializer {
         vm.expectEmit(true, true, true, true);
         emit ERC20BridgeInitiated(address(L2Token), address(L1Token), alice, bob, 100, hex"");
 
-        vm.expectEmit(true, true, true, true);
+        vm.expectEmit(true, true, true, true, address(messagePasser));
         emit MessagePassed(
             nonce,
             address(L2Messenger),
@@ -279,11 +276,11 @@ contract L2StandardBridge_Test is Bridge_Initializer {
         );
 
         // SentMessage event emitted by the CrossDomainMessenger
-        vm.expectEmit(true, true, true, true);
+        vm.expectEmit(true, true, true, true, address(L2Messenger));
         emit SentMessage(address(L1Bridge), address(L2Bridge), message, nonce, 1000);
 
         // SentMessageExtension1 event emitted by the CrossDomainMessenger
-        vm.expectEmit(true, true, true, true);
+        vm.expectEmit(true, true, true, true, address(L2Messenger));
         emit SentMessageExtension1(address(L2Bridge), 0);
 
         vm.expectEmit(true, true, true, true);
@@ -325,7 +322,7 @@ contract L2StandardBridge_Test is Bridge_Initializer {
     // - only callable by l1TokenBridge
     // - supported token pair emits DepositFinalized
     // - invalid deposit calls Withdrawer.initiateWithdrawal
-    function test_finalizeDeposit_succeeds() external {
+    function test_finalizeDeposit_depositingERC20_succeeds() external {
         vm.mockCall(
             address(L2Bridge.messenger()),
             abi.encodeWithSelector(CrossDomainMessenger.xDomainMessageSender.selector),
@@ -337,6 +334,25 @@ contract L2StandardBridge_Test is Bridge_Initializer {
             abi.encodeWithSelector(OptimismMintableERC20.mint.selector, alice, 100)
         );
 
+        // Should emit both the bedrock and legacy events
+        vm.expectEmit(true, true, true, true, address(L2Bridge));
+        emit ERC20BridgeFinalized(address(L2Token), address(L1Token), alice, alice, 100, hex"");
+
+        vm.expectEmit(true, true, true, true, address(L2Bridge));
+        emit DepositFinalized(address(L1Token), address(L2Token), alice, alice, 100, hex"");
+
+        vm.prank(address(L2Messenger));
+        L2Bridge.finalizeDeposit(address(L1Token), address(L2Token), alice, alice, 100, hex"");
+    }
+
+    function test_finalizeDeposit_depositingETH_succeeds() external {
+        vm.mockCall(
+            address(L2Bridge.messenger()),
+            abi.encodeWithSelector(CrossDomainMessenger.xDomainMessageSender.selector),
+            abi.encode(address(L2Bridge.OTHER_BRIDGE()))
+        );
+
+        // Should emit both the bedrock and legacy events
         vm.expectEmit(true, true, true, true, address(L2Bridge));
         emit ERC20BridgeFinalized(
             address(L2Token), // localToken