Skip to content

Make NU6.3 wallet test setup exception-safe#22

Merged
p0mvn merged 2 commits into
zcashd-compatfrom
roman/fix-nu6-3-wallet-test
Jul 21, 2026
Merged

Make NU6.3 wallet test setup exception-safe#22
p0mvn merged 2 commits into
zcashd-compatfrom
roman/fix-nu6-3-wallet-test

Conversation

@p0mvn

@p0mvn p0mvn commented Jul 21, 2026

Copy link
Copy Markdown

Summary

  • install a height-0 fake chain tip before calling PrepareTransaction
  • select the first valid Sapling diversifier instead of assuming index 0 is valid
  • use scoped cleanup for the fake chain tip, network upgrade, and global wallet state

Motivation / Why

The NU6.3 wallet-selection test originally ran with chainActive at height -1, causing consensus branch lookup to assert before the test reached its expectations.

The initial chain-tip fix exposed a second latent failure: Sapling diversifier index 0 is not valid for every randomly generated wallet seed. When .value() threw bad_optional_access, manual cleanup was skipped. That left a pointer to a stack-allocated CBlockIndex in global state, which caused the following test to observe height 0 and could make global teardown abort with free(): invalid pointer.

This PR makes all temporary test state exception-safe and uses FindAddress to obtain a valid Sapling address.

Tests fixed

  • WalletRPCTests.PrepareTransactionAvoidsOrchardAfterNU6point3
    • assertion from passing chain height -1 into consensus upgrade lookup
    • intermittent bad_optional_access from an invalid Sapling diversifier
  • WalletRPCTests.RPCZsendmanyTaddrToSapling
    • collateral failure from the preceding test leaking a height-0 chain tip
  • GoogleTest process teardown
    • invalid free caused by a leaked pointer to a stack CBlockIndex

Validation

  • local x86_64 macOS build via ./zcutil/build.sh
  • affected test passed 100 consecutive randomized repetitions
  • affected test and RPCZsendmanyTaddrToSapling passed sequentially
  • all CI checks pass, including every Ubuntu, Debian, ARM64, and Windows GoogleTest shard

p0mvn added 2 commits July 20, 2026 19:11
PrepareTransaction requires a nonnegative active-chain height when
selecting the current consensus branch ID.
Select a valid Sapling diversifier and scope all temporary global state so
failures cannot corrupt later wallet tests or global teardown.
@p0mvn p0mvn changed the title Initialize chain tip in NU6.3 wallet test Make NU6.3 wallet test setup exception-safe Jul 21, 2026
@p0mvn
p0mvn merged commit c44e282 into zcashd-compat Jul 21, 2026
97 checks passed
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.

1 participant