Merge pull request #2765 from ethereum-optimism/m/oz-med-on-develop

.changeset/three-pants-punch.md

+---
+'@eth-optimism/contracts-bedrock': patch
+---
+
+Fixes to medium severity OZ findings
+
+- Disallow reentrant withdrawals
+- remove donateEth
+- Correct ordering of \_from and \_to arguments on refunds of failed deposits

packages/contracts-bedrock/.gas-snapshot

@@ -23,39 +23,40 @@ L1BlockTest:test_updateValues() (gas: 28215)
 L1BlockNumberTest:test_fallback() (gas: 18773)
 L1BlockNumberTest:test_getL1BlockNumber() (gas: 10589)
 L1BlockNumberTest:test_receive() (gas: 25436)
-L1CrossDomainMessenger_Test:testCannot_L1MessengerPause() (gas: 10909)
-L1CrossDomainMessenger_Test:test_L1MessengerMessageVersion() (gas: 8366)
-L1CrossDomainMessenger_Test:test_L1MessengerPause() (gas: 31882)
-L1CrossDomainMessenger_Test:test_L1MessengerRelayMessageSucceeds() (gas: 61239)
-L1CrossDomainMessenger_Test:test_L1MessengerRelayMessageToSystemContract() (gas: 44859)
-L1CrossDomainMessenger_Test:test_L1MessengerRelayShouldRevertIfPaused() (gas: 41631)
-L1CrossDomainMessenger_Test:test_L1MessengerSendMessage() (gas: 172148)
-L1CrossDomainMessenger_Test:test_L1MessengerTwiceSendMessage() (gas: 1254131)
-L1CrossDomainMessenger_Test:test_L1MessengerXDomainSenderReverts() (gas: 10566)
-L1CrossDomainMessenger_Test:test_L1MessengerxDomainMessageSenderResets() (gas: 58535)
-L1StandardBridge_Test:test_depositERC20() (gas: 452873)
-L1StandardBridge_Test:test_depositERC20To() (gas: 454650)
-L1StandardBridge_Test:test_depositETH() (gas: 247054)
-L1StandardBridge_Test:test_depositETHTo() (gas: 204938)
-L1StandardBridge_Test:test_donateETH() (gas: 17545)
-L1StandardBridge_Test:test_finalizeERC20Withdrawal() (gas: 438824)
+L1CrossDomainMessenger_Test:testCannot_L1MessengerPause() (gas: 10844)
+L1CrossDomainMessenger_Test:testCannot_L1MessengerUnpause() (gas: 10858)
+L1CrossDomainMessenger_Test:test_L1MessengerMessageVersion() (gas: 8388)
+L1CrossDomainMessenger_Test:test_L1MessengerPause() (gas: 31860)
+L1CrossDomainMessenger_Test:test_L1MessengerRelayMessageSucceeds() (gas: 61261)
+L1CrossDomainMessenger_Test:test_L1MessengerRelayMessageToSystemContract() (gas: 44881)
+L1CrossDomainMessenger_Test:test_L1MessengerRelayShouldRevertIfPaused() (gas: 41587)
+L1CrossDomainMessenger_Test:test_L1MessengerSendMessage() (gas: 172193)
+L1CrossDomainMessenger_Test:test_L1MessengerTwiceSendMessage() (gas: 1254199)
+L1CrossDomainMessenger_Test:test_L1MessengerUnpause() (gas: 23804)
+L1CrossDomainMessenger_Test:test_L1MessengerXDomainSenderReverts() (gas: 10599)
+L1CrossDomainMessenger_Test:test_L1MessengerxDomainMessageSenderResets() (gas: 58579)
+L1StandardBridge_Test:test_depositERC20() (gas: 452856)
+L1StandardBridge_Test:test_depositERC20To() (gas: 454632)
+L1StandardBridge_Test:test_depositETH() (gas: 247077)
+L1StandardBridge_Test:test_depositETHTo() (gas: 204961)
+L1StandardBridge_Test:test_finalizeERC20Withdrawal() (gas: 438752)
 L1StandardBridge_Test:test_finalizeETHWithdrawal() (gas: 48005)
 L1StandardBridge_Test:test_initialize() (gas: 14885)
 L1StandardBridge_Test:test_onlyEOADepositERC20() (gas: 12085)
 L1StandardBridge_Test:test_onlyEOADepositETH() (gas: 30637)
 L1StandardBridge_Test:test_onlyL2BridgeFinalizeERC20Withdrawal() (gas: 23565)
 L1StandardBridge_Test:test_onlyPortalFinalizeERC20Withdrawal() (gas: 22919)
