From 5a95e5676ee322276b974014c63804152eba5df9 Mon Sep 17 00:00:00 2001 From: Andrew Fitzgerald Date: Wed, 4 Oct 2023 08:04:43 -0700 Subject: [PATCH] Manually add lookup table addresses instead of sanitizing (#33273) --- ledger-tool/src/main.rs | 18 +++-- ledger/src/blockstore.rs | 67 +++++++++++++------ ledger/src/lib.rs | 1 + ...ransaction_address_lookup_table_scanner.rs | 44 ++++++++++++ sdk/program/src/lib.rs | 9 ++- 5 files changed, 110 insertions(+), 29 deletions(-) create mode 100644 ledger/src/transaction_address_lookup_table_scanner.rs diff --git a/ledger-tool/src/main.rs b/ledger-tool/src/main.rs index 2fa265284..1cce5ad27 100644 --- a/ledger-tool/src/main.rs +++ b/ledger-tool/src/main.rs @@ -1024,13 +1024,14 @@ fn get_latest_optimistic_slots( /// Finds the accounts needed to replay slots `snapshot_slot` to `ending_slot`. /// Removes all other accounts from accounts_db, and updates the accounts hash /// and capitalization. This is used by the --minimize option in create-snapshot +/// Returns true if the minimized snapshot may be incomplete. fn minimize_bank_for_snapshot( blockstore: &Blockstore, bank: &Bank, snapshot_slot: Slot, ending_slot: Slot, -) { - let (transaction_account_set, transaction_accounts_measure) = measure!( +) -> bool { + let ((transaction_account_set, possibly_incomplete), transaction_accounts_measure) = measure!( blockstore.get_accounts_used_in_range(bank, snapshot_slot, ending_slot), "get transaction accounts" ); @@ -1038,6 +1039,7 @@ fn minimize_bank_for_snapshot( info!("Added {total_accounts_len} accounts from transactions. {transaction_accounts_measure}"); SnapshotMinimizer::minimize(bank, snapshot_slot, ending_slot, transaction_account_set); + possibly_incomplete } fn assert_capitalization(bank: &Bank) { @@ -3158,14 +3160,16 @@ fn main() { bank }; - if is_minimized { + let minimize_snapshot_possibly_incomplete = if is_minimized { minimize_bank_for_snapshot( &blockstore, &bank, snapshot_slot, ending_slot.unwrap(), - ); - } + ) + } else { + false + }; println!( "Creating a version {} {}snapshot of slot {}", @@ -3245,6 +3249,10 @@ fn main() { warn!("Minimized snapshot range crosses epoch boundary ({} to {}). Bank hashes after {} will not match replays from a full snapshot", starting_epoch, ending_epoch, bank.epoch_schedule().get_last_slot_in_epoch(starting_epoch)); } + + if minimize_snapshot_possibly_incomplete { + warn!("Minimized snapshot may be incomplete due to missing accounts from CPI'd address lookup table extensions. This may lead to mismatched bank hashes while replaying."); + } } } diff --git a/ledger/src/blockstore.rs b/ledger/src/blockstore.rs index 93de196cc..0e8709c01 100644 --- a/ledger/src/blockstore.rs +++ b/ledger/src/blockstore.rs @@ -21,6 +21,7 @@ use { Shred, ShredData, ShredId, ShredType, Shredder, }, slot_stats::{ShredSource, SlotsStats}, + transaction_address_lookup_table_scanner::scan_transaction, }, assert_matches::debug_assert_matches, bincode::{deserialize, serialize}, @@ -44,13 +45,15 @@ use { solana_rayon_threadlimit::get_max_thread_count, solana_runtime::bank::Bank, solana_sdk::{ + account::ReadableAccount, + address_lookup_table::state::AddressLookupTable, clock::{Slot, UnixTimestamp, DEFAULT_TICKS_PER_SECOND}, genesis_config::{GenesisConfig, DEFAULT_GENESIS_ARCHIVE, DEFAULT_GENESIS_FILE}, hash::Hash, pubkey::Pubkey, signature::{Keypair, Signature, Signer}, timing::timestamp, - transaction::VersionedTransaction, + transaction::{SanitizedVersionedTransaction, VersionedTransaction}, }, solana_storage_proto::{StoredExtendedRewards, StoredTransactionStatusMeta}, solana_transaction_status::{ @@ -2930,14 +2933,23 @@ impl Blockstore { } /// Gets accounts used in transactions in the slot range [starting_slot, ending_slot]. + /// Additionally returns a bool indicating if the set may be incomplete. /// Used by ledger-tool to create a minimized snapshot pub fn get_accounts_used_in_range( &self, bank: &Bank, starting_slot: Slot, ending_slot: Slot, - ) -> DashSet { + ) -> (DashSet, bool) { let result = DashSet::new(); + let lookup_tables = DashSet::new(); + let possible_cpi_alt_extend = AtomicBool::new(false); + + fn add_to_set<'a>(set: &DashSet, iter: impl IntoIterator) { + iter.into_iter().for_each(|key| { + set.insert(*key); + }); + } (starting_slot..=ending_slot) .into_par_iter() @@ -2945,31 +2957,44 @@ impl Blockstore { if let Ok(entries) = self.get_slot_entries(slot, 0) { entries.into_par_iter().for_each(|entry| { entry.transactions.into_iter().for_each(|tx| { - if let Some(lookups) = tx.message.address_table_lookups() { - lookups.iter().for_each(|lookup| { - result.insert(lookup.account_key); - }); + // Attempt to verify transaction and load addresses from the current bank, + // or manually scan the transaction for addresses if the transaction. + if let Ok(tx) = bank.fully_verify_transaction(tx.clone()) { + add_to_set(&result, tx.message().account_keys().iter()); + } else { + add_to_set(&result, tx.message.static_account_keys()); + if let Some(lookups) = tx.message.address_table_lookups() { + add_to_set( + &lookup_tables, + lookups.iter().map(|lookup| &lookup.account_key), + ); + } + + let tx = SanitizedVersionedTransaction::try_from(tx) + .expect("transaction failed to sanitize"); + + let alt_scan_extensions = scan_transaction(&tx); + add_to_set(&result, &alt_scan_extensions.accounts); + if alt_scan_extensions.possibly_incomplete { + possible_cpi_alt_extend.store(true, Ordering::Relaxed); + } } - // howdy, anybody who reached here from the panic messsage! - // the .unwrap() below could indicate there was an odd error or there - // could simply be a tx with a new ALT, which is just created/updated - // in this range. too bad... this edge case isn't currently supported. - // see: https://github.com/solana-labs/solana/issues/30165 - // for casual use, please choose different slot range. - let sanitized_tx = bank.fully_verify_transaction(tx).unwrap(); - sanitized_tx - .message() - .account_keys() - .iter() - .for_each(|&pubkey| { - result.insert(pubkey); - }); }); }); } }); - result + // For each unique lookup table add all accounts to the minimized set. + lookup_tables.into_par_iter().for_each(|lookup_table_key| { + bank.get_account(&lookup_table_key) + .map(|lookup_table_account| { + AddressLookupTable::deserialize(lookup_table_account.data()).map(|t| { + add_to_set(&result, &t.addresses[..]); + }) + }); + }); + + (result, possible_cpi_alt_extend.into_inner()) } fn get_completed_ranges( diff --git a/ledger/src/lib.rs b/ledger/src/lib.rs index a8f81be48..0f311ca12 100644 --- a/ledger/src/lib.rs +++ b/ledger/src/lib.rs @@ -28,6 +28,7 @@ pub mod sigverify_shreds; pub mod slot_stats; mod staking_utils; pub mod token_balances; +mod transaction_address_lookup_table_scanner; pub mod use_snapshot_archives_at_startup; #[macro_use] diff --git a/ledger/src/transaction_address_lookup_table_scanner.rs b/ledger/src/transaction_address_lookup_table_scanner.rs new file mode 100644 index 000000000..357a9c6e4 --- /dev/null +++ b/ledger/src/transaction_address_lookup_table_scanner.rs @@ -0,0 +1,44 @@ +use { + bincode::deserialize, + lazy_static::lazy_static, + solana_sdk::{ + address_lookup_table::{self, instruction::ProgramInstruction}, + pubkey::Pubkey, + sdk_ids::SDK_IDS, + transaction::SanitizedVersionedTransaction, + }, + std::collections::HashSet, +}; + +lazy_static! { + static ref SDK_IDS_SET: HashSet = SDK_IDS.iter().cloned().collect(); +} + +pub struct ScannedLookupTableExtensions { + pub possibly_incomplete: bool, + pub accounts: Vec, // empty if no extensions found +} + +pub fn scan_transaction( + transaction: &SanitizedVersionedTransaction, +) -> ScannedLookupTableExtensions { + // Accumulate accounts from account lookup table extension instructions + let mut accounts = Vec::new(); + let mut native_only = true; + for (program_id, instruction) in transaction.get_message().program_instructions_iter() { + if address_lookup_table::program::check_id(program_id) { + if let Ok(ProgramInstruction::ExtendLookupTable { new_addresses }) = + deserialize::(&instruction.data) + { + accounts.extend(new_addresses); + } + } else { + native_only &= SDK_IDS_SET.contains(program_id); + } + } + + ScannedLookupTableExtensions { + possibly_incomplete: !native_only, + accounts, + } +} diff --git a/sdk/program/src/lib.rs b/sdk/program/src/lib.rs index edcc2e3cb..0dfc8c324 100644 --- a/sdk/program/src/lib.rs +++ b/sdk/program/src/lib.rs @@ -564,9 +564,9 @@ pub mod config { pub mod sdk_ids { use { crate::{ - bpf_loader, bpf_loader_deprecated, bpf_loader_upgradeable, config, ed25519_program, - feature, incinerator, secp256k1_program, solana_program::pubkey::Pubkey, stake, - system_program, sysvar, vote, + address_lookup_table, bpf_loader, bpf_loader_deprecated, bpf_loader_upgradeable, + config, ed25519_program, feature, incinerator, loader_v4, secp256k1_program, + solana_program::pubkey::Pubkey, stake, system_program, sysvar, vote, }, lazy_static::lazy_static, }; @@ -585,6 +585,9 @@ pub mod sdk_ids { vote::program::id(), feature::id(), bpf_loader_deprecated::id(), + address_lookup_table::program::id(), + loader_v4::id(), + stake::program::id(), #[allow(deprecated)] stake::config::id(), ];