Zcash upstream#2192
Open
soralit wants to merge 16 commits into
Open
Conversation
Pure code reorganization ahead of Ironwood support, on the existing Zcash dependencies: - move validate_sapling_bundle_consistency into pczt/mod.rs behind a shared validate_supported_pczt entry point - extract transparent_derivation_matches_selected_account and map_orchard_verifier_error helpers - have check_pczt_transparent report whether a wallet input was found - thread a pool_label through the Orchard check helpers - generalize the shielded-pool renderer to GuiZcashOverviewShielded The original test suite is retained and passes on the current dependencies (cypherpunk: 73 tests, multi_coins: 38 tests), showing the refactor introduces no behavior change. The dependency bump and the Ironwood feature follow in later commits.
Pin the Zcash crates to the Ironwood fork revisions (orchard via zcash/orchard, the zcash_*/pczt crates via the valargroup fork) using [patch.crates-io], enable the zcash_unstable="nu6.3" cfg, and apply the mechanical API migrations the bump forces: - TransparentAddress::from_script_from_chain / from_script_pubkey - Note::from_parts note-version argument, SignableInput::from_parts - pczt_ext ZIP-244 sighash digest adjustments Tests embedding the old PCZT wire format or the pre-bump builder API are dropped here (they no longer parse/compile); equivalents against the new API are restored with the Ironwood feature in the following commit. NOTE: this commit does not build in isolation. The parse.rs/sign.rs API migrations are interleaved with the Ironwood parsing/signing changes and land together in the next commit; this is split out purely to isolate the dependency bump for review.
Add cypherpunk Zcash PCZT parse/check/sign support for v6/Ironwood while preserving existing Orchard and transparent behavior. Legacy multi_coins paths reject v6/Ironwood PCZTs instead of mis-handling them. Signed responses stamp the firmware version and redact optional PCZT fields (including Ironwood bundle data) before returning QR-sized results. Restores, against the new builder API, the runtime-constructed regression tests dropped during the dependency bump: - internal-OVK change-spoofing rejection (parse + check) - empty Sapling bundle with non-zero value sum rejection - Orchard spend/output value + ownership decode (parse + check)
Bring the Ironwood PCZT work up to date with master (v2.4.8 + vizor wallet).
master reverted the Zcash firmware-version feature (deleted version.rs and
build.rs, stripped the fw-version stamping from sign.rs) after this branch was
cut. This work depends on it — Ironwood signed responses stamp
KEYSTONE_FW_VERSION — so the merge keeps this branch's version of:
rust/apps/zcash/{build.rs, src/version.rs, src/lib.rs, src/pczt/sign.rs}
re-applying the firmware-version feature on top of master's revert. All other
master changes (vizor wallet, GUI fixes, SE config) are taken as-is.
Validated on the merge result:
- cargo test -p app_zcash --no-default-features --features cypherpunk (78 pass)
- cargo test -p app_zcash --no-default-features --features multi_coins (38 pass)
- cargo check -p rust_c --no-default-features --features simulator-cypherpunk
- cargo check -p rust_c --no-default-features --features simulator-multi-coins
Replace the `pool_label: &str` threaded through the Orchard/Ironwood check, parse, and sign helpers with the existing `ShieldedPool` enum, lifted into `pczt/mod.rs` as its shared home. Rename `check_orchard` to the pool-agnostic `check_shielded_bundle`. Each helper derives the label from the enum internally, so all error messages are unchanged. Addresses review feedback that the customizable string pool label was confusing next to the always-Orchard bundle type, and that an Orchard-named helper was being used for the Ironwood pool.
…n helper
Every caller of the helper already filters by seed fingerprint before
invoking it, so its internal `if seed_fingerprint != ... { Ok(false) }`
branch was unreachable. Drop it and the now-unused seed_fingerprint
parameter, return Result<()> (the helper only validates the derived pubkey,
erroring on mismatch), and rename it to check_transparent_derivation.
check_transparent_input returns Ok(true) directly for a matched derivation.
No behavior change.
Replace the inline `alloc::format!` / `alloc::vec!` prefixes with a single
`use alloc::{format, string::ToString, vec};`, matching the sibling pczt
modules and keeping the allocator import in one place. No behavior change.
Replace the nested match/match in check_transparent_input with a let-else for the script->address refinement and .map_or for the derivation lookup, per the reviewer's suggestion. The // 1/2/3 step comments are preserved. No behavior change.
Replace the removed hex-blob transparent-output decode test with a runtime-built equivalent: build a p2pkh output to an external recipient and assert parse decodes its address, value, and not-change classification. Mirrors the existing p2sh output test.
The legacy multi_coins check/parse/sign path only handles transparent data, but pczt_requires_cypherpunk_support only rejected V6/Ironwood PCZTs. A v5 PCZT carrying an Orchard or Sapling bundle slipped through: its transparent part was validated and signed while the shielded bundle was silently ignored. Extend the guard to also reject Orchard actions and Sapling spends/outputs so the boundary is strictly transparent-only.
The Zcash batch registry types have merged into the upstream keystone-sdk-rust repo, so the valargroup fork pin is no longer needed. Repoint ur-registry to the upstream rev that carries them.
The batch-signing change added a second SIG_BACKGROUND_UR_GENERATE_FAIL to the GUI signal enum, which already defines it a few lines above. A duplicate enumerator is a C redefinition error, and gui_views.h is included across the UI layer, so the firmware would fail to build. Drop the duplicate.
…-signing Add Zcash batch signing for shielded PCZTs
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
2 participants
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.
Explanation
Pre-merge check list
How to test