From 78fcb7cccb823bac4c3a20f8b736e6f410e24505 Mon Sep 17 00:00:00 2001 From: Roland Sherwin Date: Thu, 20 Jun 2024 15:34:27 +0530 Subject: [PATCH] refactor(network): remove unusued CmdOk type --- sn_node/src/node.rs | 2 +- sn_node/src/put_validation.rs | 44 ++++++++++++++-------------- sn_node/src/replication.rs | 6 ++-- sn_protocol/src/messages.rs | 2 +- sn_protocol/src/messages/response.rs | 7 ----- 5 files changed, 26 insertions(+), 35 deletions(-) diff --git a/sn_node/src/node.rs b/sn_node/src/node.rs index 7265f25a58..c8ccf090ed 100644 --- a/sn_node/src/node.rs +++ b/sn_node/src/node.rs @@ -542,7 +542,7 @@ impl Node { let _handle = spawn(async move { let key = PrettyPrintRecordKey::from(&record.key).into_owned(); match self_clone.validate_and_store_record(record).await { - Ok(cmdok) => trace!("UnverifiedRecord {key} stored with {cmdok:?}."), + Ok(()) => trace!("UnverifiedRecord {key} has been stored"), Err(err) => { self_clone.record_metrics(Marker::RecordRejected(&key, &err)); } diff --git a/sn_node/src/put_validation.rs b/sn_node/src/put_validation.rs index 0fa016848b..3bbea878d2 100644 --- a/sn_node/src/put_validation.rs +++ b/sn_node/src/put_validation.rs @@ -10,7 +10,6 @@ use crate::{node::Node, quote::verify_quote_for_storecost, Error, Marker, Result use libp2p::kad::{Record, RecordKey}; use sn_networking::{get_raw_signed_spends_from_record, GetRecordError, NetworkError}; use sn_protocol::{ - messages::CmdOk, storage::{ try_deserialize_record, try_serialize_record, Chunk, RecordHeader, RecordKind, RecordType, SpendAddress, @@ -28,7 +27,7 @@ use xor_name::XorName; impl Node { /// Validate a record and it's payment, and store the record to the RecordStore - pub(crate) async fn validate_and_store_record(&self, record: Record) -> Result { + pub(crate) async fn validate_and_store_record(&self, record: Record) -> Result<()> { let record_header = RecordHeader::from_record(&record)?; // Notify replication_fetcher to mark the attempt as completed. @@ -57,7 +56,11 @@ impl Node { // we eagery retry replicaiton as it seems like other nodes are having trouble // did not manage to get this chunk as yet self.replicate_valid_fresh_record(record_key, RecordType::Chunk); - return Ok(CmdOk::DataAlreadyPresent); + trace!( + "Chunk with addr {:?} already exists: {already_exists}, payment extracted.", + chunk.network_address() + ); + return Ok(()); } // Finally before we store, lets bail for any payment issues @@ -164,7 +167,7 @@ impl Node { } /// Store a pre-validated, and already paid record to the RecordStore - pub(crate) async fn store_replicated_in_record(&self, record: Record) -> Result { + pub(crate) async fn store_replicated_in_record(&self, record: Record) -> Result<()> { trace!("Storing record which was replicated to us {:?}", record.key); let record_header = RecordHeader::from_record(&record)?; match record_header.kind { @@ -182,12 +185,12 @@ impl Node { let already_exists = self .validate_key_and_existence(&chunk.network_address(), &record_key) .await?; - trace!( - "Chunk with addr {:?} already exists?: {already_exists}", - chunk.network_address() - ); if already_exists { - return Ok(CmdOk::DataAlreadyPresent); + trace!( + "Chunk with addr {:?} already exists?: {already_exists}, do nothing", + chunk.network_address() + ); + return Ok(()); } self.store_chunk(&chunk) @@ -253,7 +256,7 @@ impl Node { } /// Store a `Chunk` to the RecordStore - pub(crate) fn store_chunk(&self, chunk: &Chunk) -> Result { + pub(crate) fn store_chunk(&self, chunk: &Chunk) -> Result<()> { let chunk_name = *chunk.name(); let chunk_addr = *chunk.address(); @@ -276,7 +279,7 @@ impl Node { self.events_channel() .broadcast(crate::NodeEvent::ChunkStored(chunk_addr)); - Ok(CmdOk::StoredSuccessfully) + Ok(()) } /// Validate and store a `Register` to the RecordStore @@ -284,7 +287,7 @@ impl Node { &self, register: SignedRegister, with_payment: bool, - ) -> Result { + ) -> Result<()> { let reg_addr = register.address(); debug!("Validating and storing register {reg_addr:?}"); @@ -299,7 +302,7 @@ impl Node { None => { // Notify replication_fetcher to mark the attempt as completed. self.network().notify_fetch_completed(key.clone()); - return Ok(CmdOk::DataAlreadyPresent); + return Ok(()); } }; @@ -321,7 +324,7 @@ impl Node { self.replicate_valid_fresh_record(key, RecordType::NonChunk(content_hash)); } - Ok(CmdOk::StoredSuccessfully) + Ok(()) } /// Validate and store `Vec` to the RecordStore @@ -330,7 +333,7 @@ impl Node { &self, signed_spends: Vec, record_key: &RecordKey, - ) -> Result { + ) -> Result<()> { let pretty_key = PrettyPrintRecordKey::from(record_key); debug!("Validating spends before storage at {pretty_key:?}"); @@ -396,17 +399,14 @@ impl Node { "Successfully stored validated spends with key: {unique_pubkey:?} at {pretty_key:?}" ); - // report double spends - if let Some(spend2) = maybe_spend2 { + // Just log the double spend attempt. DoubleSpend error during PUT is not used and would just lead to + // RecordRejected marker (which is incorrect, since we store double spends). + if maybe_spend2.is_some() { warn!("Got a double spend for the Spend PUT with unique_pubkey {unique_pubkey}"); - return Err(NetworkError::DoubleSpendAttempt( - Box::new(spend1), - Box::new(spend2), - ))?; } self.record_metrics(Marker::ValidSpendRecordPutFromNetwork(&pretty_key)); - Ok(CmdOk::StoredSuccessfully) + Ok(()) } /// Gets CashNotes out of Transfers, this includes network verifications of the Transfers diff --git a/sn_node/src/replication.rs b/sn_node/src/replication.rs index d8caf554e2..728014a754 100644 --- a/sn_node/src/replication.rs +++ b/sn_node/src/replication.rs @@ -79,10 +79,8 @@ impl Node { trace!( "Got Replication Record {pretty_key:?} from network, validating and storing it" ); - let result = node.store_replicated_in_record(record).await?; - trace!( - "Completed storing Replication Record {pretty_key:?} from network, result: {result:?}" - ); + node.store_replicated_in_record(record).await?; + trace!("Completed storing Replication Record {pretty_key:?} from network."); Ok(()) }); diff --git a/sn_protocol/src/messages.rs b/sn_protocol/src/messages.rs index a04dc0870e..7aebb88295 100644 --- a/sn_protocol/src/messages.rs +++ b/sn_protocol/src/messages.rs @@ -20,7 +20,7 @@ pub use self::{ node_id::NodeId, query::Query, register::RegisterCmd, - response::{CmdOk, CmdResponse, QueryResponse}, + response::{CmdResponse, QueryResponse}, }; use super::NetworkAddress; diff --git a/sn_protocol/src/messages/response.rs b/sn_protocol/src/messages/response.rs index 49635be740..93f05ecef3 100644 --- a/sn_protocol/src/messages/response.rs +++ b/sn_protocol/src/messages/response.rs @@ -117,10 +117,3 @@ pub enum CmdResponse { /// Response to the considered as bad notification PeerConsideredAsBad(Result<()>), } - -/// The Ok variant of a CmdResponse -#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -pub enum CmdOk { - StoredSuccessfully, - DataAlreadyPresent, -}