diff --git a/MISSING-FEATURES.md b/MISSING-FEATURES.md new file mode 100644 index 000000000..e186e6aa3 --- /dev/null +++ b/MISSING-FEATURES.md @@ -0,0 +1,257 @@ +# Z3 stack feature gaps (from disabled RPC tests) + +Notes on the gaps that keep migrated zcashd tests disabled, grouped by the +component that needs the work. Ordered roughly by how many tests each +unblocks. + +Verified against source: zallet RPC trait (wallet/zallet/.../json_rpc/ +methods.rs), zebra RPC methods (zebra-rpc/src), and zebra regtest activation +logic (zebra-chain/.../testnet.rs). One earlier hypothesis was WRONG and is +corrected below (pre-NU5 heights ARE configurable in zebra regtest). + +## zallet: missing legacy wallet RPCs (largest group, ~70 tests) + +zallet implements a modern account/unified API only. These zcashd wallet +RPCs are not implemented, so the tests using them cannot run: + +- getnewaddress, z_getnewaddress +- sendtoaddress, sendmany +- getbalance, z_getbalance, z_getbalanceforaccount +- gettransaction, getreceivedbyaddress, listreceivedbyaddress +- getrawchangeaddress, getunconfirmedbalance +- dumpprivkey, importprivkey, importpubkey, importaddress (transparent) +- z_exportkey, z_importkey, z_exportviewingkey +- encryptwallet, keypoolrefill, backupwallet +- resendwallettransactions + +Implemented today (zallet jsonrpsee trait, verified in +zallet/src/components/json_rpc/methods.rs - 30 methods total): +z_getnewaccount, z_getaccount, z_getaddressforaccount, z_listaccounts, +z_listunifiedreceivers, z_listunspent, z_getbalances, z_gettotalbalance, +z_sendmany, z_shieldcoinbase, z_listtransactions, z_viewtransaction, +z_importaddress, z_listoperationids, z_getoperationstatus, z_getoperationresult, +z_getnotescount, z_recoveraccounts, z_converttex, getwalletinfo, listaddresses, +validateaddress, verifymessage, getrawtransaction, decoderawtransaction, +decodescript, walletpassphrase, walletlock, help, stop. +Note: wallet encryption exists via walletpassphrase/walletlock (not the +zcashd `encryptwallet` RPC). No legacy aliases in the compatibility layer. + +## wallet funding: z_sendmany cannot spend mined coinbase (affects ~all wallet tests) + +Confirmed by migrating wallet_unified_change.py and probing the wallet state. +The harness mines regtest coinbase to each wallet's miner address, which +zallet derives as the account's internal (change) transparent address +(generate_account_and_miner_address.rs: derive_internal_ivk().default_address(), +birthday height 0). After syncing 200 cached blocks, wallet 0 reports: + + z_getbalances(0) -> account 0 transparent.regular.spendable = 31875000000 zat + (~318.75 ZEC), but total.spendable = 0. + +A z_sendmany from the miner address with 'AllowRevealedSenders' fails with +"Insufficient balance (have 0, need ...)". So the mined coinbase is visible in +the transparent balance breakdown but is not spendable through z_sendmany. The +canonical path is z_shieldcoinbase (which zallet implements) to shield coinbase +into a shielded address first, then z_sendmany from there. + +Consequence: migrating any wallet test is not a simple RPC rename. Each test +that funds accounts from coinbase needs an added z_shieldcoinbase funding step +plus the account-model rewrite (account_uuid instead of integer index, +z_getbalances reshape, null fee). Other zallet API specifics found while +migrating wallet_unified_change.py: + +- z_getnewaccount requires an account_name argument (zcashd took none). +- z_getnewaccount returns {account_uuid, account: null}; key everything by + account_uuid (the ZIP 32 index is not populated). z_getaddressforaccount and + z_getbalances both use account_uuid. +- z_sendmany rejects an explicit fee ("the fee field must be null"); it always + computes the ZIP 317 fee internally. Pass null and assert against the + ZIP 317 conventional fee. +- z_getaddressforaccount with default receiver types tries to derive a + transparent receiver and hits the transparent gap limit ("index 10") for a + fresh account with no transparent funds; request explicit shielded receiver + types (e.g. ['sapling','orchard']) instead. + +## zebra: missing node RPCs + +- gettxoutsetinfo - blockchain.py, errors.py +- getchaintips - getchaintips.py +- setban / listbanned / clearbanned / disconnectnode - nodehandling.py + (also getpeerinfo peer count differs) +- signrawtransaction - signrawtransactions.py (used by ~13 tests total) + +## zebra regtest: activation-height config (CORRECTED - was wrong) + +Source check (zebra-chain testnet.rs: for_regtest + with_activation_heights): +pre-NU5 heights are NOT hardcoded. for_regtest only DEFAULTS unset upgrades +to height 1 (overwinter.or(1), sapling.or(overwinter), ...); a configured +pre-NU5 height is kept. ConfiguredActivationHeights deserializes keys +overwinter/sapling/blossom/heartwood/canopy (lowercase) and NU5/NU6/NU6.1 +(uppercase, via serde rename); the only constraints are in-order and +height > 0. So these tests are NOT blocked by a missing zebra feature -- they +need migration (pass activation_heights via ZebraArgs with correctly-cased +names): + +- feature_zip221.py, feature_zip244_blockcommitments.py, orchard_reorg.py, + post_heartwood_rollback.py, rewind_index.py, sapling_rewind_check.py, + upgrade_golden.py, mempool_nu_activation.py + +The wallet ones among these are still blocked by the zallet NU5 coupling +below. + +## zallet/zebra: per-test activation heights + shared cache + +This is a FRAMEWORK plumbing gap, not a missing zallet/zebra feature: zallet +accepts any regtest_nuparams (RegTestNuParam parsing in zallet/src/network.rs) +and zebra accepts any activation_heights, but the harness uses a static +zallet.toml (NU5@1) and builds the shared chain cache once at NU5@1, so a +wallet test needing a different NU5 height can't make node and wallet agree. +Fix is in the test harness (thread per-test heights into both, and/or a +per-config cache). Blocks the wallet_orchard* family and others. + +## zcashd-only behaviour (needs an equivalent or test rewrite) + +- RPC HTTP basic auth (rpcuser/rpcpassword) - httpbasics.py, multi_rpc.py +- wallet.dat file + -wallet flag - feature_walletfile.py +- regtest/debug.log - feature_logging.py, framework.py +- -proxy / tor - proxy_test.py +- address/spent/timestamp indexes (-insightexplorer/-txindex) - + addressindex.py, spentindex.py, timestampindex.py, + getrawtransaction_insight.py +- -regtestshieldcoinbase - wallet_treestate.py, wallet_anchorfork.py, ... +- Sprout address generation / sprout key import - sprout_sapling_migration.py, + remove_sprout_shielding.py, ... +- 5.6.0 golden/tarnished wallet files - wallet_golden_5_6_0.py, + wallet_tarnished_5_6_0.py + +## P2P / mininode framework + +These use the in-Python P2P test framework (NodeConn), not yet exercised +against zebra (10 tests, by NodeConn usage): + +- invalidblockrequest.py, invalidtxrequest.py, p2p-fullblocktest.py, + p2p_node_bloom.py, p2p_nu_peer_management.py, p2p_txexpiringsoon.py, + p2p_txexpiry_dos.py, bip65-cltv-p2p.py, bipdersig-p2p.py, feature_zip239.py + +## Confirmed: zebra reorg / sync behaviour (reorg_limit.py) + +reorg_limit.py is node-only (no wallet). After fixing it to use num_wallets=0 +and NU5@1, setup succeeds, but the core check fails: after a network split +where node 2 builds a longer (competing) chain, reconnecting does not make +node 0 reorg to it (expected getblockcount 300). + +Source check: zebra has MAX_BLOCK_REORG_HEIGHT = MIN_TRANSPARENT_COINBASE_ +MATURITY - 1 = 99 (zebra-state/src/constants.rs); blocks past that depth are +finalized and cannot be rolled back, and zebra does NOT do zcashd's deep-reorg +safety *shutdown* (it just refuses to roll back). zcashd allows reorgs up to +99 blocks and aborts beyond. So the test's exact boundary (100-block reorg +should succeed; 101 should stop the node) and the shutdown message don't match +zebra's finalize-and-refuse behaviour. Behavioural difference, not a missing +RPC; test needs reworking for zebra semantics. + +## Batch 2 findings (confirmed by running) + +- zebra/zallet missing `signrawtransaction` RPC - signrawtransactions.py +- `-proxy` / tor not supported - proxy_test.py (hangs) +- RPC HTTP basic auth not supported - httpbasics.py, multi_rpc.py (hang) + +Note: P2P / mininode tests can't be evaluated in this local python build +(missing `_dbm` C extension), so they stay categorised as "mininode +framework, untested vs zebra" rather than confirmed gaps: +invalidblockrequest.py, invalidtxrequest.py, p2p-fullblocktest.py, +p2p_node_bloom.py, p2p_nu_peer_management.py, bip65-cltv-p2p.py, +bipdersig-p2p.py. + +## Note on layering + +Many disabled tests crash first on the un-migrated `extra_args=[[...]]` list +("'list' object has no attribute 'miner_address'") in their own setup_network +before reaching the feature that actually blocks them. That first symptom is +a ZebraArgs migration task, not a missing feature; the feature gaps listed +above (legacy wallet RPCs, pre-NU5 activation heights, missing node RPCs, +zcashd-only behaviour) are the real blockers behind it, identified from each +test's RPC / nuparams usage. + +## Wallet tests: rewrite-able vs genuinely blocked (of the 73 tagged +## "zallet missing") + +Splitting the wallet-RPC group by whether zallet already has the capability +(under its account/unified API) or genuinely lacks it. + +REWRITE-ABLE (~33): use only ops with a zallet equivalent, so the test code +can be moved to zallet (a model rewrite, not a rename: per-address -> +per-account/pool, bare -> unified addresses). Mappings: +- sendtoaddress / sendmany -> z_sendmany +- getbalance / z_getbalance / + z_getbalanceforaccount -> z_gettotalbalance / z_getbalances +- getnewaddress / z_getnewaddress -> z_getaddressforaccount (+ z_listunifiedreceivers) +Caveat: some of these 33 still carry a non-wallet blocker (e.g. +wallet_treestate uses -regtestshieldcoinbase, zmq_test needs ZMQ, +wallet_golden_5_6_0 loads a zcashd wallet file, mempool_nu_activation needs +pre-NU5 boundaries), so the truly-clean subset is smaller. + +BLOCKED (~40): use an RPC zallet genuinely lacks. By feature: +- Sprout support 15 (Sprout is deprecated; likely retire, not migrate) +- signrawtransaction 13 +- key import/export + (z_importkey/z_exportkey/ + z_exportviewingkey/dumpprivkey/ + importprivkey) ~bulk +- encryptwallet 3 +- resendwallettransactions 2 +- keypoolrefill 2 +- getrawchangeaddress 2 +- importpubkey / backupwallet 1 each + +## zcashd deprecation -> zallet migration (verified in zcash doc/book) + +Many "missing" wallet RPCs are *deprecated* in zcashd (see zcashd +doc/book/src/user/deprecation.md) and are replaced by the account/unified- +address API that zallet already implements. So most of these tests are a +migration, not a feature gap: + +- getnewaddress, z_getnewaddress -> z_getaddressforaccount (+ z_getnewaccount) +- z_listaddresses -> listaddresses +- z_getbalance, getbalance, + z_getbalanceforaccount -> z_getbalances / z_gettotalbalance +- sendtoaddress, sendmany -> z_sendmany +- gettransaction -> z_viewtransaction +- encryptwallet -> walletpassphrase / walletlock +- getrawchangeaddress, keypoolrefill, + settxfee -> n/a (account model / ZIP 317 fees) +- createrawtransaction, + fundrawtransaction, + signrawtransaction -> PCZT-based, zcash/wallet#99 +- getnetworkhashps -> getnetworksolps + +Genuinely no zallet equivalent yet (not just a rename): +- transparent key import/export: dumpprivkey, importprivkey, importpubkey +- shielded key import/export: z_importkey, z_exportkey, z_exportviewingkey +- resendwallettransactions, backupwallet +- Sprout (deprecated/removed) + +## Trial migration outcome (wallet_unified_change.py) + +A one-test trial confirmed the "REWRITE-ABLE" group above is not a simple RPC +rename. Even a test that already uses the modern account API needs a structural +rewrite before it can run on the Z3 stack: + +1. Funding: insert a z_shieldcoinbase step. zallet z_sendmany cannot spend the + harness-mined coinbase (see "wallet funding" section); shield it into a + shielded address first, then z_sendmany from there. +2. Account identity: use account_uuid from z_getnewaccount, not the integer + index (which is null). Pass the uuid to z_getaddressforaccount and match it + in z_getbalances. +3. Balance asserts: reshape z_getbalanceforaccount's + {pools:{sapling:{valueZat}}, minimum_confirmations} to z_getbalances' + accounts[].{sapling,orchard}.spendable.valueZat. +4. Fees: pass null to z_sendmany and assert against the ZIP 317 conventional + fee that zallet computes. +5. Addresses: request explicit shielded receiver types from + z_getaddressforaccount to avoid the transparent gap limit. +6. Wiring: route wallet RPCs to self.wallets[i] (not self.nodes[i]); take the + coinbase source from self.miner_addresses[i]; read confirmations via the + node's getrawtransaction (zallet has no gettransaction). + +The trial was stopped before completing the z_shieldcoinbase funding rewrite; +wallet_unified_change.py remains disabled. The steps above are the recipe for +whoever picks the wallet-test migration back up. diff --git a/qa/defaults/zallet/zallet.toml b/qa/defaults/zallet/zallet.toml index a081039fd..2d7c937da 100644 --- a/qa/defaults/zallet/zallet.toml +++ b/qa/defaults/zallet/zallet.toml @@ -3,13 +3,17 @@ orchard_actions = 250 [consensus] network = "regtest" +# These activation heights MUST match the ones zebrad is started with, or +# wallet sync fails with "Missing Orchard tree state". They must agree with +# REGTEST_ACTIVATION_HEIGHTS in qa/rpc-tests/test_framework/util.py. +# The format is [consensusBranchId:activationHeight] regtest_nuparams = [ - "5ba81b19:1", - "76b809bb:1", - "2bb40e60:1", - "f5b9230b:1", - "e9ff75a6:1", - "c2d6d0b4:1", + "5ba81b19:1", # Overwinter + "76b809bb:1", # Sapling + "2bb40e60:1", # Blossom + "f5b9230b:1", # Heartwood + "e9ff75a6:1", # Canopy + "c2d6d0b4:1", # NU5 ] [database] diff --git a/qa/pull-tester/rpc-tests.py b/qa/pull-tester/rpc-tests.py index 231fb01b9..d6f5437f8 100755 --- a/qa/pull-tester/rpc-tests.py +++ b/qa/pull-tester/rpc-tests.py @@ -28,17 +28,126 @@ SERIAL_SCRIPTS = [ # These tests involve enough shielded spends (consuming all CPU - # cores) that we can't run them in parallel. - 'mergetoaddress_sapling.py', - 'mergetoaddress_ua_nu5.py', - 'mergetoaddress_ua_sapling.py', - 'wallet_shieldingcoinbase.py', + # cores) that we can't run them in parallel. When re-enabled after + # migration to the Z3 stack, move them back here (see DISABLED_SCRIPTS). ] FLAKY_SCRIPTS = [ # These tests have intermittent failures that we haven't diagnosed yet. - 'mempool_nu_activation.py', # this *may* be fixed - 'mempool_packages.py', +] + +# Disabled until migrated to the Z3 stack. Note = why it fails / suggested +# migration. Most "deprecated" RPCs are deprecated in zcashd (see its +# doc/book/.../deprecation.md) and map to zallet's account/UA API. +DISABLED_SCRIPTS = [ + 'addressindex.py', # deprecated; getnewaddress->z_getaddressforaccount, getbalance->z_getbalances + 'bip65-cltv-p2p.py', # P2P/mininode framework + 'bipdersig-p2p.py', # P2P/mininode framework + 'blockchain.py', # zebra missing gettxoutsetinfo + 'coinbase_funding_streams.py', # deprecated; getnewaddress->z_getaddressforaccount, z_getnewaddress->z_getaddressforaccount + 'errors.py', # zebra missing gettxoutsetinfo + 'feature_logging.py', # no zcashd regtest/debug.log + 'feature_walletfile.py', # zcashd wallet.dat/-wallet + 'feature_zip221.py', # pre-NU5 nuparams: migrate to ZebraArgs activation_heights + 'feature_zip239.py', # P2P/mininode framework + 'feature_zip244_blockcommitments.py', # pre-NU5 nuparams: migrate to ZebraArgs activation_heights + 'finalorchardroot.py', # deprecated; getnewaddress->z_getaddressforaccount, z_getnewaddress->z_getaddressforaccount + 'finalsaplingroot.py', # deprecated; getnewaddress->z_getaddressforaccount, z_getnewaddress->z_getaddressforaccount + 'framework.py', # no zcashd regtest/debug.log + 'fundrawtransaction.py', # no zallet equiv yet: importpubkey + 'getblocktemplate.py', # deprecated; getnewaddress->z_getaddressforaccount, z_getbalance->z_getbalances + 'getchaintips.py', # zebra missing getchaintips + 'getrawtransaction_insight.py', # deprecated; getnewaddress->z_getaddressforaccount, sendtoaddress->z_sendmany + 'httpbasics.py', # RPC basic auth (zebra uses cookie auth) + 'invalidblockrequest.py', # P2P/mininode framework + 'invalidtxrequest.py', # P2P/mininode framework + 'key_import_export.py', # no zallet equiv yet: dumpprivkey, importprivkey + 'keypool.py', # deprecated; getnewaddress->z_getaddressforaccount, encryptwallet->walletpassphrase/walletlock + 'listtransactions.py', # deprecated; getnewaddress->z_getaddressforaccount, sendtoaddress->z_sendmany + 'mempool_limit.py', # deprecated; z_getnewaddress->z_getaddressforaccount + 'mempool_nu_activation.py', # deprecated; getnewaddress->z_getaddressforaccount, z_getnewaddress->z_getaddressforaccount + 'mempool_packages.py', # deprecated; getnewaddress->z_getaddressforaccount, signrawtransaction->PCZT (wallet#99) + 'mempool_reorg.py', # deprecated; getnewaddress->z_getaddressforaccount, signrawtransaction->PCZT (wallet#99) + 'mempool_resurrect_test.py', # deprecated; getnewaddress->z_getaddressforaccount, gettransaction->z_viewtransaction + 'mempool_spendcoinbase.py', # deprecated; getnewaddress->z_getaddressforaccount, signrawtransaction->PCZT (wallet#99) + 'mempool_tx_expiry.py', # deprecated; getnewaddress->z_getaddressforaccount, z_getnewaddress->z_getaddressforaccount + 'mergetoaddress_mixednotes.py', # deprecated; getnewaddress->z_getaddressforaccount, z_getnewaddress->z_getaddressforaccount + 'mergetoaddress_sapling.py', # deprecated; z_getnewaddress->z_getaddressforaccount + 'mergetoaddress_ua_nu5.py', # -anchorconfirmations unsupported + 'mergetoaddress_ua_sapling.py', # -anchorconfirmations unsupported + 'merkle_blocks.py', # deprecated; getnewaddress->z_getaddressforaccount, getbalance->z_getbalances + 'mining_shielded_coinbase.py', # deprecated; getnewaddress->z_getaddressforaccount, z_getnewaddress->z_getaddressforaccount + 'multi_rpc.py', # RPC basic auth (zebra uses cookie auth) + 'nodehandling.py', # zebra missing setban, listbanned, clearbanned + 'orchard_reorg.py', # pre-NU5 nuparams: migrate to ZebraArgs activation_heights + 'p2p-fullblocktest.py', # P2P/mininode framework + 'p2p_node_bloom.py', # P2P/mininode framework + 'p2p_nu_peer_management.py', # P2P/mininode framework + 'p2p_txexpiringsoon.py', # P2P/mininode framework + 'p2p_txexpiry_dos.py', # P2P/mininode framework + 'post_heartwood_rollback.py', # pre-NU5 nuparams: migrate to ZebraArgs activation_heights + 'prioritisetransaction.py', # deprecated; getnewaddress->z_getaddressforaccount, getbalance->z_getbalances + 'proxy_test.py', # -proxy/tor unsupported + 'rawtransactions.py', # deprecated; getnewaddress->z_getaddressforaccount, getbalance->z_getbalances + 'regtest_signrawtransaction.py', # deprecated; getnewaddress->z_getaddressforaccount, z_getnewaddress->z_getaddressforaccount + 'remove_sprout_shielding.py', # deprecated; getnewaddress->z_getaddressforaccount, z_getnewaddress->z_getaddressforaccount + 'reorg_limit.py', # investigate + 'rest.py', # deprecated; getnewaddress->z_getaddressforaccount, getbalance->z_getbalances + 'rewind_index.py', # pre-NU5 nuparams: migrate to ZebraArgs activation_heights + 'sapling_rewind_check.py', # pre-NU5 nuparams: migrate to ZebraArgs activation_heights + 'shorter_block_times.py', # deprecated; z_getnewaddress->z_getaddressforaccount + 'show_help.py', # deprecated; getnewaddress->z_getaddressforaccount, z_getnewaddress->z_getaddressforaccount + 'signrawtransaction_offline.py', # no zallet equiv yet: dumpprivkey + 'signrawtransactions.py', # deprecated; signrawtransaction->PCZT (wallet#99) + 'spentindex.py', # deprecated; getnewaddress->z_getaddressforaccount, sendtoaddress->z_sendmany + 'sprout_sapling_migration.py', # no zallet equiv yet: z_importkey + 'threeofthreerestore.py', # no zallet equiv yet: dumpprivkey, importprivkey + 'timestampindex.py', # -insightexplorer index, -txindex unsupported + 'turnstile.py', # deprecated; z_getnewaddress->z_getaddressforaccount, z_getbalance->z_getbalances + 'txn_doublespend.py', # deprecated; getnewaddress->z_getaddressforaccount, getbalance->z_getbalances + 'upgrade_golden.py', # pre-NU5 nuparams: migrate to ZebraArgs activation_heights + 'wallet_1941.py', # no zallet equiv yet: z_exportkey, z_importkey + 'wallet_accounts.py', # no zallet equiv yet: z_exportviewingkey + 'wallet_addresses.py', # no zallet equiv yet: z_exportkey, z_importkey + 'wallet_anchorfork.py', # deprecated; z_getnewaddress->z_getaddressforaccount, getbalance->z_getbalances + 'wallet_broadcast.py', # deprecated; getnewaddress->z_getaddressforaccount, getbalance->z_getbalances + 'wallet_changeaddresses.py', # deprecated; getnewaddress->z_getaddressforaccount, z_getnewaddress->z_getaddressforaccount + 'wallet_changeindicator.py', # no zallet equiv yet: z_exportviewingkey + 'wallet_deprecation.py', # deprecated; getnewaddress->z_getaddressforaccount, z_getnewaddress->z_getaddressforaccount + 'wallet_doublespend.py', # deprecated; z_getbalanceforaccount->z_getbalances, gettransaction->z_viewtransaction + 'wallet_golden_5_6_0.py', # deprecated; z_getbalanceforaccount->z_getbalances + 'wallet_import_export.py', # no zallet equiv yet: z_exportkey, z_importkey + 'wallet_isfromme.py', # deprecated; getnewaddress->z_getaddressforaccount, z_getnewaddress->z_getaddressforaccount + 'wallet_listnotes.py', # no zallet equiv yet: z_exportviewingkey + 'wallet_listreceived.py', # deprecated; getnewaddress->z_getaddressforaccount, z_getnewaddress->z_getaddressforaccount + 'wallet_listunspent.py', # deprecated; getnewaddress->z_getaddressforaccount, getbalance->z_getbalances + 'wallet_nullifiers.py', # no zallet equiv yet: z_exportkey, z_importkey, z_exportviewingkey + 'wallet_orchard.py', # no zallet equiv yet: resendwallettransactions + 'wallet_orchard_change.py', # deprecated; z_getbalanceforaccount->z_getbalances + 'wallet_orchard_init.py', # no zallet equiv yet: resendwallettransactions + 'wallet_orchard_persistence.py', # deprecated; z_getbalanceforaccount->z_getbalances + 'wallet_orchard_reindex.py', # deprecated; z_getbalanceforaccount->z_getbalances + 'wallet_overwintertx.py', # deprecated; getnewaddress->z_getaddressforaccount, z_getnewaddress->z_getaddressforaccount + 'wallet_parsing_amounts.py', # deprecated; getnewaddress->z_getaddressforaccount, z_getnewaddress->z_getaddressforaccount + 'wallet_persistence.py', # no zallet equiv yet: z_exportkey, z_importkey, z_exportviewingkey + 'wallet_sapling.py', # no zallet equiv yet: z_exportkey, z_importkey, z_exportviewingkey + 'wallet_sendmany_any_taddr.py', # deprecated; getnewaddress->z_getaddressforaccount, z_getnewaddress->z_getaddressforaccount + 'wallet_shieldcoinbase_sapling.py', # deprecated; z_getnewaddress->z_getaddressforaccount, z_getbalance->z_getbalances + 'wallet_shieldcoinbase_ua_nu5.py', # deprecated; z_getbalance->z_getbalances, z_getbalanceforaccount->z_getbalances + 'wallet_shieldcoinbase_ua_sapling.py', # deprecated; z_getbalance->z_getbalances, z_getbalanceforaccount->z_getbalances + 'wallet_shieldingcoinbase.py', # no zallet equiv yet: z_exportviewingkey + 'wallet_tarnished_5_6_0.py', # needs ZebraArgs migration (list args) + 'wallet_treestate.py', # deprecated; z_getnewaddress->z_getaddressforaccount, z_getbalance->z_getbalances + 'wallet_unified_change.py', # needs z_shieldcoinbase funding step: zallet z_sendmany cannot spend mined coinbase (have 0) + 'wallet_z_sendmany.py', # no zallet equiv yet: z_exportviewingkey + 'wallet_z_shieldcoinbase.py', # investigate + 'wallet_z_shieldcoinbase_multi_taddr.py', # investigate + 'wallet_zero_value.py', # deprecated; getnewaddress->z_getaddressforaccount, signrawtransaction->PCZT (wallet#99) + 'wallet_zip317_default.py', # deprecated; getnewaddress->z_getaddressforaccount, z_getnewaddress->z_getaddressforaccount + 'walletbackup.py', # no zallet equiv yet: backupwallet + 'zapwallettxes.py', # deprecated; getnewaddress->z_getaddressforaccount, getbalance->z_getbalances + 'zkey_import_export.py', # no zallet equiv yet: z_exportkey, z_importkey + 'zmq_test.py', # deprecated; getnewaddress->z_getaddressforaccount, sendtoaddress->z_sendmany ] BASE_SCRIPTS= [ @@ -203,6 +312,19 @@ ALL_SCRIPTS = SERIAL_SCRIPTS + FLAKY_SCRIPTS + BASE_SCRIPTS + NEW_SCRIPTS + ZMQ_SCRIPTS + EXTENDED_SCRIPTS +# Exclude DISABLED_SCRIPTS from the lists that actually get run, matching on +# the script filename so entries with extra args (e.g. 'foo.py --bar') are +# covered too. ALL_SCRIPTS above is left complete so a disabled test can +# still be run explicitly by name. +_DISABLED_FILES = {s.split()[0] for s in DISABLED_SCRIPTS} +def _without_disabled(scripts): + return [s for s in scripts if s.split()[0] not in _DISABLED_FILES] +SERIAL_SCRIPTS = _without_disabled(SERIAL_SCRIPTS) +FLAKY_SCRIPTS = _without_disabled(FLAKY_SCRIPTS) +BASE_SCRIPTS = _without_disabled(BASE_SCRIPTS) +NEW_SCRIPTS = _without_disabled(NEW_SCRIPTS) +ZMQ_SCRIPTS = _without_disabled(ZMQ_SCRIPTS) + def main(): # Parse arguments and pass through unrecognised args parser = argparse.ArgumentParser(add_help=False, diff --git a/qa/rpc-tests/disablewallet.py b/qa/rpc-tests/disablewallet.py index 80f600e29..b4221ea04 100755 --- a/qa/rpc-tests/disablewallet.py +++ b/qa/rpc-tests/disablewallet.py @@ -5,11 +5,13 @@ # file COPYING or https://www.opensource.org/licenses/mit-license.php . # -# Exercise API with -disablewallet. +# Exercise the node API without a wallet. +# +# On the Z3 stack the wallet (zallet) is a separate process, so a node started +# without any wallet is the default; we just run a single node and no wallets. # from test_framework.test_framework import BitcoinTestFramework -from test_framework.util import start_nodes class DisableWalletTest (BitcoinTestFramework): @@ -18,11 +20,7 @@ def __init__(self): super().__init__() self.cache_behavior = 'clean' self.num_nodes = 1 - - def setup_network(self, split=False): - self.nodes = start_nodes(self.num_nodes, self.options.tmpdir, [['-disablewallet']]) - self.is_network_split = False - self.sync_all() + self.num_wallets = 0 def run_test (self): # Check regression: https://github.com/bitcoin/bitcoin/issues/6963#issuecomment-154548880 diff --git a/qa/rpc-tests/mempool_nu_activation.py b/qa/rpc-tests/mempool_nu_activation.py index c617886fc..770f56de2 100755 --- a/qa/rpc-tests/mempool_nu_activation.py +++ b/qa/rpc-tests/mempool_nu_activation.py @@ -3,15 +3,10 @@ # Distributed under the MIT software license, see the accompanying # file COPYING or https://www.opensource.org/licenses/mit-license.php . +from test_framework.config import ZebraArgs from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( - BLOSSOM_BRANCH_ID, - CANOPY_BRANCH_ID, - HEARTWOOD_BRANCH_ID, - NU5_BRANCH_ID, - NU6_BRANCH_ID, assert_equal, assert_true, - nuparams, start_node, connect_nodes, wait_and_assert_operationid_status, get_coinbase_address ) @@ -30,20 +25,17 @@ def __init__(self): self.cache_behavior = 'clean' def setup_network(self): - args = [ - '-checkmempool', - '-debug=mempool', - '-blockmaxsize=4000', - '-preferredtxversion=4', - '-allowdeprecated=getnewaddress', - '-allowdeprecated=z_getnewaddress', - '-allowdeprecated=z_getbalance', - nuparams(BLOSSOM_BRANCH_ID, 200), - nuparams(HEARTWOOD_BRANCH_ID, 210), - nuparams(CANOPY_BRANCH_ID, 220), - nuparams(NU5_BRANCH_ID, 230), - nuparams(NU6_BRANCH_ID, 240), - ] + # The original zcashd test also passed -checkmempool, -debug=mempool, + # -blockmaxsize=4000, -preferredtxversion=4 and several -allowdeprecated + # flags. The Z3 stack has no ZebraArgs equivalent for these, so only the + # network-upgrade activation heights carry over here. + args = ZebraArgs(activation_heights={ + "Blossom": 200, + "Heartwood": 210, + "Canopy": 220, + "NU5": 230, + "NU6": 240, + }) self.nodes = [] self.nodes.append(start_node(0, self.options.tmpdir, args)) self.nodes.append(start_node(1, self.options.tmpdir, args)) diff --git a/qa/rpc-tests/reorg_limit.py b/qa/rpc-tests/reorg_limit.py index 47286db13..8f17e7822 100755 --- a/qa/rpc-tests/reorg_limit.py +++ b/qa/rpc-tests/reorg_limit.py @@ -7,6 +7,7 @@ # Test reorg limit # +from test_framework.config import ZebraArgs from test_framework.test_framework import BitcoinTestFramework from test_framework.util import ( check_node, @@ -31,14 +32,21 @@ def check_stopped(i, timeout=10): class ReorgLimitTest(BitcoinTestFramework): + def __init__(self): + super().__init__() + # Node-only reorg test; no wallets needed. + self.num_wallets = 0 + def setup_nodes(self): self.log_stderr = tempfile.SpooledTemporaryFile(max_size=2**16) + # Activate NU5 at height 1 to match the shared chain cache. + args = ZebraArgs(activation_heights={"NU5": 1}) nodes = [] - nodes.append(start_node(0, self.options.tmpdir, stderr=self.log_stderr)) - nodes.append(start_node(1, self.options.tmpdir)) - nodes.append(start_node(2, self.options.tmpdir)) - nodes.append(start_node(3, self.options.tmpdir)) + nodes.append(start_node(0, self.options.tmpdir, args, stderr=self.log_stderr)) + nodes.append(start_node(1, self.options.tmpdir, args)) + nodes.append(start_node(2, self.options.tmpdir, args)) + nodes.append(start_node(3, self.options.tmpdir, args)) return nodes diff --git a/qa/rpc-tests/test_framework/test_framework.py b/qa/rpc-tests/test_framework/test_framework.py index e9afee4d7..6be5a876a 100755 --- a/qa/rpc-tests/test_framework/test_framework.py +++ b/qa/rpc-tests/test_framework/test_framework.py @@ -18,6 +18,7 @@ from .config import ZebraArgs from .proxy import JSONRPCException from .util import ( + REGTEST_ACTIVATION_HEIGHTS, zcashd_binary, initialize_chain, prepare_wallets_for_mining, @@ -30,6 +31,7 @@ stop_nodes, stop_wallets, stop_zainos, + stop_all_processes, wait_bitcoinds, wait_zainods, wait_zallets, @@ -69,7 +71,17 @@ def setup_nodes(self): if self.miner_addresses is None: args = None else: - args = [ZebraArgs(miner_address=addr) for addr in self.miner_addresses] + # Activate NU5 to match zallet's regtest config; otherwise wallet + # sync fails with "Missing Orchard tree state" (see + # REGTEST_ACTIVATION_HEIGHTS). Tests needing different activation + # heights override this method. + args = [ + ZebraArgs( + miner_address=addr, + activation_heights=dict(REGTEST_ACTIVATION_HEIGHTS), + ) + for addr in self.miner_addresses + ] return start_nodes(self.num_nodes, self.options.tmpdir, args) def prepare_chain(self): @@ -218,17 +230,39 @@ def main(self): print("Exiting after " + repr(e)) if not self.options.noshutdown: + # Best-effort clean shutdown via RPC. Each step is guarded because a + # failure during setup can leave self.wallets / self.zainos / + # self.nodes as None or only partially populated. We deliberately do + # NOT wait on the processes here: on the failure path the RPC stop + # was skipped, so the processes are still running and a wait-first + # teardown would block for the full timeout on each one. print("Stopping wallets") - stop_wallets(self.wallets) - wait_zallets() + try: + if self.wallets is not None: + stop_wallets(self.wallets) + except Exception as e: + print("WARN: error stopping wallets: " + repr(e)) print("Stopping indexers") - stop_zainos(self.zainos) - wait_zainods() + try: + if self.zainos is not None: + stop_zainos(self.zainos) + except Exception as e: + print("WARN: error stopping indexers: " + repr(e)) print("Stopping nodes") - stop_nodes(self.nodes) - wait_bitcoinds() + try: + if self.nodes is not None: + stop_nodes(self.nodes) + except Exception as e: + print("WARN: error stopping nodes: " + repr(e)) + + # Terminate every process we spawned (including any started during + # cache rebuild, which are not tracked by self.*). This sends + # SIGTERM to all of them first and then waits with a bound, so a + # failed setup cannot leave orphaned processes that hang the run, + # and cleanup itself stays fast regardless of how many are running. + stop_all_processes() else: print("Note: zebrads, zainods, and zallets were not stopped and may still be running") diff --git a/qa/rpc-tests/test_framework/util.py b/qa/rpc-tests/test_framework/util.py index ea134b28c..1131edfb0 100644 --- a/qa/rpc-tests/test_framework/util.py +++ b/qa/rpc-tests/test_framework/util.py @@ -290,6 +290,25 @@ def wait_for_bitcoind_start(process, url, i): raise # unknown JSON RPC exception time.sleep(0.25) +# Default regtest network-upgrade activation heights for zebrad whenever the +# harness starts a node that a zallet wallet will sync against (the shared +# 'current' cache built by rebuild_cache(), and the default setup_nodes()). +# +# Zebra (configured via ZebraArgs.activation_heights) and zallet (configured +# separately via `regtest_nuparams` in qa/defaults/zallet/zallet.toml) MUST +# agree on these. If they disagree, wallet sync fails with +# `InvalidData { message: "Missing Orchard tree state" }`: zallet asks zebrad +# for the treestate at a height where zallet believes NU5 is active and so +# requires an Orchard tree, but zebrad (which thinks NU5 is not active there) +# returns none. +# +# Zebra's regtest defaults already activate Overwinter through Canopy at height +# 1 but leave NU5 disabled, whereas zallet.toml activates NU5 at height 1, so we +# explicitly activate NU5 at height 1 here to match. Keep this in sync with the +# `regtest_nuparams` in qa/defaults/zallet/zallet.toml. Tests that need +# different activation heights override setup_nodes() themselves. +REGTEST_ACTIVATION_HEIGHTS = {"NU5": 1} + def initialize_chain(test_dir, num_nodes, cachedir, cache_behavior='current'): """ Create a set of node datadirs in `test_dir`, based upon the specified @@ -344,6 +363,11 @@ def rebuild_cache(): miner_addresses[i] = miner_address + # Persist the miner address into the cached wallet dir so that tests + # which restore this wallet from the cache can reuse it. + with open(wallet_miner_address_file(datadir), "w", encoding="utf8") as f: + f.write(miner_address.strip()) + # Create cache directories, run bitcoinds: block_time = int(time.time()) - (200 * PRE_BLOSSOM_BLOCK_TARGET_SPACING) for i in range(MAX_NODES): @@ -351,6 +375,7 @@ def rebuild_cache(): config = update_zebrad_conf(datadir, rpc_port(i), p2p_port(i), indexer_rpc_port(i), ZebraArgs( miner_address=miner_addresses[i], + activation_heights=dict(REGTEST_ACTIVATION_HEIGHTS), )) args = [ zcashd_binary(), "-c="+config, "start" ] @@ -513,6 +538,10 @@ def cache_rebuild_required(): if os.path.isdir(node_path): if not os.path.isfile(node_file(cachedir, i, 'cache_config.json')): return True + # A cache from before miner addresses were recorded cannot be + # reused by prepare_wallets_for_mining; force a rebuild. + if not os.path.isfile(wallet_miner_address_file(wallet_dir(cachedir, i))): + return True else: return True return False @@ -684,6 +713,12 @@ def node_file(dirname, n_node, filename): def wallet_dir(dirname, n_wallet): return os.path.join(dirname, "wallet"+str(n_wallet)) +def wallet_miner_address_file(datadir): + # Records the wallet's mining address so a wallet restored from the chain + # cache can be reused (see prepare_wallets_for_mining) instead of being + # re-created, which would fail because the wallet already exists. + return os.path.join(datadir, "miner_address.txt") + def check_node(i): bitcoind_processes[i].poll() return bitcoind_processes[i].returncode @@ -726,7 +761,9 @@ def wait_or_kill(proc): proc.wait() def wait_bitcoinds(): - # Wait for all bitcoinds to cleanly exit + # Wait for all bitcoinds to cleanly exit (stop_nodes already requested a + # `stop` via RPC). Bound the wait and fall back to terminating the process, + # so a node that fails to shut down cleanly cannot hang the run. for bitcoind in list(bitcoind_processes.values()): wait_or_kill(bitcoind) bitcoind_processes.clear() @@ -974,7 +1011,18 @@ def prepare_wallets_for_mining(num_wallets, dirname, binary=None): for i in range(num_wallets): datadir = wallet_dir(dirname, i) if os.path.exists(os.path.join(datadir, "wallet.db")): - raise Exception('Wallet %d already exists, cannot prepare it for mining' % i) + # The wallet was restored from the chain cache (init_from_cache + # copies the wallet dir, including its recorded miner address). + # Reuse it rather than re-creating it; the cached chain mined its + # coinbase to this address, so the test must mine to it too. + miner_address_path = wallet_miner_address_file(datadir) + if not os.path.exists(miner_address_path): + raise Exception( + 'Wallet %d already exists but has no recorded miner address; ' + 'delete the cache to force a rebuild' % i) + with open(miner_address_path, "r", encoding="utf8") as f: + miner_addresses.append(f.read().strip()) + continue zallet = binary[i] @@ -992,6 +1040,11 @@ def prepare_wallets_for_mining(num_wallets, dirname, binary=None): process = subprocess.Popen(args, stdout=subprocess.PIPE, text=True) (miner_address, _) = process.communicate() + # Record the miner address so this freshly-prepared wallet can also be + # reused if it is later captured into a chain cache. + with open(wallet_miner_address_file(datadir), "w", encoding="utf8") as f: + f.write(miner_address.strip()) + miner_addresses.append(miner_address) return miner_addresses @@ -1081,7 +1134,9 @@ def stop_wallets(wallets): del wallets[:] # Emptying array closes connections as a side effect def wait_zallets(): - # Wait for all zallets to cleanly exit + # Wait for all zallets to cleanly exit (stop_wallets already requested a + # `stop` via RPC). Bound the wait and fall back to terminating the process, + # so a wallet that fails to shut down cleanly cannot hang the run. for zallet in list(zallet_processes.values()): wait_or_kill(zallet) zallet_processes.clear() @@ -1223,3 +1278,33 @@ def wait_zainods(): pass wait_or_kill(zainod) zainod_processes.clear() + +def stop_all_processes(): + ''' + Forcibly terminate every zebrad, zainod and zallet process we spawned, + regardless of whether a test data structure still references it. + + This is the failure-path safety net. If setup fails partway through (for + example a wallet that cannot synchronize during cache rebuild), the normal + RPC-based shutdown is skipped, leaving orphaned processes that hold ports + and keep the test's stdout/stderr open. Those orphans make later tests hang + and stop the CI job from finishing until it times out. Killing them here + lets the test process exit so the run fails fast. Safe to call repeatedly + and when no processes are running. + ''' + for processes in (bitcoind_processes, zallet_processes, zainod_processes): + for p in list(processes.values()): + try: + p.terminate() + except Exception: + pass + for p in list(processes.values()): + try: + p.wait(timeout=10) + except Exception: + try: + p.kill() + p.wait(timeout=10) + except Exception: + pass + processes.clear()