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.

Sign up for GitHub

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

removes CrdsValue.{signature,data} from public interface #3450

Merged
merged 1 commit into from
Nov 4, 2024
Merged

removes CrdsValue.{signature,data} from public interface #3450

Show file tree
Hide file tree
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
32 changes: 16 additions & 16 deletions gossip/src/cluster_info.rs
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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,
gregcusack marked this conversation as resolved.
Show resolved Hide resolved
}

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
Loading