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

Add functions to collect executed transactions fee in details; #178

Merged
merged 7 commits into from
Mar 21, 2024
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
117 changes: 106 additions & 11 deletions runtime/src/bank.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ use {
self, include_loaded_accounts_data_size_in_fee_calculation,
remove_rounding_in_fee_calculation, FeatureSet,
},
fee::FeeStructure,
fee::{FeeDetails, FeeStructure},
fee_calculator::{FeeCalculator, FeeRateGovernor},
genesis_config::{ClusterType, GenesisConfig},
hard_forks::HardForks,
Expand Down Expand Up @@ -256,6 +256,23 @@ impl AddAssign for SquashTiming {
}
}

#[derive(AbiExample, Debug, Default, PartialEq)]
pub(crate) struct CollectorFeeDetails {
pub transaction_fee: u64,
pub priority_fee: u64,
}

impl CollectorFeeDetails {
pub(crate) fn accumulate(&mut self, fee_details: &FeeDetails) {
self.transaction_fee = self
.transaction_fee
.saturating_add(fee_details.transaction_fee());
self.priority_fee = self
.priority_fee
.saturating_add(fee_details.prioritization_fee());
}
}

#[derive(Debug)]
pub struct BankRc {
/// where all the Accounts are stored
Expand Down Expand Up @@ -551,6 +568,7 @@ impl PartialEq for Bank {
epoch_reward_status: _,
transaction_processor: _,
check_program_modification_slot: _,
collector_fee_details: _,
// Ignore new fields explicitly if they do not impact PartialEq.
// Adding ".." will remove compile-time checks that if a new field
// is added to the struct, this PartialEq is accordingly updated.
Expand Down Expand Up @@ -813,6 +831,9 @@ pub struct Bank {
transaction_processor: TransactionBatchProcessor<BankForks>,

check_program_modification_slot: bool,

/// Collected fee details
collector_fee_details: RwLock<CollectorFeeDetails>,
tao-stones marked this conversation as resolved.
Show resolved Hide resolved
tao-stones marked this conversation as resolved.
Show resolved Hide resolved
}

struct VoteWithStakeDelegations {
Expand Down Expand Up @@ -1000,6 +1021,7 @@ impl Bank {
epoch_reward_status: EpochRewardStatus::default(),
transaction_processor: TransactionBatchProcessor::default(),
check_program_modification_slot: false,
collector_fee_details: RwLock::new(CollectorFeeDetails::default()),
};

bank.transaction_processor = TransactionBatchProcessor::new(
Expand Down Expand Up @@ -1319,6 +1341,7 @@ impl Bank {
epoch_reward_status: parent.epoch_reward_status.clone(),
transaction_processor: TransactionBatchProcessor::default(),
check_program_modification_slot: false,
collector_fee_details: RwLock::new(CollectorFeeDetails::default()),
};

new.transaction_processor = TransactionBatchProcessor::new(
Expand Down Expand Up @@ -1866,6 +1889,8 @@ impl Bank {
epoch_reward_status: fields.epoch_reward_status,
transaction_processor: TransactionBatchProcessor::default(),
check_program_modification_slot: false,
// collector_fee_details is not serialized to snapshot
collector_fee_details: RwLock::new(CollectorFeeDetails::default()),
};

bank.transaction_processor = TransactionBatchProcessor::new(
Expand Down Expand Up @@ -4883,16 +4908,12 @@ impl Bank {
lamports_per_signature,
);

// In case of instruction error, even though no accounts
// were stored we still need to charge the payer the
// fee.
//
//...except nonce accounts, which already have their
// post-load, fee deducted, pre-execute account state
// stored
if execution_status.is_err() && !is_nonce {
self.withdraw(tx.message().fee_payer(), fee)?;
}
self.check_execution_status_and_charge_fee(
tx.message(),
execution_status,
is_nonce,
fee,
)?;

fees += fee;
Ok(())
Expand All @@ -4903,6 +4924,80 @@ impl Bank {
results
}

// Note: this function is not yet used; next PR will call it behind a feature gate
#[allow(dead_code)]
fn filter_program_errors_and_collect_fee_details(
apfitzge marked this conversation as resolved.
Show resolved Hide resolved
apfitzge marked this conversation as resolved.
Show resolved Hide resolved
&self,
txs: &[SanitizedTransaction],
execution_results: &[TransactionExecutionResult],
) -> Vec<Result<()>> {
let mut accumulated_fee_details = FeeDetails::default();

let results = txs
.iter()
.zip(execution_results)
.map(|(tx, execution_result)| {
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 is_nonce = durable_nonce_fee.is_some();
tao-stones marked this conversation as resolved.
Show resolved Hide resolved

let message = tx.message();
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.check_execution_status_and_charge_fee(
message,
execution_status,
is_nonce,
fee_details.total_fee(
self.feature_set
.is_active(&remove_rounding_in_fee_calculation::id()),
),
)?;

accumulated_fee_details.accumulate(&fee_details);
Ok(())
})
.collect();

self.collector_fee_details
.write()
.unwrap()
.accumulate(&accumulated_fee_details);
results
}

fn check_execution_status_and_charge_fee(
&self,
message: &SanitizedMessage,
execution_status: &transaction::Result<()>,
is_nonce: bool,
fee: u64,
) -> Result<()> {
// In case of instruction error, even though no accounts
// were stored we still need to charge the payer the
// fee.
//
//...except nonce accounts, which already have their
// post-load, fee deducted, pre-execute account state
// stored
if execution_status.is_err() && !is_nonce {
self.withdraw(message.fee_payer(), fee)?;
}

Ok(())
}

/// `committed_transactions_count` is the number of transactions out of `sanitized_txs`
/// that was executed. Of those, `committed_transactions_count`,
/// `committed_with_failure_result_count` is the number of executed transactions that returned
Expand Down
129 changes: 129 additions & 0 deletions runtime/src/bank/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13792,3 +13792,132 @@ fn test_failed_simulation_compute_units() {
let simulation = bank.simulate_transaction(&sanitized, false);
assert_eq!(expected_consumed_units, simulation.units_consumed);
}

#[test]
fn test_filter_program_errors_and_collect_fee_details() {
// TX | EXECUTION RESULT | is nonce | COLLECT | ADDITIONAL | COLLECT
// | | | (TX_FEE, PRIO_FEE) | WITHDRAW FROM PAYER | RESULT
// ------------------------------------------------------------------------------------------------------
// tx1 | not executed | n/a | (0 , 0) | 0 | Original Err
// tx2 | executed and no error | n/a | (5_000, 1_000) | 0 | Ok
// tx3 | executed has error | true | (5_000, 1_000) | 0 | Ok
// tx4 | executed has error | false | (5_000, 1_000) | 6_000 | Ok
// tx5 | executed error,
// payer insufficient fund | false | (0 , 0) | 0 | InsufficientFundsForFee
//
let initial_payer_balance = 7_000;
let additional_payer_withdraw = 6_000;
let expected_collected_fee_details = CollectorFeeDetails {
transaction_fee: 15_000,
priority_fee: 3_000,
};
let expected_collect_results = vec![
Err(TransactionError::AccountNotFound),
Ok(()),
Ok(()),
Ok(()),
Err(TransactionError::InsufficientFundsForFee),
];

let GenesisConfigInfo {
mut genesis_config,
mint_keypair,
..
} = create_genesis_config_with_leader(initial_payer_balance, &Pubkey::new_unique(), 3);
genesis_config.fee_rate_governor = FeeRateGovernor::new(5000, 0);
let bank = Bank::new_for_tests(&genesis_config);

let tx = SanitizedTransaction::from_transaction_for_tests(Transaction::new_signed_with_payer(
&[
system_instruction::transfer(&mint_keypair.pubkey(), &Pubkey::new_unique(), 2),
ComputeBudgetInstruction::set_compute_unit_limit(1_000),
ComputeBudgetInstruction::set_compute_unit_price(1_000_000),
],
Some(&mint_keypair.pubkey()),
&[&mint_keypair],
genesis_config.hash(),
));
let txs = vec![tx.clone(), tx.clone(), tx.clone(), tx.clone(), tx];

let results = vec![
TransactionExecutionResult::NotExecuted(TransactionError::AccountNotFound),
new_execution_result(Ok(()), None),
new_execution_result(
Err(TransactionError::InstructionError(
1,
SystemError::ResultWithNegativeLamports.into(),
)),
Some(&NonceFull::default()),
),
new_execution_result(
Err(TransactionError::InstructionError(
1,
SystemError::ResultWithNegativeLamports.into(),
)),
None,
),
new_execution_result(Err(TransactionError::AccountNotFound), None),
];

let results = bank.filter_program_errors_and_collect_fee_details(&txs, &results);

assert_eq!(
expected_collected_fee_details,
*bank.collector_fee_details.read().unwrap()
);
assert_eq!(
initial_payer_balance - additional_payer_withdraw,
bank.get_balance(&mint_keypair.pubkey())
);
assert_eq!(expected_collect_results, results);
}

#[test]
fn test_check_execution_status_and_charge_fee() {
let fee = 5000;
let initial_balance = fee - 1000;
let tx_error =
TransactionError::InstructionError(0, InstructionError::MissingRequiredSignature);
let GenesisConfigInfo {
mut genesis_config,
mint_keypair,
..
} = create_genesis_config_with_leader(initial_balance, &Pubkey::new_unique(), 3);
genesis_config.fee_rate_governor = FeeRateGovernor::new(5000, 0);
let bank = Bank::new_for_tests(&genesis_config);
let message = new_sanitized_message(Message::new(
&[system_instruction::transfer(
&mint_keypair.pubkey(),
&Pubkey::new_unique(),
1,
)],
Some(&mint_keypair.pubkey()),
));

[Ok(()), Err(tx_error)]
.iter()
.flat_map(|result| [true, false].iter().map(move |is_nonce| (result, is_nonce)))
.for_each(|(result, is_nonce)| {
if result.is_err() && !is_nonce {
assert_eq!(
Err(TransactionError::InsufficientFundsForFee),
bank.check_execution_status_and_charge_fee(&message, result, *is_nonce, fee)
);
assert_eq!(initial_balance, bank.get_balance(&mint_keypair.pubkey()));

let small_fee = 1;
assert!(bank
.check_execution_status_and_charge_fee(&message, result, *is_nonce, small_fee)
.is_ok());
assert_eq!(
initial_balance - small_fee,
bank.get_balance(&mint_keypair.pubkey())
);
} else {
assert!(bank
.check_execution_status_and_charge_fee(&message, result, *is_nonce, fee)
.is_ok());
assert_eq!(initial_balance, bank.get_balance(&mint_keypair.pubkey()));
}
});
}
17 changes: 17 additions & 0 deletions sdk/src/fee.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,23 @@ impl FeeDetails {
(total_fee as f64).round() as u64
}
}

pub fn accumulate(&mut self, fee_details: &FeeDetails) {
self.transaction_fee = self
.transaction_fee
.saturating_add(fee_details.transaction_fee);
self.prioritization_fee = self
.prioritization_fee
.saturating_add(fee_details.prioritization_fee)
}

pub fn transaction_fee(&self) -> u64 {
self.transaction_fee
}

pub fn prioritization_fee(&self) -> u64 {
self.prioritization_fee
}
}

pub const ACCOUNT_DATA_COST_PAGE_SIZE: u64 = 32_u64.saturating_mul(1024);
Expand Down
Loading