Skip to content

Commit

Permalink
removes CrdsValue.{signature,data} from pub interface
Browse files Browse the repository at this point in the history
  • Loading branch information
@behzadnouri
behzadnouri committed Nov 4, 2024
1 parent b83222f commit 0e0d0ec
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 47 deletions.
32 changes: 16 additions & 16 deletions gossip/src/cluster_info.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -424,7 +424,7 @@ fn retain_staked(
drop_unstaked_node_instance: bool,
) {
values.retain(|value| {
match value.data {
match value.data() {
CrdsData::ContactInfo(_) => true,
CrdsData::LegacyContactInfo(_) => true,
// May Impact new validators starting up without any stake yet.
Expand Down Expand Up @@ -1191,7 +1191,7 @@ impl ClusterInfo {
.time_gossip_read_lock("get_votes", &self.stats.get_votes)
.get_votes(cursor)
.map(|vote| {
let CrdsData::Vote(_, vote) = &vote.value.data else {
let CrdsData::Vote(_, vote) = vote.value.data() else {
panic!("this should not happen!");
};
vote.transaction().clone()
Expand All @@ -1211,7 +1211,7 @@ impl ClusterInfo {
.get_votes(cursor)
.map(|vote| {
let label = vote.value.label();
let CrdsData::Vote(_, vote) = &vote.value.data else {
let CrdsData::Vote(_, vote) = vote.value.data() else {
panic!("this should not happen!");
};
(label, vote.transaction().clone())
Expand Down Expand Up @@ -1257,7 +1257,7 @@ impl ClusterInfo {
let origin = entry.value.pubkey();
gossip_crds.get_shred_version(&origin) == self_shred_version
})
.map(|entry| match &entry.value.data {
.map(|entry| match entry.value.data() {
CrdsData::EpochSlots(_, slots) => slots.clone(),
_ => panic!("this should not happen!"),
})
Expand All @@ -1273,7 +1273,7 @@ impl ClusterInfo {
gossip_crds
.get_entries(cursor)
.filter_map(|entry| {
let CrdsData::RestartLastVotedForkSlots(slots) = &entry.value.data else {
let CrdsData::RestartLastVotedForkSlots(slots) = entry.value.data() else {
return None;
};
(slots.shred_version == self_shred_version).then_some(slots)
Expand All @@ -1288,7 +1288,7 @@ impl ClusterInfo {
gossip_crds
.get_entries(cursor)
.filter_map(|entry| {
let CrdsData::RestartHeaviestFork(fork) = &entry.value.data else {
let CrdsData::RestartHeaviestFork(fork) = entry.value.data() else {
return None;
};
(fork.shred_version == self_shred_version).then_some(fork)
Expand All @@ -1302,7 +1302,7 @@ impl ClusterInfo {
let gossip_crds = self.gossip.crds.read().unwrap();
gossip_crds
.get_duplicate_shreds(cursor)
.map(|entry| match &entry.value.data {
.map(|entry| match entry.value.data() {
CrdsData::DuplicateShred(_, dup) => dup.clone(),
_ => panic!("this should not happen!"),
})
Expand Down Expand Up @@ -2003,7 +2003,7 @@ impl ClusterInfo {
requests
.into_par_iter()
.with_min_len(1024)
.filter(|(_, _, caller)| match &caller.data {
.filter(|(_, _, caller)| match caller.data() {
CrdsData::LegacyContactInfo(_) | CrdsData::ContactInfo(_) => {
if caller.pubkey() == self_pubkey {
warn!("PullRequest ignored, I'm talking to myself");
Expand Down Expand Up @@ -2154,7 +2154,7 @@ impl ClusterInfo {
} else {
score
};
let score = match response.data {
let score = match response.data() {
CrdsData::LegacyContactInfo(_) | CrdsData::ContactInfo(_) => 2 * score,
_ => score,
};
Expand Down Expand Up @@ -2479,7 +2479,7 @@ impl ClusterInfo {
if should_check_duplicate_instance
&& values.iter().any(|value| {
instance.check_duplicate(value)
|| matches!(&value.data, CrdsData::ContactInfo(other)
|| matches!(value.data(), CrdsData::ContactInfo(other)
if my_contact_info.check_duplicate(other))
})
{
Expand Down Expand Up @@ -3297,7 +3297,7 @@ fn filter_on_shred_version(
// * prevent two running instances of the same identity key cross
// contaminate gossip between clusters.
if crds.get_shred_version(from) == Some(self_shred_version) {
values.retain(|value| match &value.data {
values.retain(|value| match value.data() {
// Allow contact-infos so that shred-versions are updated.
CrdsData::ContactInfo(_) => true,
CrdsData::LegacyContactInfo(_) => true,
Expand All @@ -3306,7 +3306,7 @@ fn filter_on_shred_version(
_ => crds.get_shred_version(&value.pubkey()) == Some(self_shred_version),
})
} else {
values.retain(|value| match &value.data {
values.retain(|value| match value.data() {
// Allow node to update its own contact info in case their
// shred-version changes
CrdsData::ContactInfo(node) => node.pubkey() == from,
Expand All @@ -3321,7 +3321,7 @@ fn filter_on_shred_version(
}
};
match &mut msg {
Protocol::PullRequest(_, caller) => match &caller.data {
Protocol::PullRequest(_, caller) => match caller.data() {
// Allow spy nodes with shred-verion == 0 to pull from other nodes.
CrdsData::LegacyContactInfo(node)
if node.shred_version() == 0 || node.shred_version() == self_shred_version =>
Expand Down Expand Up @@ -3373,7 +3373,7 @@ fn verify_gossip_addr<R: Rng + CryptoRng>(
ping_cache: &Mutex<PingCache>,
pings: &mut Vec<(SocketAddr, Protocol /* ::PingMessage */)>,
) -> bool {
let (pubkey, addr) = match &value.data {
let (pubkey, addr) = match value.data() {
CrdsData::ContactInfo(node) => (node.pubkey(), node.gossip()),
CrdsData::LegacyContactInfo(node) => (node.pubkey(), node.gossip()),
_ => return true, // If not a contact-info, nothing to verify.
Expand Down Expand Up @@ -4344,11 +4344,11 @@ mod tests {

let mut i = 0;
while value.size() < PUSH_MESSAGE_MAX_PAYLOAD_SIZE as u64 {
value.data = CrdsData::AccountsHashes(AccountsHashes {
value = CrdsValue::new_unsigned(CrdsData::AccountsHashes(AccountsHashes {
from: Pubkey::default(),
hashes: vec![(0, Hash::default()); i],
wallclock: 0,
});
}));
i += 1;
}
let split: Vec<_> =
Expand Down
46 changes: 23 additions & 23 deletions gossip/src/crds.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,13 +199,13 @@ fn overrides(value: &CrdsValue, other: &VersionedCrdsValue) -> bool {
// Contact-infos and node instances are special cased so that if there are
// two running instances of the same node, the more recent start is
// propagated through gossip regardless of wallclocks.
if let CrdsData::NodeInstance(value) = &value.data {
if let CrdsData::NodeInstance(value) = value.data() {
if let Some(out) = value.overrides(&other.value) {
return out;
}
}
if let CrdsData::ContactInfo(value) = &value.data {
if let CrdsData::ContactInfo(other) = &other.value.data {
if let CrdsData::ContactInfo(value) = value.data() {
if let CrdsData::ContactInfo(other) = other.value.data() {
if let Some(out) = value.overrides(other) {
return out;
}
Expand Down Expand Up @@ -250,7 +250,7 @@ impl Crds {
stats.record_insert(&value, route);
let entry_index = entry.index();
self.shards.insert(entry_index, &value);
match &value.value.data {
match value.value.data() {
CrdsData::ContactInfo(node) => {
self.nodes.insert(entry_index);
self.shred_versions.insert(pubkey, node.shred_version());
Expand All @@ -277,12 +277,12 @@ impl Crds {
let entry_index = entry.index();
self.shards.remove(entry_index, entry.get());
self.shards.insert(entry_index, &value);
match &value.value.data {
match value.value.data() {
CrdsData::ContactInfo(node) => {
self.shred_versions.insert(pubkey, node.shred_version());
// self.nodes does not need to be updated since the
// entry at this index was and stays contact-info.
debug_assert_matches!(entry.get().value.data, CrdsData::ContactInfo(_));
debug_assert_matches!(entry.get().value.data(), CrdsData::ContactInfo(_));
}
CrdsData::Vote(_, _) => {
self.votes.remove(&entry.get().ordinal);
Expand Down Expand Up @@ -355,7 +355,7 @@ impl Crds {

/// Returns ContactInfo of all known nodes.
pub(crate) fn get_nodes_contact_info(&self) -> impl Iterator<Item = &ContactInfo> {
self.get_nodes().map(|v| match &v.value.data {
self.get_nodes().map(|v| match v.value.data() {
CrdsData::ContactInfo(info) => info,
_ => panic!("this should not happen!"),
})
Expand Down Expand Up @@ -556,7 +556,7 @@ impl Crds {
};
self.purged.push_back((value.value_hash, now));
self.shards.remove(index, &value);
match value.value.data {
match value.value.data() {
CrdsData::ContactInfo(_) => {
self.nodes.swap_remove(&index);
}
Expand Down Expand Up @@ -592,7 +592,7 @@ impl Crds {
let value = self.table.index(index);
self.shards.remove(size, value);
self.shards.insert(index, value);
match value.value.data {
match value.value.data() {
CrdsData::ContactInfo(_) => {
self.nodes.swap_remove(&size);
self.nodes.insert(index);
Expand Down Expand Up @@ -695,7 +695,7 @@ impl Default for CrdsDataStats {
impl CrdsDataStats {
fn record_insert(&mut self, entry: &VersionedCrdsValue, route: GossipRoute) {
self.counts[Self::ordinal(entry)] += 1;
if let CrdsData::Vote(_, vote) = &entry.value.data {
if let CrdsData::Vote(_, vote) = entry.value.data() {
if let Some(slot) = vote.slot() {
let num_nodes = self.votes.get(&slot).copied().unwrap_or_default();
self.votes.put(slot, num_nodes + 1);
Expand All @@ -706,7 +706,7 @@ impl CrdsDataStats {
return;
};

if should_report_message_signature(&entry.value.signature) {
if should_report_message_signature(entry.value.signature()) {
datapoint_info!(
"gossip_crds_sample",
(
Expand All @@ -716,7 +716,7 @@ impl CrdsDataStats {
),
(
"signature",
entry.value.signature.to_string().get(..8),
entry.value.signature().to_string().get(..8),
Option<String>
),
(
Expand All @@ -733,7 +733,7 @@ impl CrdsDataStats {
}

fn ordinal(entry: &VersionedCrdsValue) -> usize {
match &entry.value.data {
match entry.value.data() {
CrdsData::LegacyContactInfo(_) => 0,
CrdsData::Vote(_, _) => 1,
CrdsData::LowestSlot(_, _) => 2,
Expand Down Expand Up @@ -1159,7 +1159,7 @@ mod tests {
.table
.values()
.filter(|v| v.ordinal >= since)
.filter(|v| matches!(v.value.data, CrdsData::EpochSlots(_, _)))
.filter(|v| matches!(v.value.data(), CrdsData::EpochSlots(_, _)))
.count();
let mut cursor = Cursor(since);
assert_eq!(num_epoch_slots, crds.get_epoch_slots(&mut cursor).count());
Expand All @@ -1174,21 +1174,21 @@ mod tests {
);
for value in crds.get_epoch_slots(&mut Cursor(since)) {
assert!(value.ordinal >= since);
assert_matches!(value.value.data, CrdsData::EpochSlots(_, _));
assert_matches!(value.value.data(), CrdsData::EpochSlots(_, _));
}
let num_votes = crds
.table
.values()
.filter(|v| v.ordinal >= since)
.filter(|v| matches!(v.value.data, CrdsData::Vote(_, _)))
.filter(|v| matches!(v.value.data(), CrdsData::Vote(_, _)))
.count();
let mut cursor = Cursor(since);
assert_eq!(num_votes, crds.get_votes(&mut cursor).count());
assert_eq!(
cursor.0,
crds.table
.values()
.filter(|v| matches!(v.value.data, CrdsData::Vote(_, _)))
.filter(|v| matches!(v.value.data(), CrdsData::Vote(_, _)))
.map(|v| v.ordinal)
.max()
.map(|k| k + 1)
Expand All @@ -1197,7 +1197,7 @@ mod tests {
);
for value in crds.get_votes(&mut Cursor(since)) {
assert!(value.ordinal >= since);
assert_matches!(value.value.data, CrdsData::Vote(_, _));
assert_matches!(value.value.data(), CrdsData::Vote(_, _));
}
let num_entries = crds
.table
Expand All @@ -1221,17 +1221,17 @@ mod tests {
let num_nodes = crds
.table
.values()
.filter(|v| matches!(v.value.data, CrdsData::ContactInfo(_)))
.filter(|v| matches!(v.value.data(), CrdsData::ContactInfo(_)))
.count();
let num_votes = crds
.table
.values()
.filter(|v| matches!(v.value.data, CrdsData::Vote(_, _)))
.filter(|v| matches!(v.value.data(), CrdsData::Vote(_, _)))
.count();
let num_epoch_slots = crds
.table
.values()
.filter(|v| matches!(v.value.data, CrdsData::EpochSlots(_, _)))
.filter(|v| matches!(v.value.data(), CrdsData::EpochSlots(_, _)))
.count();
assert_eq!(
crds.table.len(),
Expand All @@ -1244,10 +1244,10 @@ mod tests {
crds.get_epoch_slots(&mut Cursor::default()).count()
);
for vote in crds.get_votes(&mut Cursor::default()) {
assert_matches!(vote.value.data, CrdsData::Vote(_, _));
assert_matches!(vote.value.data(), CrdsData::Vote(_, _));
}
for epoch_slots in crds.get_epoch_slots(&mut Cursor::default()) {
assert_matches!(epoch_slots.value.data, CrdsData::EpochSlots(_, _));
assert_matches!(epoch_slots.value.data(), CrdsData::EpochSlots(_, _));
}
(num_nodes, num_votes, num_epoch_slots)
}
Expand Down
8 changes: 4 additions & 4 deletions gossip/src/crds_entry.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ macro_rules! impl_crds_entry (
type Key = Pubkey;
fn get_entry(table:&'a CrdsTable, key: Self::Key) -> Option<Self> {
let key = CrdsValueLabel::$name(key);
match &table.get(&key)?.value.data {
match table.get(&key)?.value.data() {
$pat => Some($expr),
_ => None,
}
Expand All @@ -47,7 +47,7 @@ macro_rules! impl_crds_entry (
);

// Lookup by CrdsValueLabel.
impl_crds_entry!(CrdsData, |entry| Some(&entry?.value.data));
impl_crds_entry!(CrdsData, |entry| Some(entry?.value.data()));
impl_crds_entry!(CrdsValue, |entry| Some(&entry?.value));
impl_crds_entry!(VersionedCrdsValue, |entry| entry);

Expand Down Expand Up @@ -98,10 +98,10 @@ mod tests {
for entry in entries.values() {
let key = entry.label();
assert_eq!(crds.get::<&CrdsValue>(&key), Some(entry));
assert_eq!(crds.get::<&CrdsData>(&key), Some(&entry.data));
assert_eq!(crds.get::<&CrdsData>(&key), Some(entry.data()));
assert_eq!(crds.get::<&VersionedCrdsValue>(&key).unwrap().value, *entry);
let key = entry.pubkey();
match &entry.data {
match entry.data() {
CrdsData::ContactInfo(node) => {
assert_eq!(crds.get::<&ContactInfo>(key), Some(node))
}
Expand Down
4 changes: 2 additions & 2 deletions gossip/src/crds_gossip.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ impl CrdsGossip {
let mut crds = self.crds.write().unwrap();
if crds
.get_records(&pubkey)
.any(|value| match &value.value.data {
.any(|value| match value.value.data() {
CrdsData::DuplicateShred(_, value) => value.slot == shred_slot,
_ => false,
})
Expand All @@ -121,7 +121,7 @@ impl CrdsGossip {
let mut num_dup_shreds = 0;
let offset = crds
.get_records(&pubkey)
.filter_map(|value| match &value.value.data {
.filter_map(|value| match value.value.data() {
CrdsData::DuplicateShred(ix, value) => {
num_dup_shreds += 1;
Some((value.wallclock, *ix))
Expand Down
14 changes: 12 additions & 2 deletions gossip/src/crds_value.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ pub const MAX_EPOCH_SLOTS: EpochSlotsIndex = 255;
#[cfg_attr(feature = "frozen-abi", derive(AbiExample))]
#[derive(Serialize, Deserialize, Clone, Debug, PartialEq, Eq)]
pub struct CrdsValue {
pub signature: Signature,
pub data: CrdsData,
signature: Signature,
data: CrdsData,
}

impl Sanitize for CrdsValue {
Expand Down Expand Up @@ -605,6 +605,16 @@ impl CrdsValue {
}
}

#[inline]
pub(crate) fn signature(&self) -> &Signature {
&self.signature
}

#[inline]
pub(crate) fn data(&self) -> &CrdsData {
&self.data
}

/// Totally unsecure unverifiable wallclock of the node that generated this message
/// Latest wallclock is always picked.
/// This is used to time out push messages.
Expand Down

0 comments on commit 0e0d0ec

Please sign in to comment.