Skip to content

Conversation

@zguesmi
Copy link
Member

@zguesmi zguesmi commented Nov 27, 2025

No description provided.

Copilot AI review requested due to automatic review settings November 27, 2025 17:28
@zguesmi zguesmi changed the title feat: Use custom errors when receiveApproval fails feat: Use custom errors when receiveApproval fails and set callbackgas to 200k Nov 27, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR modernizes error handling in the receiveApproval flow by replacing string-based revert messages with custom Solidity errors, improving gas efficiency and error handling clarity. It also refactors utility functions for better code organization, removing the deprecated encodeOrders helper in favor of using ethers.js's built-in populateTransaction method.

Key Changes:

  • Introduced three custom errors (UnsupportedOperation, OperationFailed, CallerIsNotTheRequester) to replace string-based reverts in the escrow token functionality
  • Refactored signStruct to return the signature string directly instead of mutating and returning the message object
  • Removed the encodeOrders utility function and replaced its usage with a cleaner matchOrdersCalldata helper function in tests

Reviewed changes

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

Show a summary per file
File Description
utils/odb-tools.ts Simplified signStruct to return signature directly; removed unused encodeOrders function and eth_signTypedData wrapper
utils/createOrders.ts Updated signOrder and signOrderOperation to mutate objects in-place rather than returning new objects
test/byContract/IexecEscrow/IexecEscrowToken-receiveApproval.test.ts Updated all test assertions to use revertedWithCustomError; added matchOrdersCalldata helper to replace encodeOrders
contracts/interfaces/IexecEscrowToken.sol Added three custom error definitions for improved error handling
contracts/facets/IexecEscrowTokenFacet.sol Replaced string-based reverts with custom errors; improved code formatting and documentation clarity
abis/human-readable-abis/contracts/interfaces/IexecEscrowToken.sol/IexecEscrowToken.json Added custom error entries to human-readable ABI
abis/human-readable-abis/contracts/facets/IexecEscrowTokenFacet.sol/IexecEscrowTokenFacet.json Added custom error entries to human-readable ABI
abis/human-readable-abis/contracts/IexecInterfaceToken.sol/IexecInterfaceToken.json Added custom error entries to human-readable ABI
abis/contracts/interfaces/IexecEscrowToken.json Added full custom error definitions to standard ABI
abis/contracts/facets/IexecEscrowTokenFacet.json Added full custom error definitions to standard ABI
abis/contracts/IexecInterfaceToken.json Added full custom error definitions to standard ABI

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@zguesmi zguesmi marked this pull request as ready for review November 27, 2025 17:34
@codecov
Copy link

codecov bot commented Nov 28, 2025

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.39%. Comparing base (52e8fc7) to head (542f86b).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #331   +/-   ##
=======================================
  Coverage   96.38%   96.39%           
=======================================
  Files          33       33           
  Lines        1135     1136    +1     
  Branches      214      214           
=======================================
+ Hits         1094     1095    +1     
  Misses         41       41           

☔ 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.

@zguesmi zguesmi merged commit 81778ee into main Nov 28, 2025
4 checks passed
@zguesmi zguesmi deleted the chore/reviews-part2 branch November 28, 2025 10:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants