feat: Ignore TEE framework bits in dataset tags for cross-framework compatibility #325
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR implements cross-framework compatibility for dataset orders by masking TEE framework bits (bits 1-3: Scone, Gramine, TDX) in dataset tags. This allows dataset orders created for SGX workerpools to be consumed on TDX workerpools and vice versa.
- Applied bit mask
~0xE(preserving bit 0 and bits ≥4, clearing bits 1-3) to dataset tags in order matching logic - Extended the same masking to dataset compatibility checks for bulk processing
- Added comprehensive test coverage for cross-framework scenarios and edge cases
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 12 comments.
| File | Description |
|---|---|
| contracts/facets/IexecPoco1Facet.sol | Implements dataset tag masking in _matchOrders and assertDatasetDealCompatibility to ignore framework-specific bits 1-3 |
| test/byContract/IexecPoco/IexecPoco1.test.ts | Adds test cases verifying cross-framework compatibility (Scone/Gramine/TDX) and proper handling of masked/unmasked bits |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const deal = await iexecPoco.viewDeal(dealId); | ||
| expect(deal.tag).to.equal( | ||
| '0x0000000000000000000000000000000000000000000000000000000000000001', | ||
| ); // TEE tag only (bits 2-4 from dataset were masked out) |
There was a problem hiding this comment.
Comment incorrectly states "bits 2-4" but the actual bits being masked are bits 1-3 (Scone, Gramine, TDX). Bit 0 is TEE, and bits 1, 2, 3 are the framework bits being ignored. The comment should say "bits 1-3" instead of "bits 2-4".
| ); // TEE tag only (bits 2-4 from dataset were masked out) | |
| ); // TEE tag only (bits 1-3 from dataset were masked out) |
| it('Should match orders when dataset bits 2-4 are set but only bit 1 (TEE) is considered', async () => { | ||
| // Dataset order with bits 2, 3, 4 set: 0b1111 = 0xF |
There was a problem hiding this comment.
Comment incorrectly states "bits 2-4" but should state "bits 1-3" (Scone, Gramine, TDX framework bits). This is consistent with the PR description which clearly states bits 1, 2, 3 are being masked.
| it('Should match orders when dataset bits 2-4 are set but only bit 1 (TEE) is considered', async () => { | |
| // Dataset order with bits 2, 3, 4 set: 0b1111 = 0xF | |
| it('Should match orders when dataset bits 1-3 are set but only bit 1 (TEE) is considered', async () => { | |
| // Dataset order with bits 1, 2, 3 set: 0b1111 = 0xF |
| }; | ||
| await signOrder(iexecWrapper.getDomain(), tdxDatasetOrder, datasetProvider); | ||
|
|
||
| // Should not revert because bits 2-4 of dataset tag are ignored |
There was a problem hiding this comment.
Comment incorrectly states "bits 2-4" but should state "bits 1-3" (Scone, Gramine, TDX framework bits). The masking ignores bits 1, 2, and 3, not bits 2, 3, and 4.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #325 +/- ##
=======================================
Coverage 96.37% 96.38%
=======================================
Files 33 33
Lines 1132 1134 +2
Branches 228 228
=======================================
+ Hits 1091 1093 +2
Misses 41 41 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
utils/constants.ts
Outdated
| * - Bit 1: Scone | ||
| * - Bit 2: Gramine | ||
| * - Bit 3: TDX | ||
| * - Bit 4: GPU (0x100) |
There was a problem hiding this comment.
The comment states "Bit 4: GPU (0x100)" but this is incorrect. Bit 4 (0-indexed) should be 0x10 (decimal 16), not 0x100 (decimal 256). The value 0x100 represents bit 8.
The constant TAG_BIT_4 on line 20 correctly uses 0x10, so only the comment needs to be corrected to: "- Bit 4: GPU (0x10)"
| * - Bit 4: GPU (0x100) | |
| * - Bit 4: GPU (0x10) |
| { | ||
| datasetTag: TAG_TEE_SCONE, | ||
| workerpoolTag: TAG_TEE_TDX, | ||
| description: 'Scone tag (0x3) and workerpool has TDX tag (0x9)', | ||
| }, | ||
| { | ||
| datasetTag: TAG_TEE_GRAMINE, | ||
| workerpoolTag: TAG_TEE_TDX, | ||
| description: 'Gramine tag (0x5) and workerpool has TDX tag (0x9)', | ||
| }, | ||
| { | ||
| datasetTag: TAG_TEE_TDX, | ||
| workerpoolTag: TAG_TEE_SCONE, | ||
| description: 'TDX tag (0x9) and workerpool has Scone tag (0x3)', | ||
| }, | ||
| { | ||
| datasetTag: TAG_ALL_TEE_FRAMEWORKS, | ||
| workerpoolTag: TAG_TEE, | ||
| description: 'all TEE framework bits (0xF) and workerpool has TEE only (0x1)', | ||
| }, |
There was a problem hiding this comment.
i thing there is many other case - but it's good like that
There was a problem hiding this comment.
added a todo thanks
| .withArgs('Requester restriction not satisfied'); | ||
| }); | ||
|
|
||
| it('Should revert when tag compatibility is not satisfied', async () => { |
There was a problem hiding this comment.
Here, we refer only to the deal. We should clarify that, because above we iterate over each workerpool TAG to check compatibility.
There was a problem hiding this comment.
Description
Modifies the
matchOrdersmechanism to ignore Scone, Gramine, and TDX framework bits (bits 1-3) in dataset order tags, enabling dataset orders from SGX workerpools to be consumed on TDX workerpools and vice versa.Changes
Smart Contract (
IexecPoco1Facet.sol)_matchOrdersfunction:~0xE(=0xFFF...FF1) to dataset order tags before combining with app and request tagsassertDatasetDealCompatibilityfunction:Bit Mapping (0-indexed)
0x1)0x2) - ignored for datasets0x4) - ignored for datasets0x8) - ignored for datasets