Skip to content

Commit

Permalink
Add feature gate;
Browse files Browse the repository at this point in the history
plumb feature-gate status to process_compute_budget_instructions()
  • Loading branch information
tao-stones committed Jun 28, 2024
1 parent fc56630 commit a5c9c97
Show file tree
Hide file tree
Showing 9 changed files with 62 additions and 18 deletions.
8 changes: 6 additions & 2 deletions core/src/banking_stage/consumer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -741,8 +741,12 @@ impl Consumer {
error_counters: &mut TransactionErrorMetrics,
) -> Result<(), TransactionError> {
let fee_payer = message.fee_payer();
let budget_limits =
process_compute_budget_instructions(message.program_instructions_iter())?.into();
let budget_limits = process_compute_budget_instructions(
message.program_instructions_iter(),
bank.feature_set
.is_active(&feature_set::default_loaded_accounts_data_size_limit::id()),
)?
.into();
let fee = bank.fee_structure().calculate_fee(
message,
bank.get_lamports_per_signature(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ use {
solana_sdk::{
self,
clock::{FORWARD_TRANSACTIONS_TO_LEADER_AT_SLOT_OFFSET, MAX_PROCESSING_AGE},
feature_set,
fee::FeeBudgetLimits,
saturating_add_assign,
transaction::SanitizedTransaction,
Expand Down Expand Up @@ -508,9 +509,13 @@ impl SchedulerController {
.is_ok()
})
.filter_map(|(packet, tx)| {
process_compute_budget_instructions(tx.message().program_instructions_iter())
.map(|compute_budget| (packet, tx, compute_budget.into()))
.ok()
process_compute_budget_instructions(
tx.message().program_instructions_iter(),
bank.feature_set
.is_active(&feature_set::default_loaded_accounts_data_size_limit::id()),
)
.map(|compute_budget| (packet, tx, compute_budget.into()))
.ok()
})
.for_each(|(packet, tx, fee_budget_limits)| {
arc_packets.push(packet);
Expand Down
6 changes: 4 additions & 2 deletions cost-model/src/cost_model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,8 +173,10 @@ impl CostModel {

// if failed to process compute_budget instructions, the transaction will not be executed
// by `bank`, therefore it should be considered as no execution cost by cost model.
match process_compute_budget_instructions(transaction.message().program_instructions_iter())
{
match process_compute_budget_instructions(
transaction.message().program_instructions_iter(),
feature_set.is_active(&feature_set::default_loaded_accounts_data_size_limit::id()),
) {
Ok(compute_budget_limits) => {
// if tx contained user-space instructions and a more accurate estimate available correct it,
// where "user-space instructions" must be specifically checked by
Expand Down
7 changes: 6 additions & 1 deletion runtime-transaction/src/runtime_transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ use {
process_compute_budget_instructions, ComputeBudgetLimits,
},
solana_sdk::{
feature_set::{self, default_loaded_accounts_data_size_limit},
hash::Hash,
message::{AddressLoader, SanitizedMessage, SanitizedVersionedMessage},
pubkey::Pubkey,
Expand Down Expand Up @@ -71,6 +72,7 @@ impl RuntimeTransaction<SanitizedVersionedMessage> {
sanitized_versioned_tx: SanitizedVersionedTransaction,
message_hash: Option<Hash>,
is_simple_vote_tx: Option<bool>,
feature_set: &feature_set::FeatureSet,
) -> Result<Self> {
let mut meta = TransactionMeta::default();
meta.set_is_simple_vote_tx(
Expand All @@ -86,7 +88,10 @@ impl RuntimeTransaction<SanitizedVersionedMessage> {
compute_unit_price,
loaded_accounts_bytes,
..
} = process_compute_budget_instructions(message.program_instructions_iter())?;
} = process_compute_budget_instructions(
message.program_instructions_iter(),
feature_set.is_active(&default_loaded_accounts_data_size_limit::id()),
)?;
meta.set_compute_unit_limit(compute_unit_limit);
meta.set_compute_unit_price(compute_unit_price);
meta.set_loaded_accounts_bytes(loaded_accounts_bytes);
Expand Down
18 changes: 14 additions & 4 deletions runtime/src/bank.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ use {
solana_measure::{measure, measure::Measure, measure_us},
solana_perf::perf_libs,
solana_program_runtime::{
compute_budget_processor::{process_compute_budget_instructions, ComputeBudgetLimits},
invoke_context::BuiltinFunctionWithContext,
loaded_programs::ProgramCacheEntry,
timings::{ExecuteTimingType, ExecuteTimings},
Expand All @@ -118,7 +119,8 @@ use {
epoch_schedule::EpochSchedule,
feature,
feature_set::{
self, include_loaded_accounts_data_size_in_fee_calculation,
self, default_loaded_accounts_data_size_limit,
include_loaded_accounts_data_size_in_fee_calculation,
remove_rounding_in_fee_calculation, reward_full_priority_fee, FeatureSet,
},
fee::{FeeDetails, FeeStructure},
Expand Down Expand Up @@ -3124,12 +3126,20 @@ impl Bank {
message: &SanitizedMessage,
lamports_per_signature: u64,
) -> u64 {
let use_default_loaded_accounts_data_size = self
.feature_set
.is_active(&default_loaded_accounts_data_size_limit::id());
self.fee_structure().calculate_fee(
message,
lamports_per_signature,
&process_compute_budget_instructions(message.program_instructions_iter())
.unwrap_or_default()
.into(),
&process_compute_budget_instructions(
message.program_instructions_iter(),
use_default_loaded_accounts_data_size,
)
.unwrap_or_else(|_| {
ComputeBudgetLimits::new_with(use_default_loaded_accounts_data_size)
})
.into(),
self.feature_set
.is_active(&include_loaded_accounts_data_size_in_fee_calculation::id()),
self.feature_set
Expand Down
9 changes: 8 additions & 1 deletion runtime/src/compute_budget_details.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,14 @@ pub trait GetComputeBudgetDetails {
instructions: impl Iterator<Item = (&'a Pubkey, &'a CompiledInstruction)>,
_round_compute_unit_price_enabled: bool,
) -> Option<ComputeBudgetDetails> {
let compute_budget_limits = process_compute_budget_instructions(instructions).ok()?;
// ComputeBudgetDetails does not concern with loaded_accounts_data_size_limit, hence safe
// to hardcode its feature gate `default_loaded_accounts_data_size_limit` as activated
let use_default_loaded_accounts_data_size = true;
let compute_budget_limits = process_compute_budget_instructions(
instructions,
use_default_loaded_accounts_data_size,
)
.ok()?;
Some(ComputeBudgetDetails {
compute_unit_price: compute_budget_limits.compute_unit_price,
compute_unit_limit: u64::from(compute_budget_limits.compute_unit_limit),
Expand Down
5 changes: 5 additions & 0 deletions sdk/src/feature_set.rs
Original file line number Diff line number Diff line change
Expand Up @@ -832,6 +832,10 @@ pub mod verify_retransmitter_signature {
solana_sdk::declare_id!("BZ5g4hRbu5hLQQBdPyo2z9icGyJ8Khiyj3QS6dhWijTb");
}

pub mod default_loaded_accounts_data_size_limit {
solana_sdk::declare_id!("CVvWw7NMVCn2Yp5RfxqrcTHXnaYyh91A2bGbkk88XXMM");
}

lazy_static! {
/// Map of feature identifiers to user-visible description
pub static ref FEATURE_NAMES: HashMap<Pubkey, &'static str> = [
Expand Down Expand Up @@ -1035,6 +1039,7 @@ lazy_static! {
(migrate_address_lookup_table_program_to_core_bpf::id(), "Migrate Address Lookup Table program to Core BPF #1651"),
(zk_elgamal_proof_program_enabled::id(), "Enable ZkElGamalProof program SIMD-0153"),
(verify_retransmitter_signature::id(), "Verify retransmitter signature #1840"),
(default_loaded_accounts_data_size_limit::id(), "add default loaded_accounts_data_size_limit #1568"),
/*************** ADD NEW FEATURES HERE ***************/
]
.iter()
Expand Down
15 changes: 10 additions & 5 deletions svm/src/account_loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -202,8 +202,10 @@ fn load_transaction_accounts<CB: TransactionProcessingCallback>(
let mut accounts_found = Vec::with_capacity(account_keys.len());
let mut rent_debits = RentDebits::default();

let requested_loaded_accounts_data_size_limit =
get_requested_loaded_accounts_data_size_limit(message)?;
let requested_loaded_accounts_data_size_limit = get_requested_loaded_accounts_data_size_limit(
message,
feature_set.is_active(&default_loaded_accounts_data_size_limit::id()),
)?;
let mut accumulated_accounts_data_size: usize = 0;

let instruction_accounts = message
Expand Down Expand Up @@ -377,10 +379,13 @@ fn load_transaction_accounts<CB: TransactionProcessingCallback>(
/// Note, requesting zero bytes will result transaction error
fn get_requested_loaded_accounts_data_size_limit(
sanitized_message: &SanitizedMessage,
use_default_loaded_accounts_data_size: bool,
) -> Result<Option<NonZeroUsize>> {
let compute_budget_limits =
process_compute_budget_instructions(sanitized_message.program_instructions_iter())
.unwrap_or_default();
let compute_budget_limits = process_compute_budget_instructions(
sanitized_message.program_instructions_iter(),
use_default_loaded_accounts_data_size,
)
.unwrap_or_else(|_| ComputeBudgetLimits::new_with(use_default_loaded_accounts_data_size));
// sanitize against setting size limit to zero
NonZeroUsize::new(
usize::try_from(compute_budget_limits.loaded_accounts_bytes).unwrap_or_default(),
Expand Down
1 change: 1 addition & 0 deletions svm/src/transaction_processor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -422,6 +422,7 @@ impl<FG: ForkGraph> TransactionBatchProcessor<FG> {
) -> transaction::Result<ValidatedTransactionDetails> {
let compute_budget_limits = process_compute_budget_instructions(
message.program_instructions_iter(),
feature_set.is_active(&feature_set::default_loaded_accounts_data_size_limit::id()),
)
.map_err(|err| {
error_counters.invalid_compute_budget += 1;
Expand Down

0 comments on commit a5c9c97

Please sign in to comment.