From f3c14493094001ea0a231b71ca3685e6072a76f5 Mon Sep 17 00:00:00 2001 From: brooks Date: Tue, 22 Oct 2024 12:48:01 -0400 Subject: [PATCH] Adds accounts lt hash featurization --- core/tests/epoch_accounts_hash.rs | 8 + runtime/src/bank.rs | 86 +++++++--- runtime/src/bank/accounts_lt_hash.rs | 155 +++++++++++++++++- runtime/src/bank/epoch_accounts_hash_utils.rs | 9 + runtime/src/bank/tests.rs | 35 ++-- runtime/src/snapshot_bank_utils.rs | 10 +- sdk/feature-set/src/lib.rs | 5 + 7 files changed, 262 insertions(+), 46 deletions(-) diff --git a/core/tests/epoch_accounts_hash.rs b/core/tests/epoch_accounts_hash.rs index c94fef0e3ffe99..ae1a52f6921955 100755 --- a/core/tests/epoch_accounts_hash.rs +++ b/core/tests/epoch_accounts_hash.rs @@ -30,6 +30,7 @@ use { solana_sdk::{ clock::Slot, epoch_schedule::EpochSchedule, + feature_set, native_token::LAMPORTS_PER_SOL, pubkey::Pubkey, signature::{Keypair, Signer}, @@ -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 diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 1242d125b475ae..c055c2cf1e09d2 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -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 @@ -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() { @@ -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 @@ -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 } @@ -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) { diff --git a/runtime/src/bank/accounts_lt_hash.rs b/runtime/src/bank/accounts_lt_hash.rs index dc97b12690fbed..63f3ef27ae185e 100644 --- a/runtime/src/bank/accounts_lt_hash.rs +++ b/runtime/src/bank/accounts_lt_hash.rs @@ -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, @@ -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 @@ -312,8 +316,11 @@ 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::{ @@ -321,6 +328,7 @@ mod tests { }, solana_sdk::{ account::{ReadableAccount as _, WritableAccount as _}, + feature::{self, Feature}, fee_calculator::FeeRateGovernor, genesis_config::{self, GenesisConfig}, native_token::LAMPORTS_PER_SOL, @@ -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()); @@ -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()); @@ -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()); @@ -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()); @@ -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()); @@ -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()); @@ -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); + } } diff --git a/runtime/src/bank/epoch_accounts_hash_utils.rs b/runtime/src/bank/epoch_accounts_hash_utils.rs index 40e30761508f79..dde0403c2fb92c 100644 --- a/runtime/src/bank/epoch_accounts_hash_utils.rs +++ b/runtime/src/bank/epoch_accounts_hash_utils.rs @@ -4,6 +4,7 @@ use { crate::bank::Bank, solana_sdk::{ clock::{Epoch, Slot}, + feature_set, vote::state::MAX_LOCKOUT_HISTORY, }, }; @@ -11,6 +12,14 @@ use { /// Is the EAH enabled this Epoch? #[must_use] pub fn is_enabled_this_epoch(bank: &Bank) -> bool { + // If the accounts lt hash feature is enabled, then the EAH is disabled. + if bank + .feature_set + .is_active(&feature_set::accounts_lt_hash::id()) + { + return false; + } + // The EAH calculation "start" is based on when a bank is *rooted*, and "stop" is based on when a // bank is *frozen*. Banks are rooted after exceeding the maximum lockout, so there is a delay // of at least `maximum lockout` number of slots the EAH calculation must take into diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index 4becb2b4b2dc98..5e29644cf17d11 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -1740,7 +1740,7 @@ fn test_rent_eager_collect_rent_in_partition(should_collect_rent: bool) { ); } -fn new_from_parent_next_epoch( +pub(in crate::bank) fn new_from_parent_next_epoch( parent: Arc, bank_forks: &RwLock, epochs: Epoch, @@ -3431,20 +3431,17 @@ fn test_bank_parent_account_spend() { #[test_case(false; "accounts lt hash disabled")] #[test_case(true; "accounts lt hash enabled")] fn test_bank_hash_internal_state(is_accounts_lt_hash_enabled: bool) { - let (genesis_config, mint_keypair) = + let (mut genesis_config, mint_keypair) = create_genesis_config_no_tx_fee_no_rent(sol_to_lamports(1.)); + if !is_accounts_lt_hash_enabled { + // Disable the accounts lt hash feature by removing its account from genesis. + genesis_config + .accounts + .remove(&feature_set::accounts_lt_hash::id()) + .unwrap(); + } let (bank0, _bank_forks0) = Bank::new_with_bank_forks_for_tests(&genesis_config); let (bank1, bank_forks1) = Bank::new_with_bank_forks_for_tests(&genesis_config); - bank0 - .rc - .accounts - .accounts_db - .set_is_experimental_accumulator_hash_enabled(is_accounts_lt_hash_enabled); - bank1 - .rc - .accounts - .accounts_db - .set_is_experimental_accumulator_hash_enabled(is_accounts_lt_hash_enabled); assert_eq!( bank0.is_accounts_lt_hash_enabled(), is_accounts_lt_hash_enabled, @@ -3485,14 +3482,16 @@ fn test_bank_hash_internal_state(is_accounts_lt_hash_enabled: bool) { #[test_case(true; "accounts lt hash enabled")] fn test_bank_hash_internal_state_verify(is_accounts_lt_hash_enabled: bool) { for pass in 0..4 { - let (genesis_config, mint_keypair) = + let (mut genesis_config, mint_keypair) = create_genesis_config_no_tx_fee_no_rent(sol_to_lamports(1.)); + if !is_accounts_lt_hash_enabled { + // Disable the accounts lt hash feature by removing its account from genesis. + genesis_config + .accounts + .remove(&feature_set::accounts_lt_hash::id()) + .unwrap(); + } let (bank0, bank_forks) = Bank::new_with_bank_forks_for_tests(&genesis_config); - bank0 - .rc - .accounts - .accounts_db - .set_is_experimental_accumulator_hash_enabled(is_accounts_lt_hash_enabled); assert_eq!( bank0.is_accounts_lt_hash_enabled(), is_accounts_lt_hash_enabled, diff --git a/runtime/src/snapshot_bank_utils.rs b/runtime/src/snapshot_bank_utils.rs index 4d90329a785191..c2606f0fad6b6d 100644 --- a/runtime/src/snapshot_bank_utils.rs +++ b/runtime/src/snapshot_bank_utils.rs @@ -1063,6 +1063,7 @@ mod tests { sorted_storages::SortedStorages, }, solana_sdk::{ + feature_set, genesis_config::create_genesis_config, native_token::{sol_to_lamports, LAMPORTS_PER_SOL}, signature::{Keypair, Signer}, @@ -1962,11 +1963,18 @@ mod tests { let full_snapshot_archives_dir = tempfile::TempDir::new().unwrap(); let incremental_snapshot_archives_dir = tempfile::TempDir::new().unwrap(); - let genesis_config_info = genesis_utils::create_genesis_config_with_leader( + let mut genesis_config_info = genesis_utils::create_genesis_config_with_leader( 1_000_000 * LAMPORTS_PER_SOL, &Pubkey::new_unique(), 100 * LAMPORTS_PER_SOL, ); + // When the accounts lt hash feature is enabled, the incremental accounts hash is bypassed. + // 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 mint = &genesis_config_info.mint_keypair; let do_transfers = |bank: &Bank| { diff --git a/sdk/feature-set/src/lib.rs b/sdk/feature-set/src/lib.rs index 600f63ea90ed6a..a66da8b3c0b210 100644 --- a/sdk/feature-set/src/lib.rs +++ b/sdk/feature-set/src/lib.rs @@ -880,6 +880,10 @@ pub mod disable_account_loader_special_case { solana_pubkey::declare_id!("EQUMpNFr7Nacb1sva56xn1aLfBxppEoSBH8RRVdkcD1x"); } +pub mod accounts_lt_hash { + solana_pubkey::declare_id!("LtHaSHHsUge7EWTPVrmpuexKz6uVHZXZL6cgJa7W7Zn"); +} + lazy_static! { /// Map of feature identifiers to user-visible description pub static ref FEATURE_NAMES: HashMap = [ @@ -1095,6 +1099,7 @@ lazy_static! { (remove_accounts_executable_flag_checks::id(), "Remove checks of accounts is_executable flag SIMD-0162"), (lift_cpi_caller_restriction::id(), "Lift the restriction in CPI that the caller must have the callee as an instruction account #2202"), (disable_account_loader_special_case::id(), "Disable account loader special case #3513"), + (accounts_lt_hash::id(), "enables lattice-based accounts hash #3333"), /*************** ADD NEW FEATURES HERE ***************/ ] .iter()