Enforce Fail-Fast Validation for Network Configurations in Subgraph Templating - #72
Open
magqqgq wants to merge 1 commit into
Open
Enforce Fail-Fast Validation for Network Configurations in Subgraph Templating#72magqqgq wants to merge 1 commit into
magqqgq wants to merge 1 commit into
Conversation
…emplating ### Description This pull request resolves Medium severity reliability and configuration findings identified in `polymarket-subgraph` during the workspace audit. Previously, missing or unrecognised network targets defaulted to empty evaluation contexts (`networks[undefined]`), prompting Handlebars to silently compile syntactically valid manifests with blank contract addresses and missing `startBlock` fields. Additionally, `templatify.ts` only accepted positional arguments, breaking scripts relying on `NETWORK_NAME`. This PR adds dual input support (`argv` and `env`) and fail-fast assertions against `networks.yaml` definitions. ### Key Changes & Remediations #### Configuration Resolution & Fail-Fast Guards (`templatify.ts`) * **Dual Target Resolution:** Updated network resolution to check `process.argv[2] || process.env.NETWORK_NAME`, ensuring alignment with `prepare:*` package scripts that pass network targets via environment variables. * **Missing Target Assertion:** Enforced an explicit error when no network name is supplied, eliminating confusing error messages that referenced obsolete flags. * **Target Existence Assertion:** Added an explicit check ensuring `networks[networkName]` exists before compiling templates. If an invalid network is supplied, the script throws an actionable error enumerating valid entries defined in `networks.yaml` rather than generating empty manifests that fail silently at deployment. ### How to Review 1. **Target Loading:** Verify in `templatify.ts` that `networkName` correctly evaluates positional arguments and `process.env.NETWORK_NAME`. 2. **Fail-Fast Guard:** Test invoking `templatify.ts` with no arguments and with an unknown target (e.g., `ts-node ./templatify.ts invalid-net`) to ensure execution halts with clear error diagnostics. 3. **Template Compilation:** Confirm valid network generation (e.g., `matic`) populates addresses and configurations as expected.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This pull request resolves Medium severity reliability and configuration findings identified in
polymarket-subgraphduring the workspace audit. Previously, missing or unrecognised network targets defaulted to empty evaluation contexts (networks[undefined]), prompting Handlebars to silently compile syntactically valid manifests with blank contract addresses and missingstartBlockfields. Additionally,templatify.tsonly accepted positional arguments, breaking scripts relying onNETWORK_NAME. This PR adds dual input support (argvandenv) and fail-fast assertions againstnetworks.yamldefinitions.Key Changes & Remediations
Configuration Resolution & Fail-Fast Guards (
templatify.ts)process.argv[2] || process.env.NETWORK_NAME, ensuring alignment withprepare:*package scripts that pass network targets via environment variables.networks[networkName]exists before compiling templates. If an invalid network is supplied, the script throws an actionable error enumerating valid entries defined innetworks.yamlrather than generating empty manifests that fail silently at deployment.How to Review
templatify.tsthatnetworkNamecorrectly evaluates positional arguments andprocess.env.NETWORK_NAME.templatify.tswith no arguments and with an unknown target (e.g.,ts-node ./templatify.ts invalid-net) to ensure execution halts with clear error diagnostics.matic) populates addresses and configurations as expected.Note
Low Risk
Build-time subgraph templating only; no runtime auth, payments, or production indexing logic changes.
Overview
templatify.tsnow resolves the target network from the first CLI argument orNETWORK_NAME, fixingprepare:mainnet/prepare:matic/prepare:mumbaiscripts that only set the env var.Validation runs before template compilation: missing network names get a corrected error (no obsolete
SUBGRAPHhint), and unknown keys fail fast with a list of valid entries fromnetworks.yamlinstead of emitting subgraph manifests with blank addresses and emptystartBlockvalues.Reviewed by Cursor Bugbot for commit 850e07e. Bugbot is set up for automated code reviews on this repo. Configure here.