feat: deposit and match orders #293

gfournieriExec · 2025-10-17T11:13:49Z

No description provided.

…ond proxy

…ive facet and updating imports

…atch interfaces

…cense year

… IexecPocoDepositAndMatchTokenFacet

…ade process

Copilot

Pull Request Overview

This PR introduces a new depositAndMatchOrders function that combines token deposit and order matching operations into a single atomic transaction, improving the user experience for builders by eliminating separate approve, deposit, and match steps.

Key changes:

Added IexecPocoDepositAndMatchTokenFacet contract with the combined deposit and match functionality
Created interface IexecPocoDepositAndMatch to define the new API
Integrated the new facet into the diamond proxy deployment and upgrade scripts

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
`contracts/facets/IexecPocoDepositAndMatchTokenFacet.sol`	Core implementation of the deposit and match functionality with balance checking and atomic execution
`contracts/interfaces/IexecPocoDepositAndMatchToken.sol`	Interface definition for the new `depositAndMatchOrders` function and `DepositAndMatch` event
`contracts/IexecInterfaceToken.sol`	Integration of the new interface into the main token interface
`test/byContract/IexecPocoDepositAndMatch/IexecPocoDepositAndMatchTokenFacet.test.ts`	Comprehensive test suite covering various scenarios including partial balances, error cases, and event verification
`scripts/upgrades/apply-deposit-match-upgrade.ts`	Deployment script for adding the new facet to the diamond proxy
`deploy/0_deploy.ts`	Updated deployment configuration to include the new facet
`abis/contracts/interfaces/IexecPocoDepositAndMatch.json`	Generated ABI for the interface
`abis/contracts/facets/IexecPocoDepositAndMatchTokenFacet.json`	Generated ABI for the facet implementation
`abis/contracts/IexecInterfaceToken.json`	Updated ABI including the new deposit and match function

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

deploy/0_deploy.ts

contracts/IexecInterfaceToken.sol

codecov · 2025-10-17T11:20:31Z

Codecov Report

❌ Patch coverage is 95.83333% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 85.05%. Comparing base (950ec4e) to head (3d55cbe).

Files with missing lines	Patch %	Lines
...acts/facets/IexecPocoDepositAndMatchTokenFacet.sol	95.83%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #293      +/-   ##
==========================================
+ Coverage   84.85%   85.05%   +0.20%     
==========================================
  Files          37       38       +1     
  Lines        1241     1265      +24     
  Branches      235      238       +3     
==========================================
+ Hits         1053     1076      +23     
- Misses        188      189       +1

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Le-Caignec

Personally, I would have chosen to add this function to the Poco1 facet, so that it would be at the same level as matchOrder.
But I don’t have a strong opinion on this — maybe it will make our deployments easier if we don’t have too many facets to manage.

contracts/interfaces/IexecPocoDepositAndMatchToken.sol

Le-Caignec · 2025-10-17T15:53:11Z

contracts/facets/IexecPocoDepositAndMatchTokenFacet.sol

+        require(_requestorder.requester == msg.sender, "DepositAndMatch: Caller must be requester");
+


can we use custom error here 🙏🏼

Le-Caignec · 2025-10-17T16:01:43Z

contracts/facets/IexecPocoDepositAndMatchTokenFacet.sol

