Add integration test for zcashd key import into zallet by nullcopy · Pull Request #82 · zcash/integration-tests

nullcopy · 2026-04-07T15:43:03Z

Adds zcashd_key_import.py which exercises zallet migrate-zcashd-wallet --no-scan against a zcashd wallet.dat. The test wallet in qa/rpc-tests/test-wallet/ was generated by the regtest wallet builder [1] and contains keys of every type zcashd ever supported (transparent, Sprout, Sapling, Orchard/unified) across all 9 network upgrades, plus per-phase imported foreign keys (spending keys, viewing keys, watch-only addresses).

Sprout keys are intentionally excluded (not supported by zallet).

[1] https://github.com/nullcopy/zcash-regtest-wallet-builder

Requires:

zallet built with zcashd-import feature
db_dump (Berkeley DB utility) on PATH

Makes progress towards zcash/wallet#405. All that will remain is to enable this integration test in Zallet CI.

Resolves: COR-1138
Resolves: zcash/wallet#409
Resolves: #96

Two CI failures in PR zcash#82, fixed independently: * Windows zallet build: Enabling `--features zcashd-import` for the cross-compile to `x86_64-pc-windows-gnu` pulls in `pqcrypto-mlkem` via `zewif → bc-envelope → bc-components`, whose C component `#include`s `<features.h>` (a glibc header) and so cannot be built with `x86_64-w64-mingw32-gcc`. The `zcashd_key_import.py` test that needs the feature only runs on Linux anyway (it depends on the `db_dump` Berkeley DB utility), so disable the feature on `mingw32`. * `zcashd_key_import.py` (RPC shard 9): The test passes both `--no-scan` and `--buffer-wallet-transactions` to `zallet migrate-zcashd-wallet`, but those flags currently conflict on `zcash/wallet@main`. The conflict is removed in commit `e979351` ("Use available transaction data to estimate exposure heights") on the `wip/fix/migration-scan` branch, but that has not yet landed upstream. Drop the test from `NEW_SCRIPTS` for now so it isn't auto-run by `rpc-tests.py`; the test file remains for manual invocation, and will be re-added once the upstream zallet fix merges. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

The migrate-zcashd-wallet subcommand is gated behind the zcashd-import cargo feature, which is required by zcashd_key_import.py and zcashd_key_import_db.py. Enabling `--features zcashd-import` for the cross-compile to `x86_64-pc-windows-gnu` pulls in `pqcrypto-mlkem`, whose C component cannot be built with `x86_64-w64-mingw32-gcc`: pqclean/common/compat.h:18:10: fatal error: features.h: No such file or directory The `zcashd_key_import.py` and `zcashd_key_import_db.py` tests that needs this feature only run on Linux anyway (it depends on the `db_dump` Berkeley DB utility), so the Windows zallet binary has no use for `migrate-zcashd-wallet`. Gate the feature flag on the matrix entry's `name` so it is only enabled on non-mingw targets. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

This test wallet was generated by the regtest wallet builder, spanning all 9 network upgrades (Sprout through NU6.1) with HD-derived keys, unified accounts, and per-phase imported foreign keys. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

dianaravenborne

Hermes Agent Review

Verdict: LGTM / Approve (advisory). Posted as a Comment review rather than a formal Approve because the reviewer account lacks explicit write access to zcash/integration-tests. The PR is in good shape. The two tests are well-scoped (API-level vs. strict DB-inspection), share a clean helper module, and the pure-Python ZIP-316 / F4Jumble-inverse decoder ships with a self-test that runs cleanly against the fixture (python3 ufvk_decode.py → OK). Documentation in test-wallet/README.md is thorough and the regeneration workflow is spelled out.

Findings are all documentation / cleanup nits — none block merge. Inline comments below.

💡 Suggestions (non-blocking)

test-wallet/README.md — "Imported into this repo at: 0c873b4" doesn't match any commit on this branch (the fixture-adding commit is 41190b842c…). Likely a stale pre-rebase SHA; either update or drop, since the same paragraph already requires future regenerations to fill in the builder commit.
zcashd_key_import.py — start_nodes is imported but never used; small dead import.
zcashd_migration.py — both subprocess.run calls share a single 120 s timeout via _MIGRATION_TIMEOUT_SECS; the migration of a 2.9 MB wallet.dat covering 9 NUs may eventually outgrow this on slow CI hardware. Worth keeping an eye on, or splitting init vs. migrate timeouts.
zcashd_key_import_db.py:262 — sapling-spending check uses an aggregate cardinality proxy (keystore row count ≥ expected). Comment is clear about why (no pure-Python Jubjub), but this means one phase's missing spending key could be hidden by an extra row elsewhere. Maybe a future TODO once Jubjub is available.
ufvk_decode.py:185 — F4Jumble inverse: nicely documented, but the magic constants 48/4194368 for the length bounds aren't sourced inline. A # ZIP-316 §5.2 (or equivalent) pointer would help future maintainers.

✅ Looks good

