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.
This commit is contained in:
Sean Young 2021-08-30 08:58:45 +01:00
parent f0be3e4ea9
commit d461a9ac10
7 changed files with 38 additions and 65 deletions

View File

@ -1,3 +1,5 @@
use solana_sdk::feature_set::FeatureSet;
use { use {
bincode::{deserialize, serialize}, bincode::{deserialize, serialize},
futures::{future, prelude::stream::StreamExt}, futures::{future, prelude::stream::StreamExt},
@ -135,15 +137,11 @@ impl BanksServer {
fn verify_transaction( fn verify_transaction(
transaction: &Transaction, transaction: &Transaction,
libsecp256k1_0_5_upgrade_enabled: bool, feature_set: &Arc<FeatureSet>,
libsecp256k1_fail_on_bad_count: bool,
) -> transaction::Result<()> { ) -> transaction::Result<()> {
if let Err(err) = transaction.verify() { if let Err(err) = transaction.verify() {
Err(err) Err(err)
} else if let Err(err) = transaction.verify_precompiles( } else if let Err(err) = transaction.verify_precompiles(feature_set) {
libsecp256k1_0_5_upgrade_enabled,
libsecp256k1_fail_on_bad_count,
) {
Err(err) Err(err)
} else { } else {
Ok(()) Ok(())
@ -236,11 +234,7 @@ impl Banks for BanksServer {
transaction: Transaction, transaction: Transaction,
commitment: CommitmentLevel, commitment: CommitmentLevel,
) -> Option<transaction::Result<()>> { ) -> Option<transaction::Result<()>> {
if let Err(err) = verify_transaction( if let Err(err) = verify_transaction(&transaction, &self.bank(commitment).feature_set) {
&transaction,
self.bank(commitment).libsecp256k1_0_5_upgrade_enabled(),
self.bank(commitment).libsecp256k1_fail_on_bad_count(),
) {
return Some(Err(err)); return Some(Err(err));
} }

View File

@ -32,6 +32,7 @@ use solana_sdk::{
Slot, DEFAULT_TICKS_PER_SLOT, MAX_PROCESSING_AGE, MAX_TRANSACTION_FORWARDING_DELAY, Slot, DEFAULT_TICKS_PER_SLOT, MAX_PROCESSING_AGE, MAX_TRANSACTION_FORWARDING_DELAY,
MAX_TRANSACTION_FORWARDING_DELAY_GPU, MAX_TRANSACTION_FORWARDING_DELAY_GPU,
}, },
feature_set,
message::Message, message::Message,
pubkey::Pubkey, pubkey::Pubkey,
short_vec::decode_shortu16_len, short_vec::decode_shortu16_len,
@ -1045,8 +1046,7 @@ impl BankingStage {
fn transactions_from_packets( fn transactions_from_packets(
msgs: &Packets, msgs: &Packets,
transaction_indexes: &[usize], transaction_indexes: &[usize],
libsecp256k1_0_5_upgrade_enabled: bool, feature_set: &Arc<feature_set::FeatureSet>,
libsecp256k1_fail_on_bad_count: bool,
cost_tracker: &Arc<RwLock<CostTracker>>, cost_tracker: &Arc<RwLock<CostTracker>>,
banking_stage_stats: &BankingStageStats, banking_stage_stats: &BankingStageStats,
demote_program_write_locks: bool, demote_program_write_locks: bool,
@ -1064,11 +1064,7 @@ impl BankingStage {
Err(TransactionError::UnsupportedVersion) Err(TransactionError::UnsupportedVersion)
}) })
.ok()?; .ok()?;
tx.verify_precompiles( tx.verify_precompiles(feature_set).ok()?;
libsecp256k1_0_5_upgrade_enabled,
libsecp256k1_fail_on_bad_count,
)
.ok()?;
Some((tx, *tx_index)) Some((tx, *tx_index))
}) })
.collect(); .collect();
@ -1163,8 +1159,7 @@ impl BankingStage {
Self::transactions_from_packets( Self::transactions_from_packets(
msgs, msgs,
&packet_indexes, &packet_indexes,
bank.libsecp256k1_0_5_upgrade_enabled(), &bank.feature_set,
bank.libsecp256k1_fail_on_bad_count(),
cost_tracker, cost_tracker,
banking_stage_stats, banking_stage_stats,
bank.demote_program_write_locks(), bank.demote_program_write_locks(),
@ -1270,8 +1265,7 @@ impl BankingStage {
Self::transactions_from_packets( Self::transactions_from_packets(
msgs, msgs,
transaction_indexes, transaction_indexes,
bank.libsecp256k1_0_5_upgrade_enabled(), &bank.feature_set,
bank.libsecp256k1_fail_on_bad_count(),
cost_tracker, cost_tracker,
banking_stage_stats, banking_stage_stats,
bank.demote_program_write_locks(), bank.demote_program_write_locks(),

View File

@ -15,6 +15,7 @@ pub fn process_instruction(
pub mod test { pub mod test {
use rand::{thread_rng, Rng}; use rand::{thread_rng, Rng};
use solana_sdk::{ use solana_sdk::{
feature_set,
hash::Hash, hash::Hash,
secp256k1_instruction::{ secp256k1_instruction::{
new_secp256k1_instruction, SecpSignatureOffsets, SIGNATURE_OFFSETS_SERIALIZED_SIZE, new_secp256k1_instruction, SecpSignatureOffsets, SIGNATURE_OFFSETS_SERIALIZED_SIZE,
@ -22,6 +23,7 @@ pub mod test {
signature::{Keypair, Signer}, signature::{Keypair, Signer},
transaction::Transaction, transaction::Transaction,
}; };
use std::sync::Arc;
#[test] #[test]
fn test_secp256k1() { fn test_secp256k1() {
@ -36,6 +38,14 @@ pub mod test {
let message_arr = b"hello"; let message_arr = b"hello";
let mut secp_instruction = new_secp256k1_instruction(&secp_privkey, message_arr); let mut secp_instruction = new_secp256k1_instruction(&secp_privkey, message_arr);
let mint_keypair = Keypair::new(); 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( let tx = Transaction::new_signed_with_payer(
&[secp_instruction.clone()], &[secp_instruction.clone()],
@ -44,7 +54,7 @@ pub mod test {
Hash::default(), 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()); let index = thread_rng().gen_range(0, secp_instruction.data.len());
secp_instruction.data[index] = secp_instruction.data[index].wrapping_add(12); secp_instruction.data[index] = secp_instruction.data[index].wrapping_add(12);
@ -54,6 +64,6 @@ pub mod test {
&[&mint_keypair], &[&mint_keypair],
Hash::default(), Hash::default(),
); );
assert!(tx.verify_precompiles(false, true).is_err()); assert!(tx.verify_precompiles(&feature_set).is_err());
} }
} }

View File

@ -56,6 +56,7 @@ use {
epoch_info::EpochInfo, epoch_info::EpochInfo,
epoch_schedule::EpochSchedule, epoch_schedule::EpochSchedule,
exit::Exit, exit::Exit,
feature_set,
hash::Hash, hash::Hash,
message::{Message, SanitizedMessage}, message::{Message, SanitizedMessage},
pubkey::Pubkey, pubkey::Pubkey,
@ -1969,17 +1970,13 @@ impl JsonRpcRequestProcessor {
fn verify_transaction( fn verify_transaction(
transaction: &SanitizedTransaction, transaction: &SanitizedTransaction,
libsecp256k1_0_5_upgrade_enabled: bool, feature_set: &Arc<feature_set::FeatureSet>,
libsecp256k1_fail_on_bad_count: bool,
) -> Result<()> { ) -> Result<()> {
if transaction.verify().is_err() { if transaction.verify().is_err() {
return Err(RpcCustomError::TransactionSignatureVerificationFailure.into()); return Err(RpcCustomError::TransactionSignatureVerificationFailure.into());
} }
if let Err(e) = transaction.verify_precompiles( if let Err(e) = transaction.verify_precompiles(feature_set) {
libsecp256k1_0_5_upgrade_enabled,
libsecp256k1_fail_on_bad_count,
) {
return Err(RpcCustomError::TransactionPrecompileVerificationFailure(e).into()); return Err(RpcCustomError::TransactionPrecompileVerificationFailure(e).into());
} }
@ -3347,11 +3344,7 @@ pub mod rpc_full {
} }
if !config.skip_preflight { if !config.skip_preflight {
if let Err(e) = verify_transaction( if let Err(e) = verify_transaction(&transaction, &preflight_bank.feature_set) {
&transaction,
preflight_bank.libsecp256k1_0_5_upgrade_enabled(),
preflight_bank.libsecp256k1_fail_on_bad_count(),
) {
return Err(e); return Err(e);
} }
@ -3437,11 +3430,7 @@ pub mod rpc_full {
let transaction = sanitize_transaction(unsanitized_tx)?; let transaction = sanitize_transaction(unsanitized_tx)?;
if config.sig_verify { if config.sig_verify {
verify_transaction( verify_transaction(&transaction, &bank.feature_set)?;
&transaction,
bank.libsecp256k1_0_5_upgrade_enabled(),
bank.libsecp256k1_fail_on_bad_count(),
)?;
} }
let TransactionSimulationResult { let TransactionSimulationResult {

View File

@ -4928,7 +4928,7 @@ impl Bank {
} }
if !skip_verification { if !skip_verification {
sanitized_tx.verify_precompiles(self.libsecp256k1_0_5_upgrade_enabled(), true)?; sanitized_tx.verify_precompiles(&self.feature_set)?;
} }
Ok(sanitized_tx) Ok(sanitized_tx)
@ -5363,16 +5363,6 @@ impl Bank {
.is_active(&feature_set::verify_tx_signatures_len::id()) .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 { pub fn merge_nonce_error_into_system_error(&self) -> bool {
self.feature_set self.feature_set
.is_active(&feature_set::merge_nonce_error_into_system_error::id()) .is_active(&feature_set::merge_nonce_error_into_system_error::id())

View File

@ -18,7 +18,9 @@ use {
}, },
serde::Serialize, serde::Serialize,
solana_program::{system_instruction::SystemInstruction, system_program}, solana_program::{system_instruction::SystemInstruction, system_program},
solana_sdk::feature_set,
std::result, std::result,
std::sync::Arc,
thiserror::Error, thiserror::Error,
}; };
@ -426,11 +428,7 @@ impl Transaction {
.collect() .collect()
} }
pub fn verify_precompiles( pub fn verify_precompiles(&self, feature_set: &Arc<feature_set::FeatureSet>) -> Result<()> {
&self,
libsecp256k1_0_5_upgrade_enabled: bool,
libsecp256k1_fail_on_bad_count: bool,
) -> Result<()> {
for instruction in &self.message().instructions { for instruction in &self.message().instructions {
// The Transaction may not be sanitized at this point // The Transaction may not be sanitized at this point
if instruction.program_id_index as usize >= self.message().account_keys.len() { if instruction.program_id_index as usize >= self.message().account_keys.len() {
@ -448,8 +446,8 @@ impl Transaction {
let e = verify_eth_addresses( let e = verify_eth_addresses(
data, data,
&instruction_datas, &instruction_datas,
libsecp256k1_0_5_upgrade_enabled, feature_set.is_active(&feature_set::libsecp256k1_0_5_upgrade_enabled::id()),
libsecp256k1_fail_on_bad_count, feature_set.is_active(&feature_set::libsecp256k1_fail_on_bad_count::id()),
); );
e.map_err(|_| TransactionError::InvalidAccountIndex)?; e.map_err(|_| TransactionError::InvalidAccountIndex)?;
} }

View File

@ -11,10 +11,12 @@ use {
secp256k1_instruction::verify_eth_addresses, secp256k1_instruction::verify_eth_addresses,
secp256k1_program, secp256k1_program,
signature::Signature, signature::Signature,
solana_sdk::feature_set,
transaction::{Result, Transaction, TransactionError, VersionedTransaction}, transaction::{Result, Transaction, TransactionError, VersionedTransaction},
}, },
solana_program::{system_instruction::SystemInstruction, system_program}, solana_program::{system_instruction::SystemInstruction, system_program},
std::convert::TryFrom, std::convert::TryFrom,
std::sync::Arc,
}; };
/// Sanitized transaction and the hash of its message /// Sanitized transaction and the hash of its message
@ -203,11 +205,7 @@ impl SanitizedTransaction {
} }
/// Verify the encoded secp256k1 signatures in this transaction /// Verify the encoded secp256k1 signatures in this transaction
pub fn verify_precompiles( pub fn verify_precompiles(&self, feature_set: &Arc<feature_set::FeatureSet>) -> Result<()> {
&self,
libsecp256k1_0_5_upgrade_enabled: bool,
libsecp256k1_fail_on_bad_count: bool,
) -> Result<()> {
for (program_id, instruction) in self.message.program_instructions_iter() { for (program_id, instruction) in self.message.program_instructions_iter() {
if secp256k1_program::check_id(program_id) { if secp256k1_program::check_id(program_id) {
let instruction_datas: Vec<_> = self let instruction_datas: Vec<_> = self
@ -220,8 +218,8 @@ impl SanitizedTransaction {
let e = verify_eth_addresses( let e = verify_eth_addresses(
data, data,
&instruction_datas, &instruction_datas,
libsecp256k1_0_5_upgrade_enabled, feature_set.is_active(&feature_set::libsecp256k1_0_5_upgrade_enabled::id()),
libsecp256k1_fail_on_bad_count, feature_set.is_active(&feature_set::libsecp256k1_fail_on_bad_count::id()),
); );
e.map_err(|_| TransactionError::InvalidAccountIndex)?; e.map_err(|_| TransactionError::InvalidAccountIndex)?;
} }