Release: develop -> main - #150
Merged
Merged
Conversation
…#148) Mirrors the existing develop → main auto-release-pr.yaml one layer down. Triggers on push to staging, opens (or no-ops on existing) "Promote: staging -> develop" PR for the operator to merge once the accumulated feature work is ready to land on DEV. Naming separates the two layers: - staging → develop is a Promote (DEV deploy is a follow-on effect, not the purpose). - develop → main stays Release (production cut). Promote PRs intentionally do NOT apply the ci:full label. The heavy M3 Ultra test + coverage gate stays reserved for the Release PR, which remains the authoritative pre-PRD gate. Promote PRs run the slim Lint & Build job + Analyze / CodeQL, matching what every ready feature PR sees. CONTRIBUTING.md updated: - Branches table now lists staging as the integration buffer between feature/* and develop. - Workflow rule rewritten: open feature PRs against staging, not develop. develop is fed by the staging auto-PR; main by the develop auto-PR. - Explicit protection statement re-anchored: develop + main reject direct pushes, hotfixes go through staging. - CI/CD table extended with the new workflow row. Bootstrap note: this PR targets develop directly because the staging-side workflow doesn't exist yet — once merged + synced to staging, all subsequent feature work follows the staging → develop flow.
github-actions
Bot
added
the
ci:full
…g backend support (#145) § 7.24 of MIGRATION_RESEARCH.md and the matching CONTRIBUTING.md historical note previously claimed that self-hosted mempool/backend:v3.3.1 does not implement the track-tx WS action. A direct websocat probe on dfxdev falsifies that: - {"action":"track-tx","data":"<txid>"} → 0 frames in 6 s - {"track-tx":"<txid>"} → immediate txPosition frame The publisher's pre-#144 frame (scanner_ws.rs on ae78798^, lines 650-655) used the {action, data} envelope. mempool.js's wsTrackTransaction (canonical client) sends {"track-tx":"<txid>"} at the top level, and mempool/backend:v3.3.1's websocket-handler.ts keys off parsedMessage['track-tx']. The backend silently dropped the malformed frame — indistinguishable from "action not supported" from the client side. PR #144 still stands on independent grounds: closed test environment (no external Esplora to subscribe against), race-free in-cluster topology (bitcoind local-mempool accept orders the two POSTs), and ~200 LOC removed for zero behavioural loss. Measured DEV impact: /api/mint p50 40 s → 8.7 s (4.6×), /api/send + /api/commit p50 42 s → 12.7 s (3.3×). Doc-only; no source files touched.
…aults (#149) * feat(config): require explicit chain config — no silent Mutinynet defaults 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). * fix(config): scrub remaining chain URLs from production code and tests 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.
The hostname does not tell you which chain a stage backs: api.zkcoins.app runs against Mainnet (electrs-mainnet, IS_MAINNET=true), dev-api.zkcoins.app runs against Mutinynet (electrs-mutinynet, IS_MAINNET=false). Without this column a reader has to cross-reference the deploy compose or trip over the Mutinynet-flavoured source defaults (NETWORK_NAME, DEFAULT_ESPLORA_*_URL) and conclude PRD is on Mutinynet.
3 tasks
Bring the send-commit path in line with the mint-commit Phase E so the
SMT integration completes synchronously before /api/commit returns
200, closing the race window where a follow-up send reads
`account.commitment_public_key` from server state but finds no
matching SMT entry yet (the async scanner has not observed the
on-chain inscription).
The race surfaced as 422 "Unable to get merkle proofs for provided
public key" in the regression test
`second_send_roundtrip_succeeds_without_prev_commitment_pubkey_field`
when run against dev-api.zkcoins.app: a wallet that chains /api/send
+ /api/commit + /api/send hit the second send before the scanner
finished its ~20 s reveal-observation lap on Mutinynet. There was no
server code regression — the send path had always relied exclusively
on the scanner to integrate the commit, while the mint path already
did it inline after the broadcast (Phase E, router.rs::mint_handler).
The asymmetry was latent and Mutinynet latency made it visible.
Changes:
* Extract the Phase E body (state.update + atomic
persist_state_and_mark_complete_tx) into a shared helper
`apply_commit_and_persist_phase_e` in router.rs. The helper takes
a `flow_label` for logging and returns a structured
`PhaseEFailure` so the two call sites can preserve their existing
flow-specific public error strings ("mint broadcast..." vs
"commit broadcast...").
* `mint_handler` now delegates Phase E to the helper. Behaviour is
byte-identical for happy path and both Err arms; existing tests
(`mint_handler_advances_state_synchronously_with_broadcast`,
`mint_handler_atomic_tx_rollback_leaves_state_and_row_consistent`,
`mint_handler_in_process_state_advance_collision_returns_503`)
continue to pass.
* `broadcast_commit_and_deliver` now invokes the helper synchronously
between the Bitcoin broadcast and the recipient `receive_coin`
mutation. On failure: 503, no retry, no fallback — scanner-replay
remains the single source of repair, exactly as in the mint flow.
* `broadcast_commit_and_deliver` also switches from the process-wide
`NETWORK_CONFIG` lazy_static to `state.esplora_config` so the
send-commit path becomes testable with a wiremock Esplora, matching
the testability shape already in place for `mint_handler`.
Production behaviour is unchanged because `start_rest_node` clones
`NETWORK_CONFIG` into that slot.
* Update the `commit_handler` doc to describe the new Phase E
symmetry and remove the stale "no analogue of the mint state-desync
class here" sentence.
* Add two new tests in `router_tests.rs` mirroring the existing mint
Phase E coverage:
- `commit_handler_advances_state_synchronously_with_broadcast` —
happy path: /api/send + /api/commit with mocked accepting Esplora
and live Postgres, then verify SMT contains pk_0, MMR
leaf_count == 1, root_indices has the new entry, and
pending_inscriptions row sits at `complete` so
`should_skip_scanner_state_update` fires.
- `commit_handler_atomic_tx_rollback_leaves_state_and_row_consistent`
— install a trigger that fails the in-tx UPDATE to `complete`,
assert 503 with the expected error substring, and verify on-disk
SMT/MMR/root_index stays untouched and the row stays at
`reveal_broadcast` for scanner-replay to integrate from chain.
Lock topology and crash-recovery contract are preserved verbatim
(both documented in the helper's docstring). The scanner remains the
authoritative path for external recovery inscriptions but is now a
redundant observer for our own send commits too — exactly as it
already was for mint commits.
Promote: staging -> develop
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
1 participant
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.
Automatic Release PR
Commits: 1 new commit(s)