feat: don't settle outputs in reactor (#10)

* Refactor tests and add arbitrary call

* forge fmt

* add comment

* add comment

* Remove output token and lib

* Remove output tokens and update tests

* bubble up revert reason

* Remove unused currencyLibary

* Change local imports to remote and natspe

* forge fmt

* Re-add errors

* remove constructor param (#13)

.forge-snapshots/RelayOrderReactorIntegrationTest-testExecute.snap

@@ -1 +1 @@
-288593
+254346

.forge-snapshots/RelayOrderReactorIntegrationTest-testExecuteWithNativeAsOutput.snap

@@ -1 +1 @@
-291187
+280710

.forge-snapshots/RelayOrderReactorIntegrationTest-testPermitAndExecute.snap

@@ -1 +1 @@
-340639
+310415

src/base/ReactorErrors.sol

@@ -9,10 +9,6 @@ interface ReactorErrors {
     // A nested call failed
     error CallFailed();
 
-    error InvalidToken();
-    error UnsupportedAction();
-    error ReactorCallbackNotSupported();
-
     /// @notice thrown when an order's deadline is before its end time
     error DeadlineBeforeEndTime();
 

src/base/ReactorStructs.sol

@@ -17,7 +17,6 @@ struct ResolvedRelayOrder {
     OrderInfo info;
     bytes[] actions;
     InputTokenWithRecipient[] inputs;
-    OutputToken[] outputs;
     bytes sig;
     bytes32 hash;
 }

src/lib/RelayOrderLib.sol

@@ -1,7 +1,7 @@
 // SPDX-License-Identifier: GPL-2.0-or-later
 pragma solidity ^0.8.0;
 
-import {OrderInfo, OutputToken} from "UniswapX/src/base/ReactorStructs.sol";
+import {OrderInfo} from "UniswapX/src/base/ReactorStructs.sol";
 import {OrderInfoLib} from "UniswapX/src/lib/OrderInfoLib.sol";
 import {InputTokenWithRecipient} from "../base/ReactorStructs.sol";
 
@@ -17,8 +17,6 @@ struct RelayOrder {
     bytes[] actions;
     // The tokens that the swapper will provide when settling the order
     InputTokenWithRecipient[] inputs;
-    // The tokens that must be received to satisfy the order
-    OutputToken[] outputs;
 }
 
 /// @notice helpers for handling relay order objects
@@ -27,10 +25,8 @@ library RelayOrderLib {
 
     bytes private constant INPUT_TOKEN_TYPE =
         "InputTokenWithRecipient(address token,uint256 amount,uint256 maxAmount,address recipient)";
-    bytes private constant OUTPUT_TOKEN_TYPE = "OutputToken(address token,uint256 amount,address recipient)";
 
     bytes32 private constant INPUT_TOKEN_TYPE_HASH = keccak256(INPUT_TOKEN_TYPE);
-    bytes32 private constant OUTPUT_TOKEN_TYPE_HASH = keccak256(OUTPUT_TOKEN_TYPE);
 
     bytes internal constant ORDER_TYPE = abi.encodePacked(
         "RelayOrder(",
@@ -41,8 +37,7 @@ library RelayOrderLib {
         "InputTokenWithRecipient[] inputs,",
         "OutputToken[] outputs)",
         OrderInfoLib.ORDER_INFO_TYPE,
-        INPUT_TOKEN_TYPE,
-        OUTPUT_TOKEN_TYPE
+        INPUT_TOKEN_TYPE
     );
     bytes32 internal constant ORDER_TYPE_HASH = keccak256(ORDER_TYPE);
 
@@ -55,11 +50,6 @@ library RelayOrderLib {
         return keccak256(abi.encode(INPUT_TOKEN_TYPE_HASH, input.token, input.amount, input.maxAmount));
     }
 
-    /// @notice returns the hash of an output token struct
-    function hash(OutputToken memory output) private pure returns (bytes32) {
-        return keccak256(abi.encode(OUTPUT_TOKEN_TYPE_HASH, output.token, output.amount, output.recipient));
-    }
-
     /// @notice returns the hash of an input token struct
     function hash(InputTokenWithRecipient[] memory inputs) private pure returns (bytes32) {
         unchecked {
@@ -76,22 +66,6 @@ library RelayOrderLib {
         }
     }
 
-    /// @notice returns the hash of an output token struct
-    function hash(OutputToken[] memory outputs) private pure returns (bytes32) {
-        unchecked {
-            bytes memory packedHashes = new bytes(32 * outputs.length);
-
-            for (uint256 i = 0; i < outputs.length; i++) {
-                bytes32 outputHash = hash(outputs[i]);
-                assembly {
-                    mstore(add(add(packedHashes, 0x20), mul(i, 0x20)), outputHash)
-                }
-            }
-
-            return keccak256(packedHashes);
-        }
-    }
-
     /// @notice hash the given order
     /// @param order the order to hash
     /// @return the eip-712 order hash
@@ -103,8 +77,7 @@ library RelayOrderLib {
                 order.decayStartTime,
                 order.decayEndTime,
                 order.actions,
-                hash(order.inputs),
-                hash(order.outputs)
+                hash(order.inputs)
             )
         );
     }

src/reactors/RelayOrderReactor.sol

@@ -5,19 +5,20 @@ import {SafeTransferLib} from "solmate/src/utils/SafeTransferLib.sol";
 import {ReentrancyGuard} from "@openzeppelin/contracts/security/ReentrancyGuard.sol";
 import {ERC20} from "solmate/src/tokens/ERC20.sol";
 import {IPermit2} from "permit2/src/interfaces/IPermit2.sol";
-import {SignedOrder, OrderInfo, OutputToken} from "UniswapX/src/base/ReactorStructs.sol";
+import {SignedOrder, OrderInfo} from "UniswapX/src/base/ReactorStructs.sol";
+import {ReactorEvents} from "UniswapX/src/base/ReactorEvents.sol";
+import {CurrencyLibrary} from "UniswapX/src/lib/CurrencyLibrary.sol";
 import {IRelayOrderReactor} from "../interfaces/IRelayOrderReactor.sol";
-import {ReactorEvents} from "../base/ReactorEvents.sol";
-import {ReactorErrors} from "../base/ReactorErrors.sol";
 import {InputTokenWithRecipient, ResolvedRelayOrder} from "../base/ReactorStructs.sol";
-import {CurrencyLibrary, NATIVE} from "../lib/CurrencyLibrary.sol";
+import {ReactorErrors} from "../base/ReactorErrors.sol";
 import {Permit2Lib} from "../lib/Permit2Lib.sol";
 import {RelayOrderLib, RelayOrder} from "../lib/RelayOrderLib.sol";
 import {ResolvedRelayOrderLib} from "../lib/ResolvedRelayOrderLib.sol";
 import {RelayDecayLib} from "../lib/RelayDecayLib.sol";
 
 /// @notice Reactor for handling the execution of RelayOrders
 /// @notice This contract MUST NOT have approvals or priviledged access
+/// @notice any funds in this contract can be swept away by anyone
 contract RelayOrderReactor is ReactorEvents, ReactorErrors, ReentrancyGuard, IRelayOrderReactor {
     using SafeTransferLib for ERC20;
     using CurrencyLibrary for address;
@@ -29,11 +30,8 @@ contract RelayOrderReactor is ReactorEvents, ReactorErrors, ReentrancyGuard, IRe
     /// @notice permit2 address used for token transfers and signature verification
     IPermit2 public immutable permit2;
 
-    address public immutable universalRouter;
-
-    constructor(IPermit2 _permit2, address _universalRouter) {
+    constructor(IPermit2 _permit2) {
         permit2 = _permit2;
-        universalRouter = _universalRouter;
     }
 
     function execute(SignedOrder calldata order) external payable nonReentrant {
@@ -86,7 +84,7 @@ contract RelayOrderReactor is ReactorEvents, ReactorErrors, ReentrancyGuard, IRe
         }
     }
 
-    /// @notice validates, injects fees, and transfers input tokens in preparation for order fill
+    /// @notice validates and transfers input tokens in preparation for order fill
     /// @param orders The orders to prepare
     function _prepare(ResolvedRelayOrder[] memory orders) internal {
         uint256 ordersLength = orders.length;
@@ -102,31 +100,17 @@ contract RelayOrderReactor is ReactorEvents, ReactorErrors, ReentrancyGuard, IRe
         }
     }
 
-    /// @notice fills a list of orders, ensuring all outputs are satisfied
-    /// @param orders The orders to fill
+    /// @notice emits a Fill event for each order
+    /// @notice all output token checks must be done in the encoded actions within the order
+    /// @param orders The orders that have been filled
     function _fill(ResolvedRelayOrder[] memory orders) internal {
         uint256 ordersLength = orders.length;
-        // attempt to transfer all currencies to all recipients
         unchecked {
-            // transfer output tokens to their respective recipients
             for (uint256 i = 0; i < ordersLength; i++) {
                 ResolvedRelayOrder memory resolvedOrder = orders[i];
-                uint256 outputsLength = resolvedOrder.outputs.length;
-                for (uint256 j = 0; j < outputsLength; j++) {
-                    OutputToken memory output = resolvedOrder.outputs[j];
-                    output.token.transferFillFromBalance(output.recipient, output.amount);
-                }
-
                 emit Fill(orders[i].hash, msg.sender, resolvedOrder.info.swapper, resolvedOrder.info.nonce);
             }
         }
-
-        // refund any remaining ETH to the filler. Only occurs when filler sends more ETH than required to
-        // `execute()` or `executeBatch()`, or when there is excess contract balance remaining from others
-        // incorrectly calling execute/executeBatch without direct filler method but with a msg.value
-        if (address(this).balance > 0) {
-            CurrencyLibrary.transferNative(msg.sender, address(this).balance);
-        }
     }
 
     receive() external payable {
@@ -145,7 +129,6 @@ contract RelayOrderReactor is ReactorEvents, ReactorErrors, ReentrancyGuard, IRe
             info: order.info,
             actions: order.actions,
             inputs: order.inputs.decay(order.decayStartTime, order.decayEndTime),
-            outputs: order.outputs,
             sig: signedOrder.sig,
             hash: order.hash()
         });

src/sample-executors/PermitExecutor.sol

@@ -5,8 +5,8 @@ import {ERC20} from "solmate/src/tokens/ERC20.sol";
 import {Owned} from "solmate/src/auth/Owned.sol";
 import {SafeTransferLib} from "solmate/src/utils/SafeTransferLib.sol";
 import {SignedOrder} from "UniswapX/src/base/ReactorStructs.sol";
+import {CurrencyLibrary} from "UniswapX/src/lib/CurrencyLibrary.sol";
 import {IRelayOrderReactor} from "../interfaces/IRelayOrderReactor.sol";
-import {CurrencyLibrary} from "../lib/CurrencyLibrary.sol";
 
 /// @notice A simple fill contract that relays 2612 style permits on chain before filling a Relay order
 contract PermitExecutor is Owned {

test/foundry-tests/integration/RelayOrderReactorIntegration.t.sol

@@ -6,13 +6,12 @@ import {SafeTransferLib} from "solmate/src/utils/SafeTransferLib.sol";
 import {ERC20} from "solmate/src/tokens/ERC20.sol";
 import {Test, stdJson} from "forge-std/Test.sol";
 import {IPermit2} from "permit2/src/interfaces/IPermit2.sol";
-import {OrderInfo, OutputToken, SignedOrder} from "UniswapX/src/base/ReactorStructs.sol";
+import {OrderInfo, SignedOrder} from "UniswapX/src/base/ReactorStructs.sol";
 import {OrderInfoBuilder} from "UniswapX/test/util/OrderInfoBuilder.sol";
-import {OutputsBuilder} from "UniswapX/test/util/OutputsBuilder.sol";
 import {ArrayBuilder} from "UniswapX/test/util/ArrayBuilder.sol";
+import {CurrencyLibrary} from "UniswapX/src/lib/CurrencyLibrary.sol";
 import {InputTokenWithRecipient, ResolvedRelayOrder} from "../../../src/base/ReactorStructs.sol";
 import {ReactorEvents} from "../../../src/base/ReactorEvents.sol";
-import {CurrencyLibrary} from "../../../src/lib/CurrencyLibrary.sol";
 import {PermitSignature} from "../util/PermitSignature.sol";
 import {RelayOrderLib, RelayOrder} from "../../../src/lib/RelayOrderLib.sol";
 import {RelayOrderReactor} from "../../../src/reactors/RelayOrderReactor.sol";
@@ -65,7 +64,7 @@ contract RelayOrderReactorIntegrationTest is GasSnapshot, Test, Interop, PermitS
         json = vm.readFile(string.concat(root, "/test/foundry-tests/interop.json"));
         vm.createSelectFork(vm.envString("FOUNDRY_RPC_URL"), 17972788);
 
-        deployCodeTo("RelayOrderReactor.sol", abi.encode(PERMIT2, UNIVERSAL_ROUTER), RELAY_ORDER_REACTOR);
+        deployCodeTo("RelayOrderReactor.sol", abi.encode(PERMIT2), RELAY_ORDER_REACTOR);
         reactor = RelayOrderReactor(RELAY_ORDER_REACTOR);
         permitExecutor = new PermitExecutor(address(filler), reactor, address(filler));
 
@@ -118,8 +117,7 @@ contract RelayOrderReactorIntegrationTest is GasSnapshot, Test, Interop, PermitS
             decayStartTime: block.timestamp,
             decayEndTime: block.timestamp + 100,
             actions: actions,
-            inputs: inputTokens,
-            outputs: OutputsBuilder.single(address(USDC), amountOutMin, address(swapper))
+            inputs: inputTokens
         });
 
         SignedOrder memory signedOrder =
@@ -193,16 +191,15 @@ contract RelayOrderReactorIntegrationTest is GasSnapshot, Test, Interop, PermitS
         );
 
         bytes[] memory actions = new bytes[](1);
