feat: enhance isDatasetCompatibleWithDeal function to return with a reason
#267
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Added a new public view function `isDatasetCompatibleWithDeal` to verify the compatibility of a dataset order with a deal. This function includes multiple checks such as deal existence, dataset order owner signature, and restrictions on app, workerpool, and requester. Removed the `Matching` struct from `IexecPoco1Facet` and moved it to the `IexecPoco1` interface.
…ility Removed redundant comment in `IexecPoco1Facet` and added comprehensive tests for the new `isDatasetCompatibleWithDeal` function. The tests cover various scenarios including compatibility checks, invalid signatures, and restrictions on dataset orders.
Introduced a new script to deploy and update the IexecPocoAccessorsFacet and IexecPoco1Facet. The script handles the deployment of new facets, removal of old facets, and updates the diamond proxy accordingly. It includes checks for required deployment options and logs the process for better traceability.
Refactored tests for the `isDatasetCompatibleWithDeal` function to improve clarity and coverage. Removed the creation of an incompatible dataset order and added checks for non-existent deals and deals that already have datasets. Updated test cases to ensure accurate validation of dataset order compatibility, including scenarios for incompatible app restrictions and tags.
Co-authored-by: Copilot <[email protected]>
Co-authored-by: gfournieriExec <[email protected]>
…and IexecPoco1Facet
…acets in upgrade script
Co-authored-by: gfournieriExec <[email protected]>
Co-authored-by: gfournieriExec <[email protected]>
…e and update tests
…ailed status for compatibility checks
Co-authored-by: Zied Guesmi <[email protected]>
…in IexecPoco1Facet
Co-authored-by: Zied Guesmi <[email protected]>
Co-authored-by: Zied Guesmi <[email protected]>
Le-Caignec
changed the title
…feature/add-isDatasetCompatibleWithDeal-function-part2
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #267 +/- ##
==========================================
+ Coverage 83.66% 84.83% +1.17%
==========================================
Files 38 37 -1
Lines 1218 1240 +22
Branches 227 235 +8
==========================================
+ Hits 1019 1052 +33
+ Misses 199 188 -11 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Le-Caignec
changed the title
isDatasetCompatibleWithDeal function to revert with a reason
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the isDatasetCompatibleWithDeal function to return both a boolean result and a string reason for why compatibility checks fail, improving error reporting and debugging capabilities.
- Modifies function signature to return a tuple
(bool result, string memory reason)instead of justbool - Updates all compatibility check failure points to return specific error messages
- Updates all tests to verify both the boolean result and the specific error reason
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| contracts/interfaces/IexecPoco1.v8.sol | Updates interface signature to return both result and reason |
| contracts/interfaces/IexecPoco1.sol | Updates interface signature to return both result and reason |
| contracts/facets/IexecPoco1Facet.sol | Implements enhanced function with specific error messages for each failure case |
| test/byContract/IexecPoco/IexecPoco1.test.ts | Updates all test cases to destructure and verify both result and reason values |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Le-Caignec
changed the title
isDatasetCompatibleWithDeal function to revert with a reasonisDatasetCompatibleWithDeal function to return with a reason
gfournierPro
approved these changes
Sep 30, 2025
…feature/add-isDatasetCompatibleWithDeal-function-part2
zguesmi
reviewed
Sep 30, 2025
Base automatically changed from
feature/add-isDatasetCompatibleWithDeal-function
to
main
September 30, 2025 13:40
Co-authored-by: Zied Guesmi <[email protected]>
…CompatibleWithDeal-function-part2
…d add new `IexecPocoBoost` functions
Le-Caignec
deleted the
feature/add-isDatasetCompatibleWithDeal-function-part2
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.