diff --git a/.github/workflows/memcheck.yml b/.github/workflows/memcheck.yml index d7e089b4da..528ba95adb 100644 --- a/.github/workflows/memcheck.yml +++ b/.github/workflows/memcheck.yml @@ -279,6 +279,16 @@ jobs: SN_LOG: "all" timeout-minutes: 10 + # Disable this test temporarily as it takes too long time, which blocks the merge queue. + # - name: Audit from genesis to collect entire spend DAG and dump to a dot file + # run: | + # cargo run --bin safe --release -- --log-output-dest=data-dir wallet audit --dot > spend_dag_and_statistics.txt + # echo "==============================================================================" + # cat spend_dag_and_statistics.txt + # env: + # SN_LOG: "all" + # timeout-minutes: 60 + - name: Check nodes running shell: bash timeout-minutes: 1 @@ -436,3 +446,12 @@ jobs: path: faucet_log.log continue-on-error: true if: always() + + # Re-enable this block once the previous step of audit got enabled. + # - name: Upload spend DAG and statistics + # uses: actions/upload-artifact@main + # with: + # name: memory_check_spend_dag_and_statistics + # path: spend_dag_and_statistics.txt + # continue-on-error: true + # if: always() diff --git a/Cargo.lock b/Cargo.lock index 0ba158c686..03d627fd86 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6876,6 +6876,7 @@ dependencies = [ "sn_logging", "sn_peers_acquisition", "sn_protocol", + "sn_transfers", "tempfile", "tiny-keccak", "tokio", diff --git a/sn_cli/Cargo.toml b/sn_cli/Cargo.toml index f4f2c3990d..08dd6f3051 100644 --- a/sn_cli/Cargo.toml +++ b/sn_cli/Cargo.toml @@ -62,6 +62,7 @@ sn_client = { path = "../sn_client", version = "0.105.3" } sn_logging = { path = "../sn_logging", version = "0.2.25" } sn_peers_acquisition = { path = "../sn_peers_acquisition", version = "0.2.10" } sn_protocol = { path = "../sn_protocol", version = "0.16.3" } +sn_transfers = { path = "../sn_transfers", version = "0.17.2" } tempfile = "3.6.0" tiny-keccak = "~2.0.2" tokio = { version = "1.32.0", features = [ diff --git a/sn_cli/src/bin/subcommands/wallet/audit.rs b/sn_cli/src/bin/subcommands/wallet/audit.rs index ca6c8adb15..b05d09bc90 100644 --- a/sn_cli/src/bin/subcommands/wallet/audit.rs +++ b/sn_cli/src/bin/subcommands/wallet/audit.rs @@ -40,7 +40,14 @@ async fn gather_spend_dag(client: &Client, root_dir: &Path) -> Result pub async fn audit(client: &Client, to_dot: bool, royalties: bool, root_dir: &Path) -> Result<()> { if to_dot { let dag = gather_spend_dag(client, root_dir).await?; + println!( + "========================== spends DAG diagraph =============================" + ); println!("{}", dag.dump_dot_format()); + println!( + "======================= spends purpose statistics ==========================" + ); + println!("{}", dag.dump_creation_reasons_statistics()); } else if royalties { let dag = gather_spend_dag(client, root_dir).await?; let royalties = dag.all_royalties()?; diff --git a/sn_cli/src/bin/subcommands/wallet/wo_wallet.rs b/sn_cli/src/bin/subcommands/wallet/wo_wallet.rs index 99e694d7b2..3659312b6a 100644 --- a/sn_cli/src/bin/subcommands/wallet/wo_wallet.rs +++ b/sn_cli/src/bin/subcommands/wallet/wo_wallet.rs @@ -20,6 +20,7 @@ use sn_client::transfers::{ WatchOnlyWallet, }; use sn_client::Client; +use sn_transfers::CASHNOTE_PURPOSE_OF_TRANSFER; use std::{ collections::{BTreeMap, BTreeSet}, path::Path, @@ -233,7 +234,7 @@ fn build_unsigned_transaction(from: &str, amount: &str, to: &str, root_dir: &Pat }; let unsigned_transfer = wallet.build_unsigned_transaction( - vec![("CASH_NOTE_REASON_FOR_TRANSFER".to_string(), amount, to)], + vec![(CASHNOTE_PURPOSE_OF_TRANSFER.to_string(), amount, to)], None, )?; diff --git a/sn_client/src/audit/spend_dag.rs b/sn_client/src/audit/spend_dag.rs index 0b5053273f..1e059e77fb 100644 --- a/sn_client/src/audit/spend_dag.rs +++ b/sn_client/src/audit/spend_dag.rs @@ -10,12 +10,35 @@ use petgraph::dot::Dot; use petgraph::graph::{DiGraph, NodeIndex}; use petgraph::visit::EdgeRef; use serde::{Deserialize, Serialize}; -use sn_transfers::{is_genesis_spend, CashNoteRedemption, NanoTokens, SignedSpend, SpendAddress}; -use std::collections::{BTreeMap, BTreeSet}; -use std::path::Path; +use sn_transfers::{ + is_genesis_spend, CashNoteRedemption, NanoTokens, SignedSpend, SpendAddress, + CASHNOTE_PURPOSE_OF_GENESIS, CASHNOTE_PURPOSE_OF_NETWORK_ROYALTIES, +}; +use std::{ + collections::{BTreeMap, BTreeSet}, + fmt, + path::Path, +}; use super::dag_error::{DagError, SpendFault}; +#[derive(Clone, Eq, PartialEq, Ord, PartialOrd, Hash, Serialize, Deserialize)] +pub struct SpendDagAddress { + address: SpendAddress, + purpose: String, +} + +impl std::fmt::Debug for SpendDagAddress { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> std::fmt::Result { + write!( + f, + "address: {}\npurpose: {}", + &self.address.to_hex()[0..6], + self.purpose + ) + } +} + /// A DAG representing the spends from a specific Spend all the way to the UTXOs. /// Starting from Genesis, this would encompass all the spends that have happened on the network /// at a certain point in time. @@ -33,13 +56,15 @@ use super::dag_error::{DagError, SpendFault}; #[derive(Debug, Clone, Serialize, Deserialize)] pub struct SpendDag { /// A directed graph of spend addresses - dag: DiGraph, + dag: DiGraph, /// All the spends refered to in the dag indexed by their SpendAddress spends: BTreeMap, /// The source of the DAG (aka Genesis) source: SpendAddress, /// Recorded faults in the DAG faults: BTreeMap>, + /// Creation reasons (purpose) of the spends + creation_reasons: BTreeMap, } type DagIndex = usize; @@ -91,6 +116,7 @@ impl SpendDag { spends: BTreeMap::new(), source, faults: BTreeMap::new(), + creation_reasons: BTreeMap::new(), } } @@ -110,17 +136,42 @@ impl SpendDag { Ok(()) } + fn get_dag_address(&self, spend_addr: &SpendAddress) -> SpendDagAddress { + let spend_creation_reason = + if let Some((creation_reason, _amount)) = self.creation_reasons.get(spend_addr) { + creation_reason.clone() + } else { + CASHNOTE_PURPOSE_OF_GENESIS.to_string() + }; + SpendDagAddress { + address: *spend_addr, + purpose: spend_creation_reason, + } + } + /// Insert a spend into the dag /// Creating edges (links) from its ancestors and to its descendants /// If the inserted spend is already known, it will be ignored /// If the inserted spend is a double spend, it will be saved along with the previous spend /// Return true if the spend was inserted and false if it was already in the DAG pub fn insert(&mut self, spend_addr: SpendAddress, spend: SignedSpend) -> bool { + for output in spend.outputs().iter() { + let output_spend_address = SpendAddress::from_unique_pubkey(&output.unique_pubkey); + if let Some(old) = self.creation_reasons.insert( + output_spend_address, + (output.purpose.clone(), output.amount), + ) { + error!("Spend {output_spend_address:?} already have an un-expected creation reason: {old:?}"); + } + } + + let spend_dag_address = self.get_dag_address(&spend_addr); + let existing_entry = self.spends.get(&spend_addr).cloned(); let new_node_idx = match existing_entry { // add new spend to the DAG None => { - let node_idx = self.dag.add_node(spend_addr); + let node_idx = self.dag.add_node(spend_dag_address); self.spends.insert( spend_addr, DagEntry::Spend(Box::new(spend.clone()), node_idx.index()), @@ -142,7 +193,7 @@ impl SpendDag { return false; } - let node_idx = self.dag.add_node(spend_addr); + let node_idx = self.dag.add_node(spend_dag_address); let double_spend = DagEntry::DoubleSpend(vec![ (existing_spend.clone(), idx), (spend.clone(), node_idx.index()), @@ -156,7 +207,7 @@ impl SpendDag { return false; } - let node_idx = self.dag.add_node(spend_addr); + let node_idx = self.dag.add_node(spend_dag_address); let mut vec_s = vec_s.clone(); vec_s.push((spend.clone(), node_idx.index())); self.spends.insert(spend_addr, DagEntry::DoubleSpend(vec_s)); @@ -167,10 +218,11 @@ impl SpendDag { // link to descendants for descendant in spend.spend.spent_tx.outputs.iter() { let descendant_addr = SpendAddress::from_unique_pubkey(&descendant.unique_pubkey); + let descendant_dag_address = self.get_dag_address(&descendant_addr); // add descendant if not already in dag let spends_at_addr = self.spends.entry(descendant_addr).or_insert_with(|| { - let node_idx = self.dag.add_node(descendant_addr); + let node_idx = self.dag.add_node(descendant_dag_address); DagEntry::NotGatheredYet(node_idx.index()) }); @@ -191,10 +243,11 @@ impl SpendDag { const PENDING_AMOUNT: NanoTokens = NanoTokens::from(0); for ancestor in spend.spend.parent_tx.inputs.iter() { let ancestor_addr = SpendAddress::from_unique_pubkey(&ancestor.unique_pubkey); + let ancestor_dag_address = self.get_dag_address(&ancestor_addr); // add ancestor if not already in dag let spends_at_addr = self.spends.entry(ancestor_addr).or_insert_with(|| { - let node_idx = self.dag.add_node(ancestor_addr); + let node_idx = self.dag.add_node(ancestor_dag_address); DagEntry::NotGatheredYet(node_idx.index()) }); @@ -265,7 +318,7 @@ impl SpendDag { .neighbors_directed(node_index, petgraph::Direction::Outgoing) .any(|_| true) { - let utxo_addr = self.dag[node_index]; + let utxo_addr = self.dag[node_index].address; leaves.insert(utxo_addr); } } @@ -276,6 +329,23 @@ impl SpendDag { format!("{:?}", Dot::with_config(&self.dag, &[])) } + pub fn dump_creation_reasons_statistics(&self) -> String { + let mut statistics: BTreeMap> = Default::default(); + for (_spend_addr, (reason, amount)) in self.creation_reasons.iter() { + let holders = statistics.entry(reason.clone()).or_default(); + holders.push(*amount); + } + let mut content = "Purpose,Times,Amount".to_string(); + for (purpose, payments) in statistics.iter() { + let total_amount: u64 = payments + .iter() + .map(|nano_tokens| nano_tokens.as_nano()) + .sum(); + content = format!("{content}\n{purpose},{},{total_amount}", payments.len()); + } + content + } + /// Merges the given dag into ours pub fn merge(&mut self, sub_dag: SpendDag) -> Result<(), DagError> { let source = self.source(); @@ -350,7 +420,7 @@ impl SpendDag { royalties.push(CashNoteRedemption::new( *derivation_idx, spend_addr, - "CASH_NOTE_REASON_FOR_NETWORK_ROYALTIES".to_string(), + CASHNOTE_PURPOSE_OF_NETWORK_ROYALTIES.to_string(), )); } } @@ -456,7 +526,7 @@ impl SpendDag { .flat_map(|idx| { self.dag .neighbors_directed(NodeIndex::new(idx), petgraph::Direction::Outgoing) - .map(|i| &self.dag[i]) + .map(|i| &self.dag[i].address) }) .collect(); @@ -494,7 +564,7 @@ impl SpendDag { self.dag .neighbors_directed(NodeIndex::new(idx), petgraph::Direction::Incoming) }) - .map(|parent_idx| &self.dag[parent_idx]) + .map(|parent_idx| &self.dag[parent_idx].address) .collect(); let non_orphans = BTreeSet::from_iter(all_descendants.into_iter().chain(parents).chain([source])); diff --git a/sn_client/src/wallet.rs b/sn_client/src/wallet.rs index 1f60f09d1e..ba034b7a82 100644 --- a/sn_client/src/wallet.rs +++ b/sn_client/src/wallet.rs @@ -18,6 +18,7 @@ use sn_protocol::NetworkAddress; use sn_transfers::{ CashNote, CashNoteOutputDetails, HotWallet, MainPubkey, NanoTokens, Payment, PaymentQuote, SignedSpend, SpendAddress, Transaction, Transfer, UniquePubkey, WalletError, WalletResult, + CASHNOTE_PURPOSE_OF_TRANSFER, }; use std::{ collections::{BTreeMap, BTreeSet}, @@ -307,7 +308,7 @@ impl WalletClient { verify_store: bool, ) -> WalletResult { let created_cash_notes = self.wallet.local_send( - vec![("CASH_NOTE_REASON_FOR_TRANSFER".to_string(), amount, to)], + vec![(CASHNOTE_PURPOSE_OF_TRANSFER.to_string(), amount, to)], None, )?; diff --git a/sn_transfers/src/cashnotes.rs b/sn_transfers/src/cashnotes.rs index baacd00590..8ee0c80d2d 100644 --- a/sn_transfers/src/cashnotes.rs +++ b/sn_transfers/src/cashnotes.rs @@ -15,6 +15,11 @@ mod signed_spend; mod transaction; mod unique_keys; +pub const CASHNOTE_PURPOSE_OF_GENESIS: &str = "GENESIS"; +pub const CASHNOTE_PURPOSE_OF_NETWORK_ROYALTIES: &str = "ROYALTY"; +pub const CASHNOTE_PURPOSE_OF_CHANGE: &str = "CHANGE"; +pub const CASHNOTE_PURPOSE_OF_TRANSFER: &str = "TRANSFER"; + pub(crate) use builder::{CashNoteBuilder, TransactionBuilder}; pub(crate) use transaction::Input; diff --git a/sn_transfers/src/cashnotes/signed_spend.rs b/sn_transfers/src/cashnotes/signed_spend.rs index d0b71f4a5b..2d0ebb6cdf 100644 --- a/sn_transfers/src/cashnotes/signed_spend.rs +++ b/sn_transfers/src/cashnotes/signed_spend.rs @@ -7,6 +7,7 @@ // permissions and limitations relating to use of the SAFE Network Software. use super::{Hash, NanoTokens, Transaction, UniquePubkey}; +use crate::cashnotes::transaction::Output; use crate::{DerivationIndex, Result, Signature, SpendAddress, TransferError}; use custom_debug::Debug; @@ -183,6 +184,11 @@ impl SignedSpend { trace!("Validated parent_spends for {unique_key}"); Ok(()) } + + /// Get a reference to the outputs + pub fn outputs(&self) -> &Vec { + &self.spend.spent_tx.outputs + } } // Impl manually to avoid clippy complaint about Hash conflict. diff --git a/sn_transfers/src/genesis.rs b/sn_transfers/src/genesis.rs index 2b02b68049..acdcce290d 100644 --- a/sn_transfers/src/genesis.rs +++ b/sn_transfers/src/genesis.rs @@ -8,6 +8,7 @@ use super::wallet::HotWallet; +use crate::cashnotes::CASHNOTE_PURPOSE_OF_GENESIS; use crate::{ CashNote, DerivationIndex, Hash, Input, MainPubkey, MainSecretKey, NanoTokens, SignedSpend, Transaction, TransactionBuilder, TransferError as CashNoteError, @@ -159,7 +160,7 @@ pub fn create_first_cash_note_from_key( ) .add_output( NanoTokens::from(GENESIS_CASHNOTE_AMOUNT), - "CASH_NOTE_REASON_FOR_GENESIS".to_string(), + CASHNOTE_PURPOSE_OF_GENESIS.to_string(), main_pubkey, derivation_index, ) diff --git a/sn_transfers/src/lib.rs b/sn_transfers/src/lib.rs index 90dc0cf21e..1bb42fd573 100644 --- a/sn_transfers/src/lib.rs +++ b/sn_transfers/src/lib.rs @@ -21,7 +21,8 @@ pub(crate) use cashnotes::{Input, TransactionBuilder}; pub use cashnotes::{ CashNote, CashNoteOutputDetails, DerivationIndex, DerivedSecretKey, Hash, MainPubkey, MainSecretKey, NanoTokens, SignedSpend, Spend, SpendAddress, Transaction, UniquePubkey, - UnsignedTransfer, + UnsignedTransfer, CASHNOTE_PURPOSE_OF_GENESIS, CASHNOTE_PURPOSE_OF_NETWORK_ROYALTIES, + CASHNOTE_PURPOSE_OF_TRANSFER, }; pub use error::{Result, TransferError}; pub use transfers::{CashNoteRedemption, OfflineTransfer, Transfer}; diff --git a/sn_transfers/src/transfers/offline_transfer.rs b/sn_transfers/src/transfers/offline_transfer.rs index a81d9130ab..4899bf4736 100644 --- a/sn_transfers/src/transfers/offline_transfer.rs +++ b/sn_transfers/src/transfers/offline_transfer.rs @@ -7,7 +7,7 @@ // permissions and limitations relating to use of the SAFE Network Software. use crate::{ - cashnotes::{CashNoteBuilder, UnsignedTransfer}, + cashnotes::{CashNoteBuilder, UnsignedTransfer, CASHNOTE_PURPOSE_OF_CHANGE}, rng, CashNote, CashNoteOutputDetails, DerivationIndex, DerivedSecretKey, Hash, Input, MainPubkey, NanoTokens, Result, SignedSpend, Transaction, TransactionBuilder, TransferError, UniquePubkey, NETWORK_ROYALTIES_PK, @@ -19,7 +19,7 @@ use std::collections::{BTreeMap, BTreeSet}; /// List of CashNotes, with (optionally when needed) their corresponding derived owning secret key. pub type CashNotesAndSecretKey = Vec<(CashNote, Option)>; -/// RecipientDetails: (amount, cash_note_reason, pub_key, derivation_index) +/// RecipientDetails: (amount, cash_note_purpose, pub_key, derivation_index) pub type TransferRecipientDetails = (NanoTokens, String, MainPubkey, DerivationIndex); /// Offline Transfer @@ -277,7 +277,7 @@ fn create_transaction_builder_with( if !change.is_zero() { tx_builder = tx_builder.add_output( change, - "CASH_NOTE_REASON_FOR_CHANGE".to_string(), + CASHNOTE_PURPOSE_OF_CHANGE.to_string(), change_to, derivation_index, ); diff --git a/sn_transfers/src/wallet/hot_wallet.rs b/sn_transfers/src/wallet/hot_wallet.rs index 55a0ff4999..60846f0a58 100644 --- a/sn_transfers/src/wallet/hot_wallet.rs +++ b/sn_transfers/src/wallet/hot_wallet.rs @@ -24,7 +24,7 @@ use crate::{ transfers::{CashNotesAndSecretKey, OfflineTransfer}, CashNote, CashNoteOutputDetails, CashNoteRedemption, DerivationIndex, DerivedSecretKey, Hash, MainPubkey, MainSecretKey, NanoTokens, SignedSpend, Spend, Transaction, Transfer, UniquePubkey, - WalletError, NETWORK_ROYALTIES_PK, + WalletError, CASHNOTE_PURPOSE_OF_NETWORK_ROYALTIES, NETWORK_ROYALTIES_PK, }; use std::{ collections::{BTreeMap, BTreeSet, HashSet}, @@ -402,7 +402,7 @@ impl HotWallet { let royalties_fee = calculate_royalties_fee(quote.cost); let royalties_payee = ( royalties_fee, - "CASH_NOTE_REASON_FOR_NETWORK_ROYALTIES".to_string(), + CASHNOTE_PURPOSE_OF_NETWORK_ROYALTIES.to_string(), *NETWORK_ROYALTIES_PK, DerivationIndex::random(&mut rng), );