Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: store multiple double spend attempts #1909

Merged
merged 2 commits into from
Jun 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
80 changes: 43 additions & 37 deletions sn_client/src/audit/dag_crawling.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ const SPENDS_PROCESSING_BUFFER_SIZE: usize = 4096;

enum InternalGetNetworkSpend {
Spend(Box<SignedSpend>),
DoubleSpend(Box<SignedSpend>, Box<SignedSpend>),
DoubleSpend(Vec<SignedSpend>),
NotFound,
Error(Error),
}
Expand All @@ -33,17 +33,18 @@ impl Client {
pub async fn new_dag_with_genesis_only(&self) -> WalletResult<SpendDag> {
let genesis_addr = SpendAddress::from_unique_pubkey(&GENESIS_SPEND_UNIQUE_KEY);
let mut dag = SpendDag::new(genesis_addr);
let genesis_spend = match self.get_spend_from_network(genesis_addr).await {
Ok(s) => s,
Err(Error::Network(NetworkError::DoubleSpendAttempt(spend1, spend2))) => {
let addr = spend1.address();
println!("Double spend detected at Genesis: {addr:?}");
dag.insert(genesis_addr, *spend2);
*spend1
match self.get_spend_from_network(genesis_addr).await {
Ok(spend) => {
dag.insert(genesis_addr, spend);
}
Err(Error::Network(NetworkError::DoubleSpendAttempt(spends))) => {
println!("Double spend detected at Genesis: {genesis_addr:?}");
for spend in spends.into_iter() {
dag.insert(genesis_addr, spend);
}
}
Err(e) => return Err(WalletError::FailedToGetSpend(e.to_string())),
};
dag.insert(genesis_addr, genesis_spend);

Ok(dag)
}
Expand Down Expand Up @@ -163,7 +164,7 @@ impl Client {
.map_err(|e| WalletError::SpendProcessing(e.to_string()));
addrs_to_get.extend(for_further_track);
}
InternalGetNetworkSpend::DoubleSpend(_s1, _s2) => {
InternalGetNetworkSpend::DoubleSpend(_spends) => {
warn!("Detected double spend regarding {address:?}");
}
InternalGetNetworkSpend::NotFound => {
Expand Down Expand Up @@ -193,14 +194,27 @@ impl Client {
let mut utxos = BTreeSet::new();

// get first spend
let first_spend = match self.crawl_spend(spend_addr).await {
InternalGetNetworkSpend::Spend(s) => *s,
InternalGetNetworkSpend::DoubleSpend(s1, s2) => {
let mut txs_to_follow = match self.crawl_spend(spend_addr).await {
InternalGetNetworkSpend::Spend(spend) => {
let spend = *spend;
let txs = BTreeSet::from_iter([spend.spend.spent_tx.clone()]);

spend_processing
.send(*s2)
.send(spend)
.await
.map_err(|e| WalletError::SpendProcessing(e.to_string()))?;
*s1
txs
}
InternalGetNetworkSpend::DoubleSpend(spends) => {
let mut txs = BTreeSet::new();
for spend in spends.into_iter() {
txs.insert(spend.spend.spent_tx.clone());
spend_processing
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Earlier only the first spend was sent for spend_processing, but here we do it for all the spends, is this alright?

.send(spend)
.await
.map_err(|e| WalletError::SpendProcessing(e.to_string()))?;
}
txs
}
InternalGetNetworkSpend::NotFound => {
// the cashnote was not spent yet, so it's an UTXO
Expand All @@ -212,13 +226,8 @@ impl Client {
return Err(WalletError::FailedToGetSpend(e.to_string()));
}
};
spend_processing
.send(first_spend.clone())
.await
.map_err(|e| WalletError::SpendProcessing(e.to_string()))?;

// use iteration instead of recursion to avoid stack overflow
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comment not relevant anymore?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is for the one inside the for loop? I'm not sure.

let mut txs_to_follow = BTreeSet::from_iter([first_spend.spend.spent_tx]);
let mut known_tx = BTreeSet::new();
let mut gen: u32 = 0;
let start = std::time::Instant::now();
Expand Down Expand Up @@ -260,18 +269,15 @@ impl Client {
.await
.map_err(|e| WalletError::SpendProcessing(e.to_string()))?;
}
InternalGetNetworkSpend::DoubleSpend(s1, s2) => {
info!("Fetched double spend at {addr:?} from network, following both...");
next_gen_tx.insert(s1.spend.spent_tx.clone());
next_gen_tx.insert(s2.spend.spent_tx.clone());
spend_processing
.send(*s1.clone())
.await
.map_err(|e| WalletError::SpendProcessing(e.to_string()))?;
spend_processing
.send(*s2.clone())
.await
.map_err(|e| WalletError::SpendProcessing(e.to_string()))?;
InternalGetNetworkSpend::DoubleSpend(spends) => {
info!("Fetched double spend(s) of len {} at {addr:?} from network, following all of them.", spends.len());
for s in spends.into_iter() {
next_gen_tx.insert(s.spend.spent_tx.clone());
spend_processing
.send(s.clone())
.await
.map_err(|e| WalletError::SpendProcessing(e.to_string()))?;
}
}
InternalGetNetworkSpend::NotFound => {
info!("Reached UTXO at {addr:?}");
Expand Down Expand Up @@ -360,8 +366,8 @@ impl Client {
InternalGetNetworkSpend::Spend(s) => {
spends.insert(*s);
}
InternalGetNetworkSpend::DoubleSpend(s1, s2) => {
spends.extend([*s1, *s2]);
InternalGetNetworkSpend::DoubleSpend(s) => {
spends.extend(s.into_iter());
}
InternalGetNetworkSpend::NotFound => {
return Err(WalletError::FailedToGetSpend(format!(
Expand Down Expand Up @@ -498,9 +504,9 @@ impl Client {
debug!("DAG crawling: spend at {spend_addr:?} not found on the network");
InternalGetNetworkSpend::NotFound
}
Err(Error::Network(NetworkError::DoubleSpendAttempt(s1, s2))) => {
debug!("DAG crawling: got a double spend at {spend_addr:?} on the network");
InternalGetNetworkSpend::DoubleSpend(s1, s2)
Err(Error::Network(NetworkError::DoubleSpendAttempt(spends))) => {
debug!("DAG crawling: got a double spend(s) of len {} at {spend_addr:?} on the network", spends.len());
InternalGetNetworkSpend::DoubleSpend(spends)
}
Err(e) => {
debug!(
Expand Down
2 changes: 1 addition & 1 deletion sn_networking/src/driver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ pub(crate) type BadNodes = BTreeMap<PeerId, (Vec<(NodeIssue, Instant)>, bool)>;
/// What is the largest packet to send over the network.
/// Records larger than this will be rejected.
// TODO: revisit once cashnote_redemption is in
const MAX_PACKET_SIZE: usize = 1024 * 1024 * 5; // the chunk size is 1mb, so should be higher than that to prevent failures, 5mb here to allow for CashNote storage
pub const MAX_PACKET_SIZE: usize = 1024 * 1024 * 5; // the chunk size is 1mb, so should be higher than that to prevent failures, 5mb here to allow for CashNote storage

// Timeout for requests sent/received through the request_response behaviour.
const REQUEST_TIMEOUT_DEFAULT_S: Duration = Duration::from_secs(30);
Expand Down
4 changes: 2 additions & 2 deletions sn_networking/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,8 +138,8 @@ pub enum NetworkError {
// ---------- Spend Errors
#[error("Spend not found: {0:?}")]
NoSpendFoundInsideRecord(SpendAddress),
#[error("A double spend was detected. Two diverging signed spends: {0:?}, {1:?}")]
DoubleSpendAttempt(Box<SignedSpend>, Box<SignedSpend>),
#[error("Double spend(s) was detected. The signed spends are: {0:?}")]
DoubleSpendAttempt(Vec<SignedSpend>),

// ---------- Store Error
#[error("No Store Cost Responses")]
Expand Down
6 changes: 2 additions & 4 deletions sn_networking/src/event/kad.rs
Original file line number Diff line number Diff line change
Expand Up @@ -412,10 +412,8 @@ impl SwarmDriver {
}
if !accumulated_spends.is_empty() {
info!("For record {pretty_key:?} task {query_id:?}, found split record for a spend, accumulated and sending them as a single record");
let accumulated_spends = accumulated_spends
.into_iter()
.take(2)
.collect::<Vec<SignedSpend>>();
let accumulated_spends =
accumulated_spends.into_iter().collect::<Vec<SignedSpend>>();

let bytes = try_serialize_record(&accumulated_spends, RecordKind::Spend)?;

Expand Down
4 changes: 3 additions & 1 deletion sn_networking/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,9 @@ pub use target_arch::{interval, sleep, spawn, Instant, Interval};

pub use self::{
cmd::{NodeIssue, SwarmLocalState},
driver::{GetRecordCfg, NetworkBuilder, PutRecordCfg, SwarmDriver, VerificationKind},
driver::{
GetRecordCfg, NetworkBuilder, PutRecordCfg, SwarmDriver, VerificationKind, MAX_PACKET_SIZE,
},
error::{GetRecordError, NetworkError},
event::{MsgResponder, NetworkEvent},
record_store::{calculate_cost_for_records, NodeRecordStore},
Expand Down
3 changes: 2 additions & 1 deletion sn_networking/src/record_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
// permissions and limitations relating to use of the SAFE Network Software.
#![allow(clippy::mutable_key_type)] // for the Bytes in NetworkAddress

use crate::driver::MAX_PACKET_SIZE;
use crate::target_arch::{spawn, Instant};
use crate::CLOSE_GROUP_SIZE;
use crate::{cmd::SwarmCmd, event::NetworkEvent, log_markers::Marker, send_swarm_cmd};
Expand Down Expand Up @@ -109,7 +110,7 @@ impl Default for NodeRecordStoreConfig {
storage_dir: historic_quote_dir.clone(),
historic_quote_dir,
max_records: MAX_RECORDS_COUNT,
max_value_bytes: 65 * 1024,
max_value_bytes: MAX_PACKET_SIZE,
}
}
}
Expand Down
21 changes: 11 additions & 10 deletions sn_networking/src/transfers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -236,21 +236,22 @@ pub fn get_signed_spend_from_record(
address: &SpendAddress,
record: &Record,
) -> Result<SignedSpend> {
match get_raw_signed_spends_from_record(record)?.as_slice() {
[one, two, ..] => {
warn!("Found double spend for {address:?}");
Err(NetworkError::DoubleSpendAttempt(
Box::new(one.to_owned()),
Box::new(two.to_owned()),
))
let spends = get_raw_signed_spends_from_record(record)?;
match spends.as_slice() {
[] => {
error!("Found no spend for {address:?}");
Err(NetworkError::NoSpendFoundInsideRecord(*address))
}
[one] => {
trace!("Spend get for address: {address:?} successful");
Ok(one.clone())
}
_ => {
trace!("Found no spend for {address:?}");
Err(NetworkError::NoSpendFoundInsideRecord(*address))
_double_spends => {
warn!(
"Found double spend(s) of len {} for {address:?}",
spends.len()
);
Err(NetworkError::DoubleSpendAttempt(spends))
}
}
}
Loading
Loading