diff --git a/Cargo.lock b/Cargo.lock index 6b4d9f1f0b7..d083163ed62 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -442,6 +442,7 @@ dependencies = [ "solana-svm-transaction", "solana-system-interface", "solana-transaction", + "solana-transaction-context", ] [[package]] @@ -10442,6 +10443,7 @@ dependencies = [ "solana-system-interface", "solana-system-transaction", "solana-transaction", + "solana-transaction-context", "solana-transaction-error", "solana-vote-interface", "thiserror 2.0.16", diff --git a/banks-server/src/banks_server.rs b/banks-server/src/banks_server.rs index 8cadef88582..85c6bb1d3f4 100644 --- a/banks-server/src/banks_server.rs +++ b/banks-server/src/banks_server.rs @@ -174,6 +174,8 @@ fn simulate_transaction( Some(false), // is_simple_vote_tx bank, bank.get_reserved_account_keys(), + bank.feature_set + .is_active(&agave_feature_set::static_instruction_limit::id()), ) { Err(err) => { return BanksTransactionResultWithSimulation { diff --git a/core/src/banking_stage/consume_worker.rs b/core/src/banking_stage/consume_worker.rs index 3d616e30f68..5446db7ecf5 100644 --- a/core/src/banking_stage/consume_worker.rs +++ b/core/src/banking_stage/consume_worker.rs @@ -1207,6 +1207,8 @@ mod tests { None, loader, &HashSet::default(), + bank.feature_set + .is_active(&agave_feature_set::static_instruction_limit::id()), ) .unwrap() }; diff --git a/core/src/banking_stage/consumer.rs b/core/src/banking_stage/consumer.rs index 060cb3394c1..4852cbbfc3e 100644 --- a/core/src/banking_stage/consumer.rs +++ b/core/src/banking_stage/consumer.rs @@ -1789,6 +1789,8 @@ mod tests { Some(false), bank.as_ref(), &ReservedAccountKeys::empty_key_set(), + bank.feature_set + .is_active(&agave_feature_set::static_instruction_limit::id()), ) .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 a60702b34af..a6bd32d8eac 100644 --- a/core/src/banking_stage/transaction_scheduler/receive_and_buffer.rs +++ b/core/src/banking_stage/transaction_scheduler/receive_and_buffer.rs @@ -667,7 +667,7 @@ impl TransactionViewReceiveAndBuffer { transaction_account_lock_limit: usize, ) -> Result { // Parsing and basic sanitization checks - let Ok(view) = SanitizedTransactionView::try_new_sanitized(bytes) else { + let Ok(view) = SanitizedTransactionView::try_new_sanitized(bytes, working_bank.feature_set.is_active(&agave_feature_set::static_instruction_limit::id())) 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 dad2bfd2506..0e7bea5268f 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).unwrap(); + let view = SanitizedTransactionView::try_new_sanitized(data, true).unwrap(); let view = RuntimeTransaction::>::try_from( view, MessageHash::Compute, diff --git a/core/src/forwarding_stage.rs b/core/src/forwarding_stage.rs index 7c92b401744..c29c86f1c1a 100644 --- a/core/src/forwarding_stage.rs +++ b/core/src/forwarding_stage.rs @@ -291,6 +291,9 @@ impl is_tpu_vote_batch: bool, bank: &Bank, ) { + let enable_static_instruction_limit = bank + .feature_set + .is_active(&agave_feature_set::static_instruction_limit::id()); for batch in packet_batches.iter() { for packet in batch .iter() @@ -311,19 +314,21 @@ impl // Perform basic sanitization checks and calculate priority. // If any steps fail, drop the packet. - let Some(priority) = SanitizedTransactionView::try_new_sanitized(packet_data) + let Some(priority) = SanitizedTransactionView::try_new_sanitized( + packet_data, + enable_static_instruction_limit, + ) + .map_err(|_| ()) + .and_then(|transaction| { + RuntimeTransaction::>::try_from( + transaction, + MessageHash::Compute, + Some(packet.meta().is_simple_vote_tx()), + ) .map_err(|_| ()) - .and_then(|transaction| { - RuntimeTransaction::>::try_from( - transaction, - MessageHash::Compute, - Some(packet.meta().is_simple_vote_tx()), - ) - .map_err(|_| ()) - }) - .ok() - .and_then(|transaction| calculate_priority(&transaction, bank)) - else { + }) + .ok() + .and_then(|transaction| calculate_priority(&transaction, bank)) else { self.metrics.votes_dropped_on_receive += vote_count; self.metrics.non_votes_dropped_on_receive += non_vote_count; continue; diff --git a/cost-model/src/transaction_cost.rs b/cost-model/src/transaction_cost.rs index f28325d0784..0450d4213b6 100644 --- a/cost-model/src/transaction_cost.rs +++ b/cost-model/src/transaction_cost.rs @@ -341,6 +341,7 @@ mod tests { Some(true), SimpleAddressLoader::Disabled, &ReservedAccountKeys::empty_key_set(), + true, ) .unwrap(); @@ -360,6 +361,7 @@ mod tests { Some(false), SimpleAddressLoader::Disabled, &ReservedAccountKeys::empty_key_set(), + true, ) .unwrap(); diff --git a/entry/benches/entry_sigverify.rs b/entry/benches/entry_sigverify.rs index 12b30e23200..8648ada1bb0 100644 --- a/entry/benches/entry_sigverify.rs +++ b/entry/benches/entry_sigverify.rs @@ -45,6 +45,7 @@ fn bench_gpusigverify(bencher: &mut Bencher) { None, SimpleAddressLoader::Disabled, &ReservedAccountKeys::empty_key_set(), + true, ) }?; @@ -89,6 +90,7 @@ fn bench_cpusigverify(bencher: &mut Bencher) { None, SimpleAddressLoader::Disabled, &ReservedAccountKeys::empty_key_set(), + true, ) }?; diff --git a/entry/src/entry.rs b/entry/src/entry.rs index ba0c7fa068d..7a7364d6744 100644 --- a/entry/src/entry.rs +++ b/entry/src/entry.rs @@ -1076,6 +1076,7 @@ mod tests { None, SimpleAddressLoader::Disabled, &ReservedAccountKeys::empty_key_set(), + true, ) }?; diff --git a/feature-set/src/lib.rs b/feature-set/src/lib.rs index f4d6967b711..7da3aae04ac 100644 --- a/feature-set/src/lib.rs +++ b/feature-set/src/lib.rs @@ -1137,6 +1137,10 @@ pub mod provide_instruction_data_offset_in_vm_r2 { solana_pubkey::declare_id!("5xXZc66h4UdB6Yq7FzdBxBiRAFMMScMLwHxk2QZDaNZL"); } +pub mod static_instruction_limit { + solana_pubkey::declare_id!("64ixypL1HPu8WtJhNSMb9mSgfFaJvsANuRkTbHyuLfnx"); +} + pub static FEATURE_NAMES: LazyLock> = LazyLock::new(|| { [ (secp256k1_program_enabled::id(), "secp256k1 program"), @@ -2054,6 +2058,10 @@ pub static FEATURE_NAMES: LazyLock> = LazyLock::n provide_instruction_data_offset_in_vm_r2::id(), "SIMD-0321: Provide instruction data offset in VM r2", ), + ( + static_instruction_limit::id(), + "SIMD-0160: static instruction limit", + ), /*************** ADD NEW FEATURES HERE ***************/ ] .iter() diff --git a/ledger-tool/src/main.rs b/ledger-tool/src/main.rs index 4cadb5559e8..6159599b748 100644 --- a/ledger-tool/src/main.rs +++ b/ledger-tool/src/main.rs @@ -470,6 +470,7 @@ fn compute_slot_cost( None, SimpleAddressLoader::Disabled, &reserved_account_keys.active, + feature_set.is_active(&agave_feature_set::static_instruction_limit::id()), ) .map_err(|err| { warn!("Failed to compute cost of transaction: {err:?}"); diff --git a/programs/sbf/Cargo.lock b/programs/sbf/Cargo.lock index c59b0cee3bf..a4fbeac1b15 100644 --- a/programs/sbf/Cargo.lock +++ b/programs/sbf/Cargo.lock @@ -193,6 +193,7 @@ dependencies = [ "solana-short-vec", "solana-signature", "solana-svm-transaction", + "solana-transaction-context", ] [[package]] @@ -8207,6 +8208,7 @@ dependencies = [ "solana-signature", "solana-svm-transaction", "solana-transaction", + "solana-transaction-context", "solana-transaction-error", "thiserror 2.0.16", ] diff --git a/rpc/src/rpc.rs b/rpc/src/rpc.rs index f884657ee3b..5a58b230df2 100644 --- a/rpc/src/rpc.rs +++ b/rpc/src/rpc.rs @@ -3838,6 +3838,9 @@ pub mod rpc_full { unsanitized_tx, preflight_bank, preflight_bank.get_reserved_account_keys(), + preflight_bank + .feature_set + .is_active(&agave_feature_set::static_instruction_limit::id()), )?; let blockhash = *transaction.message().recent_blockhash(); let message_hash = *transaction.message_hash(); @@ -3988,8 +3991,13 @@ pub mod rpc_full { }); } - let transaction = - sanitize_transaction(unsanitized_tx, bank, bank.get_reserved_account_keys())?; + let transaction = sanitize_transaction( + unsanitized_tx, + bank, + bank.get_reserved_account_keys(), + bank.feature_set + .is_active(&agave_feature_set::static_instruction_limit::id()), + )?; if sig_verify { verify_transaction(&transaction)?; } @@ -4391,6 +4399,7 @@ fn sanitize_transaction( transaction: VersionedTransaction, address_loader: impl AddressLoader, reserved_account_keys: &HashSet, + enable_static_instruction_limit: bool, ) -> Result> { RuntimeTransaction::try_create( transaction, @@ -4398,6 +4407,7 @@ fn sanitize_transaction( None, address_loader, reserved_account_keys, + enable_static_instruction_limit, ) .map_err(|err| Error::invalid_params(format!("invalid transaction: {err}"))) } @@ -9097,7 +9107,8 @@ pub mod tests { sanitize_transaction( unsanitary_versioned_tx, SimpleAddressLoader::Disabled, - &ReservedAccountKeys::empty_key_set() + &ReservedAccountKeys::empty_key_set(), + true, ) .unwrap_err(), expect58 @@ -9122,7 +9133,8 @@ pub mod tests { sanitize_transaction( versioned_tx, SimpleAddressLoader::Disabled, - &ReservedAccountKeys::empty_key_set() + &ReservedAccountKeys::empty_key_set(), + true, ) .unwrap_err(), Error::invalid_params( diff --git a/runtime-transaction/Cargo.toml b/runtime-transaction/Cargo.toml index 3e3542baa1a..f9974cb3708 100644 --- a/runtime-transaction/Cargo.toml +++ b/runtime-transaction/Cargo.toml @@ -31,6 +31,7 @@ solana-sdk-ids = { workspace = true } solana-signature = { workspace = true } solana-svm-transaction = { workspace = true } solana-transaction = { workspace = true } +solana-transaction-context = { workspace = true } solana-transaction-error = { workspace = true } thiserror = { workspace = true } diff --git a/runtime-transaction/src/runtime_transaction/sdk_transactions.rs b/runtime-transaction/src/runtime_transaction/sdk_transactions.rs index f8662918ff1..79e21702665 100644 --- a/runtime-transaction/src/runtime_transaction/sdk_transactions.rs +++ b/runtime-transaction/src/runtime_transaction/sdk_transactions.rs @@ -80,7 +80,14 @@ impl RuntimeTransaction { is_simple_vote_tx: Option, address_loader: impl AddressLoader, reserved_account_keys: &HashSet, + enable_static_instruction_limit: bool, ) -> Result { + if enable_static_instruction_limit + && tx.message.instructions().len() + > solana_transaction_context::MAX_INSTRUCTION_TRACE_LENGTH + { + return Err(solana_transaction_error::TransactionError::SanitizeFailure); + } let statically_loaded_runtime_tx = RuntimeTransaction::::try_from( SanitizedVersionedTransaction::try_from(tx)?, @@ -142,12 +149,14 @@ impl TransactionWithMeta for RuntimeTransaction { 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; Self::try_create( versioned_transaction, MessageHash::Compute, None, solana_message::SimpleAddressLoader::Disabled, &HashSet::new(), + enable_static_instruction_limit, ) .expect("failed to create RuntimeTransaction from Transaction") } diff --git a/runtime-transaction/src/runtime_transaction/transaction_view.rs b/runtime-transaction/src/runtime_transaction/transaction_view.rs index 5f41103cf2e..20ff7e92ed1 100644 --- a/runtime-transaction/src/runtime_transaction/transaction_view.rs +++ b/runtime-transaction/src/runtime_transaction/transaction_view.rs @@ -220,7 +220,7 @@ mod tests { let hash = Hash::new_unique(); let transaction = - SanitizedTransactionView::try_new_sanitized(&serialized_transaction[..]).unwrap(); + SanitizedTransactionView::try_new_sanitized(&serialized_transaction[..], true).unwrap(); let static_runtime_transaction = RuntimeTransaction::>::try_from( transaction, @@ -252,7 +252,8 @@ mod tests { reserved_account_keys: &HashSet, ) { let bytes = bincode::serialize(&original_transaction).unwrap(); - let transaction_view = SanitizedTransactionView::try_new_sanitized(&bytes[..]).unwrap(); + let transaction_view = + SanitizedTransactionView::try_new_sanitized(&bytes[..], true).unwrap(); let runtime_transaction = RuntimeTransaction::>::try_from( transaction_view, MessageHash::Compute, @@ -318,7 +319,8 @@ mod tests { ) { let bytes = bincode::serialize(&original_transaction.to_versioned_transaction()).unwrap(); - let transaction_view = SanitizedTransactionView::try_new_sanitized(&bytes[..]).unwrap(); + let transaction_view = + SanitizedTransactionView::try_new_sanitized(&bytes[..], true).unwrap(); let runtime_transaction = RuntimeTransaction::>::try_from( transaction_view, MessageHash::Compute, diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index efddab55348..0d5266230a2 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -2901,6 +2901,9 @@ impl Bank { &self, txs: Vec, ) -> Result>> { + let enable_static_instruction_limit = self + .feature_set + .is_active(&agave_feature_set::static_instruction_limit::id()); let sanitized_txs = txs .into_iter() .map(|tx| { @@ -2910,6 +2913,7 @@ impl Bank { None, self, self.get_reserved_account_keys(), + enable_static_instruction_limit, ) }) .collect::>>()?; @@ -4591,6 +4595,9 @@ impl Bank { tx: VersionedTransaction, verification_mode: TransactionVerificationMode, ) -> Result> { + let enable_static_instruction_limit = self + .feature_set + .is_active(&agave_feature_set::static_instruction_limit::id()); let sanitized_tx = { let size = bincode::serialized_size(&tx).map_err(|_| TransactionError::SanitizeFailure)?; @@ -4599,6 +4606,13 @@ impl Bank { } let message_hash = if verification_mode == TransactionVerificationMode::FullVerification { + // SIMD-0160, check instruction limit before signature verificaton + if enable_static_instruction_limit + && tx.message.instructions().len() + > solana_transaction_context::MAX_INSTRUCTION_TRACE_LENGTH + { + return Err(solana_transaction_error::TransactionError::SanitizeFailure); + } tx.verify_and_hash_message()? } else { tx.message.hash() @@ -4610,6 +4624,7 @@ impl Bank { None, self, self.get_reserved_account_keys(), + enable_static_instruction_limit, ) }?; diff --git a/runtime/src/bank/tests.rs b/runtime/src/bank/tests.rs index 81149df6acd..3186a961055 100644 --- a/runtime/src/bank/tests.rs +++ b/runtime/src/bank/tests.rs @@ -9131,6 +9131,51 @@ fn test_verify_transactions_packet_data_size() { } } +#[test_case(false; "pre_simd160_static_instruction_limit")] +#[test_case(true; "simd160_static_instruction_limit")] +fn test_verify_transactions_instruction_limit(simd_0160_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_0160_enabled { + bank.deactivate_feature(&feature_set::static_instruction_limit::id()); + } + + let recent_blockhash = Hash::new_unique(); + let keypair = Keypair::new(); + let pubkey = keypair.pubkey(); + let ix_count = 65; + let ixs: Vec<_> = std::iter::repeat_with(|| CompiledInstruction { + program_id_index: 1, + accounts: vec![0], + data: vec![], + }) + .take(ix_count) + .collect(); + let message = Message::new_with_compiled_instructions( + 1, + 0, + 1, + vec![pubkey, Pubkey::new_unique()], + recent_blockhash, + ixs, + ); + let tx = Transaction::new(&[&keypair], message, recent_blockhash); + + assert!(bincode::serialized_size(&tx).unwrap() <= PACKET_DATA_SIZE as u64); + + if simd_0160_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/Cargo.toml b/transaction-view/Cargo.toml index 256136558ea..8216725548e 100644 --- a/transaction-view/Cargo.toml +++ b/transaction-view/Cargo.toml @@ -21,6 +21,7 @@ solana-sdk-ids = { workspace = true } solana-short-vec = { workspace = true } solana-signature = { workspace = true } solana-svm-transaction = { workspace = true } +solana-transaction-context = { workspace = true } [dev-dependencies] # See order-crates-for-publishing.py for using this unusual `path = "."` diff --git a/transaction-view/benches/transaction_view.rs b/transaction-view/benches/transaction_view.rs index 815cf59d407..a38cf90998f 100644 --- a/transaction-view/benches/transaction_view.rs +++ b/transaction-view/benches/transaction_view.rs @@ -64,7 +64,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())).unwrap(); + let _ = + TransactionView::try_new_sanitized(black_box(bytes.as_ref()), true).unwrap(); } }); }); @@ -132,8 +133,8 @@ fn packed_transfers() -> Vec { fn packed_noops() -> Vec { // Creating noop instructions to maximize the number of instructions per - // transaction. We can fit up to 355 noops. - const MAX_INSTRUCTIONS_PER_TRANSACTION: usize = 355; + // transaction. We are allowed to fit up to 64 instructions per transaction. + const MAX_INSTRUCTIONS_PER_TRANSACTION: usize = 64; (0..NUM_TRANSACTIONS) .map(|_| { diff --git a/transaction-view/src/resolved_transaction_view.rs b/transaction-view/src/resolved_transaction_view.rs index 584c95c9a4b..379f9cab6d5 100644 --- a/transaction-view/src/resolved_transaction_view.rs +++ b/transaction-view/src/resolved_transaction_view.rs @@ -284,7 +284,7 @@ mod tests { }), }; let bytes = bincode::serialize(&transaction).unwrap(); - let view = SanitizedTransactionView::try_new_sanitized(bytes.as_ref()).unwrap(); + let view = SanitizedTransactionView::try_new_sanitized(bytes.as_ref(), true).unwrap(); let result = ResolvedTransactionView::try_new(view, None, &HashSet::default()); assert!(matches!( result, @@ -315,7 +315,7 @@ mod tests { }), }; let bytes = bincode::serialize(&transaction).unwrap(); - let view = SanitizedTransactionView::try_new_sanitized(bytes.as_ref()).unwrap(); + let view = SanitizedTransactionView::try_new_sanitized(bytes.as_ref(), true).unwrap(); let result = ResolvedTransactionView::try_new(view, Some(loaded_addresses), &HashSet::default()); assert!(matches!( @@ -352,7 +352,7 @@ mod tests { }), }; let bytes = bincode::serialize(&transaction).unwrap(); - let view = SanitizedTransactionView::try_new_sanitized(bytes.as_ref()).unwrap(); + let view = SanitizedTransactionView::try_new_sanitized(bytes.as_ref(), true).unwrap(); let result = ResolvedTransactionView::try_new(view, Some(loaded_addresses), &HashSet::default()); assert!(matches!( @@ -402,7 +402,7 @@ 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()).unwrap(); + let view = SanitizedTransactionView::try_new_sanitized(bytes.as_ref(), true).unwrap(); let resolved_view = ResolvedTransactionView::try_new( view, Some(loaded_addresses), @@ -425,7 +425,7 @@ 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()).unwrap(); + let view = SanitizedTransactionView::try_new_sanitized(bytes.as_ref(), true).unwrap(); let resolved_view = ResolvedTransactionView::try_new( view, Some(loaded_addresses), @@ -448,7 +448,7 @@ 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()).unwrap(); + let view = SanitizedTransactionView::try_new_sanitized(bytes.as_ref(), true).unwrap(); let resolved_view = ResolvedTransactionView::try_new( view, Some(loaded_addresses), @@ -509,7 +509,7 @@ 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()).unwrap(); + let view = SanitizedTransactionView::try_new_sanitized(bytes.as_ref(), true).unwrap(); let resolved_view = ResolvedTransactionView::try_new( view, Some(loaded_addresses.clone()), @@ -528,7 +528,7 @@ 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()).unwrap(); + let view = SanitizedTransactionView::try_new_sanitized(bytes.as_ref(), true).unwrap(); let resolved_view = ResolvedTransactionView::try_new( view, Some(loaded_addresses.clone()), @@ -551,7 +551,7 @@ 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()).unwrap(); + let view = SanitizedTransactionView::try_new_sanitized(bytes.as_ref(), true).unwrap(); let resolved_view = ResolvedTransactionView::try_new( view, diff --git a/transaction-view/src/sanitize.rs b/transaction-view/src/sanitize.rs index 992a24e3086..0da141c30af 100644 --- a/transaction-view/src/sanitize.rs +++ b/transaction-view/src/sanitize.rs @@ -4,10 +4,13 @@ use crate::{ transaction_view::UnsanitizedTransactionView, }; -pub(crate) fn sanitize(view: &UnsanitizedTransactionView) -> Result<()> { +pub(crate) fn sanitize( + view: &UnsanitizedTransactionView, + enable_static_instruction_limit: bool, +) -> Result<()> { sanitize_signatures(view)?; sanitize_account_access(view)?; - sanitize_instructions(view)?; + sanitize_instructions(view, enable_static_instruction_limit)?; sanitize_address_table_lookups(view) } @@ -51,7 +54,18 @@ fn sanitize_account_access(view: &UnsanitizedTransactionView) -> Result<()> { +fn sanitize_instructions( + view: &UnsanitizedTransactionView, + enable_static_instruction_limit: bool, +) -> Result<()> { + // SIMD-160: transaction can not have more than 64 top level instructions + if enable_static_instruction_limit + && usize::from(view.num_instructions()) + > solana_transaction_context::MAX_INSTRUCTION_TRACE_LENGTH + { + return Err(TransactionViewError::SanitizeError); + } + // already verified there is at least one static account. let max_program_id_index = view.num_static_account_keys().wrapping_sub(1); // verified that there are no more than 256 accounts in `sanitize_account_access` @@ -172,7 +186,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().is_ok()); + assert!(view.sanitize(true).is_ok()); } #[test] @@ -379,7 +393,7 @@ mod tests { ); let data = bincode::serialize(&transaction).unwrap(); let view = TransactionView::try_new_unsanitized(data.as_ref()).unwrap(); - assert!(sanitize_instructions(&view).is_ok()); + assert!(sanitize_instructions(&view, true).is_ok()); let transaction = create_v0_transaction( num_signatures, @@ -390,7 +404,7 @@ mod tests { ); let data = bincode::serialize(&transaction).unwrap(); let view = TransactionView::try_new_unsanitized(data.as_ref()).unwrap(); - assert!(sanitize_instructions(&view).is_ok()); + assert!(sanitize_instructions(&view, true).is_ok()); } for instruction_index in 0..valid_instructions.len() { @@ -407,7 +421,7 @@ mod tests { let data = bincode::serialize(&transaction).unwrap(); let view = TransactionView::try_new_unsanitized(data.as_ref()).unwrap(); assert_eq!( - sanitize_instructions(&view), + sanitize_instructions(&view, true), Err(TransactionViewError::SanitizeError) ); } @@ -426,7 +440,7 @@ mod tests { let data = bincode::serialize(&transaction).unwrap(); let view = TransactionView::try_new_unsanitized(data.as_ref()).unwrap(); assert_eq!( - sanitize_instructions(&view), + sanitize_instructions(&view, true), Err(TransactionViewError::SanitizeError) ); } @@ -444,7 +458,7 @@ mod tests { let data = bincode::serialize(&transaction).unwrap(); let view = TransactionView::try_new_unsanitized(data.as_ref()).unwrap(); assert_eq!( - sanitize_instructions(&view), + sanitize_instructions(&view, true), Err(TransactionViewError::SanitizeError) ); } @@ -464,7 +478,7 @@ mod tests { let data = bincode::serialize(&transaction).unwrap(); let view = TransactionView::try_new_unsanitized(data.as_ref()).unwrap(); assert_eq!( - sanitize_instructions(&view), + sanitize_instructions(&view, true), Err(TransactionViewError::SanitizeError) ); } @@ -488,11 +502,49 @@ mod tests { let data = bincode::serialize(&transaction).unwrap(); let view = TransactionView::try_new_unsanitized(data.as_ref()).unwrap(); assert_eq!( - sanitize_instructions(&view), + sanitize_instructions(&view, true), Err(TransactionViewError::SanitizeError) ); } } + + // SIMD-0160, too many instructions are invalid + { + let too_many_instructions: Vec<_> = valid_instructions + .iter() + .cycle() + .take(65) + .cloned() + .collect(); + let transaction = create_legacy_transaction( + num_signatures, + header, + account_keys.clone(), + too_many_instructions.clone(), + ); + let data = bincode::serialize(&transaction).unwrap(); + let view = TransactionView::try_new_unsanitized(data.as_ref()).unwrap(); + assert_eq!( + sanitize_instructions(&view, true), + Err(TransactionViewError::SanitizeError) + ); + assert!(sanitize_instructions(&view, false).is_ok()); + + let transaction = create_v0_transaction( + num_signatures, + header, + account_keys.clone(), + too_many_instructions.clone(), + atls.clone(), + ); + let data = bincode::serialize(&transaction).unwrap(); + let view = TransactionView::try_new_unsanitized(data.as_ref()).unwrap(); + assert_eq!( + sanitize_instructions(&view, true), + Err(TransactionViewError::SanitizeError) + ); + assert!(sanitize_instructions(&view, false).is_ok()); + } } #[test] diff --git a/transaction-view/src/transaction_view.rs b/transaction-view/src/transaction_view.rs index f15db6b95a9..47b7091ceb4 100644 --- a/transaction-view/src/transaction_view.rs +++ b/transaction-view/src/transaction_view.rs @@ -36,8 +36,11 @@ impl TransactionView { } /// Sanitizes the transaction view, returning a sanitized view on success. - pub fn sanitize(self) -> Result> { - sanitize(&self)?; + pub fn sanitize( + self, + enable_static_instruction_limit: bool, + ) -> Result> { + sanitize(&self, enable_static_instruction_limit)?; Ok(SanitizedTransactionView { data: self.data, frame: self.frame, @@ -47,9 +50,9 @@ impl TransactionView { impl TransactionView { /// Creates a new `TransactionView`, running sanitization checks. - pub fn try_new_sanitized(data: D) -> Result { + pub fn try_new_sanitized(data: D, enable_static_instruction_limit: bool) -> Result { let unsanitized_view = TransactionView::try_new_unsanitized(data)?; - unsanitized_view.sanitize() + unsanitized_view.sanitize(enable_static_instruction_limit) } }