From c879e7c1ad7bb499566bf621faf679cd91a5511f Mon Sep 17 00:00:00 2001 From: carllin Date: Thu, 8 Oct 2020 23:44:41 -0700 Subject: [PATCH] Fix fee mismatch on snapshot deserialize (#12697) Co-authored-by: Carl Lin --- core/tests/snapshots.rs | 2 +- explorer/wasm/src/stake_account.rs | 4 +- runtime/src/bank.rs | 92 ++++++++++++++--------------- runtime/src/blockhash_queue.rs | 2 +- runtime/src/epoch_stakes.rs | 4 +- runtime/src/serde_snapshot/tests.rs | 2 +- sdk/src/fee_calculator.rs | 2 +- sdk/src/hard_forks.rs | 2 +- sdk/src/stake_history.rs | 4 +- 9 files changed, 57 insertions(+), 57 deletions(-) diff --git a/core/tests/snapshots.rs b/core/tests/snapshots.rs index ee9cb05999..85c55dbb7a 100644 --- a/core/tests/snapshots.rs +++ b/core/tests/snapshots.rs @@ -156,7 +156,7 @@ mod tests { .get(&deserialized_bank.slot()) .unwrap() .clone(); - bank.compare_bank(&deserialized_bank); + assert!(*bank == deserialized_bank); let slot_snapshot_paths = snapshot_utils::get_snapshot_paths(&snapshot_path); diff --git a/explorer/wasm/src/stake_account.rs b/explorer/wasm/src/stake_account.rs index 530e8faa2b..5c6a299274 100644 --- a/explorer/wasm/src/stake_account.rs +++ b/explorer/wasm/src/stake_account.rs @@ -113,7 +113,7 @@ pub type Epoch = u64; #[wasm_bindgen] #[repr(transparent)] -#[derive(Serialize, Deserialize, Clone, Copy, Eq, PartialEq, Ord, PartialOrd, Hash)] +#[derive(Serialize, Debug, Deserialize, Clone, Copy, Eq, PartialEq, Ord, PartialOrd, Hash)] pub struct Pubkey([u8; 32]); #[wasm_bindgen] @@ -164,7 +164,7 @@ impl Stake { } #[wasm_bindgen] -#[derive(Serialize, Deserialize, PartialEq, Clone, Copy)] +#[derive(Serialize, Debug, Deserialize, PartialEq, Clone, Copy)] pub struct Delegation { /// to whom the stake is delegated voter_pubkey: Pubkey, diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index e786921266..91dddfd1a4 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -448,6 +448,7 @@ pub(crate) struct BankFieldsToDeserialize { // This is separated from BankFieldsToDeserialize to avoid cloning by using refs. // So, sync fields with BankFieldsToDeserialize! // all members are made public to remain Bank private and to make versioned serializer workable on this +#[derive(Debug)] pub(crate) struct BankFieldsToSerialize<'a> { pub(crate) blockhash_queue: &'a RwLock, pub(crate) ancestors: &'a Ancestors, @@ -482,6 +483,46 @@ pub(crate) struct BankFieldsToSerialize<'a> { pub(crate) is_delta: bool, } +// Can't derive PartialEq because RwLock doesn't implement PartialEq +impl PartialEq for Bank { + fn eq(&self, other: &Self) -> bool { + if ptr::eq(self, other) { + return true; + } + *self.blockhash_queue.read().unwrap() == *other.blockhash_queue.read().unwrap() + && self.ancestors == other.ancestors + && *self.hash.read().unwrap() == *other.hash.read().unwrap() + && self.parent_hash == other.parent_hash + && self.parent_slot == other.parent_slot + && *self.hard_forks.read().unwrap() == *other.hard_forks.read().unwrap() + && self.transaction_count.load(Relaxed) == other.transaction_count.load(Relaxed) + && self.tick_height.load(Relaxed) == other.tick_height.load(Relaxed) + && self.signature_count.load(Relaxed) == other.signature_count.load(Relaxed) + && self.capitalization.load(Relaxed) == other.capitalization.load(Relaxed) + && self.max_tick_height == other.max_tick_height + && self.hashes_per_tick == other.hashes_per_tick + && self.ticks_per_slot == other.ticks_per_slot + && self.ns_per_slot == other.ns_per_slot + && self.genesis_creation_time == other.genesis_creation_time + && self.slots_per_year == other.slots_per_year + && self.unused == other.unused + && self.slot == other.slot + && self.epoch == other.epoch + && self.block_height == other.block_height + && self.collector_id == other.collector_id + && self.collector_fees.load(Relaxed) == other.collector_fees.load(Relaxed) + && self.fee_calculator == other.fee_calculator + && self.fee_rate_governor == other.fee_rate_governor + && self.collected_rent.load(Relaxed) == other.collected_rent.load(Relaxed) + && self.rent_collector == other.rent_collector + && self.epoch_schedule == other.epoch_schedule + && *self.inflation.read().unwrap() == *other.inflation.read().unwrap() + && *self.stakes.read().unwrap() == *other.stakes.read().unwrap() + && self.epoch_stakes == other.epoch_stakes + && self.is_delta.load(Relaxed) == other.is_delta.load(Relaxed) + } +} + #[derive(Debug, PartialEq, Serialize, Deserialize, AbiExample, Default, Clone, Copy)] pub struct RewardInfo { pub lamports: i64, // Reward amount @@ -883,7 +924,11 @@ impl Bank { ); assert_eq!(bank.epoch_schedule, genesis_config.epoch_schedule); assert_eq!(bank.epoch, bank.epoch_schedule.get_epoch(bank.slot)); - + bank.fee_rate_governor.lamports_per_signature = bank.fee_calculator.lamports_per_signature; + assert_eq!( + bank.fee_rate_governor.create_fee_calculator(), + bank.fee_calculator + ); bank } @@ -3652,51 +3697,6 @@ impl Bank { } } - pub fn compare_bank(&self, dbank: &Bank) { - if ptr::eq(self, dbank) { - return; - } - assert_eq!(self.slot, dbank.slot); - assert_eq!(self.collector_id, dbank.collector_id); - assert_eq!(self.epoch_schedule, dbank.epoch_schedule); - assert_eq!(self.hashes_per_tick, dbank.hashes_per_tick); - assert_eq!(self.ticks_per_slot, dbank.ticks_per_slot); - assert_eq!(self.parent_hash, dbank.parent_hash); - assert_eq!( - self.tick_height.load(Relaxed), - dbank.tick_height.load(Relaxed) - ); - assert_eq!(self.is_delta.load(Relaxed), dbank.is_delta.load(Relaxed)); - - { - let bh = self.hash.read().unwrap(); - let dbh = dbank.hash.read().unwrap(); - assert_eq!(*bh, *dbh); - } - - { - let st = self.stakes.read().unwrap(); - let dst = dbank.stakes.read().unwrap(); - assert_eq!(*st, *dst); - } - - { - let bhq = self.blockhash_queue.read().unwrap(); - let dbhq = dbank.blockhash_queue.read().unwrap(); - assert_eq!(*bhq, *dbhq); - } - - { - let sc = self.src.status_cache.read().unwrap(); - let dsc = dbank.src.status_cache.read().unwrap(); - assert_eq!(*sc, *dsc); - } - assert_eq!( - self.rc.accounts.bank_hash_at(self.slot), - dbank.rc.accounts.bank_hash_at(dbank.slot) - ); - } - pub fn clean_accounts(&self, skip_last: bool) { let max_clean_slot = if skip_last { // Don't clean the slot we're snapshotting because it may have zero-lamport diff --git a/runtime/src/blockhash_queue.rs b/runtime/src/blockhash_queue.rs index 9343c178d0..fd077cdaf3 100644 --- a/runtime/src/blockhash_queue.rs +++ b/runtime/src/blockhash_queue.rs @@ -13,7 +13,7 @@ struct HashAge { /// Low memory overhead, so can be cloned for every checkpoint #[frozen_abi(digest = "EwaoLA34VN18E95GvjmkeStUpWqTeBd7FGpd7mppLmEw")] -#[derive(Clone, Debug, PartialEq, Serialize, Deserialize, AbiExample)] +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize, AbiExample)] pub struct BlockhashQueue { /// updated whenever an hash is registered hash_height: u64, diff --git a/runtime/src/epoch_stakes.rs b/runtime/src/epoch_stakes.rs index 20d6079acb..696103cf08 100644 --- a/runtime/src/epoch_stakes.rs +++ b/runtime/src/epoch_stakes.rs @@ -7,13 +7,13 @@ use std::{collections::HashMap, sync::Arc}; pub type NodeIdToVoteAccounts = HashMap; pub type EpochAuthorizedVoters = HashMap; -#[derive(Clone, Serialize, Debug, Deserialize, Default, PartialEq, AbiExample)] +#[derive(Clone, Serialize, Debug, Deserialize, Default, PartialEq, Eq, AbiExample)] pub struct NodeVoteAccounts { pub vote_accounts: Vec, pub total_stake: u64, } -#[derive(Clone, Serialize, Deserialize, AbiExample)] +#[derive(Clone, Debug, Serialize, Deserialize, AbiExample, PartialEq)] pub struct EpochStakes { stakes: Arc, total_stake: u64, diff --git a/runtime/src/serde_snapshot/tests.rs b/runtime/src/serde_snapshot/tests.rs index f16a023c0a..9238ae0e4e 100644 --- a/runtime/src/serde_snapshot/tests.rs +++ b/runtime/src/serde_snapshot/tests.rs @@ -219,7 +219,7 @@ fn test_bank_serialize_style(serde_style: SerdeStyle) { assert_eq!(dbank.get_balance(&key1.pubkey()), 0); assert_eq!(dbank.get_balance(&key2.pubkey()), 10); assert_eq!(dbank.get_balance(&key3.pubkey()), 0); - bank2.compare_bank(&dbank); + assert!(bank2 == dbank); } #[cfg(test)] diff --git a/sdk/src/fee_calculator.rs b/sdk/src/fee_calculator.rs index 5abc497099..018aafffc3 100644 --- a/sdk/src/fee_calculator.rs +++ b/sdk/src/fee_calculator.rs @@ -68,7 +68,7 @@ pub struct FeeRateGovernor { // The current cost of a signature This amount may increase/decrease over time based on // cluster processing load. #[serde(skip)] - lamports_per_signature: u64, + pub lamports_per_signature: u64, // The target cost of a signature when the cluster is operating around target_signatures_per_slot // signatures diff --git a/sdk/src/hard_forks.rs b/sdk/src/hard_forks.rs index 6ae25076b7..2b65419db1 100644 --- a/sdk/src/hard_forks.rs +++ b/sdk/src/hard_forks.rs @@ -4,7 +4,7 @@ use byteorder::{ByteOrder, LittleEndian}; use solana_sdk::clock::Slot; -#[derive(Default, Clone, Deserialize, Serialize, AbiExample)] +#[derive(Default, Clone, Debug, Deserialize, Serialize, AbiExample, PartialEq, Eq)] pub struct HardForks { hard_forks: Vec<(Slot, usize)>, } diff --git a/sdk/src/stake_history.rs b/sdk/src/stake_history.rs index 818a0c427e..8a3c89d073 100644 --- a/sdk/src/stake_history.rs +++ b/sdk/src/stake_history.rs @@ -8,7 +8,7 @@ use std::ops::Deref; pub const MAX_ENTRIES: usize = 512; // it should never take as many as 512 epochs to warm up or cool down -#[derive(Debug, Serialize, Deserialize, PartialEq, Default, Clone, AbiExample)] +#[derive(Debug, Serialize, Deserialize, PartialEq, Eq, Default, Clone, AbiExample)] pub struct StakeHistoryEntry { pub effective: u64, // effective stake at this epoch pub activating: u64, // sum of portion of stakes not fully warmed up @@ -16,7 +16,7 @@ pub struct StakeHistoryEntry { } #[repr(C)] -#[derive(Debug, Serialize, Deserialize, PartialEq, Default, Clone, AbiExample)] +#[derive(Debug, Serialize, Deserialize, PartialEq, Eq, Default, Clone, AbiExample)] pub struct StakeHistory(Vec<(Epoch, StakeHistoryEntry)>); impl StakeHistory {