feat: update fill logic (#55)

Author: anna-carroll

.gas-snapshot

@@ -1,13 +1,18 @@
-OrdersTest:test_initiate_ERC20() (gas: 81255)
-OrdersTest:test_initiate_ETH() (gas: 44801)
-OrdersTest:test_initiate_both() (gas: 118573)
-OrdersTest:test_initiate_multiERC20() (gas: 688314)
-OrdersTest:test_initiate_multiETH() (gas: 75288)
-OrdersTest:test_initiate_underflowETH() (gas: 63455)
-OrdersTest:test_onlyBuilder() (gas: 12793)
-OrdersTest:test_orderExpired() (gas: 27993)
-OrdersTest:test_sweep_ERC20() (gas: 60250)
-OrdersTest:test_sweep_ETH() (gas: 81788)
+OrdersTest:test_fill_ERC20() (gas: 70364)
+OrdersTest:test_fill_ETH() (gas: 68414)
+OrdersTest:test_fill_both() (gas: 166580)
+OrdersTest:test_fill_multiETH() (gas: 131926)
+OrdersTest:test_fill_underflowETH() (gas: 115281)
+OrdersTest:test_initiate_ERC20() (gas: 81435)
+OrdersTest:test_initiate_ETH() (gas: 44949)
+OrdersTest:test_initiate_both() (gas: 118677)
+OrdersTest:test_initiate_multiERC20() (gas: 688417)
+OrdersTest:test_initiate_multiETH() (gas: 75304)
+OrdersTest:test_onlyBuilder() (gas: 12815)
+OrdersTest:test_orderExpired() (gas: 27956)
+OrdersTest:test_sweepERC20() (gas: 60402)
+OrdersTest:test_sweepETH() (gas: 81940)
+OrdersTest:test_underflowETH() (gas: 63528)
 PassageTest:test_configureEnter() (gas: 82311)
 PassageTest:test_disallowedEnter() (gas: 17916)
 PassageTest:test_enter() (gas: 25563)

src/Orders.sol

@@ -22,36 +22,37 @@ struct Output {
     uint256 amount;
     /// @dev The address to receive the output tokens
     address recipient;
-    /// @dev The destination chain for this output
+    /// @dev When emitted on the origin chain, the destination chain for the Output.
+    ///      When emitted on the destination chain, the origin chain for the Order containing the Output.
     uint32 chainId;
 }
 
 /// @notice Contract capable of processing fulfillment of intent-based Orders.
 abstract contract OrderDestination {
-    /// @notice Emitted when an Order's Output is sent to the recipient.
-    /// @dev There may be multiple Outputs per Order.
-    /// @param originChainId - The chainId on which the Order was initiated.
-    /// @param recipient - The recipient of the token.
-    /// @param token - The address of the token transferred to the recipient. address(0) corresponds to native Ether.
-    /// @param amount - The amount of the token transferred to the recipient.
-    event OutputFilled(uint256 indexed originChainId, address indexed recipient, address indexed token, uint256 amount);
-
-    /// @notice Send the Output(s) of an Order to fulfill it.
-    ///         The user calls `initiate` on a rollup; the Builder calls `fill` on the target chain for each Output.
-    /// @custom:emits OutputFilled
-    /// @param originChainId - The chainId on which the Order was initiated.
-    /// @param recipient - The recipient of the token.
-    /// @param token - The address of the token to be transferred to the recipient.
-    ///                address(0) corresponds to native Ether.
-    /// @param amount - The amount of the token to be transferred to the recipient.
-    function fill(uint256 originChainId, address recipient, address token, uint256 amount) external payable {
-        if (token == address(0)) {
-            require(amount == msg.value);
-            payable(recipient).transfer(msg.value);
-        } else {
-            IERC20(token).transferFrom(msg.sender, recipient, amount);
+    /// @notice Emitted when Order Outputs are sent to their recipients.
+    /// @dev NOTE that here, Output.chainId denotes the *origin* chainId.
+    event Filled(Output[] outputs);
+
+    /// @notice Send the Output(s) of any number of Orders.
+    ///         The user calls `initiate` on a rollup; the Builder calls `fill` on the target chain aggregating Outputs.
+    ///         Builder may aggregate multiple Outputs with the same (`chainId`, `recipient`, `token`) into a single Output with the summed `amount`.
+    /// @dev NOTE that here, Output.chainId denotes the *origin* chainId.
+    /// @param outputs - The Outputs to be transferred.
+    /// @custom:emits Filled
+    function fill(Output[] memory outputs) external payable {
+        // transfer outputs
+        uint256 value = msg.value;
+        for (uint256 i; i < outputs.length; i++) {
+            if (outputs[i].token == address(0)) {
+                // this line should underflow if there's an attempt to spend more ETH than is attached to the transaction
+                value -= outputs[i].amount;
+                payable(outputs[i].recipient).transfer(outputs[i].amount);
+            } else {
+                IERC20(outputs[i].token).transferFrom(msg.sender, outputs[i].recipient, outputs[i].amount);
+            }
         }
-        emit OutputFilled(originChainId, recipient, token, amount);
+        // emit
+        emit Filled(outputs);
     }
 }
 
@@ -64,6 +65,7 @@ abstract contract OrderOrigin {
     error OnlyBuilder();
 
     /// @notice Emitted when an Order is submitted for fulfillment.
+    /// @dev NOTE that here, Output.chainId denotes the *destination* chainId.
     event Order(uint256 deadline, Input[] inputs, Output[] outputs);
 
     /// @notice Emitted when tokens or native Ether is swept from the contract.

test/Orders.t.sol

@@ -19,7 +19,7 @@ contract OrdersTest is Test {
     uint256 amount = 200;
     uint256 deadline = block.timestamp;
 
-    event OutputFilled(uint256 indexed originChainId, address indexed recipient, address indexed token, uint256 amount);
+    event Filled(Output[] outputs);
 
     event Order(uint256 deadline, Input[] inputs, Output[] outputs);
 
@@ -122,13 +122,13 @@ contract OrdersTest is Test {
         assertEq(address(target).balance, amount * 3);
     }
 
-    function test_initiate_underflowETH() public {
+    function test_underflowETH() public {
         // change first input to ETH
         inputs[0].token = address(0);
         // add second ETH input
         inputs.push(Input(address(0), 1));
 
-        // total ETH inputs should be `amount` + 1; function should underflow only sending `amount`
+        // total ETH inputs should be amount + 1; function should underflow only sending amount
         vm.expectRevert();
         target.initiate{value: amount}(deadline, inputs, outputs);
     }
@@ -140,7 +140,7 @@ contract OrdersTest is Test {
         target.initiate(deadline, inputs, outputs);
     }
 
-    function test_sweep_ETH() public {
+    function test_sweepETH() public {
         // set self as Builder
         vm.coinbase(address(this));
 
@@ -158,7 +158,7 @@ contract OrdersTest is Test {
         assertEq(recipient.balance, amount);
     }
 
-    function test_sweep_ERC20() public {
+    function test_sweepERC20() public {
         // set self as Builder
         vm.coinbase(address(this));
 
@@ -176,4 +176,63 @@ contract OrdersTest is Test {
         vm.expectRevert(OrderOrigin.OnlyBuilder.selector);
         target.sweep(recipient, token);
     }
+
+    function test_fill_ETH() public {
+        outputs[0].token = address(0);
+
+        vm.expectEmit();
+        emit Filled(outputs);
+        target.fill{value: amount}(outputs);
+
+        // ETH is transferred to recipient
+        assertEq(recipient.balance, amount);
+    }
+
+    function test_fill_ERC20() public {
+        vm.expectEmit();
+        emit Filled(outputs);
+        vm.expectCall(token, abi.encodeWithSelector(ERC20.transferFrom.selector, address(this), recipient, amount));
+        target.fill(outputs);
+    }
+
+    function test_fill_both() public {
+        // add ETH output
+        outputs.push(Output(address(0), amount * 2, recipient, chainId));
+
+        // expect Outputs are filled, ERC20 is transferred
+        vm.expectEmit();
+        emit Filled(outputs);
+        vm.expectCall(token, abi.encodeWithSelector(ERC20.transferFrom.selector, address(this), recipient, amount));
+        target.fill{value: amount * 2}(outputs);
+
+        // ETH is transferred to recipient
+        assertEq(recipient.balance, amount * 2);
+    }
+
+    // fill multiple ETH outputs
+    function test_fill_multiETH() public {
+        // change first output to ETH
+        outputs[0].token = address(0);
+        // add second ETH oputput
+        outputs.push(Output(address(0), amount * 2, recipient, chainId));
+
+        // expect Order event is initiated
+        vm.expectEmit();
+        emit Filled(outputs);
+        target.fill{value: amount * 3}(outputs);
+
+        // ETH is transferred to recipient
+        assertEq(recipient.balance, amount * 3);
+    }
+
+    function test_fill_underflowETH() public {
+        // change first output to ETH
+        outputs[0].token = address(0);
+        // add second ETH output
+        outputs.push(Output(address(0), 1, recipient, chainId));
+
+        // total ETH outputs should be `amount` + 1; function should underflow only sending `amount`
+        vm.expectRevert();
+        target.fill{value: amount}(outputs);
+    }
 }