Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
66 changes: 27 additions & 39 deletions runtime/src/bank.rs
Original file line number Diff line number Diff line change
Expand Up @@ -480,8 +480,7 @@ pub struct BankFieldsToSerialize {
pub is_delta: bool,
pub accounts_data_len: u64,
pub versioned_epoch_stakes: HashMap<u64, VersionedEpochStakes>,
// When removing the accounts lt hash featurization code, also remove this Option wrapper
pub accounts_lt_hash: Option<AccountsLtHash>,
Comment on lines -483 to -484
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was the other main fallout. We always have an AccountsLtHash now.

pub accounts_lt_hash: AccountsLtHash,
}

// Can't derive PartialEq because RwLock doesn't implement PartialEq
Expand Down Expand Up @@ -596,8 +595,7 @@ impl PartialEq for Bank {
// different Mutexes.
&& (Arc::ptr_eq(hash_overrides, &other.hash_overrides) ||
*hash_overrides.lock().unwrap() == *other.hash_overrides.lock().unwrap())
&& !(self.is_accounts_lt_hash_enabled() && other.is_accounts_lt_hash_enabled()
&& *accounts_lt_hash.lock().unwrap() != *other.accounts_lt_hash.lock().unwrap())
&& *accounts_lt_hash.lock().unwrap() == *other.accounts_lt_hash.lock().unwrap()
&& *block_id.read().unwrap() == *other.block_id.read().unwrap()
}
}
Expand Down Expand Up @@ -637,7 +635,7 @@ impl BankFieldsToSerialize {
is_delta: bool::default(),
accounts_data_len: u64::default(),
versioned_epoch_stakes: HashMap::default(),
accounts_lt_hash: Some(AccountsLtHash(LtHash([0x7E57; LtHash::NUM_ELEMENTS]))),
accounts_lt_hash: AccountsLtHash(LtHash([0x7E57; LtHash::NUM_ELEMENTS])),
}
}
}
Expand Down Expand Up @@ -1405,29 +1403,25 @@ impl Bank {
.transaction_processor
.fill_missing_sysvar_cache_entries(&new));

let (num_accounts_modified_this_slot, populate_cache_for_accounts_lt_hash_us) = new
.is_accounts_lt_hash_enabled()
.then(|| {
measure_us!({
// The cache for accounts lt hash needs to be made aware of accounts modified
// before transaction processing begins. Otherwise we may calculate the wrong
// accounts lt hash due to having the wrong initial state of the account. The
// lt hash cache's initial state must always be from an ancestor, and cannot be
// an intermediate state within this Bank's slot. If the lt hash cache has the
// wrong initial account state, we'll mix out the wrong lt hash value, and thus
// have the wrong overall accounts lt hash, and diverge.
let accounts_modified_this_slot =
new.rc.accounts.accounts_db.get_pubkeys_for_slot(slot);
let num_accounts_modified_this_slot = accounts_modified_this_slot.len();
for pubkey in accounts_modified_this_slot {
new.cache_for_accounts_lt_hash
.entry(pubkey)
.or_insert(AccountsLtHashCacheValue::BankNew);
}
num_accounts_modified_this_slot
})
})
.unzip();
let (num_accounts_modified_this_slot, populate_cache_for_accounts_lt_hash_us) =
measure_us!({
// The cache for accounts lt hash needs to be made aware of accounts modified
// before transaction processing begins. Otherwise we may calculate the wrong
// accounts lt hash due to having the wrong initial state of the account. The
// lt hash cache's initial state must always be from an ancestor, and cannot be
// an intermediate state within this Bank's slot. If the lt hash cache has the
// wrong initial account state, we'll mix out the wrong lt hash value, and thus
// have the wrong overall accounts lt hash, and diverge.
let accounts_modified_this_slot =
new.rc.accounts.accounts_db.get_pubkeys_for_slot(slot);
let num_accounts_modified_this_slot = accounts_modified_this_slot.len();
for pubkey in accounts_modified_this_slot {
new.cache_for_accounts_lt_hash
.entry(pubkey)
.or_insert(AccountsLtHashCacheValue::BankNew);
}
num_accounts_modified_this_slot
});

time.stop();
report_new_bank_metrics(
Expand Down Expand Up @@ -1926,9 +1920,7 @@ impl Bank {
is_delta: self.is_delta.load(Relaxed),
accounts_data_len: self.load_accounts_data_size(),
versioned_epoch_stakes: self.epoch_stakes.clone(),
accounts_lt_hash: self
.is_accounts_lt_hash_enabled()
.then(|| self.accounts_lt_hash.lock().unwrap().clone()),
accounts_lt_hash: self.accounts_lt_hash.lock().unwrap().clone(),
}
}

