Skip to content

Commit

Permalink
renames CrdsValue::new_signed to CrdsValue::new (#3531)
Browse files Browse the repository at this point in the history
Except for #[cfg(test)], all CrdsValues should be signed.

The commit renames CrdsValue::new_signed to CrdsValue::new, 
and adds #[cfg(test)] to CrdsValue::new_unsigned and removes
it from public interface.
  • Loading branch information
behzadnouri authored Nov 7, 2024
1 parent 2916218 commit f621667
Show file tree
Hide file tree
Showing 11 changed files with 50 additions and 51 deletions.
24 changes: 12 additions & 12 deletions gossip/src/cluster_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -292,7 +292,7 @@ impl ClusterInfo {
CrdsData::ContactInfo(node),
]
.into_iter()
.map(|entry| CrdsValue::new_signed(entry, &self.keypair()))
.map(|entry| CrdsValue::new(entry, &self.keypair()))
.collect();
let mut gossip_crds = self.gossip.crds.write().unwrap();
for entry in entries {
Expand Down Expand Up @@ -439,7 +439,7 @@ impl ClusterInfo {
self.my_contact_info.write().unwrap().hot_swap_pubkey(id);

self.refresh_my_gossip_contact_info();
self.push_message(CrdsValue::new_signed(
self.push_message(CrdsValue::new(
CrdsData::Version(Version::new(self.id())),
&self.keypair(),
));
Expand Down Expand Up @@ -653,7 +653,7 @@ impl ClusterInfo {
};
if min > last {
let now = timestamp();
let entry = CrdsValue::new_signed(
let entry = CrdsValue::new(
CrdsData::LowestSlot(0, LowestSlot::new(self_pubkey, min, now)),
&self.keypair(),
);
Expand Down Expand Up @@ -715,7 +715,7 @@ impl ClusterInfo {
update = &update[n..];
if n > 0 {
let epoch_slots = CrdsData::EpochSlots(ix, slots);
let entry = CrdsValue::new_signed(epoch_slots, &keypair);
let entry = CrdsValue::new(epoch_slots, &keypair);
entries.push(entry);
}
epoch_slot_index += 1;
Expand Down Expand Up @@ -743,7 +743,7 @@ impl ClusterInfo {
last_vote_bankhash,
self.my_shred_version(),
)?;
self.push_message(CrdsValue::new_signed(
self.push_message(CrdsValue::new(
CrdsData::RestartLastVotedForkSlots(last_voted_fork_slots),
&self.keypair(),
));
Expand All @@ -764,7 +764,7 @@ impl ClusterInfo {
observed_stake,
shred_version: self.my_shred_version(),
};
self.push_message(CrdsValue::new_signed(
self.push_message(CrdsValue::new(
CrdsData::RestartHeaviestFork(restart_heaviest_fork),
&self.keypair(),
));
Expand Down Expand Up @@ -800,7 +800,7 @@ impl ClusterInfo {
incremental,
wallclock: timestamp(),
});
self.push_message(CrdsValue::new_signed(message, &self.keypair()));
self.push_message(CrdsValue::new(message, &self.keypair()));

Ok(())
}
Expand All @@ -811,7 +811,7 @@ impl ClusterInfo {
let now = timestamp();
let vote = Vote::new(self_pubkey, vote, now).unwrap();
let vote = CrdsData::Vote(vote_index, vote);
let vote = CrdsValue::new_signed(vote, &self.keypair());
let vote = CrdsValue::new(vote, &self.keypair());
let mut gossip_crds = self.gossip.crds.write().unwrap();
if let Err(err) = gossip_crds.insert(vote, now, GossipRoute::LocalMessage) {
error!("push_vote failed: {:?}", err);
Expand Down Expand Up @@ -1214,7 +1214,7 @@ impl ClusterInfo {
CrdsData::NodeInstance(instance),
]
.into_iter()
.map(|entry| CrdsValue::new_signed(entry, &keypair))
.map(|entry| CrdsValue::new(entry, &keypair))
.collect();
let mut gossip_crds = self.gossip.crds.write().unwrap();
for entry in entries {
Expand Down Expand Up @@ -1284,7 +1284,7 @@ impl ClusterInfo {
Vec<(SocketAddr, Protocol)>, // Pull requests
) {
let now = timestamp();
let self_info = CrdsValue::new_signed(
let self_info = CrdsValue::new(
CrdsData::ContactInfo(self.my_contact_info()),
&self.keypair(),
);
Expand Down Expand Up @@ -1568,7 +1568,7 @@ impl ClusterInfo {
),
];
for value in crds_data {
let value = CrdsValue::new_signed(value, &self.keypair());
let value = CrdsValue::new(value, &self.keypair());
self.push_message(value);
}
let mut generate_pull_requests = true;
Expand Down Expand Up @@ -4113,7 +4113,7 @@ mod tests {
let mut rand_ci = ContactInfo::new_rand(&mut rng, Some(keypair.pubkey()));
rand_ci.set_shred_version(shred_version);
rand_ci.set_wallclock(timestamp());
CrdsValue::new_signed(CrdsData::ContactInfo(rand_ci), &keypair)
CrdsValue::new(CrdsData::ContactInfo(rand_ci), &keypair)
})
.take(NO_ENTRIES)
.collect();
Expand Down
4 changes: 2 additions & 2 deletions gossip/src/crds_data.rs
Original file line number Diff line number Diff line change
Expand Up @@ -540,7 +540,7 @@ mod test {
let mut rng = rand::thread_rng();
let keypair = Keypair::new();
let vote = Vote::new(keypair.pubkey(), new_test_vote_tx(&mut rng), timestamp()).unwrap();
let vote = CrdsValue::new_signed(CrdsData::Vote(MAX_VOTES, vote), &keypair);
let vote = CrdsValue::new(CrdsData::Vote(MAX_VOTES, vote), &keypair);
assert!(vote.sanitize().is_err());
}

Expand Down Expand Up @@ -580,7 +580,7 @@ mod test {
#[test]
fn test_max_epoch_slots_index() {
let keypair = Keypair::new();
let item = CrdsValue::new_signed(
let item = CrdsValue::new(
CrdsData::EpochSlots(
MAX_EPOCH_SLOTS,
EpochSlots::new(keypair.pubkey(), timestamp()),
Expand Down
2 changes: 1 addition & 1 deletion gossip/src/crds_gossip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ impl CrdsGossip {
let entries = chunks.enumerate().map(|(k, chunk)| {
let index = (offset + k as DuplicateShredIndex) % MAX_DUPLICATE_SHREDS;
let data = CrdsData::DuplicateShred(index, chunk);
CrdsValue::new_signed(data, keypair)
CrdsValue::new(data, keypair)
});
let now = timestamp();
for entry in entries {
Expand Down
8 changes: 4 additions & 4 deletions gossip/src/crds_gossip_pull.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1483,13 +1483,13 @@ pub(crate) mod tests {
node
};
{
let caller = CrdsValue::new_signed(CrdsData::ContactInfo(node.clone()), &keypair);
let caller = CrdsValue::new(CrdsData::ContactInfo(node.clone()), &keypair);
assert_eq!(get_max_bloom_filter_bytes(&caller), 1175);
verify_get_max_bloom_filter_bytes(&mut rng, &caller, num_items);
}
let node = LegacyContactInfo::try_from(&node).unwrap();
{
let caller = CrdsValue::new_signed(CrdsData::LegacyContactInfo(node), &keypair);
let caller = CrdsValue::new(CrdsData::LegacyContactInfo(node), &keypair);
assert_eq!(get_max_bloom_filter_bytes(&caller), 1136);
verify_get_max_bloom_filter_bytes(&mut rng, &caller, num_items);
}
Expand All @@ -1501,13 +1501,13 @@ pub(crate) mod tests {
node
};
{
let caller = CrdsValue::new_signed(CrdsData::ContactInfo(node.clone()), &keypair);
let caller = CrdsValue::new(CrdsData::ContactInfo(node.clone()), &keypair);
assert_eq!(get_max_bloom_filter_bytes(&caller), 1165);
verify_get_max_bloom_filter_bytes(&mut rng, &caller, num_items);
}
let node = LegacyContactInfo::try_from(&node).unwrap();
{
let caller = CrdsValue::new_signed(CrdsData::LegacyContactInfo(node), &keypair);
let caller = CrdsValue::new(CrdsData::LegacyContactInfo(node), &keypair);
assert_eq!(get_max_bloom_filter_bytes(&caller), 992);
verify_get_max_bloom_filter_bytes(&mut rng, &caller, num_items);
}
Expand Down
19 changes: 10 additions & 9 deletions gossip/src/crds_value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,30 +95,31 @@ impl CrdsValueLabel {
}

impl CrdsValue {
pub fn new_unsigned(data: CrdsData) -> Self {
pub fn new(data: CrdsData, keypair: &Keypair) -> Self {
let bincode_serialized_data = bincode::serialize(&data).unwrap();
let signature = keypair.sign_message(&bincode_serialized_data);
Self { signature, data }
}

#[cfg(test)]
pub(crate) fn new_unsigned(data: CrdsData) -> Self {
Self {
signature: Signature::default(),
data,
}
}

pub fn new_signed(data: CrdsData, keypair: &Keypair) -> Self {
let mut value = Self::new_unsigned(data);
value.sign(keypair);
value
}

/// New random CrdsValue for tests and benchmarks.
pub fn new_rand<R: Rng>(rng: &mut R, keypair: Option<&Keypair>) -> CrdsValue {
match keypair {
None => {
let keypair = Keypair::new();
let data = CrdsData::new_rand(rng, Some(keypair.pubkey()));
Self::new_signed(data, &keypair)
Self::new(data, &keypair)
}
Some(keypair) => {
let data = CrdsData::new_rand(rng, Some(keypair.pubkey()));
Self::new_signed(data, keypair)
Self::new(data, keypair)
}
}
}
Expand Down
14 changes: 6 additions & 8 deletions gossip/src/protocol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,7 @@ pub(crate) mod tests {
for _ in 0..256 {
let accounts_hash = AccountsHashes::new_rand(&mut rng, None);
let crds_value =
CrdsValue::new_signed(CrdsData::AccountsHashes(accounts_hash), &Keypair::new());
CrdsValue::new(CrdsData::AccountsHashes(accounts_hash), &Keypair::new());
let message = Protocol::PushMessage(Pubkey::new_unique(), vec![crds_value]);
let socket = new_rand_socket_addr(&mut rng);
assert!(Packet::from_data(Some(&socket), message).is_ok());
Expand All @@ -397,7 +397,7 @@ pub(crate) mod tests {
for _ in 0..256 {
let accounts_hash = AccountsHashes::new_rand(&mut rng, None);
let crds_value =
CrdsValue::new_signed(CrdsData::AccountsHashes(accounts_hash), &Keypair::new());
CrdsValue::new(CrdsData::AccountsHashes(accounts_hash), &Keypair::new());
let response = Protocol::PullResponse(Pubkey::new_unique(), vec![crds_value]);
let socket = new_rand_socket_addr(&mut rng);
assert!(Packet::from_data(Some(&socket), response).is_ok());
Expand All @@ -413,8 +413,7 @@ pub(crate) mod tests {
incremental: vec![(Slot::default(), Hash::default()); MAX_INCREMENTAL_SNAPSHOT_HASHES],
wallclock: timestamp(),
};
let crds_value =
CrdsValue::new_signed(CrdsData::SnapshotHashes(snapshot_hashes), &Keypair::new());
let crds_value = CrdsValue::new(CrdsData::SnapshotHashes(snapshot_hashes), &Keypair::new());
let message = Protocol::PushMessage(Pubkey::new_unique(), vec![crds_value]);
let socket = new_rand_socket_addr(&mut rng);
assert!(Packet::from_data(Some(&socket), message).is_ok());
Expand All @@ -429,8 +428,7 @@ pub(crate) mod tests {
incremental: vec![(Slot::default(), Hash::default()); MAX_INCREMENTAL_SNAPSHOT_HASHES],
wallclock: timestamp(),
};
let crds_value =
CrdsValue::new_signed(CrdsData::SnapshotHashes(snapshot_hashes), &Keypair::new());
let crds_value = CrdsValue::new(CrdsData::SnapshotHashes(snapshot_hashes), &Keypair::new());
let response = Protocol::PullResponse(Pubkey::new_unique(), vec![crds_value]);
let socket = new_rand_socket_addr(&mut rng);
assert!(Packet::from_data(Some(&socket), response).is_ok());
Expand Down Expand Up @@ -499,7 +497,7 @@ pub(crate) mod tests {
assert!(chunks.len() > 1);
for chunk in chunks {
let data = CrdsData::DuplicateShred(MAX_DUPLICATE_SHREDS - 1, chunk);
let value = CrdsValue::new_signed(data, &keypair);
let value = CrdsValue::new(data, &keypair);
let pull_response = Protocol::PullResponse(keypair.pubkey(), vec![value.clone()]);
assert!(bincode::serialized_size(&pull_response).unwrap() < PACKET_DATA_SIZE as u64);
let push_message = Protocol::PushMessage(keypair.pubkey(), vec![value.clone()]);
Expand Down Expand Up @@ -663,7 +661,7 @@ pub(crate) mod tests {
0, // wallclock
)
.unwrap();
let vote = CrdsValue::new_signed(CrdsData::Vote(1, vote), &Keypair::new());
let vote = CrdsValue::new(CrdsData::Vote(1, vote), &Keypair::new());
assert!(bincode::serialized_size(&vote).unwrap() <= PUSH_MESSAGE_MAX_PAYLOAD_SIZE as u64);
}

Expand Down
3 changes: 1 addition & 2 deletions gossip/src/restart_crds_values.rs
Original file line number Diff line number Diff line change
Expand Up @@ -284,8 +284,7 @@ mod test {
shred_version,
)
.unwrap();
let value =
CrdsValue::new_signed(CrdsData::RestartLastVotedForkSlots(slots.clone()), &keypair);
let value = CrdsValue::new(CrdsData::RestartLastVotedForkSlots(slots.clone()), &keypair);
assert_eq!(value.sanitize(), Ok(()));
let label = value.label();
assert_eq!(
Expand Down
16 changes: 8 additions & 8 deletions gossip/tests/crds_gossip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ fn star_network_create(num: usize) -> Network {
let gossip_port_offset = 9000;
let node_keypair = Arc::new(Keypair::new());
let contact_info = ContactInfo::new_localhost(&node_keypair.pubkey(), 0);
let entry = CrdsValue::new_unsigned(CrdsData::ContactInfo(contact_info.clone()));
let entry = CrdsValue::new(CrdsData::ContactInfo(contact_info.clone()), &node_keypair);
let mut network: HashMap<_, _> = (1..num)
.map(|k| {
let node_keypair = Arc::new(Keypair::new());
Expand All @@ -111,7 +111,7 @@ fn star_network_create(num: usize) -> Network {
contact_info
.set_gossip((Ipv4Addr::LOCALHOST, gossip_port))
.unwrap();
let new = CrdsValue::new_unsigned(CrdsData::ContactInfo(contact_info.clone()));
let new = CrdsValue::new(CrdsData::ContactInfo(contact_info.clone()), &node_keypair);
let node = CrdsGossip::default();
{
let mut node_crds = node.crds.write().unwrap();
Expand Down Expand Up @@ -141,7 +141,7 @@ fn star_network_create(num: usize) -> Network {
fn rstar_network_create(num: usize) -> Network {
let node_keypair = Arc::new(Keypair::new());
let contact_info = ContactInfo::new_localhost(&node_keypair.pubkey(), 0);
let entry = CrdsValue::new_unsigned(CrdsData::ContactInfo(contact_info.clone()));
let entry = CrdsValue::new(CrdsData::ContactInfo(contact_info.clone()), &node_keypair);
let origin = CrdsGossip::default();
let id = entry.label().pubkey();
origin
Expand All @@ -154,7 +154,7 @@ fn rstar_network_create(num: usize) -> Network {
.map(|_| {
let node_keypair = Arc::new(Keypair::new());
let contact_info = ContactInfo::new_localhost(&node_keypair.pubkey(), 0);
let new = CrdsValue::new_unsigned(CrdsData::ContactInfo(contact_info.clone()));
let new = CrdsValue::new(CrdsData::ContactInfo(contact_info.clone()), &node_keypair);
let node = CrdsGossip::default();
node.crds
.write()
Expand All @@ -181,7 +181,7 @@ fn ring_network_create(num: usize) -> Network {
.map(|_| {
let node_keypair = Arc::new(Keypair::new());
let contact_info = ContactInfo::new_localhost(&node_keypair.pubkey(), 0);
let new = CrdsValue::new_unsigned(CrdsData::ContactInfo(contact_info.clone()));
let new = CrdsValue::new(CrdsData::ContactInfo(contact_info.clone()), &node_keypair);
let node = CrdsGossip::default();
node.crds
.write()
Expand Down Expand Up @@ -216,7 +216,7 @@ fn connected_staked_network_create(stakes: &[u64]) -> Network {
.map(|n| {
let node_keypair = Arc::new(Keypair::new());
let contact_info = ContactInfo::new_localhost(&node_keypair.pubkey(), 0);
let new = CrdsValue::new_unsigned(CrdsData::ContactInfo(contact_info.clone()));
let new = CrdsValue::new(CrdsData::ContactInfo(contact_info.clone()), &node_keypair);
let node = CrdsGossip::default();
node.crds
.write()
Expand Down Expand Up @@ -318,7 +318,7 @@ fn network_simulator(thread_pool: &ThreadPool, network: &mut Network, max_conver
node.gossip.process_push_message(
vec![(
Pubkey::default(),
vec![CrdsValue::new_unsigned(CrdsData::ContactInfo(m))],
vec![CrdsValue::new(CrdsData::ContactInfo(m), &Keypair::new())],
)],
now,
);
Expand Down Expand Up @@ -761,7 +761,7 @@ fn test_prune_errors() {
.write()
.unwrap()
.insert(
CrdsValue::new_unsigned(CrdsData::ContactInfo(ci.clone())),
CrdsValue::new(CrdsData::ContactInfo(ci.clone()), &Keypair::new()),
0,
GossipRoute::LocalMessage,
)
Expand Down
2 changes: 1 addition & 1 deletion local-cluster/src/cluster_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -681,7 +681,7 @@ pub fn submit_vote_to_cluster_gossip(
);

cluster_info::push_messages_to_peer(
vec![CrdsValue::new_signed(
vec![CrdsValue::new(
CrdsData::Vote(
0,
crds_data::Vote::new(node_keypair.pubkey(), vote_tx, timestamp()).unwrap(),
Expand Down
3 changes: 2 additions & 1 deletion turbine/src/cluster_nodes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -554,11 +554,12 @@ pub fn make_test_cluster<R: Rng>(
let cluster_info = ClusterInfo::new(this_node, keypair, SocketAddrSpace::Unspecified);
{
let now = timestamp();
let keypair = Keypair::new();
let mut gossip_crds = cluster_info.gossip.crds.write().unwrap();
// First node is pushed to crds table by ClusterInfo constructor.
for node in nodes.iter().skip(1) {
let node = CrdsData::ContactInfo(node.clone());
let node = CrdsValue::new_unsigned(node);
let node = CrdsValue::new(node, &keypair);
assert_eq!(
gossip_crds.insert(node, now, GossipRoute::LocalMessage),
Ok(())
Expand Down
6 changes: 3 additions & 3 deletions wen-restart/src/wen_restart.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1466,8 +1466,8 @@ mod tests {
)
.unwrap();
let entries = vec![
CrdsValue::new_signed(CrdsData::ContactInfo(node.clone()), node_keypair),
CrdsValue::new_signed(CrdsData::RestartLastVotedForkSlots(slots), node_keypair),
CrdsValue::new(CrdsData::ContactInfo(node.clone()), node_keypair),
CrdsValue::new(CrdsData::RestartLastVotedForkSlots(slots), node_keypair),
];
{
let mut gossip_crds = cluster_info.gossip.crds.write().unwrap();
Expand Down Expand Up @@ -1502,7 +1502,7 @@ mod tests {
.write()
.unwrap()
.insert(
CrdsValue::new_signed(CrdsData::RestartHeaviestFork(heaviest_fork), node_keypair),
CrdsValue::new(CrdsData::RestartHeaviestFork(heaviest_fork), node_keypair),
/*now=*/ 0,
GossipRoute::LocalMessage
)
Expand Down

0 comments on commit f621667

Please sign in to comment.