Skip to content

Commit

Permalink
feat(protocol)!: implement PrettyPrintRecordKey as a Cow type
Browse files Browse the repository at this point in the history
- this allows us to hold a reference to RecordKey when we just want to
  log things.
  • Loading branch information
RolandSherwin authored and joshuef committed Oct 24, 2023
1 parent 2487e85 commit 1174fbf
Show file tree
Hide file tree
Showing 18 changed files with 142 additions and 127 deletions.
16 changes: 8 additions & 8 deletions sn_client/src/api.rs
Original file line number Diff line number Diff line change
Expand Up @@ -284,7 +284,7 @@ impl Client {

debug!(
"Got record from the network, {:?}",
PrettyPrintRecordKey::from(record.key.clone())
PrettyPrintRecordKey::from(&record.key)
);

let register = get_register_from_record(record)
Expand Down Expand Up @@ -497,7 +497,7 @@ impl Client {

trace!(
"Getting spend {unique_pubkey:?} with record_key {:?}",
PrettyPrintRecordKey::from(key.clone())
PrettyPrintRecordKey::from(&key)
);
let record = self
.network
Expand All @@ -510,7 +510,7 @@ impl Client {
})?;
debug!(
"For spend {unique_pubkey:?} got record from the network, {:?}",
PrettyPrintRecordKey::from(record.key.clone())
PrettyPrintRecordKey::from(&record.key)
);

let header = RecordHeader::from_record(&record).map_err(|err| {
Expand Down Expand Up @@ -564,7 +564,7 @@ impl Client {
error!("Found double spend for {address:?}");
Err(Error::CouldNotVerifyTransfer(format!(
"Found double spend for the unique_pubkey {unique_pubkey:?} - {:?}: spend_one {:?} and spend_two {:?}",
PrettyPrintRecordKey::from(key), one.derived_key_sig, two.derived_key_sig
PrettyPrintRecordKey::from(&key), one.derived_key_sig, two.derived_key_sig
)))
}
}
Expand Down Expand Up @@ -615,15 +615,15 @@ fn merge_split_register_records(
address: RegisterAddress,
map: &HashMap<XorName, (Record, HashSet<PeerId>)>,
) -> Result<SignedRegister> {
let key =
PrettyPrintRecordKey::from(NetworkAddress::from_register_address(address).to_record_key());
debug!("Got multiple records from the network for key: {key:?}");
let key = NetworkAddress::from_register_address(address).to_record_key();
let pretty_key = PrettyPrintRecordKey::from(&key);
debug!("Got multiple records from the network for key: {pretty_key:?}");
let mut all_registers = vec![];
for (record, peers) in map.values() {
match get_register_from_record(record.clone()) {
Ok(r) => all_registers.push(r),
Err(e) => {
warn!("Ignoring invalid register record found for {key:?} received from {peers:?}: {:?}", e);
warn!("Ignoring invalid register record found for {pretty_key:?} received from {peers:?}: {:?}", e);
continue;
}
}
Expand Down
2 changes: 1 addition & 1 deletion sn_client/src/chunks/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ pub(crate) type Result<T, E = Error> = std::result::Result<T, E>;
#[allow(missing_docs)]
pub enum Error {
#[error("Failed to get find payment for record: {0:?}")]
NoPaymentForRecord(PrettyPrintRecordKey),
NoPaymentForRecord(PrettyPrintRecordKey<'static>),

#[error("Failed to get chunk permit")]
CouldNotGetChunkPermit,
Expand Down
6 changes: 3 additions & 3 deletions sn_client/src/file_apis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -169,9 +169,9 @@ impl Files {

if payment.is_empty() {
warn!("Failed to get payment proof for chunk: {chunk_addr:?} it was not found in the local wallet");
return Err(ChunksError::NoPaymentForRecord(PrettyPrintRecordKey::from(
chunk_addr.to_record_key(),
)))?;
return Err(ChunksError::NoPaymentForRecord(
PrettyPrintRecordKey::from(&chunk_addr.to_record_key()).into_owned(),
))?;
}

trace!(
Expand Down
4 changes: 2 additions & 2 deletions sn_networking/src/cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ impl SwarmDriver {
let query_id = self.swarm.behaviour_mut().kademlia.get_record(key.clone());

if !expected_holders.is_empty() {
debug!("Record {:?} with task {query_id:?} expected to be held by {expected_holders:?}", PrettyPrintRecordKey::from(key));
debug!("Record {:?} with task {query_id:?} expected to be held by {expected_holders:?}", PrettyPrintRecordKey::from(&key));
}

if self
Expand Down Expand Up @@ -225,7 +225,7 @@ impl SwarmDriver {
let _ = sender.send(record);
}
SwarmCmd::PutRecord { record, sender } => {
let record_key = PrettyPrintRecordKey::from(record.key.clone());
let record_key = PrettyPrintRecordKey::from(&record.key).into_owned();
trace!(
"Putting record sized: {:?} to network {:?}",
record.value.len(),
Expand Down
7 changes: 3 additions & 4 deletions sn_networking/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,10 @@ pub enum Error {

/// No put_record attempts were successfully verified.
#[error("Could not retrieve the record after storing it: {0:}")]
FailedToVerifyRecordWasStored(PrettyPrintRecordKey),
FailedToVerifyRecordWasStored(PrettyPrintRecordKey<'static>),

#[error("Record retrieved from the network does not match the one we attempted to store {0:}")]
ReturnedRecordDoesNotMatch(PrettyPrintRecordKey),
ReturnedRecordDoesNotMatch(PrettyPrintRecordKey<'static>),

#[error("Could not create storage dir: {path:?}, error: {source}")]
FailedToCreateRecordStoreDir {
Expand Down Expand Up @@ -130,8 +130,7 @@ mod tests {
let address = ChunkAddress::new(xor_name);
let network_address = NetworkAddress::from_chunk_address(address);
let record_key = network_address.to_record_key();
let pretty_record: PrettyPrintRecordKey = record_key.into();
let record_str = format!("{}", pretty_record);
let record_str = format!("{}", PrettyPrintRecordKey::from(&record_key));
let xor_name_str = format!(
"{:64x}({:?})",
xor_name,
Expand Down
18 changes: 9 additions & 9 deletions sn_networking/src/event.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ impl Debug for NetworkEvent {
NetworkEvent::KeysForReplication(list) => {
let pretty_list: Vec<_> = list
.iter()
.map(|(holder, key)| (*holder, PrettyPrintRecordKey::from(key.clone())))
.map(|(holder, key)| (*holder, PrettyPrintRecordKey::from(key)))
.collect();
write!(f, "NetworkEvent::KeysForReplication({pretty_list:?})")
}
Expand All @@ -183,11 +183,11 @@ impl Debug for NetworkEvent {
write!(f, "NetworkEvent::NatStatusChanged({nat_status:?})")
}
NetworkEvent::UnverifiedRecord(record) => {
let pretty_key = PrettyPrintRecordKey::from(record.key.clone());
let pretty_key = PrettyPrintRecordKey::from(&record.key);
write!(f, "NetworkEvent::UnverifiedRecord({pretty_key:?})")
}
NetworkEvent::FailedToWrite(record_key) => {
let pretty_key = PrettyPrintRecordKey::from(record_key.clone());
let pretty_key = PrettyPrintRecordKey::from(record_key);
write!(f, "NetworkEvent::FailedToWrite({pretty_key:?})")
}
NetworkEvent::GossipsubMsgReceived { topic, .. } => {
Expand Down Expand Up @@ -607,7 +607,7 @@ impl SwarmDriver {
let content_hash = XorName::from_content(&peer_record.record.value);
trace!(
"Query task {id:?} returned with record {:?}(content {content_hash:?}) from peer {:?}, {stats:?} - {step:?}",
PrettyPrintRecordKey::from(peer_record.record.key.clone()),
PrettyPrintRecordKey::from(&peer_record.record.key),
peer_record.peer
);
self.accumulate_get_record_ok(id, peer_record, step.count);
Expand All @@ -634,7 +634,7 @@ impl SwarmDriver {
.map_err(|_| Error::InternalMsgChannelDropped)?;
format!(
"Getting record {:?} completed with only {:?} copies received",
PrettyPrintRecordKey::from(record.key.clone()),
PrettyPrintRecordKey::from(&record.key),
usize::from(step.count) - 1
)
} else {
Expand Down Expand Up @@ -664,22 +664,22 @@ impl SwarmDriver {
match err.clone() {
GetRecordError::NotFound { key, closest_peers } => {
info!("Query task {id:?} NotFound record {:?} among peers {closest_peers:?}, {stats:?} - {step:?}",
PrettyPrintRecordKey::from(key.clone()));
PrettyPrintRecordKey::from(&key));
}
GetRecordError::QuorumFailed {
key,
records,
quorum,
} => {
let pretty_key = PrettyPrintRecordKey::from(key.clone());
let pretty_key = PrettyPrintRecordKey::from(&key);
let peers = records
.iter()
.map(|peer_record| peer_record.peer)
.collect_vec();
info!("Query task {id:?} QuorumFailed record {pretty_key:?} among peers {peers:?} with quorum {quorum:?}, {stats:?} - {step:?}");
}
GetRecordError::Timeout { key } => {
let pretty_key = PrettyPrintRecordKey::from(key.clone());
let pretty_key = PrettyPrintRecordKey::from(&key);

debug!(
"Query task {id:?} timed out when looking for record {pretty_key:?}"
Expand Down Expand Up @@ -909,7 +909,7 @@ impl SwarmDriver {
if let Some((sender, mut result_map, quorum, mut expected_holders)) =
self.pending_get_record.remove(&query_id)
{
let pretty_key = PrettyPrintRecordKey::from(peer_record.record.key.clone());
let pretty_key = PrettyPrintRecordKey::from(&peer_record.record.key).into_owned();

if !expected_holders.is_empty() {
if expected_holders.remove(&peer_id) {
Expand Down
34 changes: 13 additions & 21 deletions sn_networking/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -295,12 +295,11 @@ impl Network {
let total_attempts = if re_attempt { VERIFICATION_ATTEMPTS } else { 1 };

let mut verification_attempts = 0;
let pretty_key = PrettyPrintRecordKey::from(key.clone());
let pretty_key = PrettyPrintRecordKey::from(&key);
while verification_attempts < total_attempts {
verification_attempts += 1;
info!(
"Getting record of {:?} attempts {verification_attempts:?}/{total_attempts:?}",
PrettyPrintRecordKey::from(key.clone()),
"Getting record of {pretty_key:?} attempts {verification_attempts:?}/{total_attempts:?}",
);

let (sender, receiver) = oneshot::channel();
Expand All @@ -318,10 +317,7 @@ impl Network {
Ok(returned_record) => {
let header = RecordHeader::from_record(&returned_record)?;
let is_chunk = matches!(header.kind, RecordKind::Chunk);
info!(
"Record returned: {:?}",
PrettyPrintRecordKey::from(key.clone())
);
info!("Record returned: {pretty_key:?}",);

// Returning OK whenever fulfill one of the followings:
// 1, No targeting record
Expand All @@ -339,7 +335,7 @@ impl Network {
} else if verification_attempts >= total_attempts {
info!("Error: Returned record does not match target");
return Err(Error::ReturnedRecordDoesNotMatch(
returned_record.key.into(),
PrettyPrintRecordKey::from(&returned_record.key).into_owned(),
));
}
}
Expand All @@ -354,7 +350,7 @@ impl Network {
return Ok(returned_record);
} else {
return Err(Error::ReturnedRecordDoesNotMatch(
returned_record.key.into(),
PrettyPrintRecordKey::from(&returned_record.key).into_owned(),
));
}
}
Expand All @@ -367,20 +363,14 @@ impl Network {
break;
}

warn!(
"No holder of record '{:?}' found. Retrying the fetch ...",
PrettyPrintRecordKey::from(key.clone()),
);
warn!("No holder of record '{pretty_key:?}' found. Retrying the fetch ...",);
}
Err(error) => {
error!("{error:?}");
if verification_attempts >= total_attempts {
break;
}
warn!(
"Did not retrieve Record '{:?}' from network!. Retrying...",
PrettyPrintRecordKey::from(key.clone()),
);
warn!("Did not retrieve Record '{pretty_key:?}' from network!. Retrying...",);
}
}

Expand Down Expand Up @@ -431,7 +421,7 @@ impl Network {
while retries < PUT_RECORD_RETRIES {
info!(
"Attempting to PUT record of {:?} to network",
PrettyPrintRecordKey::from(record.key.clone())
PrettyPrintRecordKey::from(&record.key)
);

let res = self
Expand All @@ -451,7 +441,9 @@ impl Network {
retries += 1;
}

Err(Error::FailedToVerifyRecordWasStored(record.key.into()))
Err(Error::FailedToVerifyRecordWasStored(
PrettyPrintRecordKey::from(&record.key).into_owned(),
))
}

async fn put_record_once(
Expand All @@ -461,7 +453,7 @@ impl Network {
expected_holders: ExpectedHoldersList,
) -> Result<()> {
let record_key = record.key.clone();
let pretty_key = PrettyPrintRecordKey::from(record_key.clone());
let pretty_key = PrettyPrintRecordKey::from(&record_key);
info!(
"Putting record of {} - length {:?} to network",
pretty_key,
Expand Down Expand Up @@ -504,7 +496,7 @@ impl Network {
pub fn put_local_record(&self, record: Record) -> Result<()> {
trace!(
"Writing Record locally, for {:?} - length {:?}",
PrettyPrintRecordKey::from(record.key.clone()),
PrettyPrintRecordKey::from(&record.key),
record.value.len()
);
self.send_swarm_cmd(SwarmCmd::PutLocalRecord { record })
Expand Down
12 changes: 6 additions & 6 deletions sn_networking/src/record_store.rs
Original file line number Diff line number Diff line change
Expand Up @@ -160,8 +160,8 @@ impl NodeRecordStore {
{
trace!(
"{:?} will be pruned to make space for new record: {:?}",
PrettyPrintRecordKey::from(furthest_record.clone()),
PrettyPrintRecordKey::from(r.clone())
PrettyPrintRecordKey::from(&furthest_record),
PrettyPrintRecordKey::from(r)
);
// we should prune and make space
self.remove(&furthest_record);
Expand Down Expand Up @@ -207,7 +207,7 @@ impl NodeRecordStore {
/// Warning: PUTs a `Record` to the store without validation
/// Should be used in context where the `Record` is trusted
pub(crate) fn put_verified(&mut self, r: Record) -> Result<()> {
let record_key = PrettyPrintRecordKey::from(r.key.clone());
let record_key = PrettyPrintRecordKey::from(&r.key).into_owned();
trace!("PUT a verified Record: {record_key:?}");

self.prune_storage_if_needed_for_record(&r.key)?;
Expand Down Expand Up @@ -301,7 +301,7 @@ impl RecordStore for NodeRecordStore {
// When a client calls GET, the request is forwarded to the nodes until one node returns
// with the record. Thus a node can be bombarded with GET reqs for random keys. These can be safely
// ignored if we don't have the record locally.
let key = PrettyPrintRecordKey::from(k.clone());
let key = PrettyPrintRecordKey::from(k);
if !self.records.contains(k) {
trace!("Record not found locally: {key}");
return None;
Expand All @@ -324,15 +324,15 @@ impl RecordStore for NodeRecordStore {
if self.records.contains(&record.key) {
trace!(
"Unverified Record {:?} already exists.",
PrettyPrintRecordKey::from(record.key.clone())
PrettyPrintRecordKey::from(&record.key)
);

// Blindly sent to validation to allow double spend can be detected.
// TODO: consider avoid throw duplicated chunk to validation.
}
trace!(
"Unverified Record {:?} try to validate and store",
PrettyPrintRecordKey::from(record.key.clone())
PrettyPrintRecordKey::from(&record.key)
);
if let Some(event_sender) = self.event_sender.clone() {
// push the event off thread so as to be non-blocking
Expand Down
4 changes: 2 additions & 2 deletions sn_networking/src/replication_fetcher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ impl ReplicationFetcher {

let pretty_keys: Vec<_> = data_to_fetch
.iter()
.map(|(holder, key)| (*holder, PrettyPrintRecordKey::from(key.clone())))
.map(|(holder, key)| (*holder, PrettyPrintRecordKey::from(key)))
.collect();

if !data_to_fetch.is_empty() {
Expand All @@ -123,7 +123,7 @@ impl ReplicationFetcher {
self.on_going_fetches.retain(|data, node| data != key || node != holder);
debug!(
"Prune record {:?} at {holder:?} from the replication_fetcher due to timeout.",
PrettyPrintRecordKey::from(key.clone())
PrettyPrintRecordKey::from(key)
);
true
} else {
Expand Down
2 changes: 1 addition & 1 deletion sn_networking/src/transfers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ impl Network {
.map_err(|_| Error::SpendNotFound(address))?;
debug!(
"Got record from the network, {:?}",
PrettyPrintRecordKey::from(record.key.clone())
PrettyPrintRecordKey::from(&record.key)
);
let header =
RecordHeader::from_record(&record).map_err(|_| Error::SpendNotFound(address))?;
Expand Down
8 changes: 4 additions & 4 deletions sn_node/src/log_markers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -51,14 +51,14 @@ pub enum Marker<'a> {
},

/// Valid non-existing Chunk record PUT from the network received and stored
ValidChunkRecordPutFromNetwork(&'a PrettyPrintRecordKey),
ValidChunkRecordPutFromNetwork(&'a PrettyPrintRecordKey<'a>),
/// Valid non-existing Register record PUT from the network received and stored
ValidRegisterRecordPutFromNetwork(&'a PrettyPrintRecordKey),
ValidRegisterRecordPutFromNetwork(&'a PrettyPrintRecordKey<'a>),
/// Valid non-existing Spend record PUT from the network received and stored
ValidSpendRecordPutFromNetwork(&'a PrettyPrintRecordKey),
ValidSpendRecordPutFromNetwork(&'a PrettyPrintRecordKey<'a>),

/// Record rejected
RecordRejected(&'a PrettyPrintRecordKey),
RecordRejected(&'a PrettyPrintRecordKey<'a>),
}

impl<'a> Marker<'a> {
Expand Down
Loading

0 comments on commit 1174fbf

Please sign in to comment.