Revert "Remove feature switch for secp256k1 program (#18467)"

This reverts commit fd574dcb3b.
This commit is contained in:
Trent Nelson 2021-07-13 14:20:02 -06:00 committed by mergify[bot]
parent c19fce1b19
commit 568660b402
10 changed files with 103 additions and 41 deletions

View File

@ -1059,6 +1059,7 @@ impl BankingStage {
fn transactions_from_packets(
msgs: &Packets,
transaction_indexes: &[usize],
secp256k1_program_enabled: bool,
cost_tracker: &Arc<RwLock<CostTracker>>,
banking_stage_stats: &BankingStageStats,
) -> (Vec<HashedTransaction<'static>>, Vec<usize>, Vec<usize>) {
@ -1069,7 +1070,9 @@ impl BankingStage {
.filter_map(|tx_index| {
let p = &msgs.packets[*tx_index];
let tx: Transaction = limited_deserialize(&p.data[0..p.meta.size]).ok()?;
tx.verify_precompiles().ok()?;
if secp256k1_program_enabled {
tx.verify_precompiles().ok()?;
}
Some((tx, *tx_index))
})
.collect();
@ -1177,6 +1180,7 @@ impl BankingStage {
Self::transactions_from_packets(
msgs,
&packet_indexes,
bank.secp256k1_program_enabled(),
cost_tracker,
banking_stage_stats,
);
@ -1288,6 +1292,7 @@ impl BankingStage {
Self::transactions_from_packets(
msgs,
transaction_indexes,
bank.secp256k1_program_enabled(),
cost_tracker,
banking_stage_stats,
);

View File

@ -359,6 +359,7 @@ pub trait EntrySlice {
fn verify_and_hash_transactions(
&self,
skip_verification: bool,
secp256k1_program_enabled: bool,
verify_tx_signatures_len: bool,
) -> Option<Vec<EntryType<'_>>>;
}
@ -514,6 +515,7 @@ impl EntrySlice for [Entry] {
fn verify_and_hash_transactions<'a>(
&'a self,
skip_verification: bool,
secp256k1_program_enabled: bool,
verify_tx_signatures_len: bool,
) -> Option<Vec<EntryType<'a>>> {
let verify_and_hash = |tx: &'a Transaction| -> Option<HashedTransaction<'a>> {
@ -522,7 +524,10 @@ impl EntrySlice for [Entry] {
if size > PACKET_DATA_SIZE as u64 {
return None;
}
tx.verify_precompiles().ok()?;
if secp256k1_program_enabled {
// Verify tx precompiles if secp256k1 program is enabled.
tx.verify_precompiles().ok()?;
}
if verify_tx_signatures_len && !tx.verify_signatures_len() {
return None;
}
@ -922,10 +927,10 @@ mod tests {
let tx = make_transaction(TestCase::RemoveSignature);
let entries = vec![next_entry(&recent_blockhash, 1, vec![tx])];
assert!(entries[..]
.verify_and_hash_transactions(false, false)
.verify_and_hash_transactions(false, false, false)
.is_some());
assert!(entries[..]
.verify_and_hash_transactions(false, true)
.verify_and_hash_transactions(false, false, true)
.is_none());
}
// Too many signatures.
@ -933,10 +938,10 @@ mod tests {
let tx = make_transaction(TestCase::AddSignature);
let entries = vec![next_entry(&recent_blockhash, 1, vec![tx])];
assert!(entries[..]
.verify_and_hash_transactions(false, false)
.verify_and_hash_transactions(false, false, false)
.is_some());
assert!(entries[..]
.verify_and_hash_transactions(false, true)
.verify_and_hash_transactions(false, false, true)
.is_none());
}
}
@ -962,7 +967,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)
.verify_and_hash_transactions(false, false, false)
.is_some());
}
// Big transaction.
@ -971,7 +976,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)
.verify_and_hash_transactions(false, false, false)
.is_none());
}
// Assert that verify fails as soon as serialized
@ -982,7 +987,7 @@ mod tests {
assert_eq!(
bincode::serialized_size(&tx).unwrap() <= PACKET_DATA_SIZE as u64,
entries[..]
.verify_and_hash_transactions(false, false)
.verify_and_hash_transactions(false, false, false)
.is_some(),
);
}

