Remove the code that handles cost update for separate pr
This commit is contained in:
parent
85281de981
commit
578d59c802
|
@ -206,13 +206,9 @@ impl QosService {
|
||||||
.for_each(|((tx_cost, qos_inclusion_result), was_committed)| {
|
.for_each(|((tx_cost, qos_inclusion_result), was_committed)| {
|
||||||
// Only transactions that the qos service included have to be
|
// Only transactions that the qos service included have to be
|
||||||
// checked for update
|
// checked for update
|
||||||
if qos_inclusion_result.is_ok() {
|
if qos_inclusion_result.is_ok() && !*was_committed {
|
||||||
if *was_committed {
|
|
||||||
cost_tracker.update_execution_cost(tx_cost, None);
|
|
||||||
} else {
|
|
||||||
cost_tracker.remove(tx_cost);
|
cost_tracker.remove(tx_cost);
|
||||||
}
|
}
|
||||||
}
|
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
@ -5,7 +5,7 @@
|
||||||
//!
|
//!
|
||||||
use {
|
use {
|
||||||
crate::{block_cost_limits::*, cost_model::TransactionCost},
|
crate::{block_cost_limits::*, cost_model::TransactionCost},
|
||||||
solana_sdk::{clock::Slot, num_utils::*, pubkey::Pubkey},
|
solana_sdk::{clock::Slot, pubkey::Pubkey},
|
||||||
std::collections::HashMap,
|
std::collections::HashMap,
|
||||||
};
|
};
|
||||||
|
|
||||||
|
@ -95,36 +95,6 @@ impl CostTracker {
|
||||||
Ok(self.block_cost)
|
Ok(self.block_cost)
|
||||||
}
|
}
|
||||||
|
|
||||||
pub fn update_execution_cost(
|
|
||||||
&mut self,
|
|
||||||
estimated_tx_cost: &TransactionCost,
|
|
||||||
actual_execution_units: Option<u64>,
|
|
||||||
) {
|
|
||||||
if let Some(actual_execution_units) = actual_execution_units {
|
|
||||||
let estimated_execution_units = estimated_tx_cost.execution_cost;
|
|
||||||
if actual_execution_units == estimated_execution_units {
|
|
||||||
return;
|
|
||||||
}
|
|
||||||
|
|
||||||
if let Some(adjustment) =
|
|
||||||
Self::checked_diff_unsigned(actual_execution_units, estimated_execution_units)
|
|
||||||
{
|
|
||||||
self.adjust_transaction_execution_cost(estimated_tx_cost, adjustment);
|
|
||||||
} else {
|
|
||||||
// handle adjustment too big to fit in `i64` error, log event as error,
|
|
||||||
// set block_cost to limit to prevent more transactions being added into
|
|
||||||
// curernt block.
|
|
||||||
log::error!(
|
|
||||||
"cost_tracker detected erroneous attemopt to adjust execution cost, \
|
|
||||||
estimated transactino cost {:?}, actual execution units {}",
|
|
||||||
estimated_tx_cost,
|
|
||||||
actual_execution_units
|
|
||||||
);
|
|
||||||
self.block_cost = self.block_cost_limit;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
pub fn remove(&mut self, tx_cost: &TransactionCost) {
|
pub fn remove(&mut self, tx_cost: &TransactionCost) {
|
||||||
self.remove_transaction_cost(tx_cost);
|
self.remove_transaction_cost(tx_cost);
|
||||||
}
|
}
|
||||||
|
@ -264,42 +234,6 @@ impl CostTracker {
|
||||||
.saturating_sub(tx_cost.account_data_size);
|
.saturating_sub(tx_cost.account_data_size);
|
||||||
self.transaction_count = self.transaction_count.saturating_sub(1);
|
self.transaction_count = self.transaction_count.saturating_sub(1);
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Apply execution units adjustment to cost-tracker.
|
|
||||||
/// If adjustment causes arithmetic overflow, it bumps costs to their limits to
|
|
||||||
/// prevent adding further transactions into current block.
|
|
||||||
fn adjust_transaction_execution_cost(&mut self, tx_cost: &TransactionCost, adjustment: i64) {
|
|
||||||
for account_key in tx_cost.writable_accounts.iter() {
|
|
||||||
let account_cost = self
|
|
||||||
.cost_by_writable_accounts
|
|
||||||
.entry(*account_key)
|
|
||||||
.or_insert(0);
|
|
||||||
match UintUtil::checked_add_signed(*account_cost, adjustment) {
|
|
||||||
Some(adjusted_cost) => *account_cost = adjusted_cost,
|
|
||||||
None => *account_cost = self.account_cost_limit,
|
|
||||||
}
|
|
||||||
}
|
|
||||||
match UintUtil::checked_add_signed(self.block_cost, adjustment) {
|
|
||||||
Some(adjusted_cost) => self.block_cost = adjusted_cost,
|
|
||||||
None => self.block_cost = self.block_cost_limit,
|
|
||||||
}
|
|
||||||
if tx_cost.is_simple_vote {
|
|
||||||
match UintUtil::checked_add_signed(self.vote_cost, adjustment) {
|
|
||||||
Some(adjusted_cost) => self.vote_cost = adjusted_cost,
|
|
||||||
None => self.vote_cost = self.vote_cost_limit,
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
/// Checked get deltai as `i64` of two `u64`, computes `lhs - rhs`,
|
|
||||||
/// returning `None` if delta overflows `i64`
|
|
||||||
fn checked_diff_unsigned(lhs: u64, rhs: u64) -> Option<i64> {
|
|
||||||
if lhs >= rhs {
|
|
||||||
IntUtil::checked_add_unsigned(0i64, lhs - rhs)
|
|
||||||
} else {
|
|
||||||
IntUtil::checked_sub_unsigned(0i64, rhs - lhs)
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
|
@ -742,122 +676,4 @@ mod tests {
|
||||||
assert_eq!(acct2, costliest_account);
|
assert_eq!(acct2, costliest_account);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
|
||||||
fn test_adjust_transaction_execution_cost() {
|
|
||||||
let acct1 = Pubkey::new_unique();
|
|
||||||
let acct2 = Pubkey::new_unique();
|
|
||||||
let acct3 = Pubkey::new_unique();
|
|
||||||
let cost = 100;
|
|
||||||
let account_max = cost * 2;
|
|
||||||
let block_max = account_max * 3; // for three accts
|
|
||||||
|
|
||||||
let mut testee = CostTracker::new(account_max, block_max, block_max, None);
|
|
||||||
let tx_cost = TransactionCost {
|
|
||||||
writable_accounts: vec![acct1, acct2, acct3],
|
|
||||||
execution_cost: cost,
|
|
||||||
..TransactionCost::default()
|
|
||||||
};
|
|
||||||
let mut expected_block_cost = tx_cost.sum();
|
|
||||||
let expected_tx_count = 1;
|
|
||||||
assert!(testee.try_add(&tx_cost).is_ok());
|
|
||||||
assert_eq!(expected_block_cost, testee.block_cost());
|
|
||||||
assert_eq!(expected_tx_count, testee.transaction_count());
|
|
||||||
testee
|
|
||||||
.cost_by_writable_accounts
|
|
||||||
.iter()
|
|
||||||
.for_each(|(_key, units)| {
|
|
||||||
assert_eq!(expected_block_cost, *units);
|
|
||||||
});
|
|
||||||
|
|
||||||
// adjust up
|
|
||||||
{
|
|
||||||
let adjustment = 50i64;
|
|
||||||
testee.adjust_transaction_execution_cost(&tx_cost, adjustment);
|
|
||||||
expected_block_cost += 50;
|
|
||||||
assert_eq!(expected_block_cost, testee.block_cost());
|
|
||||||
assert_eq!(expected_tx_count, testee.transaction_count());
|
|
||||||
testee
|
|
||||||
.cost_by_writable_accounts
|
|
||||||
.iter()
|
|
||||||
.for_each(|(_key, units)| {
|
|
||||||
assert_eq!(expected_block_cost, *units);
|
|
||||||
});
|
|
||||||
}
|
|
||||||
|
|
||||||
// adjust down
|
|
||||||
{
|
|
||||||
let adjustment = -50i64;
|
|
||||||
testee.adjust_transaction_execution_cost(&tx_cost, adjustment);
|
|
||||||
expected_block_cost -= 50;
|
|
||||||
assert_eq!(expected_block_cost, testee.block_cost());
|
|
||||||
assert_eq!(expected_tx_count, testee.transaction_count());
|
|
||||||
testee
|
|
||||||
.cost_by_writable_accounts
|
|
||||||
.iter()
|
|
||||||
.for_each(|(_key, units)| {
|
|
||||||
assert_eq!(expected_block_cost, *units);
|
|
||||||
});
|
|
||||||
}
|
|
||||||
|
|
||||||
// adjust overflow up
|
|
||||||
{
|
|
||||||
testee.adjust_transaction_execution_cost(&tx_cost, i64::MAX);
|
|
||||||
testee.adjust_transaction_execution_cost(&tx_cost, i64::MAX);
|
|
||||||
// expect block cost set to limit
|
|
||||||
assert_eq!(block_max, testee.block_cost());
|
|
||||||
assert_eq!(expected_tx_count, testee.transaction_count());
|
|
||||||
testee
|
|
||||||
.cost_by_writable_accounts
|
|
||||||
.iter()
|
|
||||||
.for_each(|(_key, units)| {
|
|
||||||
assert_eq!(account_max, *units);
|
|
||||||
});
|
|
||||||
}
|
|
||||||
|
|
||||||
// adjust overflow down
|
|
||||||
{
|
|
||||||
testee.adjust_transaction_execution_cost(&tx_cost, i64::MIN);
|
|
||||||
testee.adjust_transaction_execution_cost(&tx_cost, i64::MIN);
|
|
||||||
// expect block cost set to limit
|
|
||||||
assert_eq!(block_max, testee.block_cost());
|
|
||||||
assert_eq!(expected_tx_count, testee.transaction_count());
|
|
||||||
testee
|
|
||||||
.cost_by_writable_accounts
|
|
||||||
.iter()
|
|
||||||
.for_each(|(_key, units)| {
|
|
||||||
assert_eq!(account_max, *units);
|
|
||||||
});
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
#[test]
|
|
||||||
fn test_checked_diff_unsigned() {
|
|
||||||
solana_logger::setup();
|
|
||||||
|
|
||||||
// no diff
|
|
||||||
assert_eq!(0, CostTracker::checked_diff_unsigned(10u64, 10u64).unwrap());
|
|
||||||
assert_eq!(
|
|
||||||
0,
|
|
||||||
CostTracker::checked_diff_unsigned(u64::MAX, u64::MAX).unwrap()
|
|
||||||
);
|
|
||||||
|
|
||||||
// positive diff
|
|
||||||
assert_eq!(2, CostTracker::checked_diff_unsigned(10u64, 8u64).unwrap());
|
|
||||||
assert_eq!(
|
|
||||||
8,
|
|
||||||
CostTracker::checked_diff_unsigned(u64::MAX, u64::MAX - 8u64).unwrap()
|
|
||||||
);
|
|
||||||
|
|
||||||
// negative diff
|
|
||||||
assert_eq!(-2, CostTracker::checked_diff_unsigned(8u64, 10u64).unwrap());
|
|
||||||
assert_eq!(
|
|
||||||
-10,
|
|
||||||
CostTracker::checked_diff_unsigned(u64::MIN, 10u64).unwrap()
|
|
||||||
);
|
|
||||||
|
|
||||||
// diff too big for i64 (overflow)
|
|
||||||
assert_eq!(None, CostTracker::checked_diff_unsigned(u64::MAX, 0u64));
|
|
||||||
assert_eq!(None, CostTracker::checked_diff_unsigned(0u64, u64::MAX));
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in New Issue