-L1StandardBridge_Test:test_receive() (gas: 391794)
+L1StandardBridge_Test:test_receive() (gas: 391817)
 L2CrossDomainMessenger_Test:testCannot_L2MessengerPause() (gas: 10843)
 L2CrossDomainMessenger_Test:test_L2MessengerMessageVersion() (gas: 8410)
 L2CrossDomainMessenger_Test:test_L2MessengerPause() (gas: 31837)
 L2CrossDomainMessenger_Test:test_L2MessengerRelayMessageSucceeds() (gas: 57538)
 L2CrossDomainMessenger_Test:test_L2MessengerRelayMessageToSystemContract() (gas: 24567)
 L2CrossDomainMessenger_Test:test_L2MessengerRelayShouldRevertIfPaused() (gas: 41599)
-L2CrossDomainMessenger_Test:test_L2MessengerSendMessage() (gas: 119659)
-L2CrossDomainMessenger_Test:test_L2MessengerTwiceSendMessage() (gas: 133096)
-L2CrossDomainMessenger_Test:test_L2MessengerXDomainSenderReverts() (gas: 10588)
-L2CrossDomainMessenger_Test:test_L2MessengerxDomainMessageSenderResets() (gas: 54859)
+L2CrossDomainMessenger_Test:test_L2MessengerSendMessage() (gas: 119682)
+L2CrossDomainMessenger_Test:test_L2MessengerTwiceSendMessage() (gas: 133142)
+L2CrossDomainMessenger_Test:test_L2MessengerXDomainSenderReverts() (gas: 10599)
+L2CrossDomainMessenger_Test:test_L2MessengerxDomainMessageSenderResets() (gas: 54881)
 L2OutputOracleTest:testCannot_appendEmptyOutput() (gas: 18442)
 L2OutputOracleTest:testCannot_appendFutureTimetamp() (gas: 20072)
 L2OutputOracleTest:testCannot_appendOnWrongFork() (gas: 20710)
@@ -71,14 +72,15 @@ L2OutputOracleTest:test_deleteL2Output() (gas: 66081)
 L2OutputOracleTest:test_getL2Output() (gas: 76274)
 L2OutputOracleTest:test_latestBlockNumber() (gas: 70075)
 L2OutputOracleTest:test_nextBlockNumber() (gas: 9279)
-L2StandardBridge_Test:test_cannotWithdrawEthWithoutSendingIt() (gas: 21578)
-L2StandardBridge_Test:test_finalizeDeposit() (gas: 93165)
-L2StandardBridge_Test:test_finalizeDeposit_failsToCompleteOutboundTransfer() (gas: 140106)
-L2StandardBridge_Test:test_initialize() (gas: 14834)
-L2StandardBridge_Test:test_receive() (gas: 136459)
-L2StandardBridge_Test:test_withdraw() (gas: 352724)
-L2StandardBridge_Test:test_withdrawTo() (gas: 353480)
-L2StandardBridge_Test:test_withdraw_onlyEOA() (gas: 251952)
+L2StandardBridge_Test:test_ERC20BridgeFailed_whenLocalTokenIsBridge() (gas: 133097)
+L2StandardBridge_Test:test_cannotWithdrawEthWithoutSendingIt() (gas: 21611)
+L2StandardBridge_Test:test_finalizeDeposit() (gas: 93100)
+L2StandardBridge_Test:test_finalizeDeposit_failsToCompleteOutboundTransfer() (gas: 140129)
+L2StandardBridge_Test:test_initialize() (gas: 14856)
+L2StandardBridge_Test:test_receive() (gas: 136415)
+L2StandardBridge_Test:test_withdraw() (gas: 352760)
+L2StandardBridge_Test:test_withdrawTo() (gas: 353481)
+L2StandardBridge_Test:test_withdraw_onlyEOA() (gas: 251970)
 L2ToL1MessagePasserTest:test_burn() (gas: 112046)
 L2ToL1MessagePasserTest:test_initiateWithdrawal_fromContract() (gas: 67890)
 L2ToL1MessagePasserTest:test_initiateWithdrawal_fromEOA() (gas: 74851)
