Skip to content

Conversation

silent-cipher
Copy link
Collaborator

decode provider product data and store it as a json string

@FilOzzy FilOzzy added this to FS Sep 24, 2025
@github-project-automation github-project-automation bot moved this to 📌 Triage in FS Sep 24, 2025
@rjan90 rjan90 moved this from 📌 Triage to 🔎 Awaiting review in FS Sep 24, 2025
Copy link
Contributor

@Copilot 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 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.

@rvagg
Copy link
Collaborator

rvagg commented Sep 24, 2025

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?

Comment on lines +55 to +62
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();
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

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?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

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

@silent-cipher
Copy link
Collaborator Author

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?

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 silent-cipher force-pushed the refactor/subgraph/product-data branch from 6d1df4d to 8e71f65 Compare October 1, 2025 08:17
@silent-cipher
Copy link
Collaborator Author

removed abi duplication in subgraph

Copy link
Collaborator

@rvagg rvagg left a comment

Choose a reason for hiding this comment

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

@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 github-project-automation bot moved this from 🔎 Awaiting review to ✔️ Approved by reviewer in FS Oct 6, 2025
@rvagg
Copy link
Collaborator

rvagg commented Oct 7, 2025

lgtm, thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: ✔️ Approved by reviewer
Development

Successfully merging this pull request may close these issues.

3 participants