feat(zcash): resolve compact PCZT fields once in preflight
What the developer wrote
Authored by Adam Tucker
feat(zcash): resolve compact PCZT fields once in preflight
What changed, and why it matters
This commit changes how a hardware wallet handles Zcash transaction data before signing. Previously, some compact or abbreviated fields (like encrypted memos and hidden value commitments) were left as placeholders to be filled in later. Now they are fully reconstructed once during a 'preflight' check, and the complete data is baked into the bytes that get shown to the user and signed. This reduces the risk that an attacker could sneak different values into those fields between display and signing, or that the wallet would accidentally sign incomplete data. However, the commit is only a partial fix: a related multi-coin code path still has a 'future work' comment and does not call the new resolution routine.
Review whether check_pczt_multi_coins needs an equivalent resolution step or a documented exclusion, since it still carries the FUTURE TODO. Confirm that resolve_fields cannot be abused to inject malicious values, and that error handling surfaces failures clearly to the user. Consider adding tests for failure modes (e.g., unresolvable compact fields) and for the multi-coin path if applicable.
Security signals we found
Normalization of compact/omitted cryptographic fields before signing
Removal of deferred 'FUTURE(omitted-field-recompute)' TODO in two entry points
Prevention of potential signing-time re-resolution of memo/cv_net fields
Partial fix: check_pczt_multi_coins still defers omitted-field handling
New unit test covering compact-to-resolved round trip and signing
Evidence from the diff
The patch adds pczt.resolve_fields() calls in check_pczt_cypherpunk and preflight_batch_pczt_cypherpunk in rust/apps/zcash/src/lib.rs. resolve_fields expands compact PCZT representations such as memo-plaintext ciphertexts and omitted cv_net values. The resolved values are then serialized into normalized PCZT bytes, so downstream display and signing operate on complete, consistent actions. A new unit test demonstrates compacting a sample PCZT (clearing cv_net, anchors, and replacing Ironwood output enc_ciphertext with decrypted memo plaintext), running it through check_pczt_cypherpunk, and verifying that the normalized output has cv_net restored and full ciphertexts re-encrypted, and that it can be signed successfully. The multi-coin entry point check_pczt_multi_coins is explicitly left unchanged because it is compiled without the orchard feature and resolve_fields.
Changed components
rust/apps/zcash/src/lib.rscheck_pczt_cypherpunkpreflight_batch_pczt_cypherpunkZcash PCZT preflight/signing flowInspect captured patch +94 / −6
diff --git a/rust/apps/zcash/src/lib.rs b/rust/apps/zcash/src/lib.rs
index 38fdfd6..22bcc10 100644
--- a/rust/apps/zcash/src/lib.rs
+++ b/rust/apps/zcash/src/lib.rs
@@ -72,9 +72,14 @@ pub fn check_pczt_cypherpunk<P: consensus::Parameters>(
seed_fingerprint: &[u8; 32],
account_index: u32,
) -> Result<Vec<u8>> {
- let pczt = pczt::parse_pczt(pczt_bytes)?;
- // FUTURE(omitted-field-recompute): recompute-or-check omitted fields here,
- // mutating `pczt` so the normalized bytes carry the verified values forward.
+ let mut pczt = pczt::parse_pczt(pczt_bytes)?;
+ // Resolve compact field representations (memo-plaintext ciphertexts,
+ // omitted cv_net) once, up front: the checks below then see complete
+ // actions, and `serialize()` bakes the resolved values into the
+ // normalized bytes so display and signing never re-resolve.
+ pczt.resolve_fields().map_err(|e| {
+ ZcashError::InvalidPczt(alloc::format!("resolve compact PCZT fields: {e:?}"))
+ })?;
check_parsed_pczt_cypherpunk(params, &pczt, ufvk_text, seed_fingerprint, account_index)?;
pczt.serialize()
.map_err(|e| ZcashError::InvalidPczt(alloc::format!("serialize normalized PCZT: {e:?}")))
@@ -121,9 +126,14 @@ pub fn preflight_batch_pczt_cypherpunk<P: consensus::Parameters>(
seed_fingerprint: &[u8; 32],
account_index: u32,
) -> Result<Vec<u8>> {
- let pczt = pczt::parse_pczt(pczt_bytes)?;
- // FUTURE(omitted-field-recompute): recompute-or-check omitted fields here,
- // as in check_pczt_cypherpunk.
+ let mut pczt = pczt::parse_pczt(pczt_bytes)?;
+ // Resolve compact field representations (memo-plaintext ciphertexts,
+ // omitted cv_net) once, up front: the checks below then see complete
+ // actions, and `serialize()` bakes the resolved values into the
+ // normalized bytes so display and signing never re-resolve.
+ pczt.resolve_fields().map_err(|e| {
+ ZcashError::InvalidPczt(alloc::format!("resolve compact PCZT fields: {e:?}"))
+ })?;
check_parsed_pczt_cypherpunk(params, &pczt, ufvk_text, seed_fingerprint, account_index)?;
let account_id = zip32::AccountId::try_from(account_index)
.map_err(|_e| ZcashError::InvalidDataError("invalid account index".to_string()))?;
@@ -158,6 +168,7 @@ pub fn check_pczt_multi_coins<P: consensus::Parameters>(
let pczt = pczt::parse_pczt(pczt_bytes)?;
// FUTURE(omitted-field-recompute): recompute-or-check omitted fields here,
// mutating `pczt` so the normalized bytes carry the verified values forward.
+ // transparent-only build: pczt's orchard feature (and resolve_fields) is not compiled here.
check_parsed_pczt_multi_coins(params, &pczt, xpub, seed_fingerprint, account_index)?;
pczt.serialize()
.map_err(|e| ZcashError::InvalidPczt(alloc::format!("serialize normalized PCZT: {e:?}")))
@@ -1497,6 +1508,83 @@ mod tests {
}
}
+ #[cfg(zcash_unstable = "nu6.3")]
+ #[test]
+ fn test_preflight_resolves_compact_pczt_and_signs() {
+ use zcash_vendor::pczt::roles::redactor::Redactor;
+
+ let sample = pczt::test_support::sample_migration_pczt();
+ // Compact the sample the way the wallet's batch redaction will: drop cv_net and
+ // the v6 anchors, and swap each Ironwood output's ciphertext down to its memo
+ // plaintext. `resolve_fields` in the preflight must undo all of it.
+ let compact = {
+ let parsed = Pczt::parse(&sample.bytes).unwrap();
+ let redacted = Redactor::new(parsed)
+ .redact_orchard_with(|mut r| {
+ r.redact_actions(|mut ar| ar.clear_cv_net());
+ r.clear_anchor();
+ })
+ .redact_ironwood_with(|mut r| {
+ r.redact_actions(|mut ar| {
+ ar.clear_cv_net();
+ ar.replace_enc_ciphertext_with_decrypted_memo_plaintext(
+ orchard::note::NoteVersion::V3,
+ );
+ });
+ r.clear_anchor();
+ })
+ .finish();
+ redacted.serialize().unwrap()
+ };
+ assert!(compact.len() < sample.bytes.len());
+ // Confirm the swap actually compacted an Ironwood output (otherwise the
+ // ciphertext round-trip below would be vacuous).
+ assert!(Pczt::parse(&compact)
+ .unwrap()
+ .ironwood()
+ .actions()
+ .iter()
+ .any(|action| matches!(
+ action.output().enc_ciphertext(),
+ ::pczt::orchard::EncCiphertext::MemoPlaintext(_)
+ )));
+
+ let normalized = check_pczt_cypherpunk(
+ &pczt::test_support::Nu6_3Network,
+ &compact,
+ &sample.ufvk_text,
+ &sample.seed_fingerprint,
+ 0,
+ )
+ .expect("preflight must resolve compact fields before checking");
+
+ let reparsed = Pczt::parse(&normalized).expect("normalized bytes must parse");
+ assert!(reparsed
+ .orchard()
+ .actions()
+ .iter()
+ .all(|action| action.cv_net().is_some()));
+ // resolve_fields recomputed every Ironwood output's full ciphertext.
+ assert!(reparsed.ironwood().actions().iter().all(|action| matches!(
+ action.output().enc_ciphertext(),
+ ::pczt::orchard::EncCiphertext::Encrypted(_)
+ )));
+ let signed = sign_checked_pczt(
+ &pczt::test_support::Nu6_3Network,
+ &normalized,
+ &sample.seed,
+ &sample.seed_fingerprint,
+ 0,
+ )
+ .expect("resolved normalized PCZT must sign");
+ assert!(Pczt::parse(&signed)
+ .unwrap()
+ .orchard()
+ .actions()
+ .iter()
+ .any(|action| action.spend().spend_auth_sig().is_some()));
+ }
+
#[cfg(zcash_unstable = "nu6.3")]
#[test]
fn test_preflight_batch_pczt_rejects_sapling_outputs() {
Why this scored 57/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.