From c23eb32eacb826636fadaf6107b984a8890edaa9 Mon Sep 17 00:00:00 2001 From: grumbach Date: Thu, 2 Nov 2023 15:39:23 +0900 Subject: [PATCH] feat: keep transfers in mem instead of heavy cashnotes --- sn_client/src/wallet.rs | 24 +++---- sn_node/src/put_validation.rs | 59 +++++++++------- .../src/transfers/offline_transfer.rs | 4 +- sn_transfers/src/transfers/transfer.rs | 60 ++++------------ sn_transfers/src/wallet/local_store.rs | 68 ++++++++++--------- 5 files changed, 97 insertions(+), 118 deletions(-) diff --git a/sn_client/src/wallet.rs b/sn_client/src/wallet.rs index f3df871e13..a5af7e5856 100644 --- a/sn_client/src/wallet.rs +++ b/sn_client/src/wallet.rs @@ -59,13 +59,13 @@ impl WalletClient { pub fn get_payment_transfers(&self, address: &NetworkAddress) -> WalletResult> { match &address.as_xorname() { Some(xorname) => { - let cash_notes = self.wallet.get_payment_cash_notes(xorname); + let transfers = self.wallet.get_payment_transfers(xorname); info!( - "Payment cash notes retrieved from wallet: {:?}", - cash_notes.len() + "Payment transfers retrieved for {xorname:?} from wallet: {:?}", + transfers.len() ); - Ok(Transfer::transfers_from_cash_notes(cash_notes)?) + Ok(transfers) } None => Err(WalletError::InvalidAddressType), } @@ -139,10 +139,9 @@ impl WalletClient { content_addrs: impl Iterator, ) -> WalletResult<(NanoTokens, NanoTokens)> { let verify_store = true; - let mut payment_map = BTreeMap::default(); + // get store cost from network in parrallel let mut tasks = JoinSet::new(); - // we can collate all the payments together into one transfer for content_addr in content_addrs { let client = self.client.clone(); tasks.spawn(async move { @@ -156,8 +155,10 @@ impl WalletClient { (content_addr, costs) }); } - debug!("Pending store cost tasks: {:?}", tasks.len()); + + // collect store costs + let mut payment_map = BTreeMap::default(); while let Some(res) = tasks.join_next().await { // In case of cann't fetch cost from network for a content, // just skip it as it will then get verification failure, @@ -179,13 +180,14 @@ impl WalletClient { } } } - info!("Storecosts retrieved"); + // pay for records if payment_map.is_empty() { Ok((NanoTokens::zero(), NanoTokens::zero())) } else { - self.wallet.adjust_payment_map(&mut payment_map); + self.wallet + .take_previous_payments_into_account(&mut payment_map); self.pay_for_records(payment_map, verify_store).await } } @@ -200,16 +202,12 @@ impl WalletClient { all_data_payments: BTreeMap>, verify_store: bool, ) -> WalletResult<(NanoTokens, NanoTokens)> { - // TODO: - // Check for any existing payment CashNotes, and use them if they exist, only topping up if needs be - let total_cost = self .wallet .local_send_storage_payment(all_data_payments, None)?; // send to network trace!("Sending storage payment transfer to the network"); - let spend_attempt_result = self .client .send( diff --git a/sn_node/src/put_validation.rs b/sn_node/src/put_validation.rs index c3df090821..8614f8a832 100644 --- a/sn_node/src/put_validation.rs +++ b/sn_node/src/put_validation.rs @@ -414,8 +414,8 @@ impl Node { Ok(CmdOk::StoredSuccessfully) } - /// Gets CashNotes out of a Payment, this includes network verifications of the Transfer. - /// Return the CashNotes corresponding to our wallet and the trasfers corresponding to network royalties payment. + /// Gets CashNotes out of a Payment, this includes network verifications of the Transfers + /// Rewraps the royalties transfers into encrypted Transfers ready to be sent directly to the beneficiary async fn cash_notes_from_payment( &self, payment: &Vec, @@ -429,7 +429,7 @@ impl Node { for transfer in payment { match transfer { - Transfer::Encrypted(_) if cash_notes.is_empty() => match self + Transfer::Encrypted(_) => match self .network .verify_and_unpack_transfer(transfer, wallet) .await @@ -438,35 +438,41 @@ impl Node { Err(ProtocolError::FailedToDecypherTransfer) => continue, // transfer invalid Err(e) => return Err(e), - // transfer ok - Ok(cns) => cash_notes = cns, + // transfer ok, add to cash_notes and continue as more transfers might be ours + Ok(cns) => cash_notes.extend(cns), }, - Transfer::Encrypted(_) => continue, // we already have our cash notes, so we can skip this one Transfer::NetworkRoyalties(cashnote_redemptions) => { - if let Ok(cash_notes) = self + match self .network .verify_cash_notes_redemptions(royalties_pk, cashnote_redemptions) .await { - let received_royalties = - total_cash_notes_amount(&cash_notes, pretty_key.clone())?; - trace!( - "{} network royalties payment cash notes found for record {pretty_key} for a total value of {received_royalties:?}", - cash_notes.len() - ); - let encrypted_cashnote_redemptions = cash_notes - .into_iter() - .map(Transfer::transfers_from_cash_note) - .collect::, sn_transfers::Error>>() - .map_err(|err| { - error!("Error generating royalty transfer: {err:?}"); - ProtocolError::FailedToEncryptTransfer - })?; - - royalties_transfers.extend(encrypted_cashnote_redemptions); - received_fee = received_fee - .checked_add(received_royalties) - .ok_or_else(|| ProtocolError::PaymentExceedsTotalTokens)?; + Ok(cash_notes) => { + let received_royalties = + total_cash_notes_amount(&cash_notes, pretty_key.clone())?; + trace!( + "{} network royalties payment cash notes found for record {pretty_key} for a total value of {received_royalties:?}", + cash_notes.len() + ); + let rewraped_transfers = cash_notes + .into_iter() + .map(Transfer::transfers_from_cash_note) + .collect::, sn_transfers::Error>>() + .map_err(|err| { + error!("Error generating royalty transfer: {err:?}"); + ProtocolError::FailedToEncryptTransfer + })?; + + royalties_transfers.extend(rewraped_transfers); + received_fee = received_fee + .checked_add(received_royalties) + .ok_or_else(|| ProtocolError::PaymentExceedsTotalTokens)?; + } + Err(e) => { + warn!( + "Invalid network royalties payment for record {pretty_key}: {e:?}" + ); + } } } } @@ -522,6 +528,7 @@ impl Node { .set(wallet.balance().as_nano() as i64); if royalties_transfers.is_empty() { + warn!("No network royalties payment found for record {pretty_key}"); return Err(ProtocolError::NoNetworkRoyaltiesPayment( pretty_key.into_owned(), )); diff --git a/sn_transfers/src/transfers/offline_transfer.rs b/sn_transfers/src/transfers/offline_transfer.rs index a401349123..ee984a8416 100644 --- a/sn_transfers/src/transfers/offline_transfer.rs +++ b/sn_transfers/src/transfers/offline_transfer.rs @@ -8,7 +8,7 @@ use crate::{ rng, CashNote, DerivationIndex, DerivedSecretKey, Hash, Input, MainPubkey, NanoTokens, - SignedSpend, Transaction, TransactionBuilder, UniquePubkey, + SignedSpend, Transaction, TransactionBuilder, Transfer, UniquePubkey, }; use crate::{Error, Result}; @@ -36,7 +36,7 @@ pub struct OfflineTransfer { pub all_spend_requests: Vec, } -pub type PaymentDetails = (UniquePubkey, MainPubkey, NanoTokens); +pub type PaymentDetails = (Transfer, MainPubkey, NanoTokens); /// Xorname of data from which the content was fetched, mapping to the CashNote UniquePubkey (its id on disk) /// the main key for that CashNote and the value diff --git a/sn_transfers/src/transfers/transfer.rs b/sn_transfers/src/transfers/transfer.rs index ae370c754f..e6a8cb7495 100644 --- a/sn_transfers/src/transfers/transfer.rs +++ b/sn_transfers/src/transfers/transfer.rs @@ -13,7 +13,6 @@ use rayon::prelude::IntoParallelRefIterator; use serde::{Deserialize, Serialize}; use std::collections::hash_map::DefaultHasher; -use std::collections::BTreeMap; use std::hash::{Hash, Hasher}; use crate::error::{Error, Result}; @@ -54,50 +53,6 @@ impl std::fmt::Debug for Transfer { } impl Transfer { - /// This function is used to create Transfer from CashNotes, can be done offline, and sent to the recipient. - /// Creates Transfers from the given cash_notes - /// Grouping CashNotes by recipient into different transfers - /// This Transfer can be sent safely to the recipients as all data in it is encrypted - /// The recipients can then decrypt the data and use it to verify and reconstruct the CashNotes - pub fn transfers_from_cash_notes(cash_notes: Vec) -> Result> { - let mut cashnote_redemptions_map: BTreeMap> = - BTreeMap::new(); - for cash_note in cash_notes { - let recipient = cash_note.main_pubkey; - let derivation_index = cash_note.derivation_index(); - let parent_spend_addr = match cash_note.signed_spends.iter().next() { - Some(s) => SpendAddress::from_unique_pubkey(s.unique_pubkey()), - None => { - warn!( - "Skipping CashNote {cash_note:?} while creating Transfer as it has no parent spends." - ); - continue; - } - }; - - info!("Creating Transfer for CashNote {cash_note:?} with recipient {recipient:?} of value {:?}", cash_note.value()?); - let u = CashNoteRedemption::new(derivation_index, parent_spend_addr); - cashnote_redemptions_map - .entry(recipient) - .or_default() - .push(u); - } - - let mut transfers = Vec::new(); - for (recipient, cashnote_redemptions) in cashnote_redemptions_map { - let t = if recipient == *crate::NETWORK_ROYALTIES_PK { - // create a network royalties transfer type - Self::NetworkRoyalties(cashnote_redemptions.clone()) - } else { - Self::create(cashnote_redemptions, recipient) - .map_err(|_| Error::CashNoteRedemptionEncryptionFailed)? - }; - - transfers.push(t); - } - Ok(transfers) - } - /// This function is used to create a Transfer from a CashNote, can be done offline, and sent to the recipient. /// Creates a Transfer from the given cash_note /// This Transfer can be sent safely to the recipients as all data in it is encrypted @@ -118,6 +73,21 @@ impl Transfer { Ok(t) } + /// This function is used to create a Network Royalties Transfer from a CashNote + /// can be done offline, and should be sent along with a data payment + pub(crate) fn royalties_transfers_from_cash_note(cash_note: CashNote) -> Result { + let derivation_index = cash_note.derivation_index(); + let parent_spend_addr = match cash_note.signed_spends.iter().next() { + Some(s) => SpendAddress::from_unique_pubkey(s.unique_pubkey()), + None => { + return Err(Error::CashNoteHasNoParentSpends); + } + }; + + let u = CashNoteRedemption::new(derivation_index, parent_spend_addr); + Ok(Self::NetworkRoyalties(vec![u])) + } + /// Create a new transfer /// cashnote_redemptions: List of CashNoteRedemptions to be used for payment /// recipient: main Public key (donation key) of the recipient, diff --git a/sn_transfers/src/wallet/local_store.rs b/sn_transfers/src/wallet/local_store.rs index 520cc9099c..2ad6ab00f1 100644 --- a/sn_transfers/src/wallet/local_store.rs +++ b/sn_transfers/src/wallet/local_store.rs @@ -227,24 +227,17 @@ impl LocalWallet { } /// Return the payment cash_note ids for the given content address name if cached. - pub fn get_payment_cash_notes(&self, name: &XorName) -> Vec { - let ids = self.get_payment_unique_pubkeys_and_values(name); - // now grab all those cash_notes - let mut cash_notes: Vec = vec![]; - - if let Some(ids) = ids { - for (id, _main_pub_key, _value) in ids { - if let Some(cash_note) = load_created_cash_note(id, &self.wallet_dir) { - trace!( - "Current cash_note of chunk {name:?} is paying {:?} tokens.", - cash_note.value() - ); - cash_notes.push(cash_note); - } + pub fn get_payment_transfers(&self, name: &XorName) -> Vec { + let mut transfers: Vec = vec![]; + + if let Some(payment) = self.get_payment_unique_pubkeys_and_values(name) { + for (trans, _main_pub_key, value) in payment { + trace!("Current transfer for chunk {name:?} is of {value:?} tokens."); + transfers.push(trans.to_owned()); } } - cash_notes + transfers } /// Make a transfer and return all created cash_notes @@ -289,7 +282,7 @@ impl LocalWallet { Ok(created_cash_notes) } - /// Performs a CashNote payment for each content address. + /// Performs a payment for each content address. /// Returns the amount paid for storage, including the network royalties fee paid. pub fn local_send_storage_payment( &mut self, @@ -350,7 +343,7 @@ impl LocalWallet { for (content_addr, payees) in all_data_payments { for (payee, token) in payees { if let Some(cash_note) = - &offline_transfer + offline_transfer .created_cash_notes .iter() .find(|cash_note| { @@ -358,16 +351,27 @@ impl LocalWallet { && !used_cash_notes.contains(&cash_note.unique_pubkey().to_bytes()) }) { - trace!("Created transaction regarding {content_addr:?} paying {:?}(origin {token:?}) to payee {payee:?}.", - cash_note.value()); used_cash_notes.insert(cash_note.unique_pubkey().to_bytes()); let cash_notes_for_content: &mut Vec = all_transfers_per_address.entry(content_addr).or_default(); - cash_notes_for_content.push(( - cash_note.unique_pubkey(), - *cash_note.main_pubkey(), - cash_note.value()?, - )); + let value = cash_note.value(); + + // diffentiate between network royalties and storage payments + if cash_note.main_pubkey == *crate::NETWORK_ROYALTIES_PK { + trace!("Created netowrk royalties transaction regarding {content_addr:?} paying {value:?}(origin {token:?}) to payee {payee:?}."); + cash_notes_for_content.push(( + Transfer::royalties_transfers_from_cash_note(cash_note.to_owned())?, + *cash_note.main_pubkey(), + value?, + )); + } else { + trace!("Created transaction regarding {content_addr:?} paying {value:?}(origin {token:?}) to payee {payee:?}."); + cash_notes_for_content.push(( + Transfer::transfers_from_cash_note(cash_note.to_owned())?, + *cash_note.main_pubkey(), + value?, + )); + } } } } @@ -499,24 +503,24 @@ impl LocalWallet { /// Lookup previous payments and subtract them from the payments we're about to do. /// In essence this will make sure we don't overpay. - pub fn adjust_payment_map( + pub fn take_previous_payments_into_account( &self, payment_map: &mut BTreeMap>, ) { // For each target address for (xor, payments) in payment_map.iter_mut() { // All payments done for an address, should be multiple nodes. - if let Some(notes) = self.get_payment_unique_pubkeys_and_values(xor) { - for (_note_main_key, main_pub_key, note_value) in notes { - // Find new payment to a node, for which we have a cashnote already + if let Some(prev_payment) = self.get_payment_unique_pubkeys_and_values(xor) { + for (_transfer, prev_payment_pub_key, prev_payment_value) in prev_payment { + // Find new payment to a node, for which we have a transfer already if let Some((_main_pub_key, payment_tokens)) = payments .iter_mut() - .find(|(pubkey, _)| pubkey.public_key() == main_pub_key.public_key()) + .find(|(pubkey, _)| pubkey == prev_payment_pub_key) { // Subtract what we already paid from what we're about to pay. - // `checked_sub` overflows if would be negative, so we set it to 0. + // `checked_sub` underflows if would be negative, so we set it to 0. *payment_tokens = payment_tokens - .checked_sub(*note_value) + .checked_sub(*prev_payment_value) .unwrap_or(NanoTokens::zero()); } } @@ -994,7 +998,7 @@ mod tests { map.get_mut(&xor4).expect("to have value")[0].1 = 390.into(); // decrease: 400 -> 390 map.get_mut(&xor4).expect("to have value")[1].1 = 410.into(); // increase: 401 -> 410 - sender.adjust_payment_map(&mut map); + sender.take_previous_payments_into_account(&mut map); // The map should now only have the entries where store costs increased: assert_eq!(