fix(commit): sync SMT update in broadcast_commit_and_deliver - #152
Merged
Conversation
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.
TaprootFreak
marked this pull request as ready for review
May 31, 2026 12:10
TaprootFreak
added
the
ci:full
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.
Summary
Brings the Send-Commit path in line with Mint-Commit Phase E: the SMT integration now completes synchronously inside
broadcast_commit_and_deliver, before/api/commitreturns 200. Eliminates the race window where a follow-up send reads `account.commitment_public_key` from server state but finds no matching SMT entry yet (because the scanner hasn't yet observed the on-chain inscription).Closes the latent root cause behind the deterministic-on-#150 failure of `second_send_roundtrip_succeeds_without_prev_commitment_pubkey_field` — the test was always race-prone, Mutinynet latency only made it visible on one specific run. Code diff between green run (e0e256f) and red run (fc31964) was 0 server bytes; the test landed on the wrong side of the timing window.
What changed
Architectural symmetry
DRY helper
Extracted Phase E into `router.rs::apply_commit_and_persist_phase_e(state, commitment, commit_txid, flow_label)`, returning a structured `PhaseEFailure` enum (`StateUpdate` | `DurablePersist`). Both call sites (mint and commit) map the variant onto their own flow-specific error string so existing tests + KNOWN_SERVER_ERRORS stay unchanged.
Scanner symmetry
`should_skip_scanner_state_update` already gates on `pending_inscriptions.status='complete'`. Since the new commit-Phase-E sets that marker in the same atomic Postgres transaction as the state advance, the scanner becomes a redundant observer for our own send-commits (identical to its existing role for mint-commits). External recovery inscriptions still flow through the scanner path unchanged.
Files
Verification (local)
(Heavy nextest / Coverage Gate is CI-only per repo convention.)
Test plan
PR target
`staging` per the new `feature/* → staging → develop → main` flow introduced in #148.