Skip to content

Commit

Permalink
Make BankHashDetails use a subtype for bank hash mixins (#2644)
Browse files Browse the repository at this point in the history
The option to only record bank hash, and not the items that factor into
the bank hash, was added a while ago. So, all of the items that factor
into bank hash are optional.

Someone is likely to either care about all of the bank hash mixins or
none of them, so move the items into a sub type and make it an Option<_>
  • Loading branch information
steviez authored Aug 20, 2024
1 parent 3d2639e commit 4f9679a
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 97 deletions.
19 changes: 7 additions & 12 deletions ledger-tool/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -571,13 +571,13 @@ fn setup_slot_recording(
exit(1);
});

let mut include_bank = false;
let mut include_bank_hash_components = false;
let mut include_tx = false;
if let Some(args) = arg_matches.values_of("record_slots_config") {
for arg in args {
match arg {
"tx" => include_tx = true,
"accounts" => include_bank = true,
"accounts" => include_bank_hash_components = true,
_ => unreachable!(),
}
}
Expand All @@ -603,16 +603,11 @@ fn setup_slot_recording(
let slot_callback = Arc::new({
let slots = Arc::clone(&slot_details);
move |bank: &Bank| {
let mut details = if include_bank {
bank_hash_details::SlotDetails::try_from(bank).unwrap()
} else {
bank_hash_details::SlotDetails {
slot: bank.slot(),
bank_hash: bank.hash().to_string(),
..Default::default()
}
};

let mut details = bank_hash_details::SlotDetails::new_from_bank(
bank,
include_bank_hash_components,
)
.unwrap();
let mut slots = slots.lock().unwrap();

if let Some(recorded_slot) = slots.iter_mut().find(|f| f.slot == details.slot) {
Expand Down
136 changes: 51 additions & 85 deletions runtime/src/bank/bank_hash_details.rs
Original file line number Diff line number Diff line change
Expand Up @@ -104,93 +104,63 @@ impl From<CommittedTransaction> for TransactionCommitDetails {
}
}

/// The components that go into a bank hash calculation for a single bank/slot.
#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize, Default)]
pub struct SlotDetails {
pub slot: Slot,
pub bank_hash: String,
#[serde(skip_serializing_if = "String::is_empty")]
#[serde(default)]
#[serde(skip_serializing_if = "Option::is_none", default, flatten)]
pub bank_hash_components: Option<BankHashComponents>,
#[serde(skip_serializing_if = "Vec::is_empty", default)]
pub transactions: Vec<TransactionDetails>,
}

/// The components that go into a bank hash calculation for a single bank
#[derive(Clone, Debug, Deserialize, Eq, PartialEq, Serialize, Default)]
pub struct BankHashComponents {
pub parent_bank_hash: String,
#[serde(skip_serializing_if = "String::is_empty")]
#[serde(default)]
pub accounts_delta_hash: String,
#[serde(skip_serializing_if = "u64_is_zero")]
#[serde(default)]
pub signature_count: u64,
#[serde(skip_serializing_if = "String::is_empty")]
#[serde(default)]
pub last_blockhash: String,
#[serde(skip_serializing_if = "accounts_is_empty")]
#[serde(default)]
pub accounts: AccountsDetails,
#[serde(skip_serializing_if = "Vec::is_empty")]
#[serde(default)]
pub transactions: Vec<TransactionDetails>,
}

fn u64_is_zero(val: &u64) -> bool {
*val == 0
}

fn accounts_is_empty(accounts: &AccountsDetails) -> bool {
accounts.accounts.is_empty()
}

impl SlotDetails {
pub fn new(
slot: Slot,
bank_hash: Hash,
parent_bank_hash: Hash,
accounts_delta_hash: Hash,
signature_count: u64,
last_blockhash: Hash,
accounts: AccountsDetails,
) -> Self {
Self {
slot,
bank_hash: bank_hash.to_string(),
parent_bank_hash: parent_bank_hash.to_string(),
accounts_delta_hash: accounts_delta_hash.to_string(),
signature_count,
last_blockhash: last_blockhash.to_string(),
accounts,
transactions: Vec::new(),
}
}
}

impl TryFrom<&Bank> for SlotDetails {
type Error = String;

fn try_from(bank: &Bank) -> Result<Self, Self::Error> {
pub fn new_from_bank(bank: &Bank, include_bank_hash_components: bool) -> Result<Self, String> {
let slot = bank.slot();
if !bank.is_frozen() {
return Err(format!(
"Bank {slot} must be frozen in order to get bank hash details"
));
}

// This bank is frozen; as a result, we know that the state has been
// hashed which means the delta hash is Some(). So, .unwrap() is safe
let AccountsDeltaHash(accounts_delta_hash) = bank
.rc
.accounts
.accounts_db
.get_accounts_delta_hash(slot)
.unwrap();

let accounts = bank.get_accounts_for_bank_hash_details();
let bank_hash_components = if include_bank_hash_components {
// This bank is frozen; as a result, we know that the state has been
// hashed which means the delta hash is Some(). So, .unwrap() is safe
let AccountsDeltaHash(accounts_delta_hash) = bank
.rc
.accounts
.accounts_db
.get_accounts_delta_hash(slot)
.unwrap();
let accounts = bank.get_accounts_for_bank_hash_details();

Some(BankHashComponents {
parent_bank_hash: bank.parent_hash().to_string(),
accounts_delta_hash: accounts_delta_hash.to_string(),
signature_count: bank.signature_count(),
last_blockhash: bank.last_blockhash().to_string(),
accounts: AccountsDetails { accounts },
})
} else {
None
};

Ok(Self::new(
Ok(Self {
slot,
bank.hash(),
bank.parent_hash(),
accounts_delta_hash,
bank.signature_count(),
bank.last_blockhash(),
AccountsDetails { accounts },
))
bank_hash: bank.hash().to_string(),
bank_hash_components,
transactions: Vec::new(),
})
}
}

Expand Down Expand Up @@ -291,7 +261,7 @@ impl<'de> Deserialize<'de> for AccountsDetails {

/// Output the components that comprise the overall bank hash for the supplied `Bank`
pub fn write_bank_hash_details_file(bank: &Bank) -> std::result::Result<(), String> {
let slot_details = SlotDetails::try_from(bank)?;
let slot_details = SlotDetails::new_from_bank(bank, /*include_bank_hash_mixins:*/ true)?;
let details = BankHashDetails::new(vec![slot_details]);

let parent_dir = bank
Expand Down Expand Up @@ -328,11 +298,9 @@ pub mod tests {
use super::*;

fn build_details(num_slots: usize) -> BankHashDetails {
use solana_sdk::hash::{hash, hashv};

let slot_details: Vec<_> = (0..num_slots)
.map(|slot| {
let signature_count = 314;
let slot = slot as u64;

let account = AccountSharedData::from(Account {
lamports: 123_456_789,
Expand All @@ -342,7 +310,7 @@ pub mod tests {
rent_epoch: 123,
});
let account_pubkey = Pubkey::new_unique();
let account_hash = AccountHash(hash("account".as_bytes()));
let account_hash = AccountHash(solana_sdk::hash::hash("account".as_bytes()));
let accounts = AccountsDetails {
accounts: vec![PubkeyHashAccount {
pubkey: account_pubkey,
Expand All @@ -351,20 +319,18 @@ pub mod tests {
}],
};

let bank_hash = hashv(&["bank".as_bytes(), &slot.to_le_bytes()]);
let parent_bank_hash = hash("parent_bank".as_bytes());
let accounts_delta_hash = hash("accounts_delta".as_bytes());
let last_blockhash = hash("last_blockhash".as_bytes());

SlotDetails::new(
slot as Slot,
bank_hash,
parent_bank_hash,
accounts_delta_hash,
signature_count,
last_blockhash,
accounts,
)
SlotDetails {
slot,
bank_hash: format!("bank{slot}"),
bank_hash_components: Some(BankHashComponents {
parent_bank_hash: "parent_bank_hash".into(),
accounts_delta_hash: "accounts_delta_hash".into(),
signature_count: slot + 10,
last_blockhash: "last_blockhash".into(),
accounts,
}),
transactions: vec![],
}
})
.collect();

Expand Down

0 comments on commit 4f9679a

Please sign in to comment.