Expand Down Expand Up @@ -2538,11 +2530,9 @@ impl Bank {

// freeze is a one-way trip, idempotent
self.freeze_started.store(true, Relaxed);
if self.is_accounts_lt_hash_enabled() {
// updating the accounts lt hash must happen *outside* of hash_internal_state() so
// that rehash() can be called and *not* modify self.accounts_lt_hash.
self.update_accounts_lt_hash();
}
// updating the accounts lt hash must happen *outside* of hash_internal_state() so
// that rehash() can be called and *not* modify self.accounts_lt_hash.
self.update_accounts_lt_hash();
*hash = self.hash_internal_state();
self.rc.accounts.accounts_db.mark_slot_frozen(self.slot());
}
Expand Down Expand Up @@ -5897,9 +5887,7 @@ impl TransactionProcessingCallback for Bank {
}

fn inspect_account(&self, address: &Pubkey, account_state: AccountState, is_writable: bool) {
if self.is_accounts_lt_hash_enabled() {
self.inspect_account_for_accounts_lt_hash(address, &account_state, is_writable);
}
self.inspect_account_for_accounts_lt_hash(address, &account_state, is_writable);
}
}

Expand Down
48 changes: 7 additions & 41 deletions runtime/src/bank/accounts_lt_hash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,22 +17,15 @@ use {
};

impl Bank {
/// Returns if the accounts lt hash is enabled
pub fn is_accounts_lt_hash_enabled(&self) -> bool {
true
}
Comment on lines -20 to -23
Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Main change.


/// Returns if snapshots use the accounts lt hash
pub fn is_snapshots_lt_hash_enabled(&self) -> bool {
self.is_accounts_lt_hash_enabled()
&& (self
.rc
.accounts
.accounts_db
.snapshots_use_experimental_accumulator_hash()
|| self
.feature_set
.is_active(&feature_set::snapshots_lt_hash::id()))
self.rc
.accounts
.accounts_db
.snapshots_use_experimental_accumulator_hash()
|| self
.feature_set
.is_active(&feature_set::snapshots_lt_hash::id())
}

/// Updates the accounts lt hash
Expand All @@ -44,7 +37,6 @@ impl Bank {
///
/// Since this function is non-idempotent, it should only be called once per bank.
pub fn update_accounts_lt_hash(&self) {
debug_assert!(self.is_accounts_lt_hash_enabled());
let delta_lt_hash = self.calculate_delta_lt_hash();
let mut accounts_lt_hash = self.accounts_lt_hash.lock().unwrap();
accounts_lt_hash.0.mix_in(&delta_lt_hash);
Expand All @@ -60,7 +52,6 @@ impl Bank {
///
/// This function is idempotent, and may be called more than once.
fn calculate_delta_lt_hash(&self) -> LtHash {
debug_assert!(self.is_accounts_lt_hash_enabled());
let measure_total = Measure::start("");
let slot = self.slot();

Expand Down Expand Up @@ -308,7 +299,6 @@ impl Bank {
account_state: &AccountState,
is_writable: bool,
) {
debug_assert!(self.is_accounts_lt_hash_enabled());
if !is_writable {
// if the account is not writable, then it cannot be modified; nothing to do here
return;
Expand Down Expand Up @@ -486,9 +476,6 @@ mod tests {
genesis_config.fee_rate_governor = FeeRateGovernor::new(0, 0);
let (bank, bank_forks) = Bank::new_with_bank_forks_for_tests(&genesis_config);

// ensure the accounts lt hash is enabled, otherwise this test doesn't actually do anything...
assert!(bank.is_accounts_lt_hash_enabled());

let amount = cmp::max(
bank.get_minimum_balance_for_rent_exemption(0),
LAMPORTS_PER_SOL,
Expand Down Expand Up @@ -648,9 +635,6 @@ mod tests {
let (genesis_config, mint_keypair) = genesis_config_with(features);
let (bank, _bank_forks) = Bank::new_with_bank_forks_for_tests(&genesis_config);

// ensure the accounts lt hash is enabled, otherwise this test doesn't actually do anything...
assert!(bank.is_accounts_lt_hash_enabled());

// ensure this bank is for slot 0, otherwise this test doesn't actually do anything...
assert_eq!(bank.slot(), 0);

Expand All @@ -677,9 +661,6 @@ mod tests {
let (genesis_config, _mint_keypair) = genesis_config_with(features);
let (bank, _bank_forks) = Bank::new_with_bank_forks_for_tests(&genesis_config);

// ensure the accounts lt hash is enabled, otherwise this test doesn't actually do anything...
assert!(bank.is_accounts_lt_hash_enabled());

// the cache should start off empty
assert_eq!(bank.cache_for_accounts_lt_hash.len(), 0);

Expand Down Expand Up @@ -788,9 +769,6 @@ mod tests {
let (genesis_config, mint_keypair) = genesis_config_with(features);
let (mut bank, bank_forks) = Bank::new_with_bank_forks_for_tests(&genesis_config);

// ensure the accounts lt hash is enabled, otherwise this test doesn't actually do anything...
assert!(bank.is_accounts_lt_hash_enabled());

let amount = cmp::max(
bank.get_minimum_balance_for_rent_exemption(0),
LAMPORTS_PER_SOL,
Expand Down Expand Up @@ -833,9 +811,6 @@ mod tests {
let (genesis_config, mint_keypair) = genesis_config_with(features);
let (mut bank, bank_forks) = Bank::new_with_bank_forks_for_tests(&genesis_config);

// ensure the accounts lt hash is enabled, otherwise this test doesn't actually do anything...
assert!(bank.is_accounts_lt_hash_enabled());

let amount = cmp::max(
bank.get_minimum_balance_for_rent_exemption(0),
LAMPORTS_PER_SOL,
Expand Down Expand Up @@ -933,9 +908,6 @@ mod tests {
genesis_config.fee_rate_governor = FeeRateGovernor::new(0, 0);
let (mut bank, bank_forks) = Bank::new_with_bank_forks_for_tests(&genesis_config);

// ensure the accounts lt hash is enabled, otherwise this test doesn't actually do anything...
assert!(bank.is_accounts_lt_hash_enabled());

let amount = cmp::max(
bank.get_minimum_balance_for_rent_exemption(0),
LAMPORTS_PER_SOL,
Expand Down Expand Up @@ -1058,9 +1030,6 @@ mod tests {
let (genesis_config, _mint_keypair) = genesis_config_with(features);
let (mut bank, bank_forks) = Bank::new_with_bank_forks_for_tests(&genesis_config);

// ensure the accounts lt hash is enabled, otherwise this test doesn't actually do anything...
assert!(bank.is_accounts_lt_hash_enabled());

let slot = bank.slot() + 1;
bank = new_bank_from_parent_with_bank_forks(&bank_forks, bank, &Pubkey::default(), slot);

Expand Down Expand Up @@ -1096,9 +1065,6 @@ mod tests {
let (genesis_config, mint_keypair) = genesis_config_with(features);
let (mut bank, bank_forks) = Bank::new_with_bank_forks_for_tests(&genesis_config);

// ensure the accounts lt hash is enabled, otherwise the snapshot lt hash is disabled
assert!(bank.is_accounts_lt_hash_enabled());

bank.rc
.accounts
.accounts_db
Expand Down
8 changes: 4 additions & 4 deletions runtime/src/bank/metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ pub(crate) struct NewBankTimings {
pub(crate) cache_preparation_time_us: u64,
pub(crate) update_sysvars_time_us: u64,
pub(crate) fill_sysvar_cache_time_us: u64,
pub(crate) populate_cache_for_accounts_lt_hash_us: Option<u64>,
pub(crate) populate_cache_for_accounts_lt_hash_us: u64,
}

pub(crate) fn report_new_epoch_metrics(
Expand Down Expand Up @@ -98,7 +98,7 @@ pub(crate) fn report_new_bank_metrics(
slot: Slot,
parent_slot: Slot,
block_height: u64,
num_accounts_modified_this_slot: Option<usize>,
num_accounts_modified_this_slot: usize,
timings: NewBankTimings,
) {
datapoint_info!(
Expand Down Expand Up @@ -151,12 +151,12 @@ pub(crate) fn report_new_bank_metrics(
(
"num_accounts_modified_this_slot",
num_accounts_modified_this_slot,
Option<i64>
i64
),
(
"populate_cache_for_accounts_lt_hash_us",
timings.populate_cache_for_accounts_lt_hash_us,
Option<i64>
i64
),
);
}
Expand Down
2 changes: 1 addition & 1 deletion runtime/src/bank/serde_snapshot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ mod tests {
{
let mut bank_fields = bank2.get_fields_to_serialize();
let versioned_epoch_stakes = mem::take(&mut bank_fields.versioned_epoch_stakes);
let accounts_lt_hash = bank_fields.accounts_lt_hash.clone().map(Into::into);
let accounts_lt_hash = Some(bank_fields.accounts_lt_hash.clone().into());
serde_snapshot::serialize_bank_snapshot_into(
&mut writer,
bank_fields,
Expand Down
2 changes: 1 addition & 1 deletion runtime/src/serde_snapshot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -702,7 +702,7 @@ impl Serialize for SerializableBankAndStorage<'_> {
let write_version = accounts_db.write_version.load(Ordering::Acquire);
let lamports_per_signature = bank_fields.fee_rate_governor.lamports_per_signature;
let versioned_epoch_stakes = std::mem::take(&mut bank_fields.versioned_epoch_stakes);
let accounts_lt_hash = bank_fields.accounts_lt_hash.clone().map(Into::into);
let accounts_lt_hash = Some(bank_fields.accounts_lt_hash.clone().into());
let bank_fields_to_serialize = (
SerializableVersionedBank::from(bank_fields),
SerializableAccountsDb::<'_> {
Expand Down
2 changes: 1 addition & 1 deletion runtime/src/snapshot_hash.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ impl SnapshotHash {
#[must_use]
pub fn new(
merkle_or_lattice_accounts_hash: &MerkleOrLatticeAccountsHash,
accounts_lt_hash_checksum: Option<AccountsLtHashChecksum>,
accounts_lt_hash_checksum: Option<AccountsLtHashChecksum>, // option wrapper will be removed next
) -> Self {
let accounts_hash = match merkle_or_lattice_accounts_hash {
MerkleOrLatticeAccountsHash::Merkle(accounts_hash_kind) => {
Expand Down
22 changes: 8 additions & 14 deletions runtime/src/snapshot_minimizer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,16 +70,14 @@ impl<'a> SnapshotMinimizer<'a> {
.bank
.set_capitalization_for_tests(minimizer.bank.calculate_capitalization_for_tests());

if minimizer.bank.is_accounts_lt_hash_enabled() {
// Since the account state has changed, the accounts lt hash must be recalculated
let new_accounts_lt_hash = minimizer
.accounts_db()
.calculate_accounts_lt_hash_at_startup_from_index(
&minimizer.bank.ancestors,
minimizer.bank.slot(),
);
bank.set_accounts_lt_hash_for_snapshot_minimizer(new_accounts_lt_hash);
}
// Since the account state has changed, the accounts lt hash must be recalculated
let new_accounts_lt_hash = minimizer
.accounts_db()
.calculate_accounts_lt_hash_at_startup_from_index(
&minimizer.bank.ancestors,
minimizer.bank.slot(),
);
bank.set_accounts_lt_hash_for_snapshot_minimizer(new_accounts_lt_hash);
}

/// Helper function to measure time and number of accounts added
Expand Down Expand Up @@ -594,10 +592,6 @@ mod tests {
let (bank, bank_forks) =
Bank::new_with_bank_forks_for_tests(&genesis_config_info.genesis_config);

// ensure the accounts lt hash is enabled, otherwise minimization
// doesn't need to recalculate it
assert!(bank.is_accounts_lt_hash_enabled());

// write to multiple accounts and keep track of one, for minimization later
let pubkey_to_keep = Pubkey::new_unique();
let slot = bank.slot() + 1;
Expand Down
12 changes: 7 additions & 5 deletions runtime/src/snapshot_package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -248,11 +248,13 @@ impl SnapshotPackage {
block_height: accounts_package.block_height,
hash: SnapshotHash::new(
&merkle_or_lattice_accounts_hash,
snapshot_info
.bank_fields_to_serialize
.accounts_lt_hash
.as_ref()
.map(|accounts_lt_hash| accounts_lt_hash.0.checksum()),
Some(
snapshot_info
.bank_fields_to_serialize
.accounts_lt_hash
.0
.checksum(),
),
),
snapshot_storages: accounts_package.snapshot_storages,
status_cache_slot_deltas: snapshot_info.status_cache_slot_deltas,
Expand Down
2 changes: 1 addition & 1 deletion runtime/src/snapshot_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -963,7 +963,7 @@ fn serialize_snapshot(
incremental_snapshot_persistence: bank_incremental_snapshot_persistence,
obsolete_epoch_accounts_hash: None,
versioned_epoch_stakes,
accounts_lt_hash: bank_fields.accounts_lt_hash.clone().map(Into::into),
accounts_lt_hash: Some(bank_fields.accounts_lt_hash.clone().into()),
};
serde_snapshot::serialize_bank_snapshot_into(
stream,
Expand Down
Loading