Skip to content

Commit

Permalink
Adds accounts lt hash featurization
Browse files Browse the repository at this point in the history
  • Loading branch information
@brooksprumo
brooksprumo committed Nov 11, 2024
1 parent 683ca1b commit f3c1449
Show file tree
Hide file tree
Showing 7 changed files with 262 additions and 46 deletions.
8 changes: 8 additions & 0 deletions core/tests/epoch_accounts_hash.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ use {
solana_sdk::{
clock::Slot,
epoch_schedule::EpochSchedule,
feature_set,
native_token::LAMPORTS_PER_SOL,
pubkey::Pubkey,
signature::{Keypair, Signer},
Expand Down Expand Up @@ -102,6 +103,13 @@ impl TestEnvironment {
);
genesis_config_info.genesis_config.epoch_schedule =
EpochSchedule::custom(Self::SLOTS_PER_EPOCH, Self::SLOTS_PER_EPOCH, false);
// When the accounts lt hash feature is enabled, the EAH is *disabled*.
// Disable the accounts lt hash feature by removing its account from genesis.
genesis_config_info
.genesis_config
.accounts
.remove(&feature_set::accounts_lt_hash::id())
.unwrap();
let snapshot_config = SnapshotConfig {
full_snapshot_archives_dir: full_snapshot_archives_dir.path().to_path_buf(),
incremental_snapshot_archives_dir: incremental_snapshot_archives_dir
Expand Down
86 changes: 67 additions & 19 deletions runtime/src/bank.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1505,9 +1505,9 @@ impl Bank {
.build()
.expect("new rayon threadpool"));

let (_, apply_feature_activations_time_us) = measure_us!(
let (_, apply_feature_activations_time_us) = measure_us!(thread_pool.install(|| {
self.apply_feature_activations(ApplyFeatureActivationsCaller::NewFromParent, false)
);
}));

// Add new entry to stakes.stake_history, set appropriate epoch and
// update vote accounts with warmed up stakes before saving a
Expand Down Expand Up @@ -1765,8 +1765,13 @@ impl Bank {
*bank.accounts_lt_hash.get_mut().unwrap() = accounts_lt_hash;
} else {
// Use the accounts lt hash from the snapshot, if present, otherwise calculate it.
// When there is a feature gate for the accounts lt hash, if the feature is enabled
// then it will be *required* that the snapshot contains an accounts lt hash.
// When the feature gate is enabled, the snapshot *must* contain an accounts lt hash.
assert!(
!bank
.feature_set
.is_active(&feature_set::accounts_lt_hash::id()),
"snapshot must have an accounts lt hash if the feature is enabled",
);
if bank.is_accounts_lt_hash_enabled() {
info!("Calculating the accounts lt hash...");
let (ancestors, slot) = if bank.is_frozen() {
Expand Down Expand Up @@ -5526,9 +5531,21 @@ impl Bank {
self.last_blockhash().as_ref(),
]);

let epoch_accounts_hash = self.wait_get_epoch_accounts_hash();
if let Some(epoch_accounts_hash) = epoch_accounts_hash {
hash = hashv(&[hash.as_ref(), epoch_accounts_hash.as_ref().as_ref()]);
let accounts_hash_info = if self
.feature_set
.is_active(&feature_set::accounts_lt_hash::id())
{
let accounts_lt_hash = &*self.accounts_lt_hash.lock().unwrap();
let lt_hash_bytes = bytemuck::must_cast_slice(&accounts_lt_hash.0 .0);
hash = hashv(&[hash.as_ref(), lt_hash_bytes]);
let checksum = accounts_lt_hash.0.checksum();
Some(format!(", accounts_lt_hash checksum: {checksum}"))
} else {
let epoch_accounts_hash = self.wait_get_epoch_accounts_hash();
epoch_accounts_hash.map(|epoch_accounts_hash| {
hash = hashv(&[hash.as_ref(), epoch_accounts_hash.as_ref().as_ref()]);
format!(", epoch_accounts_hash: {:?}", epoch_accounts_hash.as_ref())
})
};

let buf = self
Expand Down Expand Up @@ -5580,22 +5597,12 @@ impl Bank {
("accounts_delta_hash_us", accounts_delta_hash_us, i64),
);
info!(
"bank frozen: {slot} hash: {hash} accounts_delta: {} signature_count: {} last_blockhash: {} capitalization: {}{}, stats: {bank_hash_stats:?}{}",
"bank frozen: {slot} hash: {hash} accounts_delta: {} signature_count: {} last_blockhash: {} capitalization: {}{}, stats: {bank_hash_stats:?}",
accounts_delta_hash.0,
self.signature_count(),
self.last_blockhash(),
self.capitalization(),
if let Some(epoch_accounts_hash) = epoch_accounts_hash {
format!(", epoch_accounts_hash: {:?}", epoch_accounts_hash.as_ref())
} else {
"".to_string()
},
if self.is_accounts_lt_hash_enabled() {
let checksum = self.accounts_lt_hash.lock().unwrap().0.checksum();
format!(", accounts_lt_hash checksum: {checksum}")
} else {
String::new()
},
accounts_hash_info.unwrap_or_default(),
);
hash
}
Expand Down Expand Up @@ -6702,6 +6709,47 @@ impl Bank {
if new_feature_activations.contains(&feature_set::update_hashes_per_tick6::id()) {
self.apply_updated_hashes_per_tick(UPDATED_HASHES_PER_TICK6);
}

if new_feature_activations.contains(&feature_set::accounts_lt_hash::id()) {
// Activating the accounts lt hash feature means we need to have an accounts lt hash
// value at the end of this if-block. If the cli arg has been used, that means we
// already have an accounts lt hash and do not need to recalculate it.
if self
.rc
.accounts
.accounts_db
.is_experimental_accumulator_hash_enabled()
{
// We already have an accounts lt hash value, so no need to recalculate it.
// Nothing else to do here.
} else {
info!(
"Calculating the accounts lt hash as part of feature activation; \
this may take some time...",
);
// We must calculate the accounts lt hash now as part of feature activation.
// Note, this bank is *not* frozen yet, which means it will later call
// `update_accounts_lt_hash()`. Therefore, we calculate the accounts lt hash based
// on *our parent*, not us!
let parent_ancestors = {
let mut ancestors = self.ancestors.clone();
ancestors.remove(&self.slot());
ancestors
};
let parent_slot = self.parent_slot;
let (parent_accounts_lt_hash, duration) = meas_dur!({
self.rc
.accounts
.accounts_db
.calculate_accounts_lt_hash_at_startup_from_index(
&parent_ancestors,
parent_slot,
)
});
*self.accounts_lt_hash.get_mut().unwrap() = parent_accounts_lt_hash;
info!("Calculating the accounts lt hash completed in {duration:?}");
}
}
}

fn apply_updated_hashes_per_tick(&mut self, hashes_per_tick: u64) {
Expand Down
155 changes: 147 additions & 8 deletions runtime/src/bank/accounts_lt_hash.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ use {
solana_measure::{meas_dur, measure::Measure},
solana_sdk::{
account::{accounts_equal, AccountSharedData},
feature_set,
pubkey::Pubkey,
},
solana_svm::transaction_processing_callback::AccountState,
Expand All @@ -19,6 +20,9 @@ impl Bank {
.accounts
.accounts_db
.is_experimental_accumulator_hash_enabled()
|| self
.feature_set
.is_active(&feature_set::accounts_lt_hash::id())
}

/// Updates the accounts lt hash
Expand Down Expand Up @@ -312,15 +316,19 @@ mod tests {
use {
super::*,
crate::{
bank::tests::new_bank_from_parent_with_bank_forks, genesis_utils,
runtime_config::RuntimeConfig, snapshot_bank_utils, snapshot_config::SnapshotConfig,
bank::tests::{new_bank_from_parent_with_bank_forks, new_from_parent_next_epoch},
genesis_utils,
runtime_config::RuntimeConfig,
snapshot_bank_utils,
snapshot_config::SnapshotConfig,
snapshot_utils,
},
solana_accounts_db::accounts_db::{
AccountsDbConfig, DuplicatesLtHash, ACCOUNTS_DB_CONFIG_FOR_TESTING,
},
solana_sdk::{
account::{ReadableAccount as _, WritableAccount as _},
feature::{self, Feature},
fee_calculator::FeeRateGovernor,
genesis_config::{self, GenesisConfig},
native_token::LAMPORTS_PER_SOL,
Expand Down Expand Up @@ -554,7 +562,7 @@ mod tests {
bank.rc
.accounts
.accounts_db
.set_is_experimental_accumulator_hash_enabled(true);
.set_is_experimental_accumulator_hash_enabled(features == Features::None);

// ensure the accounts lt hash is enabled, otherwise this test doesn't actually do anything...
assert!(bank.is_accounts_lt_hash_enabled());
Expand Down Expand Up @@ -587,7 +595,7 @@ mod tests {
bank.rc
.accounts
.accounts_db
.set_is_experimental_accumulator_hash_enabled(true);
.set_is_experimental_accumulator_hash_enabled(features == Features::None);

// ensure the accounts lt hash is enabled, otherwise this test doesn't actually do anything...
assert!(bank.is_accounts_lt_hash_enabled());
Expand Down Expand Up @@ -700,7 +708,7 @@ mod tests {
bank.rc
.accounts
.accounts_db
.set_is_experimental_accumulator_hash_enabled(true);
.set_is_experimental_accumulator_hash_enabled(features == Features::None);

// ensure the accounts lt hash is enabled, otherwise this test doesn't actually do anything...
assert!(bank.is_accounts_lt_hash_enabled());
Expand Down Expand Up @@ -749,7 +757,7 @@ mod tests {
bank.rc
.accounts
.accounts_db
.set_is_experimental_accumulator_hash_enabled(true);
.set_is_experimental_accumulator_hash_enabled(features == Features::None);

// ensure the accounts lt hash is enabled, otherwise this test doesn't actually do anything...
assert!(bank.is_accounts_lt_hash_enabled());
Expand Down Expand Up @@ -849,7 +857,7 @@ mod tests {
bank.rc
.accounts
.accounts_db
.set_is_experimental_accumulator_hash_enabled(true);
.set_is_experimental_accumulator_hash_enabled(features == Features::None);

// ensure the accounts lt hash is enabled, otherwise this test doesn't actually do anything...
assert!(bank.is_accounts_lt_hash_enabled());
Expand Down Expand Up @@ -943,7 +951,7 @@ mod tests {
bank.rc
.accounts
.accounts_db
.set_is_experimental_accumulator_hash_enabled(true);
.set_is_experimental_accumulator_hash_enabled(features == Features::None);

// ensure the accounts lt hash is enabled, otherwise this test doesn't actually do anything...
assert!(bank.is_accounts_lt_hash_enabled());
Expand Down Expand Up @@ -974,4 +982,135 @@ mod tests {
actual_cache.sort_unstable_by(|a, b| a.0.cmp(&b.0));
assert_eq!(expected_cache, actual_cache.as_slice());
}

/// Ensure that feature activation plays nicely with the cli arg
#[test_matrix(
[Features::None, Features::All],
[Cli::Off, Cli::On],
[Cli::Off, Cli::On]
)]
fn test_accounts_lt_hash_feature_activation(features: Features, cli: Cli, verify_cli: Cli) {
let (mut genesis_config, mint_keypair) = genesis_config_with(features);
// since we're testing feature activation, it must start deactivated (i.e. not present)
_ = genesis_config
.accounts
.remove(&feature_set::accounts_lt_hash::id());
let (mut bank, bank_forks) = Bank::new_with_bank_forks_for_tests(&genesis_config);
bank.rc
.accounts
.accounts_db
.set_is_experimental_accumulator_hash_enabled(match cli {
Cli::Off => false,
Cli::On => true,
});

let amount = cmp::max(
bank.get_minimum_balance_for_rent_exemption(Feature::size_of()),
1,
);

// create some banks with some modified accounts so that there are stored accounts
// (note: the number of banks and transfers are arbitrary)
for _ in 0..9 {
let slot = bank.slot() + 1;
bank =
new_bank_from_parent_with_bank_forks(&bank_forks, bank, &Pubkey::default(), slot);
bank.register_unique_recent_blockhash_for_test();
bank.transfer(amount, &mint_keypair, &pubkey::new_rand())
.unwrap();
bank.fill_bank_with_ticks_for_tests();
bank.squash();
bank.force_flush_accounts_cache();
}

// Create a new bank so that we can store the feature gate account;
// this triggers feature activation at the next epoch boundary.
// Then create another bank in the next epoch to activate the feature.
let slot = bank.slot() + 1;
bank = new_bank_from_parent_with_bank_forks(&bank_forks, bank, &Pubkey::default(), slot);
bank.store_account_and_update_capitalization(
&feature_set::accounts_lt_hash::id(),
&feature::create_account(&Feature { activated_at: None }, amount),
);
assert!(!bank
.feature_set
.is_active(&feature_set::accounts_lt_hash::id()));
bank = new_from_parent_next_epoch(bank, &bank_forks, 1);
assert!(bank
.feature_set
.is_active(&feature_set::accounts_lt_hash::id()));

// create some more banks with some more modified accounts
for _ in 0..5 {
let slot = bank.slot() + 1;
bank =
new_bank_from_parent_with_bank_forks(&bank_forks, bank, &Pubkey::default(), slot);
bank.register_unique_recent_blockhash_for_test();
bank.transfer(amount, &mint_keypair, &pubkey::new_rand())
.unwrap();
bank.fill_bank_with_ticks_for_tests();
}

// Now verify the accounts lt hash from feature activation is the same as if we calculated
// it at startup. We root the bank and flush the accounts write cache for snapshots,
// yet also do it here explicitly. This allows us to verify the accounts lt hash with both
// the index-based and the storages-based calculation in similar startup-like states.
bank.squash();
bank.force_flush_accounts_cache();
let calculated_accounts_lt_hash_from_index = bank
.rc
.accounts
.accounts_db
.calculate_accounts_lt_hash_at_startup_from_index(&bank.ancestors, bank.slot);
assert_eq!(
calculated_accounts_lt_hash_from_index,
*bank.accounts_lt_hash.lock().unwrap(),
);

// Verification using storages happens at startup.
// Mimic the behavior by taking, then loading from, a snapshot.
let snapshot_config = SnapshotConfig::default();
let bank_snapshots_dir = TempDir::new().unwrap();
let snapshot_archives_dir = TempDir::new().unwrap();
let snapshot = snapshot_bank_utils::bank_to_full_snapshot_archive(
&bank_snapshots_dir,
&bank,
Some(snapshot_config.snapshot_version),
&snapshot_archives_dir,
&snapshot_archives_dir,
snapshot_config.archive_format,
)
.unwrap();
let (_accounts_tempdir, accounts_dir) = snapshot_utils::create_tmp_accounts_dir_for_tests();
let accounts_db_config = AccountsDbConfig {
enable_experimental_accumulator_hash: match verify_cli {
Cli::Off => false,
Cli::On => true,
},
..ACCOUNTS_DB_CONFIG_FOR_TESTING
};
let (roundtrip_bank, _) = snapshot_bank_utils::bank_from_snapshot_archives(
&[accounts_dir],
&bank_snapshots_dir,
&snapshot,
None,
&genesis_config,
&RuntimeConfig::default(),
None,
None,
None,
false,
false,
false,
false,
Some(accounts_db_config),
None,
Arc::default(),
)
.unwrap();

// Wait for the startup verification to complete. If we don't panic, then we're good!
roundtrip_bank.wait_for_initial_accounts_hash_verification_completed_for_tests();
assert_eq!(roundtrip_bank, *bank);
}
}
Loading

0 comments on commit f3c1449

Please sign in to comment.