feat(subgraph): add PDP offering decoding #233
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds PDP (Provider Data Product) offering decoding functionality to the subgraph, allowing provider product data to be decoded and stored as a JSON string for better accessibility.
- Introduces a new PDPOffering class with structured data fields and JSON serialization
- Adds decoding functionality to convert raw bytes data into structured PDPOffering objects
- Updates the schema to replace serviceUrl with decodedProductData field
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| subgraph/src/utils/types.ts | Adds PDPOffering class with constructor, empty factory method, and JSON serialization |
| subgraph/src/utils/contract-calls.ts | Implements decodePDPOfferingData function to decode raw bytes into PDPOffering objects |
| subgraph/src/utils/entity.ts | Updates createProviderProduct to use decoded product data instead of serviceUrl |
| subgraph/src/service-provider-registry.ts | Updates handleProductUpdated to decode and store product data as JSON |
| subgraph/schema.graphql | Replaces serviceUrl field with decodedProductData in ProviderProduct entity |
| subgraph/abis/ServiceProviderRegistry.json | Adds decodePDPOffering function ABI definition |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
we have service_contract/abi, and copied content in here, can we minimise the duplication of these and just use service_contract/abi for the subgraph too? |
| const serviceProviderRegistryInstance = ServiceProviderRegistry.bind(registryAddress); | ||
|
|
||
| const pdpOfferingTry = serviceProviderRegistryInstance.try_decodePDPOffering(data); | ||
|
|
||
| if (pdpOfferingTry.reverted) { | ||
| log.warning("decodePDPOfferingData: contract call reverted for data: {}", [data.toHexString()]); | ||
| return PDPOffering.empty(); | ||
| } |
There was a problem hiding this comment.
I've been considering just removing the decode* and encode* functions off the service registry, at least as public. They just do abi.encode and abi.decode and there's no good reason to invoke the EVM just do to that. Do we have good abi.decode tools available to us at this point that we could just call to get pdpOffering?
There was a problem hiding this comment.
If subgraph is the one that blocks removing these decode* functions, then feel free to remove these
And, I think these invocations will only be limited to ( productAdded and productUpdated ) from subgraph side
we can minimize the duplication of these abis if we can make sure that abi changes are also reflected into subgraph as well |
silent-cipher
force-pushed
the
refactor/subgraph/product-data
branch
from
6d1df4d to
8e71f65
Compare
|
removed abi duplication in subgraph |
There was a problem hiding this comment.
@silent-cipher before you merge this could you add a note in subgraph/templates/subgraph.template.yaml and subgraph/src/filecoin-warm-storage-service-legacy.ts the purpose of these "legacy" files and make it clear what the path to removal of them is, just so when we have an upgrade this doesn't get lost if it falls in someone else's lap. I'm sure you can keep this in your head, but it's going to look confusing to anyone else casually browsing, and it's the kind of thing that could be left in place accidentally because the next person doesn't understand why it's there.
github-project-automation
bot
moved this from 🔎 Awaiting review
to ✔️ Approved by reviewer
in FS
|
lgtm, thanks |
|
@rvagg anything preventing this from being merged? |
|
nope, I thought @silent-cipher had permissions to merge so I was leaving it up to him |
3 participants
decode provider product data and store it as a json string