feat(SpokePoolPeriphery): Support multiple exchanges

contracts/SpokePoolV3Periphery.sol

@@ -11,28 +11,71 @@ import { V3SpokePoolInterface } from "./interfaces/V3SpokePoolInterface.sol";
 import { IERC20Auth } from "./external/interfaces/IERC20Auth.sol";
 import { WETH9Interface } from "./external/interfaces/WETH9Interface.sol";
 
+/**
+ * @title SpokePoolProxy
+ * @notice User should only interact with SpokePool via the SpokePoolV3Periphery contract through this
+ * contract. This is purposefully a simple passthrough contract so that the user only approves this contract
+ * to pull its assets while the SpokePoolV3Periphery contract can be used to call any calldata on any exchange
+ * that the user wants to. By separating the contract that gets approved from the contract that executes arbitrary
+ * calldata, the SpokePoolPeriphery does not need to validate the calldata that gets executed.
+ * @dev If this proxy didn't exist and users instead approved and interacted directly with the SpokePoolV3Periphery
+ * then users would run the unneccessary risk that another user could instruct the Periphery contract to steal
+ * any approved tokens that the user had left outstanding.
+ */
+contract SpokePoolProxy is Lockable {
+    using SafeERC20 for IERC20;
+
+    error LeftoverInputTokens();
+
+    // The SpokePoolPeriphery should be deterministically deployed at the same address across all networks,
+    // so this contract should also be able to be deterministically deployed at the same address across all networks
+    // since the periphery address is the only constructor argument.
+    address public immutable SPOKE_POOL_PERIPHERY;
+
+    constructor(address _spokePoolPeriphery) {
+        SPOKE_POOL_PERIPHERY = _spokePoolPeriphery;
+    }
+
+    /**
+     * @notice Caller must insure that exactly `inputAmount` of `inputToken` is used in the subsequent call to
+     * the SpokePoolPeriphery contract. All of the periphery functions have an easily identifiable input
+     * amount and input token so this should be easy to verify. Any leftover tokens would be locked in this contract
+     * and could be used by ANYONE in a subsequent call to the SpokePoolPeriphery contract. So, this function
+     * attempts to protect the user from locking tokens by reverting if not exactly `inputAmount` of `inputToken`
+     * is used in the periphery contract call.
+     */
+    function callSpokePoolPeriphery(
+        address inputToken,
+        uint256 inputAmount,
+        bytes memory peripheryFunctionCalldata
+    ) external payable nonReentrant {
+        uint256 balanceBefore = IERC20(inputToken).balanceOf(address(this));
+        IERC20(inputToken).safeTransferFrom(msg.sender, address(this), inputAmount);
+        IERC20(inputToken).forceApprove(SPOKE_POOL_PERIPHERY, inputAmount);
+        // solhint-disable-next-line avoid-low-level-calls
+        (bool success, bytes memory result) = SPOKE_POOL_PERIPHERY.call{ value: msg.value }(peripheryFunctionCalldata);
+        require(success, string(result));
+        uint256 balanceAfter = IERC20(inputToken).balanceOf(address(this));
+        if (balanceAfter != balanceBefore) revert LeftoverInputTokens();
+    }
+}
+
 /**
  * @title SpokePoolV3Periphery
- * @notice Contract for performing more complex interactions with an AcrossV3 spoke pool deployment.
- * @dev Variables which may be immutable are not marked as immutable, nor defined in the constructor, so that this contract may be deployed deterministically.
+ * @notice Contract for performing more complex interactions with an Across spoke pool deployment.
+ * @dev Variables which may be immutable are not marked as immutable, nor defined in the constructor, so that this
+ * contract may be deployed deterministically at the same address across different networks.
  * @custom:security-contact [email protected]
  */
 contract SpokePoolV3Periphery is Lockable, MultiCaller {
     using SafeERC20 for IERC20;
     using Address for address;
 
-    // This contract performs a low level call with arbirary data to an external contract. This is a large attack
-    // surface and we should whitelist which function selectors are allowed to be called on the exchange.
-    mapping(bytes4 => bool) public allowedSelectors;
-
     // Across SpokePool we'll submit deposits to with acrossInputToken as the input token.
     V3SpokePoolInterface public spokePool;
 
-    // Exchange address or router where the swapping will happen.
-    address public exchange;
-
     // Wrapped native token contract address.
-    WETH9Interface internal wrappedNativeToken;
+    WETH9Interface public wrappedNativeToken;
 
     // Boolean indicating whether the contract is initialized.
     bool private initialized;
@@ -52,7 +95,7 @@ contract SpokePoolV3Periphery is Lockable, MultiCaller {
         // chain and if this is a contract then they will receive an ERC20.
         address recipient;
         // The destination chain identifier.
-        uint256 destinationChainid;
+        uint256 destinationChainId;
         // The account that can exclusively fill the deposit before the exclusivity parameter.
         address exclusiveRelayer;
         // Timestamp of the deposit used by system to charge fees. Must be within short window of time into the past
@@ -69,6 +112,7 @@ contract SpokePoolV3Periphery is Lockable, MultiCaller {
 
     event SwapBeforeBridge(
         address exchange,
+        bytes exchangeCalldata,
         address indexed swapToken,
         address indexed acrossInputToken,
         uint256 swapTokenAmount,
@@ -82,45 +126,36 @@ contract SpokePoolV3Periphery is Lockable, MultiCaller {
      ****************************************/
     error MinimumExpectedInputAmount();
     error LeftoverSrcTokens();
-    error InvalidFunctionSelector();
     error ContractInitialized();
     error InvalidMsgValue();
     error InvalidSpokePool();
     error InvalidSwapToken();
 
     /**
      * @notice Construct a new SwapAndBridgeBase contract.
-     * @param _allowedSelectors Function selectors that are allowed to be called on the exchange.
+     * @dev Is empty and all of the state variables are initialized in the initialize function
+     * to allow for deployment at a deterministic address via create2, which requires that the bytecode
+     * across different networks is the same. Constructor parameters affect the bytecode so we can only
+     * add parameters here that are consistent across networks.
      */
-    constructor(bytes4[] memory _allowedSelectors) {
-        for (uint256 i = 0; i < _allowedSelectors.length; i++) {
-            allowedSelectors[_allowedSelectors[i]] = true;
-        }
-    }
+    constructor() {}
 
     /**
      * @notice Initializes the SwapAndBridgeBase contract.
+     * @dev Only the owner can call this function.
      * @param _spokePool Address of the SpokePool contract that we'll submit deposits to.
      * @param _wrappedNativeToken Address of the wrapped native token for the network this contract is deployed to.
-     * @param _exchange Address of the exchange where tokens will be swapped.
      * @dev These values are initialized in a function and not in the constructor so that the creation code of this contract
-     * is the same across networks with different addresses for the wrapped native token, the exchange this contract uses to
-     * swap and bridge, and this network's corresponding spoke pool contract. This is to allow this contract to be deterministically
-     * deployed with CREATE2.
-     * @dev This function can be front-run by anybody, so it is critical to check that the `spokePool`, `wrappedNativeToken`, and `exchange`
-     * values used in the single call to this function were passed in correctly before enabling the usage of this contract.
+     * is the same across networks with different addresses for the wrapped native token and this network's
+     * corresponding spoke pool contract. This is to allow this contract to be deterministically deployed with CREATE2.
      */
-    function initialize(
-        V3SpokePoolInterface _spokePool,
-        WETH9Interface _wrappedNativeToken,
-        address _exchange
-    ) external {
+    function initialize(V3SpokePoolInterface _spokePool, WETH9Interface _wrappedNativeToken) external nonReentrant {
         if (initialized) revert ContractInitialized();
         initialized = true;
 
+        if (!address(_spokePool).isContract()) revert InvalidSpokePool();
         spokePool = _spokePool;
         wrappedNativeToken = _wrappedNativeToken;
-        exchange = _exchange;
     }
 
     /**
@@ -184,6 +219,7 @@ contract SpokePoolV3Periphery is Lockable, MultiCaller {
      * the assumption is that this function will handle only ERC20 tokens.
      * @param swapToken Address of the token that will be swapped for acrossInputToken.
      * @param acrossInputToken Address of the token that will be bridged via Across as the inputToken.
+     * @param exchange Address of the exchange contract to call.
      * @param routerCalldata ABI encoded function data to call on router. Should form a swap of swapToken for
      * enough of acrossInputToken, otherwise this function will revert.
      * @param swapTokenAmount Amount of swapToken to swap for a minimum amount of depositData.inputToken.
@@ -194,6 +230,7 @@ contract SpokePoolV3Periphery is Lockable, MultiCaller {
     function swapAndBridge(
         IERC20 swapToken,
         IERC20 acrossInputToken,
+        address exchange,
         bytes calldata routerCalldata,
         uint256 swapTokenAmount,
         uint256 minExpectedInputTokenAmount,
@@ -209,6 +246,7 @@ contract SpokePoolV3Periphery is Lockable, MultiCaller {
             swapToken.safeTransferFrom(msg.sender, address(this), swapTokenAmount);
         }
         _swapAndBridge(
+            exchange,
             routerCalldata,
             swapTokenAmount,
             minExpectedInputTokenAmount,
@@ -224,6 +262,7 @@ contract SpokePoolV3Periphery is Lockable, MultiCaller {
      * @dev If swapToken does not implement `permit` to the specifications of EIP-2612, this function will fail.
      * @param swapToken Address of the token that will be swapped for acrossInputToken.
      * @param acrossInputToken Address of the token that will be bridged via Across as the inputToken.
+     * @param exchange Address of the exchange contract to call.
      * @param routerCalldata ABI encoded function data to call on router. Should form a swap of swapToken for
      * enough of acrossInputToken, otherwise this function will revert.
      * @param swapTokenAmount Amount of swapToken to swap for a minimum amount of depositData.inputToken.
@@ -238,6 +277,7 @@ contract SpokePoolV3Periphery is Lockable, MultiCaller {
     function swapAndBridgeWithPermit(
         IERC20Permit swapToken,
         IERC20 acrossInputToken,
+        address exchange,
         bytes calldata routerCalldata,
         uint256 swapTokenAmount,
         uint256 minExpectedInputTokenAmount,
@@ -255,6 +295,7 @@ contract SpokePoolV3Periphery is Lockable, MultiCaller {
 
         _swapToken.safeTransferFrom(msg.sender, address(this), swapTokenAmount);
         _swapAndBridge(
+            exchange,
             routerCalldata,
             swapTokenAmount,
             minExpectedInputTokenAmount,
@@ -270,6 +311,7 @@ contract SpokePoolV3Periphery is Lockable, MultiCaller {
      * @dev If swapToken does not implement `receiveWithAuthorization` to the specifications of EIP-3009, this call will revert.
      * @param swapToken Address of the token that will be swapped for acrossInputToken.
      * @param acrossInputToken Address of the token that will be bridged via Across as the inputToken.
+     * @param exchange Address of the exchange contract to call.
      * @param routerCalldata ABI encoded function data to call on router. Should form a swap of swapToken for
      * enough of acrossInputToken, otherwise this function will revert.
      * @param swapTokenAmount Amount of swapToken to swap for a minimum amount of depositData.inputToken.
@@ -286,6 +328,7 @@ contract SpokePoolV3Periphery is Lockable, MultiCaller {
     function swapAndBridgeWithAuthorization(
         IERC20Auth swapToken,
         IERC20 acrossInputToken,
+        address exchange,
         bytes calldata routerCalldata,
         uint256 swapTokenAmount,
         uint256 minExpectedInputTokenAmount,
@@ -314,6 +357,7 @@ contract SpokePoolV3Periphery is Lockable, MultiCaller {
         IERC20 _swapToken = IERC20(address(swapToken)); // Cast IERC20Auth to IERC20.
 
         _swapAndBridge(
+            exchange,
             routerCalldata,
             swapTokenAmount,
             minExpectedInputTokenAmount,
@@ -411,7 +455,7 @@ contract SpokePoolV3Periphery is Lockable, MultiCaller {
             depositData.outputToken, // output token
             _acrossInputAmount, // input amount.
             depositData.outputAmount, // output amount
-            depositData.destinationChainid,
+            depositData.destinationChainId,
             depositData.exclusiveRelayer,
             depositData.quoteTimestamp,
             depositData.fillDeadline,
@@ -422,18 +466,14 @@ contract SpokePoolV3Periphery is Lockable, MultiCaller {
 
     // This contract supports two variants of swap and bridge, one that allows one token and another that allows the caller to pass them in.
     function _swapAndBridge(
+        address exchange,
         bytes calldata routerCalldata,
         uint256 swapTokenAmount,
         uint256 minExpectedInputTokenAmount,
         DepositData calldata depositData,
         IERC20 _swapToken,
         IERC20 _acrossInputToken
     ) private {
-        // Note: this check should never be impactful, but is here out of an abundance of caution.
-        // For example, if the exchange address in the contract is also an ERC20 token that is approved by some
-        // user on this contract, a malicious actor could call transferFrom to steal the user's tokens.
-        if (!allowedSelectors[bytes4(routerCalldata)]) revert InvalidFunctionSelector();
-
         // Swap and run safety checks.
         uint256 srcBalanceBefore = _swapToken.balanceOf(address(this));
         uint256 dstBalanceBefore = _acrossInputToken.balanceOf(address(this));
@@ -444,6 +484,8 @@ contract SpokePoolV3Periphery is Lockable, MultiCaller {
         require(success, string(result));
 
         _checkSwapOutputAndDeposit(
+            exchange,
+            routerCalldata,
             swapTokenAmount,
             srcBalanceBefore,
             dstBalanceBefore,
@@ -462,6 +504,8 @@ contract SpokePoolV3Periphery is Lockable, MultiCaller {
      * @param minExpectedInputTokenAmount Minimum amount of received acrossInputToken that we'll bridge
      **/
     function _checkSwapOutputAndDeposit(
+        address exchange,
+        bytes memory routerCalldata,
         uint256 swapTokenAmount,
         uint256 swapTokenBalanceBefore,
         uint256 inputTokenBalanceBefore,
@@ -480,6 +524,7 @@ contract SpokePoolV3Periphery is Lockable, MultiCaller {
 
         emit SwapBeforeBridge(
             exchange,
+            routerCalldata,
             address(_swapToken),
             address(_acrossInputToken),
             swapTokenAmount,