feat: Enable deposit and match orders in a single tx (#316)

gfournierPro · web-flow · commit 4100b90fe76f · 2025-11-10T14:54:05.000+01:00
diff --git a/abis/contracts/facets/IexecEscrowTokenFacet.json b/abis/contracts/facets/IexecEscrowTokenFacet.json
@@ -227,7 +227,7 @@
       },
       {
         "internalType": "bytes",
-        "name": "",
+        "name": "data",
         "type": "bytes"
       }
     ],
diff --git a/abis/human-readable-abis/contracts/tools/testing/ReceiveApprovalTestHelper.sol/ReceiveApprovalTestHelper.json b/abis/human-readable-abis/contracts/tools/testing/ReceiveApprovalTestHelper.sol/ReceiveApprovalTestHelper.json
@@ -0,0 +1,3 @@
+[
+  "function matchOrders(tuple(address,uint256,uint256,bytes32,address,address,address,bytes32,bytes),tuple(address,uint256,uint256,bytes32,address,address,address,bytes32,bytes),tuple(address,uint256,uint256,bytes32,uint256,uint256,address,address,address,bytes32,bytes),tuple(address,uint256,address,uint256,address,uint256,address,uint256,bytes32,uint256,uint256,address,address,string,bytes32,bytes)) pure returns (bytes32)"
+]
diff --git a/contracts/facets/IexecEscrowTokenFacet.sol b/contracts/facets/IexecEscrowTokenFacet.sol
@@ -7,6 +7,8 @@ import {IexecERC20Core} from "./IexecERC20Core.sol";
 import {FacetBase} from "./FacetBase.sol";
 import {IexecEscrowToken} from "../interfaces/IexecEscrowToken.sol";
 import {IexecTokenSpender} from "../interfaces/IexecTokenSpender.sol";
+import {IexecPoco1} from "../interfaces/IexecPoco1.sol";
+import {IexecLibOrders_v5} from "../libs/IexecLibOrders_v5.sol";
 import {PocoStorageLib} from "../libs/PocoStorageLib.sol";
 
 contract IexecEscrowTokenFacet is IexecEscrowToken, IexecTokenSpender, FacetBase, IexecERC20Core {
@@ -64,23 +66,121 @@ contract IexecEscrowTokenFacet is IexecEscrowToken, IexecTokenSpender, FacetBase
         return delta;
     }
 
-    // Token Spender (endpoint for approveAndCallback calls to the proxy)
+    /***************************************************************************
+     *            Token Spender: Atomic Deposit+Match                  *
+     ***************************************************************************/
+
+    /**
+     * @notice Receives approval, deposit and optionally matches orders in one transaction
+     *
+     * Usage patterns:
+     * 1. Simple deposit: RLC.approveAndCall(escrow, amount, "")
+     * 2. Deposit + match: RLC.approveAndCall(escrow, amount, encodedOrders)
+     *
+     * The `data` parameter should be ABI-encoded orders if matching is desired:
+     * abi.encode(appOrder, datasetOrder, workerpoolOrder, requestOrder)
+     *
+     * @dev Important notes:
+     * - Match orders sponsoring is NOT supported. The requester (sender) always pays for the deal.
+     * - Clients must compute the exact deal cost and deposit the right amount for the deal to be matched.
+     *   The deal cost = (appPrice + datasetPrice + workerpoolPrice) * volume.
+     * - If insufficient funds are deposited, the match will fail.
+     *
+     * @param sender The address that approved tokens (must be requester if matching)
+     * @param amount Amount of tokens approved and to be deposited
+     * @param token Address of the token (must be RLC)
+     * @param data Optional: ABI-encoded orders for matching
+     * @return success True if operation succeeded
+     *
+     *
+     * @custom:example
+     * ```solidity
+     * // Compute deal cost
+     * uint256 dealCost = (appPrice + datasetPrice + workerpoolPrice) * volume;
+     *
+     * // Encode orders
+     * bytes memory data = abi.encode(appOrder, datasetOrder, workerpoolOrder, requestOrder);
+     *
+     * // One transaction does it all
+     * RLC(token).approveAndCall(iexecProxy, dealCost, data);
+     * ```
+     */
     function receiveApproval(
         address sender,
         uint256 amount,
         address token,
-        bytes calldata
+        bytes calldata data
     ) external override returns (bool) {
         PocoStorageLib.PocoStorage storage $ = PocoStorageLib.getPocoStorage();
         require(token == address($.m_baseToken), "wrong-token");
         _deposit(sender, amount);
         _mint(sender, amount);
+        if (data.length > 0) {
+            _decodeDataAndMatchOrders(sender, data);
+        }
         return true;
     }
 
