Skip to content

refactor: reuse calculated fees #1462

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.

Sign up for GitHub

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

Merged
mergify merged 5 commits into from
May 29, 2024
Merged

refactor: reuse calculated fees #1462

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
a7b1ade
refactor: reuse calculated fees
jstarry May 22, 2024
abe1f46
fix tests
jstarry May 24, 2024
d05e31c
feedback
jstarry May 29, 2024
ac50b81
fix test
jstarry May 29, 2024
9999afe
fix unused warn
jstarry May 29, 2024
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
11 changes: 8 additions & 3 deletions accounts-db/src/accounts.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -838,6 +838,7 @@ mod tests {
solana_sdk::{
account::{AccountSharedData, WritableAccount},
address_lookup_table::state::LookupTableMeta,
fee::FeeDetails,
hash::Hash,
instruction::{CompiledInstruction, InstructionError},
message::{Message, MessageHeader},
Expand All @@ -848,8 +849,7 @@ mod tests {
transaction::{Transaction, MAX_TX_ACCOUNT_LOCKS},
},
solana_svm::{
account_loader::LoadedTransaction,
transaction_results::{DurableNonceFee, TransactionExecutionDetails},
account_loader::LoadedTransaction, transaction_results::TransactionExecutionDetails,
},
std::{
borrow::Cow,
Expand Down Expand Up @@ -880,7 +880,8 @@ mod tests {
status,
log_messages: None,
inner_instructions: None,
durable_nonce_fee: nonce.map(DurableNonceFee::from),
fee_details: FeeDetails::default(),
is_nonce: nonce.is_some(),
return_data: None,
executed_units: 0,
accounts_data_len_delta: 0,
Expand Down Expand Up @@ -1578,6 +1579,7 @@ mod tests {
accounts: transaction_accounts0,
program_indices: vec![],
nonce: None,
fee_details: FeeDetails::default(),
rent: 0,
rent_debits: RentDebits::default(),
});
Expand All @@ -1586,6 +1588,7 @@ mod tests {
accounts: transaction_accounts1,
program_indices: vec![],
nonce: None,
fee_details: FeeDetails::default(),
rent: 0,
rent_debits: RentDebits::default(),
});
Expand Down Expand Up @@ -1962,6 +1965,7 @@ mod tests {
accounts: transaction_accounts,
program_indices: vec![],
nonce: nonce.clone(),
fee_details: FeeDetails::default(),
rent: 0,
rent_debits: RentDebits::default(),
});
Expand Down Expand Up @@ -2066,6 +2070,7 @@ mod tests {
accounts: transaction_accounts,
program_indices: vec![],
nonce: nonce.clone(),
fee_details: FeeDetails::default(),
rent: 0,
rent_debits: RentDebits::default(),
});
Expand Down
9 changes: 5 additions & 4 deletions core/src/banking_stage/transaction_scheduler/scheduler_controller.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -653,9 +653,9 @@ mod tests {
solana_poh::poh_recorder::{PohRecorder, Record, WorkingBankEntry},
solana_runtime::bank::Bank,
solana_sdk::{
compute_budget::ComputeBudgetInstruction, hash::Hash, message::Message,
poh_config::PohConfig, pubkey::Pubkey, signature::Keypair, signer::Signer,
system_instruction, system_transaction, transaction::Transaction,
compute_budget::ComputeBudgetInstruction, fee_calculator::FeeRateGovernor, hash::Hash,
message::Message, poh_config::PohConfig, pubkey::Pubkey, signature::Keypair,
signer::Signer, system_instruction, system_transaction, transaction::Transaction,
},
std::sync::{atomic::AtomicBool, Arc, RwLock},
tempfile::TempDir,
Expand All @@ -682,10 +682,11 @@ mod tests {

fn create_test_frame(num_threads: usize) -> (TestFrame, SchedulerController) {
let GenesisConfigInfo {
genesis_config,
mut genesis_config,
mint_keypair,
..
} = create_slow_genesis_config(u64::MAX);
genesis_config.fee_rate_governor = FeeRateGovernor::new(5000, 0);
let (bank, bank_forks) = Bank::new_no_wallclock_throttle_for_tests(&genesis_config);

let ledger_path = get_tmp_ledger_path_auto_delete!();
Expand Down
25 changes: 3 additions & 22 deletions programs/sbf/tests/programs.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ use {
},
solana_svm::transaction_processor::ExecutionRecordingConfig,
solana_svm::transaction_results::{
DurableNonceFee, InnerInstruction, TransactionExecutionDetails, TransactionExecutionResult,
InnerInstruction, TransactionExecutionDetails, TransactionExecutionResult,
TransactionResults,
},
solana_transaction_status::{
Expand Down Expand Up @@ -185,38 +185,19 @@ fn execute_transactions(
status,
log_messages,
inner_instructions,
durable_nonce_fee,
fee_details,
return_data,
executed_units,
..
} = details;

let lamports_per_signature = match durable_nonce_fee {
Some(DurableNonceFee::Valid(lamports_per_signature)) => {
Some(lamports_per_signature)
}
Some(DurableNonceFee::Invalid) => None,
None => bank.get_lamports_per_signature_for_blockhash(
&tx.message().recent_blockhash,
),
}
.expect("lamports_per_signature must be available");
let fee = bank.get_fee_for_message_with_lamports_per_signature(
&SanitizedMessage::try_from_legacy_message(
tx.message().clone(),
&ReservedAccountKeys::empty_key_set(),
)
.unwrap(),
lamports_per_signature,
);

let inner_instructions = inner_instructions.map(|inner_instructions| {
map_inner_instructions(inner_instructions).collect()
});

let tx_status_meta = TransactionStatusMeta {
status,
fee,
fee: fee_details.total_fee(),
pre_balances,
post_balances,
pre_token_balances: Some(pre_token_balances),
Expand Down
33 changes: 6 additions & 27 deletions rpc/src/transaction_status_service.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use {
blockstore::Blockstore,
blockstore_processor::{TransactionStatusBatch, TransactionStatusMessage},
},
solana_svm::transaction_results::{DurableNonceFee, TransactionExecutionDetails},
solana_svm::transaction_results::TransactionExecutionDetails,
solana_transaction_status::{
extract_and_fmt_memos, map_inner_instructions, Reward, TransactionStatusMeta,
},
Expand Down Expand Up @@ -99,27 +99,14 @@ impl TransactionStatusService {
status,
log_messages,
inner_instructions,
durable_nonce_fee,
return_data,
executed_units,
fee_details,
..
} = details;
let lamports_per_signature = match durable_nonce_fee {
Some(DurableNonceFee::Valid(lamports_per_signature)) => {
Some(lamports_per_signature)
}
Some(DurableNonceFee::Invalid) => None,
None => bank.get_lamports_per_signature_for_blockhash(
transaction.message().recent_blockhash(),
),
}
.expect("lamports_per_signature must be available");
let fee = bank.get_fee_for_message_with_lamports_per_signature(
transaction.message(),
lamports_per_signature,
);
let tx_account_locks = transaction.get_account_locks_unchecked();

let fee = fee_details.total_fee();
let inner_instructions = inner_instructions.map(|inner_instructions| {
map_inner_instructions(inner_instructions).collect()
});
Expand Down Expand Up @@ -217,6 +204,7 @@ pub(crate) mod tests {
solana_sdk::{
account_utils::StateMut,
clock::Slot,
fee::FeeDetails,
hash::Hash,
nonce::{self, state::DurableNonce},
nonce_account,
Expand All @@ -230,7 +218,6 @@ pub(crate) mod tests {
VersionedTransaction,
},
},
solana_svm::nonce_info::{NonceFull, NoncePartial},
solana_transaction_status::{
token_balances::TransactionTokenBalancesSet, TransactionStatusMeta,
TransactionTokenBalance,
Expand Down Expand Up @@ -333,23 +320,15 @@ pub(crate) mod tests {
)))
.unwrap();

let rollback_partial = NoncePartial::new(pubkey, nonce_account.clone());

let mut rent_debits = RentDebits::default();
rent_debits.insert(&pubkey, 123, 456);

let fee_payer_address = &pubkey;
let fee_payer_account = nonce_account;
let transaction_result = Some(TransactionExecutionDetails {
status: Ok(()),
log_messages: None,
inner_instructions: None,
durable_nonce_fee: Some(DurableNonceFee::from(&NonceFull::from_partial(
&rollback_partial,
fee_payer_address,
fee_payer_account,
&rent_debits,
))),
fee_details: FeeDetails::default(),
is_nonce: true,
return_data: None,
executed_units: 0,
accounts_data_len_delta: 0,
Expand Down
86 changes: 19 additions & 67 deletions runtime/src/bank.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3889,29 +3889,23 @@ impl Bank {
txs: &[SanitizedTransaction],
execution_results: &[TransactionExecutionResult],
) -> Vec<Result<()>> {
let hash_queue = self.blockhash_queue.read().unwrap();
let mut fees = 0;

let results = txs
.iter()
.zip(execution_results)
.map(|(tx, execution_result)| {
let message = tx.message();
let (execution_status, is_nonce, lamports_per_signature) =
Self::get_details_from_execution_result(
&hash_queue,
execution_result,
message.recent_blockhash(),
)?;
let fee = self.get_fee_for_message_with_lamports_per_signature(
message,
lamports_per_signature,
);
let details = match &execution_result {
TransactionExecutionResult::Executed { details, .. } => details,
TransactionExecutionResult::NotExecuted(err) => return Err(err.clone()),
};

let fee = details.fee_details.total_fee();
self.check_execution_status_and_charge_fee(
message,
execution_status,
is_nonce,
&details.status,
details.is_nonce,
fee,
)?;

Expand All @@ -3930,42 +3924,27 @@ impl Bank {
txs: &[SanitizedTransaction],
execution_results: &[TransactionExecutionResult],
) -> Vec<Result<()>> {
let hash_queue = self.blockhash_queue.read().unwrap();
let mut accumulated_fee_details = FeeDetails::default();

let results = txs
.iter()
.zip(execution_results)
.map(|(tx, execution_result)| {
let message = tx.message();
let (execution_status, is_nonce, lamports_per_signature) =
Self::get_details_from_execution_result(
&hash_queue,
execution_result,
message.recent_blockhash(),
)?;

if !FeeStructure::to_clear_transaction_fee(lamports_per_signature) {
let fee_details = self.fee_structure().calculate_fee_details(
message,
&process_compute_budget_instructions(message.program_instructions_iter())
.unwrap_or_default()
.into(),
self.feature_set
.is_active(&include_loaded_accounts_data_size_in_fee_calculation::id()),
self.feature_set
.is_active(&remove_rounding_in_fee_calculation::id()),
);
let details = match &execution_result {
TransactionExecutionResult::Executed { details, .. } => details,
TransactionExecutionResult::NotExecuted(err) => return Err(err.clone()),
};

self.check_execution_status_and_charge_fee(
message,
execution_status,
is_nonce,
fee_details.total_fee(),
)?;
self.check_execution_status_and_charge_fee(
message,
&details.status,
details.is_nonce,
details.fee_details.total_fee(),
)?;

accumulated_fee_details.accumulate(&details.fee_details);

accumulated_fee_details.accumulate(&fee_details);
}
Ok(())
})
.collect();
Expand All @@ -3977,33 +3956,6 @@ impl Bank {
results
}

fn get_details_from_execution_result<'a>(
hash_queue_readonly: &RwLockReadGuard<'a, BlockhashQueue>,
execution_result: &'a TransactionExecutionResult,
transaction_blockhash: &Hash,
) -> Result<(&'a transaction::Result<()>, bool, u64)> {
let (execution_status, durable_nonce_fee) = match &execution_result {
TransactionExecutionResult::Executed { details, .. } => {
Ok((&details.status, details.durable_nonce_fee.as_ref()))
}
TransactionExecutionResult::NotExecuted(err) => Err(err.clone()),
}?;

let (lamports_per_signature, is_nonce) = durable_nonce_fee
.map(|durable_nonce_fee| durable_nonce_fee.lamports_per_signature())
.map(|maybe_lamports_per_signature| (maybe_lamports_per_signature, true))
.unwrap_or_else(|| {
(
hash_queue_readonly.get_lamports_per_signature(transaction_blockhash),
false,
)
});

let lamports_per_signature =
lamports_per_signature.ok_or(TransactionError::BlockhashNotFound)?;
Ok((execution_status, is_nonce, lamports_per_signature))
}

fn check_execution_status_and_charge_fee(
&self,
message: &SanitizedMessage,
Expand Down
1 change: 1 addition & 0 deletions runtime/src/bank/fee_distribution.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ impl Bank {
) -> u64 {
let fee_details = self.fee_structure().calculate_fee_details(
transaction.message(),
self.get_lamports_per_signature(),
fee_budget_limits,
self.feature_set
.is_active(&include_loaded_accounts_data_size_in_fee_calculation::id()),
Expand Down
Loading