Update v1.1 (#179)

* fix: reactor callback duplicate order fill

This commit fixes a fundamental issue with the ExpectedBalanceLib
balance checks settlement methodology. Effectively if a swapper creates
an order on UniswapX and also an order with similar input/output tokens
on another protocol they can fill the other order during the
reactorCallback to satisfy balance checks without actually filling the
UniswapX order itself fully.

This commit fixes the issue by switching back to an approve/transferFrom
model so every order output is transferred explicitly from the
fillContract during settlement.

To alleviate security issues with being able to transferFrom fill
contracts in the reactor, we add a new constraint that msg.sender is
always used as the fillContract. This means all fillContract
implementations must themselves call reactor.execute. Direct filler
strategies from an EOA are now enabled during the same flow by sending
a 0 length fillData field.

Another difference is that direct fills are now processed through direct
approvals to the reactor rather than permit2. This simplifies and
unifies things, and saves gas generally for fill strategies. All
transferFrom calls out of the reactor are done with msg.sender as from

* feat: make swaprouter02executor only approve when needed

* fix: use helper instead of duplicating code

* feat: make direct taker explicit

This commit makes direct taker occur when fillData == bytes(0x01), so it
has to be explicitly chosen

* fix: readme

* feat: rename direct fill -> skip

* fix: readme

* fix: sara comments

* fix: memory -> calldata in swaprouter02executor

* fix: assume not fillContract

* feat: cache lengths

* gas limit for native transfer

* fix: natspec

* feat: add test for EOA calling execute with data

* fix: use currencylibrary in mocks

* feat: add double execution test

Fill an order on one reactor from inside the callback of another order.
Ensure both are fully satisfied

* feat: add FOT disclaimer

* fix: CVF-1 modifiers for SwapRouter02Executor

* feat: separate entrypoints for callback

CVF-2

* fix: comment

* fix: readme

* fix: Readme

* fix: cache orderslength

* update: snapshots

* fix: gas limit

---------

Co-authored-by: Emily Williams <emag3m@gmail.com>

.forge-snapshots/Base-DutchOrder-BaseExecuteSingleWithFee.snap

@@ -1 +1 @@
-138311
+150417

.forge-snapshots/Base-DutchOrder-ExecuteBatch.snap

@@ -1 +1 @@
-169496
+196879

.forge-snapshots/Base-DutchOrder-ExecuteBatchMultipleOutputs.snap

@@ -1 +1 @@
-176377
+206650

.forge-snapshots/Base-DutchOrder-ExecuteBatchMultipleOutputsDifferentTokens.snap

@@ -1 +1 @@
-207790
+260307

.forge-snapshots/Base-DutchOrder-ExecuteBatchNativeOutput.snap

@@ -1 +1 @@
-175959
+190447

.forge-snapshots/Base-DutchOrder-ExecuteSingle.snap

@@ -1 +1 @@
-124916
+148194

.forge-snapshots/Base-DutchOrder-ExecuteSingleNativeOutput.snap

@@ -1 +1 @@
-127164
+133774

.forge-snapshots/Base-DutchOrder-ExecuteSingleValidation.snap

@@ -1 +1 @@
-134226
+157505

.forge-snapshots/Base-ExclusiveDutchOrder-BaseExecuteSingleWithFee.snap

@@ -1 +1 @@
-158419
+182186

.forge-snapshots/Base-ExclusiveDutchOrder-ExecuteBatch.snap

@@ -1 +1 @@
-169874
+197283

.forge-snapshots/Base-ExclusiveDutchOrder-ExecuteBatchMultipleOutputs.snap

@@ -1 +1 @@
-176749
+207049

.forge-snapshots/Base-ExclusiveDutchOrder-ExecuteBatchMultipleOutputsDifferentTokens.snap

@@ -1 +1 @@
-208184
+260720

.forge-snapshots/Base-ExclusiveDutchOrder-ExecuteBatchNativeOutput.snap

@@ -1 +1 @@
-176329
+190845

.forge-snapshots/Base-ExclusiveDutchOrder-ExecuteSingle.snap

@@ -1 +1 @@
-125107
+148401

.forge-snapshots/Base-ExclusiveDutchOrder-ExecuteSingleNativeOutput.snap

@@ -1 +1 @@
-127352
+133986

.forge-snapshots/Base-ExclusiveDutchOrder-ExecuteSingleValidation.snap

@@ -1 +1 @@
-134422
+157716

.forge-snapshots/Base-LimitOrderReactor-BaseExecuteSingleWithFee.snap

@@ -1 +1 @@
-123620
+146554

.forge-snapshots/Base-LimitOrderReactor-ExecuteBatch.snap

@@ -1 +1 @@
-161701
+189056

.forge-snapshots/Base-LimitOrderReactor-ExecuteBatchMultipleOutputs.snap

@@ -1 +1 @@
-167646
+197889

.forge-snapshots/Base-LimitOrderReactor-ExecuteBatchMultipleOutputsDifferentTokens.snap

@@ -1 +1 @@
-198131
+250617

.forge-snapshots/Base-LimitOrderReactor-ExecuteBatchNativeOutput.snap

@@ -1 +1 @@
-168164
+182618

.forge-snapshots/Base-LimitOrderReactor-ExecuteSingle.snap

@@ -1 +1 @@
-121070
+124433

.forge-snapshots/Base-LimitOrderReactor-ExecuteSingleNativeOutput.snap

@@ -1 +1 @@
-123319
+129913

.forge-snapshots/Base-LimitOrderReactor-ExecuteSingleValidation.snap

@@ -1 +1 @@
-130381
+153643

.forge-snapshots/DirectFillerFillMacroSingleOrder.snap

@@ -1 +1 @@
-140223
+136213

.forge-snapshots/DirectFillerFillMacroSingleOrderWithFee.snap

@@ -1 +1 @@
-181893
+175064

.forge-snapshots/DirectFillerFillMacroTestEth1Output.snap

@@ -1 +1 @@
-147989
+147399

.forge-snapshots/DirectFillerFillMacroTestEth2Outputs.snap

@@ -1 +1 @@
-171264
+170622

.forge-snapshots/DirectFillerFillMacroThreeOrdersWithFees.snap

@@ -1 +1 @@
-454545
+435187

.forge-snapshots/DirectFillerFillMacroTwoOrders.snap

@@ -1 +1 @@
-264858
+255972

.forge-snapshots/EthOutputTest3OrdersWithEthAndERC20Outputs.snap

@@ -1 +1 @@
-326649
+363514

.forge-snapshots/EthOutputTestEthOutput.snap

@@ -1 +1 @@
-149627
+156241

.forge-snapshots/ProtocolFeesGasComparisonTest-InterfaceAndProtocolFee.snap

@@ -1 +1 @@
-152826
+176915

.forge-snapshots/ProtocolFeesGasComparisonTest-InterfaceAndProtocolFeeEthOutput.snap

@@ -1 +1 @@
-144858
+162961

.forge-snapshots/ProtocolFeesGasComparisonTest-InterfaceFee.snap

@@ -1 +1 @@
-142255
+166046

.forge-snapshots/ProtocolFeesGasComparisonTest-InterfaceFeeEthOutput.snap

@@ -1 +1 @@
-134441
+146909

.forge-snapshots/ProtocolFeesGasComparisonTest-NoFees.snap

@@ -1 +1 @@
-125972
+149267

.forge-snapshots/ProtocolFeesGasComparisonTest-NoFeesEthOutput.snap

@@ -1 +1 @@
-118312
+124947

.forge-snapshots/SwapRouter02ExecutorExecute.snap

@@ -0,0 +1 @@
+262973

.forge-snapshots/SwapRouter02ExecutorExecuteAlreadyApproved.snap

@@ -0,0 +1 @@
+118092

.forge-snapshots/testExclusiveFillerSucceeds.snap

@@ -1 +1 @@
-150778
+174066

README.md

@@ -19,7 +19,7 @@ Reactors process orders using the following steps:
 - Resolve the order into inputs and outputs
 - Pull input tokens from the swapper to the fillContract using permit2 `permitWitnessTransferFrom` with the order as witness
 - Call `reactorCallback` on the fillContract
-- Verify that the output tokens were received by the output recipients
+- Transfer output tokens from the fillContract to the output recipients
 
 Reactors implement the [IReactor](./src/interfaces/IReactor.sol) interface which abstracts the specifics of the order specification. This allows for different reactor implementations with different order formats to be used with the same interface, allowing for shared infrastructure and easy extension by fillers.
 
@@ -30,14 +30,14 @@ Current reactor implementations:
 
 ### Fill Contracts
 
-Order fillContracts _fill_ UniswapX orders. They specify the filler's strategy for fulfilling orders and are called by the reactor with `reactorCallback`.
+Order fillContracts _fill_ UniswapX orders. They specify the filler's strategy for fulfilling orders and are called by the reactor with `reactorCallback` when using `executeWithCallback` or `executeBatchWithCallback`.
 
 Some sample fillContract implementations are provided in this repository:
 - [SwapRouter02Executor](./src/sample-executors/SwapRouter02Executor.sol): A fillContract that uses UniswapV2 and UniswapV3 via the SwapRouter02 router
 
 ### Direct Fill
 
-If a filler wants to fill orders using funds on-hand rather than a fillContract, they can do so gas efficiently using the `directFill` macro by specifying `address(1)` as the fillContract. This will pull tokens from the filler using `msg.sender` to satisfy the order outputs.
+If a filler wants to simply fill orders using funds held by an address rather than using a fillContract strategy, they can do so gas efficiently by using `execute` or `executeBatch`. These functions cause the reactor to skip the `reactorCallback` and simply pull tokens from the filler using `msg.sender`.
 
 # Integrating with UniswapX
 Jump to the docs for [Creating a Filler Integration](https://docs.uniswap.org/contracts/uniswapx/guides/createfiller).
@@ -66,6 +66,10 @@ forge test
 FOUNDRY_PROFILE=integration forge test
 ```
 
+# Fee-on-Transfer Disclaimer
+
+Note that UniswapX handles fee-on-transfer tokens by transferring the amount specified to the recipient. This means that the actual amount received by the recipient will be _after_ fees.
+
 # Audit
 
 This codebase was audited by [ABDK](./audit/ABDK.pdf).

src/interfaces/IReactor.sol

@@ -6,19 +6,20 @@ import {IReactorCallback} from "./IReactorCallback.sol";
 
 /// @notice Interface for order execution reactors
 interface IReactor {
-    /// @notice Execute a single order using the given fill specification
+    /// @notice Execute a single order
     /// @param order The order definition and valid signature to execute
-    /// @param fillContract The contract which will fill the order
-    /// @param fillData The fillData to pass to the fillContract callback
-    function execute(SignedOrder calldata order, IReactorCallback fillContract, bytes calldata fillData)
-        external
-        payable;
+    function execute(SignedOrder calldata order) external payable;
 
-    /// @notice Execute the given orders at once with the specified fill specification
+    /// @notice Execute a single order using the given callback data
+    /// @param order The order definition and valid signature to execute
+    function executeWithCallback(SignedOrder calldata order, bytes calldata callbackData) external payable;
+
+    /// @notice Execute the given orders at once
+    /// @param orders The order definitions and valid signatures to execute
+    function executeBatch(SignedOrder[] calldata orders) external payable;
+
+    /// @notice Execute the given orders at once using a callback with the given callback data
     /// @param orders The order definitions and valid signatures to execute
-    /// @param fillContract The contract which will fill the order
-    /// @param fillData The fillData to pass to the fillContract callback
-    function executeBatch(SignedOrder[] calldata orders, IReactorCallback fillContract, bytes calldata fillData)
-        external
-        payable;
+    /// @param callbackData The callbackData to pass to the callback
+    function executeBatchWithCallback(SignedOrder[] calldata orders, bytes calldata callbackData) external payable;
 }

src/interfaces/IReactorCallback.sol

@@ -7,8 +7,7 @@ import {ResolvedOrder} from "../base/ReactorStructs.sol";
 interface IReactorCallback {
     /// @notice Called by the reactor during the execution of an order
     /// @param resolvedOrders Has inputs and outputs
-    /// @param filler The address who called execute on the reactor
-    /// @param fillData The fillData specified for an order execution
+    /// @param callbackData The callbackData specified for an order execution
     /// @dev Must have approved each token and amount in outputs to the msg.sender
-    function reactorCallback(ResolvedOrder[] memory resolvedOrders, address filler, bytes memory fillData) external;
+    function reactorCallback(ResolvedOrder[] memory resolvedOrders, bytes memory callbackData) external;
 }

src/lens/OrderQuoter.sol

@@ -21,7 +21,7 @@ contract OrderQuoter is IReactorCallback {
     /// @param sig The order signature
     /// @return result The ResolvedOrder
     function quote(bytes memory order, bytes memory sig) external returns (ResolvedOrder memory result) {
-        try IReactor(getReactor(order)).execute(SignedOrder(order, sig), this, bytes("")) {}
+        try IReactor(getReactor(order)).executeWithCallback(SignedOrder(order, sig), bytes("")) {}
         catch (bytes memory reason) {
             result = parseRevertReason(reason);
         }
@@ -52,9 +52,7 @@ contract OrderQuoter is IReactorCallback {
     /// @notice Reactor callback function
     /// @dev reverts with the resolved order as reason
     /// @param resolvedOrders The resolved orders
-    /// @param filler The filler of the order
-    function reactorCallback(ResolvedOrder[] memory resolvedOrders, address filler, bytes memory) external view {
-        require(filler == address(this));
+    function reactorCallback(ResolvedOrder[] memory resolvedOrders, bytes memory) external pure {
         if (resolvedOrders.length != 1) {
             revert OrdersLengthIncorrect();
         }

src/lib/CurrencyLibrary.sol

@@ -7,6 +7,7 @@ import {SafeCast} from "openzeppelin-contracts/utils/math/SafeCast.sol";
 import {SafeTransferLib} from "solmate/src/utils/SafeTransferLib.sol";
 
 address constant NATIVE = 0x0000000000000000000000000000000000000000;
+uint256 constant TRANSFER_NATIVE_GAS_LIMIT = 6900;
 
 /// @title CurrencyLibrary
 /// @dev This library allows for transferring native ETH and ERC20s via direct filler OR fill contract.
@@ -28,33 +29,27 @@ library CurrencyLibrary {
         }
     }
 
-    /// @notice Transfer currency to recipient
+    /// @notice Transfer currency from the caller to recipient
+    /// @dev for native outputs we will already have the currency in local balance
     /// @param currency The currency to transfer
     /// @param recipient The recipient of the currency
     /// @param amount The amount of currency to transfer
-    function transfer(address currency, address recipient, uint256 amount) internal {
+    function transferFill(address currency, address recipient, uint256 amount) internal {
         if (isNative(currency)) {
-            (bool success,) = recipient.call{value: amount}("");
-            if (!success) revert NativeTransferFailed();
+            // we will have received native assets directly so can directly transfer
+            transferNative(recipient, amount);
         } else {
-            ERC20(currency).safeTransfer(recipient, amount);
+            // else the caller must have approved the token for the fill
+            ERC20(currency).safeTransferFrom(msg.sender, recipient, amount);
         }
     }
 
-    /// @notice Transfer currency from msg.sender to the recipient
-    /// @dev if currency is ETH, the value must have been sent in the execute call and is transferred directly
-    /// @dev if currency is token, the value is transferred from msg.sender via permit2
-    /// @param currency The currency to transfer
+    /// @notice Transfer native currency to recipient
     /// @param recipient The recipient of the currency
     /// @param amount The amount of currency to transfer
-    /// @param permit2 The deployed permit2 address
-    function transferFromDirectFiller(address currency, address recipient, uint256 amount, IPermit2 permit2) internal {
-        if (isNative(currency)) {
-            (bool success,) = recipient.call{value: amount}("");
-            if (!success) revert NativeTransferFailed();
-        } else {
-            permit2.transferFrom(msg.sender, recipient, SafeCast.toUint160(amount), currency);
-        }
+    function transferNative(address recipient, uint256 amount) internal {
+        (bool success,) = recipient.call{value: amount, gas: TRANSFER_NATIVE_GAS_LIMIT}("");
+        if (!success) revert NativeTransferFailed();
     }
 
     /// @notice returns true if currency is native