Skip to content

Commit

Permalink
refactor(network): remove unusued CmdOk type
Browse files Browse the repository at this point in the history
  • Loading branch information
RolandSherwin authored and b-zee committed Jun 20, 2024
1 parent 0231062 commit 78fcb7c
Show file tree
Hide file tree
Showing 5 changed files with 26 additions and 35 deletions.
2 changes: 1 addition & 1 deletion sn_node/src/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Expand Down
44 changes: 22 additions & 22 deletions sn_node/src/put_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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<CmdOk> {
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.
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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<CmdOk> {
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 {
Expand All @@ -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)
Expand Down Expand Up @@ -253,7 +256,7 @@ impl Node {
}

/// Store a `Chunk` to the RecordStore
pub(crate) fn store_chunk(&self, chunk: &Chunk) -> Result<CmdOk> {
pub(crate) fn store_chunk(&self, chunk: &Chunk) -> Result<()> {
let chunk_name = *chunk.name();
let chunk_addr = *chunk.address();

Expand All @@ -276,15 +279,15 @@ impl Node {
self.events_channel()
.broadcast(crate::NodeEvent::ChunkStored(chunk_addr));

Ok(CmdOk::StoredSuccessfully)
Ok(())
}

/// Validate and store a `Register` to the RecordStore
pub(crate) async fn validate_and_store_register(
&self,
register: SignedRegister,
with_payment: bool,
) -> Result<CmdOk> {
) -> Result<()> {
let reg_addr = register.address();
debug!("Validating and storing register {reg_addr:?}");

Expand All @@ -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(());
}
};

Expand All @@ -321,7 +324,7 @@ impl Node {
self.replicate_valid_fresh_record(key, RecordType::NonChunk(content_hash));
}

Ok(CmdOk::StoredSuccessfully)
Ok(())
}

/// Validate and store `Vec<SignedSpend>` to the RecordStore
Expand All @@ -330,7 +333,7 @@ impl Node {
&self,
signed_spends: Vec<SignedSpend>,
record_key: &RecordKey,
) -> Result<CmdOk> {
) -> Result<()> {
let pretty_key = PrettyPrintRecordKey::from(record_key);
debug!("Validating spends before storage at {pretty_key:?}");

Expand Down Expand Up @@ -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
Expand Down
6 changes: 2 additions & 4 deletions sn_node/src/replication.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(())
});
Expand Down
2 changes: 1 addition & 1 deletion sn_protocol/src/messages.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ pub use self::{
node_id::NodeId,
query::Query,
register::RegisterCmd,
response::{CmdOk, CmdResponse, QueryResponse},
response::{CmdResponse, QueryResponse},
};

use super::NetworkAddress;
Expand Down
7 changes: 0 additions & 7 deletions sn_protocol/src/messages/response.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}

0 comments on commit 78fcb7c

Please sign in to comment.