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.

(cherry picked from commit 0f62771f42)

 Conflicts:
	banks-server/src/banks_server.rs
	core/src/banking_stage.rs
	programs/secp256k1/src/lib.rs
	rpc/src/rpc.rs
	runtime/src/bank.rs
	sdk/src/transaction.rs
	sdk/src/transaction/sanitized.rs
This commit is contained in:
Sean Young
2021-08-30 08:58:45 +01:00
parent 5350250a06
commit c8f6a0817b
8 changed files with 67 additions and 54 deletions

View File

@ -12,6 +12,7 @@ use solana_sdk::{
account::Account,
clock::Slot,
commitment_config::CommitmentLevel,
feature_set::FeatureSet,
fee_calculator::FeeCalculator,
hash::Hash,
pubkey::Pubkey,
@ -142,11 +143,11 @@ impl BanksServer {
fn verify_transaction(
transaction: &Transaction,
libsecp256k1_0_5_upgrade_enabled: bool,
feature_set: &Arc<FeatureSet>,
) -> transaction::Result<()> {
if let Err(err) = transaction.verify() {
Err(err)
} else if let Err(err) = transaction.verify_precompiles(libsecp256k1_0_5_upgrade_enabled) {
} else if let Err(err) = transaction.verify_precompiles(feature_set) {
Err(err)
} else {
Ok(())
@ -236,10 +237,7 @@ impl Banks for BanksServer {
transaction: Transaction,
commitment: CommitmentLevel,
) -> Option<transaction::Result<()>> {
if let Err(err) = verify_transaction(
&transaction,
self.bank(commitment).libsecp256k1_0_5_upgrade_enabled(),
) {
if let Err(err) = verify_transaction(&transaction, &self.bank(commitment).feature_set) {
return Some(Err(err));
}

View File

@ -35,6 +35,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,
@ -1129,7 +1130,7 @@ impl BankingStage {
fn transactions_from_packets(
msgs: &Packets,
transaction_indexes: &[usize],
libsecp256k1_0_5_upgrade_enabled: bool,
feature_set: &Arc<feature_set::FeatureSet>,
votes_only: bool,
cost_tracker: &Arc<RwLock<CostTracker>>,
banking_stage_stats: &BankingStageStats,
@ -1147,8 +1148,7 @@ impl BankingStage {
}
let tx: Transaction = limited_deserialize(&p.data[0..p.meta.size]).ok()?;
tx.verify_precompiles(libsecp256k1_0_5_upgrade_enabled)
.ok()?;
tx.verify_precompiles(feature_set).ok()?;
Some((tx, *tx_index))
})
@ -1270,7 +1270,7 @@ impl BankingStage {
Self::transactions_from_packets(
msgs,
&packet_indexes,
bank.libsecp256k1_0_5_upgrade_enabled(),
&bank.feature_set,
bank.vote_only_bank(),
cost_tracker,
banking_stage_stats,
@ -1380,7 +1380,7 @@ impl BankingStage {
Self::transactions_from_packets(
msgs,
&transaction_indexes,
bank.libsecp256k1_0_5_upgrade_enabled(),
&bank.feature_set,
bank.vote_only_bank(),
cost_tracker,
banking_stage_stats,
@ -3204,6 +3204,7 @@ mod tests {
&keypair,
None,
);
let features = Arc::new(feature_set::FeatureSet::default());
// packets with no votes
{
@ -3212,10 +3213,11 @@ mod tests {
make_test_packets(vec![transfer_tx.clone(), transfer_tx.clone()], vote_indexes);
let mut votes_only = false;
let (txs, tx_packet_index, _) = BankingStage::transactions_from_packets(
&packets,
&packet_indexes,
false,
&features,
votes_only,
&Arc::new(RwLock::new(CostTracker::new(Arc::new(RwLock::new(
CostModel::default(),
@ -3231,7 +3233,7 @@ mod tests {
let (txs, tx_packet_index, _) = BankingStage::transactions_from_packets(
&packets,
&packet_indexes,
false,
&features,
votes_only,
&Arc::new(RwLock::new(CostTracker::new(Arc::new(RwLock::new(
CostModel::default(),
@ -3256,7 +3258,7 @@ mod tests {
let (txs, tx_packet_index, _) = BankingStage::transactions_from_packets(
&packets,
&packet_indexes,
false,
&features,
votes_only,
&Arc::new(RwLock::new(CostTracker::new(Arc::new(RwLock::new(
CostModel::default(),
@ -3272,7 +3274,7 @@ mod tests {
let (txs, tx_packet_index, _) = BankingStage::transactions_from_packets(
&packets,
&packet_indexes,
false,
&features,
votes_only,
&Arc::new(RwLock::new(CostTracker::new(Arc::new(RwLock::new(
CostModel::default(),
@ -3292,12 +3294,13 @@ mod tests {
vec![vote_tx.clone(), vote_tx.clone(), vote_tx],
vote_indexes,
);
let features = Arc::new(feature_set::FeatureSet::default());
let mut votes_only = false;
let (txs, tx_packet_index, _) = BankingStage::transactions_from_packets(
&packets,
&packet_indexes,
false,
&features,
votes_only,
&Arc::new(RwLock::new(CostTracker::new(Arc::new(RwLock::new(
CostModel::default(),
@ -3313,7 +3316,7 @@ mod tests {
let (txs, tx_packet_index, _) = BankingStage::transactions_from_packets(
&packets,
&packet_indexes,
false,
&features,
votes_only,
&Arc::new(RwLock::new(CostTracker::new(Arc::new(RwLock::new(
CostModel::default(),

View File

@ -872,11 +872,7 @@ pub fn confirm_slot(
};
let check_start = Instant::now();
let check_result = entries.verify_and_hash_transactions(
skip_verification,
bank.libsecp256k1_0_5_upgrade_enabled(),
bank.verify_tx_signatures_len_enabled(),
);
let check_result = entries.verify_and_hash_transactions(skip_verification, &bank.feature_set);
if check_result.is_none() {
warn!("Ledger proof of history failed at slot: {}", slot);
return Err(BlockError::InvalidEntryHash.into());

View File

@ -22,6 +22,7 @@ use solana_sdk::hash::Hash;
use solana_sdk::packet::PACKET_DATA_SIZE;
use solana_sdk::timing;
use solana_sdk::transaction::Transaction;
use solana_sdk::{feature_set, feature_set::FeatureSet};
use std::borrow::Cow;
use std::cell::RefCell;
use std::ffi::OsStr;
@ -359,8 +360,7 @@ pub trait EntrySlice {
fn verify_and_hash_transactions(
&self,
skip_verification: bool,
libsecp256k1_0_5_upgrade_enabled: bool,
verify_tx_signatures_len: bool,
feature_set: &Arc<FeatureSet>,
) -> Option<Vec<EntryType<'_>>>;
}
@ -515,8 +515,7 @@ impl EntrySlice for [Entry] {
fn verify_and_hash_transactions<'a>(
&'a self,
skip_verification: bool,
libsecp256k1_0_5_upgrade_enabled: bool,
verify_tx_signatures_len: bool,
feature_set: &Arc<FeatureSet>,
) -> Option<Vec<EntryType<'a>>> {
let verify_and_hash = |tx: &'a Transaction| -> Option<HashedTransaction<'a>> {
let message_hash = if !skip_verification {
@ -524,9 +523,10 @@ impl EntrySlice for [Entry] {
if size > PACKET_DATA_SIZE as u64 {
return None;
}
tx.verify_precompiles(libsecp256k1_0_5_upgrade_enabled)
.ok()?;
if verify_tx_signatures_len && !tx.verify_signatures_len() {
tx.verify_precompiles(feature_set).ok()?;
if feature_set.is_active(&feature_set::verify_tx_signatures_len::id())
&& !tx.verify_signatures_len()
{
return None;
}
tx.verify_and_hash_message().ok()?
@ -924,22 +924,34 @@ mod tests {
{
let tx = make_transaction(TestCase::RemoveSignature);
let entries = vec![next_entry(&recent_blockhash, 1, vec![tx])];
let features = Arc::new(feature_set::FeatureSet::default());
assert!(entries[..]
.verify_and_hash_transactions(false, false, false)
.verify_and_hash_transactions(false, &features)
.is_some());
let mut features = feature_set::FeatureSet::default();
features
.active
.insert(feature_set::verify_tx_signatures_len::id(), 0);
let features = Arc::new(features);
assert!(entries[..]
.verify_and_hash_transactions(false, false, true)
.verify_and_hash_transactions(false, &features)
.is_none());
}
// Too many signatures.
{
let tx = make_transaction(TestCase::AddSignature);
let entries = vec![next_entry(&recent_blockhash, 1, vec![tx])];
let features = Arc::new(feature_set::FeatureSet::default());
assert!(entries[..]
.verify_and_hash_transactions(false, false, false)
.verify_and_hash_transactions(false, &features)
.is_some());
let mut features = feature_set::FeatureSet::default();
features
.active
.insert(feature_set::verify_tx_signatures_len::id(), 0);
let features = Arc::new(features);
assert!(entries[..]
.verify_and_hash_transactions(false, false, true)
.verify_and_hash_transactions(false, &features)
.is_none());
}
}
@ -950,6 +962,7 @@ mod tests {
let recent_blockhash = hash_new_rand(&mut rng);
let keypair = Keypair::new();
let pubkey = keypair.pubkey();
let features = Arc::new(feature_set::FeatureSet::default());
let make_transaction = |size| {
let ixs: Vec<_> = std::iter::repeat_with(|| {
system_instruction::transfer(&pubkey, &Pubkey::new_unique(), 1)
@ -965,7 +978,7 @@ mod tests {
let entries = vec![next_entry(&recent_blockhash, 1, vec![tx.clone()])];
assert!(bincode::serialized_size(&tx).unwrap() <= PACKET_DATA_SIZE as u64);
assert!(entries[..]
.verify_and_hash_transactions(false, false, false)
.verify_and_hash_transactions(false, &features)
.is_some());
}
// Big transaction.
@ -974,7 +987,7 @@ mod tests {
let entries = vec![next_entry(&recent_blockhash, 1, vec![tx.clone()])];
assert!(bincode::serialized_size(&tx).unwrap() > PACKET_DATA_SIZE as u64);
assert!(entries[..]
.verify_and_hash_transactions(false, false, false)
.verify_and_hash_transactions(false, &features)
.is_none());
}
// Assert that verify fails as soon as serialized
@ -985,7 +998,7 @@ mod tests {
assert_eq!(
bincode::serialized_size(&tx).unwrap() <= PACKET_DATA_SIZE as u64,
entries[..]
.verify_and_hash_transactions(false, false, false)
.verify_and_hash_transactions(false, &features)
.is_some(),
);
}

View File

@ -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).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).is_err());
assert!(tx.verify_precompiles(&feature_set).is_err());
}
}

View File

@ -58,6 +58,7 @@ use {
epoch_info::EpochInfo,
epoch_schedule::EpochSchedule,
exit::Exit,
feature_set,
hash::Hash,
pubkey::Pubkey,
sanitize::Sanitize,
@ -1937,13 +1938,13 @@ fn optimize_filters(filters: &mut Vec<RpcFilterType>) {
fn verify_transaction(
transaction: &Transaction,
libsecp256k1_0_5_upgrade_enabled: bool,
feature_set: &Arc<feature_set::FeatureSet>,
) -> Result<()> {
if transaction.verify().is_err() {
return Err(RpcCustomError::TransactionSignatureVerificationFailure.into());
}
if let Err(e) = transaction.verify_precompiles(libsecp256k1_0_5_upgrade_enabled) {
if let Err(e) = transaction.verify_precompiles(feature_set) {
return Err(RpcCustomError::TransactionPrecompileVerificationFailure(e).into());
}
@ -3097,10 +3098,7 @@ pub mod rpc_full {
}
if !config.skip_preflight {
if let Err(e) = verify_transaction(
&transaction,
preflight_bank.libsecp256k1_0_5_upgrade_enabled(),
) {
if let Err(e) = verify_transaction(&transaction, &preflight_bank.feature_set) {
return Err(e);
}
@ -3172,9 +3170,7 @@ pub mod rpc_full {
));
}
if let Err(e) =
verify_transaction(&transaction, bank.libsecp256k1_0_5_upgrade_enabled())
{
if let Err(e) = verify_transaction(&transaction, &bank.feature_set) {
return Err(e);
}
}

View File

@ -5609,11 +5609,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 merge_nonce_error_into_system_error(&self) -> bool {
self.feature_set
.is_active(&feature_set::merge_nonce_error_into_system_error::id())

View File

@ -5,6 +5,7 @@
use crate::sanitize::{Sanitize, SanitizeError};
use crate::secp256k1_instruction::verify_eth_addresses;
use crate::{
feature_set,
hash::Hash,
instruction::{CompiledInstruction, Instruction, InstructionError},
message::Message,
@ -18,6 +19,7 @@ use crate::{
system_program,
};
use std::result;
use std::sync::Arc;
use thiserror::Error;
/// Reasons a transaction might be rejected.
@ -409,7 +411,7 @@ impl Transaction {
.collect()
}
pub fn verify_precompiles(&self, libsecp256k1_0_5_upgrade_enabled: bool) -> Result<()> {
pub fn verify_precompiles(&self, feature_set: &Arc<feature_set::FeatureSet>) -> 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() {
@ -427,7 +429,7 @@ impl Transaction {
let e = verify_eth_addresses(
data,
&instruction_datas,
libsecp256k1_0_5_upgrade_enabled,
feature_set.is_active(&feature_set::libsecp256k1_0_5_upgrade_enabled::id()),
);
e.map_err(|_| TransactionError::InvalidAccountIndex)?;
}