Skip to content

Commit

Permalink
Add functions to collect executed transactions fee in details; (#178)
Browse files Browse the repository at this point in the history
* Add functions to collect executed transactions fee in details;

* remove unnecessary derive attributes

* change function name from add to accumulate; remove collector_fee_details from PartialEq

* add AbiExample

* add test

* share function to withdraw errored transaction

* more tests
  • Loading branch information
tao-stones authored Mar 21, 2024
1 parent 9818815 commit 1e08e90
Show file tree
Hide file tree
Showing 3 changed files with 252 additions and 11 deletions.
117 changes: 106 additions & 11 deletions runtime/src/bank.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,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 @@ -259,6 +259,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 @@ -554,6 +571,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 @@ -816,6 +834,9 @@ pub struct Bank {
transaction_processor: TransactionBatchProcessor<BankForks>,

check_program_modification_slot: bool,

/// Collected fee details
collector_fee_details: RwLock<CollectorFeeDetails>,
}

struct VoteWithStakeDelegations {
Expand Down Expand Up @@ -1003,6 +1024,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 @@ -1322,6 +1344,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 @@ -1869,6 +1892,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 @@ -4886,16 +4911,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 @@ -4906,6 +4927,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(
&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();

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

0 comments on commit 1e08e90

Please sign in to comment.