From 3d1e32211daa1c3bbf65fc1099b74eb87eea980d Mon Sep 17 00:00:00 2001 From: Jack Grigg Date: Fri, 13 Dec 2024 18:25:00 +0000 Subject: [PATCH] pczt: Add output field for storing the user-facing address This is necessary in order for Signers to display the address encoding that a user is expecting to confirm. Co-authored-by: Kris Nuttycombe --- Cargo.lock | 4 +- Cargo.toml | 4 +- pczt/src/orchard.rs | 12 ++ pczt/src/sapling.rs | 12 ++ pczt/src/transparent.rs | 12 ++ zcash_client_backend/src/data_api/wallet.rs | 171 +++++++++++------- .../components/transparent/builder.rs | 1 + .../components/transparent/pczt.rs | 7 + .../components/transparent/pczt/parse.rs | 2 + .../components/transparent/pczt/updater.rs | 5 + 10 files changed, 164 insertions(+), 66 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index b8e051aad..9c74234c1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2777,7 +2777,7 @@ checksum = "04744f49eae99ab78e0d5c0b603ab218f515ea8cfe5a456d7629ad883a3b6e7d" [[package]] name = "orchard" version = "0.10.0" -source = "git+https://github.com/zcash/orchard.git?rev=3d951b4201a63f3c07cba8b179dd9abde142cf33#3d951b4201a63f3c07cba8b179dd9abde142cf33" +source = "git+https://github.com/zcash/orchard.git?rev=7a44e3279b5747819022c4d8f4474fa79b2d9746#7a44e3279b5747819022c4d8f4474fa79b2d9746" dependencies = [ "aes", "bitvec", @@ -3807,7 +3807,7 @@ dependencies = [ [[package]] name = "sapling-crypto" version = "0.3.0" -source = "git+https://github.com/zcash/sapling-crypto.git?rev=231f81911628499a8877be57e66e60c09e55bdea#231f81911628499a8877be57e66e60c09e55bdea" +source = "git+https://github.com/zcash/sapling-crypto.git?rev=e47d57f5c9c46f05740328f8ef9601f6d697cf34#e47d57f5c9c46f05740328f8ef9601f6d697cf34" dependencies = [ "aes", "bellman", diff --git a/Cargo.toml b/Cargo.toml index 3e64790f8..27e593e25 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -190,5 +190,5 @@ debug = true unexpected_cfgs = { level = "warn", check-cfg = ['cfg(zcash_unstable, values("zfuture"))'] } [patch.crates-io] -orchard = { git = "https://github.com/zcash/orchard.git", rev = "3d951b4201a63f3c07cba8b179dd9abde142cf33" } -sapling-crypto = { git = "https://github.com/zcash/sapling-crypto.git", rev = "231f81911628499a8877be57e66e60c09e55bdea" } +orchard = { git = "https://github.com/zcash/orchard.git", rev = "7a44e3279b5747819022c4d8f4474fa79b2d9746" } +sapling-crypto = { git = "https://github.com/zcash/sapling-crypto.git", rev = "e47d57f5c9c46f05740328f8ef9601f6d697cf34" } diff --git a/pczt/src/orchard.rs b/pczt/src/orchard.rs index 8d26cba95..4a18f7930 100644 --- a/pczt/src/orchard.rs +++ b/pczt/src/orchard.rs @@ -232,6 +232,14 @@ pub struct Output { /// The ZIP 32 derivation path at which the spending key can be found for the output. pub(crate) zip32_derivation: Option, + /// The user-facing address to which this output is being sent, if any. + /// + /// - This is set by an Updater. + /// - Signers must parse this address (if present) and confirm that it contains + /// `recipient` (either directly, or e.g. as a receiver within a Unified Address). + #[getset(get = "pub")] + pub(crate) user_address: Option, + /// Proprietary fields related to the note being created. #[getset(get = "pub")] pub(crate) proprietary: BTreeMap>, @@ -335,6 +343,7 @@ impl Bundle { rseed: output_rseed, ock, zip32_derivation: output_zip32_derivation, + user_address, proprietary: output_proprietary, }, rcv, @@ -367,6 +376,7 @@ impl Bundle { && merge_optional(&mut lhs.output.rseed, output_rseed) && merge_optional(&mut lhs.output.ock, ock) && merge_optional(&mut lhs.output.zip32_derivation, output_zip32_derivation) + && merge_optional(&mut lhs.output.user_address, user_address) && merge_map(&mut lhs.output.proprietary, output_proprietary) && merge_optional(&mut lhs.rcv, rcv)) { @@ -430,6 +440,7 @@ impl Bundle { ) }) .transpose()?, + action.output.user_address, action.output.proprietary, )?; @@ -521,6 +532,7 @@ impl Bundle { .collect(), } }), + user_address: output.user_address().clone(), proprietary: output.proprietary().clone(), }, rcv: action.rcv().as_ref().map(|rcv| rcv.to_bytes()), diff --git a/pczt/src/sapling.rs b/pczt/src/sapling.rs index 1f6d34c5f..638cf6ef2 100644 --- a/pczt/src/sapling.rs +++ b/pczt/src/sapling.rs @@ -231,6 +231,14 @@ pub struct Output { /// The ZIP 32 derivation path at which the spending key can be found for the output. pub(crate) zip32_derivation: Option, + /// The user-facing address to which this output is being sent, if any. + /// + /// - This is set by an Updater. + /// - Signers must parse this address (if present) and confirm that it contains + /// `recipient` (either directly, or e.g. as a receiver within a Unified Address). + #[getset(get = "pub")] + pub(crate) user_address: Option, + /// Proprietary fields related to the note being spent. #[getset(get = "pub")] pub(crate) proprietary: BTreeMap>, @@ -391,6 +399,7 @@ impl Bundle { rcv, ock, zip32_derivation, + user_address, proprietary, } = rhs; @@ -410,6 +419,7 @@ impl Bundle { && merge_optional(&mut lhs.rcv, rcv) && merge_optional(&mut lhs.ock, ock) && merge_optional(&mut lhs.zip32_derivation, zip32_derivation) + && merge_optional(&mut lhs.user_address, user_address) && merge_map(&mut lhs.proprietary, proprietary)) { return None; @@ -481,6 +491,7 @@ impl Bundle { ) }) .transpose()?, + output.user_address, output.proprietary, ) }) @@ -561,6 +572,7 @@ impl Bundle { seed_fingerprint: *z.seed_fingerprint(), derivation_path: z.derivation_path().iter().map(|i| i.index()).collect(), }), + user_address: output.user_address().clone(), proprietary: output.proprietary().clone(), }) .collect(); diff --git a/pczt/src/transparent.rs b/pczt/src/transparent.rs index 311d8d571..9bdc1db8c 100644 --- a/pczt/src/transparent.rs +++ b/pczt/src/transparent.rs @@ -158,6 +158,14 @@ pub struct Output { #[serde_as(as = "BTreeMap<[_; 33], _>")] pub(crate) bip32_derivation: BTreeMap<[u8; 33], Zip32Derivation>, + /// The user-facing address to which this output is being sent, if any. + /// + /// - This is set by an Updater. + /// - Signers must parse this address (if present) and confirm that it contains + /// `recipient` (either directly, or e.g. as a receiver within a Unified Address). + #[getset(get = "pub")] + pub(crate) user_address: Option, + /// Proprietary fields related to the note being spent. #[getset(get = "pub")] pub(crate) proprietary: BTreeMap>, @@ -267,6 +275,7 @@ impl Bundle { script_pubkey, redeem_script, bip32_derivation, + user_address, proprietary, } = rhs; @@ -276,6 +285,7 @@ impl Bundle { if !(merge_optional(&mut lhs.redeem_script, redeem_script) && merge_map(&mut lhs.bip32_derivation, bip32_derivation) + && merge_optional(&mut lhs.user_address, user_address) && merge_map(&mut lhs.proprietary, proprietary)) { return None; @@ -346,6 +356,7 @@ impl Bundle { .map(|v| (k, v)) }) .collect::>()?, + output.user_address, output.proprietary, ) }) @@ -430,6 +441,7 @@ impl Bundle { ) }) .collect(), + user_address: output.user_address().clone(), proprietary: output.proprietary().clone(), }) .collect(); diff --git a/zcash_client_backend/src/data_api/wallet.rs b/zcash_client_backend/src/data_api/wallet.rs index 54c216c56..669eac78a 100644 --- a/zcash_client_backend/src/data_api/wallet.rs +++ b/zcash_client_backend/src/data_api/wallet.rs @@ -132,32 +132,30 @@ struct ProposalInfo { #[cfg(feature = "pczt")] #[derive(Serialize, Deserialize)] enum PcztRecipient { - External(ZcashAddress), - EphemeralTransparent { - receiving_account: AccountId, - }, - InternalAccount { - receiving_account: AccountId, - external_address: Option, - }, + External, + EphemeralTransparent { receiving_account: AccountId }, + InternalAccount { receiving_account: AccountId }, } #[cfg(feature = "pczt")] impl PcztRecipient { - fn from_recipient(recipient: Recipient) -> Self { + fn from_recipient(recipient: Recipient) -> (Self, Option) { match recipient { - Recipient::External(addr, _) => PcztRecipient::External(addr), + Recipient::External(addr, _) => (PcztRecipient::External, Some(addr)), Recipient::EphemeralTransparent { receiving_account, .. - } => PcztRecipient::EphemeralTransparent { receiving_account }, + } => ( + PcztRecipient::EphemeralTransparent { receiving_account }, + None, + ), Recipient::InternalAccount { receiving_account, external_address, .. - } => PcztRecipient::InternalAccount { - receiving_account, + } => ( + PcztRecipient::InternalAccount { receiving_account }, external_address, - }, + ), } } } @@ -1457,7 +1455,10 @@ where } } - if let Some(pczt_recipient) = orchard_outputs.get(&index) { + if let Some((pczt_recipient, external_address)) = orchard_outputs.get(&index) { + if let Some(user_address) = external_address { + action_updater.set_output_user_address(user_address.encode()); + } action_updater.set_output_proprietary( PROPRIETARY_OUTPUT_INFO.into(), postcard::to_allocvec(pczt_recipient).expect( @@ -1509,8 +1510,11 @@ where } for index in 0..updater.bundle().outputs().len() { - if let Some(pczt_recipient) = sapling_outputs.get(&index) { + if let Some((pczt_recipient, external_address)) = sapling_outputs.get(&index) { updater.update_output_with(index, |mut output_updater| { + if let Some(user_address) = external_address { + output_updater.set_user_address(user_address.encode()); + } output_updater.set_proprietary( PROPRIETARY_OUTPUT_INFO.into(), postcard::to_allocvec(pczt_recipient).expect( @@ -1589,9 +1593,14 @@ where build_state.transparent_output_meta.into_iter().enumerate() { updater.update_output_with(index, |mut output_updater| { + let (pczt_recipient, external_address) = + PcztRecipient::from_recipient(recipient); + if let Some(user_address) = external_address { + output_updater.set_user_address(user_address.encode()); + } output_updater.set_proprietary( PROPRIETARY_OUTPUT_INFO.into(), - postcard::to_allocvec(&PcztRecipient::from_recipient(recipient)) + postcard::to_allocvec(&pczt_recipient) .expect("postcard encoding of pczt recipient metadata should not fail"), ); Ok(()) @@ -1664,25 +1673,34 @@ where orchard::Note::from_parts(recipient, value, rho, rseed).into_option() }; + let external_address = act + .output() + .user_address() + .as_deref() + .map(ZcashAddress::try_from_encoded) + .transpose() + .map_err(|e| PcztError::Invalid(format!("Invalid user_address: {}", e)))?; + let pczt_recipient = act .output() .proprietary() .get(PROPRIETARY_OUTPUT_INFO) .map(|v| postcard::from_bytes::>(v)) - .transpose()?; + .transpose() + .map_err(|e: postcard::Error| { + PcztError::Invalid(format!( + "Postcard decoding of proprietary output info failed: {}", + e + )) + })? + .map(|pczt_recipient| (pczt_recipient, external_address)); // If the pczt recipient is not present, this is a dummy note; if the note is not // present, then the PCZT has been pruned to make this output unrecoverable and so we // also ignore it. Ok(pczt_recipient.zip(note())) }) - .collect::, _>>() - .map_err(|e: postcard::Error| { - PcztError::Invalid(format!( - "Postcard decoding of proprietary output info failed: {}", - e - )) - })?; + .collect::, PcztError>>()?; let sapling_output_info = finalized .sapling() @@ -1704,42 +1722,61 @@ where Some(::sapling::Note::from_parts(recipient, value, rseed)) }; + let external_address = out + .user_address() + .as_deref() + .map(ZcashAddress::try_from_encoded) + .transpose() + .map_err(|e| PcztError::Invalid(format!("Invalid user_address: {}", e)))?; + let pczt_recipient = out .proprietary() .get(PROPRIETARY_OUTPUT_INFO) .map(|v| postcard::from_bytes::>(v)) - .transpose()?; + .transpose() + .map_err(|e: postcard::Error| { + PcztError::Invalid(format!( + "Postcard decoding of proprietary output info failed: {}", + e + )) + })? + .map(|pczt_recipient| (pczt_recipient, external_address)); // If the pczt recipient is not present, this is a dummy note; if the note is not // present, then the PCZT has been pruned to make this output unrecoverable and so we // also ignore it. Ok(pczt_recipient.zip(note())) }) - .collect::, _>>() - .map_err(|e: postcard::Error| { - PcztError::Invalid(format!( - "Postcard decoding of proprietary output info failed: {}", - e - )) - })?; + .collect::, PcztError>>()?; let transparent_output_info = finalized .transparent() .outputs() .iter() .map(|out| { - out.proprietary() + let external_address = out + .user_address() + .as_deref() + .map(ZcashAddress::try_from_encoded) + .transpose() + .map_err(|e| PcztError::Invalid(format!("Invalid user_address: {}", e)))?; + + let pczt_recipient = out + .proprietary() .get(PROPRIETARY_OUTPUT_INFO) .map(|v| postcard::from_bytes::>(v)) .transpose() + .map_err(|e: postcard::Error| { + PcztError::Invalid(format!( + "Postcard decoding of proprietary output info failed: {}", + e + )) + })? + .map(|pczt_recipient| (pczt_recipient, external_address)); + + Ok(pczt_recipient) }) - .collect::, _>>() - .map_err(|e: postcard::Error| { - PcztError::Invalid(format!( - "Postcard decoding of proprietary output info failed: {}", - e - )) - })?; + .collect::, PcztError>>()?; let utxos_map = finalized .transparent() @@ -1775,6 +1812,7 @@ where output_pool: ShieldedProtocol, output_index: usize, pczt_recipient: PcztRecipient, + external_address: Option, note_value: impl Fn(&D::Note) -> u64, memo_bytes: impl Fn(&D::Memo) -> &[u8; 512], wallet_note: impl Fn(D::Note) -> Note, @@ -1786,21 +1824,23 @@ where }); let note_value = NonNegativeAmount::try_from(note_value(¬e))?; - let recipient = match pczt_recipient { - PcztRecipient::External(addr) => { + let recipient = match (pczt_recipient, external_address) { + (PcztRecipient::External, Some(addr)) => { Ok(Recipient::External(addr, PoolType::Shielded(output_pool))) } - PcztRecipient::EphemeralTransparent { .. } => Err(PcztError::Invalid( + (PcztRecipient::External, None) => Err(PcztError::Invalid( + "external recipient needs to have its user_address field set".into(), + )), + (PcztRecipient::EphemeralTransparent { .. }, _) => Err(PcztError::Invalid( "shielded output cannot be EphemeralTransparent".into(), )), - PcztRecipient::InternalAccount { - receiving_account, - external_address, - } => Ok(Recipient::InternalAccount { - receiving_account, - external_address, - note: wallet_note(note), - }), + (PcztRecipient::InternalAccount { receiving_account }, external_address) => { + Ok(Recipient::InternalAccount { + receiving_account, + external_address, + note: wallet_note(note), + }) + } }?; Ok(SentTransactionOutput::from_parts( @@ -1822,7 +1862,7 @@ where .zip(orchard_output_info) .enumerate() .filter_map(|(output_index, (action, output_info))| { - output_info.map(|(pczt_recipient, note)| { + output_info.map(|((pczt_recipient, external_address), note)| { let domain = OrchardDomain::for_action(action); to_sent_transaction_output::<_, _, _, DbT, _>( domain, @@ -1831,6 +1871,7 @@ where ShieldedProtocol::Orchard, output_index, pczt_recipient, + external_address, |note| note.value().inner(), |memo| memo, Note::Orchard, @@ -1851,7 +1892,7 @@ where .zip(sapling_output_info) .enumerate() .filter_map(|(output_index, (action, output_info))| { - output_info.map(|(pczt_recipient, note)| { + output_info.map(|((pczt_recipient, external_address), note)| { let domain = SaplingDomain::new(sapling::note_encryption::Zip212Enforcement::On); to_sent_transaction_output::<_, _, _, DbT, _>( @@ -1861,6 +1902,7 @@ where ShieldedProtocol::Sapling, output_index, pczt_recipient, + external_address, |note| note.value().inner(), |memo| memo, Note::Sapling, @@ -1882,18 +1924,21 @@ where .zip(transparent_output_info) .enumerate() .filter_map(|(output_index, (output, output_info))| { - output_info.map(|pczt_recipient| { + output_info.map(|(pczt_recipient, external_address)| { // This assumes that transparent outputs are pushed onto `transparent_output_meta` // with the same indices they have in the transaction's transparent outputs. // We do not reorder transparent outputs; there is no reason to do so because it // would not usefully improve privacy. let outpoint = OutPoint::new(txid.into(), output_index as u32); - let recipient = match pczt_recipient { - PcztRecipient::External(addr) => { + let recipient = match (pczt_recipient, external_address) { + (PcztRecipient::External, Some(addr)) => { Ok(Recipient::External(addr, PoolType::Transparent)) } - PcztRecipient::EphemeralTransparent { receiving_account } => output + (PcztRecipient::External, None) => Err(PcztError::Invalid( + "external recipient needs to have its user_address field set".into(), + )), + (PcztRecipient::EphemeralTransparent { receiving_account }, _) => output .recipient_address() .ok_or(PcztError::Invalid( "Ephemeral outputs cannot have a non-standard script_pubkey" @@ -1904,10 +1949,12 @@ where ephemeral_address, outpoint_metadata: outpoint, }), - PcztRecipient::InternalAccount { - receiving_account, - external_address, - } => Err(PcztError::Invalid( + ( + PcztRecipient::InternalAccount { + receiving_account, + }, + _, + ) => Err(PcztError::Invalid( "Transparent output cannot be InternalAccount".into(), )), }?; diff --git a/zcash_primitives/src/transaction/components/transparent/builder.rs b/zcash_primitives/src/transaction/components/transparent/builder.rs index b076627e5..97ef23188 100644 --- a/zcash_primitives/src/transaction/components/transparent/builder.rs +++ b/zcash_primitives/src/transaction/components/transparent/builder.rs @@ -275,6 +275,7 @@ impl TransparentBuilder { // script. redeem_script: None, bip32_derivation: BTreeMap::new(), + user_address: None, proprietary: BTreeMap::new(), }) .collect(); diff --git a/zcash_primitives/src/transaction/components/transparent/pczt.rs b/zcash_primitives/src/transaction/components/transparent/pczt.rs index 12835808c..ba8d7cfad 100644 --- a/zcash_primitives/src/transaction/components/transparent/pczt.rs +++ b/zcash_primitives/src/transaction/components/transparent/pczt.rs @@ -210,6 +210,13 @@ pub struct Output { /// In particular, it is not possible to include entries for non-BIP-32 pubkeys. pub(crate) bip32_derivation: BTreeMap<[u8; 33], Bip32Derivation>, + /// The user-facing address to which this output is being sent, if any. + /// + /// - This is set by an Updater. + /// - Signers must parse this address (if present) and confirm that it contains + /// `recipient` (either directly, or e.g. as a receiver within a Unified Address). + pub(crate) user_address: Option, + /// Proprietary fields related to the transparent coin being created. pub(crate) proprietary: BTreeMap>, } diff --git a/zcash_primitives/src/transaction/components/transparent/pczt/parse.rs b/zcash_primitives/src/transaction/components/transparent/pczt/parse.rs index 4aae36451..fe42101ae 100644 --- a/zcash_primitives/src/transaction/components/transparent/pczt/parse.rs +++ b/zcash_primitives/src/transaction/components/transparent/pczt/parse.rs @@ -94,6 +94,7 @@ impl Output { script_pubkey: Vec, redeem_script: Option>, bip32_derivation: BTreeMap<[u8; 33], Bip32Derivation>, + user_address: Option, proprietary: BTreeMap>, ) -> Result { let value = Zatoshis::from_u64(value).map_err(|_| ParseError::InvalidValue)?; @@ -109,6 +110,7 @@ impl Output { script_pubkey, redeem_script, bip32_derivation, + user_address, proprietary, }) } diff --git a/zcash_primitives/src/transaction/components/transparent/pczt/updater.rs b/zcash_primitives/src/transaction/components/transparent/pczt/updater.rs index 3e024ca9b..d2bfea116 100644 --- a/zcash_primitives/src/transaction/components/transparent/pczt/updater.rs +++ b/zcash_primitives/src/transaction/components/transparent/pczt/updater.rs @@ -135,6 +135,11 @@ impl<'a> OutputUpdater<'a> { self.0.bip32_derivation.insert(pubkey, derivation); } + /// Sets the user-facing address that the new note is being sent to. + pub fn set_user_address(&mut self, user_address: String) { + self.0.user_address = Some(user_address); + } + /// Stores the given proprietary value at the given key. pub fn set_proprietary(&mut self, key: String, value: Vec) { self.0.proprietary.insert(key, value);