+    function depositAndMatchOrders(
+        IexecLibOrders_v5.AppOrder calldata _apporder,
+        IexecLibOrders_v5.DatasetOrder calldata _datasetorder,
+        IexecLibOrders_v5.WorkerpoolOrder calldata _workerpoolorder,
+        IexecLibOrders_v5.RequestOrder calldata _requestorder
+    ) external override returns (bytes32 dealId) {


What do you think about making this function payable?
It would help reduce gas costs by allowing the SDK (client) to calculate off-chain the exact amount needed to add to the account.
It would also allow us to call our internal _deposit function directly.

Not sure to understand the point
we deal with rlc token not eth ?

Le-Caignec · 2025-10-17T16:01:59Z

contracts/facets/IexecPocoDepositAndMatchTokenFacet.sol

+        require(
+            $.m_baseToken.transferFrom(depositor, address(this), amount),
+            "DepositAndMatch: Token transfer failed"
+        );


Custom Error ?

Le-Caignec · 2025-10-17T16:02:43Z

contracts/facets/IexecPocoDepositAndMatchTokenFacet.sol

+        uint256 depositedAmount = 0;
+        if (currentBalance < dealCost) {
+            uint256 requiredDeposit = dealCost - currentBalance;
+            _depositTokens(msg.sender, requiredDeposit);


by using this internal function we read the storage twice time (PocoStorageLib.getPocoStorage())

gas reduction of 10, idk if it's worth it

buy puttin the $ as an input of the internal function
but reductin off 156 by removing the internal fct

Solution in mind

if (currentBalance < dealCost) { unchecked { // Safe: currentBalance < dealCost, so dealCost - currentBalance cannot underflow depositedAmount = dealCost - currentBalance; } if (!$.m_baseToken.transferFrom(msg.sender, address(this), depositedAmount)) { revert DepositAndMatch_TokenTransferFailed(); } // Safe: adding to balance cannot realistically overflow unchecked { $.m_balances[msg.sender] = currentBalance + depositedAmount; $.m_totalSupply += depositedAmount; } emit Transfer(address(0), msg.sender, depositedAmount); }

gas max : 879232 => 878754 = - 478

https://github.com/iExecBlockchainComputing/PoCo/compare/feature/deposit-matchOrders...feat/optimize-deposit-tokens?expand=1

Co-authored-by: Robin Le Caignec <[email protected]>

Co-authored-by: Copilot <[email protected]>

…cBlockchainComputing/PoCo into feature/deposit-matchOrders

Copilot

Pull Request Overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated no new comments.

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

zguesmi · 2025-10-24T15:39:16Z

contracts/IexecInterfaceToken.sol

 * @dev Referenced in the SDK with the current path `contracts/IexecInterfaceToken.sol`.
 * Changing the name or the path would cause a breaking change in the SDK.
 */
+// TODO Remove this interface when it's not used in the middleware anymore.


Suggested change

// TODO Remove this interface when it's not used in the middleware anymore.

// TODO Remove interface `IexecAccessorsABILegacy` when it's not used in the middleware anymore.

zguesmi · 2025-10-24T15:40:40Z

scripts/upgrades/v6.2.0-deposit-and-match.ts

To rewrite with new utility functions in proxy-tools.ts.

zguesmi · 2025-10-24T15:42:31Z

contracts/facets/IexecPocoDepositAndMatchTokenFacet.sol

+ * @dev This facet allows builders to deposit RLC tokens and match orders in a single transaction,
+ *      significantly improving the user experience by eliminating the need for separate approve+deposit+match transactions
+ */
+contract IexecPocoDepositAndMatchTokenFacet is


Suggested change

contract IexecPocoDepositAndMatchTokenFacet is

contract IexecDepositAndMatchOrdersFacet is

zguesmi · 2025-10-24T15:44:05Z

contracts/interfaces/IexecPocoDepositAndMatchToken.sol

+    /**
+     * @notice Thrown when the caller is not the requester in the request order
+     */
+    error DepositAndMatch_CallerMustBeRequester();


Suggested change

error DepositAndMatch_CallerMustBeRequester();

error CallerMustBeRequester();

gfournieriExec added 10 commits September 16, 2025 16:17

feat: Add support for Arbitrum fork configuration in Hardhat

ce0077c

feat: draft Add DepositAndMatchOrders modules and integrate with diam…

6863e62

…ond proxy

Merge branch 'main' into feature/deposit-matchOrders

c4ce955

Merge branch 'main' into feature/deposit-matchOrders

8d20181

feat: refactor deposit and match functionality by removing legacy nat…

8be5fa9

…ive facet and updating imports

feat: add IexecPocoDepositAndMatchTokenFacet and IexecPocoDepositAndM…

5b60f68

…atch interfaces

feat: remove depositAndSponsorMatchOrders function and update SPDX li…

8016518

…cense year

feat: add DepositAndMatch event and depositAndMatchOrders function to…

e53b883

… IexecPocoDepositAndMatchTokenFacet

feat: remove DepositAndMatchOrders deployment scripts and update upgr…

620617b

…ade process

feat: remove redundant comments for order hash verification in tests

bc3f563

gfournieriExec requested review from Le-Caignec and Copilot October 17, 2025 11:13

gfournieriExec self-assigned this Oct 17, 2025

Copilot AI reviewed Oct 17, 2025

View reviewed changes

deploy/0_deploy.ts Show resolved Hide resolved

contracts/IexecInterfaceToken.sol Outdated Show resolved Hide resolved

gfournieriExec requested a review from zguesmi October 17, 2025 11:17

gfournieriExec marked this pull request as draft October 17, 2025 12:53

Le-Caignec reviewed Oct 17, 2025

View reviewed changes

gfournieriExec and others added 9 commits October 20, 2025 11:11

fix: update contracts/interfaces/IexecPocoDepositAndMatchToken.sol

1dc0f62

Co-authored-by: Robin Le Caignec <[email protected]>

feat: implement custom errors for deposit and match operations

1922fd7

feat: add custom error types for Deposit and Match operations

85de82e

Merge branch 'main' into feature/deposit-matchOrders

2d14cfb

feat: add upgrade script for Deposit and Match Orders (v6.2.0)

bb6cc04

fix: Apply suggestion from @Copilot

4b6c800

Co-authored-by: Copilot <[email protected]>

refactor: remove apply-deposit-match-upgrade script

2229484

Merge branch 'feature/deposit-matchOrders' of https://github.com/iExe…

7d9d6e2

…cBlockchainComputing/PoCo into feature/deposit-matchOrders

Merge branch 'main' into feature/deposit-matchOrders

3d55cbe

Copilot AI review requested due to automatic review settings October 21, 2025 13:19

Copilot AI reviewed Oct 21, 2025

View reviewed changes

zguesmi reviewed Oct 24, 2025

View reviewed changes

		require(_requestorder.requester == msg.sender, "DepositAndMatch: Caller must be requester");

	// TODO Remove this interface when it's not used in the middleware anymore.
	// TODO Remove interface `IexecAccessorsABILegacy` when it's not used in the middleware anymore.

	contract IexecPocoDepositAndMatchTokenFacet is
	contract IexecDepositAndMatchOrdersFacet is

	error DepositAndMatch_CallerMustBeRequester();
	error CallerMustBeRequester();

feat: deposit and match orders #293

Are you sure you want to change the base?

feat: deposit and match orders #293

Uh oh!

Conversation

gfournieriExec commented Oct 17, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

codecov bot commented Oct 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Le-Caignec left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

codecov bot commented Oct 17, 2025 •

edited

Loading