Skip to content

fix(rpc): rescan already-scanned range when z_importkey re-imports a known key with rescan=yes#581

Open
oxarbitrage wants to merge 1 commit into
mainfrom
import-rescan-rewind-578
Open

fix(rpc): rescan already-scanned range when z_importkey re-imports a known key with rescan=yes#581
oxarbitrage wants to merge 1 commit into
mainfrom
import-rescan-rewind-578

Conversation

@oxarbitrage

@oxarbitrage oxarbitrage commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Closes #578.

Follow-up to #579 (merged), which made key imports scan going forward without a restart. #578 is the other half: re-importing an already-known key with rescan="yes" did nothing (z_importkey short-circuits on is_new_key). New keys are already covered by the backend's add_account re-queue; this wires up the existing-key path.

What changed (one file, import_key.rs): when the key is already known and rescan="yes", truncate_to_height(start_height - 1) so the sync engine re-scans start_height..=tip and finds notes in blocks scanned before this key was known.

(An earlier revision used the account-scoped rewind_to_chain_state, but it corrupts the wallet DB on a deep rewind — it doesn't snap the truncation target to a checkpoint shared by the Sapling and Orchard trees. Caught by the integration-tests #76 reproducer. truncate_to_height snaps to a shared checkpoint, so it's cross-pool-safe.)

Behavior notes:

  • This is a wallet-wide rewind: every account re-scans start_height..=tip (re-scanning requires rewinding the note-commitment trees — heavier than zcashd's additive rescan).
  • truncate_to_height snaps down to a shared checkpoint, so the rescan may start slightly below start_height. A start_height below the wallet's tree state fails cleanly with RequestedRewindInvalid — rescanning below existing tree state isn't supported (beyond Rescan already-scanned ranges when a key/account is imported with an earlier birthday #578's "birthday in an already-scanned range" scope).

Testing: builds / clippy / fmt clean, unit tests pass. E2E regtest in zcash/integration-tests#76 — all three rescan-on-import scenarios pass against this branch (truncate_to_height revision): the rescan="yes" rewind surfaces a previously-missed note, a fresh early-birthday import finds a historical note (#579), and a spend from an imported key still fails (tracked separately). Note the rewind can only target the last COINBASE_MATURITY blocks (the truncation floor), which the test respects.

ZIT-Revision: export-import-key-test

@oxarbitrage

Copy link
Copy Markdown
Contributor Author

Handoff from zcash/integration-tests #76: the rescan="yes" rewind corrupts the wallet DB

While wiring up the rescan-on-import test scenarios in zcash/integration-tests#76 (the ones this PR will run via ZIT-Revision: export-import-key-test), I ran the new qa/rpc-tests/wallet_import_export_key.py locally against this branch and the core rewind path fails.

Binaries under test

  • zallet + zallet-zaino built from this branch (import-rescan-rewind-578 @ 38610e3).
  • zebrad built from zcash/zebra main (2f1fd8531, reports 6.0.0-rc.0).
  • Regtest, NU6.2 active at height 1 (Orchard active; no Ironwood).

Symptom. Re-importing an already-known key with rescan="yes" aborts:

z_importkey("<sapling key already in the wallet>", "yes", 1)
→ error: Rescan failed: Wallet data source error:
  Data DB is corrupted: Sapling and Orchard should have the same checkpoints

Reproduced twice independently in the test: (a) importing the key with an early birthday so it finds a funded note, then re-importing with rescan="yes"; and (b) importing with a birthday after the note (so it is missed), then re-importing with rescan="yes", startHeight=1 to rewind and pick it up. Both hit the identical corruption at the moment of the rewind. Normal scanning is fine — the #579 fresh-early-birthday scenario finds the historical note without issue, so this is specific to the rewind, not to scanning.

Where. The new already-known-key branch this PR adds in import_key.rs:

wallet
    .rewind_to_chain_state(
        birthday.prior_chain_state().clone(),
        std::iter::once(account_id).collect(),
    )
    .map_err(|e| LegacyCode::Misc.with_message(format!("Rescan failed: {e}")))?;

is what raises the Rescan failed: … above.

Root-cause hypothesis. zcash_client_sqlite enforces that after any rewind/truncate every shielded pool has checkpoints at the same heights. The account-scoped rewind_to_chain_state(birthday.prior_chain_state(), {account_id}) here appears to leave the Sapling and Orchard note-commitment-tree checkpoints out of lockstep — likely because the ChainState from fetch_account_birthday(start_height).prior_chain_state() doesn't carry matching per-pool frontiers for a wallet that has already scanned Orchard-active blocks, so the rewind lands Sapling and Orchard checkpoints at different heights. (The removed TODO noted that WalletWrite::rewind_to_height rewinds the entire wallet across pools/accounts; the cross-pool checkpoint invariant may be exactly why the whole-wallet primitive is needed rather than the account-scoped one.)

Reproducer. qa/rpc-tests/wallet_import_export_key.py on branch export-import-key-test (integration-tests #76). Run with:

ZEBRAD=<zebra main>/target/release/zebrad \
ZALLET=<this branch launcher, with zallet-zaino beside it> \
./qa/pull-tester/rpc-tests.py wallet_import_export_key

Minimal sequence against a running regtest wallet synced to tip (NU6.2 active), with a confirmed Sapling note at an address whose key the wallet doesn't yet hold:

  1. z_importkey(<sapling key>, "whenkeyisnew", <height>) — imports, scans forward (fine).
  2. z_importkey(<same key>, "yes", 1) — the rewind branch → Data DB is corrupted: Sapling and Orchard should have the same checkpoints.

What already passes against this branch (so the rest of the fix is validated): the #579 path (fresh import with an early birthday finds the historical note), all the basic import/export/isolation/input-validation assertions, and — separately — the confirmation that spending from an imported key currently fails with Invalid from address, no payment source found for address. (tracked as a distinct issue, not part of this PR).

Happy to iterate on the test once the rewind is adjusted here.

@oxarbitrage
oxarbitrage force-pushed the import-rescan-rewind-578 branch 2 times, most recently from 8ed6618 to e451c24 Compare July 10, 2026 20:37
@oxarbitrage
oxarbitrage force-pushed the import-rescan-rewind-578 branch from e451c24 to 3e96317 Compare July 10, 2026 20:39
@oxarbitrage

Copy link
Copy Markdown
Contributor Author

Confirmed: rebuilt zallet-zaino from 3e96317 (the truncate_to_height revision) and re-ran wallet_import_export_key (zcash/integration-tests#76) against it + zebrad from zcash/zebra mainall three scenarios pass (green, 196s). The checkpoint corruption from the earlier rewind_to_chain_state revision is gone. Added ZIT-Revision: export-import-key-test to the description so interop CI runs these end-to-end.

@oxarbitrage

Copy link
Copy Markdown
Contributor Author

Interop status update: the ZIT-Revision is working — the dispatch now correctly picks up the test branch (TEST_SHA: export-import-key-test). But the zallet-interop-request run currently aborts in the Define CI matrix job for a reason unrelated to this PR.

Root cause is on the integration-tests side: after the zcash/walletzcash/zallet rename, the interop's repo-id mapping still pointed at zcash/wallet, so the App-token lookup (repos/zcash/wallet/installation) 404s before anything builds. Fixed by zcash/integration-tests#161 (one-liner).

Once #161 merges I'll re-trigger this PR's interop so it builds 3e96317 and runs the three rescan-on-import scenarios end-to-end. They already pass locally against this branch + zebrad from zebra main.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Rescan already-scanned ranges when a key/account is imported with an earlier birthday

1 participant