@@ -164,4 +166,4 @@ SequencerFeeVault_Test:test_constructor() (gas: 7611)
 SequencerFeeVault_Test:test_minWithdrawalAmount() (gas: 5429)
 SequencerFeeVault_Test:test_receive() (gas: 17280)
 SequencerFeeVault_Test:test_revertWithdraw() (gas: 9266)
-SequencerFeeVault_Test:test_withdraw() (gas: 147300)
+SequencerFeeVault_Test:test_withdraw() (gas: 147301)

packages/contracts-bedrock/contracts/L1/OptimismPortal.sol

@@ -165,7 +165,7 @@ contract OptimismPortal is ResourceMetering {
         WithdrawalVerifier.OutputRootProof calldata _outputRootProof,
         bytes calldata _withdrawalProof
     ) external payable {
-        // Prevent reentrancy.
+        // Prevent nested withdrawals within withdrawals.
         require(
             l2Sender == DEFAULT_L2_SENDER,
             "OptimismPortal: can only trigger one withdrawal per transaction"

packages/contracts-bedrock/contracts/L2/L2StandardBridge.sol

@@ -22,12 +22,12 @@ contract L2StandardBridge is StandardBridge {
      * @custom:legacy
      * @notice Emitted whenever a withdrawal from L2 to L1 is initiated.
      *
-     * @param _l1Token Address of the token on L1.
-     * @param _l2Token Address of the corresponding token on L2.
-     * @param _from    Address of the withdrawer.
-     * @param _to      Address of the recipient on L1.
-     * @param _amount  Amount of the ERC20 withdrawn.
-     * @param _data    Extra data attached to the withdrawal.
+     * @param _l1Token   Address of the token on L1.
+     * @param _l2Token   Address of the corresponding token on L2.
+     * @param _from      Address of the withdrawer.
+     * @param _to        Address of the recipient on L1.
+     * @param _amount    Amount of the ERC20 withdrawn.
+     * @param _extraData Extra data attached to the withdrawal.
      */
     event WithdrawalInitiated(
         address indexed _l1Token,
@@ -35,19 +35,19 @@ contract L2StandardBridge is StandardBridge {
         address indexed _from,
         address _to,
         uint256 _amount,
-        bytes _data
+        bytes _extraData
     );
 
     /**
      * @custom:legacy
      * @notice Emitted whenever an ERC20 deposit is finalized.
      *
-     * @param _l1Token Address of the token on L1.
-     * @param _l2Token Address of the corresponding token on L2.
-     * @param _from    Address of the depositor.
-     * @param _to      Address of the recipient on L2.
-     * @param _amount  Amount of the ERC20 deposited.
-     * @param _data    Extra data attached to the deposit.
+     * @param _l1Token   Address of the token on L1.
+     * @param _l2Token   Address of the corresponding token on L2.
+     * @param _from      Address of the depositor.
+     * @param _to        Address of the recipient on L2.
+     * @param _amount    Amount of the ERC20 deposited.
+     * @param _extraData Extra data attached to the deposit.
      */
     event DepositFinalized(
         address indexed _l1Token,
@@ -55,19 +55,19 @@ contract L2StandardBridge is StandardBridge {
         address indexed _from,
         address _to,
         uint256 _amount,
-        bytes _data
+        bytes _extraData
     );
 
     /**
      * @custom:legacy
      * @notice Emitted whenever a deposit fails.
      *
-     * @param _l1Token Address of the token on L1.
-     * @param _l2Token Address of the corresponding token on L2.
-     * @param _from    Address of the depositor.
-     * @param _to      Address of the recipient on L2.
-     * @param _amount  Amount of the ERC20 deposited.
-     * @param _data    Extra data attached to the deposit.
+     * @param _l1Token   Address of the token on L1.
+     * @param _l2Token   Address of the corresponding token on L2.
+     * @param _from      Address of the depositor.
+     * @param _to        Address of the recipient on L2.
+     * @param _amount    Amount of the ERC20 deposited.
+     * @param _extraData Extra data attached to the deposit.
      */
     event DepositFailed(
         address indexed _l1Token,
@@ -75,7 +75,7 @@ contract L2StandardBridge is StandardBridge {
         address indexed _from,
         address _to,
         uint256 _amount,
-        bytes _data
+        bytes _extraData
     );
 
     /**
@@ -94,15 +94,15 @@ contract L2StandardBridge is StandardBridge {
      * @param _l2Token     Address of the L2 token to withdraw.
      * @param _amount      Amount of the L2 token to withdraw.
      * @param _minGasLimit Minimum gas limit to use for the transaction.
-     * @param _data        Extra data attached to the withdrawal.
+     * @param _extraData   Extra data attached to the withdrawal.
      */
     function withdraw(
         address _l2Token,
         uint256 _amount,
         uint32 _minGasLimit,
-        bytes calldata _data
+        bytes calldata _extraData
     ) external payable virtual onlyEOA {
-        _initiateWithdrawal(_l2Token, msg.sender, msg.sender, _amount, _minGasLimit, _data);
+        _initiateWithdrawal(_l2Token, msg.sender, msg.sender, _amount, _minGasLimit, _extraData);
     }
 
     /**
@@ -117,28 +117,28 @@ contract L2StandardBridge is StandardBridge {
      * @param _to          Recipient account on L1.
      * @param _amount      Amount of the L2 token to withdraw.
      * @param _minGasLimit Minimum gas limit to use for the transaction.
-     * @param _data        Extra data attached to the withdrawal.
+     * @param _extraData   Extra data attached to the withdrawal.
      */
     function withdrawTo(
         address _l2Token,
         address _to,
         uint256 _amount,
         uint32 _minGasLimit,
-        bytes calldata _data
+        bytes calldata _extraData
     ) external payable virtual {
-        _initiateWithdrawal(_l2Token, msg.sender, _to, _amount, _minGasLimit, _data);
+        _initiateWithdrawal(_l2Token, msg.sender, _to, _amount, _minGasLimit, _extraData);
     }
 
     /**
      * @custom:legacy
      * @notice Finalizes a deposit from L1 to L2.
      *
-     * @param _l1Token Address of the L1 token to deposit.
-     * @param _l2Token Address of the corresponding L2 token.
-     * @param _from    Address of the depositor.
-     * @param _to      Address of the recipient.
-     * @param _amount  Amount of the tokens being deposited.
-     * @param _data    Extra data attached to the deposit.
+     * @param _l1Token   Address of the L1 token to deposit.
+     * @param _l2Token   Address of the corresponding L2 token.
+     * @param _from      Address of the depositor.
+     * @param _to        Address of the recipient.
+     * @param _amount    Amount of the tokens being deposited.
+     * @param _extraData Extra data attached to the deposit.
      */
     function finalizeDeposit(
         address _l1Token,
@@ -146,14 +146,14 @@ contract L2StandardBridge is StandardBridge {
         address _from,
         address _to,
         uint256 _amount,
-        bytes calldata _data
+        bytes calldata _extraData
     ) external payable virtual {
         if (_l1Token == address(0) && _l2Token == Lib_PredeployAddresses.OVM_ETH) {
-            finalizeBridgeETH(_from, _to, _amount, _data);
+            finalizeBridgeETH(_from, _to, _amount, _extraData);
         } else {
-            finalizeBridgeERC20(_l2Token, _l1Token, _from, _to, _amount, _data);
+            finalizeBridgeERC20(_l2Token, _l1Token, _from, _to, _amount, _extraData);
         }
-        emit DepositFinalized(_l1Token, _l2Token, _from, _to, _amount, _data);
+        emit DepositFinalized(_l1Token, _l2Token, _from, _to, _amount, _extraData);
     }
 
     /**
@@ -165,7 +165,7 @@ contract L2StandardBridge is StandardBridge {
      * @param _to          Recipient account on L1.
      * @param _amount      Amount of the L2 token to withdraw.
      * @param _minGasLimit Minimum gas limit to use for the transaction.
-     * @param _data        Extra data attached to the withdrawal.
+     * @param _extraData   Extra data attached to the withdrawal.
      */
     function _initiateWithdrawal(
         address _l2Token,
@@ -173,15 +173,15 @@ contract L2StandardBridge is StandardBridge {
         address _to,
         uint256 _amount,
         uint32 _minGasLimit,
-        bytes calldata _data
+        bytes calldata _extraData
     ) internal {
         address l1Token = OptimismMintableERC20(_l2Token).l1Token();
         if (_l2Token == Lib_PredeployAddresses.OVM_ETH) {
             require(msg.value == _amount, "ETH withdrawals must include sufficient ETH value.");
-            _initiateBridgeETH(_from, _to, _amount, _minGasLimit, _data);
+            _initiateBridgeETH(_from, _to, _amount, _minGasLimit, _extraData);
         } else {
-            _initiateBridgeERC20(_l2Token, l1Token, _from, _to, _amount, _minGasLimit, _data);
+            _initiateBridgeERC20(_l2Token, l1Token, _from, _to, _amount, _minGasLimit, _extraData);
         }
-        emit WithdrawalInitiated(l1Token, _l2Token, _from, _to, _amount, _data);
+        emit WithdrawalInitiated(l1Token, _l2Token, _from, _to, _amount, _extraData);
     }
 }

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

@@ -42,6 +42,21 @@ contract L1CrossDomainMessenger_Test is Messenger_Initializer {
         L1Messenger.pause();
     }
 
+    // unpause: should unpause the contract when called by the current owner
+    function test_L1MessengerUnpause() external {
+        L1Messenger.pause();
+        assert(L1Messenger.paused());
+        L1Messenger.unpause();
+        assert(!L1Messenger.paused());
+    }
+
+    // unpause: should not unpause the contract when called by account other than the owner
+    function testCannot_L1MessengerUnpause() external {
+        vm.expectRevert("Ownable: caller is not the owner");
+        vm.prank(address(0xABBA));
+        L1Messenger.unpause();
+    }
+
     // the version is encoded in the nonce
     function test_L1MessengerMessageVersion() external {
         assertEq(

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

@@ -390,13 +390,4 @@ contract L1StandardBridge_Test is Bridge_Initializer {
             hex""
         );
     }
-
-    // donateETH
-    // - can send ETH to the contract
-    function test_donateETH() external {
-        assertEq(address(L1Bridge).balance, 0);
-        vm.prank(alice);
-        L1Bridge.donateETH{ value: 1000 }();
-        assertEq(address(L1Bridge).balance, 1000);
-    }
 }

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

@@ -146,13 +146,12 @@ contract L2StandardBridge_Test is Bridge_Initializer {
     // - invalid deposit calls Withdrawer.initiateWithdrawal
     function test_finalizeDeposit_failsToCompleteOutboundTransfer() external {
         // TODO: events and calls
-        address invalidL2Token = address(0x1234);
-
         vm.mockCall(
             address(L2Bridge.messenger()),
             abi.encodeWithSelector(CrossDomainMessenger.xDomainMessageSender.selector),
             abi.encode(address(L2Bridge.otherBridge()))
         );
+        address invalidL2Token = address(0x1234);
         vm.prank(address(L2Messenger));
         vm.expectEmit(true, true, true, true);
         emit ERC20BridgeInitiated(
@@ -181,5 +180,21 @@ contract L2StandardBridge_Test is Bridge_Initializer {
             hex""
         );
     }
+
+    // finalizeBridgeERC20
+    // - fails when the local token's address equals bridge address
+    function test_ERC20BridgeFailed_whenLocalTokenIsBridge() external {
+        vm.mockCall(
+            address(L2Bridge.messenger()),
+            abi.encodeWithSelector(CrossDomainMessenger.xDomainMessageSender.selector),
+            abi.encode(address(L2Bridge.otherBridge()))
+        );
+        // fails when the local token's address equals bridge address
+        vm.expectEmit(true, true, true, true);
+        emit ERC20BridgeFailed(address(L2Bridge), address(L1Token), alice, bob, 100, hex"");
+
+        vm.prank(address(L2Messenger));
+        L2Bridge.finalizeDeposit(address(L1Token), address(L2Bridge), alice, bob, 100, hex"");
+    }
 }
 

packages/contracts-bedrock/contracts/universal/CrossDomainMessenger.sol

@@ -101,6 +101,13 @@ abstract contract CrossDomainMessenger is
         _pause();
     }
 
+    /**
+     * Unpause relaying.
+     */
+    function unpause() external onlyOwner {
+        _unpause();
+    }
+
     /**
      * Retrieves the address of the x-domain message sender. Will throw an error
      * if the sender is not currently set (equal to the default sender).

packages/contracts-bedrock/contracts/universal/StandardBridge.sol

@@ -31,14 +31,14 @@ abstract contract StandardBridge {
         address indexed _from,
         address indexed _to,
         uint256 _amount,
-        bytes _data
+        bytes _extraData
     );
 
     event ETHBridgeFinalized(
         address indexed _from,
         address indexed _to,
         uint256 _amount,
-        bytes _data
+        bytes _extraData
     );
 
     event ERC20BridgeInitiated(
@@ -47,7 +47,7 @@ abstract contract StandardBridge {
         address indexed _from,
         address _to,
         uint256 _amount,
-        bytes _data
+        bytes _extraData
     );
 
     event ERC20BridgeFinalized(
@@ -56,7 +56,7 @@ abstract contract StandardBridge {
         address indexed _from,
         address _to,
         uint256 _amount,
-        bytes _data
+        bytes _extraData
     );
 
     event ERC20BridgeFailed(
@@ -65,7 +65,7 @@ abstract contract StandardBridge {
         address indexed _from,
         address _to,
         uint256 _amount,
-        bytes _data
+        bytes _extraData
     );
 
     /*************
@@ -128,11 +128,6 @@ abstract contract StandardBridge {
      * Public Functions *
      ********************/
 
-    /**
-     * @notice Send ETH to this contract. This is used during upgrades
-     */
-    function donateETH() external payable {}
-
     /**
      * @notice EOAs can simply send ETH to this contract to have it be deposited
      * to L2 through the standard bridge.
@@ -144,8 +139,8 @@ abstract contract StandardBridge {
     /**
      * @notice Send ETH to the message sender on the remote domain
      */
-    function bridgeETH(uint32 _minGasLimit, bytes calldata _data) public payable onlyEOA {
-        _initiateBridgeETH(msg.sender, msg.sender, msg.value, _minGasLimit, _data);
+    function bridgeETH(uint32 _minGasLimit, bytes calldata _extraData) public payable onlyEOA {
+        _initiateBridgeETH(msg.sender, msg.sender, msg.value, _minGasLimit, _extraData);
     }
 
     /**
@@ -155,9 +150,9 @@ abstract contract StandardBridge {
     function bridgeETHTo(
         address _to,
         uint32 _minGasLimit,
-        bytes calldata _data
+        bytes calldata _extraData
     ) public payable {
-        _initiateBridgeETH(msg.sender, _to, msg.value, _minGasLimit, _data);
+        _initiateBridgeETH(msg.sender, _to, msg.value, _minGasLimit, _extraData);
     }
 
     /**
@@ -168,7 +163,7 @@ abstract contract StandardBridge {
         address _remoteToken,
         uint256 _amount,
         uint32 _minGasLimit,
-        bytes calldata _data
+        bytes calldata _extraData
     ) public virtual onlyEOA {
         _initiateBridgeERC20(
             _localToken,
@@ -177,7 +172,7 @@ abstract contract StandardBridge {
             msg.sender,
             _amount,
             _minGasLimit,
-            _data
+            _extraData
         );
     }
 
@@ -190,7 +185,7 @@ abstract contract StandardBridge {
         address _to,
         uint256 _amount,
         uint32 _minGasLimit,
-        bytes calldata _data
+        bytes calldata _extraData
     ) public virtual {
         _initiateBridgeERC20(
             _localToken,
@@ -199,7 +194,7 @@ abstract contract StandardBridge {
             _to,
             _amount,
             _minGasLimit,
-            _data
+            _extraData
         );
     }
 
@@ -210,12 +205,12 @@ abstract contract StandardBridge {
         address _from,
         address _to,
         uint256 _amount,
-        bytes calldata _data
+        bytes calldata _extraData
     ) public payable onlyOtherBridge {
         require(msg.value == _amount, "Amount sent does not match amount required.");
         require(_to != address(this), "Cannot send to self.");
 
-        emit ETHBridgeFinalized(_from, _to, _amount, _data);
+        emit ETHBridgeFinalized(_from, _to, _amount, _extraData);
         (bool success, ) = _to.call{ value: _amount }(new bytes(0));
         require(success, "ETH transfer failed.");
     }
@@ -229,23 +224,25 @@ abstract contract StandardBridge {
         address _from,
         address _to,
         uint256 _amount,
-        bytes calldata _data
+        bytes calldata _extraData
     ) public onlyOtherBridge {
         try this.completeOutboundTransfer(_localToken, _remoteToken, _to, _amount) {
-            emit ERC20BridgeFinalized(_localToken, _remoteToken, _from, _to, _amount, _data);
+            emit ERC20BridgeFinalized(_localToken, _remoteToken, _from, _to, _amount, _extraData);
         } catch {
             // Something went wrong during the bridging process, return to sender.
             // Can happen if a bridge UI specifies the wrong L2 token.
+            // We reverse both the local and remote token addresses, as well as the to and from
+            // addresses. This will preserve the accuracy of accounting based on emitted events.
             _initiateBridgeERC20Unchecked(
                 _localToken,
                 _remoteToken,
-                _from,
                 _to,
+                _from,
                 _amount,
                 0, // _minGasLimit, 0 is fine here
-                _data
+                _extraData
             );
-            emit ERC20BridgeFailed(_localToken, _remoteToken, _from, _to, _amount, _data);
+            emit ERC20BridgeFailed(_localToken, _remoteToken, _from, _to, _amount, _extraData);
         }
     }
 
@@ -296,13 +293,19 @@ abstract contract StandardBridge {
         address _to,
         uint256 _amount,
         uint32 _minGasLimit,
-        bytes memory _data
+        bytes memory _extraData
     ) internal {
-        emit ETHBridgeInitiated(_from, _to, _amount, _data);
+        emit ETHBridgeInitiated(_from, _to, _amount, _extraData);
 
         messenger.sendMessage{ value: _amount }(
             address(otherBridge),
-            abi.encodeWithSelector(this.finalizeBridgeETH.selector, _from, _to, _amount, _data),
+            abi.encodeWithSelector(
+                this.finalizeBridgeETH.selector,
+                _from,
+                _to,
+                _amount,
+                _extraData
+            ),
             _minGasLimit
         );
     }
@@ -317,7 +320,7 @@ abstract contract StandardBridge {
         address _to,
         uint256 _amount,
         uint32 _minGasLimit,
-        bytes calldata _data
+        bytes calldata _extraData
     ) internal {
         // Make sure external function calls can't be used to trigger calls to
         // completeOutboundTransfer. We only make external (write) calls to _localToken.
@@ -343,7 +346,7 @@ abstract contract StandardBridge {
             _to,
             _amount,
             _minGasLimit,
-            _data
+            _extraData
         );
     }
 
@@ -357,7 +360,7 @@ abstract contract StandardBridge {
         address _to,
         uint256 _amount,
         uint32 _minGasLimit,
-        bytes calldata _data
+        bytes calldata _extraData
     ) internal {
         messenger.sendMessage(
             address(otherBridge),
@@ -371,12 +374,12 @@ abstract contract StandardBridge {
                 _from,
                 _to,
                 _amount,
-                _data
+                _extraData
             ),
             _minGasLimit
         );
 
-        emit ERC20BridgeInitiated(_localToken, _remoteToken, _from, _to, _amount, _data);
+        emit ERC20BridgeInitiated(_localToken, _remoteToken, _from, _to, _amount, _extraData);
     }
 
     /**