Clean separation: zcashd_migration.py (shared infra), ufvk_decode.py (pure crypto-free decoders), and the two test scripts each have a clear purpose.
The CheckReporter pattern — inline PASS/FAIL per assertion, summary + failed-list at the end — is much friendlier for triaging large key-matrix tests than a single bare assert_equal.
The CI matrix gate that disables --features zcashd-import only on mingw32 is well-justified by the commit message (pqcrypto-mlkem / features.h issue) and the inline workflow comment.
Explicit skip() for sprout and watch-only-by-hash makes the wallet-feature gaps loud rather than silent.
ufvk_decode.py self-test imports manifest fixtures so it follows fixture regenerations automatically.
Both new scripts are wired into NEW_SCRIPTS in qa/pull-tester/rpc-tests.py, consistent with project convention.

Reviewed by Hermes Agent (delegated by dianaravenborne)

ValarDragon

LGTM.

Only comment is I think were missing txs with many in / many out, and mixed cases accordingly of sendersputs/receivers there

Exercises the full migration through zallet's listaddresses JSON-RPC: zebrad + zaino + zallet RPC server are started, the regtest test-wallet fixture is migrated with `migrate-zcashd-wallet --no-scan`, and every HD-derived address from the manifest is asserted to appear in zallet's address listing. Adds the shared `test_framework/zcashd_migration.py` helper (fixture path, manifest loading, migration invocation, per-check reporter) that a follow-up DB-inspection test will also consume. Sprout keys are intentionally excluded — not supported by zallet. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Companion to the API-level test: instead of going through zallet's JSON-RPC, this reads the migrated `wallet.db` SQLite directly and asserts every expected imported key class is persisted — including classes the RPC surface hides (sapling viewing keys, standalone transparent privkeys, watch-only-by-hash transparent addresses). Adds `test_framework/ufvk_decode.py` with the pure-Python bech32m + F4Jumble-inverse + ZIP-316 typed-receiver decoders needed to match shielded FVK rows in the DB without elliptic-curve math. No gap-limit leniency and no NU6.1-tx-parseability fallback; membership is strictly required. Expected to fail loudly until two zallet bugs land (transparent watch-only-by-hash import support; sapling viewing-key migration via import_account_ufvk). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

nullcopy · 2026-05-28T21:32:57Z

Force-pushed to address review feedback

nullcopy self-assigned this Apr 7, 2026

nullcopy force-pushed the feat/zcashd-migration-test branch 10 times, most recently from 0783f67 to c56dc0b Compare April 8, 2026 15:56

nullcopy force-pushed the feat/zcashd-migration-test branch 3 times, most recently from ae490ae to 1958ba9 Compare April 8, 2026 18:33

nullcopy requested review from daira and nuttycom April 8, 2026 18:33

nullcopy marked this pull request as ready for review April 8, 2026 18:33

nullcopy force-pushed the feat/zcashd-migration-test branch 2 times, most recently from c0929f8 to a67e21b Compare April 10, 2026 20:47

nuttycom requested a review from oxarbitrage April 23, 2026 16:16

nullcopy force-pushed the feat/zcashd-migration-test branch from a67e21b to 1acc72d Compare May 1, 2026 15:01

nullcopy force-pushed the feat/zcashd-migration-test branch 4 times, most recently from 1aaf2aa to 3aa87de Compare May 19, 2026 16:00

nullcopy marked this pull request as draft May 19, 2026 19:25

nullcopy force-pushed the feat/zcashd-migration-test branch 4 times, most recently from 4a60797 to d302ab8 Compare May 20, 2026 13:37

nullcopy and others added 2 commits May 20, 2026 08:38

nullcopy force-pushed the feat/zcashd-migration-test branch from d302ab8 to 8c9c01a Compare May 20, 2026 13:38

nullcopy marked this pull request as ready for review May 20, 2026 13:39

dianaravenborne reviewed May 20, 2026

View reviewed changes

ValarDragon reviewed May 20, 2026

View reviewed changes

Comment thread qa/rpc-tests/test-wallet/manifests/phase_01_overwinter.json

ValarDragon reviewed May 20, 2026

View reviewed changes

Comment thread qa/rpc-tests/test-wallet/manifests/phase_02_sapling.json

ValarDragon reviewed May 20, 2026

View reviewed changes

Comment thread qa/rpc-tests/test_framework/ufvk_decode.py Outdated

ValarDragon reviewed May 20, 2026

View reviewed changes

nullcopy force-pushed the feat/zcashd-migration-test branch from 3ec93b8 to bdf1dc4 Compare May 28, 2026 21:28

nullcopy force-pushed the feat/zcashd-migration-test branch from f287f6e to eaa4ee7 Compare May 28, 2026 21:32

nullcopy requested a review from ValarDragon May 28, 2026 21:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add integration test for zcashd key import into zallet#82

Add integration test for zcashd key import into zallet#82
nullcopy wants to merge 4 commits into
zcash:mainfrom
nullcopy:feat/zcashd-migration-test

nullcopy commented Apr 7, 2026 •

edited

Loading

Uh oh!

dianaravenborne left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ValarDragon left a comment •

edited

Loading

Uh oh!

nullcopy commented May 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

nullcopy commented Apr 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dianaravenborne left a comment

Choose a reason for hiding this comment

Hermes Agent Review

💡 Suggestions (non-blocking)

✅ Looks good

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ValarDragon left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nullcopy commented May 28, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

nullcopy commented Apr 7, 2026 •

edited

Loading

ValarDragon left a comment •

edited

Loading