-        MethodParameters memory methodParameters = readFixture(json, "._UNISWAP_V3_USDC_DAI");
+        MethodParameters memory methodParameters = readFixture(json, "._UNISWAP_V3_USDC_DAI_SWAPPER2");
         actions[0] = abi.encode(UNIVERSAL_ROUTER, methodParameters.value, methodParameters.data);
 
         RelayOrder memory order = RelayOrder({
             info: OrderInfoBuilder.init(address(reactor)).withSwapper(swapper2).withDeadline(block.timestamp + 100),
             decayStartTime: block.timestamp,
             decayEndTime: block.timestamp + 100,
             actions: actions,
-            inputs: inputTokens,
-            outputs: OutputsBuilder.single(address(DAI), amountOutMin, address(swapper2))
+            inputs: inputTokens
         });
 
         SignedOrder memory signedOrder =
@@ -262,9 +259,7 @@ contract RelayOrderReactorIntegrationTest is GasSnapshot, Test, Interop, PermitS
             decayStartTime: block.timestamp,
             decayEndTime: block.timestamp + 100,
             actions: actions,
-            inputs: inputTokens,
-            // address 0 for native output
-            outputs: OutputsBuilder.single(address(0), amountOutMin, address(swapper))
+            inputs: inputTokens
         });
 
         SignedOrder memory signedOrder =
