Add Ironwood PCZT firmware support by czarcas7ic · Pull Request #2184 · KeystoneHQ/keystone3-firmware

czarcas7ic · 2026-06-20T05:11:51Z

Adds cypherpunk Zcash PCZT parse, check, and sign support for v6/Ironwood while preserving existing Orchard and transparent behavior.

Legacy multi-coins paths now reject v6/Ironwood PCZTs instead of displaying or signing them with incomplete legacy support. Signed responses stamp the firmware version and redact optional PCZT fields, including Ironwood bundle data, before returning QR-sized signing results.

This temporarily pins Zcash dependencies to Ironwood/protocol API revisions until the needed APIs are available from released upstream crates.

Validation:

cargo test -p app_zcash --no-default-features --features cypherpunk
cargo test -p app_zcash --no-default-features --features multi_coins
cargo check -p rust_c --no-default-features --features simulator-cypherpunk
cargo check -p rust_c --no-default-features --features simulator-multi-coins

czarcas7ic · 2026-06-20T05:49:07Z

+[patch.crates-io]
+orchard = { git = "https://github.com/zcash/orchard", rev = "b2af0a11abe00f59c51258d349c2105fe7a16215" }
+pczt = { git = "https://github.com/valargroup/librustzcash", rev = "dc95dcef33a081b925db551eac8bf6533fff22ed" }
+zcash_address = { git = "https://github.com/valargroup/librustzcash", rev = "dc95dcef33a081b925db551eac8bf6533fff22ed" }
+zcash_encoding = { git = "https://github.com/valargroup/librustzcash", rev = "dc95dcef33a081b925db551eac8bf6533fff22ed" }
+zcash_keys = { git = "https://github.com/valargroup/librustzcash", rev = "dc95dcef33a081b925db551eac8bf6533fff22ed" }
+zcash_primitives = { git = "https://github.com/valargroup/librustzcash", rev = "dc95dcef33a081b925db551eac8bf6533fff22ed" }
+zcash_protocol = { git = "https://github.com/valargroup/librustzcash", rev = "dc95dcef33a081b925db551eac8bf6533fff22ed" }
+zcash_transparent = { git = "https://github.com/valargroup/librustzcash", rev = "dc95dcef33a081b925db551eac8bf6533fff22ed" }


Set as patch for now. Will update once respective orchard / librustzcash tags are made.

daira

Review would be easier if pure refactoring was split into its own commit before the Ironwood support.

nullcopy

Flushing comments. Still need to finish reviewing second half of 24f1759

nullcopy · 2026-06-23T17:24:15Z

-unexpected_cfgs = { level = "warn", check-cfg = ['cfg(coverage_nightly)'] }
+unexpected_cfgs = { level = "warn", check-cfg = [
+    'cfg(coverage_nightly)',
+    'cfg(zcash_unstable, values("nu6.3"))',


Why are these warns being silenced? They seem like good warnings that should be resolved in the near future, no?

This isn’t silencing real warnings, it just tells Rust that zcash_unstable="nu6.3" is an intentional cfg used by Ironwood.

Without this, Rust warns on every intentional use and suggests adding this exact check-cfg entry.

I understand that, but IMO that's a good warning. Once nu6.3 becomes stable, the config can be removed and the warning will go away, but until that time, it's useful to warn the dev that unstable features are enabled.

Anyways, that's really up to Keystone devs. I just figured I'd call it out

nullcopy · 2026-06-23T19:25:11Z

+    zcash_unstable = "nu6.3",
+    any(feature = "multi_coins", not(feature = "cypherpunk"))
+))]
+pub(crate) fn pczt_requires_cypherpunk_support(pczt: &zcash_vendor::pczt::Pczt) -> bool {


Isn't pczt only used by Zcash, and isn't Zcash only supported in the "cypherpunk" build? So pczt always requires cypherpunk, no?

I guess multicoin builds support transparent ZEC? In which case, this check should also include a check for orchard actions (for backwards/fork compatibility)

Zcash isn't cypherpunk only. The standard multi_coins build supports Zcash too, just for transparent txs. Only shielded Orchard/Ironwood needs cypherpunk.

This guard is needed b/c the transparent only path uses the old pre-NU6.3 sighash and can't handle V6/Ironwood, so the reject_legacy_* helpers use it to turn those PCZTs away instead of signing or showing a partial transaction. Without it, a multi_coins build would run an Ironwood PCZT through the transparent path.

Can we update the transparent path to use the new NU6.3 sighash? It would be nice to keep zcash support consistent across builds

29d9e34

I still need to manually test this but its late, will do tomorrow. Would be great to get your eyes on it though, thanks

@nullcopy I was able to validate v5 and v6. Nothing shows on the Keystone when you are signing, but this is not a new issue. Additionally, multi coins doesn't even have Zodl as an option, so I had to hack this to even test it.

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 legacy multi_coins path rejected all v6 PCZTs because its hand-rolled ZIP-244 transparent sighash predated NU6.3. Extend that sighash to v6: for a transparent-only transaction the only deltas from v5 are the v6 Orchard empty-bundle personalization and an appended empty Ironwood node, both a plain blake2b of the bundle personalization (verified byte-exact against the orchard crate's hash_bundle_txid_empty_with_domain). The header and transparent digests are unchanged and already version-aware. With the sighash version-aware, relax the legacy reject from "v6 or shielded" to shielded-only, so the transparent-only path signs, parses, and checks v6 transparent transactions while still rejecting any Sapling/Orchard/Ironwood content it cannot display.

czarcas7ic force-pushed the adam/upstream-ironwood-pczt-firmware branch from 5c41bda to 8cbf844 Compare June 20, 2026 05:45

czarcas7ic marked this pull request as ready for review June 20, 2026 05:47

czarcas7ic commented Jun 20, 2026

View reviewed changes

daira reviewed Jun 20, 2026

View reviewed changes

czarcas7ic force-pushed the adam/upstream-ironwood-pczt-firmware branch from 8cbf844 to 24f1759 Compare June 20, 2026 20:52

nullcopy suggested changes Jun 23, 2026

View reviewed changes

nullcopy reviewed Jun 23, 2026

View reviewed changes

Comment thread rust/apps/zcash/src/pczt/mod.rs

nullcopy reviewed Jun 23, 2026

View reviewed changes

Comment thread rust/apps/zcash/src/lib.rs

czarcas7ic added 3 commits June 23, 2026 13:50

czarcas7ic force-pushed the adam/upstream-ironwood-pczt-firmware branch from 24f1759 to c28575b Compare June 23, 2026 20:20

czarcas7ic added 8 commits June 23, 2026 14:46

style: import alloc::{format, vec} at the top of check.rs

658f5b5

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.

Uh oh!

Conversation

czarcas7ic commented Jun 20, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

daira left a comment

Choose a reason for hiding this comment

Uh oh!

nullcopy left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants