fix(subgraph): use deployments.json as source of truth for addresses#406
fix(subgraph): use deployments.json as source of truth for addresses#406nijoe1 wants to merge 1 commit intoFilOzone:mainfrom
Conversation
- Refactor generate-constants.js and generate-config.js to read contract - addresses from service_contracts/deployments.json instead of the manual - config/network.json. This ensures subgraph configuration stays in sync when contracts are redeployed.
There was a problem hiding this comment.
Pull request overview
This PR refactors the subgraph configuration system to use service_contracts/deployments.json as the source of truth for contract addresses instead of the manually maintained config/network.json. This ensures the subgraph configuration automatically stays synchronized when contracts are redeployed.
Changes:
- Refactored
config-loader.jsto read fromservice_contracts/deployments.jsonwith chain ID-based lookups - Added mapping layer to convert deployment keys (e.g.,
PDP_VERIFIER_PROXY_ADDRESS) to template keys (e.g.,PDPVerifier) - Implemented optional start block override mechanism via
config/start-blocks.json - Updated console output and template comments to reflect the new data source
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| subgraph/templates/constants.template.ts | Updated comment to indicate deployments.json as the source |
| subgraph/scripts/utils/config-loader.js | Complete refactor to load from deployments.json with chain ID mapping, added ABI loading utilities |
| subgraph/scripts/generate-constants.js | Updated error message and added source confirmation log |
| subgraph/scripts/generate-config.js | Added source confirmation log |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| function loadAbi(contractName) { | ||
| const abiPath = getAbiPath(contractName); | ||
|
|
||
| try { | ||
| const content = fs.readFileSync(abiPath, "utf8"); | ||
| return JSON.parse(content); | ||
| } catch (error) { | ||
| if (error.code === "ENOENT") { | ||
| console.error(`Error: ABI file not found at: ${abiPath}`); | ||
| console.error( | ||
| `Please ensure service_contracts/abi/${contractName}.abi.json exists.` | ||
| ); | ||
| process.exit(1); | ||
| } | ||
| if (error instanceof SyntaxError) { | ||
| console.error(`Error: Invalid JSON in ABI file: ${abiPath}`); | ||
| console.error(`JSON Error: ${error.message}`); | ||
| } else { | ||
| console.error(`Error reading ABI file: ${abiPath}`); | ||
| console.error(`File Error: ${error.message}`); | ||
| } | ||
| process.exit(1); | ||
| } | ||
| } |
There was a problem hiding this comment.
The loadAbi function expects ABI files to be named using the template key format (e.g., "USDFCToken.abi.json"), but the actual ABI file for the USDFCToken contract is named "FilecoinPayV1.abi.json" in the service_contracts/abi directory. While this function is currently not used in the codebase, it could cause runtime errors if someone tries to call loadAbi("USDFCToken") in the future.
Consider either:
- Documenting that
loadAbishould use the actual contract names (e.g., "FilecoinPayV1") rather than the template keys - Creating a mapping from template keys to actual ABI file names
- Ensuring ABI files are named consistently with the template keys
| const startBlock = | ||
| startBlockOverrides?.[templateKey] || | ||
| defaultStartBlocks[templateKey] || | ||
| 0; | ||
|
|
There was a problem hiding this comment.
The fallback value of 0 for startBlock (line 154) means the subgraph will start indexing from the genesis block if no override or default is provided. This could result in inefficient indexing for contracts deployed much later. Consider logging a warning when falling back to 0, or making it a more reasonable default value based on known deployment timing.
For reference, mainnet (chain ID 314) launched at block 0 in 2021, and calibration (chain ID 314159) also starts at block 0. Having a fallback of 0 means potentially scanning millions of blocks before reaching the actual contract deployments.
| const startBlock = | |
| startBlockOverrides?.[templateKey] || | |
| defaultStartBlocks[templateKey] || | |
| 0; | |
| let startBlock; | |
| if ( | |
| startBlockOverrides && | |
| Object.prototype.hasOwnProperty.call(startBlockOverrides, templateKey) | |
| ) { | |
| startBlock = startBlockOverrides[templateKey]; | |
| } else if ( | |
| defaultStartBlocks && | |
| Object.prototype.hasOwnProperty.call(defaultStartBlocks, templateKey) | |
| ) { | |
| startBlock = defaultStartBlocks[templateKey]; | |
| } else { | |
| console.warn( | |
| `Warning: No startBlock configured for '${templateKey}' on network '${network}' (chain ID ${chainId}); falling back to 0. ` + | |
| "This may cause slow indexing if the contract was deployed well after genesis." | |
| ); | |
| startBlock = 0; | |
| } |
| const config = { | ||
| name: NETWORK_NAMES[network], |
There was a problem hiding this comment.
If a network is added to NETWORK_CHAIN_IDS but not to NETWORK_NAMES, the config object will have name: undefined, which could cause issues with template rendering. Consider adding validation to ensure both mappings are in sync, or consolidating them into a single data structure.
For example:
const NETWORK_CONFIG = {
mainnet: { chainId: "314", subgraphName: "filecoin" },
calibration: { chainId: "314159", subgraphName: "filecoin-testnet" }
};This would make it impossible to have mismatched entries.
|
Meta: I'm trying to understand who uses/cares about this subgraph: https://filecoinproject.slack.com/archives/C08TVNKJV7C/p1767914301013149 |
|
We're also pulling out the subgraph retriever from the sdk here since we've as yet had no signal of its use and it's awkward to maintain (cause we don't have a testing path for it and none of us use it): FilOzone/synapse-sdk#575 |
4 participants
cc: @BigLep @rjan90 @rvagg