feat: update fill logic

Author: anna-carroll

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.
+    /// @param outputs - The Outputs transferred 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.
+    /// @param outputs - The Outputs to be transferred.
+    /// @dev NOTE that here, Output.chainId denotes the *origin* chainId.
+    /// @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);
 
@@ -176,6 +176,65 @@ 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);
+    }
 }
 
 contract OrdersFuzzTest is Test {
@@ -184,7 +243,7 @@ contract OrdersFuzzTest is Test {
 
     uint256 deadline = block.timestamp + 100;
 
-    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);
 
@@ -202,7 +261,7 @@ contract OrdersFuzzTest is Test {
 
     function testFuzz_initiate(Input[] memory fuzzInputs, Output[] memory fuzzOutputs) public {
         // can only use vm.assume 256 * 256 times, and we do 256 fuzz runs
-        vm.assume(fuzzInputs.length < 256);
+        vm.assume(fuzzInputs.length < 250);
         // setup the fuzz test by ensuring ETH and ERC20 amounts are available
         uint256 totalETH;
         for (uint256 i; i < fuzzInputs.length; i++) {
@@ -212,19 +271,7 @@ contract OrdersFuzzTest is Test {
                 continue;
             }
 
-            // ERC20 inputs must be one of the tokens we've deployed
-            vm.assume(isToken[fuzzInputs[i].token]);
-            // mint the token amount to ensure the address has it
-            TestERC20(fuzzInputs[i].token).mint(address(this), fuzzInputs[i].amount);
-            // approve the target to spend the total token amount
-            TestERC20(fuzzInputs[i].token).approve(address(target), type(uint256).max);
-            // expect the token amount to be transferred
-            vm.expectCall(
-                fuzzInputs[i].token,
-                abi.encodeWithSelector(
-                    ERC20.transferFrom.selector, address(this), address(target), fuzzInputs[i].amount
-                )
-            );
+            _setupToken(fuzzInputs[i].token, fuzzInputs[i].amount, address(target));
         }
         // deal the total ETH amount to ensure the address has it
         vm.deal(address(this), address(this).balance + totalETH);
@@ -233,4 +280,36 @@ contract OrdersFuzzTest is Test {
         emit Order(deadline, fuzzInputs, fuzzOutputs);
         target.initiate{value: totalETH}(deadline, fuzzInputs, fuzzOutputs);
     }
+
+    function testFuzz_fill(Output[] memory fuzzOutputs) public {
+        // can only use vm.assume 256 * 256 times, and we do 256 fuzz runs
+        vm.assume(fuzzOutputs.length < 250);
+        // setup the fuzz test by ensuring ETH and ERC20 amounts are available
+        uint256 totalETH;
+        for (uint256 i; i < fuzzOutputs.length; i++) {
+            if (fuzzOutputs[i].token == address(0)) {
+                totalETH += fuzzOutputs[i].amount;
+                continue;
+            }
+
+            _setupToken(fuzzOutputs[i].token, fuzzOutputs[i].amount, fuzzOutputs[i].recipient);
+        }
+        // deal the total ETH amount to ensure the address has it
+        vm.deal(address(this), address(this).balance + totalETH);
+
+        vm.expectEmit();
+        emit Filled(fuzzOutputs);
+        target.fill{value: totalETH}(fuzzOutputs);
+    }
+
+    function _setupToken(address token, uint256 amount, address recipient) internal {
+        // ERC20 inputs must be one of the tokens we've deployed
+        vm.assume(isToken[token]);
+        // mint the token amount to ensure the address has it
+        TestERC20(token).mint(address(this), amount);
+        // approve the target to spend the total token amount
+        TestERC20(token).approve(address(target), type(uint256).max);
+        // expect the token amount to be transferred
+        vm.expectCall(token, abi.encodeWithSelector(ERC20.transferFrom.selector, address(this), recipient, amount));
+    }
 }