Skip to content

Commit

Permalink
Reverts "Revert "checks for duplicate instances using the new Contact…
Browse files Browse the repository at this point in the history
…Info (#2506)"" (#2532)

* Reapply "checks for duplicate instances using the new ContactInfo (#2506)" (#2521)

This reverts commit 15c5dcb.

* removes unwrap
  • Loading branch information
behzadnouri authored Aug 9, 2024
1 parent f1a2279 commit 497941f
Show file tree
Hide file tree
Showing 2 changed files with 76 additions and 8 deletions.
21 changes: 13 additions & 8 deletions gossip/src/cluster_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2475,16 +2475,21 @@ impl ClusterInfo {

// Check if there is a duplicate instance of
// this node with more recent timestamp.
let instance = self.instance.read().unwrap();
let check_duplicate_instance = |values: &[CrdsValue]| {
if should_check_duplicate_instance {
for value in values {
if instance.check_duplicate(value) {
return Err(GossipError::DuplicateNodeInstance);
}
let check_duplicate_instance = {
let instance = self.instance.read().unwrap();
let my_contact_info = self.my_contact_info();
move |values: &[CrdsValue]| {
if should_check_duplicate_instance
&& values.iter().any(|value| {
instance.check_duplicate(value)
|| matches!(&value.data, CrdsData::ContactInfo(other)
if my_contact_info.check_duplicate(other))
})
{
return Err(GossipError::DuplicateNodeInstance);
}
Ok(())
}
Ok(())
};
let mut pings = Vec::new();
let mut rng = rand::thread_rng();
Expand Down
63 changes: 63 additions & 0 deletions gossip/src/contact_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -435,6 +435,14 @@ impl ContactInfo {
node.set_serve_repair_quic((addr, port + 4)).unwrap();
node
}

// Returns true if the other contact-info is a duplicate instance of this
// node, with a more recent `outset` timestamp.
#[inline]
#[must_use]
pub(crate) fn check_duplicate(&self, other: &ContactInfo) -> bool {
self.pubkey == other.pubkey && self.outset < other.outset
}
}

impl Default for ContactInfo {
Expand Down Expand Up @@ -1016,4 +1024,59 @@ mod tests {
Err(Error::InvalidPort(0))
);
}

#[test]
fn test_check_duplicate() {
let mut rng = rand::thread_rng();
let mut node = ContactInfo::new(
Keypair::new().pubkey(),
rng.gen(), // wallclock
rng.gen(), // shred_version
);
// Same contact-info is not a duplicate instance.
{
let other = node.clone();
assert!(!node.check_duplicate(&other));
assert!(!other.check_duplicate(&node));
}
// Updated socket address is not a duplicate instance.
{
let mut other = node.clone();
while other.set_gossip(new_rand_socket(&mut rng)).is_err() {}
while other.set_serve_repair(new_rand_socket(&mut rng)).is_err() {}
assert!(!node.check_duplicate(&other));
assert!(!other.check_duplicate(&node));
other.remove_serve_repair();
assert!(!node.check_duplicate(&other));
assert!(!other.check_duplicate(&node));
}
// Updated wallclock is not a duplicate instance.
{
let other = node.clone();
node.set_wallclock(rng.gen());
assert!(!node.check_duplicate(&other));
assert!(!other.check_duplicate(&node));
}
// Different pubkey is not a duplicate instance.
{
let other = ContactInfo::new(
Keypair::new().pubkey(),
rng.gen(), // wallclock
rng.gen(), // shred_version
);
assert!(!node.check_duplicate(&other));
assert!(!other.check_duplicate(&node));
}
// Same pubkey, more recent outset timestamp is a duplicate instance.
{
let other = ContactInfo::new(
node.pubkey,
rng.gen(), // wallclock
rng.gen(), // shred_version
);
assert!(node.outset < other.outset);
assert!(node.check_duplicate(&other));
assert!(!other.check_duplicate(&node));
}
}
}

0 comments on commit 497941f

Please sign in to comment.