[#1924] Split governance PCZT API for Keystone vs software wallets#1954
Merged
Conversation
e46377c to
5c12b8c
Compare
5c12b8c to
cefc71a
Compare
czarcas7ic
reviewed
May 14, 2026
Hotkey material now derives from a named account-0 constant instead of accepting or reusing the caller's wallet account index. This matches zcash_voting's vote construction and signing behavior, where the hotkey spending key is derived through derive_spending_key and therefore fixed to account 0. The seed-based governance PCZT path still uses the caller's account index for wallet seed to UFVK validation and PCZT ZIP-32 metadata, but uses the hotkey account constant for the hotkey raw address. The JNI, typesafe SDK, and tests now expose deriveHotkeyRawAddress without an accountIndex parameter, and a nonzero wallet-account instrumentation test verifies that explicit and seed PCZT paths remain aligned.
greg0x
previously approved these changes
May 14, 2026
The split between `buildGovernancePczt` (Keystone path, trusts caller-derived fvk + hotkey address) and `buildGovernancePcztFromSeed` (software path, validates fvk against the wallet seed and re-derives the hotkey from a fixed account index) carries real invariants but had no docs. KDoc now records both, including the warning that adding an `accountIndex` parameter to `deriveHotkeyRawAddress` would silently desync delegation construction from vote signing. `build_governance_pczt_explicit_and_seed_paths_match` asserted byte-identical PCZTs across the two paths, which is stronger than what the implementation actually guarantees — the two paths are not required to produce the same PCZT, only valid ones. Replaced with `assertValidGovernancePczt`, which checks the structural invariants that do hold: field-sized rk/sighash, a valid action index, sighash re-extractable from the PCZT bytes, and `DELEGATION_CONSTRUCTED` reached. Also adds coverage for `deriveHotkeyRawAddress` (determinism, seed/network isolation, short-seed rejection) and for the typesafe forwarding of both governance PCZT methods, which were previously stubbed as `unused()` in the recording backend.
nullcopy
requested changes
May 14, 2026
nullcopy
approved these changes
May 14, 2026
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
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.
Summary
Splits the governance PCZT JNI into two explicit paths so Keystone (and any other hardware-wallet integration that cannot expose the wallet seed) can drive a delegation round, while software wallets keep their UFVK/walletSeed validation invariant.
Why this shape
The previous
buildGovernancePcztJNI requiredwalletSeedand validated, inside Rust, that the seed derived the account UFVK before constructing the PCZT. That contract is correct for software wallets but is unsatisfiable on Keystone, which never surfaces the wallet seed. The Android Keystone integration was working around it by passing the hotkey seed as the wallet seed, which then failed withufvk does not match walletSeed for network_id=1, account_index=0on a real round, immediately after also trippingbundle_index 1 is not present in note bundle setbecause the call site was pre-filtering the note set to a single bundle (the Rust side re-chunks internally from the full snapshot usingbundle_index).We considered three alternatives before landing this shape:
walletKindflag. Rejected: it forces the FFI to know about wallet flavors, hides which inputs are actually consumed in each branch, and still requires an explicit hotkey-address path for hardware wallets.This also matches the iOS SDK contract (
zcash-swift-wallet-sdk), which already exposes the explicit-FVK path for hardware-wallet flows.What changed
buildGovernancePcztnow takes pre-derivedfvkBytes(Orchard FVK) andhotkeyRawAddress. This is the path Keystone uses; the host derives the FVK from the UFVK and the raw hotkey address from the hotkey seed before calling.buildGovernancePcztFromSeedis the new software-wallet entry point. It takesufvk,walletSeed,hotkeySeedand keeps the original UFVK<>walletSeed validation, then derives the hotkey raw address internally.build_governance_pczt_for_bundle, which validates the bundle index, enforces the round phase, persists the constructed delegation state, and only advances the phase toDelegationConstructedon success.buildAndProveDelegationnow takeshotkeyRawAddressdirectly. Keystone does not need to surface the hotkey seed beyond derivation, and software wallets derive once via the new helper.deriveHotkeyRawAddressJNI exposes raw-address derivation to the app layer so the Keystone path can stay seed-free end-to-end.TypesafeVotingBackend(Impl)and the JNI/SDK androidTest harnesses.Manual testing
This was manually tested end-to-end on a Keystone hardware wallet: entering a poll no longer raises
bundle_index 1 is not present in note bundle set, and constructing the delegation no longer raisesufvk does not match walletSeed. The software-wallet path was exercised through the existing app flow as well.Author
Reviewer