feat(config): require explicit chain config — no silent Mutinynet defaults - #149
Merged
Merged
Conversation
…aults The chain-shaping env vars (IS_MAINNET, ESPLORA_URL, ESPLORA_WS_URL) used to silently default to Mutinynet endpoints, leaving two distinct silent footguns: (1) a Mainnet deploy that forgot ESPLORA_URL / ESPLORA_WS_URL would scan Mutinynet while answering /api/info as Mainnet, with a green /health/ready and a 5-s HTTP retry loop on the scanner (#84); (2) a Mutinynet deploy that left ESPLORA_WS_URL unset coupled itself to the public wss://mutinynet.com endpoint we do not operate. All three vars are now required-or-panic on both code paths — the same contract as USERNAME_DOMAIN, PUBLISHER_KEY, and DATABASE_URL. IS_MAINNET accepts only the exact strings "true" or "false"; values like "1", "TRUE", "yes" panic instead of silently meaning Mutinynet. Empty strings (ESPLORA_URL= in a compose file) are treated as unset. Code path consolidation: - lib::build_network_config_from_env panics on missing/empty/ambiguous values for all three vars. - scanner_ws no longer carries DEFAULT_ESPLORA_*_URL constants and no longer reads env directly. ScannerWsConfig::from_env is replaced by ScannerWsConfig::from_network_config(&EsploraConfig), consuming the already-resolved NETWORK_CONFIG. main.rs calls the new constructor. - publisher.rs doc-comment on EsploraConfig.ws_url updated to reflect the new "always Some(...) from build_network_config_from_env" invariant. - recover_inscription binary applies the same explicit-or-panic contract for its IS_MAINNET read. Guardrail: - New integration test node/tests/no_chain_hardcodes.rs scans every .rs file under node/src/ (excluding *_tests.rs and comment lines) for literal mutinynet.com / mempool.space URLs and fails the build if any appear in production source. Prevents a future "small refactor" from re-introducing the same class of default. Test + CI alignment: - main_tests.rs rewritten: removed the defaults_to_mutinynet_when_is_mainnet_unset / _is_not_true tests (their semantics no longer exist), added panic-path coverage for every missing / empty / ambiguous combination, plus a happy-path Mutinynet case symmetric to the existing Mainnet one. - scanner_ws_tests.rs swaps the from_env smoke for a from_network_config smoke and adds a panic-on-missing-ws_url test. - runtime_tests.rs adds the two new required vars to its defensive set_var block. - .github/workflows/ci.yaml: both node-tests and coverage env blocks now set IS_MAINNET=false and ESPLORA_WS_URL=ws://127.0.0.1:1/api/v1/ws alongside the existing ESPLORA_URL placeholder so the bootstrap panics no longer fail CI. Docs: - README §Configuration: defaults column flips to "(required, no default)" for the three vars, with a paragraph explaining the bias the change removes and the guardrail that backstops it. Per-stage endpoint examples included. - CONTRIBUTING § Env: same — required across the board. Minimal local- dev env snippet now sets all five required vars explicitly. The four CI env blocks (.github/workflows/ci.yaml) + the two test set_var sites + the new CONTRIBUTING dev-env snippet are the only places callers need to be aware of; production deploys (infrastructure/{dfxdev,dfxprd}/zkcoins/docker-compose.yaml) will need to set IS_MAINNET and ESPLORA_WS_URL explicitly on DEV — that follows in the DFXServer/server PR (PR 2 of the chain-config track).
Three sites the first commit missed: 1. `recover_inscription` binary carried a `DEFAULT_ESPLORA_URL = "https://mutinynet.com/api"` constant plus an `unwrap_or_else` fallback that would silently broadcast against Mutinynet when the `--esplora-url` flag was omitted. Same class of footgun as the removed node-side default: an operator recovering a Mainnet inscription with the flag forgotten would target the wrong chain. `--esplora-url` is now required; the binary exits with the standard usage error if missing. Constant deleted. 2. `lib::build_network_config_from_env`'s `ESPLORA_URL` panic message embedded the DFX-specific internal hostnames (`electrs-mainnet:3000`, `electrs-mutinynet:3000`) as concrete examples. Per-stage endpoints belong in the README, not in the source — the message now points there. Mirrors the symmetric treatment already adopted for the `ESPLORA_WS_URL` panic in the same function. 3. `main_tests.rs` carried `wss://mempool.space/api/v1/ws`, `ws://mempool-api-mutinynet:8999/api/v1/ws`, and the `electrs-{main,mutinynet}:3000` hostnames as test fixtures. The guardrail test exempts `*_tests.rs`, so this was not a correctness violation — but a Mainnet PRD URL appearing verbatim in source on any merge is the wrong signal regardless. Fixtures migrated to clearly-fake `.test` hostnames (`mainnet-ws.test`, `mutinynet-ws.test`, `electrs-{mainnet,mutinynet}.test:3000`) that document intent without coupling tests to a real-world host. Doc-comment and inline-comment references to `mutinynet.com` / `mempool.space` are retained — they're historical context describing the bias this PR removes, and the guardrail test allows them mechanically.
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.
Summary
The chain-shaping env vars (
IS_MAINNET,ESPLORA_URL,ESPLORA_WS_URL) used to silently default to Mutinynet endpoints. Two distinct silent footguns:ESPLORA_URL/ESPLORA_WS_URLwould scan Mutinynet while/api/inforeported Mainnet, with a green/health/readyand a 5-s HTTP retry loop on the scanner (architecture: no polling, events only (scanner Esplora, repo-wide principle) #84).ESPLORA_WS_URLunset coupled itself to the publicwss://mutinynet.comendpoint we do not operate.This PR makes all three explicit-or-panic on every code path — same contract as
USERNAME_DOMAIN,PUBLISHER_KEY,DATABASE_URL. Plus a mechanical guardrail test that prevents the literal URLs from creeping back into the source.What changes
Code (production):
node/src/lib.rsbuild_network_config_from_envpanics on missing / empty / ambiguousIS_MAINNET,ESPLORA_URL,ESPLORA_WS_URL.IS_MAINNETaccepts only the exact stringstrue/false.node/src/scanner_ws.rsDEFAULT_ESPLORA_WS_URLandDEFAULT_ESPLORA_HTTP_URLconstants deleted.ScannerWsConfig::from_env→from_network_config(&EsploraConfig)— single source of truth.node/src/main.rsnode/src/publisher.rsEsploraConfig.ws_urldoc-comment updated to reflect "alwaysSome(...)" invariant.node/src/bin/recover_inscription.rsIS_MAINNET.Guardrail:
node/tests/no_chain_hardcodes.rs— scans every.rsfile undernode/src/(excluding*_tests.rsand comment lines) for literalmutinynet.com/mempool.spaceURLs in any scheme. Fails the build on hit. Prevents a future "harmless refactor" from re-introducing a default.Tests:
main_tests.rsrewritten — olddefaults_to_mutinynet_when_*tests deleted (their semantics no longer exist); added panic-path coverage for every missing / empty / ambiguous combination on every var; symmetric happy-path Mutinynet case added.scanner_ws_tests.rs—from_envsmoke replaced withfrom_network_configsmoke + a panic-on-missing-ws_urlregression.runtime_tests.rs— defensiveset_varblocks now also setIS_MAINNET=falseandESPLORA_WS_URL..github/workflows/ci.yaml— bothnode-testsandcoverageenv blocks now setIS_MAINNET=falseandESPLORA_WS_URL=ws://127.0.0.1:1/api/v1/wsalongside the existingESPLORA_URLplaceholder.Docs:
README.mdConfiguration table — three vars flip fromhttps://mutinynet.com/apistyle defaults to(required, no default). New paragraph explains the bias removed and the guardrail backing it.CONTRIBUTING.md— same, plus the minimal local-dev env snippet now sets all five required vars explicitly.Why a separate guardrail test
The literal URL string is exactly the pattern a reviewer cannot easily catch on a one-line diff that "moves a default somewhere sensible". Mechanical enforcement is the only durable fix — otherwise the next person hardening some adjacent piece of code re-introduces the same class of fallback in good faith.
Coordinated follow-up
DEV's deploy compose (
DFXServer/server/infrastructure/dfxdev/zkcoins/docker-compose.yaml) currently does not setESPLORA_WS_URL— it relies on the public Mutinynet default this PR removes. The companion PR on the DFXServer repo adds the explicit env. Merge order: DFXServer PR first (the new env is harmless under the old code), then this PR (the new code requires the env).Test plan
ci:full—lint-and-build,Node + Shared Tests (M3 Ultra),Coverage Gate (100% lines + functions).cargo fmt --all --check✓,cargo clippy -p node -p shared -- -D warnings✓,cargo check --workspace --all-features✓ (already verified before push).Node + Shared Testsonceci:fullis applied).