test framework: cover the account-based transaction methods#145
Draft
dannywillems wants to merge 10 commits into
Draft
test framework: cover the account-based transaction methods#145dannywillems wants to merge 10 commits into
dannywillems wants to merge 10 commits into
Conversation
dannywillems
marked this pull request as draft
June 30, 2026 15:56
dannywillems
force-pushed
the
dw/sendfromaccount
branch
4 times, most recently
from
June 30, 2026 19:33
a6140e5 to
e327192
Compare
Zallet's `z_sendmany` can now spend transparent funds, so a t-to-t transfer is testable for the first time. Nothing in the running suite exercised it: the only enabled tests that send anything drive `z_shieldcoinbase` (t-to-z) or a shielded `z_sendmany`. Add `wallet_transparent_spend.py`, and reactivate the two disabled tests whose subject is transparent spending, migrating them from the zcashd RPCs to the Z3 stack. Both were disabled only for the account/UA migration (`getnewaddress` -> `z_getaddressforaccount`, `sendtoaddress` -> `z_sendmany`): - `wallet_changeaddresses.py` asserts that two consecutive transparent sends do not reuse a change address, which would publicly link them. - `regtest_signrawtransaction.py` spends a transparent input, which only builds if it is signed under the target height's consensus branch id. The name is historical; Zallet signs transparent inputs internally. On regtest all transparent funds start as coinbase, and coinbase cannot be spent transparently, so each test first shields coinbase and unshields to obtain a non-coinbase UTXO. Two zcashd behaviours no longer hold and are pinned as such rather than dropped: a transparent source must be named explicitly (`ANY_TADDR` is unsupported), and the change of a transparent-to-shielded send is shielded rather than returned to the transparent pool. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The three transparent-spend tests each carried a private copy of the same two helpers, and open-coded the same walk over a transaction's transparent outputs. Move them into the test framework so there is one definition each: - `unified_address_for` derives a UA for an account at a pinned diversifier index. An index may only ever be used with one receiver set, so a test wanting several addresses varies the index, not the receivers; the helper documents that, which is the trap the diversifier constants exist to avoid. - `transparent_output_addresses` reads a transaction's transparent output addresses from the node, so assertions reflect what was committed on-chain rather than the wallet's view of it. - `transparent_change_address` picks the change output out of a fully transparent send, asserting the recipient-plus-change shape on the way. That shape assertion is the point: it fails if the change was shielded instead. Add a `Receiver` enum for unified-address receiver types, alongside the existing `Pool` / `PrivacyPolicy` / `TotalBalanceField` enums, and use it in `first_transparent_receiver`. It is deliberately not a `Pool`: `p2pkh` and `p2sh` are distinct receiver kinds that share the single transparent pool. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The test runner execs each script directly, so a script without the executable bit fails CI before it runs. Every other script in qa/rpc-tests is 100755; this one was committed 100644. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The transparent-spend tests passed bare `1` and `None` as the positional minconf
and fee arguments of z_sendmany. A reader has to count parameters to know what
they are, and a comment at the call site would not help: it is invisible to every
other caller and rots on the next refactor.
Name them once, at the definition. `MIN_CONFIRMATIONS` and `INTERNAL_FEE` join
the framework constants in util.py, documented there, so a call reads as intent:
w.z_sendmany(taddr, recipients, MIN_CONFIRMATIONS, INTERNAL_FEE, policy)
`INTERNAL_FEE` is deliberately not `zip317.ZIP_317_FEE`. Both are None, but that
one is documented as the zcashd sentinel for "use the conventional fee", whereas
zallet computes the fee itself and REQUIRES the argument to be null. Same value,
different reason; conflating them would mislead.
Record the rule in AGENTS.md, along with the two related ones this branch already
follows: prefer an enum to a bare string, and hoist a shared helper into util.py
rather than copying it between tests.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
…ble it The zcashd original leaned on RPCs zallet does not implement (z_exportviewingkey, z_getbalanceforviewingkey, sendtoaddress, getwalletinfo's shielded-balance fields) and on a Sprout section that has no Z3 equivalent at all, which is why it was disabled rather than migrated. Rewrite it against the account/UA API: balances come from z_getbalances rather than a viewing key, addresses from z_getaddressforaccount, and funding from z_shieldcoinbase rather than sendtoaddress. What survives is what the test was always for: that z_sendmany credits the recipient exactly, that the privacy policy is enforced, and that a source can pay across pools. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Covers the RPC surface: parameter validation (recipients, account, fund_source, privacy_policy, checked in that order), and each fund source end to end with exact per-account balances. The two properties worth naming, because they are what `fund_source` is for: - A fund source ISOLATES. Asking for `sapling` when the account holds only Orchard notes reports insufficient funds rather than quietly spending the Orchard (FS1), and naming a transparent address with no funds does the same (FA2). An account being solvent overall is not enough. - Coinbase is never selected. Consensus requires it to be spent to a single shielded output, so a transparent fund source spends only NON-coinbase UTXOs; the test manufactures those by shielding coinbase and de-shielding. The memo test sends a UTF-8 text memo. A memo whose first byte is below 0xF5 is a text memo (ZIP 302) and its bytes must be valid UTF-8; zallet does not check that when sending but does when reading the note back, so a malformed memo makes z_listunspent fail for the whole wallet. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Covers the two-step flow: propose returns an inspectable PCZT and the privacy policy the transaction requires; finalize signs and broadcasts it. The point of the split is that the caller decides AFTER seeing what the transaction reveals, so the test pins that finalize holds them to it: a policy weaker than the proposal reported is refused (R2), a more permissive one is accepted (R4), a PCZT signed by the wrong account is rejected (R5), and a tampered PCZT is rejected (R6). R3 (double-spending one set of notes via two proposals) is skipped: the second finalize crashes zallet's sync task, which is an upstream bug rather than something this test should assert. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Where the per-RPC tests cover the API surface, these exercise the workflows the fund-source feature exists for, driven the way an exchange, an OTC desk, a payroll operator, or an individual would drive them: batch customer withdrawals, a review-then-settle OTC settlement, a payroll run with memos, a merchant payment that de-shields to a t-addr, a deposit sweep from named addresses, a consolidation of all transparent reserves, and a payout from a legacy Sapling pool. Four scenarios exist to pin the guarantees rather than the happy path, because each is a way the feature could appear to work while being wrong: - A fund source ISOLATES its pool: asking for Sapling when the money is in Orchard must fail, not silently spend the Orchard. A fallback bug would pass every happy path and only show up as an accounting error. - An address list RESTRICTS: sweeping one user's deposit address must not reach into a sibling address, even though both belong to the account and together hold enough. The test proves the refusal is the restriction working and not a shortfall, by then paying the same amount from both addresses. - A proposal has NO side effects: one that is never finalized moves nothing and locks nothing. - Finalize HOLDS THE CALLER to the policy the proposal reported, so a caller cannot under-acknowledge what the transaction reveals. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
dannywillems
force-pushed
the
dw/sendfromaccount
branch
from
July 11, 2026 21:21
e327192 to
f7cd1ef
Compare
dannywillems
changed the title
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.
Stacked on #163 (transparent-to-transparent coverage). Base auto-retargets to
mainwhen that merges.Depends on zcash/zallet#531, which implements the three RPCs. These tests fail against zallet
main.History rebuilt. The previous branch's first commit extracted the shared Z3-stack wallet-test helpers into
util.py— that has since landed upstream, so it is dropped. What remains is rebuilt on the current tree.What this covers
Three new tests plus one rewrite, for
z_sendfromaccount/z_proposetransaction/z_finalizetransaction.wallet_z_sendfromaccount.py— the RPC surface. 13 validation tests (recipients, account,fund_source,privacy_policy, checked in that order), then each fund source end to end with exact per-account balances:orchard(single, multi-recipient, memo, minconf),sapling,any_transparent, and an explicit address list.wallet_z_proposetransaction.py— the two-step flow. Propose returns an inspectable PCZT and the policy the transaction requires; finalize signs and broadcasts it. The point of the split is that the caller decides after seeing what the transaction reveals, so the test pins that finalize holds them to it: a weaker policy than reported is refused, a more permissive one is accepted, a PCZT signed by the wrong account is rejected, and a tampered PCZT is rejected.wallet_z_send_scenarios.py— the workflows the feature exists for, driven as an exchange, an OTC desk, a payroll operator, or an individual would drive them: batch customer withdrawals, a review-then-settle settlement, payroll with memos, a merchant payment that de-shields to a t-addr, a deposit sweep from named addresses, a consolidation of all transparent reserves, and a payout from a legacy Sapling pool.wallet_z_sendmany.py— rewritten and re-enabled. It was disabled (not merely unmigrated) because the zcashd original leaned onz_exportviewingkey,z_getbalanceforviewingkey,sendtoaddress, and a Sprout section with no Z3 equivalent. It now uses the account/UA API.The four scenarios that matter
Most of the scenarios are happy paths. Four exist to pin the guarantees, because each is a way the feature could appear to work while being wrong:
saplingwhen the money is in Orchard must fail, not silently spend the Orchard. A fallback bug would pass every happy path and surface only as an accounting error.A zallet bug this surfaced
The ported memo test sent
deadbeef…as a memo. Per ZIP 302 a memo whose first byte is below0xF5is a UTF-8 text memo, and those bytes are not valid UTF-8. Zallet accepts such a memo when sending (it takes the raw bytes) but fails to decode it when reading the note back — soget_memoerrors andz_listunspentthen fails for the entire wallet, permanently.The test now sends valid text, which is what a real caller does. But the asymmetry is a genuine zallet bug (a single malformed memo, from any source, bricks the RPC) and it is pre-existing rather than introduced by #531, so it is not fixed here. Worth an issue.
R3(double-spending one set of notes via two proposals) is skipped: the second finalize crashes zallet's sync task. That is an upstream bug, not something this test should assert.Testing
All four pass against a zallet built from zcash/zallet#531.
pyflakesclean across the suite.Per the conventions added in #163: RPC arguments are named constants (
MIN_CONFIRMATIONS,INTERNAL_FEE) rather than bare literals, enums (FundSource,Pool,PrivacyPolicy,Receiver) rather than strings, shared helpers hoisted intoutil.pyrather than copied, and type annotations throughout.