Skip to content
This repository has been archived by the owner on Nov 6, 2020. It is now read-only.

[devp2p discovery]: remove deprecated_echo_hash #11564

Merged
merged 1 commit into from
Mar 17, 2020
Merged

[devp2p discovery]: remove deprecated_echo_hash #11564

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
56 changes: 16 additions & 40 deletions util/network-devp2p/src/discovery.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,9 +131,6 @@ struct PingRequest {
node: NodeEntry,
// The hash sent in the Ping request
echo_hash: H256,
// The hash Parity used to respond with (until rev 01f825b0e1f1c4c420197b51fc801cbe89284b29)
#[deprecated()]
deprecated_echo_hash: H256,
reason: PingReason
}

Expand Down Expand Up @@ -255,23 +252,17 @@ impl Discovery {
}.and_then(|(node_entry, bucket_distance)| {
trace!(target: "discovery", "Adding a new node {:?} into our bucket {}", &node_entry, bucket_distance);

let mut added = HashMap::with_capacity(1);
added.insert(node_entry.id, node_entry.clone());

let node_to_ping = {
let bucket = &mut self.node_buckets[bucket_distance];
bucket.nodes.push_front(BucketEntry::new(node_entry.clone()));
if bucket.nodes.len() > BUCKET_SIZE {
select_bucket_ping(bucket.nodes.iter())
} else {
None
let bucket = &mut self.node_buckets[bucket_distance];
bucket.nodes.push_front(BucketEntry::new(node_entry.clone()));
if bucket.nodes.len() > BUCKET_SIZE {
if let Some(node) = select_bucket_ping(bucket.nodes.iter()) {
self.try_ping(node, PingReason::Default);
}
};
if let Some(node) = node_to_ping {
self.try_ping(node, PingReason::Default);
};
}

if node_entry.endpoint.is_valid_sync_node() {
let mut added = HashMap::with_capacity(1);
Copy link
Collaborator Author

@niklasad1 niklasad1 Mar 11, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

move this here, to not have to clone it.

added.insert(node_entry.id, node_entry);
Some(TableUpdates { added, removed: HashSet::new() })
} else {
None
Expand Down Expand Up @@ -381,14 +372,12 @@ impl Discovery {
self.public_endpoint.to_rlp_list(&mut rlp);
node.endpoint.to_rlp_list(&mut rlp);
append_expiration(&mut rlp);
let old_parity_hash = keccak(rlp.as_raw());
let hash = self.send_packet(PACKET_PING, node.endpoint.udp_address(), rlp.drain())?;

self.in_flight_pings.insert(node.id, PingRequest {
sent_at: Instant::now(),
node: node.clone(),
echo_hash: hash,
deprecated_echo_hash: old_parity_hash,
reason,
});

Expand Down Expand Up @@ -569,28 +558,15 @@ impl Discovery {
self.check_timestamp(timestamp)?;

let expected_node = match self.in_flight_pings.entry(node_id) {
Entry::Occupied(entry) => {
let expected_node = {
let request = entry.get();
if request.echo_hash != echo_hash && request.deprecated_echo_hash != echo_hash {
debug!(target: "discovery", "Got unexpected Pong from {:?} ; packet_hash={:#x} ; expected_hash={:#x}", &from, request.echo_hash, echo_hash);
None
} else {
if request.deprecated_echo_hash == echo_hash {
trace!(target: "discovery", "Got Pong from an old open-ethereum version.");
}
Some((request.node.clone(), request.reason))
}
};

if expected_node.is_some() {
entry.remove();
}
expected_node
},
Entry::Vacant(_) => {
Entry::Occupied(entry) if entry.get().echo_hash != echo_hash => {
debug!(target: "discovery", "Got unexpected Pong from {:?} ; packet_hash={:#x} ; expected_hash={:#x}", &from, entry.get().echo_hash, echo_hash);
None
},
Entry::Occupied(entry) => {
let request = entry.remove();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

entry::remove() returns it by value, thus clone removed.

Some((request.node, request.reason))
},
Entry::Vacant(_) => None,
};

if let Some((node, ping_reason)) = expected_node {
Expand All @@ -606,7 +582,7 @@ impl Discovery {
debug!(target: "discovery", "Error occured when processing ping from a bucket node: {:?}", &error);
});
},
NodeValidity::UnknownNode | NodeValidity::ExpiredNode(NodeCategory::Observed) | NodeValidity::ValidNode(NodeCategory::Observed)=> {
NodeValidity::UnknownNode | NodeValidity::ExpiredNode(NodeCategory::Observed) | NodeValidity::ValidNode(NodeCategory::Observed) => {
trace!(target: "discovery", "Updating node {:?} in the list of other_observed_nodes", &node);
self.other_observed_nodes.insert(node.id, (node.endpoint, Instant::now()));
},
Expand Down