fix(zcash): compact migrations alongside split transactions
What changed, and why it matters
This commit changes how the Keystone hardware wallet summarizes Zcash batch transactions that include both ordinary payments and migration (Orchard-to-Ironwood) self-transfers. Previously, if a batch had more than one ordinary transaction alongside migrations, the wallet showed every transaction in full. Now, it still shows each ordinary transaction in full but compacts all migrations into a single summary page. The change is described as a fix for handling 'split transactions' alongside migrations, but the commit message and diff do not explain any security vulnerability.
Treat as a functional/UI fix unless additional context shows the prior behavior could mislead users into approving unintended transactions. Review whether compacting many migrations while showing ordinary transactions separately preserves clear auditability. No immediate security patch urgency is evident from the supplied materials.
Security signals we found
Behavioral change in transaction review UI that could affect user understanding of batch contents
Removal of ambiguity guard that previously forced full review for mixed batches
No explicit security claim or CVE reference in commit or supplied materials
Change is in cryptographic asset handling code (Zcash shielded transactions)
Evidence from the diff
The patch modifies compact_batch_migration_review in rust/apps/zcash/src/lib.rs. It removes the guard items.len() - migration_count > 1, which previously prevented compaction when more than one non-migration transaction was present. The new logic compacts migrations whenever there is at least one migration and more than one item total, regardless of how many ordinary transactions exist. A new unit test verifies that 3 ordinary split transactions and 30 migrations result in 4 review pages: one per ordinary transaction plus one migration summary. The change is gated behind the cypherpunk feature and affects only Zcash PCZT batch review display logic.
Changed components
rust/apps/zcash/src/lib.rscompact_batch_migration_review functionparse_batch_with_migration_summary_cypherpunk functionZcash batch PCZT review display (cypherpunk feature)Inspect captured patch +47 / −32
diff --git a/rust/apps/zcash/src/lib.rs b/rust/apps/zcash/src/lib.rs
index ca53ba8..a65af4d 100644
--- a/rust/apps/zcash/src/lib.rs
+++ b/rust/apps/zcash/src/lib.rs
@@ -582,12 +582,12 @@ fn migration_transfer_summary(parsed: &ParsedPczt) -> Option<BatchMigrationTrans
}
}
-/// Replaces compact-eligible transfers with one summary when the batch contains
-/// at most one other transaction. Ambiguous batches retain full review pages.
+/// Replaces compact-eligible transfers with one summary while retaining every
+/// ordinary transaction as its own full review page.
#[cfg(feature = "cypherpunk")]
fn compact_batch_migration_review(items: Vec<ParsedBatchItem>) -> Vec<ParsedPczt> {
let migration_count = items.iter().filter(|item| item.migration.is_some()).count();
- if items.len() <= 1 || migration_count == 0 || items.len() - migration_count > 1 {
+ if items.len() <= 1 || migration_count == 0 {
return items.into_iter().map(|item| item.parsed).collect();
}
@@ -627,8 +627,8 @@ pub fn compact_checked_batch_migration_review(
/// Parses checked batch PCZTs and compacts eligible Orchard-to-Ironwood
/// self-transfers without relying on PCZT position.
///
-/// Every input must be normalized bytes produced by the batch check. A batch
-/// with more than one ordinary transaction uses full review for each PCZT.
+/// Every input must be normalized bytes produced by the batch check. Ordinary
+/// transactions retain full review pages regardless of how many are present.
#[cfg(feature = "cypherpunk")]
pub fn parse_batch_with_migration_summary_cypherpunk<'a, P: consensus::Parameters>(
params: &P,
@@ -2396,39 +2396,54 @@ mod tests {
}
#[test]
- fn test_batch_migration_summary_keeps_ambiguous_batch_full() {
- let ordinary_1 = pczt::test_support::sample_orchard_change_pczt();
- let ordinary_2 = pczt::test_support::sample_orchard_change_pczt();
+ fn test_batch_migration_summary_compacts_three_splits_and_thirty_migrations() {
+ let ordinary = pczt::test_support::sample_orchard_change_pczt();
let migration = pczt::test_support::sample_migration_pczt();
- let checked = [&ordinary_1, &migration, &ordinary_2]
- .into_iter()
- .map(|sample| {
- check_batch_pczt_cypherpunk(
- &pczt::test_support::Nu6_3Network,
- &sample.bytes,
- &sample.ufvk_text,
- &sample.seed_fingerprint,
- 0,
- )
- .unwrap()
- })
- .collect::<Vec<_>>();
- let parsed = parse_batch_with_migration_summary_cypherpunk(
+ let context = BatchCheckContext::new(&ordinary.ufvk_text);
+ let (_, ordinary, ordinary_migration) = check_batch_pczt_with_display(
&pczt::test_support::Nu6_3Network,
- checked.iter().map(Vec::as_slice),
- &migration.ufvk_text,
+ &ordinary.bytes,
+ &context,
+ &ordinary.seed_fingerprint,
+ 0,
+ )
+ .unwrap();
+ let (_, migration, migration_summary) = check_batch_pczt_with_display(
+ &pczt::test_support::Nu6_3Network,
+ &migration.bytes,
+ &context,
&migration.seed_fingerprint,
+ 0,
)
.unwrap();
+ assert!(ordinary_migration.is_none());
+ assert!(migration_summary.is_some());
+
+ let mut checked = Vec::with_capacity(33);
+ for index in 1..=3 {
+ let mut split = ordinary.clone();
+ split.set_total_transfer_value(format!("split-{index}"));
+ checked.push((split, None));
+ }
+ checked.extend(core::iter::repeat_with(|| (migration.clone(), migration_summary)).take(30));
- assert_eq!(parsed.len(), 3);
- assert!(parsed.iter().all(|item| {
- item.get_orchard()
- .unwrap()
- .get_from()
- .iter()
- .all(|from| from.get_address().is_none())
- }));
+ let parsed = compact_checked_batch_migration_review(checked);
+
+ assert_eq!(parsed.len(), 4);
+ assert_eq!(parsed[0].get_total_transfer_value(), "split-1");
+ assert_eq!(parsed[1].get_total_transfer_value(), "split-2");
+ assert_eq!(parsed[2].get_total_transfer_value(), "split-3");
+ assert!(parsed[..3].iter().all(|item| item.get_ironwood().is_none()));
+
+ let summary = &parsed[3];
+ assert_eq!(summary.get_total_transfer_value(), "0.297 ZEC");
+ assert_eq!(summary.get_fee_value(), "0.006 ZEC");
+ assert_eq!(summary.get_orchard().unwrap().get_from().len(), 30);
+ assert_eq!(summary.get_ironwood().unwrap().get_to().len(), 30);
+ assert_eq!(
+ summary.get_ironwood().unwrap().get_to()[29].get_address(),
+ "Migration #30 wallet Ironwood output"
+ );
}
// A memo on the funded output forces fallback to the review that displays it.
Why this scored 46/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.