@@ -294,7 +289,9 @@ contract RelayOrderReactorIntegrationTest is GasSnapshot, Test, Interop, PermitS
         assertEq(USDC.balanceOf(filler), fillerGasInputBalanceStart + 10 * USDC_ONE, "filler balance");
     }
 
-    function testExecuteFailsIfReactorIsNotRecipient() public {
+    // in the case wehre the swapper incorrectly sets the recipient to an address that is not theirs, but the
+    // calldata includes a SWEEP back to them which should cause the transaction to revert
+    function testExecuteDoesNotSucceedIfReactorIsRecipientAndUniversalRouterSweep() public {
         InputTokenWithRecipient[] memory inputTokens = new InputTokenWithRecipient[](2);
         inputTokens[0] =
             InputTokenWithRecipient({token: DAI, amount: 100 * ONE, maxAmount: 100 * ONE, recipient: UNIVERSAL_ROUTER});
@@ -308,23 +305,23 @@ contract RelayOrderReactorIntegrationTest is GasSnapshot, Test, Interop, PermitS
         uint256 amountOutMin = 95 * USDC_ONE;
 
         bytes[] memory actions = new bytes[](1);
-        MethodParameters memory methodParameters = readFixture(json, "._UNISWAP_V3_DAI_USDC_RECIPIENT_NOT_REACTOR");
+        MethodParameters memory methodParameters =
+            readFixture(json, "._UNISWAP_V3_DAI_USDC_RECIPIENT_REACTOR_WITH_SWEEP");
         actions[0] = abi.encode(UNIVERSAL_ROUTER, methodParameters.value, methodParameters.data);
 
         RelayOrder memory order = RelayOrder({
             info: OrderInfoBuilder.init(address(reactor)).withSwapper(swapper).withDeadline(block.timestamp + 100),
             decayStartTime: block.timestamp,
             decayEndTime: block.timestamp + 100,
             actions: actions,
-            inputs: inputTokens,
-            outputs: OutputsBuilder.single(address(USDC), amountOutMin, address(swapper))
+            inputs: inputTokens
         });
 
         SignedOrder memory signedOrder =
             SignedOrder(abi.encode(order), signOrder(swapperPrivateKey, address(PERMIT2), order));
 
         vm.prank(filler);
-        vm.expectRevert(CurrencyLibrary.InsufficientBalance.selector);
+        vm.expectRevert(0x675cae38); // InvalidToken()
         reactor.execute{value: methodParameters.value}(signedOrder);
     }