Implement z_sendfromaccount, z_proposetransaction, and z_finalizetransaction#531
Implement z_sendfromaccount, z_proposetransaction, and z_finalizetransaction#531dannywillems wants to merge 9 commits into
Conversation
d9e1c03 to
d8b0a0c
Compare
Trying out the new RPC methods
Prerequisitescargo build --release --features rpc-cli # binary at target/release/zallet
zallet start # synced zebrad; wallet unlocked + fundedDiscover an account and a destination addresszallet rpc z_listaccounts # copy a "uuid"
zallet rpc z_getaddressforaccount '"<ACCOUNT_UUID>"'1. Propose → inspect → finalize (the PCZT flow)
# fund_source: "orchard" | "sapling" | "any_transparent" | ["t-addr", ...]
zallet rpc z_proposetransaction \
'"<ACCOUNT_UUID>"' \
'"orchard"' \
'[{"address":"<DEST_ADDRESS>","amount":"0.002"}]' \
'1'Returns
zallet rpc z_finalizetransaction \
'"<ACCOUNT_UUID>"' \
'"<PCZT_HEX>"' \
'"<PRIVACY_POLICY>"'The 2. One-shot send
zallet rpc z_sendfromaccount \
'"<ACCOUNT_UUID>"' \
'"orchard"' \
'[{"address":"<DEST_ADDRESS>","amount":"0.002"}]' \
'1' \
'"AllowRevealedAmounts"'Notes
Raw JSON-RPC alternativecurl -s --user "<rpcuser>:<rpcpass>" -H 'content-type: application/json' \
--data '{"jsonrpc":"2.0","id":1,"method":"z_sendfromaccount","params":["<ACCOUNT_UUID>","orchard",[{"address":"<DEST_ADDRESS>","amount":"0.002"}],1,"AllowRevealedAmounts"]}' \
http://127.0.0.1:<rpc-port>/This usage guide was generated by Claude (Claude Code). |
zcash/zallet#531 adds z_sendfromaccount, which sends from a single account with its inputs restricted to a caller-named fund source ("orchard", "sapling", "any_transparent", or an array of transparent addresses) in one shot, requiring an explicit privacy_policy. Add wallet_z_sendfromaccount.py against the Z3 stack. It covers the synchronous argument validation (recipients, account, every fund_source error, and the required privacy_policy, matched to the backend's exact messages) and the functional paths: an Orchard-sourced shielded send, the fund-source restriction excluding a pool the account has no notes in, and transparent-sourced sends via both "any_transparent" and an explicit single-address array. The account is funded via z_shieldcoinbase and balance movement is checked against the transaction fee. Register it in the NEW_SCRIPTS group. Co-Authored-By: Claude <noreply@anthropic.com>
zcash/zallet#531 adds z_sendfromaccount, which sends from a single account with its inputs restricted to a caller-named fund source ("orchard", "sapling", "any_transparent", or an array of transparent addresses) in one shot, requiring an explicit privacy_policy. Add wallet_z_sendfromaccount.py against the Z3 stack. It covers the synchronous argument validation (recipients, account, every fund_source error, and the required privacy_policy, matched to the backend's exact messages) and the functional paths with exact per-account balances via z_getbalances: Orchard-sourced sends (single, multi-recipient, memo, and a too-high minconf), and Sapling-sourced sends (rejected while the pool is empty, then a funded send). The transparent fund sources (any_transparent / [taddr]) are skipped pending a zallet#531 fix: propose_transfer selects no transparent inputs even for a spendable non-coinbase UTXO. Separately, coinbase is only ever spendable via z_shieldcoinbase, never selected by a transfer. Register the test in the NEW_SCRIPTS group. Co-Authored-By: Claude <noreply@anthropic.com>
zcash/zallet#531 adds the PCZT send flow: z_proposetransaction builds an unsigned PCZT from an account's funds (restricted to a fund_source) and reports the strictest privacy policy the transaction needs, and z_finalizetransaction signs, proves, and broadcasts that PCZT after the caller acknowledges a sufficient privacy policy. Add wallet_z_proposetransaction.py against the Z3 stack covering both RPCs: z_proposetransaction validation (recipients, account, and fund_source errors), z_finalizetransaction validation (privacy_policy parsing and PCZT hex/structure decoding), and a funded propose -> finalize round trip with exact per-account balances via z_getbalances. The round trip exercises an Orchard -> Orchard send (required policy FullPrivacy) and the finalize-time required-policy enforcement via an Orchard -> transparent-recipient de-shield (finalizing under FullPrivacy is rejected, and under the policy the proposal reported it succeeds). A transparent-source variant is deferred pending a zallet#531 fix (propose_transfer selects no transparent inputs); PCZTs also cannot create Sapling outputs at this low regtest height (ZIP 212). Register the test in NEW_SCRIPTS. Co-Authored-By: Claude <noreply@anthropic.com>
Integration testing: transparent
|
zcash/zallet#531 adds z_sendfromaccount, which sends from a single account with its inputs restricted to a caller-named fund source ("orchard", "sapling", "any_transparent", or an array of transparent addresses) in one shot, requiring an explicit privacy_policy. Add wallet_z_sendfromaccount.py against the Z3 stack. It covers the synchronous argument validation (recipients, account, every fund_source error, and the required privacy_policy, matched to the backend's exact messages) and the functional paths with exact per-account balances via z_getbalances, exercising every fund_source type: - Orchard: single, multi-recipient, memo, and a too-high minconf. - Sapling: rejected while empty, then a funded Sapling-sourced send. - any_transparent / [taddr] array: refused under FullPrivacy, then successful transparent -> transparent sends under AllowFullyTransparent, plus an array restricted to an unfunded address (failure). A transparent fund source can only pay transparent recipients (a single-step proposal cannot carry an ephemeral shielded change output), and coinbase is only spendable via z_shieldcoinbase, so the transparent tests spend non-coinbase UTXOs created by de-shielding to the account's own taddrs. Register the test in the NEW_SCRIPTS group. Co-Authored-By: Claude <noreply@anthropic.com>
zcash/zallet#531 adds the PCZT send flow: z_proposetransaction builds an unsigned PCZT from an account's funds (restricted to a fund_source) and reports the strictest privacy policy the transaction needs, and z_finalizetransaction signs, proves, and broadcasts that PCZT after the caller acknowledges a sufficient privacy policy. Add wallet_z_proposetransaction.py against the Z3 stack covering both RPCs: z_proposetransaction validation (recipients, account, and fund_source errors), z_finalizetransaction validation (privacy_policy parsing and PCZT hex/structure decoding), and a funded propose -> finalize round trip with exact per-account balances via z_getbalances. The round trip exercises an Orchard -> Orchard send (required policy FullPrivacy) and the finalize-time required-policy enforcement via an Orchard -> transparent-recipient de-shield (finalizing under FullPrivacy is rejected, and under the policy the proposal reported it succeeds). A transparent recipient is used rather than Sapling because PCZTs cannot create Sapling outputs at this low regtest height (ZIP 212). The transparent fund sources share this propose_transfer path and are exercised end to end in wallet_z_sendfromaccount.py. Register the test in NEW_SCRIPTS. Co-Authored-By: Claude <noreply@anthropic.com>
|
Verified fixed by
One behavioural note the tests now encode: a transparent fund source pays transparent recipients only — sending to a shielded address is rejected with "a transparent fund source supports only transparent recipients; use a shielded fund source to send to shielded addresses" (single-step proposals can't carry an ephemeral shielded change output). The tests create non-coinbase transparent UTXOs by de-shielding (coinbase remains spendable only via The transparent positive tests are no longer skipped; the |
zcash/zallet#531 adds the PCZT send flow: z_proposetransaction builds an unsigned PCZT from an account's funds (restricted to a fund_source) and reports the strictest privacy policy the transaction needs, and z_finalizetransaction signs, proves, and broadcasts that PCZT after the caller acknowledges a sufficient privacy policy. Add wallet_z_proposetransaction.py against the Z3 stack covering both RPCs: z_proposetransaction validation (recipients, account, and fund_source errors), z_finalizetransaction validation (privacy_policy parsing and PCZT hex/structure decoding), and a funded propose -> finalize round trip with exact per-account balances via z_getbalances. The round trip exercises an Orchard -> Orchard send (required policy FullPrivacy) and the finalize-time required-policy enforcement via an Orchard -> transparent-recipient de-shield (finalizing under FullPrivacy is rejected, and under the policy the proposal reported it succeeds). It also covers edge cases of the propose/finalize split: finalizing a proposal under a more permissive policy than it requires succeeds, while finalizing with the wrong account or a tampered PCZT is rejected. A double-spend case (two proposals over the same notes) is skipped for now: finalizing the second broadcasts a transaction the network rejects, which currently crashes zallet's sync task. A transparent recipient is used rather than Sapling because PCZTs cannot create Sapling outputs at this low regtest height (ZIP 212). The transparent fund sources share this propose_transfer path and are exercised end to end in wallet_z_sendfromaccount.py. Register the test in NEW_SCRIPTS. Co-Authored-By: Claude <noreply@anthropic.com>
Bug found while adding adversarial tests: a rejected/double-spend transaction crashes the whole walletWhile writing edge-case tests for the propose/finalize split, I hit a reproducible crash that takes down the entire Repro (regtest, Z3 stack):
Observed: the second finalize does not detect that the inputs are already spent. The network rejects the broadcast, and then the wallet's sync task dies: All subsequent RPCs get connection-refused — the daemon is gone. Impact: any transaction the wallet broadcasts but the network then rejects (a double-spend here, but plausibly any conflict/eviction) is treated as Suggested fixes (either/both):
Test status: I added R4 (finalizing under a more-permissive policy succeeds), R5 (wrong-account finalize rejected), and R6 (tampered PCZT rejected) to |
86c16a0 to
003e1bb
Compare
`z_sendmany` passed `SpendPolicy::default()` to `propose_transfer`, which permits the shielded pools and no transparent spending, so no proposal could ever select a transparent input. Transparent-to-transparent transfers were therefore impossible, and the `AllowFullyTransparent` privacy policy was unreachable even though `enforce_privacy_policy` already implemented it. Derive the spend policy from the source address instead. A bare transparent `fromaddress` now permits transparent spending restricted to that one address's UTXOs (`TransparentSpendPolicy::from_one_address`), so a shielded send can never silently reach into transparent funds, and the named address is not linked to the account's other transparent receivers. Every other source stays shielded-only. Coinbase UTXOs remain excluded, since consensus requires them to be spent to a single shielded output; that is still `z_shieldcoinbase`'s job. Allow transparent change, so a fully transparent send keeps its change in the transparent pool at an internal-scope BIP 44 address rather than sweeping it into Orchard. The change strategy only emits transparent change when the net flows are fully transparent, so shielded flows are unaffected. Three gaps had to be closed for this to work. All were latent, because nothing could reach them while transparent spending was impossible: - `get_account_for_address` compared the source against `list_addresses`, which yields unified addresses. A bare transparent receiver never compares equal to one, so a taddr `fromaddress` always failed with "no payment source found for address". Resolve it with `find_account_for_address`, which maps an address back to its account through the receivers. - `DbConnection` did not forward `InputSource::select_spendable_transparent_outputs` or `WalletWrite::reserve_next_n_internal_addresses`. Both have `unimplemented!()` defaults, so selecting transparent inputs and producing transparent change each panicked the wallet process rather than returning an error. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Integration and scenario tests belong in zcash/integration-tests, which drives the whole Z3 stack over real RPC against a regtest chain. A test here that stands up a chain, mines blocks, or sends a transaction end to end is in the wrong repo. State what does belong here (pure derivations, parameter parsing and validation, error mapping, response shapes, property tests), and note the corollary: if a piece of logic can only be tested by building a chain, that is usually a sign it should be extracted into a pure function that can be unit-tested, with the end-to-end behaviour covered in integration-tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`z_sendmany` inlines the Orchard action-limit check and owns the private `run` that builds, signs, and broadcasts a proposal. The account-based transaction methods need both: they build a proposal the same way and must enforce the same limit, and re-implementing either would let the two paths drift. Lift the limit check into `check_orchard_actions_limit`, which takes any proposal rather than reaching into `z_sendmany`'s locals, and make `run` visible to its sibling methods. No behaviour change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
`z_sendmany` builds its proposal inline: fee rule, change strategy, dust and split policies, input selector, and the `propose_transfer` call. The account-based transaction methods build the same proposal from the same pieces, and differ only in which funds they may draw upon. Lift it into `propose_transfer_with_policy`, taking the `SpendPolicy` as its one varying input. `z_sendmany` derives that policy from its source address; the account methods will derive it from `fund_source`. Everything else about the proposal is then shared by construction rather than by two copies agreeing. `transparent_change_policy_for` moves with it, since it is a property of the spend policy rather than of `z_sendmany`. No behaviour change. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
003e1bb to
0059377
Compare
Proposes a transaction from an account and returns it as a PCZT, alongside the
privacy policy required to execute it. Nothing is signed, no proof is generated,
and no key is touched: the caller inspects what the transaction would reveal and
decides, rather than having to commit to a privacy policy up front as `z_sendmany`
demands.
The `fund_source` parameter names where the account's funds may be drawn from:
`orchard`, `sapling`, `any_transparent`, or an explicit list of transparent
addresses. It is translated into a `SpendPolicy`, which is what the proposal
builder already consumes, so the account methods select inputs through exactly the
same path as `z_sendmany` rather than a second implementation of selection.
That translation is the whole of `fund_source`. This is worth spelling out because
it is where the transparent-spend work pays off: `zcash_client_backend` could not
select transparent inputs when these methods were first written, so the original
implementation carried its own transparent input selector and an `InputSource`
wrapper to filter notes by pool, around a thousand lines in total. librustzcash
now does both -- `TransparentSpendPolicy` names which transparent addresses may be
drawn upon, and `SpendPolicy::shielded_pools` restricts the shielded pools -- so
all of it collapses into a `match` from `FundSource` to `SpendPolicy`, and the
`if spends_transparent { .. } else { .. }` fork in each method disappears.
`fund_source: "orchard"` permits the Ironwood pool as well as Orchard. Ironwood
notes are Orchard-shaped, and once NU6.3 activates an account's Orchard-receiver
funds are held there rather than in the legacy Orchard pool, so restricting the
source to Orchard alone would report insufficient funds for an account whose
Orchard money is all in Ironwood.
The required privacy policy is recorded against the PCZT's content hash so that
`z_finalizetransaction` can hold the caller's acknowledgement against what the
transaction actually reveals, rather than re-deriving it from the PCZT, which does
not carry enough information to do so faithfully.
Enables the `pczt` feature of `zcash_client_backend` and the `serde` feature of
`zcash_client_sqlite` (`create_pczt_from_proposal` serializes the account id), and
adds the `pczt` crate at the librustzcash rev the workspace already pins, in all
three manifests so the lockstep check still sees one rev.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sends funds from an account in one shot: propose, enforce the privacy policy, sign, and broadcast, returning the txid rather than an operation id. The source of funds is an account UUID plus a `fund_source`, rather than an address as in `z_sendmany`. Because the send is executed immediately rather than proposed for review, the privacy policy is required rather than optional: the caller has no opportunity to inspect what the transaction reveals, so they must acknowledge it up front. The proposal is built by the shared `propose_transfer_with_policy`, so there is no separate transparent-spend path here: `fund_source.spend_policy()` is the whole of the difference from `z_sendmany`. `SendResult` gains the `Documented` and `JsonSchema` derives that the OpenRPC generator requires of a method result type; it was previously only used as an async-operation payload, which is not part of that surface. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Takes a PCZT from `z_proposetransaction`, applies this account's proof generation keys, proofs, and spend authorizing signatures, extracts the transaction, and broadcasts it. Shielded spends do not say which of them belong to this account, so each candidate signature is attempted and wrong-key errors are ignored, following the reference driver in `zcash_client_backend`. Transparent inputs, by contrast, identify themselves through their BIP 44 derivation, so the seed fingerprint and coin type pick out this account's inputs and the corresponding key is derived to sign each. The privacy policy the caller supplies is held against the policy `z_proposetransaction` recorded for this PCZT, so a caller cannot acknowledge a weaker policy than the transaction actually requires. On a cache miss (eviction, restart, or a PCZT proposed elsewhere) the requirement cannot be re-derived faithfully from the PCZT, so the supplied policy is accepted as acknowledgement only; see #217. The Orchard proving and verifying keys are deterministic and take seconds to build, so they are built once and shared. Proving, signing, and verification are CPU-bound and run on the blocking pool. `bip32` is taken at the 0.6 line rather than through the workspace: the workspace entry is the 0.2 line used by the zcashd-import path, and `extract_bip_44_fields` speaks the newer one. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
0059377 to
5d61c6b
Compare
e864d11 to
5f542ba
Compare
Stacked on #585 (transparent spending in
z_sendmany), which is stacked on #586 (Docker fix). Base auto-retargets tomainas each merges.History rebuilt. The previous 13 commits predated the backend split (they edited
zallet/src/…, which is nowzallet-core/src/…) and predated several of their own pieces landing upstream, so they could not be rebased. This is the same work rebuilt as 6 commits on the current tree.What this adds
Three account-based transaction methods. Where
z_sendmanytakes an address as its source of funds, these take an account UUID plus afund_sourcenaming where within the account the funds may be drawn from:"orchard","sapling","any_transparent", or an explicit array of transparent addresses.z_proposetransactionz_finalizetransactionz_sendfromaccountThe split exists so a caller can see what the transaction would reveal and then decide, instead of having to commit to a privacy policy before knowing, as
z_sendmanydemands.The interesting part:
fund_sourceis just aSpendPolicyThis is where landing transparent spending first pays off, and it is most of the review.
zcash_client_backendcould not select transparent inputs when these methods were first written. So the original implementation carried its own transparent input selector (propose_transparent_spend) and anInputSourcewrapper to filter notes by pool (FundSourceFilter) — around 1097 lines, with each RPC forking onif spends_transparent { … } else { … }.librustzcash now does both:
TransparentSpendPolicynames which transparent addresses may be drawn upon, andSpendPolicy::shielded_poolsrestricts the shielded pools. So all of it collapses into amatch:fund_source.rsis now ~120 lines, the hand-rolled selector and theInputSourcewrapper are gone, and both forks disappear. There is one proposal path —propose_transfer_with_policy— whichz_sendmanyalso goes through, so the account methods differ from it only in theSpendPolicythey ask for. Two of the six commits exist to make that sharing real rather than incidental.What
fund_sourceguaranteesIt isolates. A source that cannot cover the payment reports insufficient funds; it never quietly reaches into another pool. Naming
saplingwhen the money is in Orchard must fail, or "spend my Sapling" silently spends Orchard — the accounting error the parameter exists to prevent. Same for an address list: sweeping one deposit address must not reach into a sibling address of the same account.Both are pinned by tests (zcash/integration-tests#145,
FS1/FA2and two dedicated scenarios) precisely because a fallback bug would pass every happy path and only surface as an accounting error.fund_source: "orchard"permits Ironwood too. Ironwood notes are Orchard-shaped, and once NU6.3 activates an account's Orchard-receiver funds are held there rather than in the legacy Orchard pool. Restricting toShieldedPool::Orchardalone would report insufficient funds for an account whose Orchard money is all in Ironwood. The original branch predates Ironwood and never faced this; flagging it as a deliberate choice.Already upstream, dropped
7 of the original 13 commits: the
AmountParameter/build_requestextraction, its proptests,required_privacy_policy, the NU6.3 regtest activation height, and the librustzcash/zebra pin bumps —mainis now newer than what the branch pinned.Dependencies
pcztcrate, at the librustzcash rev the workspace already pins, added to all three manifests socheck-lockstep.shstill sees one rev.pcztfeature onzcash_client_backend, andserdeonzcash_client_sqlite(create_pczt_from_proposalserializes the account id).bip32at the 0.6 line inzallet-core: the workspace entry is the 0.2 line used by the zcashd-import path, andextract_bip_44_fieldsspeaks the newer one.Known weakness, ported as-is
z_finalizetransactionchecks the caller's policy against the one recorded for that PCZT at proposal time. On a cache miss (eviction, restart, or a PCZT proposed elsewhere) the requirement cannot be re-derived faithfully from the PCZT, so the supplied policy is accepted as acknowledgement only — see #217. This is the existing design; I ported it faithfully rather than silently changing the security posture, but it is a real gap: a caller who proposes elsewhere can under-acknowledge.Testing
Clippy clean, 205
zallet-coreunit tests pass (8 new forfund_source, incl. proptests over arbitrary seeds), all three workspaces build, librustzcash rev in lockstep across the three lockfiles.Covered end to end by zcash/integration-tests#145, all passing against a zallet built from this branch: parameter validation, every fund source, propose/review/finalize, and 11 real-world scenarios.
ZIT-Revision: dw/sendfromaccount