View File

@ -2404,13 +2404,13 @@ fn main() {
let mut store_failed_count = 0;
if force_enabled_count >= 1 {
if base_bank
.get_account(&feature_set::spl_token_v2_multisig_fix::id())
.get_account(&feature_set::secp256k1_program_enabled::id())
.is_some()
{
// steal some lamports from the pretty old feature not to affect
// capitalizaion, which doesn't affect inflation behavior!
base_bank.store_account(
&feature_set::spl_token_v2_multisig_fix::id(),
&feature_set::secp256k1_program_enabled::id(),
&AccountSharedData::default(),
);
force_enabled_count -= 1;

View File

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

View File

@ -25,7 +25,7 @@ use solana_sdk::{
bpf_loader_upgradeable::{self, UpgradeableLoaderState},
clock::{BankId, Slot, INITIAL_RENT_EPOCH},
feature_set::{self, FeatureSet},
fee_calculator::FeeCalculator,
fee_calculator::{FeeCalculator, FeeConfig},
genesis_config::ClusterType,
hash::Hash,
message::Message,
@ -424,6 +424,10 @@ impl Accounts {
rent_collector: &RentCollector,
feature_set: &FeatureSet,
) -> Vec<TransactionLoadResult> {
let fee_config = FeeConfig {
secp256k1_program_enabled: feature_set
.is_active(&feature_set::secp256k1_program_enabled::id()),
};
txs.zip(lock_results)
.map(|etx| match etx {
(tx, (Ok(()), nonce_rollback)) => {
@ -436,7 +440,7 @@ impl Accounts {
.cloned()
});
let fee = if let Some(fee_calculator) = fee_calculator {
fee_calculator.calculate_fee(tx.message())
fee_calculator.calculate_fee_with_config(tx.message(), &fee_config)
} else {
return (Err(TransactionError::BlockhashNotFound), None);
};

View File

@ -80,7 +80,7 @@ use solana_sdk::{
epoch_schedule::EpochSchedule,
feature,
feature_set::{self, FeatureSet},
fee_calculator::{FeeCalculator, FeeRateGovernor},
fee_calculator::{FeeCalculator, FeeConfig, FeeRateGovernor},
genesis_config::{ClusterType, GenesisConfig},
hard_forks::HardForks,
hash::{extend_and_hash, hashv, Hash},
@ -3374,6 +3374,10 @@ impl Bank {
let hash_queue = self.blockhash_queue.read().unwrap();
let mut fees = 0;
let fee_config = FeeConfig {
secp256k1_program_enabled: self.secp256k1_program_enabled(),
};
let results = txs
.zip(executed)
.map(|(tx, (res, nonce_rollback))| {
@ -3391,7 +3395,7 @@ impl Bank {
});
let fee_calculator = fee_calculator.ok_or(TransactionError::BlockhashNotFound)?;
let fee = fee_calculator.calculate_fee(tx.message());
let fee = fee_calculator.calculate_fee_with_config(tx.message(), &fee_config);
let message = tx.message();
match *res {
@ -5091,6 +5095,11 @@ impl Bank {
self.rc.accounts.accounts_db.shrink_candidate_slots()
}
pub fn secp256k1_program_enabled(&self) -> bool {
self.feature_set
.is_active(&feature_set::secp256k1_program_enabled::id())
}
pub fn no_overflow_rent_distribution_enabled(&self) -> bool {
self.feature_set
.is_active(&feature_set::no_overflow_rent_distribution::id())
@ -5713,7 +5722,7 @@ pub(crate) mod tests {
cluster_type: ClusterType::MainnetBeta,
..GenesisConfig::default()
}));
let sysvar_and_native_proram_delta0 = 11;
let sysvar_and_native_proram_delta0 = 10;
assert_eq!(
bank0.capitalization(),
42 * 42 + sysvar_and_native_proram_delta0
@ -7415,10 +7424,10 @@ pub(crate) mod tests {
// not being eagerly-collected for exact rewards calculation
bank0.restore_old_behavior_for_fragile_tests();
let sysvar_and_native_program_delta0 = 11;
let sysvar_and_native_proram_delta0 = 10;
assert_eq!(
bank0.capitalization(),
42 * 1_000_000_000 + sysvar_and_native_program_delta0
42 * 1_000_000_000 + sysvar_and_native_proram_delta0
);
assert!(bank0.rewards.read().unwrap().is_empty());
@ -7538,7 +7547,7 @@ pub(crate) mod tests {
// not being eagerly-collected for exact rewards calculation
bank.restore_old_behavior_for_fragile_tests();
let sysvar_and_native_proram_delta = 11;
let sysvar_and_native_proram_delta = 10;
assert_eq!(
bank.capitalization(),
42 * 1_000_000_000 + sysvar_and_native_proram_delta
@ -10774,25 +10783,25 @@ pub(crate) mod tests {
if bank.slot == 0 {
assert_eq!(
bank.hash().to_string(),
"BfvaoHkrQwrkQo7T1mW6jmJXveRy11rut8bva2H1Rt5H"
"Cn7Wmi7w1n9NbK7RGnTQ4LpbJ2LtoJoc1sufiTwb57Ya"
);
}
if bank.slot == 32 {
assert_eq!(
bank.hash().to_string(),
"JBGPApnSMPKZaYiR16v46XSSGcKxy8kCbVtN1CG1XDxW"
"BXupB8XsZukMTnDbKshJ8qPCydWnc8BKtSj7YTJ6gAH"
);
}
if bank.slot == 64 {
assert_eq!(
bank.hash().to_string(),
"BDCt9cGPfxpgJXzp8Tq1nX1zSqpbs8xrkAFyRhmXKiuX"
"EDkKefgSMSV1NhxnGnJP7R5AGZ2JZD6oxnoZtGuEGBCU"
);
}
if bank.slot == 128 {
assert_eq!(
bank.hash().to_string(),
"4zUpK4VUhKLaPUgeMMSeDR2w827goriRL5NndJxGDVmz"
"AtWu4tubU9zGFChfHtQghQx3RVWtMQu6Rj49rQymFc4z"
);
break;
}
@ -10942,7 +10951,7 @@ pub(crate) mod tests {
// No more slots should be shrunk
assert_eq!(bank2.shrink_candidate_slots(), 0);
// alive_counts represents the count of alive accounts in the three slots 0,1,2
assert_eq!(alive_counts, vec![10, 1, 7]);
assert_eq!(alive_counts, vec![9, 1, 7]);
}
#[test]
@ -10990,7 +10999,7 @@ pub(crate) mod tests {
.map(|_| bank.process_stale_slot_with_budget(0, force_to_return_alive_account))
.sum();
// consumed_budgets represents the count of alive accounts in the three slots 0,1,2
assert_eq!(consumed_budgets, 11);
assert_eq!(consumed_budgets, 10);
}
#[test]

View File

@ -3,6 +3,7 @@ use crate::{
system_instruction_processor,
};
use solana_sdk::{
feature_set,
instruction::InstructionError,
process_instruction::{stable_log, InvokeContext, ProcessInstructionWithContext},
pubkey::Pubkey,
@ -63,11 +64,6 @@ fn genesis_builtins() -> Vec<Builtin> {
solana_config_program::id(),
with_program_logging!(solana_config_program::config_processor::process_instruction),
),
Builtin::new(
"secp256k1_program",
solana_sdk::secp256k1_program::id(),
solana_secp256k1_program::process_instruction,
),
]
}
@ -86,7 +82,15 @@ pub enum ActivationType {
/// normal child Bank creation.
/// https://github.com/solana-labs/solana/blob/84b139cc94b5be7c9e0c18c2ad91743231b85a0d/runtime/src/bank.rs#L1723
fn feature_builtins() -> Vec<(Builtin, Pubkey, ActivationType)> {
vec![]
vec![(
Builtin::new(
"secp256k1_program",
solana_sdk::secp256k1_program::id(),
solana_secp256k1_program::process_instruction,
),
feature_set::secp256k1_program_enabled::id(),
ActivationType::NewProgram,
)]
}
pub(crate) fn get() -> Builtins {

View File

@ -273,7 +273,7 @@ mod tests {
..GenesisConfig::default()
};
let mut bank = Arc::new(Bank::new(&genesis_config));
let sysvar_and_native_program_delta = 11;
let sysvar_and_native_program_delta = 10;
assert_eq!(
bank.capitalization(),
(num_genesis_accounts + num_non_circulating_accounts + num_stake_accounts) * balance

View File

@ -20,6 +20,18 @@ impl Default for FeeCalculator {
}
}
pub struct FeeConfig {
pub secp256k1_program_enabled: bool,
}
impl Default for FeeConfig {
fn default() -> Self {
Self {
secp256k1_program_enabled: true,
}
}
}
impl FeeCalculator {
pub fn new(lamports_per_signature: u64) -> Self {
Self {
@ -28,14 +40,20 @@ impl FeeCalculator {
}
pub fn calculate_fee(&self, message: &Message) -> u64 {
self.calculate_fee_with_config(message, &FeeConfig::default())
}
pub fn calculate_fee_with_config(&self, message: &Message, fee_config: &FeeConfig) -> u64 {
let mut num_secp256k1_signatures: u64 = 0;
for instruction in &message.instructions {
let program_index = instruction.program_id_index as usize;
// Transaction may not be sanitized here
if program_index < message.account_keys.len() {
let id = message.account_keys[program_index];
if secp256k1_program::check_id(&id) && !instruction.data.is_empty() {
num_secp256k1_signatures += instruction.data[0] as u64;
if fee_config.secp256k1_program_enabled {
for instruction in &message.instructions {
let program_index = instruction.program_id_index as usize;
// Transaction may not be sanitized here
if program_index < message.account_keys.len() {
let id = message.account_keys[program_index];
if secp256k1_program::check_id(&id) && !instruction.data.is_empty() {
num_secp256k1_signatures += instruction.data[0] as u64;
}
}
}
}
@ -241,6 +259,15 @@ mod tests {
Some(&pubkey0),
);
assert_eq!(FeeCalculator::new(1).calculate_fee(&message), 2);
assert_eq!(
FeeCalculator::new(1).calculate_fee_with_config(
&message,
&FeeConfig {
secp256k1_program_enabled: false
}
),
1
);
secp_instruction.data = vec![0];
secp_instruction2.data = vec![10];

View File

@ -10,6 +10,10 @@ pub mod instructions_sysvar_enabled {
solana_sdk::declare_id!("EnvhHCLvg55P7PDtbvR1NwuTuAeodqpusV3MR5QEK8gs");
}
pub mod secp256k1_program_enabled {
solana_sdk::declare_id!("E3PHP7w8kB7np3CTQ1qQ2tW3KCtjRSXBQgW9vM2mWv2Y");
}
pub mod consistent_recent_blockhashes_sysvar {
solana_sdk::declare_id!("3h1BQWPDS5veRsq6mDBWruEpgPxRJkfwGexg5iiQ9mYg");
}
@ -171,6 +175,7 @@ lazy_static! {
/// Map of feature identifiers to user-visible description
pub static ref FEATURE_NAMES: HashMap<Pubkey, &'static str> = [
(instructions_sysvar_enabled::id(), "instructions sysvar"),
(secp256k1_program_enabled::id(), "secp256k1 program"),
(consistent_recent_blockhashes_sysvar::id(), "consistent recentblockhashes sysvar"),
(deprecate_rewards_sysvar::id(), "deprecate unused rewards sysvar"),
(pico_inflation::id(), "pico inflation"),