From d461a9ac10d2f67b55a655d01ed20f272d34851f Mon Sep 17 00:00:00 2001 From: Sean Young Date: Mon, 30 Aug 2021 08:58:45 +0100 Subject: [PATCH] verify_precompiles needs FeatureSet Rather than pass in individual features, pass in the entire feature set so that we can add the ed25519 program feature in a later commit. --- banks-server/src/banks_server.rs | 16 +++++----------- core/src/banking_stage.rs | 16 +++++----------- programs/secp256k1/src/lib.rs | 14 ++++++++++++-- rpc/src/rpc.rs | 21 +++++---------------- runtime/src/bank.rs | 12 +----------- sdk/src/transaction/mod.rs | 12 +++++------- sdk/src/transaction/sanitized.rs | 12 +++++------- 7 files changed, 38 insertions(+), 65 deletions(-) diff --git a/banks-server/src/banks_server.rs b/banks-server/src/banks_server.rs index bf219fbf0a..4054ee4fed 100644 --- a/banks-server/src/banks_server.rs +++ b/banks-server/src/banks_server.rs @@ -1,3 +1,5 @@ +use solana_sdk::feature_set::FeatureSet; + use { bincode::{deserialize, serialize}, futures::{future, prelude::stream::StreamExt}, @@ -135,15 +137,11 @@ impl BanksServer { fn verify_transaction( transaction: &Transaction, - libsecp256k1_0_5_upgrade_enabled: bool, - libsecp256k1_fail_on_bad_count: bool, + feature_set: &Arc, ) -> transaction::Result<()> { if let Err(err) = transaction.verify() { Err(err) - } else if let Err(err) = transaction.verify_precompiles( - libsecp256k1_0_5_upgrade_enabled, - libsecp256k1_fail_on_bad_count, - ) { + } else if let Err(err) = transaction.verify_precompiles(feature_set) { Err(err) } else { Ok(()) @@ -236,11 +234,7 @@ impl Banks for BanksServer { transaction: Transaction, commitment: CommitmentLevel, ) -> Option> { - if let Err(err) = verify_transaction( - &transaction, - self.bank(commitment).libsecp256k1_0_5_upgrade_enabled(), - self.bank(commitment).libsecp256k1_fail_on_bad_count(), - ) { + if let Err(err) = verify_transaction(&transaction, &self.bank(commitment).feature_set) { return Some(Err(err)); } diff --git a/core/src/banking_stage.rs b/core/src/banking_stage.rs index 16ea95fab6..7976d09e2e 100644 --- a/core/src/banking_stage.rs +++ b/core/src/banking_stage.rs @@ -32,6 +32,7 @@ use solana_sdk::{ Slot, DEFAULT_TICKS_PER_SLOT, MAX_PROCESSING_AGE, MAX_TRANSACTION_FORWARDING_DELAY, MAX_TRANSACTION_FORWARDING_DELAY_GPU, }, + feature_set, message::Message, pubkey::Pubkey, short_vec::decode_shortu16_len, @@ -1045,8 +1046,7 @@ impl BankingStage { fn transactions_from_packets( msgs: &Packets, transaction_indexes: &[usize], - libsecp256k1_0_5_upgrade_enabled: bool, - libsecp256k1_fail_on_bad_count: bool, + feature_set: &Arc, cost_tracker: &Arc>, banking_stage_stats: &BankingStageStats, demote_program_write_locks: bool, @@ -1064,11 +1064,7 @@ impl BankingStage { Err(TransactionError::UnsupportedVersion) }) .ok()?; - tx.verify_precompiles( - libsecp256k1_0_5_upgrade_enabled, - libsecp256k1_fail_on_bad_count, - ) - .ok()?; + tx.verify_precompiles(feature_set).ok()?; Some((tx, *tx_index)) }) .collect(); @@ -1163,8 +1159,7 @@ impl BankingStage { Self::transactions_from_packets( msgs, &packet_indexes, - bank.libsecp256k1_0_5_upgrade_enabled(), - bank.libsecp256k1_fail_on_bad_count(), + &bank.feature_set, cost_tracker, banking_stage_stats, bank.demote_program_write_locks(), @@ -1270,8 +1265,7 @@ impl BankingStage { Self::transactions_from_packets( msgs, transaction_indexes, - bank.libsecp256k1_0_5_upgrade_enabled(), - bank.libsecp256k1_fail_on_bad_count(), + &bank.feature_set, cost_tracker, banking_stage_stats, bank.demote_program_write_locks(), diff --git a/programs/secp256k1/src/lib.rs b/programs/secp256k1/src/lib.rs index a54163f4d9..de879e0924 100644 --- a/programs/secp256k1/src/lib.rs +++ b/programs/secp256k1/src/lib.rs @@ -15,6 +15,7 @@ pub fn process_instruction( pub mod test { use rand::{thread_rng, Rng}; use solana_sdk::{ + feature_set, hash::Hash, secp256k1_instruction::{ new_secp256k1_instruction, SecpSignatureOffsets, SIGNATURE_OFFSETS_SERIALIZED_SIZE, @@ -22,6 +23,7 @@ pub mod test { signature::{Keypair, Signer}, transaction::Transaction, }; + use std::sync::Arc; #[test] fn test_secp256k1() { @@ -36,6 +38,14 @@ pub mod test { let message_arr = b"hello"; let mut secp_instruction = new_secp256k1_instruction(&secp_privkey, message_arr); let mint_keypair = Keypair::new(); + let mut feature_set = feature_set::FeatureSet::all_enabled(); + feature_set + .active + .remove(&feature_set::libsecp256k1_0_5_upgrade_enabled::id()); + feature_set + .inactive + .insert(feature_set::libsecp256k1_0_5_upgrade_enabled::id()); + let feature_set = Arc::new(feature_set); let tx = Transaction::new_signed_with_payer( &[secp_instruction.clone()], @@ -44,7 +54,7 @@ pub mod test { Hash::default(), ); - assert!(tx.verify_precompiles(false, true).is_ok()); + assert!(tx.verify_precompiles(&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); @@ -54,6 +64,6 @@ pub mod test { &[&mint_keypair], Hash::default(), ); - assert!(tx.verify_precompiles(false, true).is_err()); + assert!(tx.verify_precompiles(&feature_set).is_err()); } } diff --git a/rpc/src/rpc.rs b/rpc/src/rpc.rs index 1cd73ca5a7..c850d394bb 100644 --- a/rpc/src/rpc.rs +++ b/rpc/src/rpc.rs @@ -56,6 +56,7 @@ use { epoch_info::EpochInfo, epoch_schedule::EpochSchedule, exit::Exit, + feature_set, hash::Hash, message::{Message, SanitizedMessage}, pubkey::Pubkey, @@ -1969,17 +1970,13 @@ impl JsonRpcRequestProcessor { fn verify_transaction( transaction: &SanitizedTransaction, - libsecp256k1_0_5_upgrade_enabled: bool, - libsecp256k1_fail_on_bad_count: bool, + feature_set: &Arc, ) -> Result<()> { if transaction.verify().is_err() { return Err(RpcCustomError::TransactionSignatureVerificationFailure.into()); } - if let Err(e) = transaction.verify_precompiles( - libsecp256k1_0_5_upgrade_enabled, - libsecp256k1_fail_on_bad_count, - ) { + if let Err(e) = transaction.verify_precompiles(feature_set) { return Err(RpcCustomError::TransactionPrecompileVerificationFailure(e).into()); } @@ -3347,11 +3344,7 @@ pub mod rpc_full { } if !config.skip_preflight { - if let Err(e) = verify_transaction( - &transaction, - preflight_bank.libsecp256k1_0_5_upgrade_enabled(), - preflight_bank.libsecp256k1_fail_on_bad_count(), - ) { + if let Err(e) = verify_transaction(&transaction, &preflight_bank.feature_set) { return Err(e); } @@ -3437,11 +3430,7 @@ pub mod rpc_full { let transaction = sanitize_transaction(unsanitized_tx)?; if config.sig_verify { - verify_transaction( - &transaction, - bank.libsecp256k1_0_5_upgrade_enabled(), - bank.libsecp256k1_fail_on_bad_count(), - )?; + verify_transaction(&transaction, &bank.feature_set)?; } let TransactionSimulationResult { diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index d3f1b1eda2..cecddf24ea 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -4928,7 +4928,7 @@ impl Bank { } if !skip_verification { - sanitized_tx.verify_precompiles(self.libsecp256k1_0_5_upgrade_enabled(), true)?; + sanitized_tx.verify_precompiles(&self.feature_set)?; } Ok(sanitized_tx) @@ -5363,16 +5363,6 @@ impl Bank { .is_active(&feature_set::verify_tx_signatures_len::id()) } - pub fn libsecp256k1_0_5_upgrade_enabled(&self) -> bool { - self.feature_set - .is_active(&feature_set::libsecp256k1_0_5_upgrade_enabled::id()) - } - - pub fn libsecp256k1_fail_on_bad_count(&self) -> bool { - self.feature_set - .is_active(&feature_set::libsecp256k1_fail_on_bad_count::id()) - } - pub fn merge_nonce_error_into_system_error(&self) -> bool { self.feature_set .is_active(&feature_set::merge_nonce_error_into_system_error::id()) diff --git a/sdk/src/transaction/mod.rs b/sdk/src/transaction/mod.rs index c4cdcdcf6c..d24d64753e 100644 --- a/sdk/src/transaction/mod.rs +++ b/sdk/src/transaction/mod.rs @@ -18,7 +18,9 @@ use { }, serde::Serialize, solana_program::{system_instruction::SystemInstruction, system_program}, + solana_sdk::feature_set, std::result, + std::sync::Arc, thiserror::Error, }; @@ -426,11 +428,7 @@ impl Transaction { .collect() } - pub fn verify_precompiles( - &self, - libsecp256k1_0_5_upgrade_enabled: bool, - libsecp256k1_fail_on_bad_count: bool, - ) -> Result<()> { + pub fn verify_precompiles(&self, feature_set: &Arc) -> Result<()> { for instruction in &self.message().instructions { // The Transaction may not be sanitized at this point if instruction.program_id_index as usize >= self.message().account_keys.len() { @@ -448,8 +446,8 @@ impl Transaction { let e = verify_eth_addresses( data, &instruction_datas, - libsecp256k1_0_5_upgrade_enabled, - libsecp256k1_fail_on_bad_count, + feature_set.is_active(&feature_set::libsecp256k1_0_5_upgrade_enabled::id()), + feature_set.is_active(&feature_set::libsecp256k1_fail_on_bad_count::id()), ); e.map_err(|_| TransactionError::InvalidAccountIndex)?; } diff --git a/sdk/src/transaction/sanitized.rs b/sdk/src/transaction/sanitized.rs index d3ffc77af6..e7e38c8519 100644 --- a/sdk/src/transaction/sanitized.rs +++ b/sdk/src/transaction/sanitized.rs @@ -11,10 +11,12 @@ use { secp256k1_instruction::verify_eth_addresses, secp256k1_program, signature::Signature, + solana_sdk::feature_set, transaction::{Result, Transaction, TransactionError, VersionedTransaction}, }, solana_program::{system_instruction::SystemInstruction, system_program}, std::convert::TryFrom, + std::sync::Arc, }; /// Sanitized transaction and the hash of its message @@ -203,11 +205,7 @@ impl SanitizedTransaction { } /// Verify the encoded secp256k1 signatures in this transaction - pub fn verify_precompiles( - &self, - libsecp256k1_0_5_upgrade_enabled: bool, - libsecp256k1_fail_on_bad_count: bool, - ) -> Result<()> { + pub fn verify_precompiles(&self, feature_set: &Arc) -> Result<()> { for (program_id, instruction) in self.message.program_instructions_iter() { if secp256k1_program::check_id(program_id) { let instruction_datas: Vec<_> = self @@ -220,8 +218,8 @@ impl SanitizedTransaction { let e = verify_eth_addresses( data, &instruction_datas, - libsecp256k1_0_5_upgrade_enabled, - libsecp256k1_fail_on_bad_count, + feature_set.is_active(&feature_set::libsecp256k1_0_5_upgrade_enabled::id()), + feature_set.is_active(&feature_set::libsecp256k1_fail_on_bad_count::id()), ); e.map_err(|_| TransactionError::InvalidAccountIndex)?; }