From 63be72234acb100f902f623ae71b46d66642b485 Mon Sep 17 00:00:00 2001 From: zancas Date: Mon, 3 Aug 2026 17:39:16 -0700 Subject: [PATCH] feat!: carry memos losslessly typed in the canonical summaries The three summary memo fields become zcash_protocol Memo and the text-only filters leave transaction-summary construction, so the canonical layer stops editorializing what it carries. The text-only display policy moves to zingo-perspective, which now derives transfer memos and the memo-to-self classification through its own text_memo view. zingolib's summary renderers stay bug-compatible, rendering a non-text memo exactly as the previously-dropped memo rendered, so no canonical output bytes change and every golden fixture holds byte-identical. A new derivation test pins the typed memo present on the summary while the derived value transfer hides it. Co-Authored-By: Claude Fable 5 --- libtonode-tests/tests/unit_test_twins.rs | 21 +++--- zingo-perspective/src/ext.rs | 49 ++++++++++-- zingo-perspective/tests/derivation.rs | 23 +++++- zingolib/src/lightclient/mock_chain_tests.rs | 21 +++--- zingolib/src/wallet/summary.rs | 79 +++++--------------- zingolib/src/wallet/summary/data.rs | 55 ++++++-------- 6 files changed, 131 insertions(+), 117 deletions(-) diff --git a/libtonode-tests/tests/unit_test_twins.rs b/libtonode-tests/tests/unit_test_twins.rs index 651d7ed08b..79e4dd402d 100644 --- a/libtonode-tests/tests/unit_test_twins.rs +++ b/libtonode-tests/tests/unit_test_twins.rs @@ -28,10 +28,13 @@ //! either side. mod unit_test_twins { + use std::str::FromStr as _; + use pepper_sync::wallet::IronwoodNote; use zcash_primitives::transaction::fees::zip317::{MARGINAL_FEE, MINIMUM_FEE}; use zcash_protocol::PoolType; use zcash_protocol::consensus::{BlockHeight, COINBASE_MATURITY_BLOCKS}; + use zcash_protocol::memo::Memo; use zcash_protocol::value::Zatoshis; use zingo_perspective::{LightClientPerspectiveExt as _, SentValueTransfer, ValueTransferKind}; use zingo_status::confirmation_status::ConfirmationStatus; @@ -176,7 +179,7 @@ mod unit_test_twins { utils::conversion::txid_from_hex_encoded_str(TEST_TXID).unwrap(), ), 0, - None, + Memo::Empty, )], orchard_notes: vec![], sapling_notes: vec![], @@ -218,7 +221,7 @@ mod unit_test_twins { utils::conversion::txid_from_hex_encoded_str(TEST_TXID).unwrap(), ), 0, - None, + Memo::Empty, )], orchard_notes: vec![], sapling_notes: vec![], @@ -228,7 +231,7 @@ mod unit_test_twins { outgoing_sapling_notes: vec![OutgoingNoteSummary { output_index: 0, value: first_send_to_sapling, - memo: None, + memo: Memo::Empty, recipient: "zregtestsapling1sa4rckrf4zs6ny3l3ljnezupacvxfnjjn90lpeaa4ddtjeyww2ypzqr3jxfsta3t8dn3jk8cm4f".to_string(), recipient_unified_address: Some("uregtest183rtm3qhxxermx3nxwa706va0xnypt3td648tayetchlp28hue08vrcnwq02ryyk5rh3y0xhftay8a5ynjdg8kr3juq5x0d9ygd5ffht".to_string()), account_id: AccountId::ZERO, @@ -255,7 +258,7 @@ mod unit_test_twins { 99_925_000, SpendStatus::Unspent, 0, - None, + Memo::Empty, )], orchard_notes: vec![], sapling_notes: vec![], @@ -344,7 +347,7 @@ mod unit_test_twins { utils::conversion::txid_from_hex_encoded_str(TEST_TXID).unwrap(), ), 0, - Some("Second wave incoming".to_string()), + Memo::from_str("Second wave incoming").unwrap(), )], orchard_notes: vec![], sapling_notes: vec![], @@ -388,7 +391,7 @@ mod unit_test_twins { utils::conversion::txid_from_hex_encoded_str(TEST_TXID).unwrap(), ), 0, - None, + Memo::Empty, )], orchard_notes: vec![], sapling_notes: vec![], @@ -427,7 +430,7 @@ TransactionSummary { 99_885_000, SpendStatus::Unspent, 0, - None, + Memo::Empty, )], orchard_notes: vec![], sapling_notes: vec![], @@ -437,7 +440,7 @@ TransactionSummary { outgoing_sapling_notes: vec![OutgoingNoteSummary { output_index: 0, value: second_send_to_sapling, - memo: None, + memo: Memo::Empty, recipient: "zregtestsapling1sa4rckrf4zs6ny3l3ljnezupacvxfnjjn90lpeaa4ddtjeyww2ypzqr3jxfsta3t8dn3jk8cm4f".to_string(), recipient_unified_address: Some("uregtest183rtm3qhxxermx3nxwa706va0xnypt3td648tayetchlp28hue08vrcnwq02ryyk5rh3y0xhftay8a5ynjdg8kr3juq5x0d9ygd5ffht".to_string()), account_id: AccountId::ZERO, @@ -477,7 +480,7 @@ TransactionSummary { 930_000, SpendStatus::Unspent, 0, - None, + Memo::Empty, )], orchard_notes: vec![], sapling_notes: vec![], diff --git a/zingo-perspective/src/ext.rs b/zingo-perspective/src/ext.rs index 2bc7783cce..3928e063a0 100644 --- a/zingo-perspective/src/ext.rs +++ b/zingo-perspective/src/ext.rs @@ -7,6 +7,7 @@ use std::collections::{HashMap, HashSet}; use zcash_protocol::PoolType; +use zcash_protocol::memo::Memo; use zingolib::lightclient::LightClient; use zingolib::wallet::LightWallet; @@ -24,6 +25,41 @@ use crate::value_transfer::{ SelfSendValueTransfer, SentValueTransfer, ValueTransfer, ValueTransferKind, ValueTransfers, }; +/// The text-only memo policy this editorial layer applies: a text memo +/// shows as its string; empty and non-text memos are not shown. The +/// canonical summaries carry every memo losslessly typed. +fn text_memo(memo: &Memo) -> Option { + if let Memo::Text(text) = memo { + Some(text.to_string()) + } else { + None + } +} + +/// All text memos on the transaction's wallet-received shielded notes, +/// in pool order ironwood, orchard, sapling: the received-memo view the +/// text-only policy exposes, driving the memo-to-self classification. +fn received_text_memos(transaction: &TransactionSummary) -> Vec { + transaction + .shielded_notes_by_pool() + .into_iter() + .flat_map(|(notes, _)| notes.iter().filter_map(|note| text_memo(¬e.memo))) + .collect() +} + +/// The value of the wallet-received shielded notes whose memos the +/// text-only policy displays, so a memo-to-self row's value agrees with +/// the memos it shows; memo-less change is excluded with the rest. +fn received_memo_value(transaction: &TransactionSummary) -> u64 { + transaction + .shielded_notes_by_pool() + .into_iter() + .flat_map(|(notes, _)| notes.iter()) + .filter(|note| text_memo(¬e.memo).is_some()) + .map(|note| note.value) + .sum() +} + /// Creates one value transfer of `kind` for each shielded pool the transaction /// received notes into, newest pool first (ironwood, orchard, sapling). fn shielded_pool_value_transfers( @@ -41,7 +77,10 @@ fn shielded_pool_value_transfers( notes.iter().map(|output| output.value).sum(), None, vec![pool], - notes.iter().filter_map(|note| note.memo.clone()).collect(), + notes + .iter() + .filter_map(|note| text_memo(¬e.memo)) + .collect(), ) }) .collect() @@ -136,7 +175,7 @@ fn create_send_value_transfers( .sum(); let memos: Vec = outgoing_notes_to_address .iter() - .filter_map(|&(note, _)| note.memo.clone()) + .filter_map(|&(note, _)| text_memo(¬e.memo)) .collect(); let has_notes_in = |pool: PoolType| { outgoing_notes_to_address @@ -238,12 +277,12 @@ impl LightWalletPerspectiveExt for LightWallet { value_transfers.append(&mut create_send_value_transfers(self, &transaction)?); // create 1 memo-to-self if any number of memos are received in the sending transaction - let memos = transaction.received_memos(); + let memos = received_text_memos(&transaction); if !memos.is_empty() { value_transfers.push(self_send_value_transfer( &transaction, SelfSendValueTransfer::MemoToSelf, - transaction.received_memo_value(), + received_memo_value(&transaction), memos, )); } @@ -264,7 +303,7 @@ impl LightWalletPerspectiveExt for LightWallet { // any number of memos, otherwise 1 basic send-to-self so every transaction // creates at least 1 value transfer. // (deshield and other pool-movement kinds may join this list later.) - let memos = transaction.received_memos(); + let memos = received_text_memos(&transaction); let self_send_kind = if transaction.is_orchard_to_ironwood_migration() { SelfSendValueTransfer::Migration } else if !memos.is_empty() { diff --git a/zingo-perspective/tests/derivation.rs b/zingo-perspective/tests/derivation.rs index ae04d35dbe..6bbaf58d64 100644 --- a/zingo-perspective/tests/derivation.rs +++ b/zingo-perspective/tests/derivation.rs @@ -24,7 +24,28 @@ use zingolib::config::ChainType; use zingolib::mocks::orchard_note::OrchardCryptoNoteBuilder; use zingolib::wallet::keys::unified::ReceiverSelection; -use common::{received_texts, regtest_wallet, sent}; +use common::{received, received_texts, regtest_wallet, sent}; + +/// The canonical summaries carry every memo losslessly typed; the +/// text-only policy is applied by this crate, never at construction. +/// An arbitrary-bytes memo is present and typed on the transaction +/// summary, while the derived value transfer shows no memo for it. +#[tokio::test] +async fn arbitrary_memo_is_carried_typed_and_hidden_editorially() { + let arbitrary = Memo::Arbitrary(Box::new([7u8; 511])); + let mut wallet = regtest_wallet(seeds::HOSPITAL_MUSEUM_SEED); + wallet.wallet_transactions.insert( + TxId::from_bytes([1; 32]), + received(1, 10, std::slice::from_ref(&arbitrary)), + ); + + let summaries = wallet.transaction_summaries(false).await.unwrap(); + assert_eq!(summaries.0[0].ironwood_notes[0].memo, arbitrary); + + let value_transfers = wallet.value_transfers(true).await.unwrap(); + assert_eq!(value_transfers.len(), 1); + assert!(value_transfers[0].memos.is_empty()); +} /// Migrated from libtonode `fast::filter_empty_messages`. #[tokio::test] diff --git a/zingolib/src/lightclient/mock_chain_tests.rs b/zingolib/src/lightclient/mock_chain_tests.rs index 59db43d5eb..7c946b207a 100644 --- a/zingolib/src/lightclient/mock_chain_tests.rs +++ b/zingolib/src/lightclient/mock_chain_tests.rs @@ -9,9 +9,12 @@ //! live versions are never removed. They eventually move to a gated //! "pre-migration" mod once side-by-side equivalence is documented). +use std::str::FromStr as _; + use pepper_sync::wallet::IronwoodNote; use zcash_protocol::PoolType; use zcash_protocol::ShieldedPool; +use zcash_protocol::memo::Memo; use crate::check_client_balances; use crate::testutils::lightclient::{from_inputs, get_base_address}; @@ -274,7 +277,7 @@ async fn send_to_transparent_and_sapling_maintain_balance() { recipient_initial_funds, SpendStatus::Spent(placeholder_txid), 0, - None, + Memo::Empty, )], orchard_notes: vec![], sapling_notes: vec![], @@ -309,7 +312,7 @@ async fn send_to_transparent_and_sapling_maintain_balance() { 99_960_000, SpendStatus::TransmittedSpent(placeholder_txid), 0, - None, + Memo::Empty, )], orchard_notes: vec![], sapling_notes: vec![], @@ -319,7 +322,7 @@ async fn send_to_transparent_and_sapling_maintain_balance() { outgoing_sapling_notes: vec![OutgoingNoteSummary { output_index: 0, value: first_send_to_sapling, - memo: None, + memo: Memo::Empty, recipient: "zregtestsapling1sa4rckrf4zs6ny3l3ljnezupacvxfnjjn90lpeaa4ddtjeyww2ypzqr3jxfsta3t8dn3jk8cm4f".to_string(), recipient_unified_address: Some("uregtest183rtm3qhxxermx3nxwa706va0xnypt3td648tayetchlp28hue08vrcnwq02ryyk5rh3y0xhftay8a5ynjdg8kr3juq5x0d9ygd5ffht".to_string()), account_id: zip32::AccountId::ZERO, @@ -345,7 +348,7 @@ async fn send_to_transparent_and_sapling_maintain_balance() { 99_925_000, SpendStatus::Unspent, 0, - None, + Memo::Empty, )], orchard_notes: vec![], sapling_notes: vec![], @@ -432,7 +435,7 @@ async fn send_to_transparent_and_sapling_maintain_balance() { recipient_second_funding, SpendStatus::Spent(placeholder_txid), 0, - Some("Second wave incoming".to_string()), + Memo::from_str("Second wave incoming").unwrap(), )], orchard_notes: vec![], sapling_notes: vec![], @@ -475,7 +478,7 @@ async fn send_to_transparent_and_sapling_maintain_balance() { 965_000, SpendStatus::Spent(placeholder_txid), 0, - None, + Memo::Empty, )], orchard_notes: vec![], sapling_notes: vec![], @@ -499,7 +502,7 @@ async fn send_to_transparent_and_sapling_maintain_balance() { 99_885_000, SpendStatus::Unspent, 0, - None, + Memo::Empty, )], orchard_notes: vec![], sapling_notes: vec![], @@ -509,7 +512,7 @@ async fn send_to_transparent_and_sapling_maintain_balance() { outgoing_sapling_notes: vec![OutgoingNoteSummary { output_index: 0, value: second_send_to_sapling, - memo: None, + memo: Memo::Empty, recipient: "zregtestsapling1sa4rckrf4zs6ny3l3ljnezupacvxfnjjn90lpeaa4ddtjeyww2ypzqr3jxfsta3t8dn3jk8cm4f".to_string(), recipient_unified_address: Some("uregtest183rtm3qhxxermx3nxwa706va0xnypt3td648tayetchlp28hue08vrcnwq02ryyk5rh3y0xhftay8a5ynjdg8kr3juq5x0d9ygd5ffht".to_string()), account_id: zip32::AccountId::ZERO, @@ -542,7 +545,7 @@ async fn send_to_transparent_and_sapling_maintain_balance() { 930_000, SpendStatus::Unspent, 0, - None, + Memo::Empty, )], orchard_notes: vec![], sapling_notes: vec![], diff --git a/zingolib/src/wallet/summary.rs b/zingolib/src/wallet/summary.rs index 19a2e7459f..d51d0d2202 100644 --- a/zingolib/src/wallet/summary.rs +++ b/zingolib/src/wallet/summary.rs @@ -3,8 +3,6 @@ /// Not to be used for internal logic in the system. use std::cmp::Ordering; -use zcash_protocol::memo::Memo; - use pepper_sync::keys::transparent; use pepper_sync::wallet::{ KeyIdInterface, NoteInterface, OutgoingNoteInterface, OutputInterface, TransparentCoin, @@ -54,17 +52,11 @@ impl LightWallet { .map(|output| { let spend_status = self.output_spend_status(output); - let memo = if let Memo::Text(memo_text) = output.memo() { - Some(memo_text.to_string()) - } else { - None - }; - BasicNoteSummary::from_parts( output.value(), spend_status, output.output_id().output_index(), - memo, + output.memo().clone(), ) }) .collect::>(); @@ -74,17 +66,11 @@ impl LightWallet { .map(|output| { let spend_status = self.output_spend_status(output); - let memo = if let Memo::Text(memo_text) = output.memo() { - Some(memo_text.to_string()) - } else { - None - }; - BasicNoteSummary::from_parts( output.value(), spend_status, output.output_id().output_index(), - memo, + output.memo().clone(), ) }) .collect::>(); @@ -94,17 +80,11 @@ impl LightWallet { .map(|output| { let spend_status = self.output_spend_status(output); - let memo = if let Memo::Text(memo_text) = output.memo() { - Some(memo_text.to_string()) - } else { - None - }; - BasicNoteSummary::from_parts( output.value(), spend_status, output.output_id().output_index(), - memo, + output.memo().clone(), ) }) .collect::>(); @@ -126,14 +106,8 @@ impl LightWallet { .outgoing_ironwood_notes() .iter() .map(|note| { - let memo = if let Memo::Text(memo_text) = note.memo() { - Some(memo_text.to_string()) - } else { - None - }; - Ok(OutgoingNoteSummary { - memo, + memo: note.memo().clone(), value: note.value(), recipient: note .encoded_recipient(&self.chain_type) @@ -150,14 +124,8 @@ impl LightWallet { .outgoing_orchard_notes() .iter() .map(|note| { - let memo = if let Memo::Text(memo_text) = note.memo() { - Some(memo_text.to_string()) - } else { - None - }; - Ok(OutgoingNoteSummary { - memo, + memo: note.memo().clone(), value: note.value(), recipient: note .encoded_recipient(&self.chain_type) @@ -173,25 +141,17 @@ impl LightWallet { let outgoing_sapling_notes = transaction .outgoing_sapling_notes() .iter() - .map(|note| { - let memo = if let Memo::Text(memo_text) = note.memo() { - Some(memo_text.to_string()) - } else { - None - }; - - OutgoingNoteSummary { - output_index: note.output_id().output_index(), - memo, - value: note.value(), - recipient: note - .encoded_recipient(&self.chain_type) - .expect("infallible"), - recipient_unified_address: note - .encoded_recipient_full_unified_address(&self.chain_type), - account_id: note.key_id().account_id, - scope: Scope::from(note.key_id().scope), - } + .map(|note| OutgoingNoteSummary { + output_index: note.output_id().output_index(), + memo: note.memo().clone(), + value: note.value(), + recipient: note + .encoded_recipient(&self.chain_type) + .expect("infallible"), + recipient_unified_address: note + .encoded_recipient_full_unified_address(&self.chain_type), + account_id: note.key_id().account_id, + scope: Scope::from(note.key_id().scope), }) .collect::>(); @@ -303,11 +263,6 @@ impl LightWallet { } }) .map(|note| { - let memo = if let Memo::Text(memo_text) = note.memo() { - Some(memo_text.to_string()) - } else { - None - }; let transaction = self.output_transaction(note); NoteSummary { @@ -315,7 +270,7 @@ impl LightWallet { status: transaction.status(), block_height: transaction.status().get_height(), spend_status: self.output_spend_status(note), - memo, + memo: note.memo().clone(), time: transaction.datetime(), txid: note.output_id().txid(), output_index: note.output_id().output_index(), diff --git a/zingolib/src/wallet/summary/data.rs b/zingolib/src/wallet/summary/data.rs index f0b4d43879..3bc7cbd5e2 100644 --- a/zingolib/src/wallet/summary/data.rs +++ b/zingolib/src/wallet/summary/data.rs @@ -3,6 +3,7 @@ use chrono::DateTime; use json::JsonValue; +use zcash_protocol::memo::Memo; use zcash_protocol::{PoolType, TxId, consensus::BlockHeight}; use pepper_sync::keys::transparent::TransparentScope; @@ -85,6 +86,18 @@ fn pools_to_json(pools: &[PoolType]) -> JsonValue { JsonValue::from(pool_names(pools)) } +/// The rendering the pre-typed summaries carried: a text memo renders as +/// its string; empty and non-text memos render as the absent memo did. +/// Display and JSON bytes stay identical to the era when construction +/// dropped non-text memos; the typed field carries them losslessly. +fn text_memo(memo: &Memo) -> Option { + if let Memo::Text(text) = memo { + Some(text.to_string()) + } else { + None + } +} + /// The pools flagged present, in protocol order. `present` indices are /// (transparent, sapling, orchard, ironwood). This function is the single /// definition of that order for pool lists exposed by summaries. @@ -162,15 +175,6 @@ impl TransactionSummary { ] } - /// All memos on this transaction's wallet-received shielded notes, in pool - /// order ironwood, orchard, sapling. - pub fn received_memos(&self) -> Vec { - self.shielded_notes_by_pool() - .into_iter() - .flat_map(|(notes, _)| notes.iter().filter_map(|note| note.memo.clone())) - .collect() - } - /// The sum of every output this transaction delivered to the wallet's own /// addresses: all wallet-received shielded notes plus transparent coins. pub fn self_received_value(&self) -> u64 { @@ -181,17 +185,6 @@ impl TransactionSummary { .sum() } - /// The sum of the wallet-received shielded notes that carry a memo, - /// excluding memo-less change. - pub fn received_memo_value(&self) -> u64 { - self.shielded_notes_by_pool() - .into_iter() - .flat_map(|(notes, _)| notes.iter()) - .filter(|note| note.memo.is_some()) - .map(|note| note.value) - .sum() - } - #[must_use] pub fn balance_delta(&self) -> Option { match self.kind { @@ -410,7 +403,7 @@ pub struct NoteSummary { pub status: ConfirmationStatus, pub block_height: BlockHeight, pub spend_status: SpendStatus, - pub memo: Option, + pub memo: Memo, pub time: u32, pub txid: TxId, pub output_index: u32, @@ -420,7 +413,7 @@ pub struct NoteSummary { impl std::fmt::Display for NoteSummary { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - let memo = self.memo.clone().unwrap_or_default(); + let memo = text_memo(&self.memo).unwrap_or_default(); let time = if let Some(dt) = chrono::DateTime::from_timestamp(i64::from(self.time), 0) { format!("{dt}") } else { @@ -460,7 +453,7 @@ impl From for json::JsonValue { "value" => note.value, "status" => format!("{} at block height {}", note.status, note.block_height), "spend_status" => note.spend_status.to_string(), - "memo" => note.memo, + "memo" => text_memo(¬e.memo), "time" => note.time, "txid" => note.txid.to_string(), "output_index" => note.output_index, @@ -545,7 +538,7 @@ pub struct BasicNoteSummary { pub value: u64, pub spend_status: SpendStatus, pub output_index: u32, - pub memo: Option, + pub memo: Memo, // TODO: add key id with address index, not implemented into sync engine yet } @@ -556,7 +549,7 @@ impl BasicNoteSummary { value: u64, spend_status: SpendStatus, output_index: u32, - memo: Option, + memo: Memo, ) -> Self { BasicNoteSummary { value, @@ -569,7 +562,7 @@ impl BasicNoteSummary { impl std::fmt::Display for BasicNoteSummary { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - let memo = self.memo.clone().unwrap_or_default(); + let memo = text_memo(&self.memo).unwrap_or_default(); write!( f, "\t{{ @@ -589,7 +582,7 @@ impl From for JsonValue { "value" => note.value, "spend_status" => note.spend_status.to_string(), "output_index" => note.output_index, - "memo" => note.memo, + "memo" => text_memo(¬e.memo), } } } @@ -735,7 +728,7 @@ impl std::fmt::Display for BasicCoinSummaries { #[derive(Clone, PartialEq, Debug)] pub struct OutgoingNoteSummary { pub value: u64, - pub memo: Option, + pub memo: Memo, pub recipient: String, pub recipient_unified_address: Option, pub output_index: u32, @@ -745,7 +738,7 @@ pub struct OutgoingNoteSummary { impl std::fmt::Display for OutgoingNoteSummary { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - let memo = self.memo.clone().unwrap_or_default(); + let memo = text_memo(&self.memo).unwrap_or_default(); let recipient_unified_address = self .recipient_unified_address .clone() @@ -777,7 +770,7 @@ impl From for JsonValue { fn from(note: OutgoingNoteSummary) -> Self { json::object! { "value" => note.value, - "memo" => note.memo, + "memo" => text_memo(¬e.memo), "recipient" => note.recipient, "recipient_unified_address" => note.recipient_unified_address, "output_index" => note.output_index, @@ -851,7 +844,7 @@ mod tests { use zingo_status::confirmation_status::ConfirmationStatus; fn note(value: u64) -> BasicNoteSummary { - BasicNoteSummary::from_parts(value, SpendStatus::Unspent, 0, None) + BasicNoteSummary::from_parts(value, SpendStatus::Unspent, 0, super::Memo::Empty) } /// A minimal send-to-self summary with the given funding pools and received