diff --git a/accountsdb-plugin-postgres/scripts/create_schema.sql b/accountsdb-plugin-postgres/scripts/create_schema.sql index 994a2176cfb178..4863e54b7e1274 100644 --- a/accountsdb-plugin-postgres/scripts/create_schema.sql +++ b/accountsdb-plugin-postgres/scripts/create_schema.sql @@ -48,7 +48,11 @@ Create TYPE "TransactionErrorCode" AS ENUM ( 'WouldExceedMaxBlockCostLimit', 'UnsupportedVersion', 'InvalidWritableAccount', - 'WouldExceedMaxAccountDataCostLimit' + 'WouldExceedMaxAccountDataCostLimit', + 'AddressLookupTableNotFound', + 'InvalidAddressLookupTableOwner', + 'InvalidAddressLookupTableData', + 'InvalidAddressLookupTableIndex', ); CREATE TYPE "TransactionError" AS ( diff --git a/accountsdb-plugin-postgres/src/postgres_client/postgres_client_transaction.rs b/accountsdb-plugin-postgres/src/postgres_client/postgres_client_transaction.rs index b2019f70f50211..76fe128c69f378 100644 --- a/accountsdb-plugin-postgres/src/postgres_client/postgres_client_transaction.rs +++ b/accountsdb-plugin-postgres/src/postgres_client/postgres_client_transaction.rs @@ -331,6 +331,10 @@ pub enum DbTransactionErrorCode { UnsupportedVersion, InvalidWritableAccount, WouldExceedMaxAccountDataCostLimit, + AddressLookupTableNotFound, + InvalidAddressLookupTableOwner, + InvalidAddressLookupTableData, + InvalidAddressLookupTableIndex, } impl From<&TransactionError> for DbTransactionErrorCode { @@ -362,6 +366,14 @@ impl From<&TransactionError> for DbTransactionErrorCode { TransactionError::WouldExceedMaxAccountDataCostLimit => { Self::WouldExceedMaxAccountDataCostLimit } + TransactionError::AddressLookupTableNotFound => Self::AddressLookupTableNotFound, + TransactionError::InvalidAddressLookupTableOwner => { + Self::InvalidAddressLookupTableOwner + } + TransactionError::InvalidAddressLookupTableData => Self::InvalidAddressLookupTableData, + TransactionError::InvalidAddressLookupTableIndex => { + Self::InvalidAddressLookupTableIndex + } } } } diff --git a/core/src/banking_stage.rs b/core/src/banking_stage.rs index 343b13354bea18..1af0c4b67585d6 100644 --- a/core/src/banking_stage.rs +++ b/core/src/banking_stage.rs @@ -35,7 +35,10 @@ use { MAX_TRANSACTION_FORWARDING_DELAY_GPU, }, feature_set, - message::Message, + message::{ + v0::{LoadedAddresses, MessageAddressTableLookup}, + Message, + }, pubkey::Pubkey, short_vec::decode_shortu16_len, signature::Signature, @@ -1120,6 +1123,7 @@ impl BankingStage { transaction_indexes: &[usize], feature_set: &Arc, votes_only: bool, + address_loader: impl Fn(&[MessageAddressTableLookup]) -> transaction::Result, ) -> (Vec, Vec) { transaction_indexes .iter() @@ -1136,7 +1140,7 @@ impl BankingStage { tx, message_hash, Some(p.meta.is_simple_vote_tx), - |_| Err(TransactionError::UnsupportedVersion), + &address_loader, ) .ok()?; tx.verify_precompiles(feature_set).ok()?; @@ -1202,6 +1206,7 @@ impl BankingStage { &packet_indexes, &bank.feature_set, bank.vote_only_bank(), + |lookup| bank.load_lookup_table_addresses(lookup), ); packet_conversion_time.stop(); inc_new_counter_info!("banking_stage-packet_conversion", 1); @@ -1276,6 +1281,7 @@ impl BankingStage { transaction_indexes, &bank.feature_set, bank.vote_only_bank(), + |lookup| bank.load_lookup_table_addresses(lookup), ); unprocessed_packet_conversion_time.stop(); @@ -3117,6 +3123,7 @@ mod tests { &packet_indexes, &Arc::new(FeatureSet::default()), votes_only, + |_| Err(TransactionError::UnsupportedVersion), ); assert_eq!(2, txs.len()); assert_eq!(vec![0, 1], tx_packet_index); @@ -3127,6 +3134,7 @@ mod tests { &packet_indexes, &Arc::new(FeatureSet::default()), votes_only, + |_| Err(TransactionError::UnsupportedVersion), ); assert_eq!(0, txs.len()); assert_eq!(0, tx_packet_index.len()); @@ -3146,6 +3154,7 @@ mod tests { &packet_indexes, &Arc::new(FeatureSet::default()), votes_only, + |_| Err(TransactionError::UnsupportedVersion), ); assert_eq!(3, txs.len()); assert_eq!(vec![0, 1, 2], tx_packet_index); @@ -3156,6 +3165,7 @@ mod tests { &packet_indexes, &Arc::new(FeatureSet::default()), votes_only, + |_| Err(TransactionError::UnsupportedVersion), ); assert_eq!(2, txs.len()); assert_eq!(vec![0, 2], tx_packet_index); @@ -3175,6 +3185,7 @@ mod tests { &packet_indexes, &Arc::new(FeatureSet::default()), votes_only, + |_| Err(TransactionError::UnsupportedVersion), ); assert_eq!(3, txs.len()); assert_eq!(vec![0, 1, 2], tx_packet_index); @@ -3185,6 +3196,7 @@ mod tests { &packet_indexes, &Arc::new(FeatureSet::default()), votes_only, + |_| Err(TransactionError::UnsupportedVersion), ); assert_eq!(3, txs.len()); assert_eq!(vec![0, 1, 2], tx_packet_index); diff --git a/ledger/src/blockstore/blockstore_purge.rs b/ledger/src/blockstore/blockstore_purge.rs index 15ab065ac24071..0779e27b042891 100644 --- a/ledger/src/blockstore/blockstore_purge.rs +++ b/ledger/src/blockstore/blockstore_purge.rs @@ -335,8 +335,8 @@ impl Blockstore { if let Some(&signature) = transaction.signatures.get(0) { batch.delete::((0, signature, slot))?; batch.delete::((1, signature, slot))?; - // TODO: support purging mapped addresses from versioned transactions - for pubkey in transaction.message.unmapped_keys() { + // TODO: support purging dynamically loaded addresses from versioned transactions + for pubkey in transaction.message.into_static_account_keys() { batch.delete::((0, pubkey, slot, signature))?; batch.delete::((1, pubkey, slot, signature))?; } diff --git a/programs/address-lookup-table/src/state.rs b/programs/address-lookup-table/src/state.rs index 8bf7fc3457a8ed..b86173b1a2324c 100644 --- a/programs/address-lookup-table/src/state.rs +++ b/programs/address-lookup-table/src/state.rs @@ -6,6 +6,7 @@ use { instruction::InstructionError, pubkey::Pubkey, slot_hashes::{SlotHashes, MAX_ENTRIES}, + transaction::AddressLookupError, }, std::borrow::Cow, }; @@ -133,6 +134,39 @@ impl<'a> AddressLookupTable<'a> { Ok(()) } + /// Lookup addresses for provided table indexes. Since lookups are performed on + /// tables which are not read-locked, this implementation needs to be careful + /// about resolving addresses consistently. + pub fn lookup( + &self, + current_slot: Slot, + indexes: &[u8], + slot_hashes: &SlotHashes, + ) -> Result, AddressLookupError> { + if !self.meta.is_active(current_slot, slot_hashes) { + // Once a lookup table is no longer active, it can be closed + // at any point, so returning a specific error for deactivated + // lookup tables could result in a race condition. + return Err(AddressLookupError::LookupTableAccountNotFound); + } + + // If the address table was extended in the same slot in which it is used + // to lookup addresses for another transaction, the recently extended + // addresses are not considered active and won't be accessible. + let active_addresses_len = if current_slot > self.meta.last_extended_slot { + self.addresses.len() + } else { + self.meta.last_extended_slot_start_index as usize + }; + + let active_addresses = &self.addresses[0..active_addresses_len]; + indexes + .iter() + .map(|idx| active_addresses.get(*idx as usize).cloned()) + .collect::>() + .ok_or(AddressLookupError::InvalidLookupIndex) + } + /// Serialize an address table including its addresses pub fn serialize_for_tests(self, data: &mut Vec) -> Result<(), InstructionError> { data.resize(LOOKUP_TABLE_META_SIZE, 0); @@ -322,4 +356,117 @@ mod tests { test_case(case); } } + + #[test] + fn test_lookup_from_empty_table() { + let lookup_table = AddressLookupTable { + meta: LookupTableMeta::default(), + addresses: Cow::Owned(vec![]), + }; + + assert_eq!( + lookup_table.lookup(0, &[], &SlotHashes::default()), + Ok(vec![]) + ); + assert_eq!( + lookup_table.lookup(0, &[0], &SlotHashes::default()), + Err(AddressLookupError::InvalidLookupIndex) + ); + } + + #[test] + fn test_lookup_from_deactivating_table() { + let current_slot = 1; + let slot_hashes = SlotHashes::default(); + let addresses = vec![Pubkey::new_unique()]; + let lookup_table = AddressLookupTable { + meta: LookupTableMeta { + deactivation_slot: current_slot, + last_extended_slot: current_slot - 1, + ..LookupTableMeta::default() + }, + addresses: Cow::Owned(addresses.clone()), + }; + + assert_eq!( + lookup_table.meta.status(current_slot, &slot_hashes), + LookupTableStatus::Deactivating { + remaining_blocks: MAX_ENTRIES + 1 + } + ); + + assert_eq!( + lookup_table.lookup(current_slot, &[0], &slot_hashes), + Ok(vec![addresses[0]]), + ); + } + + #[test] + fn test_lookup_from_deactivated_table() { + let current_slot = 1; + let slot_hashes = SlotHashes::default(); + let lookup_table = AddressLookupTable { + meta: LookupTableMeta { + deactivation_slot: current_slot - 1, + last_extended_slot: current_slot - 1, + ..LookupTableMeta::default() + }, + addresses: Cow::Owned(vec![]), + }; + + assert_eq!( + lookup_table.meta.status(current_slot, &slot_hashes), + LookupTableStatus::Deactivated + ); + assert_eq!( + lookup_table.lookup(current_slot, &[0], &slot_hashes), + Err(AddressLookupError::LookupTableAccountNotFound) + ); + } + + #[test] + fn test_lookup_from_table_extended_in_current_slot() { + let current_slot = 0; + let addresses: Vec<_> = (0..2).map(|_| Pubkey::new_unique()).collect(); + let lookup_table = AddressLookupTable { + meta: LookupTableMeta { + last_extended_slot: current_slot, + last_extended_slot_start_index: 1, + ..LookupTableMeta::default() + }, + addresses: Cow::Owned(addresses.clone()), + }; + + assert_eq!( + lookup_table.lookup(current_slot, &[0], &SlotHashes::default()), + Ok(vec![addresses[0]]) + ); + assert_eq!( + lookup_table.lookup(current_slot, &[1], &SlotHashes::default()), + Err(AddressLookupError::InvalidLookupIndex), + ); + } + + #[test] + fn test_lookup_from_table_extended_in_previous_slot() { + let current_slot = 1; + let addresses: Vec<_> = (0..10).map(|_| Pubkey::new_unique()).collect(); + let lookup_table = AddressLookupTable { + meta: LookupTableMeta { + last_extended_slot: current_slot - 1, + last_extended_slot_start_index: 1, + ..LookupTableMeta::default() + }, + addresses: Cow::Owned(addresses.clone()), + }; + + assert_eq!( + lookup_table.lookup(current_slot, &[0, 3, 1, 5], &SlotHashes::default()), + Ok(vec![addresses[0], addresses[3], addresses[1], addresses[5]]) + ); + assert_eq!( + lookup_table.lookup(current_slot, &[10], &SlotHashes::default()), + Err(AddressLookupError::InvalidLookupIndex), + ); + } } diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs index 9b562b36b82d80..560e8d390b7f1f 100644 --- a/runtime/src/accounts.rs +++ b/runtime/src/accounts.rs @@ -22,6 +22,7 @@ use { }, log::*, rand::{thread_rng, Rng}, + solana_address_lookup_table_program::state::AddressLookupTable, solana_sdk::{ account::{Account, AccountSharedData, ReadableAccount, WritableAccount}, account_utils::StateMut, @@ -30,13 +31,17 @@ use { feature_set::{self, FeatureSet}, genesis_config::ClusterType, hash::Hash, - message::SanitizedMessage, + message::{ + v0::{LoadedAddresses, MessageAddressTableLookup}, + SanitizedMessage, + }, native_loader, nonce::{state::Versions as NonceVersions, State as NonceState}, pubkey::Pubkey, + slot_hashes::SlotHashes, system_program, sysvar::{self, instructions::construct_instructions_data}, - transaction::{Result, SanitizedTransaction, TransactionError}, + transaction::{AddressLookupError, Result, SanitizedTransaction, TransactionError}, transaction_context::TransactionAccount, }, std::{ @@ -514,6 +519,40 @@ impl Accounts { .collect() } + pub fn load_lookup_table_addresses( + &self, + ancestors: &Ancestors, + address_table_lookup: &MessageAddressTableLookup, + slot_hashes: &SlotHashes, + ) -> std::result::Result { + let table_account = self + .accounts_db + .load_with_fixed_root(ancestors, &address_table_lookup.account_key) + .map(|(account, _rent)| account) + .ok_or(AddressLookupError::LookupTableAccountNotFound)?; + + if table_account.owner() == &solana_address_lookup_table_program::id() { + let current_slot = ancestors.max_slot(); + let lookup_table = AddressLookupTable::deserialize(table_account.data()) + .map_err(|_ix_err| AddressLookupError::InvalidAccountData)?; + + Ok(LoadedAddresses { + writable: lookup_table.lookup( + current_slot, + &address_table_lookup.writable_indexes, + slot_hashes, + )?, + readonly: lookup_table.lookup( + current_slot, + &address_table_lookup.readonly_indexes, + slot_hashes, + )?, + }) + } else { + Err(AddressLookupError::InvalidAccountOwner) + } + } + fn filter_zero_lamport_account( account: AccountSharedData, slot: Slot, @@ -1243,6 +1282,7 @@ mod tests { use { super::*, crate::rent_collector::RentCollector, + solana_address_lookup_table_program::state::LookupTableMeta, solana_sdk::{ account::{AccountSharedData, WritableAccount}, epoch_schedule::EpochSchedule, @@ -1257,6 +1297,7 @@ mod tests { transaction::Transaction, }, std::{ + borrow::Cow, convert::TryFrom, sync::atomic::{AtomicBool, AtomicU64, Ordering}, thread, time, @@ -1829,6 +1870,149 @@ mod tests { } } + #[test] + fn test_load_lookup_table_addresses_account_not_found() { + let ancestors = vec![(0, 0)].into_iter().collect(); + let accounts = Accounts::new_with_config_for_tests( + Vec::new(), + &ClusterType::Development, + AccountSecondaryIndexes::default(), + false, + AccountShrinkThreshold::default(), + ); + + let invalid_table_key = Pubkey::new_unique(); + let address_table_lookup = MessageAddressTableLookup { + account_key: invalid_table_key, + writable_indexes: vec![], + readonly_indexes: vec![], + }; + + assert_eq!( + accounts.load_lookup_table_addresses( + &ancestors, + &address_table_lookup, + &SlotHashes::default(), + ), + Err(AddressLookupError::LookupTableAccountNotFound), + ); + } + + #[test] + fn test_load_lookup_table_addresses_invalid_account_owner() { + let ancestors = vec![(0, 0)].into_iter().collect(); + let accounts = Accounts::new_with_config_for_tests( + Vec::new(), + &ClusterType::Development, + AccountSecondaryIndexes::default(), + false, + AccountShrinkThreshold::default(), + ); + + let invalid_table_key = Pubkey::new_unique(); + let invalid_table_account = AccountSharedData::default(); + accounts.store_slow_uncached(0, &invalid_table_key, &invalid_table_account); + + let address_table_lookup = MessageAddressTableLookup { + account_key: invalid_table_key, + writable_indexes: vec![], + readonly_indexes: vec![], + }; + + assert_eq!( + accounts.load_lookup_table_addresses( + &ancestors, + &address_table_lookup, + &SlotHashes::default(), + ), + Err(AddressLookupError::InvalidAccountOwner), + ); + } + + #[test] + fn test_load_lookup_table_addresses_invalid_account_data() { + let ancestors = vec![(0, 0)].into_iter().collect(); + let accounts = Accounts::new_with_config_for_tests( + Vec::new(), + &ClusterType::Development, + AccountSecondaryIndexes::default(), + false, + AccountShrinkThreshold::default(), + ); + + let invalid_table_key = Pubkey::new_unique(); + let invalid_table_account = + AccountSharedData::new(1, 0, &solana_address_lookup_table_program::id()); + accounts.store_slow_uncached(0, &invalid_table_key, &invalid_table_account); + + let address_table_lookup = MessageAddressTableLookup { + account_key: invalid_table_key, + writable_indexes: vec![], + readonly_indexes: vec![], + }; + + assert_eq!( + accounts.load_lookup_table_addresses( + &ancestors, + &address_table_lookup, + &SlotHashes::default(), + ), + Err(AddressLookupError::InvalidAccountData), + ); + } + + #[test] + fn test_load_lookup_table_addresses() { + let ancestors = vec![(1, 1), (0, 0)].into_iter().collect(); + let accounts = Accounts::new_with_config_for_tests( + Vec::new(), + &ClusterType::Development, + AccountSecondaryIndexes::default(), + false, + AccountShrinkThreshold::default(), + ); + + let table_key = Pubkey::new_unique(); + let table_addresses = vec![Pubkey::new_unique(), Pubkey::new_unique()]; + let table_account = { + let table_state = AddressLookupTable { + meta: LookupTableMeta::default(), + addresses: Cow::Owned(table_addresses.clone()), + }; + let table_data = { + let mut data = vec![]; + table_state.serialize_for_tests(&mut data).unwrap(); + data + }; + AccountSharedData::create( + 1, + table_data, + solana_address_lookup_table_program::id(), + false, + 0, + ) + }; + accounts.store_slow_uncached(0, &table_key, &table_account); + + let address_table_lookup = MessageAddressTableLookup { + account_key: table_key, + writable_indexes: vec![0], + readonly_indexes: vec![1], + }; + + assert_eq!( + accounts.load_lookup_table_addresses( + &ancestors, + &address_table_lookup, + &SlotHashes::default(), + ), + Ok(LoadedAddresses { + writable: vec![table_addresses[0]], + readonly: vec![table_addresses[1]], + }), + ); + } + #[test] fn test_load_by_program_slot() { let accounts = Accounts::new_with_config_for_tests( diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index d7ac6745658ce0..24f994a5fd1318 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -105,7 +105,10 @@ use { inflation::Inflation, instruction::CompiledInstruction, lamports::LamportsError, - message::SanitizedMessage, + message::{ + v0::{LoadedAddresses, MessageAddressTableLookup}, + SanitizedMessage, + }, native_loader, native_token::sol_to_lamports, nonce, nonce_account, @@ -121,7 +124,7 @@ use { sysvar::{self, Sysvar, SysvarId}, timing::years_as_slots, transaction::{ - Result, SanitizedTransaction, Transaction, TransactionError, + AddressLookupError, Result, SanitizedTransaction, Transaction, TransactionError, TransactionVerificationMode, VersionedTransaction, }, transaction_context::{TransactionAccount, TransactionContext}, @@ -238,7 +241,7 @@ impl ExecuteTimings { } type BankStatusCache = StatusCache>; -#[frozen_abi(digest = "2pPboTQ9ixNuR1hvRt7McJriam5EHfd3vpBWfxnVbmF3")] +#[frozen_abi(digest = "4vXQj7XbDnGzRbufYMsEi6oQokva2qrEeNLiFBbyVepR")] pub type BankSlotDelta = SlotDelta>; // Eager rent collection repeats in cyclic manner. @@ -3464,6 +3467,44 @@ impl Bank { cache.remove(pubkey); } + /// Get the value of a cached sysvar by its id + pub fn get_cached_sysvar(&self, id: &Pubkey) -> Option { + self.sysvar_cache + .read() + .unwrap() + .iter() + .find_map(|(key, data)| { + if id == key { + bincode::deserialize(data).ok() + } else { + None + } + }) + } + + pub fn load_lookup_table_addresses( + &self, + address_table_lookups: &[MessageAddressTableLookup], + ) -> Result { + if !self.versioned_tx_message_enabled() { + return Err(TransactionError::UnsupportedVersion); + } + + let slot_hashes: SlotHashes = self + .get_cached_sysvar(&sysvar::slot_hashes::id()) + .ok_or(TransactionError::AccountNotFound)?; + Ok(address_table_lookups + .iter() + .map(|address_table_lookup| { + self.rc.accounts.load_lookup_table_addresses( + &self.ancestors, + address_table_lookup, + &slot_hashes, + ) + }) + .collect::>()?) + } + #[allow(clippy::type_complexity)] pub fn load_and_execute_transactions( &self, @@ -3583,31 +3624,26 @@ impl Bank { let (blockhash, lamports_per_signature) = self.last_blockhash_and_lamports_per_signature(); - if let Some(legacy_message) = tx.message().legacy_message() { - process_result = MessageProcessor::process_message( - &self.builtin_programs.vec, - legacy_message, - &loaded_transaction.program_indices, - &transaction_context, - self.rent_collector.rent, - log_collector.clone(), - executors.clone(), - instruction_recorder.clone(), - feature_set, - compute_budget, - &mut timings.details, - &*self.sysvar_cache.read().unwrap(), - blockhash, - lamports_per_signature, - self.load_accounts_data_len(), - ) - .map(|process_result| { - self.store_accounts_data_len(process_result.accounts_data_len) - }); - } else { - // TODO: support versioned messages - process_result = Err(TransactionError::UnsupportedVersion); - } + process_result = MessageProcessor::process_message( + &self.builtin_programs.vec, + tx.message(), + &loaded_transaction.program_indices, + &transaction_context, + self.rent_collector.rent, + log_collector.clone(), + executors.clone(), + instruction_recorder.clone(), + feature_set, + compute_budget, + &mut timings.details, + &*self.sysvar_cache.read().unwrap(), + blockhash, + lamports_per_signature, + self.load_accounts_data_len(), + ) + .map(|process_result| { + self.store_accounts_data_len(process_result.accounts_data_len) + }); let log_messages: Option = log_collector.and_then(|log_collector| { @@ -5242,8 +5278,8 @@ impl Bank { tx.message.hash() }; - SanitizedTransaction::try_create(tx, message_hash, None, |_| { - Err(TransactionError::UnsupportedVersion) + SanitizedTransaction::try_create(tx, message_hash, None, |lookups| { + self.load_lookup_table_addresses(lookups) }) }?; diff --git a/runtime/src/message_processor.rs b/runtime/src/message_processor.rs index 96ccebc215cba9..720998d2b2877c 100644 --- a/runtime/src/message_processor.rs +++ b/runtime/src/message_processor.rs @@ -12,7 +12,7 @@ use { compute_budget::ComputeBudget, feature_set::{prevent_calling_precompiles_as_programs, FeatureSet}, hash::Hash, - message::Message, + message::SanitizedMessage, precompiles::is_precompile, pubkey::Pubkey, rent::Rent, @@ -51,7 +51,7 @@ impl MessageProcessor { #[allow(clippy::too_many_arguments)] pub fn process_message( builtin_programs: &[BuiltinProgram], - message: &Message, + message: &SanitizedMessage, program_indices: &[Vec], transaction_context: &TransactionContext, rent: Rent, @@ -81,14 +81,12 @@ impl MessageProcessor { current_accounts_data_len, ); - debug_assert_eq!(program_indices.len(), message.instructions.len()); - for (instruction_index, (instruction, program_indices)) in message - .instructions - .iter() + debug_assert_eq!(program_indices.len(), message.instructions().len()); + for (instruction_index, ((program_id, instruction), program_indices)) in message + .program_instructions_iter() .zip(program_indices.iter()) .enumerate() { - let program_id = instruction.program_id(&message.account_keys); if invoke_context .feature_set .is_active(&prevent_calling_precompiles_as_programs::id()) @@ -138,7 +136,7 @@ impl MessageProcessor { ); time.stop(); timings.accumulate_program( - instruction.program_id(&message.account_keys), + program_id, time.as_us(), compute_units_consumed, result.is_err(), @@ -250,14 +248,14 @@ mod tests { AccountMeta::new_readonly(*transaction_context.get_key_of_account_at_index(1), false), ]; - let message = Message::new( + let message = SanitizedMessage::Legacy(Message::new( &[Instruction::new_with_bincode( mock_system_program_id, &MockSystemInstruction::Correct, account_metas.clone(), )], Some(transaction_context.get_key_of_account_at_index(0)), - ); + )); let result = MessageProcessor::process_message( builtin_programs, &message, @@ -291,14 +289,14 @@ mod tests { 0 ); - let message = Message::new( + let message = SanitizedMessage::Legacy(Message::new( &[Instruction::new_with_bincode( mock_system_program_id, &MockSystemInstruction::AttemptCredit { lamports: 50 }, account_metas.clone(), )], Some(transaction_context.get_key_of_account_at_index(0)), - ); + )); let result = MessageProcessor::process_message( builtin_programs, &message, @@ -324,14 +322,14 @@ mod tests { )) ); - let message = Message::new( + let message = SanitizedMessage::Legacy(Message::new( &[Instruction::new_with_bincode( mock_system_program_id, &MockSystemInstruction::AttemptDataChange { data: 50 }, account_metas, )], Some(transaction_context.get_key_of_account_at_index(0)), - ); + )); let result = MessageProcessor::process_message( builtin_programs, &message, @@ -468,14 +466,14 @@ mod tests { ]; // Try to borrow mut the same account - let message = Message::new( + let message = SanitizedMessage::Legacy(Message::new( &[Instruction::new_with_bincode( mock_program_id, &MockSystemInstruction::BorrowFail, account_metas.clone(), )], Some(transaction_context.get_key_of_account_at_index(0)), - ); + )); let result = MessageProcessor::process_message( builtin_programs, &message, @@ -502,14 +500,14 @@ mod tests { ); // Try to borrow mut the same account in a safe way - let message = Message::new( + let message = SanitizedMessage::Legacy(Message::new( &[Instruction::new_with_bincode( mock_program_id, &MockSystemInstruction::MultiBorrowMut, account_metas.clone(), )], Some(transaction_context.get_key_of_account_at_index(0)), - ); + )); let result = MessageProcessor::process_message( builtin_programs, &message, @@ -530,7 +528,7 @@ mod tests { assert!(result.is_ok()); // Do work on the same account but at different location in keyed_accounts[] - let message = Message::new( + let message = SanitizedMessage::Legacy(Message::new( &[Instruction::new_with_bincode( mock_program_id, &MockSystemInstruction::DoWork { @@ -540,7 +538,7 @@ mod tests { account_metas, )], Some(transaction_context.get_key_of_account_at_index(0)), - ); + )); let result = MessageProcessor::process_message( builtin_programs, &message, @@ -604,7 +602,7 @@ mod tests { ]; let transaction_context = TransactionContext::new(accounts, 1); - let message = Message::new( + let message = SanitizedMessage::Legacy(Message::new( &[ new_secp256k1_instruction( &libsecp256k1::SecretKey::random(&mut rand::thread_rng()), @@ -613,7 +611,7 @@ mod tests { Instruction::new_with_bytes(mock_program_id, &[], vec![]), ], None, - ); + )); let result = MessageProcessor::process_message( builtin_programs, &message, diff --git a/sdk/program/src/message/sanitized.rs b/sdk/program/src/message/sanitized.rs index b09902774cb300..ca8807beb9ab11 100644 --- a/sdk/program/src/message/sanitized.rs +++ b/sdk/program/src/message/sanitized.rs @@ -18,7 +18,7 @@ use { pub enum SanitizedMessage { /// Sanitized legacy message Legacy(LegacyMessage), - /// Sanitized version #0 message with mapped addresses + /// Sanitized version #0 message with dynamically loaded addresses V0(v0::LoadedMessage), } @@ -134,7 +134,7 @@ impl SanitizedMessage { }) } - /// Iterator of all account keys referenced in this message, included mapped keys. + /// Iterator of all account keys referenced in this message, including dynamically loaded keys. pub fn account_keys_iter(&self) -> Box + '_> { match self { Self::Legacy(message) => Box::new(message.account_keys.iter()), @@ -142,7 +142,7 @@ impl SanitizedMessage { } } - /// Length of all account keys referenced in this message, included mapped keys. + /// Length of all account keys referenced in this message, including dynamically loaded keys. pub fn account_keys_len(&self) -> usize { match self { Self::Legacy(message) => message.account_keys.len(), @@ -261,11 +261,11 @@ impl SanitizedMessage { /// Return the number of readonly accounts loaded by this message. pub fn num_readonly_accounts(&self) -> usize { - let mapped_readonly_addresses = self + let loaded_readonly_addresses = self .loaded_lookup_table_addresses() .map(|keys| keys.readonly.len()) .unwrap_or_default(); - mapped_readonly_addresses + loaded_readonly_addresses .saturating_add(usize::from(self.header().num_readonly_signed_accounts)) .saturating_add(usize::from(self.header().num_readonly_unsigned_accounts)) } diff --git a/sdk/program/src/message/versions/mod.rs b/sdk/program/src/message/versions/mod.rs index 9242731af611bb..ffb571579b585b 100644 --- a/sdk/program/src/message/versions/mod.rs +++ b/sdk/program/src/message/versions/mod.rs @@ -43,21 +43,28 @@ impl VersionedMessage { } } - pub fn unmapped_keys(self) -> Vec { + pub fn static_account_keys(&self) -> &[Pubkey] { + match self { + Self::Legacy(message) => &message.account_keys, + Self::V0(message) => &message.account_keys, + } + } + + pub fn into_static_account_keys(self) -> Vec { match self { Self::Legacy(message) => message.account_keys, Self::V0(message) => message.account_keys, } } - pub fn unmapped_keys_iter(&self) -> impl Iterator { + pub fn static_account_keys_iter(&self) -> impl Iterator { match self { Self::Legacy(message) => message.account_keys.iter(), Self::V0(message) => message.account_keys.iter(), } } - pub fn unmapped_keys_len(&self) -> usize { + pub fn static_account_keys_len(&self) -> usize { match self { Self::Legacy(message) => message.account_keys.len(), Self::V0(message) => message.account_keys.len(), diff --git a/sdk/program/src/message/versions/v0/loaded.rs b/sdk/program/src/message/versions/v0/loaded.rs index 7bb62b8b0b3fe2..1c82f2960f4283 100644 --- a/sdk/program/src/message/versions/v0/loaded.rs +++ b/sdk/program/src/message/versions/v0/loaded.rs @@ -5,7 +5,7 @@ use { pubkey::Pubkey, sysvar, }, - std::{collections::HashSet, ops::Deref, convert::TryFrom}, + std::{collections::HashSet, ops::Deref}, }; /// Combination of a version #0 message and its loaded addresses @@ -34,6 +34,19 @@ pub struct LoadedAddresses { pub readonly: Vec, } +impl FromIterator for LoadedAddresses { + fn from_iter>(iter: T) -> Self { + let (writable, readonly): (Vec>, Vec>) = iter + .into_iter() + .map(|addresses| (addresses.writable, addresses.readonly)) + .unzip(); + LoadedAddresses { + writable: writable.into_iter().flatten().collect(), + readonly: readonly.into_iter().flatten().collect(), + } + } +} + impl LoadedMessage { /// Returns an iterator of account key segments. The ordering of segments /// affects how account indexes from compiled instructions are resolved and @@ -68,8 +81,9 @@ impl LoadedMessage { } /// Returns the address of the account at the specified index of the list of - /// message account keys constructed from unmapped keys, followed by mapped - /// writable addresses, and lastly the list of mapped readonly addresses. + /// message account keys constructed from static keys, followed by dynamically + /// loaded writable addresses, and lastly the list of dynamically loaded + /// readonly addresses. pub fn get_account_key(&self, mut index: usize) -> Option<&Pubkey> { for key_segment in self.account_keys_segment_iter() { if index < key_segment.len() { @@ -88,8 +102,8 @@ impl LoadedMessage { let num_account_keys = self.message.account_keys.len(); let num_signed_accounts = usize::from(header.num_required_signatures); if key_index >= num_account_keys { - let mapped_addresses_index = key_index.saturating_sub(num_account_keys); - mapped_addresses_index < self.loaded_addresses.writable.len() + let loaded_addresses_index = key_index.saturating_sub(num_account_keys); + loaded_addresses_index < self.loaded_addresses.writable.len() } else if key_index >= num_signed_accounts { let num_unsigned_accounts = num_account_keys.saturating_sub(num_signed_accounts); let num_writable_unsigned_accounts = num_unsigned_accounts diff --git a/sdk/src/transaction/error.rs b/sdk/src/transaction/error.rs index 60ed8c39f5563d..80df2d0a930b61 100644 --- a/sdk/src/transaction/error.rs +++ b/sdk/src/transaction/error.rs @@ -105,6 +105,22 @@ pub enum TransactionError { /// Transaction would exceed max account data limit within the block #[error("Transaction would exceed max account data limit within the block")] WouldExceedMaxAccountDataCostLimit, + + /// Address lookup table not found + #[error("Transaction loads an address table account that doesn't exist")] + AddressLookupTableNotFound, + + /// Attempted to lookup addresses from an account owned by the wrong program + #[error("Transaction loads an address table account with an invalid owner")] + InvalidAddressLookupTableOwner, + + /// Attempted to lookup addresses from an invalid account + #[error("Transaction loads an address table account with invalid data")] + InvalidAddressLookupTableData, + + /// Address table lookup uses an invalid index + #[error("Transaction address table lookup uses an invalid index")] + InvalidAddressLookupTableIndex, } impl From for TransactionError { @@ -123,3 +139,33 @@ impl From for TransactionError { } } } + +#[derive(Debug, Error, PartialEq, Eq, Clone)] +pub enum AddressLookupError { + /// Attempted to lookup addresses from a table that does not exist + #[error("Attempted to lookup addresses from a table that does not exist")] + LookupTableAccountNotFound, + + /// Attempted to lookup addresses from an account owned by the wrong program + #[error("Attempted to lookup addresses from an account owned by the wrong program")] + InvalidAccountOwner, + + /// Attempted to lookup addresses from an invalid account + #[error("Attempted to lookup addresses from an invalid account")] + InvalidAccountData, + + /// Address lookup contains an invalid index + #[error("Address lookup contains an invalid index")] + InvalidLookupIndex, +} + +impl From for TransactionError { + fn from(err: AddressLookupError) -> Self { + match err { + AddressLookupError::LookupTableAccountNotFound => Self::AddressLookupTableNotFound, + AddressLookupError::InvalidAccountOwner => Self::InvalidAddressLookupTableOwner, + AddressLookupError::InvalidAccountData => Self::InvalidAddressLookupTableData, + AddressLookupError::InvalidLookupIndex => Self::InvalidAddressLookupTableIndex, + } + } +} diff --git a/sdk/src/transaction/sanitized.rs b/sdk/src/transaction/sanitized.rs index 4bf68597c9f165..f01ac66d5c1c7b 100644 --- a/sdk/src/transaction/sanitized.rs +++ b/sdk/src/transaction/sanitized.rs @@ -1,10 +1,9 @@ #![cfg(feature = "full")] - use { crate::{ hash::Hash, message::{ - v0::{self, LoadedAddresses}, + v0::{self, LoadedAddresses, MessageAddressTableLookup}, SanitizedMessage, VersionedMessage, }, nonce::NONCED_TX_MARKER_IX_INDEX, @@ -46,7 +45,7 @@ impl SanitizedTransaction { tx: VersionedTransaction, message_hash: Hash, is_simple_vote_tx: Option, - address_loader: impl Fn(&v0::Message) -> Result, + address_loader: impl FnOnce(&[MessageAddressTableLookup]) -> Result, ) -> Result { tx.sanitize()?; @@ -54,7 +53,7 @@ impl SanitizedTransaction { let message = match tx.message { VersionedMessage::Legacy(message) => SanitizedMessage::Legacy(message), VersionedMessage::V0(message) => SanitizedMessage::V0(v0::LoadedMessage { - loaded_addresses: address_loader(&message)?, + loaded_addresses: address_loader(&message.address_table_lookups)?, message, }), }; diff --git a/sdk/src/transaction/versioned.rs b/sdk/src/transaction/versioned.rs index c343dd5cc2344f..fa56596e110e33 100644 --- a/sdk/src/transaction/versioned.rs +++ b/sdk/src/transaction/versioned.rs @@ -37,9 +37,9 @@ impl Sanitize for VersionedTransaction { Ordering::Equal => Ok(()), }?; - // Signatures are verified before message keys are mapped so all signers - // must correspond to unmapped keys. - if self.signatures.len() > self.message.unmapped_keys_len() { + // Signatures are verified before message keys are loaded so all signers + // must correspond to static account keys. + if self.signatures.len() > self.message.static_account_keys_len() { return Err(SanitizeError::IndexOutOfBounds); } @@ -71,16 +71,28 @@ impl VersionedTransaction { /// Verify the transaction and hash its message pub fn verify_and_hash_message(&self) -> Result { let message_bytes = self.message.serialize(); - if self - .signatures + if !self + ._verify_with_results(&message_bytes) .iter() - .zip(self.message.unmapped_keys_iter()) - .map(|(signature, pubkey)| signature.verify(pubkey.as_ref(), &message_bytes)) - .any(|verified| !verified) + .all(|verify_result| *verify_result) { Err(TransactionError::SignatureFailure) } else { Ok(VersionedMessage::hash_raw_message(&message_bytes)) } } + + /// Verify the transaction and return a list of verification results + pub fn verify_with_results(&self) -> Vec { + let message_bytes = self.message.serialize(); + self._verify_with_results(&message_bytes) + } + + fn _verify_with_results(&self, message_bytes: &[u8]) -> Vec { + self.signatures + .iter() + .zip(self.message.static_account_keys_iter()) + .map(|(signature, pubkey)| signature.verify(pubkey.as_ref(), message_bytes)) + .collect() + } } diff --git a/storage-proto/proto/transaction_by_addr.proto b/storage-proto/proto/transaction_by_addr.proto index 582c31991f3b4a..5825251bb1be14 100644 --- a/storage-proto/proto/transaction_by_addr.proto +++ b/storage-proto/proto/transaction_by_addr.proto @@ -46,6 +46,10 @@ enum TransactionErrorType { INVALID_WRITABLE_ACCOUNT = 19; WOULD_EXCEED_MAX_ACCOUNT_COST_LIMIT = 20; WOULD_EXCEED_MAX_ACCOUNT_DATA_COST_LIMIT = 21; + ADDRESS_LOOKUP_TABLE_NOT_FOUND = 22; + INVALID_ADDRESS_LOOKUP_TABLE_OWNER = 23; + INVALID_ADDRESS_LOOKUP_TABLE_DATA = 24; + INVALID_ADDRESS_LOOKUP_TABLE_INDEX = 25; } message InstructionError { diff --git a/storage-proto/src/convert.rs b/storage-proto/src/convert.rs index 423732196010a5..718115fbfa56c2 100644 --- a/storage-proto/src/convert.rs +++ b/storage-proto/src/convert.rs @@ -569,6 +569,10 @@ impl TryFrom for TransactionError { 19 => TransactionError::InvalidWritableAccount, 20 => TransactionError::WouldExceedMaxAccountCostLimit, 21 => TransactionError::WouldExceedMaxAccountDataCostLimit, + 22 => TransactionError::AddressLookupTableNotFound, + 23 => TransactionError::InvalidAddressLookupTableOwner, + 24 => TransactionError::InvalidAddressLookupTableData, + 25 => TransactionError::InvalidAddressLookupTableIndex, _ => return Err("Invalid TransactionError"), }) } @@ -642,6 +646,18 @@ impl From for tx_by_addr::TransactionError { TransactionError::WouldExceedMaxAccountDataCostLimit => { tx_by_addr::TransactionErrorType::WouldExceedMaxAccountDataCostLimit } + TransactionError::AddressLookupTableNotFound => { + tx_by_addr::TransactionErrorType::AddressLookupTableNotFound + } + TransactionError::InvalidAddressLookupTableOwner => { + tx_by_addr::TransactionErrorType::InvalidAddressLookupTableOwner + } + TransactionError::InvalidAddressLookupTableData => { + tx_by_addr::TransactionErrorType::InvalidAddressLookupTableData + } + TransactionError::InvalidAddressLookupTableIndex => { + tx_by_addr::TransactionErrorType::InvalidAddressLookupTableIndex + } } as i32, instruction_error: match transaction_error { TransactionError::InstructionError(index, ref instruction_error) => {