diff --git a/accountsdb-plugin-postgres/scripts/create_schema.sql b/accountsdb-plugin-postgres/scripts/create_schema.sql index 5385663c47..e5405f402a 100644 --- a/accountsdb-plugin-postgres/scripts/create_schema.sql +++ b/accountsdb-plugin-postgres/scripts/create_schema.sql @@ -49,7 +49,11 @@ Create TYPE "TransactionErrorCode" AS ENUM ( 'UnsupportedVersion', 'InvalidWritableAccount', 'WouldExceedMaxAccountDataCostLimit', - 'TooManyAccountLocks' + 'TooManyAccountLocks', + '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 37112b0b10..ea095a0b2f 100644 --- a/accountsdb-plugin-postgres/src/postgres_client/postgres_client_transaction.rs +++ b/accountsdb-plugin-postgres/src/postgres_client/postgres_client_transaction.rs @@ -332,6 +332,10 @@ pub enum DbTransactionErrorCode { InvalidWritableAccount, WouldExceedMaxAccountDataCostLimit, TooManyAccountLocks, + AddressLookupTableNotFound, + InvalidAddressLookupTableOwner, + InvalidAddressLookupTableData, + InvalidAddressLookupTableIndex, } impl From<&TransactionError> for DbTransactionErrorCode { @@ -364,6 +368,14 @@ impl From<&TransactionError> for DbTransactionErrorCode { Self::WouldExceedMaxAccountDataCostLimit } TransactionError::TooManyAccountLocks => Self::TooManyAccountLocks, + 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 e91b18627a..f42088e4f9 100644 --- a/core/src/banking_stage.rs +++ b/core/src/banking_stage.rs @@ -33,7 +33,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, @@ -1114,6 +1117,7 @@ impl BankingStage { transaction_indexes: &[usize], feature_set: &Arc, votes_only: bool, + address_loader: impl Fn(&[MessageAddressTableLookup]) -> transaction::Result, ) -> (Vec, Vec) { transaction_indexes .iter() @@ -1130,7 +1134,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()?; @@ -1196,6 +1200,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); @@ -1270,6 +1275,7 @@ impl BankingStage { transaction_indexes, &bank.feature_set, bank.vote_only_bank(), + |lookup| bank.load_lookup_table_addresses(lookup), ); unprocessed_packet_conversion_time.stop(); @@ -3109,6 +3115,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); @@ -3119,6 +3126,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()); @@ -3138,6 +3146,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); @@ -3148,6 +3157,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); @@ -3167,6 +3177,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); @@ -3177,6 +3188,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 15ab065ac2..0779e27b04 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 8bf7fc3457..c2f50c6ab7 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,49 @@ impl<'a> AddressLookupTable<'a> { Ok(()) } + /// Get the length of addresses that are active for lookups + pub fn get_active_addresses_len( + &self, + current_slot: Slot, + slot_hashes: &SlotHashes, + ) -> Result { + 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 + }; + + Ok(active_addresses_len) + } + + /// 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> { + let active_addresses_len = self.get_active_addresses_len(current_slot, slot_hashes)?; + 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 +366,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 976a8c782b..da2a18cdd8 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,20 @@ 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, TransactionAccountLocks, TransactionError}, + transaction::{ + AddressLookupError, Result, SanitizedTransaction, TransactionAccountLocks, + TransactionError, + }, transaction_context::TransactionAccount, }, std::{ @@ -514,6 +522,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, @@ -1268,6 +1310,7 @@ mod tests { bank::{DurableNonceFee, TransactionExecutionDetails}, rent_collector::RentCollector, }, + solana_address_lookup_table_program::state::LookupTableMeta, solana_sdk::{ account::{AccountSharedData, WritableAccount}, epoch_schedule::EpochSchedule, @@ -1282,6 +1325,7 @@ mod tests { transaction::{Transaction, MAX_TX_ACCOUNT_LOCKS}, }, std::{ + borrow::Cow, convert::TryFrom, sync::atomic::{AtomicBool, AtomicU64, Ordering}, thread, time, @@ -1866,6 +1910,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 9794131eb7..3548e9a48f 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -107,7 +107,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, @@ -123,7 +126,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}, @@ -210,7 +213,7 @@ impl RentDebits { } type BankStatusCache = StatusCache>; -#[frozen_abi(digest = "6XG6H1FChrDdY39K62KFWj5XfDao4dd24WZgcJkdMu1E")] +#[frozen_abi(digest = "2r36f5cfgP7ABq7D3kRkRfQZWdggGFUnnhwTrVEWhoTC")] pub type BankSlotDelta = SlotDelta>; // Eager rent collection repeats in cyclic manner. @@ -3537,6 +3540,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::>()?) + } + /// Execute a transaction using the provided loaded accounts and update /// the executors cache if the transaction was successful. fn execute_loaded_transaction( @@ -3550,16 +3591,6 @@ impl Bank { timings: &mut ExecuteTimings, error_counters: &mut ErrorCounters, ) -> TransactionExecutionResult { - let legacy_message = match tx.message().legacy_message() { - Some(message) => message, - None => { - // TODO: support versioned messages - return TransactionExecutionResult::NotExecuted( - TransactionError::UnsupportedVersion, - ); - } - }; - let mut get_executors_time = Measure::start("get_executors_time"); let executors = self.get_executors( tx.message(), @@ -3598,7 +3629,7 @@ impl Bank { let mut process_message_time = Measure::start("process_message_time"); let process_result = MessageProcessor::process_message( &self.builtin_programs.vec, - legacy_message, + tx.message(), &loaded_transaction.program_indices, &mut transaction_context, self.rent_collector.rent, @@ -5348,8 +5379,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 4253e1a920..758ecba692 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, @@ -52,7 +52,7 @@ impl MessageProcessor { #[allow(clippy::too_many_arguments)] pub fn process_message( builtin_programs: &[BuiltinProgram], - message: &Message, + message: &SanitizedMessage, program_indices: &[Vec], transaction_context: &mut TransactionContext, rent: Rent, @@ -82,14 +82,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()) @@ -139,7 +137,7 @@ impl MessageProcessor { ); time.stop(); timings.details.accumulate_program( - instruction.program_id(&message.account_keys), + program_id, time.as_us(), compute_units_consumed, result.is_err(), @@ -251,14 +249,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, @@ -292,14 +290,14 @@ mod tests { 0 ); - let message = Message::new( + let message = SanitizedMessage::Legacy(Message::new( &[Instruction::new_with_bincode( mock_system_program_id, &MockSystemInstruction::TransferLamports { lamports: 50 }, account_metas.clone(), )], Some(transaction_context.get_key_of_account_at_index(0)), - ); + )); let result = MessageProcessor::process_message( builtin_programs, &message, @@ -325,14 +323,14 @@ mod tests { )) ); - let message = Message::new( + let message = SanitizedMessage::Legacy(Message::new( &[Instruction::new_with_bincode( mock_system_program_id, &MockSystemInstruction::ChangeData { data: 50 }, account_metas, )], Some(transaction_context.get_key_of_account_at_index(0)), - ); + )); let result = MessageProcessor::process_message( builtin_programs, &message, @@ -451,14 +449,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, @@ -485,14 +483,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, @@ -513,7 +511,7 @@ mod tests { assert!(result.is_ok()); // Do work on the same transaction account but at different instruction accounts - let message = Message::new( + let message = SanitizedMessage::Legacy(Message::new( &[Instruction::new_with_bincode( mock_program_id, &MockSystemInstruction::DoWork { @@ -523,7 +521,7 @@ mod tests { account_metas, )], Some(transaction_context.get_key_of_account_at_index(0)), - ); + )); let result = MessageProcessor::process_message( builtin_programs, &message, @@ -587,7 +585,7 @@ mod tests { ]; let mut 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()), @@ -596,7 +594,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 cec03d8675..fa27d5b881 100644 --- a/sdk/program/src/message/sanitized.rs +++ b/sdk/program/src/message/sanitized.rs @@ -22,7 +22,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), } @@ -130,7 +130,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()), @@ -138,7 +138,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(), @@ -257,11 +257,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 b1392e114a..ace0a3bf72 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 235dbd2657..1f67dfdf85 100644 --- a/sdk/program/src/message/versions/v0/loaded.rs +++ b/sdk/program/src/message/versions/v0/loaded.rs @@ -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 075635c21d..a207b91951 100644 --- a/sdk/src/transaction/error.rs +++ b/sdk/src/transaction/error.rs @@ -109,6 +109,22 @@ pub enum TransactionError { /// Transaction locked too many accounts #[error("Transaction locked too many accounts")] TooManyAccountLocks, + + /// 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 { @@ -122,3 +138,33 @@ impl From for TransactionError { Self::SanitizeFailure } } + +#[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 a6e7218130..3ccb80846c 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, @@ -51,7 +50,7 @@ impl SanitizedTransaction { tx: VersionedTransaction, message_hash: Hash, is_simple_vote_tx: Option, - address_loader: impl Fn(&v0::Message) -> Result, + address_loader: impl Fn(&[MessageAddressTableLookup]) -> Result, ) -> Result { tx.sanitize()?; @@ -59,7 +58,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 c343dd5cc2..fa56596e11 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 4febf026cc..6b467f2694 100644 --- a/storage-proto/proto/transaction_by_addr.proto +++ b/storage-proto/proto/transaction_by_addr.proto @@ -47,6 +47,10 @@ enum TransactionErrorType { WOULD_EXCEED_MAX_ACCOUNT_COST_LIMIT = 20; WOULD_EXCEED_MAX_ACCOUNT_DATA_COST_LIMIT = 21; TOO_MANY_ACCOUNT_LOCKS = 22; + ADDRESS_LOOKUP_TABLE_NOT_FOUND = 23; + INVALID_ADDRESS_LOOKUP_TABLE_OWNER = 24; + INVALID_ADDRESS_LOOKUP_TABLE_DATA = 25; + INVALID_ADDRESS_LOOKUP_TABLE_INDEX = 26; } message InstructionError { diff --git a/storage-proto/src/convert.rs b/storage-proto/src/convert.rs index 0e3a6c1541..05d1c4be2e 100644 --- a/storage-proto/src/convert.rs +++ b/storage-proto/src/convert.rs @@ -570,6 +570,10 @@ impl TryFrom for TransactionError { 20 => TransactionError::WouldExceedMaxAccountCostLimit, 21 => TransactionError::WouldExceedMaxAccountDataCostLimit, 22 => TransactionError::TooManyAccountLocks, + 23 => TransactionError::AddressLookupTableNotFound, + 24 => TransactionError::InvalidAddressLookupTableOwner, + 25 => TransactionError::InvalidAddressLookupTableData, + 26 => TransactionError::InvalidAddressLookupTableIndex, _ => return Err("Invalid TransactionError"), }) } @@ -646,6 +650,18 @@ impl From for tx_by_addr::TransactionError { TransactionError::TooManyAccountLocks => { tx_by_addr::TransactionErrorType::TooManyAccountLocks } + 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) => {