fix: M-01 relayTokens Calls Made by Forwarders May Fail

contracts/chain-adapters/ForwarderBase.sol

@@ -2,8 +2,10 @@
 pragma solidity ^0.8.0;
 
 import { UUPSUpgradeable } from "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol";
+import { ReentrancyGuardUpgradeable } from "@openzeppelin/contracts-upgradeable/security/ReentrancyGuardUpgradeable.sol";
 import { ForwarderInterface } from "./interfaces/ForwarderInterface.sol";
 import { AdapterInterface } from "./interfaces/AdapterInterface.sol";
+import { MultiCaller } from "@uma/core/contracts/common/implementation/MultiCaller.sol";
 import { WETH9Interface } from "../external/interfaces/WETH9Interface.sol";
 
 /**
@@ -16,7 +18,7 @@ import { WETH9Interface } from "../external/interfaces/WETH9Interface.sol";
  * bridge. In other words, this contract maintains a mapping of important contracts which helps transmit messages to the "next layer".
  * @custom:security-contact bugs@across.to
  */
-abstract contract ForwarderBase is UUPSUpgradeable, ForwarderInterface {
+abstract contract ForwarderBase is UUPSUpgradeable, ForwarderInterface, MultiCaller, ReentrancyGuardUpgradeable {
     // Address of the wrapped native token contract on this L2.
     WETH9Interface public immutable WRAPPED_NATIVE_TOKEN;
     // Address that can relay messages using this contract and also upgrade this contract.
@@ -27,14 +29,17 @@ abstract contract ForwarderBase is UUPSUpgradeable, ForwarderInterface {
     // ID to be sent along with a message or token relay, ForwarderInterface's relay functions include an extra field, `destinationChainId`, when compared to the
     // relay functions of `AdapterInterface`.
     mapping(uint256 => address) chainAdapters;
+    // An array of which contains all token relays sent to this forwarder. A token relay is only stored here if it is received from the L1 cross domain admin.
+    // Each TokenRelay element contains a yes/no value describing whether or not the token relay has been executed. TokenRelays can only ever be executed once,
+    // but anybody can execute a stored TokenRelay in the array.
+    TokenRelay[] public tokenRelays;
 
     event ChainAdaptersUpdated(uint256 indexed destinationChainId, address l2Adapter);
     event SetXDomainAdmin(address indexed crossDomainAdmin);
 
     error InvalidCrossDomainAdmin();
     error InvalidChainAdapter();
-    error RelayMessageFailed();
-    error RelayTokensFailed(address baseToken);
+    error InvalidTokenRelayId();
     // Error which is triggered when there is no adapter set in the `chainAdapters` mapping.
     error UninitializedChainAdapter();
     // Error which is triggered when the contract attempts to wrap a native token for an amount greater than
@@ -73,6 +78,7 @@ abstract contract ForwarderBase is UUPSUpgradeable, ForwarderInterface {
      */
     function __Forwarder_init(address _crossDomainAdmin) public onlyInitializing {
         __UUPSUpgradeable_init();
+        __ReentrancyGuard_init();
         _setCrossDomainAdmin(_crossDomainAdmin);
     }
 
@@ -124,35 +130,67 @@ abstract contract ForwarderBase is UUPSUpgradeable, ForwarderInterface {
     }
 
     /**
-     * @notice Relays `amount` of a token to a contract on L3. Importantly, this contract assumes that `target` exists on L3.
-     * @param baseToken This layer's address of the token to send.
-     * @param destinationChainToken The next layer's address of the token to send.
+     * @notice Stores information about sending `amount` of a token to a target on L3. Importantly, this contract assumes that `target` exists on L3.
+     * @param l2Token This layer's address of the token to send.
+     * @param l3Token The next layer's address of the token to send.
      * @param amount The amount of the token to send.
      * @param destinationChainId The chain ID of the network which contains `target`.
-     * @param target The address of the contract that which will *ultimately* receive the tokens. For most cases, this is the spoke pool contract on L3.
+     * @param to The address of the contract that which will *ultimately* receive the tokens. For most cases, this is the spoke pool contract on L3.
      * @dev While `relayMessage` also assumes that `target` is correct, this function has the potential of deleting funds if `target` is incorrectly set.
      * This should be guarded by the logic of the Hub Pool on L1, since the Hub Pool will always set `target` to the L3 spoke pool per UMIP-157.
+     * @dev This function does not perform the bridging action. Instead, it receives information from the hub pool on L1 and saves it to state, so that it
+     * may be executed in the future by any EOA.
      */
     function relayTokens(
-        address baseToken,
-        address destinationChainToken,
+        address l2Token,
+        address l3Token,
         uint256 amount,
         uint256 destinationChainId,
-        address target
+        address to
     ) external payable override onlyAdmin {
-        address adapter = chainAdapters[destinationChainId];
+        uint32 tokenRelayId = uint32(tokenRelays.length);
+        // Create a TokenRelay struct with all the information provided.
+        TokenRelay memory tokenRelay = TokenRelay({
+            l2Token: l2Token,
+            l3Token: l3Token,
+            to: to,
+            amount: amount,
+            destinationChainId: destinationChainId,
+            executed: false
+        });
+        // Save the token relay to state.
+        tokenRelays.push(tokenRelay);
+        emit ReceivedTokenRelay(tokenRelayId, tokenRelay);
+    }
+
+    /**
+     * @notice Sends a stored TokenRelay to L3.
+     * @param tokenRelayId Index of the relay to send in the `tokenRelays` array.
+     */
+    function executeRelayTokens(uint32 tokenRelayId) external payable nonReentrant {
+        if (tokenRelayId >= tokenRelays.length) revert InvalidTokenRelayId();
+        TokenRelay storage tokenRelay = tokenRelays[tokenRelayId];
+        if (tokenRelay.executed) revert TokenRelayExecuted();
+
+        address adapter = chainAdapters[tokenRelay.destinationChainId];
         if (adapter == address(0)) revert UninitializedChainAdapter();
-        if (baseToken == address(WRAPPED_NATIVE_TOKEN)) {
+        if (tokenRelay.l2Token == address(WRAPPED_NATIVE_TOKEN)) {
             // Only wrap the minimum required amount of the native token.
             uint256 wrappedNativeTokenBalance = WRAPPED_NATIVE_TOKEN.balanceOf(address(this));
-            if (wrappedNativeTokenBalance < amount) _wrapNativeToken(amount - wrappedNativeTokenBalance);
+            if (wrappedNativeTokenBalance < tokenRelay.amount)
+                _wrapNativeToken(tokenRelay.amount - wrappedNativeTokenBalance);
         }
 
+        tokenRelay.executed = true;
         (bool success, ) = adapter.delegatecall(
-            abi.encodeCall(AdapterInterface.relayTokens, (baseToken, destinationChainToken, amount, target))
+            abi.encodeCall(
+                AdapterInterface.relayTokens,
+                (tokenRelay.l2Token, tokenRelay.l3Token, tokenRelay.amount, tokenRelay.to)
+            )
         );
-        if (!success) revert RelayTokensFailed(baseToken);
-        emit TokensForwarded(baseToken, destinationChainToken, amount, destinationChainId, target);
+        if (!success) revert RelayTokensFailed(tokenRelay.l2Token);
+
+        emit ExecutedTokenRelay(tokenRelayId);
     }
 
     // Function to be overridden in order to authenticate that messages sent to this contract originated

contracts/chain-adapters/interfaces/ForwarderInterface.sol

@@ -7,15 +7,25 @@ pragma solidity ^0.8.0;
  */
 
 interface ForwarderInterface {
-    event MessageForwarded(address indexed target, uint256 indexed chainId, bytes message);
+    // A TokenRelay defines all the information a forwarder needs to send tokens to an L3.
+    struct TokenRelay {
+        // The address of the token on L2 to send to L3.
+        address l2Token;
+        // The address of the token on L3 to receive.
+        address l3Token;
+        // The L3 address of the recipient.
+        address to;
+        // The amount of the L2 token to send over the L2-L3 bridge.
+        uint256 amount;
+        // The chain ID of the L3 to send tokens to.
+        uint256 destinationChainId;
+        // A yes/no value for determining whether the token relay which was stored has been executed.
+        bool executed;
+    }
 
-    event TokensForwarded(
-        address baseToken,
-        address remoteToken,
-        uint256 amount,
-        uint256 indexed destinationChainId,
-        address indexed to
-    );
+    event MessageForwarded(address indexed target, uint256 indexed chainId, bytes message);
+    event ReceivedTokenRelay(uint32 indexed tokenRelayId, TokenRelay tokenRelay);
+    event ExecutedTokenRelay(uint32 indexed tokenRelayId);
 
     /**
      * @notice Send message to `target` on L3.
@@ -50,4 +60,8 @@ interface ForwarderInterface {
         uint256 destinationChainId,
         address to
     ) external payable;
+
+    error RelayMessageFailed();
+    error RelayTokensFailed(address l2Token);
+    error TokenRelayExecuted();
 }

test/evm/foundry/local/Forwarder.t.sol

@@ -17,6 +17,7 @@ import { MockBedrockL1StandardBridge, MockBedrockCrossDomainMessenger } from "..
 import { Arbitrum_Forwarder } from "../../../../contracts/chain-adapters/Arbitrum_Forwarder.sol";
 import { ForwarderBase } from "../../../../contracts/chain-adapters/ForwarderBase.sol";
 import { CrossDomainAddressUtils } from "../../../../contracts/libraries/CrossDomainAddressUtils.sol";
+import { ForwarderInterface } from "../../../../contracts/chain-adapters/interfaces/ForwarderInterface.sol";
 
 contract Token_ERC20 is ERC20 {
     constructor(string memory name, string memory symbol) ERC20(name, symbol) {}
@@ -87,21 +88,38 @@ contract ForwarderTest is Test {
         vm.stopPrank();
     }
 
-    // Token relays should be routed through the Optimism Adapter's `relayTokens` function.
+    // Token relays should first be saved to state (when called by the cross domain admin).
+    // In a follow-up `sendTokens` call, tokens should then be routed through the Optimism
+    // Adapter's `relayTokens` function.
     function testForwardTokens(uint256 amountToSend, address random) public {
         l2Token.mint(address(arbitrumForwarder), amountToSend);
         vm.expectRevert();
         arbitrumForwarder.relayTokens(address(l2Token), address(l3Token), amountToSend, L3_CHAIN_ID, random);
 
+        // Save token info to state.
         vm.startPrank(aliasedOwner);
+        vm.expectEmit(address(arbitrumForwarder));
+        emit ForwarderInterface.ReceivedTokenRelay(
+            0,
+            ForwarderInterface.TokenRelay(address(l2Token), address(l3Token), random, amountToSend, L3_CHAIN_ID, false)
+        );
+        arbitrumForwarder.relayTokens(address(l2Token), address(l3Token), amountToSend, L3_CHAIN_ID, random);
+        vm.stopPrank();
+
+        // Execute a saved token relay.
+        vm.startPrank(random);
         vm.expectEmit(address(standardBridge));
         emit MockBedrockL1StandardBridge.ERC20DepositInitiated(
             random,
             address(l2Token),
             address(l3Token),
             amountToSend
         );
-        arbitrumForwarder.relayTokens(address(l2Token), address(l3Token), amountToSend, L3_CHAIN_ID, random);
+        arbitrumForwarder.executeRelayTokens(0);
+
+        // Verify a relay cannot be executed twice.
+        vm.expectRevert(ForwarderInterface.TokenRelayExecuted.selector);
+        arbitrumForwarder.executeRelayTokens(0);
         vm.stopPrank();
     }