Release: develop -> main - #18
Merged
Merged
Conversation
The Explorer is now its own top-level domain (brand strategy A: zkCoins is one brand across Wallet/Exchange/Explorer), not a subdomain of the Wallet. The Open Tasks pointer for the planned explorer endpoints reflects that.
Trustless HTLC-based swap design where the atomicity primitive lives on the Bitcoin funding tx of the 4242-prefix inscription, not on the coin layer. Covers both swap directions (LN→zkCoins via reverse submarine, zkCoins→LN via mirror), Bitcoin script construction (P2WSH and Taproot variants), CLTV/T_lock coordination, failure-mode matrix, provider operations, and privacy analysis. D7 reorg safety is the single open zkCoins-side dependency; mitigated in v1 with conservative 6-confirm gating until the conditional_nav fix lands per ROADMAP pre-mainnet hardening. Orthogonal to the Plonky2 migration — the 24h LN CLTV window dwarfs even SP1 minute-scale proof times, so swap implementation can ship on either backend.
Companion to LIGHTNING_ATOMIC_SWAP.md. Addresses D11 — the operator- controlled MINTING_ADDRESS bypass — by binding coin issuance to BTC custody in a BitVM2-style bridge. Covers: - Why D11 is the largest residual trust gap (recipient inflation risk) - BitVM2 / Clementine architecture as deployed by Citrea, with exact trust assumptions (1-of-N setup honesty, 1-of-N watchtower liveness) - New IssuanceProof and BurnProof circuit branches - Peg-in and peg-out flows step-by-step - Realistic alternatives at lower cost (Liquid-style federation as short-term path, BitVM2 as long-term) - Privacy implications, open questions, comparison tables Recommends Liquid-style federation as 2-3 month implementable v2, BitVM2 as 6-9 month v3 with multi-org federation recruitment. D11 fix should be added to ROADMAP pre-mainnet hardening block (currently missing).
Initial BITVM_BRIDGE.md draft treated BitVM2 as the only mature option and BitVM3 as "research-stage, may upgrade later". Reality in 2026-05 is more nuanced: - BitVM3-RSA was withdrawn after Eagen / Fairgate found security flaws. Successor work continues under BitVM3-CC (BOB demoed on mainnet at ~$10.91 per dispute). - Glock (Alpen Labs, eprint 2025/1485, by Liam Eagen — same author family as Shielded CSV) uses garbled circuits + a novel compact DV-SNARK to achieve 430-550x on-chain efficiency over BitVM2. Strata bridge transition planned. Starknet partnership announced. - Mosaic (eprint 2026/812, Khambhati / Tiwari / Bajracharya / Bista / Eagen / Lewe / Feickert) uses cut-and-choose with polynomial label correlation + adaptor signatures to achieve on-chain footprint independent of N copies. Full Rust impl exists. - Of all these, BitVM2 (Citrea Clementine, mainnet since 2026-01-27) remains the only mainnet-deployed option. New §12 surveys the landscape; §3 gets a 2026-context note; §11.1 / §11.2 / §11.3 comparison tables refreshed; §13 Bottom Line includes hedging strategy (circuit-side IssuanceProof / BurnProof contracts are construction-agnostic, so we can build now and pick the verifier later); references reorganised into themed groups.
Concrete 8-phase engineering plan for a trustless BTC <-> zkCoins bridge MVP. Federation topology is 3 nodes (initially same data centre) — this proves the cryptographic and protocol-level correctness of the bridge mechanism; trust-distribution via independent operators is a separate operational concern handled by swapping federation members in config. Locks three technical decisions: - BitVM2 as v1 construction (Glock/Mosaic deferred) - Bitcoin Light Client as separate Plonky2 sub-proof, recursively verified inside IssuanceProof (avoids SHA256d gate explosion) - Single-contributor trusted setup SRS for MVP, with explicit DO-NOT-USE-IN-PRODUCTION marker; real ceremony before federation deployment Phases: 1. Circuit extension (IssuanceProof + BurnProof) 3-4 weeks 2. Bitcoin Light Client gadget 3-5 weeks 3. State extension (peg_in_consumed_smt etc.) 1 week 4. MuSig2 signer node 3-4 weeks 5. Operator + watchtower daemons 3 weeks 6. Bridge-aware server API 2 weeks 7. Plonky2 -> Groth16 wrapping 3-4 weeks 8. Integration on signet 3-4 weeks Total: ~5-7 months for a 3-node functional MVP. Closes D11 (per SPEC.md §15) for the activated federation surface once Phase 8 passes; trust-distribution is then a configuration swap.
The bridge MVP runs with N=3 in same data centre (engineering
correctness only). The production target is N=100 independent
members under strict 1-of-N setup-honesty.
N=100 is the practical upper bound of the BitVM2 framework today
per Bitlayer's analysis ("in practice the value of n can be 100").
Beyond N=100 is open research (Bitlayer: "It is necessary to
research a permissionless multi-party OP challenge protocol that
could expand BitVM's existing 1-of-n trust model to 1-of-N, where
N is much larger than n") and not a current goal.
Updates:
- BRIDGE_MVP.md §2.2: federation scaling beyond N=3 added as
deferred item with N=100 target and N=10 -> N=30 -> N=100
milestones
- BITVM_BRIDGE.md §13 Bottom Line: explicit production target N=100
Three substantive corrections plus structural polish across all four documents (BITVM_BRIDGE.md, BRIDGE_MVP.md, LIGHTNING_ATOMIC_SWAP.md, README.md): 1. BITVM_BRIDGE.md §12.4 — correct the Glock trusted-setup claim. Previous wording suggested Glock's DV-SNARK might not require a setup. It does. Glock is instantiated with Pari (Eagen et al., eprint 2024/1245), and the Pari paper explicitly states it requires a circuit-specific trusted setup, comparable to Groth16. The advantage of Glock is on-chain efficiency and proof size, not setup transparency. 2. Branch notes on all three design docs (LIGHTNING_ATOMIC_SWAP.md, BITVM_BRIDGE.md, BRIDGE_MVP.md) explaining that SPEC.md / MIGRATION_RESEARCH.md / ROADMAP.md currently live on feat/plonky2-migration and will resolve on develop only after PR #17 lands. Hyperlinks to those files in BRIDGE_MVP.md §15 References downgraded to plain references with branch annotation. 3. README.md — new "Design Documents" section listing the three draft documents with scope summaries and the branch caveat, placed between "Related" and "Protocol" sections. Consistency checks passed: - N=100 federation target referenced consistently in BITVM_BRIDGE.md §13 and BRIDGE_MVP.md §2.2. - Citrea mainnet date (2026-01-27) used consistently. - D11 / MINTING_ADDRESS / IssuanceProof / BurnProof terminology consistent across all three design docs. - Pattern 9.4 in LIGHTNING_ATOMIC_SWAP.md cross-references resolved. - §-level cross-references between docs (BITVM_BRIDGE.md §12.8 from BRIDGE_MVP.md §13.1, etc.) verified correct.
Substantive cleanups across LIGHTNING_ATOMIC_SWAP.md and
BITVM_BRIDGE.md, focused on professionalism and internal consistency.
LIGHTNING_ATOMIC_SWAP.md:
- §9 restructured. Previous version walked through four candidate
Flow B patterns (9.2, 9.2a, 9.2b, 9.3, 9.4) in stream-of-
consciousness style — three of them were explicitly flagged as
non-trustless during the discussion, and the recommended one
(9.4) sat at the end. The new §9 leads with §9.2 as the
recommended construction (mirror of Flow A) including a clean
step-by-step protocol and a dedicated failure-mode table; §9.3
explains tersely why the "provider generates preimage" patterns
fail to close the trustlessness gap. The §9 narrative is now
goal-first, not exploration-first.
- Fix four broken internal section references:
- §8.1, §8.2 step 7, §8.3 row: "see §10" → "see §12"
(§10 is Bitcoin Script Construction; the intended target is
§12 Timing Coordination).
- §13 failure-modes-matrix: "See §15 (D7 dependency)" → "See §16
(D7 dependency)" (D7 lives in §16, §15 is Privacy Analysis).
- §13 failure-mode "Provider claims LN but withholds inscription
broadcast" rewritten to reflect the new §9.2 design (the row
previously referenced the deleted patterns 9.3/9.4).
- §10.5 Pubkey choices: drop "Pattern 9.4" reference (which no
longer exists after the §9 restructure).
- §19.1 Phase 0 prerequisites: drop "Pattern 9.3" hold-invoice
reference; standard HTLC support suffices.
- §19.3 API surface: simplify "Flow B Pattern 9.4" → "Flow B".
- §20 Open Questions: remove the now-redundant first question
("Pattern choice for Flow B") since §9 has settled it. Renumber
remaining questions.
BITVM_BRIDGE.md:
- §6.2 Step 1: replace the one-off term "WithdrawalProof" with
`BurnProof` (the canonical name used everywhere else — §4.1,
§6.3, §11.2). Fix the broken cross-reference §5.2 → §6.3
(§5.2 is the Peg-In protocol-steps section; the intended target
for the BurnProof predicate branch is §6.3).
Polish pass across all three bridge documents (LIGHTNING_ATOMIC_SWAP.md,
BITVM_BRIDGE.md, BRIDGE_MVP.md) for professional presentation and
consistency with the rest of the repository.
Header structure harmonised. All three docs now use the same order:
**Status:** … (companion-to-other-docs reference)
**Authoritative source for:** …
**Audience:** …
> **Branch note.** …
Previously LIGHTNING_ATOMIC_SWAP.md folded the "Authoritative source"
into the Status block, BITVM_BRIDGE.md placed the Branch note before
Audience, and BRIDGE_MVP.md had Audience before Authoritative source.
Now identical across all three.
Organisation-specific references removed. The bridge documents
previously contained nine references to "DFX" (one document even
named a person), while the rest of the repository — SPEC.md,
ROADMAP.md, CONTRIBUTING.md, README.md — contains zero. The bridge
docs are now consistent with the repo convention: generic wording
("a regulated provider", "a single-organisation issuer", "the
initiating operator", "the bridge operator", etc.). The one
organisation-named mention in MIGRATION_RESEARCH.md §5.6 is left
as-is because it pre-dates this convention and lives on the
migration branch.
Other small polish:
- LIGHTNING_ATOMIC_SWAP.md §4.2: define `asth` and `ocr` at first
use (referring to SPEC.md's glossary for consistency).
- LIGHTNING_ATOMIC_SWAP.md §17 heading tightened from "Plonky2
Relevance (Spoiler: Orthogonal)" to "Plonky2 Relevance —
Orthogonal to the Swap Design".
A well-formed address that has never been observed on chain is the canonical zero-balance state, not a not-found condition. Returning 404 broke first-poll flows on brand-new wallets: the client's generic `if (!res.ok) throw` path swallowed the response and setBalance was never called, leaving wallets stuck in a loading state. Now 4xx is reserved for genuinely malformed input (invalid hex, wrong length, missing address parameter). Also preserves the username field when the address is registered via the independent username_store but has no on-chain activity yet. Closes #20
…rved address (#24) - README "Get balance" section now spells out the canonical zero response (200 + balance:0) versus malformed input (422) versus missing param (404). - Add `balance_unknown_address_with_claimed_username_returns_username` to lock in the new semantic that the username field is populated whenever the address is registered in the independent username_store, even before any on-chain activity.
…lformed inputs (#27) The `/api/balance` endpoint returned 404 when the required `address` query parameter was absent. That clashed with the 422 returned for the other malformed-input branches in the same handler (invalid hex, wrong length) and made the status-code surface inconsistent. A missing required parameter is unprocessable input, not a routing miss. - server/src/server.rs: switch the no-address branch from 404 to 422 - server/src/server_tests.rs: rename `balance_missing_address_param_returns_not_found` → `_returns_unprocessable` and update the assertion - README.md: collapse the malformed-input clause so all three cases (invalid hex / wrong length / missing param) share one status code
Add a capabilities object to /api/info reflecting the compile-time Cargo feature set (address_list, faucet, usernames, lnurl) so the app can render capability-driven UI from a single server-side source of truth instead of mirroring NEXT_PUBLIC_ENABLE_* build flags.
…ook (#33) The full server+shared test suite plus the MVP coverage gate is ~8 min on an M3 Ultra and >75 min on ubuntu-latest, repeatedly hitting the runner timeout. Re-running locally-passed tests in CI just to fail them slowly adds no signal. Drop the Tests and Coverage (MVP scope) jobs from ci.yaml. Add .githooks/pre-push that runs fmt, three clippy invocations, MVP and all-features release builds, the full test suite (server + shared, all-features, including account_server::tests), the program lib tests, and cargo llvm-cov with the existing --fail-under-lines 100 / --fail-under-functions 100 gate. Document the one-time core.hooksPath activation in a new CONTRIBUTING.md Setup section and update the CI/CD table to reflect that ci.yaml now only does lint and build. Closes #30.
…in> per stage (#32) * feat(info): report username_domain so the client can render `<x>@<domain>` per stage Companion to zk-coins/app#95. DEV and PRD live behind different external hostnames (`dev.zkcoins.app` vs. `zkcoins.app`) but can serve the same chain — the client cannot derive the rendering domain from `network` or `apiUrl` and the current PRD-suffix hardcode in the app silently routes funds to the wrong stage on a hex-prefix collision. Add a separate `USERNAME_DOMAIN` lazy_static (default `zkcoins.app`) and return it next to `network` from `/api/info`. The server is now the source of truth — operator misconfig is visible in the response instead of hidden behind a client-side env var. - main.rs: USERNAME_DOMAIN env-driven with PRD default, prints the resolved value at startup like NETWORK_CONFIG. - server.rs: InfoResponse gains `username_domain`; info_handler reads the constant. - server_tests.rs: info test now asserts both fields are non-empty (renamed from `info_returns_network_name` to `info_returns_network_name_and_username_domain`). - README.md: behaviour and env-var table updated. Deploy: DEV container env must set `USERNAME_DOMAIN=dev.zkcoins.app` (see env-var table in README.md). PRD leaves it unset. * feat(info): make USERNAME_DOMAIN required — no silent fallback A silent default to `zkcoins.app` would let a misconfigured DEV image (env forgotten in compose) keep serving the PRD-looking hostname and silently reproduce the cross-network routing bug this whole envelope was added to fix. Crash the bootstrap instead so the misconfig is visible the moment it happens, not after a wrong-stage send. - main.rs: switch `unwrap_or_else` → `expect` with a message that spells out both stages' values and links to #95. - .github/workflows/ci.yaml: set `USERNAME_DOMAIN=test.zkcoins.local` at the workflow level so every job (lint, test, coverage, clippy) inherits a test value. CI runs the same binary the deploy images ship — the env now must be set there too. - README.md: mark `USERNAME_DOMAIN` as required (panics on startup if unset) in both the Network info behaviour section and the env var table.
* docs: add circuit specification for proof-system migration
Extracts an implementation-agnostic spec of the zkCoins state-transition
circuit from the current SP1 implementation. Covers types, hash-function
abstraction, SMT/MMR structures, commitment format, program inputs,
circuit asserts, recursion contract, off-circuit responsibilities, and
migration notes for porting to a Plonky2/Poseidon backend.
Intended as a stable reference for follow-up work in this branch.
* docs: add migration research with reference comparison
Synthesises analysis of the BitVM/zkCoins Plonky2 prototype and the
ShieldedCSV/ShieldedCSV reference implementation against the current
SP1 codebase.
Surfaces 11 protocol-level divergences (D1-D11) between our current
implementation and the published Shielded CSV protocol — including a
privacy regression around plaintext recipient addresses, missing fee
support, missing conditional-noop on reorg, and the structural switch
from the paper's tuple-of-sets nullifier accumulator to our SMT+MMR
scanner model.
Provides an adoption plan per area (from BitVM repo, from upstream
reference impl, from our SP1 code, newly required) and a recommended
8-step sequencing for the actual Plonky2 port.
Open decisions for product/protocol owner are listed explicitly so they
can be resolved before any Plonky2 code is written.
* docs(SPEC): reconcile with Shielded CSV paper, add divergences section
Adds a scope note up front clarifying this spec describes the zkCoins
MVP variant, not paper-fidelity Shielded CSV. Adds §15 with the
divergence table (D1-D11) cross-referenced to MIGRATION_RESEARCH.md.
Adds ShieldedCSV/ShieldedCSV as the normative reference (alongside the
paper); demotes BitVM/zkCoins to "IVC scaffold only" since that's what
it actually is.
Flags D2/D7/D8/D10 as mainnet blockers (privacy, reorg safety,
soundness) so the table doubles as a pre-mainnet checklist.
* feat(program-plonky2): scaffold standalone crate for plonky2 backend
Adds program-plonky2/ as a standalone crate (not a workspace member).
Plonky2 1.1.0 requires nightly Rust for feature(specialization); the
rest of the workspace stays pinned to stable 1.81.0 for SP1, so the
new crate carries its own rust-toolchain.toml selecting a recent
nightly.
Contents are minimal on purpose: the prelude (field, hash config,
recursion arity) and a smoke test that builds and verifies a trivial
proof. Real gadgets and the monolithic state-transition circuit will
land in follow-up commits once the open protocol decisions in
MIGRATION_RESEARCH.md §5 are resolved.
The crate has not been compiled locally yet — nightly is not installed
on the current machine. The choice of plonky2 1.1.0 is deliberate
(latest stable release on crates.io); the BitVM reference used 0.2.0
which is now several major versions stale.
* chore(program-plonky2): lock toolchain choice, record §5 decisions
- Lock §5 design decisions in MIGRATION_RESEARCH: zkCoins MVP variant
(paper fidelity deferred to v2), MAX_IN_COINS = 8, Poseidon over
Goldilocks everywhere in Merkle structures, BIP-340 Schnorr kept at
the Poseidon ↔ Bitcoin boundary, plaintext recipient v1 (D2/D10
deferred), no fee in v1 (we publish ourselves).
- Validate program-plonky2/ scaffold builds and the smoke test passes
on nightly-2025-04-15 + plonky2 1.1.0. Commit the resolved Cargo.lock
to pin transitive deps. Fix two warnings (set_target returns Result
in plonky2 1.x).
* feat(program-plonky2): add Poseidon hash module
Defines the protocol hash function H for the Plonky2 backend:
- HashDigest type alias to plonky2's HashOut<F> (4 Goldilocks elements,
~256 bits) — the canonical in-circuit and off-circuit shape.
- hash_concat(left, right) -> Poseidon two-to-one for Merkle node hashing.
- hash_bytes(&[u8]) -> Poseidon hash with 7-byte-per-field-element packing
to guarantee canonical Goldilocks reduction (8-byte chunks would risk
non-canonical wrap on inputs above the field modulus).
- digest_to_bytes / digest_from_bytes for the Bitcoin-signing boundary
(Schnorr message = SHA256 over these exact bytes).
- ZERO_HASH constant for MMR padding and SMT sentinel slots.
Five unit tests pin the API surface: determinism, input distinction,
byte round-trip with witnessed layout, ZERO_HASH shape, and canonical
chunk safety on max-byte inputs.
Building block for the upcoming Poseidon SMT and MMR ports.
* feat(program-plonky2): port sparse Merkle tree to Poseidon
Ports program/src/merkle/sparse_merkle_tree.rs algorithmically (SHA256 →
Poseidon, [u8;32] value → HashOut<F>), keeping the byte-level 256-bit
key layout for compatibility with the existing MSB-first bit selector
path.
Discovered and fixed a structural collision with the naive
DEFAULT_HASHES seed. Choosing ZERO_HASH as DEFAULT_HASHES[TREE_DEPTH]
makes every level's default a Poseidon image of the all-zero state.
Plonky2's Poseidon sponge has the property that hash_no_pad([F::ZERO])
and two_to_one(ZERO, ZERO) both permute the all-zero state and produce
the SAME digest — so any leaf whose value+key are themselves
hash-of-zero outputs (very natural for derived test data and for real
fresh accounts) collides with DEFAULT_HASHES[TREE_DEPTH - 1], and the
chase loop in generate_non_inclusion_proof silently follows the wrong
branch.
Fix: seed DEFAULT_HASHES[TREE_DEPTH] with hash_bytes of a fixed
domain-separated tag ("zkcoins:smt:empty-leaf:v1"). This breaks the
structural collision without changing the tree's protocol semantics.
Adds a regression test (leaf_hash_never_collides_with_defaults) that
fails fast if anyone reverts the seed back to a zero-derived constant.
Ports the original SMT's 10 tests verbatim (renamed test corpus to
deterministic sample_keys() + sample_value()), plus the regression
guard. 17 tests pass on nightly-2025-04-15 + plonky2 1.1.0.
Persistence helpers (save/load to file) and serde derives are
intentionally absent for now — they will be added when the host wires
this into the server's state module.
* feat(program-plonky2): port Merkle mountain range to Poseidon
Ports program/src/merkle/merkle_mountain_range.rs algorithmically:
SHA256 hash_concat → Poseidon two-to-one, [u8;32] HashDigest →
HashOut<F>. Capacity doubling, branch-only updates, and ZERO_HASH
padding for missing right siblings are all preserved unchanged from the
SP1 implementation.
The MMR has no chase loop or sparse logic, so the structural
DEFAULT_HASHES collision that bit the SMT does not apply here —
ZERO_HASH as the padding sentinel is safe (it appears only as a
literal sibling, never as a recursively-derived level default).
Ports the SHA256 MMR's tests (renamed to be intent-describing, dropped
the bincode serialization test since serde derives are deferred). 8
tests cover empty tree, single leaf, two leaves, growing trees,
out-of-bounds proofs, tamper detection, and capacity expansion.
All 25 tests in program-plonky2 pass on nightly-2025-04-15.
* feat(program-plonky2): port AccountState, Coin, ProofData with field-element layouts
Port the protocol's host-side data types from SHA256/bincode to a
canonical field-element representation that both Rust (off-circuit) and
a future Plonky2 gadget (in-circuit) can compute identically.
- AccountState::hash: 11 field elements [owner(4), balance(2 limbs),
pubkey(5 limbs of 7 bytes each)]. Single Poseidon hash_no_pad call.
- calculate_coin_identifier: [asth(4), coin_index(1)] -> Poseidon.
- ProofData::{to,from}_field_elements: 16-element flat layout, matches
what the circuit will commit as public inputs.
- u64 split into two 32-bit limbs (avoids any Goldilocks-modulus wrap
for high balances).
- 33-byte compressed pubkey packed 7 bytes per field element (canonical
reduction guaranteed by the 56-bit safe ceiling).
MINTING_ADDRESS is a domain-separated placeholder for now (LazyLock).
The server wiring will replace it with the Poseidon hash of the real
minting public key once we know it — tracked as D11 in
MIGRATION_RESEARCH.md.
8 unit tests cover: balance-zero seeding, hash determinism, hash
collision resistance across (balance, pubkey) variations, apply_coin
recipient + overflow rejection, identifier round-trip, ProofData field
round-trip, minting-address stability. All 33 tests in program-plonky2
pass.
* feat(program-plonky2): add MMR inclusion gadget
First Plonky2 circuit gadget. Adds constraints that fail the proof
unless an off-circuit MMRProof verifies against its claimed root.
- swap_if: element-wise conditional swap of two HashOutTargets, used to
handle the index-bit-driven left/right ordering at each path step.
- verify_mmr_inclusion: takes pre-split index bits + path siblings;
hashes up with PoseidonHash::two_to_one at each level; asserts the
final hash equals expected_root via builder.connect_hashes.
- verify_mmr_inclusion_with_index: convenience wrapper that splits
index to bits first (LSB-first, length-equal-to-path).
4 tests build, prove, and verify circuits for: single leaf, two-leaf
tree (both indices), all 36 (n, i) combinations for n in 1..=8, and a
tampered-root negative case that asserts prove() returns an error.
Circuit tests are heavy: ~10 minutes total locally because each builds
a CircuitData under standard_recursion_config and runs a real Plonky2
prove + verify. We'll likely want a #[cfg(feature = "slow-tests")]
gate before adding many more — tracked for the next gadget.
Off-circuit equivalent of this gadget is
crate::merkle::merkle_mountain_range::MMRProof::verify. Both code
paths use PoseidonHash::two_to_one with identical input ordering, so
any proof generated off-circuit by the SP1-equivalent host code (once
ported) will satisfy this gadget without re-derivation.
* docs: add ROADMAP with live status, effort estimates, risk register
Tracks the SP1 → Plonky2 migration on this branch with:
- Status-at-a-glance table: 7 steps done, 9 todo, with per-step
effort estimates (days) and risk callouts.
- Done section with commit refs back to each completed unit.
- Next section: ordered list with files-to-touch, test plans, and
risk notes for steps 4b through 9.
- Pre-mainnet hardening list (D2/D10, D7, D8, paper-derived tests)
with totals — adds 2–3 weeks on top of the MVP.
- Risk register (5 risks, each with mitigation + escalation trigger).
- Update protocol: how to keep the doc honest as commits land.
MVP total estimate: 4–6 weeks full-time. Biggest unknowns are step 5
(Plonky2 recursion vk-pinning correctness) and step 8 (browser
Poseidon performance).
* feat(program-plonky2): add SMT inclusion gadget
Second Plonky2 circuit gadget. Adds constraints that fail the proof
unless an off-circuit SparseMerkleTree::InclusionProof verifies
against its claimed root.
- key_bits_msb_first: decompose a 256-bit key (HashOutTarget) into 256
BoolTargets in the canonical MSB-first ordering matching get_bit on
the big-endian byte serialisation. Uses builder.split_le per element
then reverses, ending up with bit 0 = MSB of element[0].
- verify_smt_inclusion: leaf_hash = Poseidon(leaf || key), then walks
the path in reverse (deepest first) hashing up with conditional
swap_if; asserts connect_hashes(final, expected_root).
- Variable-depth path supported (matches off-circuit path-compression
variable length). Fixed-depth padding for the monolithic circuit
lands later as part of step 5.
- swap_if extracted from circuit/mmr.rs to circuit/util.rs so both
gadgets share the implementation.
4 tests cover: 2-leaf at bit-0 divergence (1 sibling), 2-leaf at bit-7
divergence (8 siblings), 3-leaf tree all queries, and tampered-leaf
negative case. All 41 tests in program-plonky2 pass on
nightly-2025-04-15.
Updates ROADMAP: step 4b done, next is 4c (non-inclusion + insert).
* feat(program-plonky2): add SMT non-inclusion verify gadget
Third Plonky2 circuit gadget. Adds constraints that fail the proof
unless an off-circuit NonInclusionProof verifies against its claimed
root.
The off-circuit verify branches on whether `key == other_key`:
- Case A: empty subtree. other_value must equal DEFAULT_HASHES[depth].
Start hashing from other_value.
- Case B: path-compressed sibling leaf. Start from leaf_hash(other_value,
other_key) and walk up using other_key's bits.
In-circuit the branch is replaced with a witness boolean derived from
element-wise equality, plus:
- A product-equals-zero constraint that enforces the case-A invariant
(is_case_a * (other_value - default) == 0 per element).
- A select between the two possible starting hashes based on the same
boolean.
Navigation uses other_key's bits — in case A this equals key's bits;
in case B both keys share the prefix down to the divergence level, so
the same bits work for the hash-up portion below.
3 tests:
- case A: empty subtree at level 0 (lookup key on the side untouched
by inserts).
- case B: path-compressed neighbour (single-leaf tree, lookup key
diverges deep so chase picks up the stored leaf as sibling).
- case A negative: gelogenes non-default other_value — must not prove.
The insert / new-root computation is deferred to a follow-up (4c+) and
will be wired alongside the monolithic circuit's fixed-depth padding.
All 44 tests in program-plonky2 pass on nightly-2025-04-15.
* feat(program-plonky2): port ProgramInputs and CommitmentMerkleProofs
Ports the higher-level off-circuit data shapes from program/src/lib.rs
to the Plonky2 backend. Lives in a new module `inputs.rs`.
- ProofType: enum InitialProof | AccountUpdateProof. Unchanged.
- CommitmentMerkleProofs: same field set as SP1; HashDigest is now
HashOut<F>, InclusionProof/MMRProof imported from our Poseidon
merkle modules. Methods commitment, verify_commitment_root,
verify_commitment, verify_previous_root ported verbatim.
- ProgramInputs: same shape as SP1 with two changes:
1. Dropped `verification_key` — Plonky2 binds the circuit digest
via add_verifier_data_public_inputs at build time, not as a
witness.
2. `prev_proof_public_values` and `in_coin_proofs_public_values`
are now typed ProofData (instead of SP1's Vec<u8> byte blobs).
The actual recursive proof artifacts are passed to the prover
separately as ProofWithPublicInputs<F, C, D>; they're not part
of this witness struct.
4 tests:
- ProofType variant equality smoke.
- commitment() == H(asth || ocr) matches the off-circuit definition.
- End-to-end: build an SMT + MMR, derive a CommitmentMerkleProofs,
verify_commitment(history_root) returns true.
- ProgramInputs InitialProof branch leaves prev_* as None.
All 48 tests in program-plonky2 pass on nightly-2025-04-15.
Updates ROADMAP: step 4d done, next is 4c+ (smt insert gadget, planned
together with monolithic circuit's fixed-depth padding scheme).
* docs(ROADMAP): correct architecture note — server-side compute, no wasm Poseidon
zkCoins MVP is architecturally server-side compute: the server runs all
ZK proving; the wallet holds only the private key and signs BIP-340
Schnorr over SHA256 of the proof's public digest. There is no in-app
Poseidon, no wasm-Plonky2 verifier, no in-browser ZK gadget.
This corrects the prior estimate for step 8 (was 3–5 days for "wasm
Poseidon", now 1–2 days for "Schnorr-signing boundary + server-API
integration"). Risk R3 (browser-Poseidon performance) is removed
entirely. Risk R2 (1s proof time) is reframed against server hardware
(M3 Ultra / GPU / Succinct Prover Network) rather than laptop.
MVP total revised from 4–6 weeks to 3–5 weeks. Pre-mainnet hardening
estimate (+2–3 weeks) is unchanged.
* docs(ROADMAP): add Plonky3 as post-MVP path; document rejected alternative
Following an external reviewer's suggestion to adopt BabyBear field and
Poseidon2 hash inside the current Plonky2 migration:
1. Adds R6 to the risk register: Plonky2 is bridge tech, not destination.
Plonky3 is the active-development substrate (BabyBear field, Poseidon2
hash, GPU paths).
2. Adds a "Post-MVP Path: Plonky3" section laying out the planned cutover
after step 9: same algorithmic structure, primarily a plumbing rewrite,
estimated 2-4 weeks effort.
3. Documents the rejected alternative ("BabyBear + Poseidon2 in Plonky2
now") with four reasons: fork-land dependencies, custom Poseidon2
implementation, non-trivial migration cost during MVP, and the
Plonky3 cutover later isn't "pure glue code" anyway (gate sets and
recursion ergonomics differ regardless of field/hash choice).
The point of recording the rejected alternative is so a future reviewer
asking "why didn't you just use BabyBear from the start?" gets a written
answer instead of guessing.
* docs: capture session-learnings — CONTRIBUTING + Lessons Learned section
Everything I figured out implementing steps 1–4d that wasn't captured
in commit messages or the existing prose docs. Future contributors
(human or agent) should not have to rediscover this.
Adds:
1. program-plonky2/CONTRIBUTING.md — operational handoff for the new
crate:
- Why standalone-from-workspace (Plonky2 needs nightly).
- First-time toolchain setup (nightly-2025-04-15).
- Build / test / lint commands. Test runtime characteristics per
module (hash <1s, merkle <2s, circuit 5–60s/test).
- Orphan-binary cleanup pattern (the 35 GB swap leak we hit).
- Project layout map.
- Gadget-authoring pattern (mirror off-circuit method, use
connect_hashes, share swap_if from circuit/util).
- Pinning + version philosophy (why plonky2 1.1.0, not 0.2.0).
2. MIGRATION_RESEARCH.md §7 Lessons Learned (8 entries):
- 7.1 Poseidon zero-state collision in SMT defaults (HIGH severity,
fixed + regression-guarded).
- 7.2 Variable vs fixed depth in SMT proofs (MEDIUM, decision still
pending for step 5).
- 7.3 pw.set_target returns Result in plonky2 1.x (LOW).
- 7.4 Field-element packing conventions (7 bytes/elt for byte
inputs, canonical-reduction safety table).
- 7.5 The Schnorr ↔ Poseidon boundary lives at byte serialisation
(off-circuit only, codified for any future privacy work).
- 7.6 Orphan test binaries leak 30+ GB of swap (operational).
- 7.7 gh needs --repo in background tasks (sandbox quirk).
- 7.8 BitVM/zkCoins ≠ normative reference (ShieldedCSV/ShieldedCSV is).
3. ROADMAP.md updated to point at the new CONTRIBUTING + §7 entries
from its "source documents" header. The doc graph now is:
SPEC (what) → MIGRATION_RESEARCH (why + bit us) → CONTRIBUTING (how
to hack) → ROADMAP (when).
* docs(ROADMAP): closed test env — replace SP1, don't migrate
zkCoins runs in a closed test environment (DEV and PRD). No external
users, no real money, no existing user-base. Step 7 therefore replaces
SP1 with Plonky2 outright rather than running a dual backend behind a
Cargo feature flag.
Concrete simplifications:
- Step 7 effort: 3–5 d → 2–3 d. The compatibility plumbing (feature
flag, migration helpers, parallel deploy) is gone.
- Step 7 risk: medium → low. No coexistence surface area.
- R5 (SP1 stays forever) is now structurally mitigated. The risk
itself is downgraded to "for posterity" — there's no realistic path
to ending up on dual backends, because step 7 deletes the SP1 path
in the same PR that adds Plonky2.
- MVP total: 3–5 weeks → 2.5–4 weeks.
Deploy strategy when step 7 lands: stop server, delete state files
(smt.bin, mmr.bin, accounts.bin, latest_block.bin), start new
Plonky2 server with a fresh state. No state migration. No
parallel-run. No rollback path beyond "redeploy the old image and
restore the old state from backup if absolutely needed".
The same rule applies to any other backward-compat question that
comes up: don't migrate, replace.
* docs: consistency review pass — fix stale counts, add glossary, reconcile §6
Systematic audit found several inconsistencies across SPEC,
MIGRATION_RESEARCH, ROADMAP, CONTRIBUTING, and the code. This commit
brings them back in sync.
ROADMAP.md:
- Status-at-a-glance legend added (✅ / 🟡 / ⏳ symbols explained).
- "Done" section: replaced the "(next commit)" placeholder with the
actual commit hash for 2fed8f0; added the four doc-commits that
landed since (401f813, cd94f85, 4cf98ac, 1967087) plus 5c92a62 for
the initial ROADMAP itself.
- Test count corrected from 37 (stale) to 48, with a per-module
breakdown so it's verifiable.
SPEC.md:
- New "Glossary" section after §1 listing every abbreviation used
across the docs (asth, ocr, vk, pk, SMT, MMR, PCD, NIP, IP, D1-D11,
R1-R6, MAX_IN_COINS, TREE_DEPTH, BIP-340, Goldilocks, Poseidon).
- §4.1 TREE_DEPTH note corrected: was "e.g. 254 for a 254-bit field",
now explicitly explains "256 for Poseidon-Goldilocks; pointer to
the concrete constant in program-plonky2/src/merkle/sparse_merkle_tree.rs".
- §12.1 / §12.2 MINTING_ADDRESS reworded: was "currently a hard-coded
[u8; 32]", now reflects the Plonky2 port's LazyLock placeholder and
points at ROADMAP step 7 for the final substitution.
MIGRATION_RESEARCH.md:
- §6 "Recommended Sequencing" replaced with a pointer to ROADMAP plus
a short list of adjustments made since the original 9-step outline
(gadget ordering changed, no Cargo feature flag, scanner+state DO
change). The old 9-step list had drifted from the executed plan and
was misleading.
- §7.1 code snippet replaced with the verbatim block from
sparse_merkle_tree.rs so the audit invariant holds (paste-and-read).
program-plonky2/src/merkle/merkle_mountain_range.rs:
- Added doc-comment to MMRProof struct (had no rustdoc; clippy didn't
warn because it's not lint-required, but human reviewers complained).
program-plonky2/src/types.rs:
- MINTING_ADDRESS comment: pointed at ROADMAP step 7 + SPEC §12.1 +
MIGRATION_RESEARCH §3 D11 instead of a non-existent "TODO.md".
All 48 tests still pass; fmt + clippy clean.
* feat: 100% test coverage on program-plonky2 (MVP = minimal + 100% covered)
Per ROADMAP "Definition of MVP" (added in this PR earlier), MVP means
minimal feature surface AND 100% coverage on the activated surface,
not one or the other. Measured baseline before this commit was 96.43%
lines; this commit closes the gap.
Test count: 48 → 64 (16 new tests).
New positive / negative tests, by area:
- types.rs: CoinTemplate::new / Coin::new constructor coverage.
- inputs.rs: verify_previous_root e2e with a real 2-leaf MMR; asserts
the (older_smt_root, older_proof) pair is recognised as a prefix.
- merkle/merkle_mountain_range.rs: Default::default, leaf_count,
get_leaf (in-range + out-of-range), odd-leaf-count proof with
ZERO_HASH sibling.
- merkle/sparse_merkle_tree.rs: Default::default, idempotent
re-insert, conflicting re-insert errors, generate_non_inclusion
on existing key errors, case-A NonInclusion::verify rejects wrong
default, verify_and_insert rejects invalid proof, insert (without
verify) rejects case-A with wrong default.
- circuit/mmr.rs: should_panic test for index_bits/path length mismatch.
- circuit/smt.rs: should_panic tests for both verify_smt_inclusion and
verify_smt_non_inclusion length-mismatch assertions.
Production-side refactors to remove genuinely-unreachable branches:
- merkle/merkle_mountain_range.rs: append + get_proof now use
`.levels[level].get(idx).copied().unwrap_or(ZERO_HASH)` instead of an
explicit `if idx < len { ... } else { ZERO_HASH }`. The else branch
was unreachable in correctly-maintained state (capacity is a power
of two, so 2*index+1 is always in bounds at the parent level) but
it was generating a perpetually-uncovered region. Collapsing into
`.get()` keeps the safety fallback for future capacity tweaks while
letting coverage hit 100%.
Coverage-measurement infrastructure:
- Cargo.toml: register `coverage_nightly` cfg key in
`[lints.rust]`/`check-cfg` to silence the "unexpected cfg" warning.
- lib.rs: feature-gate the `coverage_attribute` nightly feature behind
`cfg(coverage_nightly)` so non-coverage builds aren't affected.
- All `#[cfg(test)] mod tests { ... }` blocks now also carry
`#[cfg_attr(coverage_nightly, coverage(off))]`. This excludes
test-internal assertion-message-string regions from coverage
measurement — they were the bulk of the remaining "uncovered" lines
in the previous baseline; they're inside production tests that all
execute, but `assert!(cond, "msg")` macros track the msg-evaluation
region separately from the success path.
ROADMAP updated: test count 48 → 64, coverage explicitly recorded as
100% lines / functions / regions with breakdown.
Verified: cargo fmt --check ✓, cargo clippy --all-targets ✓,
cargo llvm-cov --fail-under-lines 100 ✓.
* docs: hardware target — M3 Ultra CPU-only, no GPU, no cloud prover
Explicit architectural constraint: zkCoins runs on a single Mac Studio
M3 Ultra (96 GB unified RAM, Apple Silicon CPU). No discrete GPU. No
external cloud proving service (no Succinct Prover Network, no AWS,
no Lambda Labs). If a design overshoots the performance budget, the
design changes — we do not add hardware.
ROADMAP changes:
- Architecture summary: replaces "M3 Ultra baseline; GPU / Succinct
Prover Network as upgrade paths" (wrong) with the hardened
constraint.
- Risk R2 (1s proof time): mitigation knobs now restricted to
design-level options (reduce MAX_IN_COINS, drop in-coin recursion,
switch to folding). GPU / cloud-prover explicitly marked off the
table.
- Step 9 performance budget: ≤ 5s warm proof / ≤ 30s cold-start /
< 64 GB peak memory on M3 Ultra. If missed → redesign per R2, not
add hardware.
- Plonky3 post-MVP rationale: clarified that BabyBear's GPU-friendliness
is a generic benefit, NOT a benefit for us (we're CPU-only). Motivation
for the eventual switch reduces to "matches SP1 / Plonky3-native".
CONTRIBUTING.md: stripped the "GPU prover" example from the test-
exclusion guidance — there is no GPU path in our architecture.
The constraint is also recorded in auto-memory
(feedback_zkcoins_hardware_target) so it persists across sessions.
* docs: second consistency review pass — close audit findings
Second audit (after the e14d9df + 79bd39e commits) found:
ROADMAP fixes:
- Done section was stale: 2b6f2cb (first consistency review),
e14d9df (100% coverage), 79bd39e (hardware target) were missing.
Added with their hashes and short summaries.
- Test breakdown row mis-counted merkle::smt as 19; actual is 18
(verified by cargo test --list, 18 tests in
merkle::sparse_merkle_tree::tests). Added prelude::1 to the
breakdown so 1+5+18+11+10+5+5+9 = 64 sums correctly.
- Coverage gate command harmonised across docs: ROADMAP now says
`cargo llvm-cov --fail-under-lines 100 -- --test-threads=1` (matches
the CONTRIBUTING.md command exactly), with a note that
--test-threads=1 is for predictable memory peaks on the M3 Ultra
during circuit tests.
MIGRATION_RESEARCH.md §7 additions (3 new Lessons Learned):
- 7.9 Defensive bounds checks → use Option::get().copied().unwrap_or()
pattern instead of explicit if/else. Closes coverage debt without
losing safety; rule of thumb codified for future code.
- 7.10 Coverage-on-tests: how the #[cfg_attr(coverage_nightly,
coverage(off))] annotation works, why it's needed, what crate-level
feature gate and Cargo.toml lints config support it. Future test
modules MUST include the annotation.
- 7.11 Hardware target M3 Ultra single host, CPU only, no GPU,
no cloud prover. Implications for hash choice, Schnorr boundary,
performance budget (linked to ROADMAP step 9), and the Plonky3
post-MVP path (BabyBear's GPU-friendliness no longer a benefit).
Verified after edits: 64/64 tests pass, fmt + clippy clean,
no stale references.
* docs: add CLAUDE.md — onboarding doc for fresh agent / human sessions
The four existing docs (SPEC, MIGRATION_RESEARCH, ROADMAP, CONTRIBUTING)
each cover their slice, but a contributor opening this branch cold has
to piece together the cross-cutting invariants from multiple places.
This commit adds a single canonical entry-point at the repo root.
CLAUDE.md contents (10 sections):
1. What this branch is — one-paragraph framing, why Plonky2 and not
SP1, why this isn't a "rewrite for fun".
2. Reading order — explicit numbered list of which doc to read when,
with the role each plays.
3. Project invariants — 5 non-negotiable rules, each linking to the
doc where it's fully specified:
- 3.1 Server-side compute (wallet holds only privkey)
- 3.2 Closed test env (DEV+PRD, replace-not-migrate)
- 3.3 Hardware: Mac Studio M3 Ultra, CPU-only, no GPU, no cloud
- 3.4 MVP = minimal features + 100% test coverage (both, not either)
- 3.5 Plonky2 is bridge tech, Plonky3 is destination — but not now
4. Decision recipe — 6-step checklist for "should X go in the MVP?"
that encodes the implicit reasoning we've been applying.
5. Pre-push checklist — concrete cargo commands. All five must pass.
6. Branch hygiene rules — no force-pushes, no --no-verify, no squash,
no merge-by-agent, when doc-only commits skip the Done list.
7. Where to put new knowledge — 5-row table mapping knowledge type
to file, so future drift goes to the right place.
8. Current branch state at time of writing — 64 tests, 100% coverage,
step 4d complete, step 4c+/5 next. Pointer to ROADMAP for live.
9. Common foot-guns — 8 condensed pointers into MIGRATION_RESEARCH §7
so a fresh session doesn't fall into known traps.
10. Upstream references.
Cross-doc edits:
- ROADMAP.md: CLAUDE.md added as first source document.
- SPEC.md scope note: "New here? Start with CLAUDE.md" pointer.
- MIGRATION_RESEARCH.md header: same pointer.
- program-plonky2/CONTRIBUTING.md header: same pointer.
This is also captured in auto-memory (the same project invariants are
already there as separate feedback entries), but the in-repo
CLAUDE.md is necessary because:
(a) it's visible to human contributors, not just Claude sessions.
(b) it's git-tracked, reviewable in PRs, and travels with the
codebase.
(c) a fresh Claude session WITHOUT prior memory (different machine,
different agent process) still gets the full context by reading
this one file.
64 tests still pass, fmt + clippy clean.
* docs: drop CLAUDE.md, merge into CONTRIBUTING.md; clarify hardware + step 4c+
Three corrections in one pass:
1. Convention: NEVER CLAUDE.md, ALWAYS CONTRIBUTING.md.
The CLAUDE.md created in the previous commit (f5eaf58) violated the
project-wide convention. Deleted; its content merged into the existing
repo-root CONTRIBUTING.md as a new top section "Working on the Plonky2
Migration" — condensed (saving ~70 lines vs the original CLAUDE.md)
and cross-linked. Existing CONTRIBUTING.md content stays as the
long-standing dev guide for the develop/SP1 branch.
All cross-references updated:
- SPEC.md → CONTRIBUTING.md (was → CLAUDE.md)
- MIGRATION_RESEARCH.md → CONTRIBUTING.md (was → CLAUDE.md)
- ROADMAP.md → CONTRIBUTING.md (was → CLAUDE.md)
- program-plonky2/CONTRIBUTING.md → ../CONTRIBUTING.md (was → ../CLAUDE.md)
The convention is also captured in auto-memory.
2. Hardware constraint corrected: M3 Ultra is NOT "CPU-only".
The Mac Studio M3 Ultra has a substantial integrated GPU (60- or
80-core depending on bin) reachable via Metal. That GPU is on-box and
would be usable IF the prover library supported it. Plonky2 today
ships only CPU and CUDA backends — no Metal — so de facto proving
runs on CPU. That's a library property, not a constraint we imposed.
Corrected wording across:
- ROADMAP architecture summary
- ROADMAP R2 risk register
- ROADMAP Step 9 performance budget
- ROADMAP Plonky3 post-MVP rationale
- MIGRATION_RESEARCH §7.11
- program-plonky2/CONTRIBUTING.md test-exclusion note
- The new CONTRIBUTING.md migration section
What stays correct: no external NVIDIA / CUDA hardware, no external
cloud proving service. Those are the actual decisions; "CPU-only"
was an overstatement that mis-described the box.
Memory entry feedback_zkcoins_hardware_target.md updated.
3. Step 4c+ (SMT insert gadget) is the next step, NOT step 5.
The previous message ended with "Bereit für Step 5" which was wrong
— step 4c+ is explicitly ⏳ todo in the status table. ROADMAP step
4c+ entry now flagged "**NEXT**" and explicitly states the
relationship to step 5: the fixed-depth padding scheme that 4c+
introduces is what step 5 needs, so 4c+ must complete before step 5
can use the SMT insert gadget. They can land in one PR or in
sequence, but the dependency is one-way.
Verified: 64/64 tests pass, fmt + clippy clean.
* feat(program-plonky2): add SMT insert verify gadget
verify_smt_insert mirrors NonInclusionProof::verify_and_insert: re-walks
the non-inclusion proof to bind expected_old_root, then computes the new
root after placing (new_value, key) at the lookup position. Unifies
case A (empty subtree, key == other_key) and case B (path-compressed
neighbour, key != other_key) via the same is_case_a selector used in
verify_smt_non_inclusion; case-B extension siblings are witnessed by the
caller to match the off-circuit insert padding loop.
7 tests covering both cases (incl. deep divergence with non-empty
extension), tampered new_value / expected_new_root / case-A invariant,
and the two build-time bit-length assertions.
* docs(ROADMAP): mark step 4c+ done, refresh test count, promote step 5 to NEXT
Closes a stale-roadmap gap left by 6cf949c (SMT insert gadget). The
ROADMAP Update Protocol requires "if the commit completes a step → flip
its row to ✅ and move its entry under Done"; 6cf949c added the gadget
+ 8 tests but didn't touch this file. Three corrections in one pass:
1. Status table row 4c+: ⏳ todo → ✅ done.
2. Done section: new entry for 6cf949c at the top of the
newest-first list (3 positive incl. deep-divergence Case B, 3
negative incl. case-A invariant, 2 build-time assertion panics).
3. Test count: 64 → 72; circuit::smt module 9 → 17.
4. Next-in-order: dropped the now-completed Step 4c+ block; Step 5
(monolithic state-transition circuit) is flagged **NEXT**.
Verified: cargo test -- --list reports 72 tests with the breakdown
above (5+17+5+5+11+18+1+10 = 72). No code changes, fmt + clippy
unaffected.
* docs: add Step 4 critical review (read-only, separate file)
Independent review of Step 4 (4a MMR-inclusion, 4b SMT-inclusion,
4c SMT-non-inclusion-verify, 4c+ SMT-insert-verify, 4d ProgramInputs)
performed alongside the parallel Step 5 session.
Standalone file to avoid touching ROADMAP / MIGRATION_RESEARCH / SPEC /
circuit/smt.rs while the other session works on those.
Strict distinction between:
- BUGS / MUST FIX NOW: zero items. Algorithmic correctness verified,
off-circuit ↔ in-circuit consistency holds, 72/72 tests pass at
100% line/function/region coverage.
- NICE TO HAVE: six items (N1–N6), none block Step 5:
- N1 bit-255 divergence build-time assertion (cosmetic edge case)
- N2 case_b_extension helper is test-only (Step 7 will surface)
- N3 old-root walk uses other_key_bits — correct but needs comment
- N4 verify_smt_insert is constraint-heavy (Step 5 throughput watch)
- N5 verify_smt_insert name slightly ambiguous (rename = churn)
- N6 ProgramInputs declared but unused in circuit yet (Step 5)
When Step 5 merges, this file's findings should be folded into
MIGRATION_RESEARCH.md §7 and the file deleted.
* docs: add Step 7 cutover inventory (read-only, separate file)
Inventory of every place in the existing SP1-era server code that
must change for Step 7 (replace SP1 with Plonky2, no Cargo feature
flag, no dual backend). Produced alongside the parallel Step 5 work
to avoid editing files Step 5 is also touching.
Strict classification per item:
- 🔧 mechanical: import swap / rename, no design decision
- 🧩 layout-dependent: depends on Step 5's ProofData layout choice
- 🛠 new work: persistence helpers don't exist yet in program-plonky2
- ⚙ decision: 3 open design calls (MMR leaf hash SHA256→Poseidon,
script/ crate fate, workspace toolchain unification)
Key findings:
- Server code touchpoints are surprisingly small: ~5 import-swap lines
plus 4 ProofData deserialisation sites that wait for Step 5's
ProofData::from_proof public API.
- Only real engineering item: serde + save_to_file/load_from_file
for the new Poseidon SMT and MMR (~3–4 hours).
- Realistic effort: 1–1.5 days full-time (vs ROADMAP's 2–3 days).
- State-file cleanup is a deploy runbook step, not code: smt.bin /
mmr.bin / mmr.bin.prev_root / latest_block.bin must be deleted.
Three open design decisions documented with recommendations:
- MMR leaf hash: switch to Poseidon (consistency)
- script/ crate: delete entirely
- workspace toolchain: move everything to nightly once SP1 is gone
Once Step 5 + Step 6 land, this file's content should be folded into
the actual Step 7 PR and the file deleted.
* docs(CONTRIBUTING): refresh stale test count 64 → 72
Step 4c+ (commit 6cf949c) added 8 tests for verify_smt_insert; ROADMAP
was updated to 72, but the project-invariants section in CONTRIBUTING
still claimed 64. Caught by the latest audit pass.
Also adds a pointer to ROADMAP "Done" for the live count, so future
drift only has to be fixed in one place.
* feat(program-plonky2): stage 5a — cyclic recursion plumbing PoC
Per ROADMAP R1 mitigation ("start step 5 with the simplest possible 'I
verify myself with a trivial payload' circuit"), wires the cyclic-
recursion machinery in isolation before any real state-transition
predicate goes in.
What lands here:
- New circuit/main.rs (300 LOC + 2 tests) wrapping
`conditionally_verify_cyclic_proof_or_dummy` against a one-public-
input circuit whose payload is `counter = if condition { inner.counter
+ 1 } else { 0 }`.
- `common_data_for_recursion_c` is a faithful port of Plonky2 1.1.0's
internal helper (one verify_proof per pass + NoopGate padding to
2^12), not the BitVM 0.2.0 variant — that shape fails to build under
1.1.0 with "Failed to build circuit".
- `add_verifier_data_public_inputs` pins the verifier-key digest as
public input; `check_cyclic_proof_verifier_data` cross-checks each
proof's embedded digest against the circuit's own.
Tests (5–60 s each on M3 Ultra, --test-threads=1):
- stage_5a_base_proof_round_trip — condition=false → counter=0,
cyclic_base_proof dummy in the inner slot, verify passes.
- stage_5a_recursive_proof_round_trip — base, then one cycle with the
base as the inner proof, counter advances 0 → 1, verify passes.
Critical: this is the R1 evidence that circuit_digest is stable
across builds in our 1.1.0 setup.
Coverage: 100 % lines on the activated surface
(`cargo llvm-cov --fail-under-lines 100 -- --test-threads=1` passes
the MVP gate; 1 region under 100 % is the `.expect` panic branch of
`conditionally_verify_cyclic_proof_or_dummy`'s library-side
`Result<()>` — unreachable when `common_data` is well-formed by
construction, same kind of defensive code §7.9 covers).
Subsequent stages (5b–5d, tracked in ROADMAP) replace the counter
payload with the real predicate while keeping this recursion skeleton.
* docs(ROADMAP): mark step 5 in progress, capture stage 5a completion
Step 5 row in *Status at a Glance* flips from ⏳ todo to 🟡 in progress
(per Update Protocol, partial completion); commit 83fa0c1 lands stage
5a — the cyclic recursion plumbing PoC — but stages 5b–5e of step 5
are still open.
Changes:
- Row 5 in the status table: ⏳ todo → 🟡 in progress.
- *In Progress* section: populated with the five-stage breakdown of
step 5 (5a ✅ done, 5b–5e ⏳), so future sessions opening this file
see exactly which substage is the next handle.
- *Done* section: 83fa0c1 added at the top of the newest-first list.
- Test count: 72 → 74; breakdown adds `circuit::main` 2.
- Step 5 heading in *Next (in order)*: **NEXT** → 🟡 **in progress
(see *In Progress* above)** so the spec stays canonical there
without misleading future readers.
* docs(MIGRATION_RESEARCH): add §7.12 + §7.13 lessons from stage 5a
Two new entries in the Lessons Learned section, codifying gotchas
discovered while building the cyclic-recursion PoC in commit 83fa0c1.
The lessons are also in main.rs's docstring comments, but §7 is the
canonical place future sessions look for "what bit us":
§7.12 — BitVM's `common_data_for_recursion` is broken under
Plonky2 1.1.0. BitVM is pinned to 0.2.0; its 2-and-3-verify-call
+ ConstantGate shape is no longer a fixed point of 1.1.0's
`conditionally_verify_cyclic_proof_or_dummy`. The fix is to port
Plonky2 1.1.0's own canonical helper (one verify_proof per pass +
NoopGate padding to 2^12). Also documents the ordering subtlety
(canonical Plonky2 order vs. BitVM's order).
§7.13 — Coverage debt from unreachable Plonky2 `Result<()>` calls.
Pattern: Plonky2's `set_target`, `set_proof_with_pis_target`,
`set_verifier_data_target`,
`conditionally_verify_cyclic_proof_or_dummy` all return
`Result<…>` even though Err is impossible under correct usage. The
fix-recipe parallels §7.9 (Option-based defensive checks): make
the function infallible by `.expect`-ing the Err with an
invariant message, drop `Result<…>` from the signature. Notes
that this is *not* a fallback per `feedback_no_fallbacks` — `.expect`
panics rather than silently recovering, which is documentation,
not silent failure.
Both lessons reference the existing §7.9 pattern.
* feat(program-plonky2): stage 5b — Initial-branch state-transition predicate
Replace the stage-5a counter payload with the real Initial-proof
predicate from SPEC §8:
- Public input: 16-element ProofData (account_state_hash,
output_coins_root, commitment_history_root, coin_history_root).
- In-circuit Poseidon AccountState::hash (owner 4 + balance limbs 2 +
pubkey limbs 5 = 11 elements), matching off-circuit layout. Balance
range-checked to 32 bits per half, pubkey to 56 bits per limb.
- Mint exception: is_minting = AND over element-wise owner ==
MINTING_ADDRESS, enforced via (1 - is_minting) * balance_limb == 0
for both halves. Only MINTING_ADDRESS may carry a non-zero starting
balance.
- output_coins_root and coin_history_root constants from
DEFAULT_HASHES[0] (empty SMT root). commitment_history_root is a
pure witness at this stage; SPEC §8's AccountUpdate branch binds it
to a real MMR in stage 5c.
- Cyclic recursion machinery (conditionally_verify_cyclic_proof_or_dummy,
add_verifier_data_public_inputs, three-pass common_data) carried
over from 5a. condition is constrained to false so the inner-proof
slot is always a dummy — stage 5c lifts that constraint.
Three tests in circuit::main: mint exception accepted, non-mint
zero-balance accepted, non-mint nonzero-balance rejected. Full suite
75/75, 100% lines coverage on program-plonky2/.
* feat(program-plonky2): stage 5c — AccountUpdate branch
Lift the stage-5b condition pinning and wire the AccountUpdate-proof
predicate per SPEC §8:
- `condition` is now a free witness `BoolTarget`. Caller passes
`true` to verify a real prev proof recursively, `false` for the
Initial base case (dummy inner).
- SPEC §8 (a) — same-circuit binding — is enforced by
`conditionally_verify_cyclic_proof_or_dummy::<C>` via the
add_verifier_data_public_inputs / circuit_digest fixpoint.
- SPEC §8 (b) — state continuity — enforced as
`condition * (account_state_hash[i] - prev.account_state_hash[i])
== 0` for each of the 4 hash elements. The inner proof's
public_inputs[0..4] is the prev's account_state_hash slot.
- `coin_history_root` carry-over: output's slot is
`select(condition, prev.coin_history_root, DEFAULT_HASHES[0])`.
- Mint exception masked with `(1 - condition) * (1 - is_minting)`
so it only applies to Initial.
SPEC §8 (c)(d)(e) — `CommitmentMerkleProofs` proving the prev was
published in the global history MMR — is NOT YET WIRED. The deferred
gap is documented in `circuit/main.rs` and in the ROADMAP. Stage 5c+
closes it with in-circuit SMT + MMR inclusion verification of the
`CommitmentMerkleProofs` shape.
Tests in `circuit::main`: 5 (3 from 5b re-asserted under the free
`condition`, 1 Initial→AccountUpdate chain that recursively verifies
end-to-end, 1 negative AccountUpdate where current.account_state.hash()
!= prev.account_state_hash → rejected by the continuity constraint).
Full suite 77/77, 100% lines coverage on program-plonky2/.
* refactor(program-plonky2): SMT to uncompressed fixed-256-depth paths
Required for Plonky2 cyclic recursion: the verifier circuit shape
must be stable across proof builds (constant `circuit_digest`), so
variable-length proof paths can't be consumed in-circuit. Switching
to uncompressed paths makes every proof exactly `TREE_DEPTH = 256`
siblings — empty subtrees contribute `DEFAULT_HASHES[level + 1]` —
and the in-circuit gadget always hashes through 256 levels.
Off-circuit (`merkle/sparse_merkle_tree.rs`):
- `InclusionProof::siblings` and `NonInclusionProof::siblings` now
always have length `TREE_DEPTH`.
- `NonInclusionProof` drops the `leaf: ([u8; 32], HashDigest)` field —
case A/B distinction is no longer meaningful; non-inclusion is a
proof that the depth-256 slot at the key holds
`DEFAULT_HASHES[TREE_DEPTH]`.
- `SparseMerkleTree::insert` removes the `current_hash != leaf_h ||
sibling != default → skip hash` short-circuit; every level now
hashes unconditionally. New `sibling_at` and
`collect_path_siblings` helpers factor out the shared
siblings-along-a-branch logic between insert / inclusion-proof /
non-inclusion-proof generation.
- All public roots produced by the new `insert` differ from the
pre-refactor compressed roots, but the closed test env makes this
a free choice.
In-circuit (`circuit/smt.rs`):
- `verify_smt_inclusion`, `verify_smt_non_inclusion`,
`verify_smt_insert` now require `path.len() == TREE_DEPTH` and
`key_bits.len() >= TREE_DEPTH`. The single `hash_up_full_path`
engine carries every gadget — `start = leaf_hash(value, key)` for
inclusion, `start = empty_leaf_default` for non-inclusion,
`verify_smt_insert` runs both walks.
- All case A/B logic, the `extension` slice, and the `default_at_path_depth`
parameter (now `empty_leaf_default`) are gone.
Test count 77 → 73 (case A/B-specific tests removed; replaced with
cleaner positive + negative paths). All gates green: cargo fmt,
cargo clippy -D warnings, cargo test (73/73), llvm-cov
--fail-under-lines 100 (100% lines, 1 region unreachable Result-on-
`.expect()` in stage-5c main.rs as documented in
MIGRATION_RESEARCH §7.13).
* feat(program-plonky2): stage 5c+ — CommitmentMerkleProofs in-circuit
Wire SPEC §8 (c)(d)(e) — the history-continuity predicate — into the
state-transition circuit using fixed-shape SMT + MMR inclusion proofs:
- (c) `account_state.hash() == mp.commitment_account_state_hash` via
element-wise difference masked with `condition`.
- (d) `mp.verify_commitment(history_root)` decomposes into in-circuit
SMT inclusion (depth `TREE_DEPTH = 256`) of `commitment = h(asth ||
ocr)` in `commitment_root`, plus MMR inclusion (depth
`MMR_PROOF_PATH_LEN = MMR_MAX_DEPTH - 1 = 31`) of `h(commitment_root
|| commitment_root_mmr_sibling)` in `history_root`.
- (e) `mp.verify_previous_root(prev.commitment_history_root,
history_root)` is a second MMR inclusion of
`h(previous_root_history_proof.0 || prev.commitment_history_root)`
in `history_root`. `prev.commitment_history_root` is read from the
inner proof's `public_inputs[8..12]`.
Masking pattern: every `connect_hashes(computed, expected)` becomes
`connect_hashes(computed, select_hash(condition, expected_witness,
computed))`. When `condition = false` the select collapses to
`computed` and the check is trivially satisfied; when `true` the
honest check fires.
Plumbing:
- new `MMR_MAX_DEPTH = 32` const in `merkle::merkle_mountain_range`.
- new `MerkleMountainRange::root_extended(target_path_len)` + 3 tests.
- new `MMRProof::extend_to(target_path_len)` + tests.
- `circuit::smt::smt_inclusion_root` exposed alongside
`verify_smt_inclusion` so callers can build masked targets.
- `circuit::mmr::mmr_inclusion_root` similarly exposed.
- `dummy_cmp()` builds a syntactically valid but semantically empty
`CommitmentMerkleProofs` for the masked-off Initial-branch path.
Tests in `circuit::main`: 7 total (3 unchanged from 5c on the
Initial side, 1 full bootstrap Init→Update chain with real
CommitmentMerkleProofs verification, 3 negatives covering (b) state
discontinuity, (c) lying `commitment_account_state_hash`, and (d)
tampered SMT path).
Lessons captured in MIGRATION_RESEARCH.md §7.14 (uncompressed-SMT
rationale), §7.15 (select_hash masking pattern), §7.16 (MMR
root_extended / extend_to). Full suite 78/78. Coverage gate runs
separately due to long execution time.
* test(program-plonky2): cover assert_eq panic messages in set_cmp_witness
cargo llvm-cov flagged 3 line-coverage misses in set_cmp_witness (the
panic-message strings of the three length-guard `assert_eq!` calls).
Add 3 `#[should_panic(expected = ...)]` tests that truncate the
fixed-shape proof paths and verify each panic message fires.
Each test builds the full circuit (~30s) before triggering the panic;
combined runtime ~130s. With these in place, llvm-cov reports 100%
lines on program-plonky2 again.
* feat(program-plonky2): stage 5d (minimal) + stage 5e (partial negatives)
## Stage 5d — in-coin slots, minimum viable structure
Add fixed-shape padding for the per-input-coin loop. `MAX_IN_COINS = 1`
for this revision (production target is 8 per SPEC §13; bumping is
mechanical — see the `MAX_IN_COINS` docstring). The circuit reserves
`MAX_IN_COINS` slot witnesses; active slots prove SMT non-inclusion of
`coin_identifier` at the running `coin_history_root` and compute the
new root after inserting `coin_identifier` (used both as key and as
leaf value, making `coin_history` a set-membership SMT per SPEC §8).
Inactive slots are masked no-ops via the `active`-bit `select_hash`
pattern (MIGRATION_RESEARCH §7.17).
New witness targets:
- `InCoinSlotTargets { active: BoolTarget, coin_identifier: HashOutTarget,
nip_path: Vec<HashOutTarget> }` per slot.
- `StateTransitionCircuit.in_coin_slots: Vec<InCoinSlotTargets>`.
New proving API:
- `prove_initial_with_in_coins(circuit, account_state, history_root, &in_coins)`
- `prove_account_update_with_in_coins(circuit, ..., &in_coins)`
- `prove_initial` / `prove_account_update` keep their existing
signatures and delegate to the `_with_in_coins` variants using
`dummy_non_inclusion_proof()` placeholders for every slot.
Tests added in `circuit::main`:
- `stage_5d_initial_with_one_active_in_coin` — positive, 1 active
slot inserting into empty `coin_history` SMT.
- `stage_5d_initial_with_tampered_nip_path_rejected` — tampered
sibling rejected at `connect_hashes(computed_old, running)`.
- 3 panic-message coverage tests (`set_in_coin_slot_witness` length
guard + slot-count guards for both `_with_in_coins` wrappers).
## Stage 5e — partial SPEC §13 negative tests against the current circuit
Four new negatives covering the (d) and (e) MMR proof shapes that 5c+
wired but didn't exhaustively negative-test:
- `stage_5e_account_update_tampered_mmr_a_path_rejected` — invalid
proof that `commitment_root` sits in `history_root`.
- `stage_5e_account_update_tampered_mmr_b_path_rejected` — invalid
proof that prev's committed history is a prefix of `history_root`.
- `stage_5e_account_update_wrong_mmr_sibling_rejected` — wrong
`commitment_root_mmr_sibling` so MMR-(d) leaf hashes wrong.
- `stage_5e_account_update_wrong_history_root_rejected` — lying
`history_root` that neither MMR proof reconstructs to.
The remaining SPEC §13 invariants (double-spend, identifier mismatch,
amount overflow / underflow, wrong vk on a recursive proof) require
the deferred 5d+ machinery (recursive verification of source proofs,
`apply_coin` semantics, out-coins processing); they land with that.
## Other
- `circuit::smt::hash_up_full_path` exported `pub` so `circuit::main`
can build masked SMT walks against arbitrary `start` values.
- MIGRATION_RESEARCH §7.17 codifies the per-slot `active`-bit masking
pattern.
- ROADMAP test count 78 → 90; coverage gate runs separately.
* feat(program-plonky2): stage 5d-next — apply_coin semantics
Add the per-coin SPEC §8 `apply_coin` predicate on top of the
stage-5d minimal coin-history side:
Per-slot witnesses extended:
- `coin_recipient: HashOutTarget` — address the coin claims to be
addressed to.
- `coin_amount_lo: Target`, `coin_amount_hi: Target` — 32-bit halves
of the coin's amount, range-checked.
Per-slot constraints (masked by `active`):
- **Recipient check**: `active * (coin_recipient[i] - owner[i]) == 0`
for each of the four hash elements. Only the owning account may
absorb a coin.
- **Balance addition with overflow check**: `sum_lo = balance_lo +
active * coin_amount_lo`; `split_le(sum_lo, 33)` produces 33 bits
via Plonky2's auto-witnessed `BaseSumGate`; `carry = bits[32]`;
`new_lo = sum_lo - 2^32 * carry`. Same for hi: `sum_hi =
balance_hi + active * coin_amount_hi + carry`, `overflow =
bits[32]`, `new_hi = sum_hi - 2^32 * overflow`. Assert
`overflow == 0` — no top-level u64 overflow.
Running balance is threaded through the `MAX_IN_COINS` slots; the
final balance feeds a second `Poseidon(owner || final_balance_lo ||
final_balance_hi || pubkey_limbs)` for the public
`ProofData.account_state_hash`. The earlier `account_state_hash`
(from the INITIAL balance) keeps serving SPEC §8 (b) state-continuity
and (c) commitment-witness checks at the start of the transition.
API updates:
- `(bool, HashDigest, &NonInclusionProof)` slot tuple replaced by
`(bool, &Coin, &NonInclusionProof)`. The full `Coin` carries
identifier + recipient + amount.
- New `dummy_coin()` helper for inactive-slot placeholders.
- `set_in_coin_slot_witness` signature extended to take the
recipient and amount alongside the identifier and nip.
Tests added in `circuit::main`:
- `stage_5d_initial_with_one_active_in_coin` — refactored to feed a
full `Coin { recipient = owner, amount = 42 }`; output
`account_state_hash` must equal `final_account_state.hash()` (with
`balance += 42`).
- `stage_5d_initial_in_coin_wrong_recipient_rejected` — recipient
!= owner triggers the masked equality check.
- `stage_5d_initial_in_coin_overflow_rejected` — initial balance
`u64::MAX` + coin.amount `1` triggers the `assert_zero(overflow)`.
Lessons codified in MIGRATION_RESEARCH §7.18: virtual targets need
explicit witnesses; prefer `split_le` for value-determined targets
to leverage Plonky2's `BaseSumGate` auto-witness generator.
* feat(program-plonky2): stage 5d-next-2 — bump MAX_IN_COINS to 8
Move the in-coin slot count from the minimum-viable `1` to SPEC §13's
production target `8`. Required circuit-shape changes:
- `MAX_IN_COINS = 8` const (docstring updated).
- `common_data_for_recursion_c` padding bumped to `INNER_PAD_BITS = 13`
(`1 << 13 = 8192` gates) so the inner circuit's `degree_bits = 13`
matches the outer's. The factored-out const + docstring make the
next bump (e.g., when out-coins land) a one-line change.
Test wiring:
- New `slots_first_active(&coin, &nip, &dummy_coin, &dummy_nip)`
helper inside `circuit::main::tests` builds a `MAX_IN_COINS`-length
slot array with the first slot active and the remaining 7 inactive
(`dummy_coin` / `dummy_non_inclusion_proof` placeholders).
- All 4 `*_with_in_coins`-based tests refactored to use the helper.
- Build + prove confirmed for `stage_5d_initial_with_one_active_in_coin`
(188s wall on the test host).
cargo fmt + cargo clippy -D warnings clean.
* feat(program-plonky2): stage 5d-next-3 — out-coins processing
Add SPEC §8 step 3 (the send_coins side of the state transition).
Per `MAX_OUT_COINS = 1` slot the circuit witnesses an active bit, an
out-coin identifier, amount limbs and a 256-sibling non-inclusion
path; per slot the in-circuit predicate enforces (masked by active):
- SMT non-inclusion + insert into the running `output_coins_root`
(the new SMT — empty at the start of the loop), mirroring the
in-coins coin_history pattern.
- Balance subtraction with **underflow check** via
`split_le(diff, 64)`: balance_u64 - active * amount_u64 must fit
in 64 bits (in the Goldilocks field, an underflowed difference
wraps to a value far larger than 2^64 and the bit-decomposition
fails).
- `out_coin_identifier == Poseidon(interim_account_state_hash ||
u32(slot_index))`, where `interim_account_state_hash` is computed
from `owner + post-subtraction balance + INITIAL pubkey`. Mirrors
off-circuit `crate::types::calculate_coin_identifier`.
Pubkey rotation: a new `next_public_key_limbs` witness target.
After the out-coin loop, the FINAL `account_state_hash` (which goes
into the public `ProofData`) uses the NEW pubkey; the interim hash
(used for identifier derivation) uses the INITIAL pubkey, matching
SPEC §8's ordering: the off-circuit account_hash for identifiers is
computed *before* the rotation.
API:
- New `prove_initial_with_in_and_out_coins(...,
out_coins, next_public_key)` and
`prove_account_update_with_in_and_out_coins(...)`.
- Existing `prove_initial` / `prove_account_update` delegate to the
new variants with all-inactive out-coin slots and
`next_public_key = account_state.public_key` (no rotation), so
every prior test path continues to behave identically.
Tests added in `circuit::main`:
- `stage_5d_next_3_initial_with_one_active_out_coin` — positive,
one out-coin emitted at index 0 with amount 30, balance
100 → 70, pubkey rotated to `dummy_pubkey(122)`; output
`ProofData.account_state_hash` matches off-circuit
`final_account_state.hash()`, `output_coins_root` matches
off-circuit `nip.insert(expected_out_id)`.
- `stage_5d_next_3_initial_out_coin_wrong_identifier_rejected` —
identifier ≠ H(interim_asth || 0) triggers the masked equality.
- `stage_5d_next_3_initial_out_coin_underflow_rejected` —
amount > balance triggers `split_le(diff, 64)`.
- Two panic guards (`OutCoinSlot` nip-length, out-slot count).
* docs(program-plonky2): stage 5d-next-4 design doc for source verification
Standalone planning document for the deferred source-side in-coin
verification work. Captures:
- The remaining SPEC §8 in-coin predicate (recursive verify, SMT
inclusion in source.output_coins_root, source CommitmentMerkleProofs).
- Three options for the multi-inner-proof challenge (parallel
cyclic-verify, recursive aggregator, sequential chain) and a
recommendation (Option A for the MVP if N=9 stays).
- Sketch of the `common_data_for_recursion_c` extension to N verify
passes.
- Test-budget realism: 25+ cyclic tests at INNER_PAD_BITS=17 would
exceed 20 hours; mitigation via a `OnceLock`-cached circuit.
- The 3 SPEC §13 negatives this stage unblocks.
Read-only; no source-code changes. Will be folded into the actual
5d-next-4 PR once that work lands and then deleted.
* feat(program-plonky2): stage 5d-next-3-bump — MAX_OUT_COINS to 8
Bump the out-coin slot count from the minimum-viable 1 to SPEC §13's
production target 8. Required circuit-shape changes:
- `MAX_OUT_COINS = 8` const (docstring updated to drop the
"minimum viable" framing).
- `INNER_PAD_BITS` bumped 13 → 14 (1 << 14 = 16384 gates) to
accommodate the larger outer circuit. Outer size at full
production parameters: 8 in-coin slots × 512 hashes (≈ 4 k) + 8
out-coin slots × 512 hashes (≈ 4 k) + 5c+ commitment proofs
(≈ 0.3 k) + apply_coin / identifier / 3 account-state hashes
(≈ 0.2 k) ≈ 8-10 k gates. INNER_PAD_BITS = 14 gives generous
headroom; bumping further is a one-line change.
`MAX_OUT_COINS` and `MAX_IN_COINS` now mirror each other at 8,
matching SPEC §13.
The existing `out_slots_first_active` test helper automatically
returns a `MAX_OUT_COINS`-length array (active in slot 0, inactive
in slots 1..=7), so no test changes were needed for the bump.
cargo fmt + cargo clippy -D warnings clean.
* test(program-plonky2): combined in-and-out integration test
Add `stage_5d_next_3_initial_combined_in_and_out_coin` — one Initial
proof that exercises BOTH the in-coins loop AND the out-coins loop
in a single transition, validating that the full SPEC §8 flow
composes correctly:
- Mint account with initial balance 100.
- 1 active in-coin (id `i1`, amount 30, recipient = owner) →
running balance 130, coin_history advances.
- 1 active out-coin (id derived from the *interim* account-state
hash with balance 80 and INITIAL pubkey, amount 50, sent to a
rotated pubkey) → running balance 80, output_coins_root advances.
- Final `ProofData.account_state_hash` matches the FINAL state
(rotated pubkey + balance 80), `coin_history_root` is the
post-insert root of the in-coin, `output_coins_root` is the
post-insert root of the out-coin.
This is the first test that exercises both running-balance
mutations (add via in-coin, sub via out-coin) plus the interim /
final account-state-hash distinction in the same prove call.
Previous positive tests only exercised one loop at a time.
Confirmed at MAX_IN_COINS=8 + MAX_OUT_COINS=8 + INNER_PAD_BITS=14
(commit `56f3a05`); test budget ~5-10 min wall per cyclic test.
* docs(ROADMAP): refresh commit list + test count after MAX_OUT_COINS=8 bump
Test count 99 (combined-in-and-out test added). Entries for the
five 5d-* commits + 5d-next-4 design doc + combined test are now
properly linked.
* test(program-plonky2): cover assert_eq panics on the *_in_and_out_coins wrappers
The stage-5d-next-3 `prove_initial_with_in_and_out_coins` and
`prove_account_update_with_in_and_out_coins` wrappers each carry two
slot-count guards (in-coin and out-coin). The existing panic tests
only covered:
- `set_out_coin_slot_witness` short nip path
- `prove_initial_with_in_and_out_coins` wrong OUT-coin count
Add the three missing guards so `cargo llvm-cov --fail-under-lines
100` stays green after the bump:
- `stage_5d_next_3_prove_initial_panics_on_wrong_in_slot_count`
- `stage_5d_next_3_prove_account_update_panics_on_wrong_in_slot_count`
- `stage_5d_next_3_prove_account_update_panics_on_wrong_out_slot_count`
Each just `build_circuit()`s and immediately panics with the
expected `assert_eq!` message; tests are `#[should_panic(expected =
...)]`. Total runtime ~90 s for the three new tests.
Also refresh `program-plonky2/CONTRIBUTING.md` test runtime
characteristics table to reflect production parameters:
- `MAX_IN_COINS = MAX_OUT_COINS = 8`, `INNER_PAD_BITS = 14`.
- Cyclic positive tests now 3-15 min; full sweep is hours.
- Recommends `cargo test <test_name>` for iteration.
Test count 99 → 102.
* docs(SPEC): note MAX_OUT_COINS in the constants table
SPEC §2's constants table listed MAX_IN_COINS = 8 but not its
mirror MAX_OUT_COINS, which the stage-5d-next-3-bump commit
(56f3a05) just lifted to the same value. Add the row for parity.
* test(program-plonky2): combined in-and-out integration test on AccountUpdate
Mirror of stage_5d_next_3_initial_combined_in_and_out_coin but on
the AccountUpdate path: builds an Init proof first (mint, balance
100, no coins), then proves an Update against the post-init history
that consumes 1 active in-coin (+30) AND emits 1 active out-coin
(-50) at index 0 with pubkey rotation.
Exercises in one prove: cyclic-recursion (`condition = true`),
SPEC §8 (b) state continuity, (c)(d)(e) CommitmentMerkleProofs
chain, apply_coin (recipient + balance + overflow), send_coins
(non-inclusion + insert + underflow + identifier derivation), pubkey
rotation. The final ProofData.account_state_hash matches the
off-circuit `final_account_state.hash()` with balance 80 and the
rotated pubkey; coin_history_root matches `nip.insert(in_coin_id)`;
output_coins_root matches `out_nip.insert(expected_out_id)`.
Test count 102 → 103. Run time on the test host: ~10-15 min
(two cyclic proofs).
* test(program-plonky2): speed up account_update panic-tests via cyclic_base_proof
The two stage_5d_next_3_prove_account_update_panics_on_wrong_*_slot_count
tests originally called `prove_initial(...)` to construct a real
prev proof before triggering the slot-count assert — paying ~13 min
per test at MAX_IN_COINS = MAX_OUT_COINS = 8.
The slot-count `assert_eq!` fires at the top of
`prove_account_update_with_in_and_out_coins`, before any witness
setting or proving, so `prev` is never consumed. Replace the
real-prove call with a `cyclic_base_proof` dummy of the same
type — same panic semantics, ~30 s per test instead of ~13 min.
Net runtime savings: ~25 min wall on every full test sweep.
* docs(program-plonky2): session-state pickup notes for next agent
Single-page summary at the end of the long Stage-5 implementation
session: what works, what's deferred (5d-next-4 source verification),
test budget realism, per-stage commit map, files most likely to be
touched next. Read first if you're picking up where Stage 5 left
off.
* docs: finalise session pickup — §7.20 + test-confirmation + verification checklist
Two doc updates to make the pickup state fully self-con…
…ges (#35) * ci: rename program crate ref + skip server-tests when no Rust changed ci.yaml still ran clippy on `zkcoins-program`, which the Plonky2 migration deleted and renamed to `zkcoins-program-plonky2`. Update the crate refs (plus add the prover crate, matching the pre-push hook). Caught by the develop CI run after the #17 merge. While here, extend the pre-push hook with the same scoping logic the circuit-sweep already uses: skip the server+shared test suite and the coverage gate when no Rust or Cargo file changed vs origin/<branch>. A 2-line YAML fix shouldn't pay a 100-min test rerun for a code path it can't touch. Document the new walls in CONTRIBUTING.md. * fix(pre-push): coerce no-match grep to exit 0 under set -e The grep-based RUST_CHANGED/CIRCUIT_CHANGED counters in the new conditional hook exited the whole script when no Rust/Cargo file (or no program-plonky2/ file) was in the diff — `set -e` killed the hook on the first non-matching grep, silently, before any echo line ran. Wrap each grep in `{ ... || true; }` and count via `grep -c '.' || true` with a ${VAR:-0} fallback so an empty file list resolves to 0 rather than aborting the hook. * docs(pre-push, ci): align wall budgets across hook header, ci.yaml, CONTRIBUTING Three callouts of the warm-cache wall budget had drifted: - .githooks/pre-push header said ~70 min for a Rust change. - .github/workflows/ci.yaml said the full suite is ~8 min. - CONTRIBUTING.md § Setup said ~100 min. The observed run on this branch was 67 min server tests + ~30 min coverage gate ≈ 100 min, matching CONTRIBUTING.md. Bring hook header and ci.yaml into line with that, and cross-link them to CONTRIBUTING as the single source of truth so the next drift is loud. Drive-by cleanup: collapse the duplicated \`if [ -n \"\$REF_BASE\" ]\` blocks in the hook into one — both guarded the same path.
* fix(faucet): force minting ClientAccount address to MINTING_ADDRESS The Plonky2 migration (D11 in MIGRATION_RESEARCH.md) moved MINTING_ADDRESS from a privkey-derived value to a well-known constant `hash_bytes(b"zkcoins: minting-address:placeholder:v1")`. ClientAccount::new still derives `address` from the privkey's first child pubkey — for the faucet wallet that derivation is meaningless, only the commitment-signing side is used. The assert_eq! in start_rest_server therefore panicked the tokio-rt-worker on every cold boot after the migration, leaving the container Up-but-unresponsive on DEV. Replace the assertion with an explicit address override and document the rationale. Matches the pattern the test harness already uses in server_tests.rs::TestAccountData::new_minting_account. * feat(runtime): abort the process on any tokio panic By default a panic in a spawned tokio task only kills that task. The chain scanner runs in one task, the HTTP bootstrap in another; when the HTTP task panicked during the Plonky2 migration the scanner kept processing blocks, the container stayed Up, but port 4242 was never bound. Cloudflare served 502s for hours because there is no upstream signal that distinguishes "alive" from "alive-but-cannot-accept". Install a global panic hook that runs the default reporter (so the stack trace is still logged) and then exits the process. `restart: unless-stopped` in compose then crash-loops the container, which is trivially visible in `docker compose ps` and surfaces immediately in the post-deploy smoke test added in a follow-up commit. * test(runtime): smoke-test start_rest_server bootstrap end-to-end server_runtime.rs is excluded from the coverage scope (it binds a real socket and owns the process lifecycle), so the bootstrap path that exploded in the Plonky2 migration was never exercised by any test. This adds an integration smoke test that spawns start_rest_server against an ephemeral port and probes /health over real TCP — a bootstrap panic manifests as a connect timeout and the test fails with a clear message. Runs in ~22 s with a warm cargo cache and is included in the suite the pre-push hook runs via `cargo test -p server --release --all-features`. * ci(deploy-dev): smoke-test /api/info before reporting deploy success A green "Build and deploy to DEV" was historically misleading — when the runtime panicked during bootstrap the container stayed Up-but- unresponsive while the workflow reported success. Curl /api/info up to 30 times with 10 s spacing after the deploy ssh command; fail the workflow if it never returns 200. This blocks the auto-release PR from collecting a green check on a broken deploy.
…H_REMOTE (#37) The hardware target documented in CONTRIBUTING.md is a Mac Studio M3 Ultra with 96 GB RAM. On the laptop hardware many contributors actually work from (8 cores, 24 GB RAM) the full pre-push suite takes ~2-3x the documented wall budget and the Plonky2 prover starts swapping under load, while the laptop's other work fights for the same RAM. Add a transparent forwarder at the top of the hook: when `ZKCOINS_PREPUSH_REMOTE` is set, rsync the working tree (excluding `target/` and `.cargo/`) to the host and re-execute the hook there with `ZKCOINS_PREPUSH_INNER=1`. The remote runs the exact same verification script against the exact same code; output streams back to the local terminal; a non-zero exit aborts the push just as a local failure would. `git push --no-verify` still bypasses everything. The macOS-specific `ZKCOINS_PREPUSH_REMOTE_RSYNC` knob points at the Homebrew rsync because macOS ships `openrsync` at /usr/bin/rsync which lacks `--mkpath` and other modern flags. The CONTRIBUTING.md "Setup" section gains a sub-section with the full local + remote setup recipe (rustup install, nightly toolchain, components, cargo-llvm-cov, brew install rsync). When `ZKCOINS_PREPUSH_REMOTE` is unset the hook runs locally exactly as before — no behaviour change for anyone who hasn't opted in.
Closes #38. Adds `test_send_coins_rejects_source_commitment_missing_from_history_mmr`, which exercises the `account_server.rs:419` "Source commitment not present in history MMR" error path of the off-circuit defense-in-depth pre-check in `send_coins` — the only branch left uncovered by the strict 100% line + function gate after the Plonky2 / Poseidon migration (commit 937925a). Construction: honest mint → `state.update` → recipient `receive_coin` (so `coin_queue[0]` carries an in-tact out-coins-SMT `inclusion_proof` that passes line 416 and a state-resolvable `commitment.public_key`). Then overwrite `state.prev_mmr_root` with `ZERO_HASH` directly. The `get_merkle_proofs` builder reads that field verbatim into `commitment_root_mmr_sibling`, so the freshly-built source CMP recomputes a leaf `hash_concat(commitment_root, ZERO_HASH)` that does not appear in `state.mmr`; the genuine MMR proof is still threaded through, the recomputed root mismatches the actual history root, and only the MMR half of `verify_commitment` rejects — leaving the line-416 SMT-out_coins-inclusion path untouched, which is exactly the branch line 419 is meant to gate. This is the off-circuit defense-in-depth analogue of the in-circuit history-MMR check (Stage 5d-next-5 Phase 2b), and the natural companion of `test_send_coins_rejects_tampered_source_proof_inclusion` which closes the line-416 branch. Pure test addition; no production code change.
#41) Move the authoritative test + coverage gate off the developer's push and onto a self-hosted GitHub Actions runner on the documented hardware target (Mac Studio M3 Ultra). The new jobs ship gated behind `if: false` so the workflow YAML can land before the runner is registered; flip to active in a follow-up once the runner is online and produces a green run. The ubuntu-latest `lint-and-build` job is unchanged — it remains the cross-platform compile-bitrot gate. scripts/ci-runner/ documents the one-time operator setup: a dedicated `gh-runner` account (blast-radius minimisation, answers open question 1 in #40), idempotent prerequisites installer, launchd-managed service, and the outside-collaborator approval gate that prevents fork PRs from running code on the runner.
…42) With the authoritative test + coverage gate now running in CI on the self-hosted M3 Ultra runner (#40 part 1, PR #41), the pre-push hook no longer needs to re-run the full suite on the developer's laptop. Drop: - the test + coverage sections (full suite + llvm-cov), - the cyclic-recursion sweep, - the ZKCOINS_PREPUSH_REMOTE remote-routing block that worked around laptop-vs-target performance. What remains is the lint + type-check gate the hook is uniquely positioned to catch fast (< 30 s warm, < 2 min cold): three clippy scopes mirroring the CI Lint & Build job plus a workspace-wide cargo check. CI is the real test gate; this hook just stops obvious lint regressions from reaching the runner. CONTRIBUTING.md § Setup is rewritten around the new flow: new wall-budget table, drop "Running pre-push on a remote host", and update the CI/CD table to surface the two new self-hosted jobs. The Plonky2 migration's "Pre-push checklist" keeps the manual sweep + llvm-cov step for program-plonky2/ — the cyclic-recursion sweep is intentionally not in CI yet (open question 4 in #40).
Self-hosted runner `dfx01` is registered with labels `self-hosted, macOS, ARM64, m3-ultra, zkcoins-prover` and reports online. Drop the `if: false` gates on both jobs so they run on every push / PR targeting develop. Workflow-header note about the initial gate is no longer accurate; trimmed to a pointer at the operator doc. Follow-up: once the first develop run goes green, the two new check names land in develop branch protection. Refs #40.
…45) The first develop run after activating the self-hosted jobs failed to compile plonky2_field — `#![feature(specialization)] may not be used on the stable release channel`. Root cause: the launchd agent that runs the self-hosted runner inherits a minimal PATH including /opt/homebrew/bin (where a stable Rust lives) but not ~/.cargo/bin (rustup proxies). cargo resolved to Homebrew's stable, the workspace rust-toolchain pinning nightly was ignored, and any dep needing a nightly feature broke. Fix: prepend ~/.cargo/bin to GITHUB_PATH at the top of both self-hosted jobs (`server-tests`, `coverage`). Now `cargo`, `rustc`, and `cargo-llvm-cov` resolve to the rustup proxy, which reads the workspace rust-toolchain file and switches to the pinned channel. No other workflow / runner / hook change needed. Refs #40.
* ci: skip CI on draft PRs, fire on ready_for_review Adds a draft guard to the CI workflow so feature branches stop consuming self-hosted-runner time while work is still in progress. - pull_request.types: include ready_for_review so the workflow fires the moment a draft is marked ready (default types omit it). - lint-and-build.if: github.event_name == 'push' || github.event.pull_request.draft == false — skips drafts; the server-tests and coverage jobs inherit the skip via needs:. * docs(ci): note draft PRs skip the ci.yaml gate Companion to the workflow change in the previous commit. Updates the CI/CD table to read 'Ready PR -> develop' and adds a paragraph explaining when CI fires (ready_for_review, push) and when it doesn't (draft). Keeps CONTRIBUTING.md the single source of truth for the CI contract; matches what reviewers will see on PR #46 once they take it ready.
The README from PR #41 prescribed a dedicated gh-runner user on the host. The actual setup uses the dfx01 admin account because creating a new local user without interactive sudo was infeasible — and that account already had arbitrary-code-execution rights on zk-coins/server content via the prior ZKCOINS_PREPUSH_REMOTE flow, so it is not a blast-radius regression. What this commit changes in scripts/ci-runner/README.md: - "Blast-radius model": narrow the mitigation to the outside-collaborator approval gate; document the dfx01-as-runner decision and the residual risk. - "One-time setup": replace the gh-runner provisioning with the three steps actually performed — verify prereqs, register via `gh api .../registration-token` (or UI), `svc.sh install/start`. - Move the dedicated-user procedure into a "Migrating to a dedicated runner user" section at the bottom so the path stays documented for the future hardening upgrade. - "Operations": all commands rewritten to run via `ssh dfx01-remote`, matching how the runner is actually managed today. Removal procedure uses `gh api .../remove-token` for the removal-token rather than the UI. No behavior change — documentation only.
* ci: gate heavy M3 Ultra jobs behind `ci:full` PR label `Server + Shared Tests` (~60-90 min) and `Coverage Gate` share one self-hosted M3 Ultra runner. Without a gate, every speculative push on a ready PR consumed a full slot and queued anything else behind it. This change splits the contract: - `Lint & Build` (3 min, GitHub-hosted, free) keeps running on every ready-PR push. - `Server + Shared Tests` runs on PRs only when the `ci:full` label is present. `Coverage Gate` inherits the skip via its existing `needs: server-tests` chain. - `push to develop` always runs the full pipeline — post-merge on develop remains the authoritative source of truth, and the auto-release PR picks up its check rollup. Two trigger changes are needed for the label to actually re-fire the workflow when toggled: - `pull_request.types` adds `labeled, unlabeled`. - `concurrency.group` becomes conditional: label events get a unique run-id group so toggling an unrelated label (`bug`, `priority/*`, …) does NOT cancel an in-flight Heavy run on the same SHA. Trade-off documented inline: removing `ci:full` mid-run does not auto-cancel the in-flight Heavy run; use `gh run cancel` if that is genuinely needed. * docs(ci): document `ci:full` label + manual-cancel workaround Mirrors the workflow change in the previous commit. The CI/CD table now reads 'Ready PR -> develop with ci:full label, push to develop' for the two Heavy jobs, and an explanation paragraph covers all three states (draft / ready-without-label / ready-with-label). Adds a note about the concurrency-group trade-off so a future contributor isn't surprised when removing `ci:full` mid-run does not stop the in-flight Heavy: that's deliberate (an unrelated label toggle on a Heavy-running PR should not waste 60 min of M3 Ultra time), and the manual workaround is `gh run cancel`.
* docs: post-Plonky2 consistency cleanup + remove internal host refs Sweep through the docs that drifted out of sync with the current state of the repo after the Plonky2 migration (PR #17 onward, closed by PR #39 reaching 100% lines + functions) and after the CI rollout to a self-hosted runner (PRs #41 / #42 / #43 / #45 closing the issue-#40 sequence). ## Internal hostnames / personal names removed zk-coins/server is a public, indexable repo. Internal hostnames, SSH aliases, and personal names do not belong in committed docs. - `CONTRIBUTING.md:382, :402`: `dfxdev` / `dfxprd` → "the DEV / PRD hosts" / "the host running the server". - `CONTRIBUTING.md:113-114`: "Cyrill squashes" / "Cyrill merges" → "the maintainer squashes" / "Maintainers merge". - `program-plonky2/SESSION_STATE.md:48, :202`: `dfxdev/dfxprd` → "the DEV / PRD hosts". - `program-plonky2/STEP7_PREP.md:146`: `dfxdev and dfxprd` → "the DEV and PRD hosts"; `Cyrill's call` → "operator's call". - `MIGRATION_RESEARCH.md:17`: "Robin / Cyrill" → "the maintainers". The remaining 3 `dfx01` references in `scripts/ci-runner/README.md` are handled by the stacked PR chain #44 → #47 and not touched here. ## Stale Plonky2-migration phrases removed - `README.md:29` "current baseline is below 100% — goal is to lift it via follow-up PRs" → "100% lines + 100% functions enforced by `--fail-under-lines 100 --fail-under-functions 100` in the Coverage Gate CI job". The lift-to-100% goal landed in PR #39. - `README.md:43-44` "STALE — measured against the SP1-era build" → pointer to the per-module summary further down + Coverage Gate job. No "STALE" warning needed once the table is correct. - `README.md:223-231` per-module coverage table: replaced the pre-Plonky2 mixed-state row set (`account_server.rs` "excluded from gate, needs SP1-fixture port"; `server.rs` "excluded"; stale `state.rs` 97%, `scanner.rs` 51%) with the current 100% baseline the Coverage Gate enforces, and added `*_runtime.rs` to the exclusion list to match the gate's `--ignore-filename-regex`. - `README.md:233` "account_server.rs + server.rs are temporarily excluded during the Step-7 SP1→Plonky2 migration" → reflects the current state: both are at 100%, only the runtime / publisher / main wrappers are excluded by design. - `README.md` Features table (15 cells): per-handler `% (module)` hints updated to the current per-module coverage (server, account → `account_server`, username, scanner, state all at 100%). The `0% (publisher)` cells stay; that module is excluded by design. * docs: drop informal first-name references (informal speech-act leaks) Five informal first-name uses ("ask Robin", "Discuss with Robin", "Same version Robin used", "Robin pointed us at") survived the first sweep — they are not academic citations, they are speech-act tokens that name an internal collaborator. Replaced with: - ROADMAP.md R1: "ask Robin / the Plonky2 community" → "escalate to the maintainers / the Plonky2 community" - SPEC.md D4: "Discuss with Robin" → "Open" - MIGRATION_RESEARCH.md §2 row 1: "Same version Robin used" → "Same version as the upstream `BitVM/zkCoins` reference" - MIGRATION_RESEARCH.md §3 closing list: "Discuss with Robin: D4 …" → "Open / discuss with the maintainers: D4 …" - MIGRATION_RESEARCH.md §7.8: "the `BitVM/zkCoins` repo Robin pointed us at" → "the upstream `BitVM/zkCoins` reference repo" Legitimate academic citations (BitVM3 paper attribution in `BITVM_BRIDGE.md`, the Shielded CSV paper authors in `README.md` / `SPEC.md` reference lists) are kept — they are public author credits on published research, not internal collaborator references.
* docs(ci-runner): genericize host references for the public repo The runner README accumulated host-specific references over PRs #41 + #44. zk-coins/server is a public repo and indexable; per repo-wide convention internal hostnames, SSH aliases, and host-derived path fragments do not belong in committed docs. Replaces: - Bare `dfx01` mentions in prose → "the host" / "the host's admin user" / "the runner host". - `ssh dfx01-remote …` (17 occurrences) → `ssh "$RUNNER_HOST" …` plus an operator-convention paragraph at the top of the doc telling operators to set `RUNNER_HOST` in `~/.ssh/config` or `~/.zshenv` locally. - `--name dfx01` in the `config.sh` invocation → `--name "$(hostname -s)"`, restoring the pre-PR-#44 self-naming behaviour. Operators who want a different label can override. - `actions.runner.zk-coins-server.dfx01.plist` → `…<runner-name>.plist` with an inline note that `<runner-name>` is whatever was passed to `--name`. - Reference to the launchd service "on `dfx01`" in § Tracking → "on the runner host". No behaviour change: the same SSH alias works locally, the same runner registers, the same plist gets installed; only the public representation is generic. * docs(ci-runner): mark CI-jobs-activation section as done The runner is already registered (PR #41), the `if: false` gates are flipped (PR #43), and develop branch protection already requires `Server + Shared Tests (M3 Ultra)` and `Coverage Gate (100% lines + functions)`. The pre-Plonky2 `Tests` and `Coverage (MVP scope)` required checks were also dropped in the same operation. The section was written as a forward-looking instruction; now that the rollout is complete, it is reframed as a "historical — done" reference for future runner additions, with explicit "(done)" notes where it described actions that already happened. No instruction the operator still needs to perform remains.
2 tasks
) PR #87 merged with the nested `if let Some(block) { if let Some(hash) { if let Ok(h) = BlockHash::from_str(hash) { ... } } }` shape. The closing brace at line 37 (the path where `block.id` is a string but not a valid hex) reads as covered by the `parse_ws_frame_returns_empty_when_block_id_is_invalid_hex` test in local `cargo test`, but llvm-cov's region tracking reports the closing-brace region as untaken — the Coverage Gate flags `server/src/scanner_ws_parse.rs:37` as the only uncovered line. Flatten the block arm to a single Option chain (`block.get("id").and_then(...).and_then(...).map(...).unwrap_or_default()`). Behavior is identical across every input shape the existing tests cover; LLVM's region tracking collapses cleanly because the closing brace no longer exists as a distinct sub-region. Affects PR #18 (Release: develop -> main) and any open PR based on develop (e.g. #90).
* fix(scanner_ws_parse): flatten block arm so llvm-cov tracks line 37 PR #87 merged with the nested `if let Some(block) { if let Some(hash) { if let Ok(h) = BlockHash::from_str(hash) { ... } } }` shape. The closing brace at line 37 (the path where `block.id` is a string but not a valid hex) reads as covered by the `parse_ws_frame_returns_empty_when_block_id_is_invalid_hex` test in local `cargo test`, but llvm-cov's region tracking reports the closing-brace region as untaken — the Coverage Gate flags `server/src/scanner_ws_parse.rs:37` as the only uncovered line. Flatten the block arm to a single Option chain (`block.get("id").and_then(...).and_then(...).map(...).unwrap_or_default()`). Behavior is identical across every input shape the existing tests cover; LLVM's region tracking collapses cleanly because the closing brace no longer exists as a distinct sub-region. Affects PR #18 (Release: develop -> main) and any open PR based on develop (e.g. #90). * fix(server): prepare-then-commit mint to prevent state desync mint_handler advanced minting_meta.num_pubkeys, mutated the in-memory minting account, and persisted recipient state BEFORE attempting the on-chain inscription broadcast. When the broadcast failed (publisher empty, Esplora 5xx, WS timeout, etc.) the server's bookkeeping had already moved on, but the SMT/MMR never received the commitment — every subsequent mint and send for the same minting-account-pubkey-N then returned 422 with either "Unable to get merkle proofs for provided public key" or "Unable to get mmr inclusion proof for the previous root". Once tripped, the only known recovery was a full DEV-state wipe. Refactor into prepare -> broadcast -> commit: 1. Snapshot: under minting_account guard, read N + derive pubkeys. 2. Proof: clone the minting account and run send_coins against the clone — no mutation of self.accounts. 3. Broadcast: create_and_broadcast_inscription. Err -> 503, no state advanced anywhere. 4. Commit (broadcast OK): single sqlx tx with an optimistic UPDATE minting_meta SET num_pubkeys = N+1 WHERE id = 1 AND num_pubkeys = N so concurrent mints can't both commit; on row count 0 -> 503 "Concurrent mint detected". After tx commit, swap the mutated snapshot into account_server, advance num_pubkeys in-memory, persist the MintProof. Return 200. Startup invariant check (server_runtime): for i in 0..num_pubkeys assert get_commitment_proof(derive_public_key(i)) is Ok. Refuse to start on first miss with a CRITICAL log line that names the recovery procedure (reset_state workflow). No flag override. Tests: four new mint-broadcast-failure tests assert num_pubkeys unchanged + in-memory state unchanged + retry succeeds + concurrent mints serialize. The existing mint_broadcast_failure_returns_503 test gained the missing state-unchanged assertions it was missing. A new server_runtime test asserts the startup check rejects a desynced state. commit_handler audit: broadcast-first-then-receive_coin pattern already correct; documented as an invariant in code. Closes #89.
4 tasks
* rename: server → node (full identity rename)
The repo, Cargo package, binary, Docker image, and all Rust modules /
types previously named "server" become "node" — reframing the codebase
to match its actual role: a self-hostable Bitcoin/zkCoins node that
operators run for full transaction privacy ("run your own node"). The
old "server" name collided with DFXServer/server and falsely framed
the project as a centralised service.
Scope:
- Cargo: workspace member + package "server" → "node"
- Directory: server/ → node/ (git rename detection preserved)
- Source files: server.rs → router.rs, server_runtime.rs → runtime.rs,
account_server.rs → account_node.rs (+ *_tests siblings)
- Rust types: AccountServer → AccountNode, LoadAccountServerError →
LoadAccountNodeError, start_rest_server() → start_rest_node()
- Module paths: crate::server → crate::router, crate::server_runtime
→ crate::runtime, crate::account_server → crate::account_node
- Binary: /usr/local/bin/zkcoins-server → zkcoins-node
- API response: /api/info "service" field "zkcoins-server" → "zkcoins-node"
- Dockerfile: -p server → -p node, ENTRYPOINT zkcoins-node
- CI workflows: -p server → -p node, image tags zkcoin/server →
zkcoin/node (beta + latest + buildcache), DEPLOY_CMD updates
- Docs: README, CONTRIBUTING, ROADMAP, SPEC, BRIDGE_MVP, MULTI_ASSET,
ARKADE_INTEGRATION, MIGRATION_RESEARCH — titles, URLs, image refs,
module-path examples
- migrations: header comment "server state-layer" → "node state-layer"
Verification: cargo check, cargo check --tests, cargo fmt, cargo
clippy, cargo clippy --all-features all green locally.
Coordinated changes required on merge (separate PRs/pushes):
- DFXServer/server: bin/deploy.sh case branches, docker-compose service
/ container_name / image / exec, READMEs
- zk-coins/app, zk-coins/docs: markdown link updates
- Docker Hub: deprecation notice on zkcoin/server
* rename: server → node (content changes — fixup to 31762e2)
Previous commit captured the directory + file renames but missed all
the content edits because git mv only stages renames, not subsequent
file modifications. This commit ships every edit listed in the
previous commit message body (Cargo identifiers, Rust types/modules,
Dockerfile, CI workflows, docs).
cargo check / clippy / fmt all green locally before push.
* rename: ci.yaml job display names server → node (cosmetic)
* fix(coverage): update ignore-filename-regex after server_runtime.rs → runtime.rs
The 100% coverage gate was failing on runtime.rs because the
hardcoded ignore-regex still listed `server_runtime\.rs` — which no
longer matches anything after the file rename, so runtime.rs slipped
into the measured scope. Replace with `runtime\.rs`; covers both
runtime.rs and scanner_runtime.rs (already in the ignore list, so a
double match is a no-op).
) The rename PR (#93) edited two comment lines in node/migrations/0001_initial.sql ("zkCoins server state-layer" → "zkCoins node state-layer" and "server bootstrap" → "node bootstrap"). sqlx::migrate! hashes the entire file content, so the hash drifted — on startup the server panics with `Migrate(VersionMismatch(1))` because the `_sqlx_migrations` row on PRD/DEV still carries the pre-rename hash. Reverting the two comments restores the original hash and the migration is considered already-applied as intended. Cosmetic prose updates to migration files are out of bounds; the historical accuracy ("server" was the name when the schema landed) is the correct value anyway.
Aligns Docker Hub namespace with the rest of the project identity:
GitHub org `zk-coins`, domain `zkcoins.app`, Cargo workspace member
`node` already use the plural form; the singular `zkcoin/*` namespace
was a holdover from the early days.
Touched:
- .github/workflows/deploy-{dev,prd}.yaml — DOCKER_TAGS + cache refs
- Dockerfile — comment header (build/run examples)
- README, CONTRIBUTING, ROADMAP, MIGRATION_RESEARCH — image refs
Coordinated DFXServer/server compose update follows immediately after
merge so dfxdev / dfxprd pull the new `zkcoins/node:{beta,latest}`.
The legacy `zkcoin/node` repo on Docker Hub stays as a deprecated
read-only mirror.
- Docker version + pulls badges at the top - Explicit hub.docker.com/r/zkcoins/node link in lead paragraph - Image-tag cells in the Live table now link directly to the corresponding Docker Hub tag listing
* test: harden suite — remove dev_skip masking + add publisher preflight
The api_remote suite reported "33 passed" while critical paths were
silently skipped via dev_skip!() on 5xx errors and 120-s retry loops
on scanner-lag 422s. The Coverage Gate and the deploy-dev API E2E
became a green stamp instead of a real signal — an empty publisher
wallet caused every mint to 503, every 5xx was masked as "ok", and CI
stayed green on a broken DEV.
Tier 1 — must-fix:
- Remove all dev_skip!() on is_server_error() (4 sites)
- Remove dev_skip!() on /health/ready, balance-not-observed, and
/api/username/claim 503
- Remove SEND_RETRY_DEADLINE retry loops on "Unable to get
merkle/mmr proofs" 422 — scanner is event-driven post-#87, the
stopgaps are obsolete. Replace with poll_until_balance before the
send op (15-s ceiling).
- Add fresh-state assertion to happy-path roundtrips
- feature_skip!() becomes a hard panic when CI=true env is set
- mint_handler_concurrent_mint_during_proof_returns_503 now
synchronizes via a #[cfg(test)] tokio::sync::Notify instead of a
200-ms sleep
- commit_with_valid_signature_fails_broadcast_returns_503 now
wiremocks Esplora and asserts exactly 503 (no more accept-either)
- fetch_capabilities .expect() instead of .unwrap_or(false) — a
missing capabilities field is a contract regression
- New /health/publisher endpoint exposes the publisher wallet's
UTXO count + total sats
- New deploy-dev preflight step probes /health/publisher before
the API E2E job runs — empty wallet -> job fails with a clear
"top up publisher" message
Tier 2 — same-PR quality:
- Value-bearing assertions replace .is_some()/.is_ok() shape checks
in api_remote, server_tests, state_tests
- Hash-byte-length + non-zero assertions on send response payloads
- Concrete bounds on LNURLp min/maxSendable
- tokio::time::sleep(60s) in test handlers replaced with
std::future::pending::<()>().await
- Ad-hoc tempdir cleanup replaced with tempfile::tempdir()
- Delete proof_id_one_returns_200_or_404 — accept-either status
was tautological
Tier 3 — documentation:
- Comment block on lock-poisoning tests' nextest isolation
requirement
The three TODO comments in account_server.rs (lines 147, 170, 416)
are tracked separately and not addressed here.
* test: relax fresh-state assertion to upper-bound (push-trigger safe)
The strict `assert_minting_balance_is_bootstrap` helper would
tripwire CI on every develop push after a manual reset: the
deploy-dev workflow only runs `reset-zkcoins-server` on explicit
workflow_dispatch with reset_state=true, not on the default push
trigger. After this PR's first run, the minting balance drops to
`bootstrap - 2*MINT_AMOUNT` and the strict equality fails forever.
Replace with `assert_minting_balance_in_bounds`: upper-bound on
BOOTSTRAP_MINTING_BALANCE (catches unauthorized re-seed bugs) plus
a non-zero lower bound (catches unexpected wipe). Both happy-path
tests now use the same helper.
Also drops the redundant second `poll_until_balance` call in
`send_commit_roundtrip_moves_balance` (the prior `poll_balance_at_least`
already covered it) and documents the deliberately-deferred B5
proof_id pin in server_tests.rs (proof store ID grows across DB
lifetime, same constraint as the minting balance bound).
* test: cover publisher_health_handler unit tests + polish
Coverage Gate audit identified publisher_health_handler (router.rs)
as uncovered by unit tests — only api_remote E2E exercises it, and
api_remote is explicitly excluded from the coverage gate via
`-E 'not binary(api_remote)'`.
Add two unit tests in router_tests.rs mirroring the /health/ready
pattern:
- 200 Ok arm with wiremocked Esplora returning two UTXOs
- 503 Err arm via mint_test_state's unreachable Esplora URL
Refactor publisher_health_handler to derive the Taproot address from
PUBLISHER_KEY once at startup (lazy_static PUBLISHER_ADDRESS in lib.rs),
removing the SecretKey::from_str / Address::p2tr from the request path.
Side benefits:
- Handler is now pure I/O (Ok/Err on get_publisher_utxo only)
- One fewer panic-able branch per request
- Coverage Gate reaches 100% with the two new tests
Also:
- Fix stale "server::create_router" comment in runtime_tests.rs
(introduced by the test-quality commit, before PR #93's rename
sweep landed)
- Update BOOTSTRAP_MINTING_BALANCE doc-comment to describe the
bound semantic (not the strict equality that the second commit
of this branch relaxed)
- Defensive `command -v jq` install in deploy-dev.yaml preflight
The sccache server defaults to a 10-GiB cap, applied at server start. The cache is user-level (~/Library/Caches/Mozilla.sccache) and shared by every m3-ultra runner on the host; with 3+ parallel runners writing to it concurrently the 10-GiB default thrashed — one runner's writes evicted cache entries another runner had not consumed yet. Bump SCCACHE_CACHE_SIZE to 50 GiB in the node-tests and coverage job envs. The host has >600 GiB free disk, and the working set across target/, llvm-cov-target/, and cross-runner overlap fits comfortably. The sccache server only reads SCCACHE_CACHE_SIZE at start, so the "Ensure sccache" step now stops any already-running server whose cap differs from the requested value before calling --start-server. On-disk cache files survive the restart.
* docs: fix residual server → node references missed in #93 PR #93 (rename server → node) touched code + most docs, but a handful of identifier-style references in README.md and the CI-runner setup guide slipped through. None were runtime-critical — the build, deploy, and runtime paths were already on the new names — but the docs were out of sync with reality, and the ci-runner bootstrap URLs pointed at the redirect-only legacy repo. Fixes: - README.md - Cargo invocations: -p server → -p node (build, run, coverage) - Type refs: AccountServer:: → AccountNode:: - File refs: server.rs:: → router.rs::, server_runtime.rs → runtime.rs - Path refs: server/src/ → node/src/, server/migrations/ → node/migrations/ - Project tree: server/ branch → node/ branch (plus an explicit runtime.rs entry that was missing entirely) - scripts/ci-runner/README.md - All zk-coins/server URLs → zk-coins/node (curl + gh api) - Runner dir: ~/actions-runner-zkcoins-server → ~/actions-runner-zkcoins-node - LaunchAgent label: actions.runner.zk-coins-server → actions.runner.zk-coins-node - Workspace cache path: _work/server/server → _work/node/node - program-plonky2/src/circuit/main.rs - One inline comment account_server::send_coins → account_node::send_coins * fix: residual server → node references in hooks, CI, and docs (post-audit pass) Second-pass post-audit found more drift than the first pass caught: BREAKING fixes: - .githooks/pre-push lines 28–32: cargo clippy -p server → -p node (would fail on every developer push with "package server not found") - .github/workflows/ci.yaml line 139: the "forbid polling patterns" grep was scanning server/src/{scanner,publisher}.rs paths that no longer exist; combined with `|| true` it would silently pass even if polling was reintroduced into the event-driven hot paths Documentation drift fixes (all flagged by parallel audits): - README.md: `cargo test -p server` examples → -p node; Features-table coverage labels "(server)" → "(router)" (the module is now router.rs) - CONTRIBUTING.md: 7 references to `server/src/...` paths + `cargo test -p server db` invocation - SPEC.md, MIGRATION_RESEARCH.md, ROADMAP.md, BRIDGE_MVP.md, BITVM_BRIDGE.md, MULTI_ASSET.md, LIGHTNING_ATOMIC_SWAP.md: all `server/src/`, `server/migrations/`, `server/tests/` path refs switched to `node/...`; `start_rest_server` symbol → start_rest_node ZERO functional code touched. Verified residuals = 0 via rg -E 'cargo (clippy|build|test) -p server|start_rest_server|server/(src|migrations|tests)/' across hooks/workflows/docs. * fix: more residual server → node refs (third-pass audit) Third-pass audit found three more residuals: BREAKING: - README.md:54,106,243 — `cargo llvm-cov -p server` / `cargo build --release -p server` examples in the contributing/ building/cheatsheet sections. Developers copy-pasting these would hit "package `server` not found" immediately. SEMANTIC DRIFT: - node/src/main.rs:35,120 — constant ACCOUNT_SERVER_ADDR. Local scope, but undermines the rename if left in the codebase. DOCUMENTATION DRIFT: - node/src/lib.rs:5 — doc-comment path reference `server/tests/api_remote.rs` → `node/tests/api_remote.rs`. cargo check -p node green after fix. * fix: more residual server → node refs (fourth-pass audit) Fourth audit pass found three more residuals — all in user-facing documentation that prior passes missed because the search patterns weren't broad enough: BREAKING: - CONTRIBUTING.md:230 — Quick Start `cd server` → `cd node` - CONTRIBUTING.md:231 — Quick Start `cargo run -p server` → -p node - CONTRIBUTING.md:255 — `cd server` in the testing section Developers following the Quick Start would hit "no such directory" and "package server not found" immediately. SEMANTIC DRIFT: - CONTRIBUTING.md:395 — Naming-convention example `ACCOUNT_SERVER_ADDR` → ACCOUNT_NODE_ADDR (matches the actual constant in node/src/main.rs) - CONTRIBUTING.md:532 — Sample log line `Loaded AccountServer from Postgres` → AccountNode (matches the actual log emitted by the current code) - .gitignore:5 — `!server/minting_secret.bin` → `!node/minting_secret.bin` (the binary is gitignored, but this allow-rule referenced the old path) ROADMAP.md and program-plonky2/*.md retain `zk-coins/server` / `-p server` references as HISTORICAL context (commit descriptions dated pre-rename); intentionally not changed.
…101) * docs(program-plonky2): update server → node refs in migration notes These 5 files document the Plonky2-migration sub-package's history. Prior audits flagged them as "historical context, OK to keep as-is" — but that classification was wrong: they're active contributor docs (CONTRIBUTING.md says "Fresh contributor? Read this first"), and the code refs they contain (`account_server.rs`, `server.rs`, `AccountServer::`, etc.) point at files that physically no longer exist after PR #93. A new reader following one of these refs hits "file not found" — that's broken docs, not historical accuracy. Updates applied via perl one-shot: - GitHub URLs: zk-coins/server → zk-coins/node (GitHub redirects worked, but textual consistency was off) - File refs: account_server.rs → account_node.rs, server.rs → router.rs, server_runtime.rs → runtime.rs, account_server_tests.rs → account_node_tests.rs, server_tests.rs → router_tests.rs - Type refs: AccountServer → AccountNode, LoadAccountServerError → LoadAccountNodeError - Module paths: account_server::, server_runtime::, server.rs:: → account_node::, runtime::, router.rs:: - Cargo: -p server → -p node (run/test/build/llvm-cov) - Path refs: server/src/, server/migrations/, server/tests/ → node/... - Function: start_rest_server → start_rest_node Counts: CONTRIBUTING (6 lines), SESSION_STATE (38), STEP7_PREP (36), STAGE_5D_NEXT_4_DESIGN (2), STEP4_REVIEW (2). All swaps preserve historical semantics — the PR-#17 etc. event descriptions stay factually correct since the PR numbers themselves don't change; we just spell the repo by its current canonical name. * fix: final residual sweep — ROADMAP commit-narrative + api_remote.rs reset cmd ROADMAP.md:75 — historical commit-narrative for dac0179. Prior pass caught lines 73/76/77 in the same file (already renamed account_node / router_tests etc.) but missed line 75's '-p server' / 'Plonky2 server'. Renamed for consistency with the rest of the doc. node/tests/api_remote.rs:904/969/1319 — three test-comment refs to the old SSH command 'reset-zkcoins-server'. The actual command on both dfxdev and dfxprd is now 'reset-zkcoins-node' (per the rename in DFXServer/server bin/deploy.sh). Comments updated to match. * fix(tests): rename remaining 'server' local-var refs to 'node' The Rust type rename AccountServer → AccountNode (PR #93) left the test files using `let mut server = AccountNode::new(...)` and `server.method()` throughout. Stylistically idiomatic but inconsistent with the rest of the rename — fixed in this commit: - node/src/account_node.rs: 23 occurrences (`fresh_node` helper + tests) - node/src/account_node_tests.rs: 67+ occurrences (every test) + 2 assertion-message strings ("in server and program" → "in node...") - node/src/router_tests.rs: 6 occurrences Plus one missed identifier ref in program-plonky2/CONTRIBUTING.md:185 (`-p server -p shared` → `-p node -p shared`). Verified: cargo check -p node --tests + cargo clippy -p node --tests both pass. No logic change, pure variable rename. * style: cargo fmt after server -> node var rename Shorter variable name (3 chars vs 6) lets several method chains collapse onto one line per rustfmt default config. Pure formatting.
* docs: reflect 6-agent runner pool + Server→Node rename residuals
Three things bled stale when the pool grew from 1→3→6 m3-ultra agents
and the Heavy job was renamed `Server + Shared Tests` → `Node + Shared
Tests`:
1. scripts/ci-runner/README.md still described a "single runner"
topology, including the explicit advice "add a single self-hosted
runner only (one concurrent job per repo) and let GitHub queue the
rest" — directly contradicted by the live pool. Rewritten:
- Title + intro pluralized; hardware-target section explains the
6-agent pool sharing one M3 Ultra host.
- New "Scaling out" section documenting the procedure used to add
dfx01-4/5/6 (registration-token reuse, tarball cache in /tmp,
SIGPIPE foot-gun with `set -o pipefail` + `head`).
- Operations section now uses `${RUNNER_DIR}` so snippets work for
any agent; added a pool-wide loop for status / cache wipe.
- Disk + RAM headroom rewritten with the measured budget table
(3 parallel jobs → ~14 GB cargo RSS, ~85 GB app memory, 0 swap;
6 parallel forecast → ~29 GB / ~95 GB / 0 swap).
- Naming-drift note for the legacy `actions-runner-zkcoins-server`
/ `…-zk-coins-server-N` directories that predate the repo rename.
- "Activating the CI jobs (historical)" section: branch protection
migrated develop → main, required-check list updated to the
current 4 contexts (incl. the Node + Shared Tests rename).
2. ci.yaml carried two stale comment blocks: the concurrency rationale
called M3 Ultra capacity "scarce" (single-runner framing) and the
node-tests job described itself as running on "the single self-
hosted M3 Ultra". Both reworded to reflect the 6-agent pool.
3. CONTRIBUTING.md and README.md referenced "the self-hosted M3 Ultra
runner" (singular) and the old `Server + Shared Tests` job name in
the CI/CD table + pre-push narrative. Pluralized and renamed; the
cargo invocation in README.md was also updated `cargo test` →
`cargo nextest run -p node -p shared --release --all-features
--test-threads=1` to match the workflow.
Branch protection on `main` was patched out-of-band today to rename
the required context `Server + Shared Tests (M3 Ultra)` → `Node +
Shared Tests (M3 Ultra)`; this commit aligns the docs with that
config change.
* docs: address PR #104 review — close remaining Server→Node residuals
Senior-review pass on PR #104 flagged two BLOCKING gaps the original
commit missed (the PR description claimed no non-historical `Server +
Shared Tests` references would remain):
- ROADMAP.md test-plan paragraph still cited the old job names.
- program-plonky2/CONTRIBUTING.md described the CI gate as running
`-p server -p shared` against `Server + Shared Tests` — both pieces
stale post-rename.
Also addressed the consistency findings:
- scripts/ci-runner/README.md budget table prose contradicted itself
(called 6 jobs a "forecast" while the prose explained that 3 PRs ×
2 jobs already saturates the pool at 6 concurrent agents). Reworded
to make clear the snapshot was captured under the pre-expansion
3-runner topology and the saturated column projects the linear
envelope of the new 6-agent pool.
- scripts/ci-runner/README.md:13 used "Six" (spelled-out) while every
other reference uses the digit "6". Harmonised.
- CONTRIBUTING.md, README.md, and the budget-table prose drifted on
the `--test-threads` form (`=1` vs ` 1`) versus the actual ci.yaml
command, and elided the `-E 'not binary(api_remote)'` test filter
that the workflow uses. Quoted commands now match ci.yaml verbatim.
- .github/workflows/ci.yaml sccache comment still read "The M3 Ultra
runner is self-hosted" (singular) while the neighbouring updated
comments now talk about the pool. Pluralised.
GiB vs GB nit (`1 TB host` vs `>600 GiB free`) left as-is — matching
the conventional usage (TB / GB for disk capacity, GiB for sccache
cap).
* docs: address second-pass review — pluralise, fix dangling URLs, refine budget table
Second senior-review pass on PR #104 flagged a handful of consistency
nits left after the first fix-up commit:
- scripts/ci-runner/README.md `## Tracking` section was still singular
("The runner is a launchd service") — pluralised to "Each agent".
- program-plonky2/CONTRIBUTING.md still linked to PRs/issues under the
pre-rename `zk-coins/server` org URL (PR #17, PR #48, issue #50).
GitHub auto-redirects, but the cross-link to the top-level
CONTRIBUTING.md inconsistency was distracting — all three rewritten
to `zk-coins/node` to match the rest of the docs.
- .github/workflows/ci.yaml had three remaining singulars: the
Docker-socket comment on both node-tests + coverage jobs ("The M3
Ultra runner on dfx01 runs Colima"), the sccache cap comment ("every
m3-ultra runner on the host; with 3+ parallel runners"), and the
concurrency block's last line ("free the runner"). All four reworded
to use the pool / per-agent framing consistently.
- scripts/ci-runner/README.md budget table:
- Prose said "linearly projects to 6", but the App-memory row goes
~85 GB → ~95 GB, which is bounded by the 96 GB host RAM ceiling,
not a linear extrapolation. Reworded to call out the cache-bound
ceiling.
- "CPU cores in use: 3 of 28 → 6 of 28" misleadingly implied an
idle box; renamed to "Active test processes (`--test-threads 1`)"
with cell content stripped of the misleading "of 28 cores" framing.
- ci.yaml:245 comment still spelled `--test-threads=1` (equals form)
while the actual command on :256 uses ` 1` (space form). Aligned.
Out-of-scope (flagged in review, intentionally not touched in this PR):
- deploy-dev.yaml + deploy-prd.yaml still use singular "the self-hosted
M3 Ultra runner" framing. Not in the PR's stated scope; carry as a
follow-up if the framing becomes confusing.
…#102) * security: require PUBLISHER_KEY env var on every network (no default) Remove the `DEFAULT_PUBLISHER_KEY = "1234567890abcdef…"` fallback from `node/src/lib.rs`. The constant was a publicly-known test key embedded in the open-source repo and Docker image since the project's inception; on-chain forensics confirms 4 historical drains of the matching Taproot publisher address, each a single-input → single-output → minimum-fee sweep against a fresh rotating recipient. A drainer bot monitors the public address and empties any top-up within minutes. Previously the fallback was network-gated: a startup panic guard fired ONLY on mainnet if `PUBLISHER_KEY` was unset. Every non-mainnet deploy (DEV, signet, any future testnet stage) silently used the burned key. This made the publisher Taproot address structurally unfundable on every non-mainnet network — every sat sent to it was drained before the publisher could spend it. This change removes the fallback network-wide: - `PUBLISHER_KEY` is now a required env var on DEV, signet, AND mainnet. The bootstrap panics on startup if it is unset, with a message pointing to the Vaultwarden item and the `openssl rand` recipe for local dev. - `DEFAULT_PUBLISHER_KEY` const removed entirely. - The mainnet-only panic guard is gone — the unconditional `.expect` on the env var subsumes it. Test fixtures (`router_tests.rs`, `publisher_tests.rs`) that derived the mock publisher Taproot address from the burned key are rebased onto a new test-only placeholder (`0000…0001`). The CI workflow (`node-tests` + `coverage` env blocks) now sets `PUBLISHER_KEY` to the same placeholder so `cargo nextest` / `cargo llvm-cov` keep passing. The placeholder is syntactically a valid 32-byte hex secret but is NEVER to be used on any chain that holds value — its sole purpose is to make a future grep for the burned `1234…` key return empty across the repo + CI config. CONTRIBUTING.md's Environment Variables table is expanded to list every variable the server actually reads (DATABASE_URL, PUBLISHER_KEY, USERNAME_DOMAIN, PROOFS_DIR, SCANNER_INITIAL_SETTLE_TIMEOUT_MS, …), marks the required ones explicitly, and documents the `openssl rand -hex 32` local-dev recipe + the Vaultwarden source of truth for DEV/PRD. Companion change required in DFXServer/server before the next dfxdev / dfxprd deploy: the deploy stack must pass `PUBLISHER_KEY` from the Vaultwarden item into the node container env, otherwise the service will fail to start. See DFXServer/server develop for the corresponding compose/env update. * chore(security): scrub deployment-environment references from product code The product code is portable and should describe deployment expectations in abstract terms only — concrete hostnames, secret-manager item names, and infra-repo links belong to a downstream operator's own ops docs, not this repo. Drops a handful of such references from the publisher-key error message, the CONTRIBUTING env table, and the CI workflow comments, and parameterizes the self-hosted runner's Colima socket path via $HOME so it no longer hard-codes a user account.
Four small follow-ups identified by the pre-CI audit, none blocking but all worth landing: 1. router.rs: switch `&*PUBLISHER_ADDRESS` deref to `.clone()` — eliminates a llvm-cov region-tracking edge case on the new handler's first line (98% safe either way; this is belt-and- braces). Address::clone is cheap. 2. router_tests.rs: wrap both await points of `mint_handler_concurrent_mint_during_proof_returns_503` in `tokio::time::timeout` (30 s + 60 s). Prevents a future regression in `mint_handler` phase 2 from hanging the 120-min CI job budget. 3. api_remote.rs: replace stale `reset-zkcoins-server` comment references with the post-rename `reset-zkcoins-node`. Cosmetic; matches the host-side dispatcher command name updated in DFXServer/server commit f74ec4a. 4. ci.yaml: the polling-pattern lint step (issue #84 guard) targets paths under `server/src/` that no longer exist after PR #93's rename to `node/src/`. The grep returned empty vacuously, which means the lint has been silently dead for 24 h. Update paths. Note: the audit also flagged the stale `server::create_router` comment in runtime_tests.rs, but that fix already landed in 687f412 on chore/test-quality-overhaul. Stacked on top of PR #94 (chore/test-quality-overhaul) per the "no force-push during running CI" project convention.
…escape (#105) * test(api_remote): add ZKCOINS_E2E_ALLOW_FEATURE_TRIMMED_SERVER escape hatch The `feature_skip!` macro panics in CI to canary an accidentally dropped `--all-features` flag. That assumption is broken for `deploy-dev.yaml`: the DEV image intentionally ships MVP-only by policy (Dockerfile `ARG FEATURES=` defaults to empty so DEV and PRD run the identical binary), so the gated `address_list` / `lnurl` tests panic instead of skipping cleanly when the suite runs against the deployed server. Introduce `ZKCOINS_E2E_ALLOW_FEATURE_TRIMMED_SERVER`: when set (any value), the macro downgrades the CI panic to the existing silent skip. Workflows pointing the suite at a feature-trimmed server opt in; the canary stays armed for every other CI invocation. Wire the env var into the `Run API E2E suite against DEV` step in `deploy-dev.yaml` with an inline comment pointing back to the Dockerfile policy. * fix(publisher): add esplora-REST fallback when track-tx WS times out Mutinynet's public WS endpoint regularly goes 30-90 s between frames, so the 30 s `TRACK_TX_TIMEOUT_SECS` safety-net can elapse even when the commit broadcast landed on-chain. Callers then see a 503 from `/api/mint` despite the commit transaction being in the mempool / a block, which blocks the develop → main Release PR on the API E2E suite. When `TrackTxStream::wait` returns `WsError::Timeout`, issue ONE REST `GET /tx/{commit_txid}` via the existing `EsploraAsyncClient`: * `Ok(Some(_))` -> success, the WS just missed the frame; continue to the reveal broadcast. * `Ok(None)` (404) -> broadcast genuinely failed; propagate the original `WsError::Timeout`. * `Err(_)` -> REST itself failed; propagate the original `WsError::Timeout` (the WS timeout is the real signal). This is a single REST call, not a poll loop, so the "No polling — events only" invariant (CONTRIBUTING.md, CI grep gate in `ci.yaml`) is preserved. `TRACK_TX_FRAME_WATCHDOG` and `TRACK_TX_TIMEOUT_SECS` keep their existing semantics; the fallback only narrows the failure mode the publisher reports on timeout. * docs(publisher): refresh TRACK_TX_TIMEOUT_SECS docstring for REST-fallback behavior The 30 s WS-wait timeout now triggers a single REST GET /tx/{txid} fallback; a 200 ⇒ proceed, a 404/other-error ⇒ propagate WsError::Timeout. Update the constant docstring, the track_tx_timeout field doc, and the matching test comment to describe the actual behavior. The underlying 'no silent fallback' rationale is preserved. * docs(publisher): clarify outer vs inner WS timeout in fallback docstring The REST fallback fires on the OUTER TRACK_TX_TIMEOUT_SECS budget owned by the publisher (TrackTxStream::wait), not on the inner per-frame TRACK_TX_FRAME_WATCHDOG reconnect loop in scanner_ws.rs — which is untouched by issue #84's fallback work. * style(publisher): use println! for broadcast fallback diagnostics Match the surrounding diagnostic style in broadcast_inscription_txs; every other log line in the function already uses println!. Out of scope: migrating the rest of the file to a structured logger.
* feat(publisher): expose build_reveal_only helper for external recovery Extract a pub fn build_reveal_only from inscription_txs that deterministically reconstructs the reveal transaction from (commit_txid, commit_output_value, commitment_data, publisher_key, publisher_address, network). The legacy in-process path (inscription_txs) and the new out-of-band recovery path (the recover_inscription CLI) share the same script-path anchor derivation + nonce mining loop via a private helper. No behavior change: the commit + reveal pair returned by inscription_txs is byte-identical to the pre-refactor output for the same inputs. The prevout scriptPubKey is now built via ScriptBuf::new_p2tr_tweaked instead of going through Address::p2tr_tweaked(...).script_pubkey(); both produce the same witness program (network-agnostic OP_1 <32-byte-output-key>). Motivation: a stuck inscription anchor (commit broadcast, reveal never broadcast) needs the reveal rebuilt out-of-band. Calling inscription_txs with synthetic outpoints would produce a reveal that spends a phantom commit txid; the recovery path needs to target the actually-broadcast commit txid. * feat(bin): add recover_inscription CLI for stuck anchor recovery Adds a node binary that reconstructs and broadcasts the reveal transaction for an inscription anchor whose commit was broadcast but whose reveal never made it to the network (process crash between commit-broadcast and reveal-broadcast, lost reveal bytes, etc.). PR #105's REST fallback covers the WS-slow / WS-flaky failure mode during normal operation; this CLI is the operator escape hatch for any other failure. The CLI takes the broadcast commit txid, the inscription payload hex (from node logs), the anchor's sats value, and the expected script-path P2TR anchor address. PUBLISHER_KEY + IS_MAINNET come from env, matching the node's own bootstrap. The reveal is rebuilt via publisher::build_reveal_only — the same code path the in-process publisher uses — then sanity-checked against --anchor-address before broadcast. On --dry-run the CLI logs the reveal hex without broadcasting. The bin is excluded from the 100% line + function coverage gate via the bin/.*\.rs$ ignore-filename-regex (operator tooling that talks to a live Esplora endpoint; not testable hermetically). The shared reveal-construction logic remains covered by the existing inscription_txs_* tests in publisher_tests.rs.
…#107) PR #105 fixed the WS-timeout race that left commit UTXOs stranded at script-path anchors when the reveal never broadcast; PR #106 added a CLI to recover stuck anchors. This commit closes the underlying gap by persisting the (commit, reveal) pair to Postgres BEFORE the first broadcast and walking it through a state machine (constructed -> commit_broadcast -> reveal_broadcast -> complete) as each step lands. On bootstrap, any non-complete row is re-driven through the remaining steps. Schema (new migration 0003_pending_inscriptions.sql): - pending_inscriptions table with UNIQUE(commit_txid), CHECK on status, partial index on status <> 'complete'. db.rs additions: - PendingInscriptionRow + PENDING_STATUS_* constants. - insert_pending_inscription, update_pending_status, load_pending_in_progress. publisher.rs: - create_and_broadcast_inscription now takes Option<&PgPool>; when Some, persists a 'constructed' row before broadcast. - broadcast_inscription_txs_with_persistence threads status updates through commit_broadcast / reveal_broadcast / complete. - resume_pending_inscriptions re-broadcasts any pending rows on boot, tolerating bad-txns-inputs-missingorspent / txn-already-known as successful idempotent retries. runtime.rs / router.rs: - mint_handler and broadcast_commit_and_deliver pass state.pool down. - start_rest_node calls resume_pending_inscriptions on bootstrap; failures are logged and swallowed so a transient Esplora outage cannot crash-loop the container. publisher_tests.rs: 8 new tests covering the forward persistence path, all three resume branches, the no-op skip on complete rows, idempotence of repeated resume calls, and the double-spend tolerance case where Esplora rejects a commit re-broadcast.
…hase C) (#108) * schema: add mmr_root_index table for Phase C persistence The in-memory `State::root_indices` map — `prev_mmr_root -> (smt_root, leaf_index)` — is the lookup that powers `State::get_mmr_inclusion_proof` on every mint and send. Before this migration the map was rebuilt empty on every restart, breaking any account whose latest proof referenced a historical `commitment_history_root`: `/api/mint` surfaced 422 "Unable to get mmr inclusion proof for the previous root". The new table mirrors the in-memory shape one row per `(prev_mmr_root)` key. INSERT ... ON CONFLICT DO NOTHING makes replays idempotent (an MMR append is monotonic, so the same prev_mmr_root cannot legitimately resolve to two distinct (smt_root, leaf_index) tuples). * feat(state): persist root_indices entries per update + rebuild on load Implements Phase C of the state-layer hardening series. The `State::root_indices` HashMap is now persisted per successful `State::update` and rebuilt by `State::load_from_pg`, closing the bug where a container restart left every account whose latest proof referenced a pre-restart `commitment_history_root` unable to mint or send (the `get_mmr_inclusion_proof` lookup returned Err, /api/mint surfaced 422). db.rs grows three helpers next to the existing MMR/SMT persistence: * `insert_root_index` (single-row, ON CONFLICT DO NOTHING) * `upsert_root_indices` (batch, single transaction) * `load_root_indices` (ORDER BY leaf_index for deterministic rebuild) state.rs: * `load_from_pg` calls `load_root_indices` and populates the map. The highest-leaf_index entry's KEY is precisely what the last successful `update()` wrote to `self.prev_mmr_root` (insert order in `update` is: write prev_mmr_root, insert root_indices keyed by same value, then `mmr.append`) — so we restore prev_mmr_root from the last entry without re-scanning the assembled HashMap. * `update()`'s public signature is unchanged. After a successful call the freshly-inserted entry is recoverable as `(self.prev_mmr_root, self.root_indices[&self.prev_mmr_root])`, so the caller (main.rs scanner callback) reads it back from `self` rather than threading a `&PgPool` into a sync method that is called from 23 test sites today. main.rs / lib.rs: * `insert_root_index_from_sync_context` mirrors the existing `persist_state_from_sync_context` block_in_place bridge for the scanner's sync callback. * The root_index write runs OUTSIDE the SMT/MMR/latest_block transaction. A failure does not corrupt canonical state; the missing row is re-derived on a future re-scan (scanner replays past blocks, `update()` is idempotent at the SMT level, and the INSERT is itself ON CONFLICT DO NOTHING). * test(state): coverage for mmr_root_index persistence + restart recovery Adds seven tokio tests in state_tests.rs covering every new branch: * `test_root_indices_persist_and_load_roundtrip` — drive three updates with per-update insert_root_index, drop, reload, assert the HashMap content + prev_mmr_root round-trip. * `test_load_from_pg_with_empty_root_index_table_yields_empty_map` — fresh DB, empty load, no error, prev_mmr_root == ZERO_HASH. * `test_get_mmr_inclusion_proof_after_restart_succeeds` — the central regression test: N historical prev_mmr_roots all resolve after a simulated restart, AND each returned proof verifies against the extended MMR root. * `test_upsert_root_indices_batch_path_idempotent` — exercises the bulk helper, including the empty-slice early return. * `test_load_root_indices_rejects_short_prev_root_blob` * `test_load_root_indices_rejects_short_smt_root_blob` * `test_load_root_indices_rejects_negative_leaf_index` — and the matching `LoadStateError::Db` surface via `State::load_from_pg`. * `test_insert_root_index_is_idempotent_on_conflict` — ON CONFLICT DO NOTHING on the single-row path. Also fixes the pre-existing `connect_and_migrate_creates_all_tables` assertion that did not list `pending_inscriptions` after PR #107; adds both that table and the new `mmr_root_index` so the assertion stays true. * fix(state): atomic persist of mmr_root_index with state snapshot Fold the per-update `mmr_root_index` INSERT into the same Postgres transaction as the SMT/MMR/latest_block snapshot. The previous two-call shape (persist_state_tx → insert_root_index) opened a crash window where a failure between the writes left the saved latest_block ahead of the missing root_index row. On restart the scanner resumed from that latest_block, re-scanned the same commit tx, replayed state.update against an already-advanced MMR, and produced a NEW prev_mmr_root keyed entry — the originally-missing row was never healed and /api/mint returned 422 for accounts whose latest proof referenced the pre-restart commitment_history_root. persist_state_tx now takes Option<(&HashDigest, &HashDigest, u64)> for the freshly-inserted root_indices entry; INSERT runs inside the BEGIN/COMMIT with ON CONFLICT (prev_mmr_root) DO NOTHING so a re-scanned commit (same MMR, same tuple) is a no-op on the row that did land. persist_state_from_sync_context bridge forwards the new argument; main.rs scanner callback drops the separate insert_root_index_from_sync_context call. Adds two db_tests covering the atomic-write and ON-CONFLICT-DO-NOTHING branches; populate_state_with_persistence in state_tests now mirrors the production single-call shape. * style(state): drop fallible usize::try_from for 64-bit-only path The `usize::try_from(leaf_index)` in `load_from_pg` is uncoverable on 64-bit targets (the only ones we ship): `db::load_root_indices` already rejects negative `i64` values, and a non-negative `i64` always fits in a `usize` on Linux x86_64 / aarch64. The Coverage Gate (100 % lines + functions on `state.rs`) cannot reach the `map_err` branch, which would fire the gate on every PR touching this file. Replace with `leaf_index as usize` plus a `debug_assert!` that catches the hypothetical 32-bit dev build without growing a runtime error path. Behaviour on 64-bit is unchanged. * chore(db): remove unused upsert_root_indices helper + test The bulk-insert helper had no production caller — only test_upsert_root_indices_batch_path_idempotent exercised it. The doc-comment referenced a "future snapshot-serialization tool" that never landed, and after the Phase-C atomicity fix every production write goes through persist_state_tx (one row per scanner callback). Deleting the helper plus its lone test removes ~40 lines of dead surface and the matching coverage-gate burden. * chore(lib): remove dead insert_root_index_from_sync_context bridge After folding the mmr_root_index INSERT into persist_state_tx, the scanner callback no longer calls the standalone sync-from-async bridge. Drop the function; the underlying db::insert_root_index helper stays exposed for potential future use and is still covered by test_insert_root_index_is_idempotent_on_conflict.
* feat(state): derive_num_pubkeys_from_smt + drop minting_meta migration Add a derivation that walks pk_0, pk_1, ... and returns the first index whose sha256(pk.serialize()) key is absent from the SMT. The SMT is already the canonical source of truth for which minting commitments landed on-chain, so collapsing the separately-stored minting_meta.num_pubkeys counter into the derived value eliminates the desync class documented in #89. Migration 0005 drops the now-orphaned minting_meta table outright (it had no other columns beyond id + num_pubkeys + updated_at). The matching db helpers and the commit_mint_tx counter step are removed in follow-up commits. Coverage exercises both branches of the algorithm: the 'found at index N' branch (empty SMT and pre-seeded prefix), and the loop-bound panic branch via a bound-parametrised inner with a tiny BOUND so the test stays fast (a million real BIP-32 derivations would take minutes). * refactor(db): drop minting_meta helpers + counter step in commit_mint_tx The pair load_minting_num_pubkeys/upsert_minting_num_pubkeys and the optimistic UPDATE-with-WHERE counter bump at the head of commit_mint_tx existed only to persist the minting account's monotonic num_pubkeys. With Phase D deriving num_pubkeys from SMT membership at runtime there is no counter to load, write, or guard with an optimistic update. commit_mint_tx now returns Result<(), sqlx::Error> — the bool race-loser discriminator is gone with the counter step. The recipient upserts that mint_handler used to issue separately after the counter+minting bundle are now folded into the same transaction (one bundle, one tx). db_tests sheds the six minting_meta tests; the three commit_mint_tx tests are rewritten to assert atomic upsert + idempotent conflict + empty-input no-op against the new shape. * refactor(runtime): drop check_minting_state_invariant + scanner_progress wiring The pre-Phase-D startup check enumerated pubkey_idx in 0..num_pubkeys and verified each had a commitment in the SMT — it existed because the in-memory counter could disagree with the SMT (the desync class fixed in #89). Phase D collapses the counter and the SMT into one derived value via derive_num_pubkeys_from_smt, so the predicate the check measured is a tautology by construction. Removing the check removes the scanner-settle wait that fed it: the shared AtomicU64 progress counter and the Arc clones plumbed through start_rest_node and the scanner callback in main.rs are gone too. The related test runtime_tests::startup_invariant_rejects_when_num_pubkeys _exceeds_smt is deleted. The minting account is no longer rehydrated from minting_meta on boot — ClientAccount::new starts with num_pubkeys=0 and the value is re-derived from SMT on every mint. * refactor(router): derive minting num_pubkeys from SMT in mint_handler mint_handler's phase-1 SNAPSHOT now calls state::derive_num_pubkeys_from_smt under the State lock instead of reading the in-memory minting_account.num_pubkeys; the value is no longer cached anywhere across requests. The phase-3 re-check re-derives from the SMT and aborts with 503 if the count advanced (equivalent shape to the pre-Phase-D in-memory compare, now measured against the canonical source). The post-commit in-memory advance and the separate per-recipient upsert loop are gone — the recipient rows ride along inside the same commit_mint_tx transaction as the minting account row. The pre-Phase-D commit_mint_tx Ok(false) race-loser arm vanished with the counter step, so the matching 503 path in the handler does too. The new in-process gate is the phase-2 re-derive; the on-chain gate is SparseMerkleTree::insert in the scanner callback, which errors on a duplicate key + different value (i.e. a true concurrent same-N mint where both inscriptions landed). The handler doc-comment describes both legs. Tests: - mint_with_nonzero_num_pubkeys_covers_prev_pubkey_arm seeds pk_0 into the SMT so derive returns 1 (was: pre-bumped in-memory counter). - mint_handler_concurrent_mint_during_proof_returns_503 injects pk_0 into the SMT between phase 1 and phase 3 (was: in-memory bump). - concurrent_mints_only_one_commits is removed; its DB-counter gate is gone. - upsert_mint_recipient_or_log_swallows_pool_dead_error is removed; the helper is gone. - The retry-after-failure and happy-path assertions no longer touch load_minting_num_pubkeys; they check the accounts row landed instead. * style(state): cargo fmt the derive_num_pubkeys_from_smt key line
* feat(db): add pending-status lookup + block-free state persist helper Two additive helpers used by the Phase E synchronous-state-advance work in subsequent commits: * `pending_inscription_status_by_commit_txid` returns the current `pending_inscriptions.status` for a commit txid (or `None` for a missing row). The scanner uses this to short-circuit its `state.update` call when the mint flow has already integrated the inscription in-process. * `persist_state_without_block_tx` writes the SMT/MMR/root_index triple in one transaction without touching `latest_block`. The mint flow advances `state.update` synchronously after a successful broadcast and persists the resulting snapshot, but does not know which Bitcoin block holds the just-broadcast inscription — the scanner remains the sole writer of `latest_block`, so splitting the helper keeps the scanner's resume marker independent. Both helpers are pure additions; no production caller in this commit. * refactor(state): add update_and_snapshot_for_persist helper `State::update` followed by `serialize_for_persist` (with a recovery read of the freshly-written `root_indices` entry) is the exact dance both the scanner callback and the Phase-E mint-flow integration need to run under the state lock before handing bytes to `db::persist_state_tx`. Extract into one method so both call sites share a single source of truth for "what goes into the snapshot tuple" — and so a future change to the snapshot shape lands in one place instead of two. Pure additive refactor; no behaviour change. Existing `update` + `serialize_for_persist` remain in place for callers that don't need the bundled tuple. * feat(scanner): thread commit_txid + add skip-decision helper Two changes wired in lockstep so the build stays clean: * `InscriptionCallback` now receives the inscription's `commit_txid` alongside the content bytes and block hash. The commit_txid is the previous-output txid of the reveal-side input that carried the envelope — by construction, the same value `publisher::create_and_broadcast_inscription` persists in the `pending_inscriptions.commit_txid` column. The scanner callback in `main.rs` uses it to look up the pending status before running `state.update`. * `should_skip_scanner_state_update` is the pure decision helper: returns `true` only when the row exists and is `complete`. Every other state (missing row, an in-progress mint, an unknown future status) falls through to `state.update`. Centralising the decision in one tested function keeps the scanner closure thin and makes the contract testable without spinning up a scanner. `main.rs` adopts the new signature, runs the pre-state.update lookup, short-circuits when the helper says so, and (when the scanner did run state.update for a non-missing row) advances the pending row to `complete` so a later re-scan also short-circuits. The Phase-C atomic state-snapshot bundle is unchanged; only the guard above it is new. `update_and_snapshot_for_persist` replaces the inline update + serialize block. * refactor(publisher): defer complete-marking to the state-update step (Phase E) `complete` now means "SMT/MMR contain this inscription's entry", not "reveal landed on chain". The publisher (and its resumer) cannot truthfully assert that contract from outside the state lock, so both stop at `reveal_broadcast` and let the caller — the mint flow or the scanner-replay path — flip the row to `complete` after running `state.update`. Three call sites changed: * `broadcast_inscription_txs_with_persistence` — drop the post-reveal advance to `complete`. * `broadcast_reveal_and_complete` (resumer helper) — same. * `resume_single_row` `PENDING_STATUS_REVEAL_BROADCAST` arm — same. Existing assertions in `publisher_tests` that expected `complete` after a happy-path broadcast or a resume sweep are updated to pin `reveal_broadcast` instead, with comments explaining the new contract. New tests pin the Phase-E contract end-to-end: * `mint_handler_advances_state_synchronously_with_broadcast` — the publisher leg stops at `reveal_broadcast`; the caller is what advances to `complete`. * `scanner_skips_already_integrated_commit_on_replay` — a `complete` row makes `should_skip_scanner_state_update` return true, the scanner short-circuits. * `scanner_falls_back_to_state_update_for_commits_not_in_pending` — the recovery path: missing row and in-progress row both let the scanner run its own `state.update`. `resume_is_idempotent_when_called_twice` is updated to reflect the new "resumer re-broadcasts the reveal on every call to a `reveal_broadcast` row" shape (the second call no longer no-ops because the row no longer reaches `complete` inside the resumer). * feat(router): mint_handler advances state.update synchronously (Phase E) After the broadcast leg returns Ok (with PR #105's REST fallback already confirming chain landing), the handler now takes the state lock, applies the freshly-broadcast commitment via `update_and_snapshot_for_persist`, drops the lock, persists the SMT/MMR/root_index bundle via `persist_state_without_block_tx`, and finally advances the `pending_inscriptions` row to `complete`. Only then does the COMMIT phase (recipient `receive_coin` + the `commit_mint_tx` upsert bundle) run and the handler return 200. Closes the regression that motivated Phase E: a second `/api/mint` issued in the ~20-30 s scanner-observation window for the first mint walked an un-updated SMT, derived `num_pubkeys = 0` again, and surfaced `Unable to get mmr inclusion proof for the previous root` at the prover. Advancing `state.update` synchronously here closes the window — the second mint's SMT walk sees the first mint's entry immediately. The scanner becomes a redundant observer for our own inscriptions and remains the authoritative path for external / recovery inscriptions. Lock topology: * state lock acquired AFTER broadcast (broadcasting is slow and would otherwise serialise all mints behind a single inscription) * state lock released BEFORE the async `persist_state_without_block_tx` await (sync Mutex across .await is unsound) * the existing phase-2 re-derive gate handles concurrent /api/mint calls; defence-in-depth — the SMT's `"Key already exists in the tree with different value"` error is logged tolerantly here (matches the scanner-side branch). Crash-recovery: a crash between broadcast Ok and the `complete` UPDATE leaves the row at `reveal_broadcast`. The scanner-replay path on next boot walks the block, sees the row is not `complete`, runs its own `state.update`, and marks the row complete. The publisher's resumer never advances to `complete` (Phase E moved that responsibility out), so it cannot lie about the in-memory state having been updated. Persistence: uses `persist_state_without_block_tx` so the mint flow never rewinds the scanner's `latest_block` pointer back to a zero / older value. The end-to-end `mint_handler_advances_state_synchronously_with_broadcast` test pins the load-bearing observable: after `POST /api/mint` returns 200, the SMT already contains pk_0 and the pending row is `complete`. A second mint in the same scanner window would now derive `num_pubkeys = 1` and proceed against the correct root. * fix(db,router): atomic persist+mark_complete tx for Phase E mint flow Replaces the two-step persist_state_without_block_tx + update_pending_status sequence in mint_handler with a single Postgres transaction (persist_state_and_mark_complete_tx) that writes SMT, MMR, mmr_root_index AND advances pending_inscriptions.status to 'complete' atomically. The previous shape opened a crash window: a crash between the SMT/MMR/ root_index COMMIT and the standalone UPDATE to 'complete' left the row at 'reveal_broadcast' on disk. On restart, State::load_from_pg rebuilt in-memory state WITH the new leaf, but the scanner re-scanned the block, observed 'reveal_broadcast', fell through should_skip_scanner_state_update, and re-ran state.update — mmr.append appended the leaf a second time, diverging the MMR root. The atomic envelope guarantees either both the state advance and the row mark land on disk, or neither does — eliminating the duplicate-append path entirely. Also: - mint_handler now returns 503 on in-process state.update Err: the broadcast already landed on chain but the caller's mint was NOT integrated synchronously; the wallet must poll for completion and the scanner-replay path will reconcile. - mint_handler returns 503 on atomic-tx Err: durable state did not advance, the in-memory mutation will not be reflected after restart, scanner-replay heals on next sweep. - The misleading "either no-op (idempotent) or log the tolerant 'Key already exists' error" comment is replaced with a description of the new atomicity invariant. - update_pending_status retained for its scanner / publisher callers (it advances rows to reveal_broadcast). - persist_state_without_block_tx removed (no remaining callers). * test(router): atomic-tx rollback and two-mint sequential coverage Adds two end-to-end mint_handler tests in router_tests: 1. mint_handler_atomic_tx_rollback_leaves_state_and_row_consistent — the BLOCKER fix's load-bearing invariant. Installs a BEFORE-UPDATE trigger on pending_inscriptions that raises whenever status would advance to 'complete'. The trigger fires inside the atomic persist_state_and_mark_complete_tx transaction. Asserts: - handler returns 503 with the expected error body - on-disk smt_state / mmr_state / mmr_root_index all stay untouched (transaction envelope rolled back) - pending_inscriptions row stays at 'reveal_broadcast' - should_skip_scanner_state_update returns false on the row, so scanner-replay will pick the inscription up cleanly 2. mint_handler_two_sequential_mints_with_different_recipients_advance_cleanly — the concurrent-mint gap. Two mints with different recipients (different commitments, different SMT keys). Asserts: - both responses 200 - in-memory MMR leaf_count == 2 (not 3 or 4 — duplicate-append regression detector) - derive_num_pubkeys_from_smt == 2 between mints - 2 mmr_root_index entries (in-memory + on-disk) - both pending_inscriptions rows reach 'complete' The earlier mint_handler_concurrent_mint_during_proof_returns_503 covers the same-num_pubkeys race that returns 503; this new test covers the clean two-mints serialization path that the atomic tx makes safe.
2 tasks
* test(router): align post-Phase-E mint error tests
- Rename mint_commit_tx_failure_returns_503 →
mint_pending_inscriptions_persist_failure_returns_503 to reflect
post-PR-107/110 reality: dead_pool fails at the publisher's
pre-broadcast pending_inscriptions INSERT, not at commit_mint_tx.
Assertion now matches the wrapped error string.
- Add mint_commit_mint_tx_failure_returns_503 to restore branch
coverage of the post-broadcast commit_mint_tx Err path. Uses a
live Postgres testcontainer + a BEFORE INSERT trigger on the
accounts table; everything earlier in the mint flow succeeds,
only the final accounts upsert raises, and the handler returns
503 with the 'Failed to persist mint commit transaction' body.
* test(router): spawn per-connection in mint_broadcast_mock_ws
The accept loop handled one connection then slept 60s before
accepting the next, which serialised sequential mint tests behind
the previous mint's keepalive sleep. The second connect_async hit
its 15s timeout and the mint flow returned 503 'WS connect failed'.
Phase E surfaced this because it added the first multi-mint-per-
test scenario (mint_handler_two_sequential_mints_with_different_recipients_advance_cleanly).
Pre-Phase-E every test ran exactly one mint so the bug was hidden.
Fix: spawn a tokio task per accepted connection so the accept loop
keeps draining new connects in parallel with the existing 60s
keepalive.
* fix(router): add deterministic phase3_release hold for concurrent-mint test
The existing race test relied on prepare_mint (~200ms in test build)
being slow enough for the test thread to acquire the SMT lock and
insert pk_0 between phase 2 entry and the phase-3 re-derive. Under
CI load this race was lost intermittently — phase 3 ran first, saw
no pk_0, proceeded to broadcast, returned 'Failed to broadcast mint
inscription on-chain' instead of 'Concurrent mint detected'.
Fix: add a second cfg(test)-only Notify (phase3_release) that the
handler awaits between prepare_mint and the phase-3 re-derive. All
test_state constructors pre-arm it so production-shaped tests
proceed immediately. The concurrent-mint race test drains the
pre-armed permit before spawning, then notify_one()s after injecting
pk_0 — a hard happens-before edge that no timing variance can lose.
This eliminates the flake observed on develop's heavy CI run
26411938492 Coverage Gate.
* fix(router): switch phase3 hold from Notify to Mutex<()>
The previous Notify-based hold pre-armed exactly ONE permit. After the
first mint consumed it, subsequent mints in the same process (e.g.
mint_handler_two_sequential_mints_with_different_recipients_advance_cleanly)
blocked forever — the test was observed SLOW [>720s] in the heavy
gate's nextest run before timing out.
A tokio::sync::Mutex<()> is the correct primitive: pre-unlocked,
handler acquires + drops in one step for production-shaped tests
(non-blocking), and the concurrent-mint race test holds the guard
across pk_N injection then drops it. Reusable for any number of
sequential mints without manual re-arming.
* refactor(state,db): drop uncoverable 64-bit u64->i64/usize fallbacks
The three `i64::try_from(leaf_index)` sites in db.rs and the
`debug_assert!` in state.rs::load_from_pg all guard a hypothetical
> i64::MAX (or 32-bit usize-overflow) condition that cannot fire on any
target we ship (Linux x86_64 / aarch64). The error/panic arms were
flagged by the 100% Coverage Gate as uncoverable, contributing
13 uncovered lines for zero production benefit.
Replace each `try_from + sqlx::Error::Encode` with a direct `as i64`
cast (and `as usize` in state.rs), keeping a one-line invariant comment
that documents the bound. No behaviour change on 64-bit targets, which
is the only deployment target.
Affected sites:
- db::persist_state_tx
- db::persist_state_and_mark_complete_tx
- db::insert_root_index
- state::State::load_from_pg
* refactor(publisher,router): drop dead Option from create_and_broadcast return
`create_and_broadcast_inscription` returned `Result<Option<(Txid, Txid)>>`
but the body has no path that yields `Ok(None)` — every success arm
builds `Ok(Some((c, r)))` and every failure surfaces as `Err`. The dead
`Ok(None)` arm in `router::mint_handler` together with the defensive
`if let Some(ctxid) = commit_txid_bytes { .. } else { .. }` fallback
contributed 13 uncovered lines to the Coverage Gate.
Flatten the API to `Result<(Txid, Txid), ...>`:
- publisher.rs: return tuple directly on Ok.
- router.rs: match yields `[u8; 32]` instead of `Option<[u8; 32]>`;
collapse the `if let Some(ctxid) { .. } else { .. }` to a single block.
- publisher_tests.rs: drop the now-redundant `.expect("Some((c,r))")`
unwrap layer at three call sites.
No behaviour change — the removed branches were unreachable.
* test(router): cover in-process state.update Err 503 path
The phase-3b state-advance Err arm in `mint_handler` (the 503 returned
when `update_and_snapshot_for_persist` fails on an SMT
key-collision-with-different-value) was uncovered by the 100% Coverage
Gate. The race that produces this state in production needs two
concurrent mints whose phase-2 re-derives BOTH pass and whose
broadcasts both land before either state lock acquires — too brittle
to reproduce deterministically with two real requests.
Add a deterministic test that exercises the same code path:
- Mirror `phase3_release_lock` with a new `state_advance_release_lock`
test-only AppState field. The handler acquires + immediately drops
it AFTER `create_and_broadcast_inscription` returns and BEFORE the
phase-3b state lock. Production builds compile this out entirely
(both the field and the hold point are `#[cfg(test)]`).
- The new test holds the guard across a colliding-SMT injection at
`pk_0`. When the guard drops, the handler's `state.update` observes
the collision and returns 503 with the documented
"in-process state advance failed" reason. Asserts also confirm the
pending row stays at `reveal_broadcast`, the on-disk SMT/MMR/
root_index DID NOT advance (no atomic persist tx ran), and the
scanner-replay path stays armed.
) deploy_dev_node + deploy_prd_node steps hit `client_loop: send disconnect: Broken pipe` exit 255 when `docker compose recreate` runs longer than the cloudflared tunnel's idle timeout (~60s with no remote stdout). The host-side deploy script keeps running and the container comes up correctly, but CI marks the step failed. Add ServerAliveInterval=30 + ServerAliveCountMax=8 — keepalive every 30s, 8 retries before declaring dead = 4 min of network silence tolerated. The deploy script's longest silent stretch (`docker compose pull` of a fresh image + container recreate) is bounded by the host's network + disk speed, comfortably under that ceiling. Observed on the PR #111 merge auto-deploy: run 26419696840 failed with the broken-pipe pattern, but ssh dfxdev-remote showed the container was already `Up (healthy)` — purely a CI-side artefact.
This was referenced May 26, 2026
TaprootFreak
added a commit
that referenced
this pull request
May 26, 2026
…114) PRD image ships the same MVP-only binary as DEV per Dockerfile policy, but deploy-prd.yaml's api-e2e step was missing the escape-hatch env var that lets feature-gated tests (address-list, lnurl) skip cleanly instead of panicking the CI canary. PR #18 (develop→main auto-release) brought the feature-trimmed-server check from PR #105 to main, and the next PRD deploy (run 26441824314) failed with 4 panics on tests gated by features the MVP build never enables. Mirror deploy-dev.yaml's env block on the PRD api-e2e step.
5 tasks
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)