From 3a5f523f4f379c6b5846882b0f807967e457bc0b Mon Sep 17 00:00:00 2001 From: Andrew Fitzgerald Date: Wed, 2 Oct 2024 09:25:59 -0500 Subject: [PATCH 1/4] generic verify_precompiles --- runtime/src/lib.rs | 1 + runtime/src/verify_precompiles.rs | 28 ++++++++++++++++++++++++++++ 2 files changed, 29 insertions(+) create mode 100644 runtime/src/verify_precompiles.rs diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index bd11e97668eec0..0d83d9bad5eda9 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -36,6 +36,7 @@ pub mod stakes; pub mod static_ids; pub mod status_cache; pub mod transaction_batch; +mod verify_precompiles; pub mod vote_sender_types; #[macro_use] diff --git a/runtime/src/verify_precompiles.rs b/runtime/src/verify_precompiles.rs new file mode 100644 index 00000000000000..6d203d856c14e4 --- /dev/null +++ b/runtime/src/verify_precompiles.rs @@ -0,0 +1,28 @@ +use { + solana_feature_set::FeatureSet, + solana_sdk::{ + precompiles::get_precompiles, + transaction::{Result, TransactionError}, + }, + solana_svm_transaction::svm_message::SVMMessage, +}; + +pub fn verify_precompiles(message: &impl SVMMessage, feature_set: &FeatureSet) -> Result<()> { + let mut all_instruction_data = None; // lazily collect this on first pre-compile + + let precompiles = get_precompiles(); + for (program_id, instruction) in message.program_instructions_iter() { + for precompile in precompiles { + if program_id == &precompile.program_id { + let all_instruction_data: &Vec<&[u8]> = all_instruction_data + .get_or_insert_with(|| message.instructions_iter().map(|ix| ix.data).collect()); + precompile + .verify(instruction.data, all_instruction_data, feature_set) + .map_err(|_| TransactionError::InvalidAccountIndex)?; + break; + } + } + } + + Ok(()) +} From d6121a6292f31eac1d7bab8bfa224827a67ef4f7 Mon Sep 17 00:00:00 2001 From: Andrew Fitzgerald Date: Wed, 2 Oct 2024 09:28:52 -0500 Subject: [PATCH 2/4] use verify_precompiles --- banks-server/src/banks_server.rs | 3 ++- core/src/banking_stage/consumer.rs | 5 +++-- rpc/src/rpc.rs | 3 ++- runtime/src/bank.rs | 3 ++- runtime/src/lib.rs | 2 +- 5 files changed, 10 insertions(+), 6 deletions(-) diff --git a/banks-server/src/banks_server.rs b/banks-server/src/banks_server.rs index 7051daac45cb0f..f2d2d10da85abb 100644 --- a/banks-server/src/banks_server.rs +++ b/banks-server/src/banks_server.rs @@ -13,6 +13,7 @@ use { bank::{Bank, TransactionSimulationResult}, bank_forks::BankForks, commitment::BlockCommitmentCache, + verify_precompiles::verify_precompiles, }, solana_sdk::{ account::Account, @@ -167,7 +168,7 @@ fn verify_transaction( let move_precompile_verification_to_svm = feature_set.is_active(&move_precompile_verification_to_svm::id()); if !move_precompile_verification_to_svm { - transaction.verify_precompiles(feature_set)?; + verify_precompiles(transaction, feature_set)?; } Ok(()) diff --git a/core/src/banking_stage/consumer.rs b/core/src/banking_stage/consumer.rs index 4bd6f50cd18b65..57f1b1958b152c 100644 --- a/core/src/banking_stage/consumer.rs +++ b/core/src/banking_stage/consumer.rs @@ -21,6 +21,7 @@ use { solana_runtime::{ bank::{Bank, LoadAndExecuteTransactionsOutput}, transaction_batch::TransactionBatch, + verify_precompiles::verify_precompiles, }, solana_runtime_transaction::instructions_processor::process_compute_budget_instructions, solana_sdk::{ @@ -401,7 +402,7 @@ impl Consumer { .map(|(tx, result)| match result { Ok(_) => { if !move_precompile_verification_to_svm { - tx.verify_precompiles(&bank.feature_set) + verify_precompiles(tx, &bank.feature_set) } else { Ok(()) } @@ -452,7 +453,7 @@ impl Consumer { } else { // Verify pre-compiles. if !move_precompile_verification_to_svm { - tx.verify_precompiles(&bank.feature_set)?; + verify_precompiles(tx, &bank.feature_set)?; } // Any transaction executed between sanitization time and now may have closed the lookup table(s). diff --git a/rpc/src/rpc.rs b/rpc/src/rpc.rs index 08fe5f3355f7af..9f6de648a40e30 100644 --- a/rpc/src/rpc.rs +++ b/rpc/src/rpc.rs @@ -59,6 +59,7 @@ use { prioritization_fee_cache::PrioritizationFeeCache, snapshot_config::SnapshotConfig, snapshot_utils, + verify_precompiles::verify_precompiles, }, solana_sdk::{ account::{AccountSharedData, ReadableAccount}, @@ -2260,7 +2261,7 @@ fn verify_transaction( let move_precompile_verification_to_svm = feature_set.is_active(&feature_set::move_precompile_verification_to_svm::id()); if !move_precompile_verification_to_svm { - if let Err(e) = transaction.verify_precompiles(feature_set) { + if let Err(e) = verify_precompiles(transaction, feature_set) { return Err(RpcCustomError::TransactionPrecompileVerificationFailure(e).into()); } } diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 77661a4bc43cf2..f74de8b2ffec64 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -57,6 +57,7 @@ use { stakes::{InvalidCacheEntryReason, Stakes, StakesCache, StakesEnum}, status_cache::{SlotDelta, StatusCache}, transaction_batch::{OwnedOrBorrowed, TransactionBatch}, + verify_precompiles::verify_precompiles, }, byteorder::{ByteOrder, LittleEndian}, dashmap::{DashMap, DashSet}, @@ -5664,7 +5665,7 @@ impl Bank { verification_mode == TransactionVerificationMode::HashAndVerifyPrecompiles || verification_mode == TransactionVerificationMode::FullVerification } { - sanitized_tx.verify_precompiles(&self.feature_set)?; + verify_precompiles(&sanitized_tx, &self.feature_set)?; } Ok(sanitized_tx) diff --git a/runtime/src/lib.rs b/runtime/src/lib.rs index 0d83d9bad5eda9..21947a64e79d02 100644 --- a/runtime/src/lib.rs +++ b/runtime/src/lib.rs @@ -36,7 +36,7 @@ pub mod stakes; pub mod static_ids; pub mod status_cache; pub mod transaction_batch; -mod verify_precompiles; +pub mod verify_precompiles; pub mod vote_sender_types; #[macro_use] From 5997ccbfc4c6134a65f81fc6bdd259f5acf693ae Mon Sep 17 00:00:00 2001 From: Andrew Fitzgerald Date: Wed, 2 Oct 2024 10:05:57 -0500 Subject: [PATCH 3/4] tests --- Cargo.lock | 1 + runtime/Cargo.toml | 1 + runtime/src/verify_precompiles.rs | 137 ++++++++++++++++++++++++++++++ 3 files changed, 139 insertions(+) diff --git a/Cargo.lock b/Cargo.lock index 65d7ce94aaee19..daf8aa27390450 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7652,6 +7652,7 @@ dependencies = [ "num_enum", "percentage", "qualifier_attr", + "rand 0.7.3", "rand 0.8.5", "rand_chacha 0.3.1", "rayon", diff --git a/runtime/Cargo.toml b/runtime/Cargo.toml index 014995d592b612..35e23a8a194a99 100644 --- a/runtime/Cargo.toml +++ b/runtime/Cargo.toml @@ -107,6 +107,7 @@ assert_matches = { workspace = true } ed25519-dalek = { workspace = true } libsecp256k1 = { workspace = true } memoffset = { workspace = true } +rand0-7 = { package = "rand", version = "0.7" } rand_chacha = { workspace = true } solana-accounts-db = { workspace = true, features = ["dev-context-only-utils"] } solana-logger = { workspace = true } diff --git a/runtime/src/verify_precompiles.rs b/runtime/src/verify_precompiles.rs index 6d203d856c14e4..2256526b19f5c2 100644 --- a/runtime/src/verify_precompiles.rs +++ b/runtime/src/verify_precompiles.rs @@ -26,3 +26,140 @@ pub fn verify_precompiles(message: &impl SVMMessage, feature_set: &FeatureSet) - Ok(()) } + +#[cfg(test)] +mod tests { + use { + super::*, + rand0_7::{thread_rng, Rng}, + solana_sdk::{ + ed25519_instruction::new_ed25519_instruction, + hash::Hash, + pubkey::Pubkey, + secp256k1_instruction::new_secp256k1_instruction, + signature::Keypair, + signer::Signer, + system_instruction, system_transaction, + transaction::{SanitizedTransaction, Transaction}, + }, + }; + + #[test] + fn test_verify_precompiles_simple_transaction() { + let tx = SanitizedTransaction::from_transaction_for_tests(system_transaction::transfer( + &Keypair::new(), + &Pubkey::new_unique(), + 1, + Hash::default(), + )); + assert!(verify_precompiles(&tx, &FeatureSet::all_enabled()).is_ok()); + } + + #[test] + fn test_verify_precompiles_secp256k1() { + let secp_privkey = libsecp256k1::SecretKey::random(&mut thread_rng()); + let message_arr = b"hello"; + let mut secp_instruction = new_secp256k1_instruction(&secp_privkey, message_arr); + let mint_keypair = Keypair::new(); + let feature_set = FeatureSet::all_enabled(); + + let tx = + SanitizedTransaction::from_transaction_for_tests(Transaction::new_signed_with_payer( + &[secp_instruction.clone()], + Some(&mint_keypair.pubkey()), + &[&mint_keypair], + Hash::default(), + )); + + assert!(verify_precompiles(&tx, &feature_set).is_ok()); + + let index = thread_rng().gen_range(0, secp_instruction.data.len()); + secp_instruction.data[index] = secp_instruction.data[index].wrapping_add(12); + let tx = + SanitizedTransaction::from_transaction_for_tests(Transaction::new_signed_with_payer( + &[secp_instruction], + Some(&mint_keypair.pubkey()), + &[&mint_keypair], + Hash::default(), + )); + + assert!(verify_precompiles(&tx, &feature_set).is_err()); + } + + #[test] + fn test_verify_precompiles_ed25519() { + let privkey = ed25519_dalek::Keypair::generate(&mut thread_rng()); + let message_arr = b"hello"; + let mut instruction = new_ed25519_instruction(&privkey, message_arr); + let mint_keypair = Keypair::new(); + let feature_set = FeatureSet::all_enabled(); + + let tx = + SanitizedTransaction::from_transaction_for_tests(Transaction::new_signed_with_payer( + &[instruction.clone()], + Some(&mint_keypair.pubkey()), + &[&mint_keypair], + Hash::default(), + )); + + assert!(verify_precompiles(&tx, &feature_set).is_ok()); + + let index = loop { + let index = thread_rng().gen_range(0, instruction.data.len()); + // byte 1 is not used, so this would not cause the verify to fail + if index != 1 { + break index; + } + }; + + instruction.data[index] = instruction.data[index].wrapping_add(12); + let tx = + SanitizedTransaction::from_transaction_for_tests(Transaction::new_signed_with_payer( + &[instruction], + Some(&mint_keypair.pubkey()), + &[&mint_keypair], + Hash::default(), + )); + assert!(verify_precompiles(&tx, &feature_set).is_err()); + } + + #[test] + fn test_verify_precompiles_mixed() { + let message_arr = b"hello"; + let secp_privkey = libsecp256k1::SecretKey::random(&mut thread_rng()); + let mut secp_instruction = new_secp256k1_instruction(&secp_privkey, message_arr); + let ed25519_privkey = ed25519_dalek::Keypair::generate(&mut thread_rng()); + let ed25519_instruction = new_ed25519_instruction(&ed25519_privkey, message_arr); + + let mint_keypair = Keypair::new(); + let feature_set = FeatureSet::all_enabled(); + + let tx = + SanitizedTransaction::from_transaction_for_tests(Transaction::new_signed_with_payer( + &[ + secp_instruction.clone(), + ed25519_instruction.clone(), + system_instruction::transfer(&mint_keypair.pubkey(), &Pubkey::new_unique(), 1), + ], + Some(&mint_keypair.pubkey()), + &[&mint_keypair], + Hash::default(), + )); + assert!(verify_precompiles(&tx, &feature_set).is_ok()); + + let index = thread_rng().gen_range(0, secp_instruction.data.len()); + secp_instruction.data[index] = secp_instruction.data[index].wrapping_add(12); + let tx = + SanitizedTransaction::from_transaction_for_tests(Transaction::new_signed_with_payer( + &[ + secp_instruction, + ed25519_instruction, + system_instruction::transfer(&mint_keypair.pubkey(), &Pubkey::new_unique(), 1), + ], + Some(&mint_keypair.pubkey()), + &[&mint_keypair], + Hash::default(), + )); + assert!(verify_precompiles(&tx, &feature_set).is_err()); + } +} From c55d2b4903a7dae054c329e24e0bf1801900e82e Mon Sep 17 00:00:00 2001 From: Andrew Fitzgerald Date: Thu, 3 Oct 2024 15:39:47 -0500 Subject: [PATCH 4/4] check feature --- runtime/src/verify_precompiles.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runtime/src/verify_precompiles.rs b/runtime/src/verify_precompiles.rs index 2256526b19f5c2..69022785af4d1f 100644 --- a/runtime/src/verify_precompiles.rs +++ b/runtime/src/verify_precompiles.rs @@ -13,7 +13,7 @@ pub fn verify_precompiles(message: &impl SVMMessage, feature_set: &FeatureSet) - let precompiles = get_precompiles(); for (program_id, instruction) in message.program_instructions_iter() { for precompile in precompiles { - if program_id == &precompile.program_id { + if precompile.check_id(program_id, |id| feature_set.is_active(id)) { let all_instruction_data: &Vec<&[u8]> = all_instruction_data .get_or_insert_with(|| message.instructions_iter().map(|ix| ix.data).collect()); precompile