feat: add view assets functions - #262
Conversation
… functionality and proxy updates
… in proxy upgrade script
…xecPocoAccessorsFacet
… mapping in deploy script
…pt for IexecPocoAccessorsFacet
…y for new facet integration
…tions in deploy-and-update-accessor-facet script
… deploy-and-update-accessor-facet script
…ccessors interface
There was a problem hiding this comment.
Pull Request Overview
This PR adds new view functions to access asset information (datasets, apps, and workerpools) in the Iexec protocol. The implementation provides a standardized way to retrieve metadata about these assets through the main contract interface.
- Adds three new view functions:
viewDataset,viewApp, andviewWorkerpool - Defines corresponding data structures (
DatasetInfo,AppInfo,WorkerpoolInfo) in the core library - Creates interface definitions for the three asset types with their respective accessor methods
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| contracts/facets/IexecPocoAccessorsFacet.sol | Implements the three view functions that aggregate asset data by calling interface methods |
| contracts/interfaces/IexecPocoAccessors.sol | Adds function signatures for the new view asset functions |
| contracts/interfaces/IexecAccessors.sol | Adds the same function signatures to the main accessor interface |
| contracts/libs/IexecLibCore_v5.sol | Defines data structures for DatasetInfo, AppInfo, and WorkerpoolInfo |
| contracts/registries/datasets/IDataset.v8.sol | New interface defining dataset accessor methods |
| contracts/registries/apps/IApp.v8.sol | New interface defining app accessor methods |
| contracts/registries/workerpools/IWorkerpool.v8.sol | Updates workerpool interface to add missing view modifiers and new methods |
| test/byContract/iexecPocoAccessors/IexecPocoAccessors.test.ts | Adds comprehensive tests for all three new view functions |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #262 +/- ##
==========================================
+ Coverage 83.66% 84.56% +0.90%
==========================================
Files 38 37 -1
Lines 1218 1218
Branches 227 227
==========================================
+ Hits 1019 1030 +11
+ Misses 199 188 -11 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Le-Caignec
left a comment
Le-Caignec
left a comment
There was a problem hiding this comment.
Clean Job
Somes questions/reflections
| import {FacetBase} from "./FacetBase.v8.sol"; | ||
| import {IexecLibCore_v5} from "../libs/IexecLibCore_v5.sol"; | ||
| import {IexecLibOrders_v5} from "../libs/IexecLibOrders_v5.sol"; | ||
| import {IDataset} from "../registries/datasets/IDataset.v8.sol"; |
There was a problem hiding this comment.
Should we indicate that this is version 8 in the file name?
There was a problem hiding this comment.
good question, matched what was already done with IWorkpool file can change it
There was a problem hiding this comment.
Let's keep them ISO yes. Otherwise we'll need to modify IexecPoco1Facet and IexecPocoBoostFacet.
| struct DatasetInfo { | ||
| address owner; | ||
| string m_datasetName; | ||
| bytes m_datasetMultiaddr; | ||
| bytes32 m_datasetChecksum; | ||
| } | ||
|
|
||
| struct AppInfo { | ||
| address owner; | ||
| string m_appName; | ||
| string m_appType; | ||
| bytes m_appMultiaddr; | ||
| bytes32 m_appChecksum; | ||
| bytes m_appMREnclave; | ||
| } | ||
|
|
||
| struct WorkerpoolInfo { | ||
| address owner; | ||
| string m_workerpoolDescription; | ||
| uint256 m_workerStakeRatioPolicy; | ||
| uint256 m_schedulerRewardRatioPolicy; | ||
| } |
There was a problem hiding this comment.
Another option could be to have those structs directly in the asset contract interface. I’m not sure which solution is better.
There was a problem hiding this comment.
could be an option but this would imply having v6 and v8 interfaces in asset contract interface
There was a problem hiding this comment.
But that's work fine as well
There was a problem hiding this comment.
Ok to put them inside LibCore since viewDataset is in the PoCo's code not in registries.
|
|
||
| it('viewWorkerpool', async function () { | ||
| const workerpoolInfo = await iexecPoco.viewWorkerpool(workerpoolAddress); | ||
| expect(workerpoolInfo.owner).to.equal(scheduler.address); |
There was a problem hiding this comment.
The owner of the workerpool is the scheduler 🤔 ?
Co-authored-by: Zied Guesmi <[email protected]>
No description provided.