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

Spend and range atop main #2033

Closed
wants to merge 38 commits into from
Closed

Conversation

joshuef
Copy link
Contributor

@joshuef joshuef commented Aug 9, 2024

#1989 and #2011 atop latest main codebase.

maqi and others added 30 commits August 9, 2024 14:26
BREAKING CHANGE: many types gone, replaced by UnsignedTransaction SignedTransaction
let mapped: Vec<_> = sorted_distances.iter().map(|d| d.ilog2()).collect();
info!("Sorted distances: {:?}", mapped);

// TODO: Test this calculation in larger networks

Check notice

Code scanning / devskim

A "TODO" or similar was left in source code, possibly indicating incomplete functionality Note

Suspicious comment
sn_networking/src/event/kad.rs Fixed Show fixed Hide fixed
sn_networking/src/event/kad.rs Fixed Show fixed Hide fixed
.await
{
Ok(record) => record,
// TODO: do we need to handle SplitRecord anywhere else?

Check notice

Code scanning / devskim

A "TODO" or similar was left in source code, possibly indicating incomplete functionality Note

Suspicious comment

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

// TODO: does this need merged with any local copy?

Check notice

Code scanning / devskim

A "TODO" or similar was left in source code, possibly indicating incomplete functionality Note

Suspicious comment
// info!("{id} verifying status of spend number({num:?}): {spend:?} : {status:?}");
// match status {
// SpendStatus::Utxo => {
// // TODO: with the new spend struct requiring `middle payment`

Check notice

Code scanning / devskim

A "TODO" or similar was left in source code, possibly indicating incomplete functionality Note test

Suspicious comment
@joshuef joshuef force-pushed the SpendAndRangeAtopMain branch from bc0afa2 to 0962caf Compare August 9, 2024 05:47
@@ -389,6 +395,7 @@
.set_max_packet_size(MAX_PACKET_SIZE)
// Require iterative queries to use disjoint paths for increased resiliency in the presence of potentially adversarial nodes.
.disjoint_query_paths(true)
// TODO: Do we want to reduce this from default still?

Check notice

Code scanning / devskim

A "TODO" or similar was left in source code, possibly indicating incomplete functionality Note

Suspicious comment
info!("RANGE: {pretty_key:?} we_have_searched_far_enough: {we_have_searched_thoroughly:?}");

let result = if num_of_versions > 1 {
// TODO: Do we want to repopulate a split record.and under what conditions?

Check notice

Code scanning / devskim

A "TODO" or similar was left in source code, possibly indicating incomplete functionality Note

Suspicious comment
@@ -476,25 +503,34 @@
let mut replication_fetcher = ReplicationFetcher::new(peer_id, event_sender);

// Set distance range
// TODO: close peers can break the distance range check here... we need a proper

Check notice

Code scanning / devskim

A "TODO" or similar was left in source code, possibly indicating incomplete functionality Note

Suspicious comment
@joshuef
Copy link
Contributor Author

joshuef commented Sep 3, 2024

closing to replace with just the getrange work atop this

@joshuef joshuef closed this Sep 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants