Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature Impl: cost model uses number of requested write locks #34820

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 40 additions & 5 deletions cost-model/src/cost_model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ use {
solana_sdk::{
borsh1::try_from_slice_unchecked,
compute_budget::{self, ComputeBudgetInstruction},
feature_set::{include_loaded_accounts_data_size_in_fee_calculation, FeatureSet},
feature_set::{self, include_loaded_accounts_data_size_in_fee_calculation, FeatureSet},
fee::FeeStructure,
instruction::CompiledInstruction,
program_utils::limited_deserialize,
Expand All @@ -44,7 +44,7 @@ impl CostModel {
let mut tx_cost = UsageCostDetails::new_with_default_capacity();

tx_cost.signature_cost = Self::get_signature_cost(transaction);
Self::get_write_lock_cost(&mut tx_cost, transaction);
Self::get_write_lock_cost(&mut tx_cost, transaction, feature_set);
Self::get_transaction_cost(&mut tx_cost, transaction, feature_set);
tx_cost.account_data_size = Self::calculate_account_data_size(transaction);

Expand Down Expand Up @@ -73,10 +73,19 @@ impl CostModel {
.collect()
}

fn get_write_lock_cost(tx_cost: &mut UsageCostDetails, transaction: &SanitizedTransaction) {
fn get_write_lock_cost(
tx_cost: &mut UsageCostDetails,
transaction: &SanitizedTransaction,
feature_set: &FeatureSet,
) {
tx_cost.writable_accounts = Self::get_writable_accounts(transaction);
tx_cost.write_lock_cost =
WRITE_LOCK_UNITS.saturating_mul(tx_cost.writable_accounts.len() as u64);
let num_write_locks =
if feature_set.is_active(&feature_set::cost_model_requested_write_lock_cost::id()) {
transaction.message().num_write_locks()
} else {
tx_cost.writable_accounts.len() as u64
};
tx_cost.write_lock_cost = WRITE_LOCK_UNITS.saturating_mul(num_write_locks);
}

fn get_transaction_cost(
Expand Down Expand Up @@ -329,6 +338,32 @@ mod tests {
assert_eq!(0, tx_cost.data_bytes_cost);
}

#[test]
fn test_cost_model_demoted_write_lock() {
let (mint_keypair, start_hash) = test_setup();

// Cannot write-lock the system program, it will be demoted when taking locks.
// However, the cost should be calculated as if it were taken.
let simple_transaction = SanitizedTransaction::from_transaction_for_tests(
system_transaction::transfer(&mint_keypair, &system_program::id(), 2, start_hash),
);

// Feature not enabled - write lock is demoted and does not count towards cost
{
let tx_cost = CostModel::calculate_cost(&simple_transaction, &FeatureSet::default());
assert_eq!(WRITE_LOCK_UNITS, tx_cost.write_lock_cost());
assert_eq!(1, tx_cost.writable_accounts().len());
}

// Feature enabled - write lock is demoted but still counts towards cost
{
let tx_cost =
CostModel::calculate_cost(&simple_transaction, &FeatureSet::all_enabled());
assert_eq!(2 * WRITE_LOCK_UNITS, tx_cost.write_lock_cost());
assert_eq!(1, tx_cost.writable_accounts().len());
}
}

#[test]
fn test_cost_model_compute_budget_transaction() {
let (mint_keypair, start_hash) = test_setup();
Expand Down
2 changes: 2 additions & 0 deletions sdk/program/src/message/sanitized.rs
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,8 @@ impl SanitizedMessage {
num_signatures
}

/// Returns the number of requested write-locks in this message.
/// This does not consider if write-locks are demoted.
pub fn num_write_locks(&self) -> u64 {
self.account_keys()
.len()
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 @@ -768,6 +768,10 @@ pub mod enable_zk_proof_from_account {
solana_sdk::declare_id!("zkiTNuzBKxrCLMKehzuQeKZyLtX2yvFcEKMML8nExU8");
}

pub mod cost_model_requested_write_lock_cost {
solana_sdk::declare_id!("wLckV1a64ngtcKPRGU4S4grVTestXjmNjxBjaKZrAcn");
}

lazy_static! {
/// Map of feature identifiers to user-visible description
pub static ref FEATURE_NAMES: HashMap<Pubkey, &'static str> = [
Expand Down Expand Up @@ -955,6 +959,7 @@ lazy_static! {
(deprecate_executable_meta_update_in_bpf_loader::id(), "deprecate executable meta flag update in bpf loader #34194"),
(enable_zk_proof_from_account::id(), "Enable zk token proof program to read proof from accounts instead of instruction data #34750"),
(curve25519_restrict_msm_length::id(), "restrict curve25519 multiscalar multiplication vector lengths #34763"),
(cost_model_requested_write_lock_cost::id(), "cost model uses number of requested write locks #34819"),
/*************** ADD NEW FEATURES HERE ***************/
]
.iter()
Expand Down