+    /******************************************************************************
+     *        Token Spender: Atomic Deposit+Match if used with RLC.approveAndCall *
+     *****************************************************************************/
+
+    /**
+     * @dev Internal function to match orders after deposit
+     * @param sender The user who deposited (must be the requester)
+     * @param data ABI-encoded orders
+     */
+    function _decodeDataAndMatchOrders(address sender, bytes calldata data) internal {
+        // Decode the orders from calldata
+        (
+            IexecLibOrders_v5.AppOrder memory apporder,
+            IexecLibOrders_v5.DatasetOrder memory datasetorder,
+            IexecLibOrders_v5.WorkerpoolOrder memory workerpoolorder,
+            IexecLibOrders_v5.RequestOrder memory requestorder
+        ) = abi.decode(
+                data,
+                (
+                    IexecLibOrders_v5.AppOrder,
+                    IexecLibOrders_v5.DatasetOrder,
+                    IexecLibOrders_v5.WorkerpoolOrder,
+                    IexecLibOrders_v5.RequestOrder
+                )
+            );
+
+        // Validate that sender is the requester
+        if (requestorder.requester != sender) revert("caller-must-be-requester");
+
+        // Call matchOrders on the IexecPoco1 facet through the diamond
+        // Using delegatecall for safety: preserves msg.sender context (RLC address in this case)
+        // Note: matchOrders doesn't use msg.sender, but delegatecall is safer
+        // in case the implementation changes in the future
+        (bool success, bytes memory result) = address(this).delegatecall(
+            abi.encodeWithSelector(
+                IexecPoco1.matchOrders.selector,
+                apporder,
+                datasetorder,
+                workerpoolorder,
+                requestorder
+            )
+        );
+
+        // Handle failure and bubble up revert reason
+        if (!success) {
+            if (result.length > 0) {
+                // Decode and revert with the original error
+                assembly {
+                    let returndata_size := mload(result)
+                    revert(add(result, 32), returndata_size)
+                }
+            } else {
+                revert("receive-approval-failed");
+            }
+        }
+    }
+
     function _deposit(address from, uint256 amount) internal {
         PocoStorageLib.PocoStorage storage $ = PocoStorageLib.getPocoStorage();
-        require($.m_baseToken.transferFrom(from, address(this), amount), "failled-transferFrom");
+        require($.m_baseToken.transferFrom(from, address(this), amount), "failed-transferFrom");
     }
 
     function _withdraw(address to, uint256 amount) internal {
diff --git a/contracts/facets/IexecPoco1Facet.sol b/contracts/facets/IexecPoco1Facet.sol
@@ -140,6 +140,11 @@ contract IexecPoco1Facet is
     /**
      * Match orders. The requester gets debited.
      *
+     * @notice This function does not use `msg.sender` to determine who pays for the deal.
+     * The sponsor is always set to `_requestorder.requester`, regardless of who calls this function.
+     * This design allows the function to be safely called via delegatecall from other facets
+     * (e.g., IexecEscrowTokenFacet.receiveApproval) without security concerns.
+     *
      * @param _apporder The app order.
      * @param _datasetorder The dataset order.
      * @param _workerpoolorder The workerpool order.
@@ -161,6 +166,7 @@ contract IexecPoco1Facet is
             );
     }
 
+    // TODO: check if we want to modify sponsor origin to be a variable instead of msg.sender
     /**
      * Sponsor match orders for a requester.
      * Unlike the standard `matchOrders(..)` hook where the requester pays for
diff --git a/contracts/interfaces/IexecEscrowToken.sol b/contracts/interfaces/IexecEscrowToken.sol
@@ -6,7 +6,6 @@ pragma solidity ^0.8.0;
 interface IexecEscrowToken {
     receive() external payable;
     fallback() external payable;
-
     function deposit(uint256) external returns (bool);
     function depositFor(uint256, address) external returns (bool);
     function depositForArray(uint256[] calldata, address[] calldata) external returns (bool);
diff --git a/contracts/tools/testing/ReceiveApprovalTestHelper.sol b/contracts/tools/testing/ReceiveApprovalTestHelper.sol
@@ -0,0 +1,31 @@
+// SPDX-FileCopyrightText: 2025 IEXEC BLOCKCHAIN TECH <contact@iex.ec>
+// SPDX-License-Identifier: Apache-2.0
+
+pragma solidity ^0.8.0;
+
+import {IexecLibOrders_v5} from "../../libs/IexecLibOrders_v5.sol";
+
+/**
+ * @title ReceiveApprovalTestHelper
+ * @notice Helper contract to test edge cases in receiveApproval function
+ * @dev This contract simulates a facet that fails silently (no error data)
+ */
+contract ReceiveApprovalTestHelper {
+    /**
+     * @notice Mock matchOrders function that fails without returning error data
+     * @dev Uses assembly to revert without data, simulating the edge case where
+     *      delegatecall fails and result.length == 0
+     */
+    function matchOrders(
+        IexecLibOrders_v5.AppOrder calldata,
+        IexecLibOrders_v5.DatasetOrder calldata,
+        IexecLibOrders_v5.WorkerpoolOrder calldata,
+        IexecLibOrders_v5.RequestOrder calldata
+    ) external pure returns (bytes32) {
+        // Revert without any error data
+        // This simulates: delegatecall fails with success=false and result.length=0
+        assembly {
+            revert(0, 0)
+        }
+    }
+}
diff --git a/docs/solidity/index.md b/docs/solidity/index.md
@@ -247,9 +247,39 @@ function recover() external returns (uint256)
 ### receiveApproval
 
 ```solidity
-function receiveApproval(address sender, uint256 amount, address token, bytes) external returns (bool)
+function receiveApproval(address sender, uint256 amount, address token, bytes data) external returns (bool)
 ```
 
+Receives approval and optionally matches orders in one transaction
+
+Usage patterns:
+1. Simple deposit: RLC.approveAndCall(escrow, amount, "")
+2. Deposit + match: RLC.approveAndCall(escrow, amount, encodedOrders)
+
+The `data` parameter should be ABI-encoded orders if matching is desired:
+abi.encode(appOrder, datasetOrder, workerpoolOrder, requestOrder)
+
+_Important notes:
+- Match orders sponsoring is NOT supported. The requester (sender) always pays for the deal.
+- Clients must compute the exact deal cost and deposit the right amount for the deal to be matched.
+  The deal cost = (appPrice + datasetPrice + workerpoolPrice) * volume.
+- If insufficient funds are deposited, the match will fail._
+
+#### Parameters
+
+| Name | Type | Description |
+| ---- | ---- | ----------- |
+| sender | address | The address that approved tokens (must be requester if matching) |
+| amount | uint256 | Amount of tokens approved and to be deposited |
+| token | address | Address of the token (must be RLC) |
+| data | bytes | Optional: ABI-encoded orders for matching |
+
+#### Return Values
+
+| Name | Type | Description |
+| ---- | ---- | ----------- |
+| [0] | bool | success True if operation succeeded @custom:example ```solidity // Compute deal cost uint256 dealCost = (appPrice + datasetPrice + workerpoolPrice) * volume; // Encode orders bytes memory data = abi.encode(appOrder, datasetOrder, workerpoolOrder, requestOrder); // One transaction does it all RLC(token).approveAndCall(iexecProxy, dealCost, data); ``` |
+
 ## IexecOrderManagementFacet
 
 ### manageAppOrder
@@ -343,6 +373,11 @@ function matchOrders(struct IexecLibOrders_v5.AppOrder _apporder, struct IexecLi
 
 Match orders. The requester gets debited.
 
+This function does not use `msg.sender` to determine who pays for the deal.
+The sponsor is always set to `_requestorder.requester`, regardless of who calls this function.
+This design allows the function to be safely called via delegatecall from other facets
+(e.g., IexecEscrowTokenFacet.receiveApproval) without security concerns.
+
 #### Parameters
 
 | Name | Type | Description |
diff --git a/test/byContract/IexecEscrow/IexecEscrowToken.receiveApproval.test.ts b/test/byContract/IexecEscrow/IexecEscrowToken.receiveApproval.test.ts
diff --git a/utils/odb-tools.ts b/utils/odb-tools.ts

Original file line number	Diff line number	Diff line change
`@@ -227,7 +227,7 @@`
`227`	`227`	`},`
`228`	`228`	`{`
`229`	`229`	`"internalType": "bytes",`
`230`		`- "name": "",`
	`230`	`+ "name": "data",`
`231`	`231`	`"type": "bytes"`
`232`	`232`	`}`
`233`	`233`	`],`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1,3 @@`
	`1`	`+[`
	`2`	`+ "function matchOrders(tuple(address,uint256,uint256,bytes32,address,address,address,bytes32,bytes),tuple(address,uint256,uint256,bytes32,address,address,address,bytes32,bytes),tuple(address,uint256,uint256,bytes32,uint256,uint256,address,address,address,bytes32,bytes),tuple(address,uint256,address,uint256,address,uint256,address,uint256,bytes32,uint256,uint256,address,address,string,bytes32,bytes)) pure returns (bytes32)"`
	`3`	`+]`