From c10da16d7bf0677d91e2e218a73a7bf33afbfe9b Mon Sep 17 00:00:00 2001 From: Michael Vines Date: Mon, 21 Sep 2020 22:36:23 -0700 Subject: [PATCH] Port instructions sysvar and secp256k1 program activation to FeatureSet --- cli/src/checks.rs | 2 +- core/src/banking_stage.rs | 43 +++++++--------- core/src/rpc_service.rs | 4 +- core/src/transaction_status_service.rs | 5 +- ledger/src/blockstore_processor.rs | 3 +- ledger/src/entry.rs | 25 +++------ poh-bench/src/main.rs | 4 +- runtime/src/accounts.rs | 35 ++++++------- runtime/src/bank.rs | 66 ++++++++++++++---------- runtime/src/builtins.rs | 55 ++++++++++---------- runtime/src/feature.rs | 7 ++- runtime/src/feature_set.rs | 57 +++++++++++++-------- runtime/src/genesis_utils.rs | 12 ++--- runtime/src/message_processor.rs | 31 ++++-------- sdk/src/fee_calculator.rs | 70 ++++++++++++-------------- sdk/src/secp256k1.rs | 22 -------- sdk/src/sysvar/instructions.rs | 10 ---- stake-o-matic/src/main.rs | 2 +- 18 files changed, 212 insertions(+), 241 deletions(-) diff --git a/cli/src/checks.rs b/cli/src/checks.rs index e84add1f28..f1c6514878 100644 --- a/cli/src/checks.rs +++ b/cli/src/checks.rs @@ -67,7 +67,7 @@ pub fn check_account_for_multiple_fees_with_commitment( pub fn calculate_fee(fee_calculator: &FeeCalculator, messages: &[&Message]) -> u64 { messages .iter() - .map(|message| fee_calculator.calculate_fee(message, None)) + .map(|message| fee_calculator.calculate_fee(message)) .sum() } diff --git a/core/src/banking_stage.rs b/core/src/banking_stage.rs index c0316daa13..f9538a3ae2 100644 --- a/core/src/banking_stage.rs +++ b/core/src/banking_stage.rs @@ -30,10 +30,9 @@ use solana_runtime::{ }; use solana_sdk::{ clock::{ - Epoch, 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, }, - genesis_config::ClusterType, poh_config::PohConfig, pubkey::Pubkey, timing::{duration_as_ms, timestamp}, @@ -737,8 +736,7 @@ impl BankingStage { fn transactions_from_packets( msgs: &Packets, transaction_indexes: &[usize], - cluster_type: ClusterType, - epoch: Epoch, + secp256k1_program_enabled: bool, ) -> (Vec, Vec) { let packets = Packets::new( transaction_indexes @@ -748,25 +746,24 @@ impl BankingStage { ); let transactions = Self::deserialize_transactions(&packets); - let maybe_secp_verified_transactions: Vec<_> = - if solana_sdk::secp256k1::is_enabled(cluster_type, epoch) { - transactions - .into_iter() - .map(|tx| { - if let Some(tx) = tx { - if tx.verify_precompiles().is_ok() { - Some(tx) - } else { - None - } + let maybe_secp_verified_transactions: Vec<_> = if secp256k1_program_enabled { + transactions + .into_iter() + .map(|tx| { + if let Some(tx) = tx { + if tx.verify_precompiles().is_ok() { + Some(tx) } else { None } - }) - .collect() - } else { - transactions - }; + } else { + None + } + }) + .collect() + } else { + transactions + }; Self::filter_transaction_indexes(maybe_secp_verified_transactions, &transaction_indexes) } @@ -820,8 +817,7 @@ impl BankingStage { let (transactions, transaction_to_packet_indexes) = Self::transactions_from_packets( msgs, &packet_indexes, - bank.cluster_type(), - bank.epoch(), + bank.secp256k1_program_enabled(), ); debug!( "bank: {} filtered transactions {}", @@ -874,8 +870,7 @@ impl BankingStage { let (transactions, transaction_to_packet_indexes) = Self::transactions_from_packets( msgs, &transaction_indexes, - bank.cluster_type(), - bank.epoch(), + bank.secp256k1_program_enabled(), ); let tx_count = transaction_to_packet_indexes.len(); diff --git a/core/src/rpc_service.rs b/core/src/rpc_service.rs index d4b24db7ca..ed69c99740 100644 --- a/core/src/rpc_service.rs +++ b/core/src/rpc_service.rs @@ -480,11 +480,11 @@ mod tests { assert_eq!(None, process_rest(&bank_forks, "not-a-supported-rest-api")); assert_eq!( - Some("0.000010127".to_string()), + Some("0.000010129".to_string()), process_rest(&bank_forks, "/v0/circulating-supply") ); assert_eq!( - Some("0.000010127".to_string()), + Some("0.000010129".to_string()), process_rest(&bank_forks, "/v0/total-supply") ); } diff --git a/core/src/transaction_status_service.rs b/core/src/transaction_status_service.rs index 108f482f71..b1ce0b38b8 100644 --- a/core/src/transaction_status_service.rs +++ b/core/src/transaction_status_service.rs @@ -80,10 +80,7 @@ impl TransactionStatusService { _ => bank.get_fee_calculator(&transaction.message().recent_blockhash), } .expect("FeeCalculator must exist"); - let fee = fee_calculator.calculate_fee( - transaction.message(), - solana_sdk::secp256k1::get_fee_config(bank.cluster_type(), bank.epoch()), - ); + let fee = fee_calculator.calculate_fee(transaction.message()); let (writable_keys, readonly_keys) = transaction.message.get_account_keys_by_lock_type(); diff --git a/ledger/src/blockstore_processor.rs b/ledger/src/blockstore_processor.rs index 3c20ede8ec..c46fd97386 100644 --- a/ledger/src/blockstore_processor.rs +++ b/ledger/src/blockstore_processor.rs @@ -681,8 +681,7 @@ pub fn confirm_slot( let entry_state = entries.start_verify( &progress.last_entry, recyclers.clone(), - bank.cluster_type(), - bank.epoch(), + bank.secp256k1_program_enabled(), ); if entry_state.status() == EntryVerificationStatus::Failure { warn!("Ledger proof of history failed at slot: {}", slot); diff --git a/ledger/src/entry.rs b/ledger/src/entry.rs index 7da6307a9e..3928749d20 100644 --- a/ledger/src/entry.rs +++ b/ledger/src/entry.rs @@ -17,8 +17,6 @@ use solana_perf::cuda_runtime::PinnedVec; use solana_perf::perf_libs; use solana_perf::recycler::Recycler; use solana_rayon_threadlimit::get_thread_count; -use solana_sdk::clock::Epoch; -use solana_sdk::genesis_config::ClusterType; use solana_sdk::hash::Hash; use solana_sdk::timing; use solana_sdk::transaction::Transaction; @@ -329,8 +327,7 @@ pub trait EntrySlice { &self, start_hash: &Hash, recyclers: VerifyRecyclers, - cluster_type: ClusterType, - epoch: Epoch, + secp256k1_program_enabled: bool, ) -> EntryVerificationState; fn verify(&self, start_hash: &Hash) -> bool; /// Checks that each entry tick has the correct number of hashes. Entry slices do not @@ -339,18 +336,13 @@ pub trait EntrySlice { fn verify_tick_hash_count(&self, tick_hash_count: &mut u64, hashes_per_tick: u64) -> bool; /// Counts tick entries fn tick_count(&self) -> u64; - fn verify_transaction_signatures(&self, cluster_type: ClusterType, epoch: Epoch) -> bool; + fn verify_transaction_signatures(&self, secp256k1_program_enabled: bool) -> bool; } impl EntrySlice for [Entry] { fn verify(&self, start_hash: &Hash) -> bool { - self.start_verify( - start_hash, - VerifyRecyclers::default(), - ClusterType::Development, - 0, - ) - .finish_verify(self) + self.start_verify(start_hash, VerifyRecyclers::default(), true) + .finish_verify(self) } fn verify_cpu_generic(&self, start_hash: &Hash) -> EntryVerificationState { @@ -497,14 +489,14 @@ impl EntrySlice for [Entry] { } } - fn verify_transaction_signatures(&self, cluster_type: ClusterType, epoch: Epoch) -> bool { + fn verify_transaction_signatures(&self, secp256k1_program_enabled: bool) -> bool { PAR_THREAD_POOL.with(|thread_pool| { thread_pool.borrow().install(|| { self.par_iter().all(|e| { e.transactions.par_iter().all(|transaction| { let sig_verify = transaction.verify().is_ok(); if sig_verify - && solana_sdk::secp256k1::is_enabled(cluster_type, epoch) + && secp256k1_program_enabled && transaction.verify_precompiles().is_err() { return false; @@ -520,11 +512,10 @@ impl EntrySlice for [Entry] { &self, start_hash: &Hash, recyclers: VerifyRecyclers, - cluster_type: ClusterType, - epoch: Epoch, + secp256k1_program_enabled: bool, ) -> EntryVerificationState { let start = Instant::now(); - let res = self.verify_transaction_signatures(cluster_type, epoch); + let res = self.verify_transaction_signatures(secp256k1_program_enabled); let transaction_duration_us = timing::duration_as_us(&start.elapsed()); if !res { return EntryVerificationState { diff --git a/poh-bench/src/main.rs b/poh-bench/src/main.rs index f462a5f22d..9e4662ab61 100644 --- a/poh-bench/src/main.rs +++ b/poh-bench/src/main.rs @@ -2,7 +2,7 @@ use clap::{crate_description, crate_name, value_t, App, Arg}; use solana_ledger::entry::{self, create_ticks, init_poh, EntrySlice, VerifyRecyclers}; use solana_measure::measure::Measure; use solana_perf::perf_libs; -use solana_sdk::{genesis_config::ClusterType, hash::hash}; +use solana_sdk::hash::hash; fn main() { solana_logger::setup(); @@ -118,7 +118,7 @@ fn main() { let recyclers = VerifyRecyclers::default(); for _ in 0..iterations { assert!(ticks[..num_entries] - .start_verify(&start_hash, recyclers.clone(), ClusterType::Development, 0) + .start_verify(&start_hash, recyclers.clone(), true) .finish_verify(&ticks[..num_entries])); } time.stop(); diff --git a/runtime/src/accounts.rs b/runtime/src/accounts.rs index 2e74af0aa4..d354930215 100644 --- a/runtime/src/accounts.rs +++ b/runtime/src/accounts.rs @@ -6,6 +6,7 @@ use crate::{ append_vec::StoredAccount, bank::{HashAgeKind, TransactionProcessResult}, blockhash_queue::BlockhashQueue, + feature_set::{self, FeatureSet}, nonce_utils, rent_collector::RentCollector, system_instruction_processor::{get_system_account_kind, SystemAccountKind}, @@ -17,7 +18,7 @@ use rayon::slice::ParallelSliceMut; use solana_sdk::{ account::Account, clock::{Epoch, Slot}, - fee_calculator::FeeCalculator, + fee_calculator::{FeeCalculator, FeeConfig}, genesis_config::ClusterType, hash::Hash, message::Message, @@ -72,11 +73,10 @@ pub enum AccountAddressFilter { impl Accounts { pub fn new(paths: Vec, cluster_type: &ClusterType) -> Self { Self { - slot: 0, - epoch: 0, accounts_db: Arc::new(AccountsDB::new(paths, cluster_type)), account_locks: Mutex::new(HashSet::new()), readonly_locks: Arc::new(RwLock::new(Some(HashMap::new()))), + ..Self::default() } } @@ -94,11 +94,10 @@ impl Accounts { pub(crate) fn new_empty(accounts_db: AccountsDB) -> Self { Self { - slot: 0, - epoch: 0, accounts_db: Arc::new(accounts_db), account_locks: Mutex::new(HashSet::new()), readonly_locks: Arc::new(RwLock::new(Some(HashMap::new()))), + ..Self::default() } } @@ -133,6 +132,7 @@ impl Accounts { fee: u64, error_counters: &mut ErrorCounters, rent_collector: &RentCollector, + feature_set: &FeatureSet, ) -> Result<(TransactionAccounts, TransactionRent)> { // Copy all the accounts let message = tx.message(); @@ -150,10 +150,8 @@ impl Accounts { payer_index = Some(i); } - if solana_sdk::sysvar::instructions::is_enabled( - self.epoch, - self.accounts_db.cluster_type.unwrap(), - ) && solana_sdk::sysvar::instructions::check_id(key) + if solana_sdk::sysvar::instructions::check_id(key) + && feature_set.active(&feature_set::instructions_sysvar_enabled::id()) { if message.is_writable(i) { return Err(TransactionError::InvalidAccountIndex); @@ -300,11 +298,17 @@ impl Accounts { hash_queue: &BlockhashQueue, error_counters: &mut ErrorCounters, rent_collector: &RentCollector, + feature_set: &FeatureSet, ) -> Vec<(Result, Option)> { //PERF: hold the lock to scan for the references, but not to clone the accounts //TODO: two locks usually leads to deadlocks, should this be one structure? let accounts_index = self.accounts_db.accounts_index.read().unwrap(); let storage = self.accounts_db.storage.read().unwrap(); + + let fee_config = FeeConfig { + secp256k1_program_enabled: feature_set + .active(&feature_set::secp256k1_program_enabled::id()), + }; OrderedIterator::new(txs, txs_iteration_order) .zip(lock_results.into_iter()) .map(|etx| match etx { @@ -318,13 +322,7 @@ impl Accounts { .cloned(), }; let fee = if let Some(fee_calculator) = fee_calculator { - fee_calculator.calculate_fee( - tx.message(), - solana_sdk::secp256k1::get_fee_config( - self.accounts_db.cluster_type.unwrap(), - self.epoch, - ), - ) + fee_calculator.calculate_fee_with_config(tx.message(), &fee_config) } else { return (Err(TransactionError::BlockhashNotFound), hash_age_kind); }; @@ -337,6 +335,7 @@ impl Accounts { fee, error_counters, rent_collector, + feature_set, ); let (accounts, rents) = match load_res { Ok((a, r)) => (a, r), @@ -888,6 +887,7 @@ mod tests { &hash_queue, error_counters, rent_collector, + &FeatureSet::default(), ) } @@ -1024,7 +1024,7 @@ mod tests { ); let fee_calculator = FeeCalculator::new(10); - assert_eq!(fee_calculator.calculate_fee(tx.message(), None), 10); + assert_eq!(fee_calculator.calculate_fee(tx.message()), 10); let loaded_accounts = load_accounts_with_fee(tx, &accounts, &fee_calculator, &mut error_counters); @@ -1832,6 +1832,7 @@ mod tests { &hash_queue, &mut error_counters, &rent_collector, + &FeatureSet::default(), ) } diff --git a/runtime/src/bank.rs b/runtime/src/bank.rs index 37b3fd21b9..d2d1cd8754 100644 --- a/runtime/src/bank.rs +++ b/runtime/src/bank.rs @@ -10,11 +10,11 @@ use crate::{ accounts_db::{ErrorCounters, SnapshotStorages}, accounts_index::Ancestors, blockhash_queue::BlockhashQueue, - builtins::get_builtins, + builtins::*, epoch_stakes::{EpochStakes, NodeVoteAccounts}, instruction_recorder::InstructionRecorder, feature::Feature, - feature_set::{FeatureSet}, + feature_set::{self, FeatureSet}, log_collector::LogCollector, message_processor::{Executors, MessageProcessor}, nonce_utils, @@ -43,7 +43,7 @@ use solana_sdk::{ }, epoch_info::EpochInfo, epoch_schedule::EpochSchedule, - fee_calculator::{FeeCalculator, FeeRateGovernor}, + fee_calculator::{FeeCalculator, FeeConfig, FeeRateGovernor}, genesis_config::{ClusterType, GenesisConfig}, hard_forks::HardForks, hash::{extend_and_hash, hashv, Hash}, @@ -1594,6 +1594,7 @@ impl Bank { &self.blockhash_queue.read().unwrap(), error_counters, &self.rent_collector, + &self.feature_set, ) } fn check_age( @@ -2032,8 +2033,7 @@ impl Bank { log_collector.clone(), executors.clone(), instruction_recorders.as_deref(), - self.cluster_type(), - self.epoch(), + &self.feature_set, ); Self::compile_recorded_instructions( @@ -2114,6 +2114,11 @@ impl Bank { ) -> Vec> { 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 = OrderedIterator::new(txs, iteration_order) .zip(executed.iter()) .map(|((_, tx), (res, hash_age_kind))| { @@ -2130,10 +2135,7 @@ impl Bank { }; let fee_calculator = fee_calculator.ok_or(TransactionError::BlockhashNotFound)?; - let fee = fee_calculator.calculate_fee( - tx.message(), - solana_sdk::secp256k1::get_fee_config(self.cluster_type(), self.epoch()), - ); + let fee = fee_calculator.calculate_fee_with_config(tx.message(), &fee_config); let message = tx.message(); match *res { @@ -3461,15 +3463,16 @@ impl Bank { consumed_budget.saturating_sub(budget_recovery_delta) } + pub fn secp256k1_program_enabled(&self) -> bool { + self.feature_set + .active(&feature_set::secp256k1_program_enabled::id()) + } + // This is called from snapshot restore AND for each epoch boundary // The entire code path herein must be idempotent fn apply_feature_activations(&mut self, init_finish_or_warp: bool, initiate_callback: bool) { - let new_feature_activations = self.compute_active_feature_set(); - for feature_id in new_feature_activations { - info!("New feature activated: {}", feature_id); - } - - self.ensure_builtins(init_finish_or_warp); + let new_feature_activations = self.compute_active_feature_set(!init_finish_or_warp); + self.ensure_builtins(init_finish_or_warp, &new_feature_activations); self.reinvoke_entered_epoch_callback(initiate_callback); self.recheck_cross_program_support(); self.recheck_compute_budget(); @@ -3478,7 +3481,7 @@ impl Bank { } // Compute the active feature set based on the current bank state, and return the set of newly activated features - fn compute_active_feature_set(&mut self) -> HashSet { + fn compute_active_feature_set(&mut self, allow_new_activations: bool) -> HashSet { let mut active = self.feature_set.active.clone(); let mut inactive = HashSet::new(); let mut newly_activated = HashSet::new(); @@ -3489,14 +3492,17 @@ impl Bank { if let Some(mut feature) = Feature::from_account(&account) { match feature.activated_at { None => { - // Feature has been requested, activate it now - feature.activated_at = Some(slot); - if feature.to_account(&mut account).is_some() { - self.store_account(feature_id, &account); + if allow_new_activations { + // Feature has been requested, activate it now + feature.activated_at = Some(slot); + if feature.to_account(&mut account).is_some() { + self.store_account(feature_id, &account); + } + newly_activated.insert(*feature_id); + active.insert(*feature_id); + info!("Feature {} activated at slot {}", feature_id, slot); + continue; } - newly_activated.insert(*feature_id); - active.insert(*feature_id); - continue; } Some(activation_slot) => { if slot >= activation_slot { @@ -3519,14 +3525,22 @@ impl Bank { newly_activated } - fn ensure_builtins(&mut self, init_or_warp: bool) { - for (program, start_epoch) in get_builtins(self.cluster_type()) { + fn ensure_builtins(&mut self, init_or_warp: bool, new_feature_activations: &HashSet) { + for (program, start_epoch) in get_cluster_builtins(self.cluster_type()) { let should_populate = init_or_warp && self.epoch() >= start_epoch || !init_or_warp && self.epoch() == start_epoch; if should_populate { self.add_builtin(&program.name, program.id, program.entrypoint); } } + + for (program, feature) in get_feature_builtins() { + let should_populate = init_or_warp && self.feature_set.active(&feature) + || !init_or_warp && new_feature_activations.contains(&feature); + if should_populate { + self.add_builtin(&program.name, program.id, program.entrypoint); + } + } } fn reinvoke_entered_epoch_callback(&mut self, initiate: bool) { @@ -8518,7 +8532,7 @@ mod tests { .collect::>(); consumed_budgets.sort(); // consumed_budgets represents the count of alive accounts in the three slots 0,1,2 - assert_eq!(consumed_budgets, vec![0, 1, 10]); + assert_eq!(consumed_budgets, vec![0, 1, 9]); } #[test] diff --git a/runtime/src/builtins.rs b/runtime/src/builtins.rs index d53a69307a..b7ac385d46 100644 --- a/runtime/src/builtins.rs +++ b/runtime/src/builtins.rs @@ -1,18 +1,21 @@ use crate::{ bank::{Builtin, Entrypoint}, - system_instruction_processor, + feature_set, system_instruction_processor, }; use solana_sdk::{ clock::{Epoch, GENESIS_EPOCH}, genesis_config::ClusterType, + pubkey::Pubkey, system_program, }; use log::*; -/// The entire set of available builtin programs that should be active at the given cluster_type -pub fn get_builtins(cluster_type: ClusterType) -> Vec<(Builtin, Epoch)> { - trace!("get_builtins: {:?}", cluster_type); +/// Builtin programs that should be active for the given cluster_type +/// +/// Old style. Use `get_feature_builtins()` instead +pub fn get_cluster_builtins(cluster_type: ClusterType) -> Vec<(Builtin, Epoch)> { + trace!("get_cluster_builtins: {:?}", cluster_type); let mut builtins = vec![]; builtins.extend( @@ -46,8 +49,8 @@ pub fn get_builtins(cluster_type: ClusterType) -> Vec<(Builtin, Epoch)> { // repurpose Testnet for test_get_builtins because the Development is overloaded... #[cfg(test)] if cluster_type == ClusterType::Testnet { + use solana_sdk::account::KeyedAccount; use solana_sdk::instruction::InstructionError; - use solana_sdk::{account::KeyedAccount, pubkey::Pubkey}; use std::str::FromStr; fn mock_ix_processor( _pubkey: &Pubkey, @@ -57,35 +60,33 @@ pub fn get_builtins(cluster_type: ClusterType) -> Vec<(Builtin, Epoch)> { Err(InstructionError::Custom(42)) } let program_id = Pubkey::from_str("7saCc6X5a2syoYANA5oUUnPZLcLMfKoSjiDhFU5fbpoK").unwrap(); - builtins.extend(vec![( + builtins.push(( Builtin::new("mock", program_id, Entrypoint::Program(mock_ix_processor)), 2, - )]); + )); } - let secp256k1_builtin = Builtin::new( - "secp256k1_program", - solana_sdk::secp256k1_program::id(), - Entrypoint::Program(solana_secp256k1_program::process_instruction), - ); - let secp_epoch = solana_sdk::secp256k1::is_enabled_epoch(cluster_type); - builtins.push((secp256k1_builtin, secp_epoch)); - builtins } +/// Builtin programs that are activated dynamically by feature +pub fn get_feature_builtins() -> Vec<(Builtin, Pubkey)> { + vec![( + Builtin::new( + "secp256k1_program", + solana_sdk::secp256k1_program::id(), + Entrypoint::Program(solana_secp256k1_program::process_instruction), + ), + feature_set::secp256k1_program_enabled::id(), + )] +} + #[cfg(test)] mod tests { use super::*; use crate::bank::Bank; - use solana_sdk::{ - genesis_config::{create_genesis_config, ClusterType}, - pubkey::Pubkey, - }; - - use std::collections::HashSet; - use std::str::FromStr; - use std::sync::Arc; + use solana_sdk::genesis_config::create_genesis_config; + use std::{collections::HashSet, str::FromStr, sync::Arc}; fn do_test_uniqueness(builtins: Vec<(Builtin, Epoch)>) { let mut unique_ids = HashSet::new(); @@ -101,10 +102,10 @@ mod tests { #[test] fn test_uniqueness() { - do_test_uniqueness(get_builtins(ClusterType::Development)); - do_test_uniqueness(get_builtins(ClusterType::Devnet)); - do_test_uniqueness(get_builtins(ClusterType::Testnet)); - do_test_uniqueness(get_builtins(ClusterType::MainnetBeta)); + do_test_uniqueness(get_cluster_builtins(ClusterType::Development)); + do_test_uniqueness(get_cluster_builtins(ClusterType::Devnet)); + do_test_uniqueness(get_cluster_builtins(ClusterType::Testnet)); + do_test_uniqueness(get_cluster_builtins(ClusterType::MainnetBeta)); } #[test] diff --git a/runtime/src/feature.rs b/runtime/src/feature.rs index c283459524..73b3091699 100644 --- a/runtime/src/feature.rs +++ b/runtime/src/feature.rs @@ -17,14 +17,17 @@ solana_sdk::declare_id!("Feature111111111111111111111111111111111111"); /// 2. When the next epoch is entered the runtime will check for new activation requests and /// active them. When this occurs, the activation slot is recorded in the feature account /// -#[derive(Default, Serialize, Deserialize)] +#[derive(Default, Debug, Serialize, Deserialize)] pub struct Feature { pub activated_at: Option, } impl Feature { pub fn size_of() -> usize { - bincode::serialized_size(&Self::default()).unwrap() as usize + bincode::serialized_size(&Self { + activated_at: Some(Slot::MAX), + }) + .unwrap() as usize } pub fn from_account(account: &Account) -> Option { if account.owner != id() { diff --git a/runtime/src/feature_set.rs b/runtime/src/feature_set.rs index 6a101d9a74..790d352670 100644 --- a/runtime/src/feature_set.rs +++ b/runtime/src/feature_set.rs @@ -1,8 +1,38 @@ +use lazy_static::lazy_static; use solana_sdk::{ hash::{Hash, Hasher}, pubkey::Pubkey, }; -use std::collections::HashSet; +use std::collections::{HashMap, HashSet}; + +pub mod instructions_sysvar_enabled { + solana_sdk::declare_id!("EnvhHCLvg55P7PDtbvR1NwuTuAeodqpusV3MR5QEK8gs"); +} + +pub mod secp256k1_program_enabled { + solana_sdk::declare_id!("E3PHP7w8kB7np3CTQ1qQ2tW3KCtjRSXBQgW9vM2mWv2Y"); +} + +lazy_static! { + pub static ref FEATURE_NAMES: HashMap = [ + (instructions_sysvar_enabled::id(), "instructions sysvar"), + (secp256k1_program_enabled::id(), "secp256k1 program") + /*************** ADD NEW FEATURES HERE ***************/ + ] + .iter() + .cloned() + .collect(); + + static ref ID: Hash = { + let mut hasher = Hasher::default(); + let mut feature_ids = FEATURE_NAMES.keys().collect::>(); + feature_ids.sort(); + for feature in feature_ids { + hasher.hash(feature.as_ref()); + } + hasher.result() + }; +} /// The `FeatureSet` struct tracks the set of available and currently active runtime features #[derive(AbiExample)] @@ -26,34 +56,19 @@ impl FeatureSet { impl Default for FeatureSet { // By default all features are disabled fn default() -> Self { - let features: [Pubkey; 0] = []; - Self { - id: { - let mut hasher = Hasher::default(); - for feature in features.iter() { - hasher.hash(feature.as_ref()); - } - hasher.result() - }, + id: *ID, active: HashSet::new(), - inactive: features.iter().cloned().collect(), + inactive: FEATURE_NAMES.keys().cloned().collect(), } } } impl FeatureSet { - // New `FeatureSet` with all features enabled - pub fn new_enabled() -> Self { - let default = Self::default(); - + pub fn enabled() -> Self { Self { - id: default.id, - active: default - .active - .intersection(&default.inactive) - .cloned() - .collect::>(), + id: *ID, + active: FEATURE_NAMES.keys().cloned().collect(), inactive: HashSet::new(), } } diff --git a/runtime/src/genesis_utils.rs b/runtime/src/genesis_utils.rs index b2638c2bba..4b52a08b8e 100644 --- a/runtime/src/genesis_utils.rs +++ b/runtime/src/genesis_utils.rs @@ -109,18 +109,18 @@ pub fn create_genesis_config_with_leader( } pub fn add_feature_accounts(genesis_config: &mut GenesisConfig) { - // Activate all features at genesis in development mode if genesis_config.cluster_type == ClusterType::Development { - let feature_set = FeatureSet::new_enabled(); - - for feature_id in feature_set.active { + // Activate all features at genesis in development mode + for feature_id in FeatureSet::default().inactive { let feature = Feature { activated_at: Some(0), }; - genesis_config.accounts.insert( feature_id, - feature.create_account(genesis_config.rent.minimum_balance(Feature::size_of())), + feature.create_account(std::cmp::max( + genesis_config.rent.minimum_balance(Feature::size_of()), + 1, + )), ); } } diff --git a/runtime/src/message_processor.rs b/runtime/src/message_processor.rs index 8dd67e1e3a..1559eba013 100644 --- a/runtime/src/message_processor.rs +++ b/runtime/src/message_processor.rs @@ -1,6 +1,7 @@ use crate::{ instruction_recorder::InstructionRecorder, log_collector::LogCollector, native_loader::NativeLoader, rent_collector::RentCollector, + feature_set::{self, FeatureSet}, }; use log::*; use serde::{Deserialize, Serialize}; @@ -11,7 +12,6 @@ use solana_sdk::{ ComputeBudget, ComputeMeter, ErasedProcessInstruction, ErasedProcessInstructionWithContext, Executor, InvokeContext, Logger, ProcessInstruction, ProcessInstructionWithContext, }, - genesis_config::ClusterType, instruction::{CompiledInstruction, Instruction, InstructionError}, message::Message, native_loader, @@ -679,12 +679,11 @@ impl MessageProcessor { executors: Rc>, instruction_recorder: Option, instruction_index: usize, - cluster_type: ClusterType, - epoch: Epoch, + feature_set: &FeatureSet, ) -> Result<(), InstructionError> { // Fixup the special instructions key if present // before the account pre-values are taken care of - if solana_sdk::sysvar::instructions::is_enabled(epoch, cluster_type) { + if feature_set.active(&feature_set::instructions_sysvar_enabled::id()) { for (i, key) in message.account_keys.iter().enumerate() { if solana_sdk::sysvar::instructions::check_id(key) { let mut mut_account_ref = accounts[i].borrow_mut(); @@ -736,8 +735,7 @@ impl MessageProcessor { log_collector: Option>, executors: Rc>, instruction_recorders: Option<&[InstructionRecorder]>, - cluster_type: ClusterType, - epoch: Epoch, + feature_set: &FeatureSet, ) -> Result<(), TransactionError> { for (instruction_index, instruction) in message.instructions.iter().enumerate() { let instruction_recorder = instruction_recorders @@ -753,8 +751,7 @@ impl MessageProcessor { executors.clone(), instruction_recorder, instruction_index, - cluster_type, - epoch, + feature_set, ) .map_err(|err| TransactionError::InstructionError(instruction_index as u8, err))?; } @@ -1349,8 +1346,7 @@ mod tests { None, executors.clone(), None, - ClusterType::Development, - 0, + &FeatureSet::default(), ); assert_eq!(result, Ok(())); assert_eq!(accounts[0].borrow().lamports, 100); @@ -1373,8 +1369,7 @@ mod tests { None, executors.clone(), None, - ClusterType::Development, - 0, + &FeatureSet::default(), ); assert_eq!( result, @@ -1401,8 +1396,7 @@ mod tests { None, executors, None, - ClusterType::Development, - 0, + &FeatureSet::default(), ); assert_eq!( result, @@ -1512,8 +1506,7 @@ mod tests { None, executors.clone(), None, - ClusterType::Development, - 0, + &FeatureSet::default(), ); assert_eq!( result, @@ -1540,8 +1533,7 @@ mod tests { None, executors.clone(), None, - ClusterType::Development, - 0, + &FeatureSet::default(), ); assert_eq!(result, Ok(())); @@ -1565,8 +1557,7 @@ mod tests { None, executors, None, - ClusterType::Development, - 0, + &FeatureSet::default(), ); assert_eq!(result, Ok(())); assert_eq!(accounts[0].borrow().lamports, 80); diff --git a/sdk/src/fee_calculator.rs b/sdk/src/fee_calculator.rs index 8210d8439b..5abc497099 100644 --- a/sdk/src/fee_calculator.rs +++ b/sdk/src/fee_calculator.rs @@ -19,9 +19,16 @@ impl Default for FeeCalculator { } } -#[derive(Clone)] pub struct FeeConfig { - pub is_secp256k1_enabled: bool, + pub secp256k1_program_enabled: bool, +} + +impl Default for FeeConfig { + fn default() -> Self { + Self { + secp256k1_program_enabled: true, + } + } } impl FeeCalculator { @@ -31,27 +38,27 @@ impl FeeCalculator { } } - // extra_config: None == everything enabled - pub fn calculate_fee(&self, message: &Message, extra_config: Option) -> u64 { - let is_secp256k1_enabled = match extra_config { - Some(config) => config.is_secp256k1_enabled, - None => true, - }; - let mut num_secp_signatures: u64 = 0; - if is_secp256k1_enabled { + 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; + 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_secp_signatures += instruction.data[0] as u64; + num_secp256k1_signatures += instruction.data[0] as u64; } } } } + self.lamports_per_signature - * (u64::from(message.header.num_required_signatures) + num_secp_signatures) + * (u64::from(message.header.num_required_signatures) + num_secp256k1_signatures) } } @@ -182,9 +189,7 @@ impl FeeRateGovernor { /// create a FeeCalculator based on current cluster signature throughput pub fn create_fee_calculator(&self) -> FeeCalculator { - FeeCalculator { - lamports_per_signature: self.lamports_per_signature, - } + FeeCalculator::new(self.lamports_per_signature) } } @@ -207,34 +212,25 @@ mod tests { #[test] fn test_fee_calculator_calculate_fee() { - let fee_config = Some(FeeConfig { - is_secp256k1_enabled: true, - }); // Default: no fee. let message = Message::default(); - assert_eq!( - FeeCalculator::default().calculate_fee(&message, fee_config.clone()), - 0 - ); + assert_eq!(FeeCalculator::default().calculate_fee(&message), 0); // No signature, no fee. - assert_eq!(FeeCalculator::new(1).calculate_fee(&message, fee_config), 0); + assert_eq!(FeeCalculator::new(1).calculate_fee(&message), 0); - let fee_config = Some(FeeConfig { - is_secp256k1_enabled: false, - }); // One signature, a fee. let pubkey0 = Pubkey::new(&[0; 32]); let pubkey1 = Pubkey::new(&[1; 32]); let ix0 = system_instruction::transfer(&pubkey0, &pubkey1, 1); let message = Message::new(&[ix0], Some(&pubkey0)); - assert_eq!(FeeCalculator::new(2).calculate_fee(&message, fee_config), 2); + assert_eq!(FeeCalculator::new(2).calculate_fee(&message), 2); // Two signatures, double the fee. let ix0 = system_instruction::transfer(&pubkey0, &pubkey1, 1); let ix1 = system_instruction::transfer(&pubkey1, &pubkey0, 1); let message = Message::new(&[ix0, ix1], Some(&pubkey0)); - assert_eq!(FeeCalculator::new(2).calculate_fee(&message, None), 4); + assert_eq!(FeeCalculator::new(2).calculate_fee(&message), 4); } #[test] @@ -262,21 +258,21 @@ mod tests { ], Some(&pubkey0), ); - let fee_config = Some(FeeConfig { - is_secp256k1_enabled: true, - }); + assert_eq!(FeeCalculator::new(1).calculate_fee(&message), 2); assert_eq!( - FeeCalculator::new(1).calculate_fee(&message, fee_config.clone()), - 2 + FeeCalculator::new(1).calculate_fee_with_config( + &message, + &FeeConfig { + secp256k1_program_enabled: false + } + ), + 1 ); secp_instruction.data = vec![0]; secp_instruction2.data = vec![10]; let message = Message::new(&[ix0, secp_instruction, secp_instruction2], Some(&pubkey0)); - assert_eq!( - FeeCalculator::new(1).calculate_fee(&message, fee_config), - 11 - ); + assert_eq!(FeeCalculator::new(1).calculate_fee(&message), 11); } #[test] diff --git a/sdk/src/secp256k1.rs b/sdk/src/secp256k1.rs index 6eddcc7a8e..4a62f5897c 100644 --- a/sdk/src/secp256k1.rs +++ b/sdk/src/secp256k1.rs @@ -1,28 +1,6 @@ -use crate::clock::{Epoch, GENESIS_EPOCH}; -use crate::fee_calculator::FeeConfig; -use crate::genesis_config::ClusterType; use digest::Digest; use serde_derive::{Deserialize, Serialize}; -pub fn get_fee_config(cluster_type: ClusterType, epoch: Epoch) -> Option { - Some(FeeConfig { - is_secp256k1_enabled: is_enabled(cluster_type, epoch), - }) -} - -pub fn is_enabled_epoch(cluster_type: ClusterType) -> Epoch { - match cluster_type { - ClusterType::Development => GENESIS_EPOCH, - ClusterType::Testnet => u64::MAX, - ClusterType::MainnetBeta => u64::MAX, - ClusterType::Devnet => u64::MAX, - } -} - -pub fn is_enabled(cluster_type: ClusterType, epoch: Epoch) -> bool { - epoch >= is_enabled_epoch(cluster_type) -} - #[derive(Debug)] pub enum Secp256k1Error { InvalidSignature, diff --git a/sdk/src/sysvar/instructions.rs b/sdk/src/sysvar/instructions.rs index e68499f845..e4efc75cf3 100644 --- a/sdk/src/sysvar/instructions.rs +++ b/sdk/src/sysvar/instructions.rs @@ -11,16 +11,6 @@ crate::declare_sysvar_id!("Sysvar1nstructions1111111111111111111111111", Instruc impl Sysvar for Instructions {} -#[cfg(not(feature = "program"))] -use crate::clock::Epoch; -#[cfg(not(feature = "program"))] -use crate::genesis_config::ClusterType; - -#[cfg(not(feature = "program"))] -pub fn is_enabled(_epoch: Epoch, cluster_type: ClusterType) -> bool { - cluster_type == ClusterType::Development -} - pub fn load_current_index(data: &[u8]) -> u16 { let mut instr_fixed_data = [0u8; 2]; let len = data.len(); diff --git a/stake-o-matic/src/main.rs b/stake-o-matic/src/main.rs index a4975c01e9..19316da707 100644 --- a/stake-o-matic/src/main.rs +++ b/stake-o-matic/src/main.rs @@ -430,7 +430,7 @@ fn transact( info!("{} transactions to send", transactions.len()); let required_fee = transactions.iter().fold(0, |fee, (transaction, _)| { - fee + fee_calculator.calculate_fee(&transaction.message, None) + fee + fee_calculator.calculate_fee(&transaction.message) }); info!("Required fee: {} SOL", lamports_to_sol(required_fee)); if required_fee > authorized_staker_balance {