Skip to content

Commit

Permalink
generic verify_precompiles (anza-xyz#3055)
Browse files Browse the repository at this point in the history
  • Loading branch information
apfitzge authored and ray-kast committed Nov 27, 2024
1 parent 45d3097 commit 95c6239
Show file tree
Hide file tree
Showing 8 changed files with 177 additions and 5 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 2 additions & 1 deletion banks-server/src/banks_server.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use {
bank::{Bank, TransactionSimulationResult},
bank_forks::BankForks,
commitment::BlockCommitmentCache,
verify_precompiles::verify_precompiles,
},
solana_sdk::{
account::Account,
Expand Down Expand Up @@ -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(())
Expand Down
5 changes: 3 additions & 2 deletions core/src/banking_stage/consumer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -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(())
}
Expand Down Expand Up @@ -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).
Expand Down
3 changes: 2 additions & 1 deletion rpc/src/rpc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ use {
prioritization_fee_cache::PrioritizationFeeCache,
snapshot_config::SnapshotConfig,
snapshot_utils,
verify_precompiles::verify_precompiles,
},
solana_sdk::{
account::{AccountSharedData, ReadableAccount},
Expand Down Expand Up @@ -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());
}
}
Expand Down
1 change: 1 addition & 0 deletions runtime/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
Expand Down
3 changes: 2 additions & 1 deletion runtime/src/bank.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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},
Expand Down Expand Up @@ -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)
Expand Down
1 change: 1 addition & 0 deletions runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ pub mod stakes;
pub mod static_ids;
pub mod status_cache;
pub mod transaction_batch;
pub mod verify_precompiles;
pub mod vote_sender_types;

#[macro_use]
Expand Down
165 changes: 165 additions & 0 deletions runtime/src/verify_precompiles.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,165 @@
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 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
.verify(instruction.data, all_instruction_data, feature_set)
.map_err(|_| TransactionError::InvalidAccountIndex)?;
break;
}
}
}

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());
}
}

0 comments on commit 95c6239

Please sign in to comment.