Skip to content

Add regression test for zallet BlockConflict recovery (zcash/zallet#556/#560)#151

Open
Cosmos-Harry wants to merge 1 commit into
mainfrom
harry/zallet-block-conflict-test
Open

Add regression test for zallet BlockConflict recovery (zcash/zallet#556/#560)#151
Cosmos-Harry wants to merge 1 commit into
mainfrom
harry/zallet-block-conflict-test

Conversation

@Cosmos-Harry

@Cosmos-Harry Cosmos-Harry commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

Summary

Adds qa/rpc-tests/zallet_block_conflict.py, a regression test for
zcash/zallet#556 /
zcash/zallet#560.

A BlockConflict (the backend's block hash at a height disagreeing with
what's already stored in the wallet's own database) must not crash the whole
wallet — it should search for the fork point and recover automatically
instead.

Closes zcash/zallet#564.

How it works

We can't reliably force this exact code path via a natural chain reorg —
zallet's own proactive fork-point check runs before every scan and already
resolves ordinary reorgs on its own, before a raw storage conflict could ever
occur.

Instead, this reproduces the bug the same way it was originally verified by
hand:

  1. Sync the wallet for real on regtest.
  2. Stop it.
  3. Plant a deliberately-wrong block hash directly in its wallet.db, at a
    height just ahead of its synced tip — cloning a real row's tree-state
    columns so the only difference is the hash, avoiding an unrelated
    deserialization error.
  4. Restart it and let it resume syncing into that conflict.

Everything else is real: real zebrad, real zainod, real zallet, a real
BlockConflict raised by the wallet's own conflict-detection logic, and a
real recovery through the actual shipped fix.

Note

Depends on #150. This only runs successfully today because the
"zebra" chain backend (currently the launcher's default) explicitly
refuses to run on regtest, and this repo's default config/CI predates
zallet's launcher-plus-backend split. Locally I verified this test by
merging #150 (not included in this branch) to get the zaino backend
wired up. Once #150 merges to main, this test should pass as-is; before
that, it'll fail to start zallet for unrelated reasons this PR doesn't
touch. Happy to rebase/re-verify once #150 lands.

Test plan

  • Ran locally 3x in a row against real zebrad/zainod/zallet (built
    from the zcash/zallet#560 branch), all passing
  • Confirmed the log shows the actual fix's warning/recovery path
    (Block at height N conflicts with previously stored data...
    Chain reorg detected, rewinding to...), not a crash

@Cosmos-Harry
Cosmos-Harry requested a review from dannywillems July 8, 2026 22:05
…/#560)

Verifies zallet doesn't crash and recovers via fork-point search when it
hits a real, planted BlockConflict during scanning, on real regtest
zebrad/zaino/zallet processes.
@dannywillems
dannywillems force-pushed the harry/zallet-block-conflict-test branch from 2efd43a to b69eaa7 Compare July 9, 2026 12:41
(conflict_height, fake_hash, *real_row),
)
conn.commit()
finally:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: we should have a branch for the exception, just to detect exceptions that are raised. If an exception is raised, it will disappear in the finally branch.

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.

Add regression tests for block conflict errors in scanning.

2 participants