diff --git a/banks-server/src/banks_server.rs b/banks-server/src/banks_server.rs index 8a960766e8d..6a3c3f55c85 100644 --- a/banks-server/src/banks_server.rs +++ b/banks-server/src/banks_server.rs @@ -180,6 +180,8 @@ fn simulate_transaction( bank.get_reserved_account_keys(), bank.feature_set .is_active(&agave_feature_set::static_instruction_limit::id()), + bank.feature_set + .is_active(&agave_feature_set::limit_instruction_accounts::id()), ) { Err(err) => { return BanksTransactionResultWithSimulation { diff --git a/core/src/banking_stage/consume_worker.rs b/core/src/banking_stage/consume_worker.rs index a0dcd54a1b7..c348ffb9b90 100644 --- a/core/src/banking_stage/consume_worker.rs +++ b/core/src/banking_stage/consume_worker.rs @@ -769,6 +769,9 @@ pub(crate) mod external { let enable_static_instruction_limit = bank .feature_set .is_active(&agave_feature_set::static_instruction_limit::ID); + let enable_instruction_accounts_limit = bank + .feature_set + .is_active(&agave_feature_set::limit_instruction_accounts::ID); let mut parsing_results = Vec::with_capacity(MAX_TRANSACTIONS_PER_MESSAGE); let mut parsed_transactions = Vec::with_capacity(MAX_TRANSACTIONS_PER_MESSAGE); for (tx_ptr, _) in batch.iter() { @@ -776,6 +779,7 @@ pub(crate) mod external { match SanitizedTransactionView::try_new_sanitized( tx_ptr, enable_static_instruction_limit, + enable_instruction_accounts_limit, ) { Ok(view) => { parsing_results.push(Ok(())); @@ -959,6 +963,9 @@ pub(crate) mod external { let enable_static_instruction_limit = bank .feature_set .is_active(&agave_feature_set::static_instruction_limit::ID); + let enable_instruction_accounts_limit = bank + .feature_set + .is_active(&agave_feature_set::limit_instruction_accounts::ID); let transaction_account_lock_limit = bank.get_transaction_account_lock_limit(); let mut translation_results = Vec::with_capacity(MAX_TRANSACTIONS_PER_MESSAGE); @@ -970,6 +977,7 @@ pub(crate) mod external { bank, enable_static_instruction_limit, transaction_account_lock_limit, + enable_instruction_accounts_limit, ) { Ok((tx, max_age)) => { transactions.push(tx); @@ -988,12 +996,14 @@ pub(crate) mod external { bank: &Bank, enable_static_instruction_limit: bool, transaction_account_lock_limit: usize, + enable_instruction_accounts_limit: bool, ) -> Result<(Tx, MaxAge), PacketHandlingError> { translate_to_runtime_view( transaction_ptr, bank, enable_static_instruction_limit, transaction_account_lock_limit, + enable_instruction_accounts_limit, ) .map(|(view, deactivation_slot)| { ( @@ -1177,6 +1187,7 @@ pub(crate) mod external { &bank, true, bank.get_transaction_account_lock_limit(), + true, ) .ok() .unwrap() @@ -1364,8 +1375,8 @@ pub(crate) mod external { let parsing_results = [Ok(()), Err(TransactionViewError::ParseError), Ok(())]; let parsed_transactions = [ - SanitizedTransactionView::try_new_sanitized(&tx1[..], true).unwrap(), - SanitizedTransactionView::try_new_sanitized(&tx2[..], true).unwrap(), + SanitizedTransactionView::try_new_sanitized(&tx1[..], true, true).unwrap(), + SanitizedTransactionView::try_new_sanitized(&tx2[..], true, true).unwrap(), ]; bank.store_account( &parsed_transactions[1].static_account_keys()[0], @@ -1415,7 +1426,7 @@ pub(crate) mod external { ) -> RuntimeTransaction> { RuntimeTransaction::>::try_new( RuntimeTransaction::>::try_new( - SanitizedTransactionView::try_new_sanitized(tx, true).unwrap(), + SanitizedTransactionView::try_new_sanitized(tx, true, true).unwrap(), solana_transaction::sanitized::MessageHash::Compute, Some(false), ) @@ -2565,6 +2576,8 @@ mod tests { &HashSet::default(), bank.feature_set .is_active(&agave_feature_set::static_instruction_limit::id()), + bank.feature_set + .is_active(&agave_feature_set::limit_instruction_accounts::id()), ) .unwrap() }; diff --git a/core/src/banking_stage/consumer.rs b/core/src/banking_stage/consumer.rs index 4e1132bd38a..ec08c35812f 100644 --- a/core/src/banking_stage/consumer.rs +++ b/core/src/banking_stage/consumer.rs @@ -1443,6 +1443,8 @@ mod tests { &ReservedAccountKeys::empty_key_set(), bank.feature_set .is_active(&agave_feature_set::static_instruction_limit::id()), + bank.feature_set + .is_active(&agave_feature_set::limit_instruction_accounts::id()), ) .unwrap(); let batch_transactions_inner = [&sanitized_tx] diff --git a/core/src/banking_stage/latest_validator_vote_packet.rs b/core/src/banking_stage/latest_validator_vote_packet.rs index 63c2f3fb19d..3ae5c707711 100644 --- a/core/src/banking_stage/latest_validator_vote_packet.rs +++ b/core/src/banking_stage/latest_validator_vote_packet.rs @@ -109,6 +109,7 @@ impl LatestValidatorVote { let vote = SanitizedTransactionView::try_new_sanitized( std::sync::Arc::new(packet.data(..).unwrap().to_vec()), false, + true, ) .unwrap(); diff --git a/core/src/banking_stage/transaction_scheduler/receive_and_buffer.rs b/core/src/banking_stage/transaction_scheduler/receive_and_buffer.rs index f8249d8cc77..a80b484c77f 100644 --- a/core/src/banking_stage/transaction_scheduler/receive_and_buffer.rs +++ b/core/src/banking_stage/transaction_scheduler/receive_and_buffer.rs @@ -244,6 +244,9 @@ impl TransactionViewReceiveAndBuffer { let enable_static_instruction_limit = root_bank .feature_set .is_active(&agave_feature_set::static_instruction_limit::ID); + let enable_instruction_accounts_limit = root_bank + .feature_set + .is_active(&agave_feature_set::limit_instruction_accounts::ID); let transaction_account_lock_limit = working_bank.get_transaction_account_lock_limit(); // Create temporary batches of transactions to be age-checked. @@ -347,6 +350,7 @@ impl TransactionViewReceiveAndBuffer { working_bank, enable_static_instruction_limit, transaction_account_lock_limit, + enable_instruction_accounts_limit, ) { Ok(state) => Ok(state), Err( @@ -407,12 +411,14 @@ impl TransactionViewReceiveAndBuffer { working_bank: &Bank, enable_static_instruction_limit: bool, transaction_account_lock_limit: usize, + enable_instruction_accounts_limit: bool, ) -> Result { let (view, deactivation_slot) = translate_to_runtime_view( bytes, root_bank, enable_static_instruction_limit, transaction_account_lock_limit, + enable_instruction_accounts_limit, )?; let Ok(compute_budget_limits) = view @@ -438,11 +444,14 @@ pub(crate) fn translate_to_runtime_view( bank: &Bank, enable_static_instruction_limit: bool, transaction_account_lock_limit: usize, + enable_instruction_accounts_limit: bool, ) -> Result<(RuntimeTransaction>, u64), PacketHandlingError> { // Parsing and basic sanitization checks - let Ok(view) = - SanitizedTransactionView::try_new_sanitized(data, enable_static_instruction_limit) - else { + let Ok(view) = SanitizedTransactionView::try_new_sanitized( + data, + enable_static_instruction_limit, + enable_instruction_accounts_limit, + ) else { return Err(PacketHandlingError::Sanitization); }; diff --git a/core/src/banking_stage/transaction_scheduler/transaction_state_container.rs b/core/src/banking_stage/transaction_scheduler/transaction_state_container.rs index f9c9f58214d..1fd73f8c1cf 100644 --- a/core/src/banking_stage/transaction_scheduler/transaction_state_container.rs +++ b/core/src/banking_stage/transaction_scheduler/transaction_state_container.rs @@ -474,7 +474,7 @@ mod tests { let reserved_addresses = HashSet::default(); let packet_parser = |data, priority, cost| { - let view = SanitizedTransactionView::try_new_sanitized(data, true).unwrap(); + let view = SanitizedTransactionView::try_new_sanitized(data, true, true).unwrap(); let view = RuntimeTransaction::>::try_new( view, MessageHash::Compute, diff --git a/core/src/banking_stage/vote_packet_receiver.rs b/core/src/banking_stage/vote_packet_receiver.rs index bf3f7ee7573..3984e0ac8a7 100644 --- a/core/src/banking_stage/vote_packet_receiver.rs +++ b/core/src/banking_stage/vote_packet_receiver.rs @@ -110,6 +110,9 @@ impl VotePacketReceiver { // NB: It's safe to always pass false in here as simple vote // transactions are guaranteed to be a single instruction. false, + // Vote instructions are created in the validator code, and they are not + // referencing more than 255 accounts, so it is safe to set this to true. + true, ) { Ok(pkt) => Some(pkt), Err(err) => { diff --git a/core/src/banking_stage/vote_storage.rs b/core/src/banking_stage/vote_storage.rs index eafa88038dc..7c3af3fdab7 100644 --- a/core/src/banking_stage/vote_storage.rs +++ b/core/src/banking_stage/vote_storage.rs @@ -393,7 +393,7 @@ pub(crate) mod tests { } fn to_sanitized_view(packet: BytesPacket) -> SanitizedTransactionView { - SanitizedTransactionView::try_new_sanitized(Arc::new(packet.buffer().to_vec()), false) + SanitizedTransactionView::try_new_sanitized(Arc::new(packet.buffer().to_vec()), false, true) .unwrap() } diff --git a/core/src/banking_stage/vote_worker.rs b/core/src/banking_stage/vote_worker.rs index ce13ae112ce..8508184f92f 100644 --- a/core/src/banking_stage/vote_worker.rs +++ b/core/src/banking_stage/vote_worker.rs @@ -587,9 +587,12 @@ mod tests { }; fn to_runtime_transaction_view(packet: BytesPacket) -> RuntimeTransactionView { - let tx = - SanitizedTransactionView::try_new_sanitized(Arc::new(packet.buffer().to_vec()), false) - .unwrap(); + let tx = SanitizedTransactionView::try_new_sanitized( + Arc::new(packet.buffer().to_vec()), + false, + true, + ) + .unwrap(); let tx = RuntimeTransaction::>::try_new( tx, MessageHash::Compute, diff --git a/core/src/forwarding_stage.rs b/core/src/forwarding_stage.rs index 779a04c8e18..672357d1a66 100644 --- a/core/src/forwarding_stage.rs +++ b/core/src/forwarding_stage.rs @@ -294,6 +294,9 @@ impl let enable_static_instruction_limit = bank .feature_set .is_active(&agave_feature_set::static_instruction_limit::id()); + let enable_instruction_accounts_limit = bank + .feature_set + .is_active(&agave_feature_set::limit_instruction_accounts::id()); for batch in packet_batches.iter() { for packet in batch .iter() @@ -317,6 +320,7 @@ impl let Some(priority) = SanitizedTransactionView::try_new_sanitized( packet_data, enable_static_instruction_limit, + enable_instruction_accounts_limit, ) .map_err(|_| ()) .and_then(|transaction| { diff --git a/cost-model/src/transaction_cost.rs b/cost-model/src/transaction_cost.rs index 276539976bd..47362be2fde 100644 --- a/cost-model/src/transaction_cost.rs +++ b/cost-model/src/transaction_cost.rs @@ -375,6 +375,7 @@ mod tests { SimpleAddressLoader::Disabled, &ReservedAccountKeys::empty_key_set(), true, + true, ) .unwrap(); @@ -430,6 +431,7 @@ mod tests { SimpleAddressLoader::Disabled, &ReservedAccountKeys::empty_key_set(), true, + true, ) .unwrap(); diff --git a/entry/benches/entry_sigverify.rs b/entry/benches/entry_sigverify.rs index 4d0538f7a24..7c2bb355a2b 100644 --- a/entry/benches/entry_sigverify.rs +++ b/entry/benches/entry_sigverify.rs @@ -37,6 +37,7 @@ fn bench_cpusigverify(bencher: &mut Bencher) { SimpleAddressLoader::Disabled, &ReservedAccountKeys::empty_key_set(), true, + true, ) }?; diff --git a/entry/src/entry.rs b/entry/src/entry.rs index cce949b7f92..8be5d59138a 100644 --- a/entry/src/entry.rs +++ b/entry/src/entry.rs @@ -698,6 +698,7 @@ mod tests { SimpleAddressLoader::Disabled, &ReservedAccountKeys::empty_key_set(), true, + true, ) }?; diff --git a/feature-set/src/lib.rs b/feature-set/src/lib.rs index df3113d1477..ff73c10c959 100644 --- a/feature-set/src/lib.rs +++ b/feature-set/src/lib.rs @@ -1287,6 +1287,10 @@ pub mod stop_use_static_simple_vote_tx_cost { solana_pubkey::declare_id!("NSVt1s8oP1A9NjEc6UNcj2voeCcfzHaq4jZTiUL2Mf5"); } +pub mod limit_instruction_accounts { + solana_pubkey::declare_id!("DqbnFPASg7tHmZ6qfpdrt2M6MWoSeiicWPXxPhxqFCQ"); +} + pub static FEATURE_NAMES: LazyLock> = LazyLock::new(|| { [ (secp256k1_program_enabled::id(), "secp256k1 program"), @@ -2298,6 +2302,10 @@ pub static FEATURE_NAMES: LazyLock> = LazyLock::n stop_use_static_simple_vote_tx_cost::id(), "stop use static SimpleVote transaction cost, issue #10227", ), + ( + limit_instruction_accounts::id(), + "SIMD-406: Maximum instruction accounts", + ), /*************** ADD NEW FEATURES HERE ***************/ ] .iter() diff --git a/ledger-tool/src/main.rs b/ledger-tool/src/main.rs index bc3f5c704be..bd3a7292f97 100644 --- a/ledger-tool/src/main.rs +++ b/ledger-tool/src/main.rs @@ -468,6 +468,7 @@ fn compute_slot_cost( SimpleAddressLoader::Disabled, &reserved_account_keys.active, feature_set.is_active(&agave_feature_set::static_instruction_limit::id()), + feature_set.is_active(&agave_feature_set::limit_instruction_accounts::id()), ) .map_err(|err| { warn!("Failed to compute cost of transaction: {err:?}"); diff --git a/rpc/src/rpc.rs b/rpc/src/rpc.rs index fc7b1ba796f..2685a7bfcc6 100644 --- a/rpc/src/rpc.rs +++ b/rpc/src/rpc.rs @@ -3837,6 +3837,9 @@ pub mod rpc_full { preflight_bank .feature_set .is_active(&agave_feature_set::static_instruction_limit::id()), + preflight_bank + .feature_set + .is_active(&agave_feature_set::limit_instruction_accounts::id()), )?; let blockhash = *transaction.message().recent_blockhash(); let message_hash = *transaction.message_hash(); @@ -3999,6 +4002,8 @@ pub mod rpc_full { bank.get_reserved_account_keys(), bank.feature_set .is_active(&agave_feature_set::static_instruction_limit::id()), + bank.feature_set + .is_active(&agave_feature_set::limit_instruction_accounts::id()), )?; let verification_error = if sig_verify { @@ -4411,6 +4416,7 @@ fn sanitize_transaction( address_loader: impl AddressLoader, reserved_account_keys: &HashSet, enable_static_instruction_limit: bool, + enable_instruction_accounts_limit: bool, ) -> Result> { RuntimeTransaction::try_create( transaction, @@ -4419,6 +4425,7 @@ fn sanitize_transaction( address_loader, reserved_account_keys, enable_static_instruction_limit, + enable_instruction_accounts_limit, ) .map_err(|err| Error::invalid_params(format!("invalid transaction: {err}"))) } @@ -9211,6 +9218,7 @@ pub mod tests { SimpleAddressLoader::Disabled, &ReservedAccountKeys::empty_key_set(), true, + true, ) .unwrap_err(), expect58 @@ -9237,6 +9245,7 @@ pub mod tests { SimpleAddressLoader::Disabled, &ReservedAccountKeys::empty_key_set(), true, + true, ) .unwrap_err(), Error::invalid_params( diff --git a/runtime-transaction/src/runtime_transaction/sdk_transactions.rs b/runtime-transaction/src/runtime_transaction/sdk_transactions.rs index 68904851c77..d636623b0bd 100644 --- a/runtime-transaction/src/runtime_transaction/sdk_transactions.rs +++ b/runtime-transaction/src/runtime_transaction/sdk_transactions.rs @@ -81,6 +81,7 @@ impl RuntimeTransaction { address_loader: impl AddressLoader, reserved_account_keys: &HashSet, enable_static_instruction_limit: bool, + enable_instruction_accounts_limit: bool, ) -> Result { if enable_static_instruction_limit && tx.message.instructions().len() @@ -88,6 +89,15 @@ impl RuntimeTransaction { { return Err(solana_transaction_error::TransactionError::SanitizeFailure); } + + if enable_instruction_accounts_limit { + for instr in tx.message.instructions() { + if instr.accounts.len() > solana_transaction_context::MAX_ACCOUNTS_PER_INSTRUCTION { + return Err(solana_transaction_error::TransactionError::SanitizeFailure); + } + } + } + let statically_loaded_runtime_tx = RuntimeTransaction::::try_from( SanitizedVersionedTransaction::try_from(tx)?, @@ -150,6 +160,7 @@ impl RuntimeTransaction { pub fn from_transaction_for_tests(transaction: solana_transaction::Transaction) -> Self { let versioned_transaction = VersionedTransaction::from(transaction); let enable_static_instruction_limit = true; + let enable_instruction_accounts_limit = true; Self::try_create( versioned_transaction, MessageHash::Compute, @@ -157,6 +168,7 @@ impl RuntimeTransaction { solana_message::SimpleAddressLoader::Disabled, &HashSet::new(), enable_static_instruction_limit, + enable_instruction_accounts_limit, ) .expect("failed to create RuntimeTransaction from Transaction") } @@ -172,7 +184,11 @@ mod tests { solana_hash::Hash, solana_instruction::Instruction, solana_keypair::Keypair, - solana_message::{Message, SimpleAddressLoader}, + solana_message::{ + Message, MessageHeader, SimpleAddressLoader, VersionedMessage, + compiled_instruction::CompiledInstruction, + }, + solana_signature::Signature, solana_signer::Signer, solana_system_interface::instruction as system_instruction, solana_transaction::{Transaction, versioned::VersionedTransaction}, @@ -352,4 +368,63 @@ mod tests { ); } } + + #[test] + fn test_simd_406_instruction_accounts_limit() { + let account_keys = vec![Pubkey::new_unique(); 3]; + let header = MessageHeader { + num_required_signatures: 1, + num_readonly_signed_accounts: 0, + num_readonly_unsigned_accounts: 1, + }; + let mut accounts: Vec = vec![0; 254]; + // Exactly 255 accounts must pass sanitization + accounts.push(1); + let instr = CompiledInstruction::new_from_raw_parts(2, Vec::new(), accounts.clone()); + let transaction = VersionedTransaction { + signatures: vec![Signature::default(); 1], + message: VersionedMessage::Legacy(solana_message::Message { + header, + account_keys: account_keys.clone(), + recent_blockhash: Hash::default(), + instructions: vec![instr], + }), + }; + let result = RuntimeTransaction::::try_create( + transaction, + MessageHash::Compute, + None, + solana_message::SimpleAddressLoader::Disabled, + &HashSet::new(), + true, + true, + ); + assert!(result.is_ok()); + + // 256 accounts must fail + accounts.push(2); + let instr = CompiledInstruction::new_from_raw_parts(2, Vec::new(), accounts.clone()); + let transaction = VersionedTransaction { + signatures: vec![Signature::default(); 1], + message: VersionedMessage::Legacy(solana_message::Message { + header, + account_keys, + recent_blockhash: Hash::default(), + instructions: vec![instr], + }), + }; + let result = RuntimeTransaction::::try_create( + transaction, + MessageHash::Compute, + None, + solana_message::SimpleAddressLoader::Disabled, + &HashSet::new(), + true, + true, + ); + assert_eq!( + result.err(), + Some(solana_transaction_error::TransactionError::SanitizeFailure) + ); + } } diff --git a/runtime-transaction/src/runtime_transaction/transaction_view.rs b/runtime-transaction/src/runtime_transaction/transaction_view.rs index 84747644a28..073c16c396b 100644 --- a/runtime-transaction/src/runtime_transaction/transaction_view.rs +++ b/runtime-transaction/src/runtime_transaction/transaction_view.rs @@ -240,7 +240,8 @@ mod tests { let hash = Hash::new_unique(); let transaction = - SanitizedTransactionView::try_new_sanitized(&serialized_transaction[..], true).unwrap(); + SanitizedTransactionView::try_new_sanitized(&serialized_transaction[..], true, true) + .unwrap(); let static_runtime_transaction = RuntimeTransaction::>::try_new( transaction, @@ -273,7 +274,7 @@ mod tests { ) { let bytes = bincode::serialize(&original_transaction).unwrap(); let transaction_view = - SanitizedTransactionView::try_new_sanitized(&bytes[..], true).unwrap(); + SanitizedTransactionView::try_new_sanitized(&bytes[..], true, true).unwrap(); let runtime_transaction = RuntimeTransaction::>::try_new( transaction_view, MessageHash::Compute, @@ -340,7 +341,7 @@ mod tests { let bytes = bincode::serialize(&original_transaction.to_versioned_transaction()).unwrap(); let transaction_view = - SanitizedTransactionView::try_new_sanitized(&bytes[..], true).unwrap(); + SanitizedTransactionView::try_new_sanitized(&bytes[..], true, true).unwrap(); let runtime_transaction = RuntimeTransaction::>::try_new( transaction_view, MessageHash::Compute, diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index f522ac2a303..f572439a496 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -3030,6 +3030,9 @@ impl Bank { let enable_static_instruction_limit = self .feature_set .is_active(&agave_feature_set::static_instruction_limit::id()); + let enable_instruction_account_limit = self + .feature_set + .is_active(&agave_feature_set::limit_instruction_accounts::id()); let sanitized_txs = txs .into_iter() .map(|tx| { @@ -3040,6 +3043,7 @@ impl Bank { self, self.get_reserved_account_keys(), enable_static_instruction_limit, + enable_instruction_account_limit, ) }) .collect::>>()?; @@ -4808,6 +4812,10 @@ impl Bank { let enable_static_instruction_limit = self .feature_set .is_active(&agave_feature_set::static_instruction_limit::id()); + let enable_instruction_account_limit = self + .feature_set + .is_active(&agave_feature_set::limit_instruction_accounts::id()); + let sanitized_tx = { let size = bincode::serialized_size(&tx).map_err(|_| TransactionError::SanitizeFailure)?; @@ -4835,6 +4843,7 @@ impl Bank { self, self.get_reserved_account_keys(), enable_static_instruction_limit, + enable_instruction_account_limit, ) }?; diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index 99230e33959..b5a292ff534 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -9328,6 +9328,50 @@ fn test_verify_transactions_instruction_limit(simd_0160_enabled: bool) { } } +#[test_case(false; "pre_simd406_limit_instruction_accounts")] +#[test_case(true; "simd406_limit_instruction_accounts")] +fn test_verify_transactions_accounts_limit(simd_406_enabled: bool) { + let GenesisConfigInfo { genesis_config, .. } = + create_genesis_config_with_leader(42, &solana_pubkey::new_rand(), 42); + let mut bank = Bank::new_for_tests(&genesis_config); + if !simd_406_enabled { + bank.deactivate_feature(&feature_set::limit_instruction_accounts::id()); + } + + let recent_blockhash = Hash::new_unique(); + let keypair = Keypair::new(); + let pubkey = keypair.pubkey(); + let mut accounts_keys = vec![Pubkey::new_unique(); 10]; + accounts_keys.insert(0, pubkey); + let accounts: Vec = (0..10).cycle().take(256).collect(); + let instruction = CompiledInstruction { + program_id_index: 1, + data: vec![], + accounts, + }; + + let message = Message::new_with_compiled_instructions( + 1, + 0, + 1, + accounts_keys, + recent_blockhash, + vec![instruction], + ); + let tx = Transaction::new(&[&keypair], message, recent_blockhash); + + if simd_406_enabled { + assert_matches!( + bank.verify_transaction(tx.into(), TransactionVerificationMode::FullVerification), + Err(TransactionError::SanitizeFailure) + ); + } else { + assert!(bank + .verify_transaction(tx.into(), TransactionVerificationMode::FullVerification) + .is_ok()); + } +} + #[test] fn test_check_reserved_keys() { let (genesis_config, _mint_keypair) = create_genesis_config(1); diff --git a/transaction-view/benches/transaction_view.rs b/transaction-view/benches/transaction_view.rs index 03d47454ed9..286898f7a8a 100644 --- a/transaction-view/benches/transaction_view.rs +++ b/transaction-view/benches/transaction_view.rs @@ -65,8 +65,8 @@ fn bench_transactions_parsing( group.bench_function("TransactionView (Sanitized)", |c| { c.iter(|| { for bytes in serialized_transactions.iter() { - let _ = - TransactionView::try_new_sanitized(black_box(bytes.as_ref()), true).unwrap(); + let _ = TransactionView::try_new_sanitized(black_box(bytes.as_ref()), true, true) + .unwrap(); } }); }); diff --git a/transaction-view/src/resolved_transaction_view.rs b/transaction-view/src/resolved_transaction_view.rs index 9f292f7f29f..7a41917bd98 100644 --- a/transaction-view/src/resolved_transaction_view.rs +++ b/transaction-view/src/resolved_transaction_view.rs @@ -290,7 +290,7 @@ mod tests { }), }; let bytes = bincode::serialize(&transaction).unwrap(); - let view = SanitizedTransactionView::try_new_sanitized(bytes.as_ref(), true).unwrap(); + let view = SanitizedTransactionView::try_new_sanitized(bytes.as_ref(), true, true).unwrap(); let result = ResolvedTransactionView::try_new(view, None, &HashSet::default()); assert!(matches!( result, @@ -321,7 +321,7 @@ mod tests { }), }; let bytes = bincode::serialize(&transaction).unwrap(); - let view = SanitizedTransactionView::try_new_sanitized(bytes.as_ref(), true).unwrap(); + let view = SanitizedTransactionView::try_new_sanitized(bytes.as_ref(), true, true).unwrap(); let result = ResolvedTransactionView::try_new(view, Some(loaded_addresses), &HashSet::default()); assert!(matches!( @@ -358,7 +358,7 @@ mod tests { }), }; let bytes = bincode::serialize(&transaction).unwrap(); - let view = SanitizedTransactionView::try_new_sanitized(bytes.as_ref(), true).unwrap(); + let view = SanitizedTransactionView::try_new_sanitized(bytes.as_ref(), true, true).unwrap(); let result = ResolvedTransactionView::try_new(view, Some(loaded_addresses), &HashSet::default()); assert!(matches!( @@ -408,7 +408,8 @@ mod tests { }; let transaction = create_transaction_with_keys(static_keys, &loaded_addresses); let bytes = bincode::serialize(&transaction).unwrap(); - let view = SanitizedTransactionView::try_new_sanitized(bytes.as_ref(), true).unwrap(); + let view = + SanitizedTransactionView::try_new_sanitized(bytes.as_ref(), true, true).unwrap(); let resolved_view = ResolvedTransactionView::try_new( view, Some(loaded_addresses), @@ -431,7 +432,8 @@ mod tests { }; let transaction = create_transaction_with_keys(static_keys, &loaded_addresses); let bytes = bincode::serialize(&transaction).unwrap(); - let view = SanitizedTransactionView::try_new_sanitized(bytes.as_ref(), true).unwrap(); + let view = + SanitizedTransactionView::try_new_sanitized(bytes.as_ref(), true, true).unwrap(); let resolved_view = ResolvedTransactionView::try_new( view, Some(loaded_addresses), @@ -454,7 +456,8 @@ mod tests { }; let transaction = create_transaction_with_keys(static_keys, &loaded_addresses); let bytes = bincode::serialize(&transaction).unwrap(); - let view = SanitizedTransactionView::try_new_sanitized(bytes.as_ref(), true).unwrap(); + let view = + SanitizedTransactionView::try_new_sanitized(bytes.as_ref(), true, true).unwrap(); let resolved_view = ResolvedTransactionView::try_new( view, Some(loaded_addresses), @@ -515,7 +518,8 @@ mod tests { let static_keys = vec![key0, key1, key2]; let transaction = create_transaction_with_static_keys(static_keys, &loaded_addresses); let bytes = bincode::serialize(&transaction).unwrap(); - let view = SanitizedTransactionView::try_new_sanitized(bytes.as_ref(), true).unwrap(); + let view = + SanitizedTransactionView::try_new_sanitized(bytes.as_ref(), true, true).unwrap(); let resolved_view = ResolvedTransactionView::try_new( view, Some(loaded_addresses.clone()), @@ -534,7 +538,8 @@ mod tests { let static_keys = vec![key0, key1, bpf_loader_upgradeable::ID]; let transaction = create_transaction_with_static_keys(static_keys, &loaded_addresses); let bytes = bincode::serialize(&transaction).unwrap(); - let view = SanitizedTransactionView::try_new_sanitized(bytes.as_ref(), true).unwrap(); + let view = + SanitizedTransactionView::try_new_sanitized(bytes.as_ref(), true, true).unwrap(); let resolved_view = ResolvedTransactionView::try_new( view, Some(loaded_addresses.clone()), @@ -557,7 +562,8 @@ mod tests { }; let transaction = create_transaction_with_static_keys(static_keys, &loaded_addresses); let bytes = bincode::serialize(&transaction).unwrap(); - let view = SanitizedTransactionView::try_new_sanitized(bytes.as_ref(), true).unwrap(); + let view = + SanitizedTransactionView::try_new_sanitized(bytes.as_ref(), true, true).unwrap(); let resolved_view = ResolvedTransactionView::try_new( view, diff --git a/transaction-view/src/sanitize.rs b/transaction-view/src/sanitize.rs index 949628a6df0..7c86fca5e4d 100644 --- a/transaction-view/src/sanitize.rs +++ b/transaction-view/src/sanitize.rs @@ -7,10 +7,15 @@ use crate::{ pub(crate) fn sanitize( view: &UnsanitizedTransactionView, enable_static_instruction_limit: bool, + enable_instruction_accounts_limit: bool, ) -> Result<()> { sanitize_signatures(view)?; sanitize_account_access(view)?; - sanitize_instructions(view, enable_static_instruction_limit)?; + sanitize_instructions( + view, + enable_static_instruction_limit, + enable_instruction_accounts_limit, + )?; sanitize_address_table_lookups(view) } @@ -57,6 +62,7 @@ fn sanitize_account_access(view: &UnsanitizedTransactionView, enable_static_instruction_limit: bool, + enable_instruction_accounts_limit: bool, ) -> Result<()> { // SIMD-160: transaction can not have more than 64 top level instructions if enable_static_instruction_limit @@ -88,6 +94,12 @@ fn sanitize_instructions( return Err(TransactionViewError::SanitizeError); } } + + if enable_instruction_accounts_limit + && instruction.accounts.len() > solana_transaction_context::MAX_ACCOUNTS_PER_INSTRUCTION + { + return Err(TransactionViewError::SanitizeError); + } } Ok(()) @@ -186,7 +198,7 @@ mod tests { let transaction = multiple_transfers(); let data = bincode::serialize(&transaction).unwrap(); let view = TransactionView::try_new_unsanitized(data.as_ref()).unwrap(); - assert!(view.sanitize(true).is_ok()); + assert!(view.sanitize(true, true).is_ok()); } #[test] @@ -393,7 +405,7 @@ mod tests { ); let data = bincode::serialize(&transaction).unwrap(); let view = TransactionView::try_new_unsanitized(data.as_ref()).unwrap(); - assert!(sanitize_instructions(&view, true).is_ok()); + assert!(sanitize_instructions(&view, true, true).is_ok()); let transaction = create_v0_transaction( num_signatures, @@ -404,7 +416,7 @@ mod tests { ); let data = bincode::serialize(&transaction).unwrap(); let view = TransactionView::try_new_unsanitized(data.as_ref()).unwrap(); - assert!(sanitize_instructions(&view, true).is_ok()); + assert!(sanitize_instructions(&view, true, true).is_ok()); } for instruction_index in 0..valid_instructions.len() { @@ -421,7 +433,7 @@ mod tests { let data = bincode::serialize(&transaction).unwrap(); let view = TransactionView::try_new_unsanitized(data.as_ref()).unwrap(); assert_eq!( - sanitize_instructions(&view, true), + sanitize_instructions(&view, true, true), Err(TransactionViewError::SanitizeError) ); } @@ -440,7 +452,7 @@ mod tests { let data = bincode::serialize(&transaction).unwrap(); let view = TransactionView::try_new_unsanitized(data.as_ref()).unwrap(); assert_eq!( - sanitize_instructions(&view, true), + sanitize_instructions(&view, true, true), Err(TransactionViewError::SanitizeError) ); } @@ -458,7 +470,7 @@ mod tests { let data = bincode::serialize(&transaction).unwrap(); let view = TransactionView::try_new_unsanitized(data.as_ref()).unwrap(); assert_eq!( - sanitize_instructions(&view, true), + sanitize_instructions(&view, true, true), Err(TransactionViewError::SanitizeError) ); } @@ -478,7 +490,7 @@ mod tests { let data = bincode::serialize(&transaction).unwrap(); let view = TransactionView::try_new_unsanitized(data.as_ref()).unwrap(); assert_eq!( - sanitize_instructions(&view, true), + sanitize_instructions(&view, true, true), Err(TransactionViewError::SanitizeError) ); } @@ -502,7 +514,7 @@ mod tests { let data = bincode::serialize(&transaction).unwrap(); let view = TransactionView::try_new_unsanitized(data.as_ref()).unwrap(); assert_eq!( - sanitize_instructions(&view, true), + sanitize_instructions(&view, true, true), Err(TransactionViewError::SanitizeError) ); } @@ -525,10 +537,10 @@ mod tests { let data = bincode::serialize(&transaction).unwrap(); let view = TransactionView::try_new_unsanitized(data.as_ref()).unwrap(); assert_eq!( - sanitize_instructions(&view, true), + sanitize_instructions(&view, true, true), Err(TransactionViewError::SanitizeError) ); - assert!(sanitize_instructions(&view, false).is_ok()); + assert!(sanitize_instructions(&view, false, true).is_ok()); let transaction = create_v0_transaction( num_signatures, @@ -540,10 +552,47 @@ mod tests { let data = bincode::serialize(&transaction).unwrap(); let view = TransactionView::try_new_unsanitized(data.as_ref()).unwrap(); assert_eq!( - sanitize_instructions(&view, true), + sanitize_instructions(&view, true, true), Err(TransactionViewError::SanitizeError) ); - assert!(sanitize_instructions(&view, false).is_ok()); + assert!(sanitize_instructions(&view, false, true).is_ok()); + } + + // SIMD-406: Limit instruction accounts to 255 + { + let mut accounts: Vec = vec![0; 254]; + accounts.push(1); + accounts.push(2); + let instr = CompiledInstruction::new_from_raw_parts(2, Vec::new(), accounts); + let transaction = create_legacy_transaction( + num_signatures, + header, + account_keys.clone(), + vec![instr], + ); + let data = bincode::serialize(&transaction).unwrap(); + let view = TransactionView::try_new_unsanitized(data.as_ref()).unwrap(); + assert_eq!( + sanitize_instructions(&view, false, true), + Err(TransactionViewError::SanitizeError) + ); + } + + // SIMD-406: Limit instruction accounts to 255 + { + let mut accounts: Vec = vec![0; 254]; + accounts.push(1); + let instr = CompiledInstruction::new_from_raw_parts(2, Vec::new(), accounts); + let transaction = create_legacy_transaction( + num_signatures, + header, + account_keys.clone(), + vec![instr], + ); + let data = bincode::serialize(&transaction).unwrap(); + let view = TransactionView::try_new_unsanitized(data.as_ref()).unwrap(); + // Exactly 255 accounts must pass sanitization. + assert!(sanitize_instructions(&view, false, true).is_ok()); } } diff --git a/transaction-view/src/transaction_view.rs b/transaction-view/src/transaction_view.rs index a939aed5505..b87b6ed029a 100644 --- a/transaction-view/src/transaction_view.rs +++ b/transaction-view/src/transaction_view.rs @@ -42,8 +42,13 @@ impl TransactionView { pub fn sanitize( self, enable_static_instruction_limit: bool, + enable_instruction_accounts_limit: bool, ) -> Result> { - sanitize(&self, enable_static_instruction_limit)?; + sanitize( + &self, + enable_static_instruction_limit, + enable_instruction_accounts_limit, + )?; Ok(SanitizedTransactionView { data: self.data, frame: self.frame, @@ -53,9 +58,16 @@ impl TransactionView { impl TransactionView { /// Creates a new `TransactionView`, running sanitization checks. - pub fn try_new_sanitized(data: D, enable_static_instruction_limit: bool) -> Result { + pub fn try_new_sanitized( + data: D, + enable_static_instruction_limit: bool, + enable_instruction_accounts_limit: bool, + ) -> Result { let unsanitized_view = TransactionView::try_new_unsanitized(data)?; - unsanitized_view.sanitize(enable_static_instruction_limit) + unsanitized_view.sanitize( + enable_static_instruction_limit, + enable_instruction_accounts_limit, + ) } }