refactor(zcash): check and parse batch PCZTs in a single pass
What changed, and why it matters
This commit refactors how Keystone's Zcash firmware checks and parses batches of partially-created shielded transactions (PCZTs). It merges what used to be separate validation and display steps into a single pass, adds a new public function for batch PCZTs, and adds tests for Ironwood spends and Orchard-to-Ironwood migrations. The change is described by the vendor as a refactor, not a security fix, and no independent security references are supplied.
Treat as a routine refactor. Reviewers should verify that the single-pass path does not skip any checks present in the previous separate check-then-parse flow, and that the new value-balance recomputation correctly handles all edge cases (e.g., all-dummy actions, negative value balances). No immediate security response is indicated by the supplied materials.
Security signals we found
New validation+parse single-pass path for batch PCZTs
Recomputes shielded bundle value balance and compares against declared value_sum
Adds account-index boundary test (account 1 returns PcztNoMyInputs)
Refactors existing parse logic into reusable helpers with no obvious removal of checks
No vendor statement of security relevance or CVE in commit message
Evidence from the diff
The patch introduces check_and_parse_batch_pczt_cypherpunk in rust/apps/zcash/src/lib.rs, which resolves compact PCZT fields, validates shielded bundles (Orchard and Ironwood) while collecting display rows, checks the transparent bundle, computes signable actions, and assembles parsed display data. check.rs gains check_and_parse_pczt_shielded and check_and_parse_shielded_bundle, which run the existing action checks (verify_cv_net, check_action_spend, output note-commitment verification) and additionally decode real spends/outputs into ParsedOrchard rows, recomputing the bundle value balance and comparing it to the declared value sum. parse.rs is refactored to expose parse_pczt_cypherpunk_with_checked_shielded and a shared assemble_parsed_pczt helper. Two unit tests exercise batch acceptance for Ironwood spends and Orchard-to-Ironwood migration, plus account-index mismatch rejection.
Changed components
rust/apps/zcash/src/lib.rsrust/apps/zcash/src/pczt/check.rsrust/apps/zcash/src/pczt/parse.rsZcash PCZT batch signing/verification flow (cypherpunk feature)Inspect captured patch +324 / −2
diff --git a/rust/apps/zcash/src/lib.rs b/rust/apps/zcash/src/lib.rs
index 39967b0..9bea271 100644
--- a/rust/apps/zcash/src/lib.rs
+++ b/rust/apps/zcash/src/lib.rs
@@ -259,6 +259,71 @@ pub fn parse_pczt_cypherpunk<P: consensus::Parameters>(
pczt::parse::parse_pczt_cypherpunk(params, seed_fingerprint, &ufvk, &pczt)
}
+/// Validates a batch PCZT for the selected account and returns its display data.
+#[cfg(feature = "cypherpunk")]
+pub fn check_and_parse_batch_pczt_cypherpunk<P: consensus::Parameters>(
+ params: &P,
+ pczt_bytes: &[u8],
+ ufvk_text: &str,
+ seed_fingerprint: &[u8; 32],
+ account_index: u32,
+) -> Result<ParsedPczt> {
+ let mut pczt = pczt::parse_pczt(pczt_bytes)?;
+ // Resolve compact field representations up front so the single-pass check
+ // sees complete actions, matching `preflight_batch_pczt_cypherpunk`.
+ pczt.resolve_fields().map_err(|e| {
+ ZcashError::InvalidPczt(alloc::format!("resolve compact PCZT fields: {e:?}"))
+ })?;
+ let account_index = zip32::AccountId::try_from(account_index)
+ .map_err(|_e| ZcashError::InvalidDataError("invalid account index".to_string()))?;
+ let ufvk = UnifiedFullViewingKey::decode(params, ufvk_text)
+ .map_err(|e| ZcashError::InvalidDataError(e.to_string()))?;
+ let xpub = ufvk.transparent().ok_or(ZcashError::InvalidDataError(
+ "transparent xpub is not present".to_string(),
+ ))?;
+
+ // Validate shielded actions while collecting their display rows.
+ let (checked_shielded, pczt) = pczt::check::check_and_parse_pczt_shielded(
+ params,
+ seed_fingerprint,
+ account_index,
+ &ufvk,
+ pczt,
+ )?;
+
+ // Check the remaining transparent bundle against the same account.
+ pczt::check::check_pczt_transparent(
+ params,
+ seed_fingerprint,
+ account_index,
+ xpub,
+ &pczt,
+ false,
+ )?;
+
+ // Reuse the PCZT returned by signability validation for display assembly.
+ let (signable_actions, pczt) = signable_shielded_actions(
+ params,
+ pczt,
+ seed_fingerprint,
+ account_index,
+ ShieldedActionPolicy::Batch,
+ )?;
+
+ if signable_actions.is_empty() {
+ Err(ZcashError::PcztNoMyInputs)
+ } else {
+ // Assemble the display from the shielded rows collected above.
+ pczt::parse::parse_pczt_cypherpunk_with_checked_shielded(
+ params,
+ seed_fingerprint,
+ &pczt,
+ checked_shielded.orchard,
+ checked_shielded.ironwood,
+ )
+ }
+}
+
#[cfg(test)]
mod additional_tests {
use super::*;
@@ -1472,6 +1537,70 @@ mod tests {
}
}
+ #[test]
+ fn test_batch_check_and_parse_accepts_ironwood_spend() {
+ let sample = pczt::test_support::sample_ironwood_pczt();
+
+ let parsed = check_and_parse_batch_pczt_cypherpunk(
+ &pczt::test_support::Nu6_3Network,
+ &sample.bytes,
+ &sample.ufvk_text,
+ &sample.seed_fingerprint,
+ 0,
+ )
+ .expect("Ironwood batch PCZT should parse");
+ assert!(parsed.get_orchard().is_some() || parsed.get_ironwood().is_some());
+
+ assert_eq!(
+ check_and_parse_batch_pczt_cypherpunk(
+ &pczt::test_support::Nu6_3Network,
+ &sample.bytes,
+ &sample.ufvk_text,
+ &sample.seed_fingerprint,
+ 1,
+ )
+ .unwrap_err(),
+ ZcashError::PcztNoMyInputs
+ );
+ }
+
+ #[test]
+ fn test_batch_check_and_parse_accepts_orchard_to_ironwood_migration() {
+ let sample = pczt::test_support::sample_migration_pczt();
+
+ let parsed = check_and_parse_batch_pczt_cypherpunk(
+ &pczt::test_support::Nu6_3Network,
+ &sample.bytes,
+ &sample.ufvk_text,
+ &sample.seed_fingerprint,
+ 0,
+ )
+ .expect("migration batch PCZT should parse");
+ assert!(!parsed
+ .get_orchard()
+ .expect("migration must show Orchard inputs")
+ .get_from()
+ .is_empty());
+ assert!(!parsed
+ .get_ironwood()
+ .expect("migration must show Ironwood outputs")
+ .get_to()
+ .is_empty());
+ assert_eq!(parsed.get_fee_value(), "0.0002 ZEC");
+
+ assert_eq!(
+ check_and_parse_batch_pczt_cypherpunk(
+ &pczt::test_support::Nu6_3Network,
+ &sample.bytes,
+ &sample.ufvk_text,
+ &sample.seed_fingerprint,
+ 1,
+ )
+ .unwrap_err(),
+ ZcashError::PcztNoMyInputs
+ );
+ }
+
#[test]
fn test_check_resolves_compact_pczt_and_signs() {
use zcash_vendor::pczt::roles::redactor::Redactor;
diff --git a/rust/apps/zcash/src/pczt/check.rs b/rust/apps/zcash/src/pczt/check.rs
index 02cde18..74b9ae8 100644
--- a/rust/apps/zcash/src/pczt/check.rs
+++ b/rust/apps/zcash/src/pczt/check.rs
@@ -23,6 +23,8 @@ use zcash_vendor::{
#[cfg(feature = "cypherpunk")]
use zcash_vendor::zcash_protocol::consensus::NetworkConstants;
+#[cfg(feature = "cypherpunk")]
+use super::structs::ParsedOrchard;
#[cfg(feature = "cypherpunk")]
use super::ShieldedPool;
@@ -79,6 +81,75 @@ pub fn check_pczt_orchard<P: consensus::Parameters>(
Ok(())
}
+/// Orchard and Ironwood display rows collected during shielded validation.
+/// A pool is `None` when it contains no displayable actions.
+#[cfg(feature = "cypherpunk")]
+pub(crate) struct CheckedShieldedParse {
+ pub(crate) orchard: Option<ParsedOrchard>,
+ pub(crate) ironwood: Option<ParsedOrchard>,
+}
+
+/// Validates the supported shielded bundles and collects their display rows.
+/// Returns the collected Orchard and Ironwood rows together with the PCZT.
+#[cfg(feature = "cypherpunk")]
+pub(crate) fn check_and_parse_pczt_shielded<P: consensus::Parameters>(
+ params: &P,
+ seed_fingerprint: &[u8; 32],
+ account_index: zip32::AccountId,
+ ufvk: &UnifiedFullViewingKey,
+ pczt: Pczt,
+) -> Result<(CheckedShieldedParse, Pczt), ZcashError> {
+ super::validate_supported_pczt(&pczt)?;
+ let mut parsed_orchard = None;
+ let mut parsed_ironwood = None;
+ let should_process_ironwood = super::pczt_should_process_ironwood(&pczt);
+
+ // Validate Orchard while collecting the rows shown during review.
+ let verifier = Verifier::new(pczt)
+ .with_orchard(|bundle| {
+ parsed_orchard = check_and_parse_shielded_bundle(
+ params,
+ seed_fingerprint,
+ account_index,
+ ufvk,
+ bundle,
+ ShieldedPool::Orchard,
+ )
+ .map_err(pczt::roles::verifier::OrchardError::Custom)?;
+ Ok(())
+ })
+ .map_err(map_orchard_verifier_error)?;
+
+ // Continue through Ironwood when this transaction version enables it.
+ let verifier = if should_process_ironwood {
+ verifier
+ .with_ironwood(|bundle| {
+ parsed_ironwood = check_and_parse_shielded_bundle(
+ params,
+ seed_fingerprint,
+ account_index,
+ ufvk,
+ bundle,
+ ShieldedPool::Ironwood,
+ )
+ .map_err(pczt::roles::verifier::OrchardError::Custom)?;
+ Ok(())
+ })
+ .map_err(map_orchard_verifier_error)?
+ } else {
+ verifier
+ };
+
+ // Return the verifier-owned PCZT for the remaining checks.
+ Ok((
+ CheckedShieldedParse {
+ orchard: parsed_orchard,
+ ironwood: parsed_ironwood,
+ },
+ verifier.finish(),
+ ))
+}
+
pub fn check_pczt_transparent<P: consensus::Parameters>(
params: &P,
seed_fingerprint: &[u8; 32],
@@ -318,6 +389,85 @@ fn check_shielded_bundle<P: consensus::Parameters>(
}
}
+/// Validates a shielded bundle while collecting its non-dummy display rows.
+/// Returns `None` when the bundle contains no displayable actions.
+#[cfg(feature = "cypherpunk")]
+fn check_and_parse_shielded_bundle<P: consensus::Parameters>(
+ params: &P,
+ seed_fingerprint: &[u8; 32],
+ account_index: zip32::AccountId,
+ ufvk: &UnifiedFullViewingKey,
+ bundle: &orchard::pczt::Bundle,
+ pool: ShieldedPool,
+) -> Result<Option<ParsedOrchard>, ZcashError> {
+ let pool_label = pool.label();
+ let fvk = ufvk.orchard().ok_or(ZcashError::InvalidDataError(
+ "orchard fvk is not present".to_string(),
+ ))?;
+
+ let mut parsed_orchard = ParsedOrchard::new(vec![], vec![]);
+ // Validate and decode each action in the canonical order.
+ bundle.actions().iter().try_for_each(|action| {
+ action.verify_cv_net().map_err(|e| {
+ ZcashError::InvalidPczt(format!("invalid cv_net in {pool_label} action: {e:?}"))
+ })?;
+
+ check_action_spend(
+ params,
+ seed_fingerprint,
+ account_index,
+ fvk,
+ action.spend(),
+ pool,
+ )?;
+ action
+ .output()
+ .verify_note_commitment(action.spend())
+ .map_err(|e| {
+ ZcashError::InvalidPczt(format!("invalid {pool_label} action cmx: {e:?}"))
+ })?;
+
+ // Add only real spends to the review.
+ if let Some(value) = action.spend().value() {
+ if value.inner() != 0 {
+ let parsed_from =
+ super::parse::parse_orchard_spend(seed_fingerprint, action.spend())?;
+ parsed_orchard.add_from(parsed_from);
+ }
+ }
+
+ // Decode real outputs once and add them to the review.
+ let parsed_to = super::parse::parse_orchard_output(params, ufvk, action, pool)?;
+ if !parsed_to.get_is_dummy() {
+ parsed_orchard.add_to(parsed_to);
+ }
+
+ Ok::<_, ZcashError>(())
+ })?;
+
+ // Recompute the bundle balance from the values just reviewed.
+ let calculated_value_balance = bundle
+ .actions()
+ .iter()
+ .map(|action| {
+ action.spend().value().expect("present") - action.output().value().expect("present")
+ })
+ .sum::<Result<ValueSum, _>>();
+
+ match calculated_value_balance {
+ Ok(value_balance) if &value_balance == bundle.value_sum() => {
+ if parsed_orchard.get_from().is_empty() && parsed_orchard.get_to().is_empty() {
+ Ok(None)
+ } else {
+ Ok(Some(parsed_orchard))
+ }
+ }
+ _ => Err(ZcashError::InvalidPczt(format!(
+ "invalid {pool_label} bundle value balance"
+ ))),
+ }
+}
+
#[cfg(feature = "cypherpunk")]
// check orchard action
fn check_action<P: consensus::Parameters>(
diff --git a/rust/apps/zcash/src/pczt/parse.rs b/rust/apps/zcash/src/pczt/parse.rs
index 53bac0f..bee446d 100644
--- a/rust/apps/zcash/src/pczt/parse.rs
+++ b/rust/apps/zcash/src/pczt/parse.rs
@@ -250,12 +250,51 @@ pub fn parse_pczt_cypherpunk<P: consensus::Parameters>(
})
.map_err(map_transparent_verifier_error)?;
+ assemble_parsed_pczt(pczt, parsed_transparent, parsed_orchard, parsed_ironwood)
+}
+
+/// Parses the transparent bundle and combines it with the supplied shielded
+/// display rows.
+#[cfg(feature = "cypherpunk")]
+pub(crate) fn parse_pczt_cypherpunk_with_checked_shielded<P: consensus::Parameters>(
+ params: &P,
+ seed_fingerprint: &[u8; 32],
+ pczt: &Pczt,
+ parsed_orchard: Option<ParsedOrchard>,
+ parsed_ironwood: Option<ParsedOrchard>,
+) -> Result<ParsedPczt, ZcashError> {
+ super::validate_supported_pczt(pczt)?;
+ let mut parsed_transparent = None;
+
+ // Parse the remaining transparent rows.
+ Verifier::new(pczt.clone())
+ .with_transparent(|bundle| {
+ parsed_transparent = parse_transparent(params, seed_fingerprint, bundle)
+ .map_err(pczt::roles::verifier::TransparentError::Custom)?;
+ Ok(())
+ })
+ .map_err(map_transparent_verifier_error)?;
+
+ // Combine all checked rows and calculate the display totals.
+ assemble_parsed_pczt(pczt, parsed_transparent, parsed_orchard, parsed_ironwood)
+}
+
+/// Assembles the per-pool parse results into the final [`ParsedPczt`],
+/// computing the transfer, change, and fee totals.
+#[cfg(feature = "cypherpunk")]
+fn assemble_parsed_pczt(
+ pczt: &Pczt,
+ parsed_transparent: Option<ParsedTransparent>,
+ parsed_orchard: Option<ParsedOrchard>,
+ parsed_ironwood: Option<ParsedOrchard>,
+) -> Result<ParsedPczt, ZcashError> {
let mut total_input_value = 0;
let mut total_output_value = 0;
let mut total_change_value = 0;
//total_input_value = total_output_value + fee_value
//total_output_value = total_transfer_value + total_change_value
+ // Fold each decoded pool into the display totals.
if let Some(orchard) = &parsed_orchard {
total_change_value += orchard
.get_to()
@@ -323,6 +362,7 @@ pub fn parse_pczt_cypherpunk<P: consensus::Parameters>(
total_input_value = total_input_value.saturating_add(sapling_value_sum as u64)
};
+ // Derive the transfer and fee values shown during confirmation.
let total_transfer_value = format_zec_value((total_output_value - total_change_value) as f64);
let fee_value = format_zec_value((total_input_value - total_output_value) as f64);
@@ -583,7 +623,7 @@ fn parse_orchard<P: consensus::Parameters>(
}
#[cfg(feature = "cypherpunk")]
-fn parse_orchard_spend(
+pub(crate) fn parse_orchard_spend(
seed_fingerprint: &[u8; 32],
spend: &orchard::pczt::Spend,
) -> Result<ParsedFrom, ZcashError> {
@@ -651,8 +691,11 @@ pub(crate) fn validate_orchard_user_address<P: consensus::Parameters>(
Ok(())
}
+/// Decodes one action's output into its [`ParsedTo`] display row, trying the
+/// wallet OVKs and then direct decryption. Every non-zero output must be
+/// recoverable.
#[cfg(feature = "cypherpunk")]
-fn parse_orchard_output<P: consensus::Parameters>(
+pub(crate) fn parse_orchard_output<P: consensus::Parameters>(
params: &P,
ufvk: &UnifiedFullViewingKey,
action: &orchard::pczt::Action,
Why this scored 29/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.