refactor: drop dead seed-fingerprint check from transparent derivation helper
What the developer wrote
Authored by Adam Tucker
refactor: drop dead seed-fingerprint check from transparent derivation 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.
What changed, and why it matters
This commit is a code cleanup (refactor) in the Zcash PCZT handling code. It removes an unused internal check and an unused function parameter, renames a helper function, and changes its return type. The commit message explicitly states there is no behavior change, and the diff supports that: every caller already filtered by seed fingerprint before calling the helper, so the removed branch was unreachable. There is no security-relevant change evident from the diff.
No security action required. Treat as routine code cleanup. If desired, verify through static analysis or review that all call sites indeed pre-filter by seed_fingerprint, confirming the removed branch was unreachable as claimed.
Security signals we found
No security-relevant behavior change in the diff
Removal of dead/unreachable code path
Function signature simplification (refactor)
No new input handling, parsing, or cryptographic operations introduced
Evidence from the diff
The helper transparent_derivation_matches_selected_account returned Result<bool, ZcashError> and contained an early Ok(false) branch when seed_fingerprint != derivation.seed_fingerprint(). All call sites already checked the seed fingerprint before invoking the helper, making that branch dead code. The refactor removes the seed_fingerprint parameter and the dead branch, renames the helper to check_transparent_derivation, and returns Result<(), ZcashError> because the only remaining purpose is to validate the derived public key. Callers now return Ok(true) directly on success. The logic for deriving and comparing the public key is unchanged.
Changed components
rust/apps/zcash/src/pczt/check.rsrust/apps/zcash/src/pczt/mod.rsInspect captured patch +14 / −23
diff --git a/rust/apps/zcash/src/pczt/check.rs b/rust/apps/zcash/src/pczt/check.rs
index 77051fc..c6f18a0 100644
--- a/rust/apps/zcash/src/pczt/check.rs
+++ b/rust/apps/zcash/src/pczt/check.rs
@@ -160,23 +160,21 @@ fn check_transparent_input<P: consensus::Parameters>(
}
Some((pubkey, derivation)) => {
// 2: derive my pubkey
- let belongs_to_selected_account =
- super::transparent_derivation_matches_selected_account(
- params,
- seed_fingerprint,
- account_index,
- xpub,
- pubkey,
- derivation,
- "input",
- )?;
+ super::check_transparent_derivation(
+ params,
+ account_index,
+ xpub,
+ pubkey,
+ derivation,
+ "input",
+ )?;
// 3: check script pubkey
if hash[..] != Ripemd160::digest(Sha256::digest(pubkey))[..] {
return Err(ZcashError::InvalidPczt(
"transparent input script pubkey mismatch".to_string(),
));
}
- Ok(belongs_to_selected_account)
+ Ok(true)
}
}
}
@@ -223,9 +221,8 @@ fn check_transparent_output<P: consensus::Parameters>(
match output.bip32_derivation().get(pubkey) {
Some(bip32_derivation) => {
if seed_fingerprint == bip32_derivation.seed_fingerprint() {
- super::transparent_derivation_matches_selected_account(
+ super::check_transparent_derivation(
params,
- seed_fingerprint,
account_index,
xpub,
pubkey,
@@ -275,9 +272,8 @@ fn check_transparent_output<P: consensus::Parameters>(
Ok(())
}
Some((pubkey, derivation)) => {
- super::transparent_derivation_matches_selected_account(
+ super::check_transparent_derivation(
params,
- seed_fingerprint,
account_index,
xpub,
pubkey,
diff --git a/rust/apps/zcash/src/pczt/mod.rs b/rust/apps/zcash/src/pczt/mod.rs
index 33f655d..0a8c154 100644
--- a/rust/apps/zcash/src/pczt/mod.rs
+++ b/rust/apps/zcash/src/pczt/mod.rs
@@ -69,21 +69,16 @@ fn validate_sapling_bundle_consistency(pczt: &Pczt) -> Result<(), ZcashError> {
Ok(())
}
-pub(crate) fn transparent_derivation_matches_selected_account<
+pub(crate) fn check_transparent_derivation<
P: zcash_vendor::zcash_protocol::consensus::Parameters,
>(
params: &P,
- seed_fingerprint: &[u8; 32],
account_index: zip32::AccountId,
xpub: &transparent::keys::AccountPubKey,
pubkey: &[u8; 33],
derivation: &transparent::pczt::Bip32Derivation,
field_label: &str,
-) -> Result<bool, ZcashError> {
- if seed_fingerprint != derivation.seed_fingerprint() {
- return Ok(false);
- }
-
+) -> Result<(), ZcashError> {
let target = xpub
.derive_pubkey_at_bip32_path(params, account_index, derivation.derivation_path())
.map_err(|_| {
@@ -97,7 +92,7 @@ pub(crate) fn transparent_derivation_matches_selected_account<
)));
}
- Ok(true)
+ Ok(())
}
/// Which shielded pool a bundle belongs to. Orchard and Ironwood share the same
Why this scored 12/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.