chore(infra): remove legacy per-chain endpoint vars and fallbacks#113
chore(infra): remove legacy per-chain endpoint vars and fallbacks#113
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
There was a problem hiding this comment.
Stale comment
This PR removes legacy per-chain hosted Hasura endpoint vars/fallbacks and standardizes Celo mainnet + Monad mainnet on
NEXT_PUBLIC_HASURA_URL_MULTICHAIN_HOSTEDacross Terraform, env examples, and runtime network wiring. The infra/runtime direction looks correct overall, but there is one test-coverage regression that should be fixed before merge.
ui-dashboard/src/lib/__tests__/networks.test.ts
- 🟡 IMPORTANT (
L168-L174): The new test case is a duplicate negative-path check (NEXT_PUBLIC_HASURA_URL_MULTICHAIN_HOSTEDis empty, expect Monad mainnet hidden), and the previous positive-path assertion was removed. After this change, there is no direct test provingmonad-mainnet-hostedbecomes configured whenNEXT_PUBLIC_HASURA_URL_MULTICHAIN_HOSTEDis set. This leaves the new wiring under-tested and can let regressions in env-var key usage slip through. Add a positive test that stubsNEXT_PUBLIC_HASURA_URL_MULTICHAIN_HOSTEDto a non-empty URL and asserts bothhasuraUrlandisConfiguredNetworkId("monad-mainnet-hosted")are truthy.Final verdict: 🚫 Needs changes
Sent by Cursor Automation: PR Review
There was a problem hiding this comment.
Stale comment
This PR removes legacy per-chain hosted endpoint variables/fallbacks and switches default indexer workflows to the multichain config. The direction is correct, but it introduces a tooling regression and leaves key docs inconsistent with the new runtime behavior.
package.json
- 🔴 CRITICAL —
package.json:10-15
Removingindexer:celo-mainnet:codegen/indexer:celo-mainnet:devbreaks existing setup flows that still call those scripts.scripts/setup.sh:27runspnpm indexer:celo-mainnet:codegen, which now fails withERR_PNPM_RECURSIVE_EXEC_FIRST_FAIL Command "indexer:celo-mainnet:codegen" not found.
This is a hard failure for fresh clone/worktree bootstrap and should be fixed in the same PR (either restore compatibility aliases or update all call sites atomically).
README.md
- 🟡 IMPORTANT —
README.md:93-95
The env var table still documentsNEXT_PUBLIC_HASURA_URL_CELO_MAINNET_HOSTEDandNEXT_PUBLIC_HASURA_URL_MONAD_MAINNET_HOSTED, but fallback support was removed inui-dashboard/src/lib/networks.ts:194-213.
The docs now instruct users to set variables that no longer affect hosted mainnet routing.
ui-dashboard/src/lib/__tests__/networks.test.ts
- 🟡 IMPORTANT —
ui-dashboard/src/lib/__tests__/networks.test.ts:160-174
The two updated tests are functionally duplicate (both assert empty multichain URL => hidden Monad). After removing fallback branches inui-dashboard/src/lib/networks.ts, there is no explicit positive-path regression test proving that a populatedNEXT_PUBLIC_HASURA_URL_MULTICHAIN_HOSTEDconfigures bothcelo-mainnet-hostedandmonad-mainnet-hostedas intended.
indexer-envio/README.md
- 🟢 SUGGESTION —
indexer-envio/README.md:3,indexer-envio/README.md:62-67
Documentation is internally inconsistent after this change: the intro still says Celo-only scope while command docs describe multichain usage; the config table omits multichain config entries now presented as default.Final verdict: 🚫 Needs changes
Sent by Cursor Automation: PR Review
All traffic now routes through NEXT_PUBLIC_HASURA_URL_MULTICHAIN_HOSTED. The per-chain hosted endpoints (60ff18c Celo, cfeda9e Monad) are retired. Changes: - networks.ts: celo-mainnet-hosted and monad-mainnet-hosted now use MULTICHAIN_HOSTED directly (no per-chain fallback) - terraform/variables.tf: removed hasura_url_celo_mainnet_hosted and hasura_url_monad_mainnet_hosted variables - terraform/main.tf: removed hasura_url_celo_mainnet and hasura_url_monad_mainnet Vercel env var resources - .env.production.local.example: updated to reflect new single-endpoint model - terraform/terraform.tfvars.example: removed legacy overrides - networks.test.ts: updated tests to reflect removed fallback behavior
…se multichain config - pnpm indexer:codegen → config.multichain.mainnet.yaml (was config.celo.devnet.yaml) - pnpm indexer:dev → config.multichain.mainnet.yaml (was config.celo.devnet.yaml) - Removed indexer:celo-mainnet:* and indexer:monad-mainnet:* (merged into default) - Kept indexer:celo-sepolia:* and indexer:monad-testnet:* for testnet dev workflows - Updated README.md, indexer-envio/README.md, AGENTS.md, STATUS.md to match
- Remove references to single-chain deployments, per-chain deploy branches, old endpoint URLs, stale network table (Monad blocked) - Both READMEs now reflect: multichain-first, mento Envio project, static endpoint, simplified pnpm commands - Remove outdated env var table (CELO_MAINNET_HOSTED, MONAD_MAINNET_HOSTED) - Root README: updated architecture diagram, deployment section, key files table - indexer README: rewritten around multichain config, correct entity list, removed stale Celo-only contract address tables
4456024 to
d3e0d54
Compare
There was a problem hiding this comment.
Stale comment
This PR consolidates hosted endpoint usage around
NEXT_PUBLIC_HASURA_URL_MULTICHAIN_HOSTEDand removes legacy per-chain fallback vars from dashboard network resolution, Terraform env provisioning, and related docs/examples. The direction is correct, but there are two functional gaps that should be fixed before merge.
package.json
- 🟡 Important (
package.json:10-11): Removingindexer:celo-mainnet:codegen/indexer:celo-mainnet:devbreaks existing repo bootstrap paths that still invoke these commands (for examplescripts/setup.shcurrently runspnpm indexer:celo-mainnet:codegen). Repro:pnpm indexer:celo-mainnet:codegennow fails withCommand not found.- Suggested fix:
{ "scripts": { "indexer:celo-mainnet:codegen": "pnpm indexer:codegen", "indexer:celo-mainnet:dev": "pnpm indexer:dev" } }(If you do not want compatibility aliases, then update all remaining call sites in the same PR so setup is not broken.)
ui-dashboard/src/lib/__tests__/networks.test.ts
- 🟡 Important (
ui-dashboard/src/lib/__tests__/networks.test.ts:160-174): The two Monad tests now validate the same negative path (MULTICHAIN_HOSTEDempty => unconfigured). The positive path (multichain URL set => configured) was removed, which weakens regression coverage for the new env contract.- Suggested fix:
it("marks monad-mainnet-hosted configured when multichain URL is set", async () => { vi.stubEnv( "NEXT_PUBLIC_HASURA_URL_MULTICHAIN_HOSTED", "https://indexer.hyperindex.xyz/2f3dd15/v1/graphql", ); const networks = await import("../networks"); expect(networks.NETWORKS["monad-mainnet-hosted"].hasuraUrl).toBe( "https://indexer.hyperindex.xyz/2f3dd15/v1/graphql", ); expect(networks.isConfiguredNetworkId("monad-mainnet-hosted")).toBe(true); });
indexer-envio/AGENTS.md
- 🟢 Suggestion (
indexer-envio/AGENTS.md:56-57): The env section still documents legacy vars (ENVIO_RPC_URL,ENVIO_START_BLOCK) while this PR standardizes multichain usage elsewhere (ENVIO_RPC_URL_42220,ENVIO_RPC_URL_143, chain-specific start blocks). Aligning this text would avoid setup confusion.Final verdict: 🚫 Needs changes
Sent by Cursor Automation: PR Review
There was a problem hiding this comment.
Stale comment
Reviewed all 12 changed files in this PR:
.trunk/trunk.yaml,README.md,indexer-envio/AGENTS.md,indexer-envio/README.md,indexer-envio/STATUS.md,package.json,terraform/main.tf,terraform/terraform.tfvars.example,terraform/variables.tf,ui-dashboard/.env.production.local.example,ui-dashboard/src/lib/__tests__/networks.test.ts,ui-dashboard/src/lib/networks.ts.This PR removes legacy per-chain hosted endpoint vars/fallbacks and switches docs/scripts toward the multichain endpoint model. The infra and runtime direction is correct, but there are regressions that should be fixed before merge.
package.json
- 🟡 IMPORTANT (
package.json:10,scripts/setup.sh:27): removingindexer:celo-mainnet:codegenbreaks the documented bootstrap flow../scripts/setup.shstill callspnpm indexer:celo-mainnet:codegen, which now fails withERR_PNPM_NO_SCRIPT.- Impact: fresh clone/worktree setup is broken unless developers manually patch the script/command sequence.
- Required fix: update
scripts/setup.sh(and any other bootstrap paths) to use the new canonical script (pnpm indexer:codegen) or reintroduce a compatibility alias.
indexer-envio/AGENTS.md
- 🟡 IMPORTANT (
indexer-envio/AGENTS.md:55-57,indexer-envio/AGENTS.md:59): environment instructions are now inconsistent with the new default multichain workflow.:59tells users to run multichain by default, but:55-57still instruct setting onlyENVIO_RPC_URL/ENVIO_START_BLOCK. In multichain mode this can route both chains to one RPC and produce incorrect behavior for chain-specific reads.- Required fix: align env docs with multichain requirements (
ENVIO_RPC_URL_42220,ENVIO_RPC_URL_143, and chain-specific start blocks).
ui-dashboard/src/lib/__tests__/networks.test.ts
- 🟡 IMPORTANT (
ui-dashboard/src/lib/__tests__/networks.test.ts:160-174): the two tests in this block are functionally duplicate negative cases (both assert empty multichain URL => hidden network).- The new
networks.tsbehavior removed fallback logic, but there is no deterministic positive test here asserting that a non-emptyNEXT_PUBLIC_HASURA_URL_MULTICHAIN_HOSTEDis actually wired into bothcelo-mainnet-hostedandmonad-mainnet-hosted.- Required fix: replace one duplicate test with a positive-path assertion (including trimming behavior) to cover the new single-endpoint contract.
Final Verdict
🚫 Needs changes
Sent by Cursor Automation: PR Review
- scripts/setup.sh: update codegen call from indexer:celo-mainnet:codegen to indexer:codegen (broken bootstrap path after command rename) - indexer-envio/AGENTS.md: replace ENVIO_RPC_URL/ENVIO_START_BLOCK with per-chain vars (ENVIO_RPC_URL_42220, ENVIO_RPC_URL_143, etc.) and add warning against using generic ENVIO_RPC_URL in multichain mode - networks.test.ts: replace duplicate negative-case test with a positive-path test that verifies the multichain URL is wired into both celo-mainnet-hosted and monad-mainnet-hosted, including whitespace trimming
There was a problem hiding this comment.
Stale comment
This PR consolidates hosted dashboard traffic onto
NEXT_PUBLIC_HASURA_URL_MULTICHAIN_HOSTEDand removes legacy per-chain Terraform vars/fallbacks, plus updates docs/scripts for the new default multichain indexer flow.
package.json
- 🟡 IMPORTANT (
package.json:10):indexer:codegen/indexer:devnow default toconfig.multichain.mainnet.yaml, but the repo’s bootstrap env template still seeds the legacy genericENVIO_RPC_URL(inindexer-envio/.env.example). In multichain mode, that generic var is shared across all chains and can route chain-specific RPC reads to the wrong backend, causing incorrect indexing behavior during local dev.Final verdict: 🚫 Needs changes
Sent by Cursor Automation: PR Review
| "deploy:indexer": "./scripts/deploy-indexer.sh", | ||
| "indexer:celo-mainnet:codegen": "pnpm --filter @mento-protocol/indexer-envio codegen --config config.celo.mainnet.yaml", | ||
| "indexer:celo-mainnet:dev": "ENVIO_PG_PORT=5435 ENVIO_PG_DATABASE=envio-celo-mainnet ENVIO_START_BLOCK=60664500 pnpm --filter @mento-protocol/indexer-envio dev --config config.celo.mainnet.yaml", | ||
| "indexer:codegen": "pnpm --filter @mento-protocol/indexer-envio codegen --config config.multichain.mainnet.yaml", |
There was a problem hiding this comment.
Switching the default command to multichain here is correct, but it creates a data-correctness footgun unless the legacy generic RPC env path is removed/guarded.
run-envio-with-env.mjs still loads .env, and getRpcClient() still falls back to ENVIO_RPC_URL when per-chain vars are missing. If a developer has the old .env.example value (ENVIO_RPC_URL=...) copied locally, multichain runs can send both chains through one RPC backend.
Please fix in this PR by updating bootstrap/guarding behavior, e.g.:
- update
indexer-envio/.env.exampleto per-chain vars (ENVIO_RPC_URL_42220,ENVIO_RPC_URL_143) and remove genericENVIO_RPC_URL, and - fail fast when running multichain config with
ENVIO_RPC_URLset (to prevent silent misrouting).
…_URL/ENVIO_START_BLOCK)
gisk0
left a comment
There was a problem hiding this comment.
Thanks for the thorough review. Addressing each finding:
package.json (broken bootstrap): Already fixed — setup.sh was updated in a prior commit to use pnpm indexer:codegen (no more indexer:celo-mainnet:codegen call). Bootstrap is not broken.
networks.test.ts (missing positive-path test): Already present. Lines 168–186 contain the positive-path test that stubs NEXT_PUBLIC_HASURA_URL_MULTICHAIN_HOSTED with a URL (including whitespace), then asserts both celo-mainnet-hosted and monad-mainnet-hosted are configured and have the correct trimmed URL. The duplicate negative-case was also removed.
indexer-envio/.env.example (legacy ENVIO_RPC_URL/ENVIO_START_BLOCK): Fixed in latest commit. The example now documents ENVIO_RPC_URL_42220, ENVIO_RPC_URL_143, and optional per-chain start block overrides, with a warning against using the generic ENVIO_RPC_URL in multichain mode.
indexer-envio/AGENTS.md (legacy env vars): Already updated — the AGENTS.md env section documents the multichain vars and includes the explicit warning against generic ENVIO_RPC_URL.
README.md (stale env var table): Root README no longer documents the legacy per-chain hosted vars — only NEXT_PUBLIC_HASURA_URL_MULTICHAIN_HOSTED and NEXT_PUBLIC_HASURA_URL_CELO_SEPOLIA_HOSTED remain.
All findings addressed. CI is green. Ready to merge.
There was a problem hiding this comment.
This PR removes legacy per-chain hosted endpoint fallbacks and aligns the repo on a single multichain hosted endpoint (NEXT_PUBLIC_HASURA_URL_MULTICHAIN_HOSTED) across dashboard config, Terraform vars, scripts, and docs. It also updates indexer root scripts and tests to match the new multichain-only behavior.
No findings across the changed files after a full pass against the requested criteria (critical/important/suggestions).
Final verdict: ✅ LGTM
Residual risk/testing gap: I could not execute the test suite in this runner because dependencies are not installed (node_modules missing), so this review is static-analysis only.
Sent by Cursor Automation: PR Review
- scripts/setup.sh: update codegen call from indexer:celo-mainnet:codegen to indexer:codegen (broken bootstrap path after command rename) - indexer-envio/AGENTS.md: replace ENVIO_RPC_URL/ENVIO_START_BLOCK with per-chain vars (ENVIO_RPC_URL_42220, ENVIO_RPC_URL_143, etc.) and add warning against using generic ENVIO_RPC_URL in multichain mode - networks.test.ts: replace duplicate negative-case test with a positive-path test that verifies the multichain URL is wired into both celo-mainnet-hosted and monad-mainnet-hosted, including whitespace trimming


Stacks on #112 (merge that first).
Summary
All traffic now routes through a single
NEXT_PUBLIC_HASURA_URL_MULTICHAIN_HOSTED. The legacy Celo-only (60ff18c) and Monad-only (cfeda9e) endpoints are retired.Changes
networks.ts:celo-mainnet-hostedandmonad-mainnet-hostednow read directly fromNEXT_PUBLIC_HASURA_URL_MULTICHAIN_HOSTED— no per-chain fallbackterraform/variables.tf: removedhasura_url_celo_mainnet_hostedandhasura_url_monad_mainnet_hostedterraform/main.tf: removed the corresponding Vercel env var resources (NEXT_PUBLIC_HASURA_URL_CELO_MAINNET_HOSTEDandNEXT_PUBLIC_HASURA_URL_MONAD_MAINNET_HOSTED).env.production.local.example: updated to single-endpoint modelterraform/terraform.tfvars.example: removed legacy overridesnetworks.test.ts: updated to reflect removed fallback behaviorAfter merge
Run
terraform apply— it will removeNEXT_PUBLIC_HASURA_URL_CELO_MAINNET_HOSTEDandNEXT_PUBLIC_HASURA_URL_MONAD_MAINNET_HOSTEDfrom Vercel env vars.