Upgrade tests#2441
Closed
zancas wants to merge 66 commits into
Closed
Conversation
…ange Follows the CryptoProvider pattern established in zaino: zingolabs/zaino@a62dcd3 rustls 0.23 auto-selects a provider only when exactly one of its ring / aws_lc_rs features is enabled. The workspace pinned rustls with the ring feature while rustls's default features already enable aws-lc-rs, so feature unification put both providers in the graph and nothing installed a process-level default - four zingo-netutils tests panicked with "Could not automatically determine the process-level CryptoProvider". Changes: - zingo-netutils gains crypto::ensure_default_crypto_provider(): a guarded, first-install-wins install of aws-lc-rs, called where the crate builds rustls configs. An embedder (e.g. zingo-mobile) that installs its own provider first keeps it. Re-exported through zingolib. - The seven scattered ad-hoc ring provider installs (LightClient constructors, sync_example_wallet, zingo-cli main, libtonode tests) are replaced with that one helper. - Workspace features: rustls drops ring and gains prefer-post-quantum (the X25519MLKEM768 hybrid group leads outbound handshakes; ML-KEM is only available on aws-lc-rs); hyper-rustls and tokio-rustls switch ring -> aws-lc-rs; tonic gains tls-aws-lc so the provider is asserted in the feature graph rather than inherited. - zingo-price: reqwest switches to rustls-tls-no-provider (never auto-selects) and installs the provider before fetching; it sits below zingo-netutils in the graph, so it carries a mirrored helper. - Real crypto use excised: the legacy BIP32 HMAC-SHA512 derivation in extended_transparent.rs moves from ring::hmac to the RustCrypto hmac + sha2 crates. The stored-wallet regression tests (verify_example_wallet_*) confirm derivation is unchanged. - The ring and rustls dependencies are dropped from zingolib, zingo-cli, and libtonode-tests; ring leaves the workspace dependency table. - zcash_proofs's download-params feature (whose minreq -> rustls 0.21 -> ring subtree is the sole ring residue) is now enabled only for the build script that fetches sapling params; the runtime dependency graph is 100% aws-lc-rs. Verified: the four previously failing zingo-netutils tests pass (including the live mainnet-indexer connect); 95/95 unit tests across zingo-netutils, zingo-price, and zingolib; cargo clippy clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit de11e16)
Flaky tests should surface as failures instead of being masked by automatic retries. - ZINGOLIB_NEXTEST_RETRIES defaults to "0" and is now a conditional [env] entry (env_not_set), matching ZINGOLIB_NEXTEST_FILTER: cargo-make otherwise clobbers inherited environment variables, so this is what makes a per-run `ZINGOLIB_NEXTEST_RETRIES=N makers test ...` override actually work when investigating a flake. - The two hardcoded `--retries 2` nextest invocations in .github/workflows/test.yaml (which bypass makers) become `--retries 0`. - `makers help` example lines updated to match. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ersions - zcash_local_net + zingo_test_vectors: git-pinned to the infrastructure repo's bump_to_NU6.3 branch (0dc4a51), replacing the stale rev pin. - ZAINO_IMAGE_TAG 0.3.1 -> 0.6.0-rc.1-no-tls (same digest published under zingodevops/zaino and zingodevops/zainod, so the docker-ci FROM line still resolves). - ZEBRA_VERSION=6.0.0-rc.0 added; not yet consumed, reserved for the zebrad provisioning wiring. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> (cherry picked from commit 0983747)
- libtonode-tests no longer pins test_lwd_zcashd; it runs the no-feature default network combo, the Core stack (zainod + zebrad). - zingolib_testutils: delete the test_zainod_zcashd and test_lwd_zcashd features; test_lwd_zebrad survives (behind the infrastructure repo's legacy-stack feature) until the Legacy stack is removed as a unit. - scenarios.rs: four network_combo cfg blocks reduced to two; stale zcashd doc comments rewritten in Validator/Indexer terms. - CONTEXT.md: seed the glossary (Network combo, Faucet; Core/Legacy stack deferred to the infrastructure repo's glossary). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Darkside is lightwalletd's darksidewalletd mode, and the bump_to_NU6.3 infrastructure gates the lightwalletd indexer behind the opt-in legacy-stack feature (scheduled for removal there). Without it the darkside suite no longer compiles; this completes the zebrad port for that suite. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The zainod+zebrad default network combo needs a zebrad binary, but the
CI image never contained one, so every test that launches a local net
died with the infrastructure's "Test executable must be set in
TEST_BINARIES_DIR" spawn panic.
Wire ZEBRA_VERSION from .env.testing-artifacts through the whole chain,
following zaino's pattern:
- docker-ci/Dockerfile: zebrad-downloader stage from
docker.io/zfnd/zebra:${ZEBRA_VERSION}; the binary is copied from
/usr/local/bin/zebrad into /usr/bin/, with a provenance LABEL.
- Makefile.toml: ZEBRA_VERSION joins the build-image --build-args and
echo output, the container_image_tag inputs (so a zebra bump rebuilds
the image), and /usr/bin/zebrad joins the container symlink list that
populates TEST_BINARIES_DIR.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`add_subtree_roots` fails only inside the CI container, with the wallet
consistently 32 sapling shards short of the server's count (1096 vs
1128); the same commit passes on the host. pepper-sync's subtree-root
fetch accepts a clean-but-premature stream end as complete, so the open
question is what ends the stream early in the container.
Add an #[ignore]d diagnostic, run manually in the environment under
investigation:
ZINGOLIB_NEXTEST_FILTER='' makers container-test -p libtonode-tests \
-E 'test(diagnose_subtree_root_stream)' --run-ignored all --no-capture
For three fresh connections it prints the sapling subtree-root count,
the chain height where the stream ended, and how it ended — then the
decisive experiment: resuming from start_index = count on the same
connection. Zero additional roots means that backend genuinely has only
that many (a lagging backend behind the zec.rocks load balancer);
roughly 32 more means the stream is being cut mid-flight in the
container's network path. Finally it reports where pepper-sync's own
fetch path lands the wallet.
On the host all connections currently report 1128 roots, clean end,
resume 0, wallet 1128.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ing it get_subtree_roots accepted a clean-but-premature stream end as complete: its retry logic only fires on DeadlineExceeded / EOF-decode errors, so a stream cut mid-flight (proxy, flow control) silently truncated the wallet's shard tree instead of surfacing. For unbounded requests (max_entries == 0, the only in-tree callers), a clean end is now trusted only after a resume pass from the current index comes back empty; a cut stream resumes where it stopped. Costs one empty round-trip on the happy path. Bounded requests keep single-pass semantics — resuming would fetch past the caller's cap. Defense-in-depth arising from the add_subtree_roots container failures (wallet 1096 vs server 1128 sapling shards); the evidence for that incident points to a stuck backend behind the zec.rocks load balancer, which no client-side change can paper over — see issue #2440 and the diagnose_subtree_root_stream diagnostic added in the previous commit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…drop obsolete shield step Two fixes discovered by the first zebrad-backed scenario runs: - Scenario defaults passed the all-at-1 ActivationHeights::default(), but zebrad rejects every NU6.x activation block whose lockbox/funding-stream config doesn't pair with the heights, stalling the chain at genesis. default_test_activation_heights() now returns the harness's regtest fixture shape (all=1, nu5=2, nu6=2, nu6_1/2=5) — the only shape its subsidy fixtures support — with nu6_3 unactivated: this branch's wallet builds against released librustzcash (NU6.2-max branch ids), so an NU6.3-active chain rejects every wallet transaction. Drop the set_nu6_3(None) override when the ironwood wallet branch lands. - zebrad_shielded_funds no longer shields before offloading: zebrad mines to Orchard natively (the "does not support shielded mining" comment was stale), so coinbase already lands in the pool and the shield found no transparent funds (InsufficientFunds(0)). fast::basic_scenario passes end-to-end on zainod + zebrad with this (launch, orchard mining, 100-block maturity, sync, sends). Exact-balance assertions calibrated for the streamless zcashd stack (e.g. 1_875_000_000 in mine_to_transparent) still need recalibration for the deferred funding stream (1/100 of subsidy from height 2). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The deferred (lockbox) funding stream in the harness fixture skims 1/100 of the block subsidy from height 2 onward, and orchard coinbase accrues from the block after NU5 activation (height 3) — both confirmed to the zatoshi by server-run failures. The zcashd-era expectations assumed full subsidy per block from height 1. - zingolib_testutils::scenarios gains DEFERRED_STREAM_SKIM, POST_STREAM_BLOCK_REWARD, FUND_OFFLOAD_AMOUNT, mined_block_rewards_total, FUNDED_FAUCET_SETUP_HEIGHT and funded_faucet_orchard_balance so expectations are derived, not magic. - Recalibrated: mine_to_transparent, mine_to_transparent_coinbase_maturity, mine_to_transparent_and_shield, mine_to_transparent_and_propose_shielding, value_transfers, filter_empty_messages, mine_to_orchard (un-ignored: its "zebrad cannot mine shielded" reason is disproven), send_orchard_back_and_forth (faucet-as-miner collects coinbase + fees). - Sapling mining is the one true zebrad gap: send_mined_sapling_to_orchard becomes send_mined_orchard_to_orchard (the spend-mined-funds state transition it exercises is pool-agnostic); unfunded_mobileclient mines transparent (no client holds the mining key); mine_to_sapling (pre-existing ignore, reason corrected) and send_heartwood_sapling_funds (also needs Canopy above 1, which the config writer refuses) stay ignored. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…tions The fast-set run showed two failure mechanisms beyond the funding-stream arithmetic (which mine_to_transparent_and_shield/propose confirmed): 1. Sync race: bare sync_and_await reaches whatever the Indexer has ingested, which lags the Validator right after generate_blocks — mine_to_transparent observed one block's coinbase at a height-3 tip. New scenarios::sync_client_to_validator_tip loops sync until the client fully scans the Validator's tip; every scenario-setup sync and the failing tests' post-generate syncs now use it. 2. Receiver selection: with an Orchard miner, block 1 (pre-NU5) pays the miner's SAPLING receiver the full pre-stream subsidy (observed s_balance: 625000000). Orchard-scenario expectations now include BLOCK_ONE_SAPLING_COINBASE. Model constants are hypothesis-encoded so the next run adjudicates each with a distinct delta: ORCHARD_COINBASE_START_HEIGHT=2 (off-by-one POST_STREAM_BLOCK_REWARD means flip to 3), fee-returns-to-miner (10_000 deltas), and a new fast::orchard_miner_coinbase_distribution probe asserts the full o/s/t distribution at a known tip. Also: sync_all_epochs_from_heartwood -> sync_all_expressible_epochs (canopy=3 and staggered NU heights are inexpressible: the config writer requires Canopy at 1 and the subsidy fixtures pair only with the fixture shape); unified_address_discovery syncs to tip before asserting discovery (same race). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Shielded coinbase has no 100-block maturity rule (transparent-only); the server run proved it: value_transfers spends orchard coinbase notes 1-3 blocks old. The 100-block generation in zebrad_shielded_funds was a relic of the old shield-transparent-coinbase flow, costing every funded scenario minutes of mining plus a 100-block wallet sync. Funded setup now ends at height 4 (launch + 2 setup + offload confirm). Every derived expectation recomputes through FUNDED_FAUCET_SETUP_HEIGHT. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
All ten funded scenarios failed at the offload with zebra's "rejected from the mempool until the next chain tip block ... could not validate orchard proof" once the maturity dance was removed: at a height-3 tip the offload had to spend the tip block's own note. Ordinary sends of blocks-old notes pass (value_transfers), so the rule being tripped is specifically about the tip block. zebrad_shielded_funds now syncs first and mines one block before the spend, so the wallet's selectable notes and anchor sit at least one block behind zebra's tip. Funded setup ends at height 5; expectations recompute through FUNDED_FAUCET_SETUP_HEIGHT. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…adder The tip-separation attempt refuted itself informatively: holding the wallet one block behind made zebra reject the offload with "incorrect consensus branch id" — the wallet signs for its-tip+1, and the fixture ladder activates NU6.1/NU6.2 at height 5, exactly where the streamlined setup operates. Tests that sync to the real tip before sending (value_transfers) never hit this. zebrad_shielded_funds now mines two extra blocks first (tip 5, clearing the ladder so tip+1 and the inclusion block share the NU6.2 branch id), syncs to the real tip, then offloads — with four pre-tip notes so oldest-first selection never touches the tip block's note (the original "could not validate orchard proof" rejection). Funded setup ends at height 6. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…IGHT The height-6 funded setup shifted every absolute block height in tests built on faucet_funded_recipient_default by +3 (funding tx confirms at setup+2, observed as "Confirmed at 8"). Rewrite the literals in received_tx_status_pending_to_confirmed_with_mempool_monitor and send_to_transparent_and_sapling_maintain_balance as offsets from scenarios::FUNDED_FAUCET_SETUP_HEIGHT so future setup-height changes recompute instead of breaking. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Sends in one transaction, then mines one block at a time — waiting for
the sender's wallet to reach each new height — until the transaction is
confirmed. Owns both send hazards discovered on the zainod+zebrad
stack, so callers need no choreography:
- indexer tip-lag (generate_blocks returns on the validator's height;
the indexer's poll-based ingestion lags 100-500ms; a sync begun in
that window silently misses the newest block);
- zebra's tip-block spend rejection ("rejected from the mempool until
the next chain tip block"): on exactly that error, one separation
block is mined and the send retried once, mirroring the
zebrad_shielded_funds fix.
Confirmation is asserted (up to five blocks of tolerance), so a
transaction that misses its block for any new reason fails at the send
site with its txids, not downstream at an unrelated balance assert.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Root cause of the historical unified_address_discovery failure, established by direct measurement: after generate_blocks returns, the validator has the new block but the indexer's poll-based ingestion reports the old tip for 100-500ms (10/10 rounds, max ~490ms, zainod 0.4.3 and 0.6.0 alike). A wallet sync begun inside that window completes against the stale tip and silently misses the newest block. Exonerated with evidence along the way: discovery bookkeeping (works when the block is visible), zebra's block templates (the send IS in the next block), zainod 0.6.0 specifically, and generate_blocks height accounting. Deliberately RED until the harness (or its callers) wait for indexer convergence after block generation; the assertion message carries the mechanism. Until then, tests must use the wallet-height-aware helpers (increase_height_and_wait_for_client, sync_client_to_validator_tip, send_and_bump). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Ten sites used bare generate_blocks + sync_and_await (or quick_send + generate_blocks(1)), each a latent indexer tip-lag or tip-block-spend flake with retries at 0: - unified_address_discovery: setup mining via increase_height_and_wait_for_client; the four-output send via send_and_bump (confirmation now asserted at the send site). - Three LibtonodeEnvironment sites (messages_*/value_transfers): bare recipient sync after increase_chain_height (which waits only on the validator) becomes sync_client_to_validator_tip. - by_address_finsight: three quick_send + mine + sync triplets become send_and_bump. - send_and_sync_with_multiple_notes_no_panic: three generate_n_blocks + dual-sync groups become increase_height_and_wait_for_client for the first client and sync_client_to_validator_tip for the second. Left alone, deliberately: second-client syncs that follow a wait-for-client on another client (ordering makes them safe — the indexer demonstrably has the block once any wallet synced to it), pure mempool-pickup syncs after unmined sends, and propose-time generate-without-sync sites whose semantics depend on the wallet NOT seeing the new blocks. Verified: unified_address_discovery green through send_and_bump on the host stack that previously failed deterministically. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…gfr8) Clears the runtime copy flagged by the high-severity Dependabot alert (panic on malformed CRL BIT STRING). Defense-in-depth only: no zls code configures CRLs or revocation checking, so the vulnerable parse is unreachable — there is no input channel that delivers a CRL to webpki. The second locked copy (0.101.7) cannot be bumped: it belongs to the build-time-only zcash_proofs -> minreq -> rustls 0.21 subtree (pinned to webpki ^0.101, no patched release in that line) and goes away only when upstream drops minreq or we drop download-params. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
ensure_taddrs_from_old_seeds_work and address_generation_deterministic_and_coherent assert key derivations against fixed vectors — nothing they check touches a chain, yet each spent ~12s launching zebrad+zainod for scaffolding it never used. They are now zingolib unit tests against LightWallet directly (21ms total). The one behavioral subtlety is made explicit: the original "next address index is 2" came from the scenario ClientBuilder generating an extra sapling-only address after construction, not from LightClient itself — the unit test reproduces that step with a comment so the vector-pinned indices and their diversified derivations are unchanged. Two fewer LocalNet launches per fast-suite run. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
15 chain_generics tests failed at fund_client_orchard's bare sync_and_await: the Indexer lags the Validator after generate_blocks (the new diagnostic measures up to ~515ms), so the faucet signs from a stale tip and zebra rejects the send near the NU6.1/NU6.2 activation boundary. ConductChain gains sync_client_to_tip with a bare-sync default (darkside unchanged); the generic funding flow calls it, and LibtonodeEnvironment overrides it with scenarios::sync_client_to_validator_tip — the one shared deterministic-sync implementation. Also: verify_old_wallet_uses_server_height_in_send's stale height literal (8, pre-dating the funded-setup change) is now derived from FUNDED_FAUCET_SETUP_HEIGHT. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
filter_empty_messages and message_thread assert message semantics — empty-memo filtering, address filtering, and message/value-transfer ordering — which are pure summary derivation over wallet transaction records. As integration tests they cost 49s and 132s of LocalNet mining and syncing (message_thread alone set the fast suite's wall-clock); as zingolib unit tests they run in 54ms combined. pepper-sync gains three test-features constructors (WalletNote, OutgoingNote, and WalletTransaction::new_for_test_with_orchard_notes) so tests can fabricate received/sent transaction records — including the recipient_full_unified_address that sent-side filtering matches against — without a chain. The unit tests reproduce the integration tests' exact shapes: two empty memos then Hello+empty for filtering; the five-transaction Alice/Bob/Charlie thread with "Reply to" memos for counting (bob 3, charlie 2) and bidirectional height ordering. Two fewer LocalNet launches and ~180s less node time per fast-suite run; the fast set drops from 18 to 16 tests with coverage relocated, not removed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The test asserts the absence of a lag that is known to exist (measured 10/10 rounds on zainod 0.4.3 and 0.6.0 alike), so in a normal run it only restates a documented harness limitation. Keep it as an on-demand probe (cargo nextest run --run-ignored) to re-measure after a zcash_local_net indexer-convergence barrier lands. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The test was marked "temporary for infrastructure integration": it ran faucet_funded_recipient_default and asserted nothing. During the zebrad port it earned its keep as the canonical probe — it was the first test to surface the crypto-provider panic, the heights/subsidy mismatch, and the funding-flow rework. All of that is now permanently covered: every funded test exercises the same setup path and then asserts on it, and orchard_miner_coinbase_distribution asserts the coinbase model explicitly. What remained was ~70s of assertion-free LocalNet runtime per run and one extra exposure to the launch port flake. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…et suite create_send_to_self_with_zfz_active asserts value-transfer KIND classification: a self-send yields SendToSelf(Basic), and the Zennies-for-Zingo output yields Sent(Send) addressed to the ZFZ address. Both are pure summary derivation over a transaction record — one fabricated SendToSelf transaction whose outgoing orchard notes are (a) a note to the wallet's own orchard receiver and (b) a note carrying the decoded ZFZ regtest unified address. 72s of LocalNet becomes 14ms. Deliberately NOT claimed by this port: the propose_send_all -> send_stored_proposal execution the integration test drove incidentally; that pipeline remains covered by the chain-bound send tests (send_to_tex and the funded-send family). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
simpool_insufficient_1_sapling_to_orchard flaked with "status regression to Transmitted" in the with_assertions confirmation poll. It cannot be a regression: pepper-sync's update_status is monotonic. An observed Transmitted means the wallet has not yet seen the tx anywhere — under instant regtest mining a tx is often mined before the mempool monitor observes it (status path Transmitted -> Confirmed, skipping Mempool), and the check can land in the measured ~500ms Indexer-lag window before the confirming block is scanned. Treat Transmitted like Mempool: keep polling. The existing patience bound converts a persistent Transmitted (a genuine broadcast failure) into a test failure. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…Net suite spendable_balance_includes_notes_in_incomplete_shards asserts that a confirmed, positioned note whose block has no completed orchard shard still counts as spendable — spendable-balance composition over wallet state, no chain required. 70s of LocalNet becomes 17ms, and the port is stricter than the original: the integration test only obtained an incomplete shard incidentally from regtest's tiny tree, while the unit test constructs the condition explicitly (empty orchard shard ranges, fully-scanned birthday..tip, sapling tip checkpoint for the anchor). Rig additions: pepper-sync's WalletNote::new_for_test gains a position parameter, and SyncState::new_for_test (test-features) builds sync state from explicit scan ranges — unlocking future spendability and witness-gating tests without a chain. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The two chain-generics proptests ran a single random-valued case each
(Config::with_cases(1)), and their parameter ranges reduced to a
sapling-only source with a transparent-or-sapling receiver. Coverage
therefore differed between runs, failures were not reproducible without
the seed, and the orchard rows that the "any source, any receiver" name
promised never ran at all.
The pool_matrix module enumerates every source and receiver pair with
fixed values, once with change and once without, plus minimum-value and
boundary-value entries on the sapling-to-transparent pair: fourteen
named tests over the unchanged fixture, parallelized by nextest where
the proptests serialized their work inside two tests.
The new coverage found a real defect on its first run. Both
orchard-source rows are rejected by zebra's mempool with "could not
validate orchard proof ... until the next chain tip block" during the
FUNDING send, while the equivalent sapling rows pass. Two fixes were
attempted and rejected: separating the spend from the tip inside the
fixture is undone by the assured-send helper's own pre-propose sync,
and mining a separation block inside the helper without re-syncing
makes the wallet build for the wrong target height whenever the next
block crosses an activation boundary ("transaction uses an incorrect
consensus branch id" at the height-five co-activation these tests
straddle). The orchard rows are left red deliberately: they document
the defect the matrix exists to catch, and the candidate fixes (a
minimum-confirmations wallet policy for tests, or a zebra-side answer
to why orchard proofs anchored at the tip are rejected where sapling
spends are not) deserve their own change.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Since the deterministic pool matrix replaced the proptest module, nothing in libtonode-tests references the proptest crate. The optional dependency, its feature mapping, and the cargo-shear ignore entry are removed. The proptests feature itself survives as a pure cfg gate for the pool_matrix module, so no CI or makers filter that names it breaks. The workspace-level proptest dependency remains for darkside-tests, which still runs a live proptest block in its chain-generics module. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
t_incoming_t_outgoing_disallowed, sapling_incoming_sapling_outgoing, and sapling_to_sapling_scan_together assert wallet-record and policy logic: summary contents, balance composition, spend-status views, and the shield-first refusal of transparent spends. Each now runs in milliseconds against fabricated records. The transparent policy test lives in zingolib/src/lightclient/send.rs on the builder's new transparent_coin extension; the two sapling tests live in zingolib/src/wallet/summary.rs on the record rig, walking a note through received, pending-spent, and confirmed-spent states by mutating fabricated records between assertion blocks. One deliberate reduction: the scan_together port does not mark the funding note spent, because summary derivation validates spend links against the spending transaction's actual bundle nullifiers, which fabricated empty-bundle transactions cannot satisfy — and the original never asserted spend status. The companion test covers spend-status views through interfaces that read the records directly. Rig additions: TransparentCoin::new_for_test plus outgoing-sapling and transparent-coin attachments on WalletTransaction in pepper-sync, and SyntheticWalletBuilder::transparent_coin, which completes the builder's coverage of all three pools. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The slow module's OutgoingNoteInterface import lost its last user when sapling_incoming_sapling_outgoing moved to zingolib. cargo fix missed it because the warning only surfaces in the test profile the container builds. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The tip_spend_rejection suite formats the root-cause search for the
pool_matrix orchard-row failures as two rounds of single-predicate
experimental cells, each varying one factor with the hypothesis table
and observed verdicts recorded in the module documentation.
Round one (tall funded-faucet chain, single non-coinbase orchard note,
note depth and output pool varied independently, anchor held at the
tip): all four cells ACCEPTED, falsifying the tip-note, tip-anchor, and
orchard-output hypotheses in that environment. Round two (the exact
chain-generics environment: short chain near the height-5 NU6.1/6.2
co-activation, miner faucet holding only young orchard coinbase notes):
an orchard-output send is REJECTED ("could not validate orchard proof
... until the next chain tip block"), the identical sapling-output send
is ACCEPTED — eliminating note selection for good, since sender,
amounts, and funds are identical — and ten extra blocks cure the
orchard rejection, eliminating coinbase youth.
Conclusion: zebra rejects orchard-OUTPUT transactions built adjacent to
the activation boundary and accepts sapling equivalents; distance from
the boundary is the operative variable. Accordingly, the simpool
fixture now mines five blocks past the boundary before funding, and
every pool_matrix row passes, including the two formerly-red orchard
rows. The zebra-side mechanism of the differential remains open for an
upstream issue.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…t rule The pool_matrix orchard-source cases failed at broadcast with "could not validate orchard proof": fund_client_orchard confirms the sender's note at the chain tip, and the send fixtures proposed immediately, so the spend consumed a tip-block note — the same zebra mempool constraint the scenario offload hit earlier. The shared assertion helpers (assure_propose_send_bump_sync_all_recipients and assure_propose_shield_bump_sync) now mine one separation block and sync the sender to the real tip through the ConductChain seam before proposing, so every fixture inherits the fix, including chained sends where each round's change note would otherwise sit in the tip block. sapling_sends_to_transparent_minimum_value failed with "transaction is dust": zcashd's regtest relayed non-standard transactions, so the case could send one zatoshi; zebra enforces its mempool dust rule everywhere. The minimum transparent receiver value is now the canonical 546-zatoshi dust threshold, with the divergence documented at the case definition. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
send_to_transparent_and_sapling_maintain_balance is kept deliberately as the one end-to-end summary-shape test, so its fixtures must track what the real send pipeline produces. The second-wave funding receipt's fee is paid by the faucet, whose note pool is fragmented by the earlier waves under the Core-stack funding model; with smallest-first note selection the send is a four-logical-action transaction, so the observed zip317 fee is 20_000 where the monolithic zcashd-era funding produced 10_000. The summary-equality helper already ignores txids, datetimes, and note output indices, so the fee was the only real mismatch in the observed failure. The remaining wave fixtures carry fees shaped by the same flow; the next server run adjudicates whether any of them drifted as well. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ress mine_to_sapling asserted balances from mining into the sapling pool. The zcash_local_net zebrad validator supports only Orchard and Transparent miner pools and panics on a Sapling request, and zebra has no plans to grow sapling mining; the test had been ignored on exactly that ground and could never run again. Orchard mining balance coverage lives on in mine_to_orchard. send_heartwood_sapling_funds was doubly inexpressible: it also mined to sapling, and its custom activation ladder placed Canopy at height 3, which the harness's zebrad config writer refuses because the emitted regtest config hardcodes Canopy at height 1. sync_all_epochs (already ignored as hanging when syncing pre-sapling) staggered every upgrade from Sapling at height 3 through NU6.2 at height 17 — a shape the config writer cannot express for the same Canopy constraint, and one the harness's subsidy fixtures could not fund. Its purpose survives in sync_all_expressible_epochs, which crosses every epoch boundary the Core stack can represent. All three premises are Legacy-stack-shaped: they describe chains only zcashd could produce. Deleting them records that judgment rather than leaving ignores that imply future work. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Both the fast and slow modules imported ActivationHeights solely for the tests deleted in the previous commit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Nothing in this repository consumes the zcashd or zcash-cli binaries anymore. The zebrad port removed every zcashd-backed network combo from the test suites, and the last tests whose premises required a zcashd-shaped chain were deleted in the preceding commit. Unlike lightwalletd, which the darkside suite still launches, zcashd's provisioning was pure dead weight: an image-build stage pulling the zodlinc/zcash image, two binary copies, a version label, a build argument, two symlinks, and a version pin that participated in the container image tag. This removes the zcashd downloader stage, ARGs, LABEL, and COPY lines from docker-ci/Dockerfile, the ZCASH_VERSION tag-string component, echo, build argument, and the zcashd/zcash-cli symlinks from Makefile.toml, and the ZCASH_VERSION pin from .env.testing-artifacts. The image tag therefore changes, forcing a one-time CI image rebuild that also picks up the zainod 0.6.0-rc.1-no-tls bump. lightwalletd remains provisioned until the darkside suite has a zaino-based replacement or is retired — the tracked precondition for removing the remainder of the Legacy stack. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Until now no test separated the indexer's mempool view from the validator's; the nearest discriminator was the block-height tip-lag probe. This suite uses zainod's send_transaction as the validator-side event marker (it relays zebra's own acceptance verdict, so quick_send resolving Ok means the transaction is in zebra's mempool) and measures the two remaining legs with one cell each. Verdicts, recorded in the module documentation: zainod's GetMempoolTx shows the transaction about fifty MILLISECONDS after zebra accepts it, falsifying the earlier attribution of multi-second waits to the indexer's polling cadence (the claim in the poll-not-sleep commit message is corrected here). The sender's wallet record reaches Mempool status at the exact microsecond sync_and_await returns, and never reaches it without a sync session, because pepper-sync's mempool monitor exists only inside a sync session's lifecycle. The seconds always belonged to the sync session itself, and the assured-send helper's old fixed sleep was pure waste. The rejection-side attribution cells for the boundary-adjacent orchard-output phenomenon are designed in the module documentation and blocked on two named seams: an rpc_listen_port accessor on the running Zebrad (infrastructure side) for direct-submission verdict parity and same-bytes resubmission, and a build-without-broadcast test-features seam in zingolib's send path for offline orchard-proof verification. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The GitHub workflows still symlinked zcashd and zcash-cli out of the CI image and omitted zainod and zebrad, which the container-test flow has provided via Makefile.toml since the zebrad port. Once the image rebuilds without zcashd those lines would create dangling symlinks. The workflows now link exactly the surviving binary set: lightwalletd (for darkside), zainod, and zebrad. The docker-ci README's version-bump note follows suit. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ra rev Advances the zcash_local_net and zingo_test_vectors pins to infrastructure 34fce91, which allocates harness listener ports from a fixed band below every default ephemeral range (partitioned per process and bind-checked) and adds LocalNet::await_indexer_convergence plus generate_blocks_converged. The port change removes the launch-time RpcReadinessTimeout flake at its root: ports picked from the kernel's ephemeral range could be reused as outgoing source ports in the window between picking and the child's bind. The convergence barrier gives callers a way to wait for the Indexer to serve the Validator's tip instead of compensating wallet-side. The scenario helpers' adoption of the barrier is in flight in a parallel session; this pin unblocks it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The infrastructure rev at 34fce91 (pinned in the previous commit) brings LocalNet::await_indexer_convergence and generate_blocks_converged (infra commit fa2da0b) — the harness-level retirement of the indexer tip-lag race this repo measured at 100-500ms in ten of ten rounds and has been closing wallet-side ever since. The barrier is defined only for zainod, whose "Syncing block" log line is the observation channel, so the scenario layer gains a small IndexerConvergence dispatch trait: the zainod implementation calls the harness barrier, and the lightwalletd implementation is a no-op behind the test_lwd_zebrad feature, where the callers' wallet-side height polling remains the functional guarantee. The two wait helpers (increase_height_and_wait_for_client, sync_client_to_validator_tip) converge the indexer before syncing, so their height polls complete on the first pass instead of spinning against a lagging indexer; the bound propagates to send_and_bump, zebrad_shielded_funds, and send_value_between_clients_and_sync. The tip_spend_rejection module doc also restructures two footnotes that tripped clippy's doc-list-indentation lint. Verified on both feature combos plus clippy, and live on the host: unified_address_discovery drops from about 75 to 38 seconds, and the mempool attribution cell passes unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The ignored diagnostic indexer_tip_lags_validator_after_block_generation existed to document that raw generate_blocks returns before the Indexer serves the new tip (measured 10/10 rounds, max 475ms). With zcash_local_net's await_indexer_convergence landed and the scenario helpers routing through it, the meaningful assertion inverts: the test is renamed indexer_converges_with_validator_after_block_generation, loses its ignore, and asserts that after the barrier the Indexer's served tip matches the Validator's in every one of ten rounds. If the barrier regresses, this is the test that goes red. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`makers test all` previously gated its three phases, stopping at the
first failure — fast feedback, but a red run reported only the phase
that broke, and three separate nextest summaries scrolled past with no
combined accounting. Zaino solved the same problem with a small Rust
runner (tools/test-runner's live-summary), and this adopts that design:
a workbench binary, test-summary, runs the packages, zingo-cli, and
libtonode phases in order — all of them, even when an earlier phase
fails, so the summary reflects the whole suite — streams each phase's
output live while capturing it, parses each nextest Summary line with
ANSI stripped, and prints a per-phase table with totals:
====================== test summary ==========================
packages: 107 run, 107 passed, 0 failed, 0 skipped
zingo-cli: 77 run, 77 passed, 0 failed, 0 skipped
libtonode: 65 run, 64 passed, 1 failed, 0 skipped
TOTAL: 249 run, 248 passed, 1 failed, 0 skipped
==============================================================
A phase that exits non-zero without producing a summary line is called
out as a probable build failure so its zero row is not read as clean,
and the front door exits non-zero if any phase failed. The runner sets
the empty ZINGOLIB_NEXTEST_FILTER itself, preserving `all`'s
slow-tests-included semantics. The parser carries zaino's unit tests
forward; the set-validation error path was exercised directly.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The running Zebrad already exposes rpc_listen_port via the copy_getters macro; the module doc claimed an infrastructure accessor was missing because a grep for literal pub fn missed the macro-generated getter. The only remaining blocker for the rejection-side attribution cells is the zingolib build-without-broadcast seam. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Restore the stop-on-failure behavior the front door had before the summary runner: a failing packages or zingo-cli phase stops the later, more expensive phases from launching, so a broken unit test never costs a libtonode run. The combined summary table still prints, with the phases a failure prevented from running marked "not run (an earlier phase failed)" so the accounting stays honest about what executed. This is a deliberate divergence from zaino's runner, which executes all partitions unconditionally; the doc comment records it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
tracing_error_from_pepper_sync_goes_to_log_file spent a fixed twelve seconds per run, and the diagnosis found the comment blaming the wrong mechanism. The test spawns its mock gRPC server on the test's own tokio runtime, whose default flavor is current-thread; the fixed std::thread::sleep then blocked that single thread, so the server was never polled, the child's get_latest_block hung unanswered, and the logged ERROR was the child's ten-second client-side transport timeout — not the mock's configured DEADLINE_EXCEEDED, which never got to be served. The multi_thread runtime flavor lets the mock actually answer, the error lands sub-second, and an event-driven poll on the log file (with a twenty-second ceiling) replaces the fixed sleep. The assertions are unchanged; the guarantee — a pepper_sync fetch ERROR reaches the log file and not stderr — is what it always was. interactive_mode_redirects_tracing_to_log_file had the same fixed-interval shape without the runtime bug; it now polls for the startup INFO lines the assertion needs. The pair drops from about fifteen seconds to about two. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Round three of the discrimination suite runs the decisive same-bytes experiments. No new wallet seam was needed: transmission failure leaves the built Transaction in the wallet's Failed record, so the exact bytes zebra judged are recoverable, and the running Zebrad already exposes its JSON-RPC port. zingolib_testutils gains a validator_rpc module (sendrawtransaction and getrawmempool probes that bypass the indexer, where a mempool rejection is data rather than an error), and the mempool-lag cell upgrades its acceptance-marker inference to a direct getrawmempool observation. The verdicts, pinned by boundary_rejection_attribution: direct submission at the boundary reproduces the rejection, so zainod relays zebra's verdict faithfully; and the IDENTICAL bytes resubmitted after five blocks of distance are rejected as "transaction uses an incorrect consensus branch id". The wallet builds boundary-adjacent transactions under the pre-activation branch id — zebra judged correctly at both heights, and its transient "until the next chain tip block" framing (plus the orchard-proof surface of a wrong-branch-id context) was the only misdirection. The culprit is the wallet-side builder's target-height and branch-id selection when the next block is an activation block: a zingolib/librustzcash concern, not a zebra or zainod one. The residual open question, recorded in the module doc, is why the sapling-output twin built at the same height carries a valid branch id. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The portability sweep's A and B verdicts, executed. Three summary-logic tests move to zingolib/src/wallet/summary.rs on the record rig: send_funds_to_all_pools (per-pool balance aggregation over one confirmed note per pool), by_address_finsight (memo-bytes-per-address over outgoing notes), and value_transfers (four-memo aggregation into one transfer plus idempotence and sort stability, renamed value_transfers_aggregation_and_ordering). Four proposal-shape tests move to a new proposal_shape module in zingolib/src/lightclient/propose.rs: shield_transparent (formerly ignored in libtonode and asserting nothing; the offline proposal now asserts the step, input, and change shape), four_coin_shield_proposal_shape (from mine_to_transparent_and_propose_shielding — all five original assertions were proposal-shape; fabricated coins propose identically to mined ones since pepper-sync carries no coinbase marker), and the load-bearing halves of the chain_generics ignore_dust_inputs and note_selection_order fixtures (dust exclusion and minimal-change covering selection, asserted directly on the proposal's selected inputs instead of on confirmed records). The two fixtures' remaining lifecycle wrappers restated what the pool_matrix family already covers, so the fixtures and their instantiations are deleted along with the four concrete.rs originals and shield_transparent.rs. The shield ports exposed a builder bug worth having found: fabricated transparent coins carried an empty script, and WalletTransparentOutput::from_parts silently drops coins whose script does not parse back to an address, so shield proposals saw no funds. TransparentCoin::new_for_test now demands a script and the builder derives the real P2PKH script from the wallet's own address. One analysis correction is recorded rather than executed: verify_old_wallet_uses_server_height_in_send was assessed as portable via a mock indexer, but the send path never fetches server height — transaction creation reads the wallet's own sync state through WalletRead. Its remaining protection is that zebra accepts a broadcast from a wallet two blocks behind the tip, which is inherently e2e, so it stays in libtonode. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The attribution triangle measured a ~2.5 second floor on sync_and_await against a ten-block regtest chain, reproducible to the millisecond — deterministic structure, not workload. Instrumenting the session found the composition: the engine's own work ends at ~1.4s, after which the caller paid a full second for the mempool monitor to notice the shutdown flag on its one-second tick (sync() joins that task), plus up to half a second of completion-detection quantization in await_sync's 500ms poll. Three changes, none touching what is checked — only how often: the scanner-shutdown wait polls the drain condition at 25ms with the old one-second ceiling as worst case instead of sleeping the second unconditionally; the mempool monitor's shutdown tick drops from one second to 50ms (it only bounds flag-notice latency, and it sits on every session's join path); and await_sync polls at 50ms instead of 500ms. Measured: sync_and_await on the attribution cell drops from 2.502s to 1.302s, and unified_address_discovery from 38 to 31 seconds (75s two days ago). Tests average five to seven syncs each, so the saving compounds across the whole live suite. pepper-sync and zingolib unit suites pass (114/114). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Every libtonode and zingo-cli test launches a private zebrad and zainod pair, and unbounded parallelism oversubscribes the container: measured stretch on individual LocalNet tests is five to seven times their solo wall-clock once roughly forty node pairs contend for cores. A nextest test group now caps those two packages at eight concurrent tests, on both the default and ci profiles; unit tests keep full parallelism. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
cargo silently ignores --package when --workspace is also present, so `makers test packages -p FOO` ran the entire packages set while appearing to scope to one package. Rather than patch the merge logic, this rebuilds the front door on cargo's own semantics. `makers test "$@"` is now a containerized `cargo nextest run "$@"`: package selections, positional test-name filters, and arbitrary flags all pass through untouched. Two reserved first words remain as canned invocations: `packages` (the workspace minus the live packages) and `live` (only libtonode-tests and zingo-cli). Explicit package-selection flags replace a reserved word's scope entirely, because merging them is exactly the trap the silent-ignore behavior sets. The `libtonode`, `zingo-cli`, and `all` words are removed; the first two are spelled `-p`, and the three-phase gated suite is invoked directly as `cargo run -q --manifest-path tools/workbench/Cargo.toml --bin test-summary`, whose phases now select their scope with `-p` instead of the removed reserved words. The default `not test(slow)` filter is removed, along with the whole ZINGOLIB_NEXTEST_FILTER override mechanism it existed to serve; filtering is spelled `-E '...'` per nextest idiom, so local runs now include the slow:: tests by default. PR CI is unaffected because ci-pr.yaml already passes its filter explicitly. The README testing section is updated to match, including its stale claim of two default retries (the default is zero). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…pend observations A note's spending_transaction field is the wallet's only durable record of an on-chain spend. Spend detection consumes the corresponding nullifier-map entry, cleanup prunes the map behind the fully-scanned frontier, and scanned ranges are never revisited, so once a spending transaction's block has been scanned the field cannot be reconstructed by any forward sync. set_transactions_failed nevertheless permitted the Confirmed -> Failed transition and unconditionally reset those fields, converting spent notes into permanent phantom unspent notes: every subsequent proposal selected them and was rejected by the validator as an orchard double-spend. This failure mode was observed in the wild as a duplicate nullifier rejection reporting "in finalized state: true". set_transactions_failed now refuses txids whose record is Confirmed and logs a warning naming the refused txid. A mined transaction cannot fail; the only protocol event that un-mines it is a reorg, and the reorg path retains the old behavior through the new pub(crate) set_transactions_failed_unchecked, which is sound there because truncation simultaneously reopens the affected scan ranges and thereby guarantees re-detection. The damage was permanent exactly when sync had raced ahead of the spending transaction, which is exactly when its record is Confirmed, so the guard is coextensive with the harm. Three new tests in sync::test::spend_reset_lifecycle pin the lifecycle: failing_a_confirmed_transaction_must_not_destroy_the_spend_observation asserts the enforced invariant, reset_before_scan_heals_on_spend_detection pins the self-healing pre-scan sequence, and scanned_transaction_overwrites_failed_record pins the record replacement that completes the healing. In support, the test-features constructor gates in wallet.rs widen to any(test, feature) so in-crate unit tests can use them, and the mock wallet implements the previously todo!() SyncNullifiers::get_nullifiers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add the term for a note the wallet offers as spendable although its nullifier is already on chain, and distinguish it from a pending-spent note, whose spend the wallet knows about and correctly excludes from selection. The term names the failure mode fixed in pepper-sync's set_transactions_failed guard and keeps the discussion from overloading "stale" or "stuck". Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
`makers test` became a flat nextest passthrough, so a failing unit test
no longer prevents the expensive live suites from running; that gating
lives only in the workbench test-summary runner, which lost its makers
front door when the `all` reserved word was removed. This restores a
front door: `makers hierarchy-test` is a pure delegation to
`cargo run -q --manifest-path tools/workbench/Cargo.toml --bin
test-summary`, forwarding extra args with cargo-make's `${@}` splat.
The help text now names the task instead of spelling out the cargo
invocation, so the incantation is written in exactly one place.
The runner itself now rejects cargo package-selection args (exit 2)
before launching any phase. Forwarded args reach every phase's nextest
invocation, where a package selection would silently replace all three
phase scopes with the same one — the same class of silent mis-scoping
the passthrough rework removed from `makers test`. The new
is_package_selection_arg mirrors the has_package_selection_args case
list in Makefile.toml's base-script, and unit tests pin both the
rejected flag forms and the ordinary nextest args that must pass.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Fix silent spend bug
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.
Numerous upgrades to the test harness:
To run invoke:
makers help <-- for help
makers test <-- for all the (containerized with podman) tests
makers hierarchy-test <-- tests from fastest to slowest stopped at the first failing category