removes id from push_lowest_slot args (#18645)

push_lowest_slot cannot sign the new crds-value unless the id (pubkey)
argument passed-in is the same pubkey as in ClusterInfo::keypair(), in
which case the id argument is redundant:
https://github.com/solana-labs/solana/blob/bb41cf346/gossip/src/cluster_info.rs#L824-L845

Additionally, the lookup is done with self.id(), but insert is done with
the id argument, which is logically a bug.
This commit is contained in:
behzad nouri 2021-07-13 22:32:59 +00:00 committed by GitHub
parent 350baece21
commit c90af3cd63
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 26 additions and 25 deletions

View File

@ -4,7 +4,7 @@ use solana_gossip::cluster_info::ClusterInfo;
use solana_ledger::blockstore::Blockstore; use solana_ledger::blockstore::Blockstore;
use solana_measure::measure::Measure; use solana_measure::measure::Measure;
use solana_runtime::bank_forks::BankForks; use solana_runtime::bank_forks::BankForks;
use solana_sdk::{clock::Slot, pubkey::Pubkey}; use solana_sdk::clock::Slot;
use std::{ use std::{
sync::{ sync::{
atomic::{AtomicBool, Ordering}, atomic::{AtomicBool, Ordering},
@ -43,8 +43,7 @@ impl ClusterSlotsService {
cluster_slots_update_receiver: ClusterSlotsUpdateReceiver, cluster_slots_update_receiver: ClusterSlotsUpdateReceiver,
exit: Arc<AtomicBool>, exit: Arc<AtomicBool>,
) -> Self { ) -> Self {
let id = cluster_info.id(); Self::initialize_lowest_slot(&blockstore, &cluster_info);
Self::initialize_lowest_slot(id, &blockstore, &cluster_info);
Self::initialize_epoch_slots(&bank_forks, &cluster_info); Self::initialize_epoch_slots(&bank_forks, &cluster_info);
let t_cluster_slots_service = Builder::new() let t_cluster_slots_service = Builder::new()
.name("solana-cluster-slots-service".to_string()) .name("solana-cluster-slots-service".to_string())
@ -93,10 +92,9 @@ impl ClusterSlotsService {
} }
}; };
let new_root = bank_forks.read().unwrap().root(); let new_root = bank_forks.read().unwrap().root();
let id = cluster_info.id();
let mut lowest_slot_elapsed = Measure::start("lowest_slot_elapsed"); let mut lowest_slot_elapsed = Measure::start("lowest_slot_elapsed");
let lowest_slot = blockstore.lowest_slot(); let lowest_slot = blockstore.lowest_slot();
Self::update_lowest_slot(&id, lowest_slot, &cluster_info); Self::update_lowest_slot(lowest_slot, &cluster_info);
lowest_slot_elapsed.stop(); lowest_slot_elapsed.stop();
let mut process_cluster_slots_updates_elapsed = let mut process_cluster_slots_updates_elapsed =
Measure::start("process_cluster_slots_updates_elapsed"); Measure::start("process_cluster_slots_updates_elapsed");
@ -151,16 +149,16 @@ impl ClusterSlotsService {
} }
} }
fn initialize_lowest_slot(id: Pubkey, blockstore: &Blockstore, cluster_info: &ClusterInfo) { fn initialize_lowest_slot(blockstore: &Blockstore, cluster_info: &ClusterInfo) {
// Safe to set into gossip because by this time, the leader schedule cache should // Safe to set into gossip because by this time, the leader schedule cache should
// also be updated with the latest root (done in blockstore_processor) and thus // also be updated with the latest root (done in blockstore_processor) and thus
// will provide a schedule to window_service for any incoming shreds up to the // will provide a schedule to window_service for any incoming shreds up to the
// last_confirmed_epoch. // last_confirmed_epoch.
cluster_info.push_lowest_slot(id, blockstore.lowest_slot()); cluster_info.push_lowest_slot(blockstore.lowest_slot());
} }
fn update_lowest_slot(id: &Pubkey, lowest_slot: Slot, cluster_info: &ClusterInfo) { fn update_lowest_slot(lowest_slot: Slot, cluster_info: &ClusterInfo) {
cluster_info.push_lowest_slot(*id, lowest_slot); cluster_info.push_lowest_slot(lowest_slot);
} }
fn initialize_epoch_slots(bank_forks: &RwLock<BankForks>, cluster_info: &ClusterInfo) { fn initialize_epoch_slots(bank_forks: &RwLock<BankForks>, cluster_info: &ClusterInfo) {
@ -179,15 +177,18 @@ impl ClusterSlotsService {
#[cfg(test)] #[cfg(test)]
mod test { mod test {
use super::*; use {
use solana_gossip::{cluster_info::Node, crds_value::CrdsValueLabel}; super::*,
solana_gossip::{cluster_info::Node, crds_value::CrdsValueLabel},
solana_sdk::pubkey::Pubkey,
};
#[test] #[test]
pub fn test_update_lowest_slot() { pub fn test_update_lowest_slot() {
let pubkey = Pubkey::new_unique(); let pubkey = Pubkey::new_unique();
let node_info = Node::new_localhost_with_pubkey(&pubkey); let node_info = Node::new_localhost_with_pubkey(&pubkey);
let cluster_info = ClusterInfo::new_with_invalid_keypair(node_info.info); let cluster_info = ClusterInfo::new_with_invalid_keypair(node_info.info);
ClusterSlotsService::update_lowest_slot(&pubkey, 5, &cluster_info); ClusterSlotsService::update_lowest_slot(5, &cluster_info);
cluster_info.flush_push_queue(); cluster_info.flush_push_queue();
let lowest = { let lowest = {
let label = CrdsValueLabel::LowestSlot(pubkey); let label = CrdsValueLabel::LowestSlot(pubkey);

View File

@ -821,20 +821,20 @@ impl ClusterInfo {
) )
} }
pub fn push_lowest_slot(&self, id: Pubkey, min: Slot) { pub fn push_lowest_slot(&self, min: Slot) {
let now = timestamp(); let self_pubkey = self.id();
let last = self let last = {
.gossip let gossip = self.gossip.read().unwrap();
.read() gossip
.unwrap() .crds
.crds .get_lowest_slot(self_pubkey)
.get(&CrdsValueLabel::LowestSlot(self.id())) .map(|x| x.lowest)
.and_then(|x| x.value.lowest_slot()) .unwrap_or_default()
.map(|x| x.lowest) };
.unwrap_or(0);
if min > last { if min > last {
let now = timestamp();
let entry = CrdsValue::new_signed( let entry = CrdsValue::new_signed(
CrdsData::LowestSlot(0, LowestSlot::new(id, min, now)), CrdsData::LowestSlot(0, LowestSlot::new(self_pubkey, min, now)),
&self.keypair(), &self.keypair(),
); );
self.local_message_pending_push_queue self.local_message_pending_push_queue

View File

@ -83,7 +83,7 @@ impl Signable for CrdsValue {
pub enum CrdsData { pub enum CrdsData {
ContactInfo(ContactInfo), ContactInfo(ContactInfo),
Vote(VoteIndex, Vote), Vote(VoteIndex, Vote),
LowestSlot(u8, LowestSlot), LowestSlot(/*DEPRECATED:*/ u8, LowestSlot),
SnapshotHashes(SnapshotHash), SnapshotHashes(SnapshotHash),
AccountsHashes(SnapshotHash), AccountsHashes(SnapshotHash),
EpochSlots(EpochSlotsIndex, EpochSlots), EpochSlots(EpochSlotsIndex, EpochSlots),