sync: Treat an absent shielded tree state as an empty note commitment tree#455
Open
oxarbitrage wants to merge 1 commit into
Open
sync: Treat an absent shielded tree state as an empty note commitment tree#455oxarbitrage wants to merge 1 commit into
oxarbitrage wants to merge 1 commit into
Conversation
oxarbitrage
added a commit
to zcash/integration-tests
that referenced
this pull request
Jun 5, 2026
…s in wallet.py The getwalletstatus sync barrier indexed `wallet_tip` unconditionally, but the RPC omits that field (Option + skip_serializing_if) until the wallet has a committed tip — which happens transiently right after blocks are mined. The barrier therefore raised `KeyError: 'wallet_tip'` ~1 run in 4, exactly during the window it is meant to poll through. Treat an absent wallet_tip as "not synced yet" and keep polling, in sync_blocks, sync_mempools, and rebuild_cache. Also make wallet.py use the now wallet-aware self.sync_all() instead of an immediate read and a fixed time.sleep(1), removing the remaining flaky wallet-scan race (and the unused `import time`). This supersedes the bespoke pollers in #106. Validated 10/10 green locally against a zallet carrying zcash/wallet#367 (getwalletstatus) + zcash/wallet#455 (empty shielded tree fix). Closes #105 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was referenced Jun 5, 2026
… tree `fetch_chain_state` errored with "Missing Sapling/Orchard tree state" whenever a shielded pool was active at the requested height but the indexer returned no tree state for it. This crashed the wallet sync task on any chain where a pool is active while its note commitment tree is still empty -- most notably regtest, where NU5 is active from height 1 but the Orchard tree has no commitments yet. An absent tree state is not an error: an empty incremental Merkle tree has no frontier to serialize, so the node (`zebra_rpc::z_get_treestate` maps the tree through `tree.map(|t| t.to_rpc_bytes())`) and the indexer both report `None` for an empty tree. Genuine fetch failures are surfaced separately via the `?` on the `z_get_treestate` call. We therefore treat "pool active but tree state absent" the same as "pool not yet active": an empty frontier. Both the Sapling and Orchard paths had this bug; both are fixed symmetrically. This path requires a live indexer connection and has no unit coverage; it was validated end-to-end against the integration-tests suite (the `wallet.py` and `wallet_orchard_init.py` regtest scenarios sync to completion with this change). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
oxarbitrage
force-pushed
the
fix/empty-shielded-tree-state
branch
from
3ab7bc7 to
00ef236
Compare
Contributor
Author
|
FYI: this is no longer blocking integration-tests. After zcash/integration-tests#104 aligned regtest NU5 activation to height 1, the wallet suite syncs to completion on That said, I think this is still worth fixing: an absent tree state ( Rebased onto current |
dannywillems
pushed a commit
to zcash/integration-tests
that referenced
this pull request
Jun 10, 2026
…s in wallet.py The getwalletstatus sync barrier indexed `wallet_tip` unconditionally, but the RPC omits that field (Option + skip_serializing_if) until the wallet has a committed tip — which happens transiently right after blocks are mined. The barrier therefore raised `KeyError: 'wallet_tip'` ~1 run in 4, exactly during the window it is meant to poll through. Treat an absent wallet_tip as "not synced yet" and keep polling, in sync_blocks, sync_mempools, and rebuild_cache. Also make wallet.py use the now wallet-aware self.sync_all() instead of an immediate read and a fixed time.sleep(1), removing the remaining flaky wallet-scan race (and the unused `import time`). This supersedes the bespoke pollers in #106. Validated 10/10 green locally against a zallet carrying zcash/wallet#367 (getwalletstatus) + zcash/wallet#455 (empty shielded tree fix). Closes #105 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
Closes #454
Problem
fetch_chain_stateerrored with"Missing Sapling/Orchard tree state"— exiting the wallet sync task — whenever a shielded pool was active at the requested height but the indexer returned no tree state for it. This crashes sync on any chain where a pool is active while its note commitment tree is still empty, most reliably on regtest (NU5 active from height 1, Orchard tree empty).Root cause
An absent tree state is not an error: an empty incremental-Merkle tree has no frontier to serialize, so the node reports
Nonefor it (zebra_rpc::z_get_treestatemaps the tree viatree.map(|t| t.to_rpc_bytes()), andzebra-state'sread::orchard_tree()returnsNonewhen no tree is stored at that height). Zaino relays thisNonefaithfully. Genuine fetch failures surface separately via the?onz_get_treestate. SoNone⟺ "empty tree", and treating it as fatal was incorrect — this is a zallet-side bug; Zaino/Zebra are behaving correctly.Fix
Treat "pool active but tree state absent" the same as "pool not yet active": an empty frontier. Both the Sapling and Orchard paths had this bug; both are fixed symmetrically.
Testing
fetch_chain_staterequires a live indexer connection and has no unit coverage. Validated end-to-end against the integration-tests suite — thewallet.pyandwallet_orchard_init.pyregtest scenarios sync to completion with this change (previously they hit"Missing Orchard tree state").🤖 Generated with Claude Code