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

Reverts "Revert "checks for duplicate instances using the new ContactInfo (#2506)"" #2532

Merged
merged 2 commits into from
Aug 9, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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));
}
}
}