gaspersic/solana
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Revert "Added vote limits to be 75% of total block limit"

Browse Source 
This reverts commit 3e131a5324.
This commit is contained in:
Tao Zhu
2022-01-18 13:13:23 -06:00
committed by Trent Nelson
parent aabcdcf8fb
commit fc8f61368d
5 changed files with 60 additions and 142 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
core/benches/banking_stage.rs
View File

@ -171,7 +171,7 @@ fn bench_banking(bencher: &mut Bencher, tx_type: TransactionType) {
// set cost tracker limits to MAX so it will not filter out TXs
bank.write_cost_tracker()
.unwrap()
.set_limits(std::u64::MAX, std::u64::MAX, std::u64::MAX);
.set_limits(std::u64::MAX, std::u64::MAX);
debug!("threads: {} txs: {}", num_threads, txes);

22
core/src/banking_stage.rs
View File

@ -1130,7 +1130,7 @@ impl BankingStage {
let tx: Transaction = limited_deserialize(&p.data[0..p.meta.size]).ok()?;
tx.verify_precompiles(feature_set).ok()?;
Some((tx, *tx_index, p.meta.is_simple_vote_tx))
Some((tx, *tx_index))
})
.collect();
banking_stage_stats.cost_tracker_check_count.fetch_add(
@ -1142,18 +1142,18 @@ impl BankingStage {
let filtered_transactions_with_packet_indexes: Vec<_> = {
verified_transactions_with_packet_indexes
.into_iter()
.filter_map(|(tx, tx_index, is_simple_vote_tx)| {
.filter_map(|(tx, tx_index)| {
// put transaction into retry queue if it wouldn't fit
// into current bank
if read_cost_tracker
.would_transaction_fit(
is_simple_vote_tx,
&cost_model
.read()
.unwrap()
.calculate_cost(&tx, demote_program_write_locks),
)
.is_err()
.would_transaction_fit(
&tx,
&cost_model
.read()
.unwrap()
.calculate_cost(&tx, demote_program_write_locks),
)
.is_err()
{
debug!("transaction {:?} would exceed limit", tx);
retryable_transaction_packet_indexes.push(tx_index);
@ -1293,7 +1293,7 @@ impl BankingStage {
transactions.iter().enumerate().for_each(|(index, tx)| {
if unprocessed_tx_indexes.iter().all(|&i| i != index) {
bank.write_cost_tracker().unwrap().add_transaction_cost(
tx.is_simple_vote_transaction(),
tx.transaction(),
&cost_model
.read()
.unwrap()

5
ledger-tool/src/main.rs
View File

@ -31,7 +31,6 @@ use {
cost_model::CostModel,
cost_tracker::CostTracker,
hardened_unpack::{open_genesis_config, MAX_GENESIS_ARCHIVE_UNPACKED_SIZE},
hashed_transaction::HashedTransaction,
snapshot_utils::{self, SnapshotVersion, DEFAULT_MAX_SNAPSHOTS_TO_RETAIN},
},
solana_sdk::{
@ -756,9 +755,7 @@ fn compute_slot_cost(blockstore: &Blockstore, slot: Slot) -> Result<(), String>
for transaction in &entry.transactions {
programs += transaction.message().instructions.len();
let tx_cost = cost_model.calculate_cost(transaction, true);
let hashed_transaction = HashedTransaction::from(transaction);
let result =
cost_tracker.try_add(hashed_transaction.is_simple_vote_transaction(), &tx_cost);
let result = cost_tracker.try_add(transaction, &tx_cost);
if result.is_err() {
println!(
"Slot: {}, CostModel rejected transaction {:?}, reason {:?}",

3
runtime/src/block_cost_limits.rs
View File

@ -59,9 +59,6 @@ pub const MAX_BLOCK_UNITS: u64 =
/// limit is to prevent too many transactions write to same account, therefore
/// reduce block's parallelism.
pub const MAX_WRITABLE_ACCOUNT_UNITS: u64 = MAX_BLOCK_REPLAY_TIME_US * COMPUTE_UNIT_TO_US_RATIO;
/// Number of compute units that a block can have for vote transactions,
/// sets at ~75% of MAX_BLOCK_UNITS to leave room for non-vote transactions
pub const MAX_VOTE_UNITS: u64 = (MAX_BLOCK_UNITS as f64 * 0.75_f64) as u64;
/// max length of account data in a slot (bytes)
pub const MAX_ACCOUNT_DATA_LEN: u64 = 100_000_000;

170
runtime/src/cost_tracker.rs
View File

@ -5,7 +5,7 @@
//!
use {
crate::{block_cost_limits::*, cost_model::TransactionCost},
solana_sdk::{clock::Slot, pubkey::Pubkey},
solana_sdk::{clock::Slot, pubkey::Pubkey, transaction::Transaction},
std::collections::HashMap,
};
@ -16,9 +16,6 @@ pub enum CostTrackerError {
/// would exceed block max limit
WouldExceedBlockMaxLimit,
/// would exceed vote max limit
WouldExceedVoteMaxLimit,
/// would exceed account max limit
WouldExceedAccountMaxLimit,
}
@ -27,66 +24,55 @@ pub enum CostTrackerError {
pub struct CostTracker {
account_cost_limit: u64,
block_cost_limit: u64,
vote_cost_limit: u64,
cost_by_writable_accounts: HashMap<Pubkey, u64>,
block_cost: u64,
vote_cost: u64,
transaction_count: u64,
}
impl Default for CostTracker {
fn default() -> Self {
CostTracker::new(MAX_WRITABLE_ACCOUNT_UNITS, MAX_BLOCK_UNITS, MAX_VOTE_UNITS)
CostTracker::new(MAX_WRITABLE_ACCOUNT_UNITS, MAX_BLOCK_UNITS)
}
}
impl CostTracker {
pub fn new(account_cost_limit: u64, block_cost_limit: u64, vote_cost_limit: u64) -> Self {
pub fn new(account_cost_limit: u64, block_cost_limit: u64) -> Self {
assert!(account_cost_limit <= block_cost_limit);
assert!(vote_cost_limit <= block_cost_limit);
Self {
account_cost_limit,
block_cost_limit,
vote_cost_limit,
cost_by_writable_accounts: HashMap::with_capacity(WRITABLE_ACCOUNTS_PER_BLOCK),
block_cost: 0,
vote_cost: 0,
transaction_count: 0,
}
}
// bench tests needs to reset limits
pub fn set_limits(
&mut self,
account_cost_limit: u64,
block_cost_limit: u64,
vote_cost_limit: u64,
) {
pub fn set_limits(&mut self, account_cost_limit: u64, block_cost_limit: u64) {
self.account_cost_limit = account_cost_limit;
self.block_cost_limit = block_cost_limit;
self.vote_cost_limit = vote_cost_limit;
}
pub fn would_transaction_fit(
&self,
is_vote: bool,
_transaction: &Transaction,
tx_cost: &TransactionCost,
) -> Result<(), CostTrackerError> {
self.would_fit(&tx_cost.writable_accounts, tx_cost.sum(), is_vote)
self.would_fit(&tx_cost.writable_accounts, &tx_cost.sum())
}
pub fn add_transaction_cost(&mut self, is_vote: bool, tx_cost: &TransactionCost) {
self.add_transaction(&tx_cost.writable_accounts, tx_cost.sum(), is_vote);
pub fn add_transaction_cost(&mut self, _transaction: &Transaction, tx_cost: &TransactionCost) {
self.add_transaction(&tx_cost.writable_accounts, &tx_cost.sum());
}
pub fn try_add(
&mut self,
is_vote: bool,
_transaction: &Transaction,
tx_cost: &TransactionCost,
) -> Result<u64, CostTrackerError> {
let cost = tx_cost.sum();
self.would_fit(&tx_cost.writable_accounts, cost, is_vote)?;
self.add_transaction(&tx_cost.writable_accounts, cost, is_vote);
self.would_fit(&tx_cost.writable_accounts, &cost)?;
self.add_transaction(&tx_cost.writable_accounts, &cost);
Ok(self.block_cost)
}
@ -102,7 +88,6 @@ impl CostTracker {
"cost_tracker_stats",
("bank_slot", bank_slot as i64, i64),
("block_cost", self.block_cost as i64, i64),
("vote_cost", self.vote_cost as i64, i64),
("transaction_count", self.transaction_count as i64, i64),
(
"number_of_accounts",
@ -127,19 +112,14 @@ impl CostTracker {
(costliest_account, costliest_account_cost)
}
fn would_fit(&self, keys: &[Pubkey], cost: u64, is_vote: bool) -> Result<(), CostTrackerError> {
fn would_fit(&self, keys: &[Pubkey], cost: &u64) -> Result<(), CostTrackerError> {
// check against the total package cost
if self.block_cost.saturating_add(cost) > self.block_cost_limit {
if self.block_cost + cost > self.block_cost_limit {
return Err(CostTrackerError::WouldExceedBlockMaxLimit);
}
// if vote transaction, check if it exceeds vote_transaction_limit
if is_vote && self.vote_cost.saturating_add(cost) > self.vote_cost_limit {
return Err(CostTrackerError::WouldExceedVoteMaxLimit);
}
// check if the transaction itself is more costly than the account_cost_limit
if cost > self.account_cost_limit {
if *cost > self.account_cost_limit {
return Err(CostTrackerError::WouldExceedAccountMaxLimit);
}
@ -147,7 +127,7 @@ impl CostTracker {
for account_key in keys.iter() {
match self.cost_by_writable_accounts.get(&account_key) {
Some(chained_cost) => {
if chained_cost.saturating_add(cost) > self.account_cost_limit {
if chained_cost + cost > self.account_cost_limit {
return Err(CostTrackerError::WouldExceedAccountMaxLimit);
} else {
continue;
@ -160,19 +140,15 @@ impl CostTracker {
Ok(())
}
fn add_transaction(&mut self, keys: &[Pubkey], cost: u64, is_vote: bool) {
fn add_transaction(&mut self, keys: &[Pubkey], cost: &u64) {
for account_key in keys.iter() {
let account_cost = self
*self
.cost_by_writable_accounts
.entry(*account_key)
.or_insert(0);
*account_cost = account_cost.saturating_add(cost);
.or_insert(0) += cost;
}
self.block_cost = self.block_cost.saturating_add(cost);
if is_vote {
self.vote_cost = self.vote_cost.saturating_add(cost);
}
self.transaction_count = self.transaction_count.saturating_add(1);
self.block_cost += cost;
self.transaction_count += 1;
}
}
@ -218,10 +194,9 @@ mod tests {
#[test]
fn test_cost_tracker_initialization() {
let testee = CostTracker::new(10, 11, 8);
let testee = CostTracker::new(10, 11);
assert_eq!(10, testee.account_cost_limit);
assert_eq!(11, testee.block_cost_limit);
assert_eq!(8, testee.vote_cost_limit);
assert_eq!(0, testee.cost_by_writable_accounts.len());
assert_eq!(0, testee.block_cost);
}
@ -232,28 +207,10 @@ mod tests {
let (_tx, keys, cost) = build_simple_transaction(&mint_keypair, &start_hash);
// build testee to have capacity for one simple transaction
let mut testee = CostTracker::new(cost, cost, cost);
assert!(testee.would_fit(&keys, cost, false).is_ok());
testee.add_transaction(&keys, cost, false);
let mut testee = CostTracker::new(cost, cost);
assert!(testee.would_fit(&keys, &cost).is_ok());
testee.add_transaction(&keys, &cost);
assert_eq!(cost, testee.block_cost);
assert_eq!(0, testee.vote_cost);
let (_costliest_account, costliest_account_cost) = testee.find_costliest_account();
assert_eq!(cost, costliest_account_cost);
}
#[test]
fn test_cost_tracker_ok_add_one_vote() {
let (mint_keypair, start_hash) = test_setup();
let (_tx, keys, cost) = build_simple_transaction(&mint_keypair, &start_hash);
// build testee to have capacity for one simple transaction
let mut testee = CostTracker::new(cost, cost, cost);
assert!(testee.would_fit(&keys, cost, true).is_ok());
testee.add_transaction(&keys, cost, true);
assert_eq!(cost, testee.block_cost);
assert_eq!(cost, testee.vote_cost);
let (_ccostliest_account, costliest_account_cost) = testee.find_costliest_account();
assert_eq!(cost, costliest_account_cost);
}
#[test]
@ -264,19 +221,17 @@ mod tests {
let (_tx2, keys2, cost2) = build_simple_transaction(&mint_keypair, &start_hash);
// build testee to have capacity for two simple transactions, with same accounts
let mut testee = CostTracker::new(cost1 + cost2, cost1 + cost2, cost1 + cost2);
let mut testee = CostTracker::new(cost1 + cost2, cost1 + cost2);
{
assert!(testee.would_fit(&keys1, cost1, false).is_ok());
testee.add_transaction(&keys1, cost1, false);
assert!(testee.would_fit(&keys1, &cost1).is_ok());
testee.add_transaction(&keys1, &cost1);
}
{
assert!(testee.would_fit(&keys2, cost2, false).is_ok());
testee.add_transaction(&keys2, cost2, false);
assert!(testee.would_fit(&keys2, &cost2).is_ok());
testee.add_transaction(&keys2, &cost2);
}
assert_eq!(cost1 + cost2, testee.block_cost);
assert_eq!(1, testee.cost_by_writable_accounts.len());
let (_ccostliest_account, costliest_account_cost) = testee.find_costliest_account();
assert_eq!(cost1 + cost2, costliest_account_cost);
}
#[test]
@ -288,19 +243,17 @@ mod tests {
let (_tx2, keys2, cost2) = build_simple_transaction(&second_account, &start_hash);
// build testee to have capacity for two simple transactions, with same accounts
let mut testee = CostTracker::new(cmp::max(cost1, cost2), cost1 + cost2, cost1 + cost2);
let mut testee = CostTracker::new(cmp::max(cost1, cost2), cost1 + cost2);
{
assert!(testee.would_fit(&keys1, cost1, false).is_ok());
testee.add_transaction(&keys1, cost1, false);
assert!(testee.would_fit(&keys1, &cost1).is_ok());
testee.add_transaction(&keys1, &cost1);
}
{
assert!(testee.would_fit(&keys2, cost2, false).is_ok());
testee.add_transaction(&keys2, cost2, false);
assert!(testee.would_fit(&keys2, &cost2).is_ok());
testee.add_transaction(&keys2, &cost2);
}
assert_eq!(cost1 + cost2, testee.block_cost);
assert_eq!(2, testee.cost_by_writable_accounts.len());
let (_ccostliest_account, costliest_account_cost) = testee.find_costliest_account();
assert_eq!(std::cmp::max(cost1, cost2), costliest_account_cost);
}
#[test]
@ -311,21 +264,20 @@ mod tests {
let (_tx2, keys2, cost2) = build_simple_transaction(&mint_keypair, &start_hash);
// build testee to have capacity for two simple transactions, but not for same accounts
let mut testee = CostTracker::new(cmp::min(cost1, cost2), cost1 + cost2, cost1 + cost2);
let mut testee = CostTracker::new(cmp::min(cost1, cost2), cost1 + cost2);
// should have room for first transaction
{
assert!(testee.would_fit(&keys1, cost1, false).is_ok());
testee.add_transaction(&keys1, cost1, false);
assert!(testee.would_fit(&keys1, &cost1).is_ok());
testee.add_transaction(&keys1, &cost1);
}
// but no more sapce on the same chain (same signer account)
{
assert!(testee.would_fit(&keys2, cost2, false).is_err());
assert!(testee.would_fit(&keys2, &cost2).is_err());
}
}
#[test]
fn test_cost_tracker_reach_limit() {
let none_vote = false;
let (mint_keypair, start_hash) = test_setup();
// build two transactions with diff accounts
let (_tx1, keys1, cost1) = build_simple_transaction(&mint_keypair, &start_hash);
@ -333,50 +285,22 @@ mod tests {
let (_tx2, keys2, cost2) = build_simple_transaction(&second_account, &start_hash);
// build testee to have capacity for each chain, but not enough room for both transactions
let mut testee =
CostTracker::new(cmp::max(cost1, cost2), cost1 + cost2 - 1, cost1 + cost2 - 1);
let mut testee = CostTracker::new(cmp::max(cost1, cost2), cost1 + cost2 - 1);
// should have room for first transaction
{
assert!(testee.would_fit(&keys1, cost1, none_vote).is_ok());
testee.add_transaction(&keys1, cost1, none_vote);
assert!(testee.would_fit(&keys1, &cost1).is_ok());
testee.add_transaction(&keys1, &cost1);
}
// but no more room for package as whole
{
assert!(testee.would_fit(&keys2, cost2, none_vote).is_err());
}
}
#[test]
fn test_cost_tracker_reach_vote_limit() {
let is_vote = true;
let (mint_keypair, start_hash) = test_setup();
// build two mocking vote transactions with diff accounts
let (_tx1, keys1, cost1) = build_simple_transaction(&mint_keypair, &start_hash);
let second_account = Keypair::new();
let (_tx2, keys2, cost2) = build_simple_transaction(&second_account, &start_hash);
// build testee to have capacity for each chain, but not enough room for both votes
let mut testee = CostTracker::new(cmp::max(cost1, cost2), cost1 + cost2, cost1 + cost2 - 1);
// should have room for first vote
{
assert!(testee.would_fit(&keys1, cost1, is_vote).is_ok());
testee.add_transaction(&keys1, cost1, is_vote);
}
// but no more room for package as whole
{
assert!(testee.would_fit(&keys2, cost2, is_vote).is_err());
}
// however there is room for tx2, if it is not a vote
{
assert!(testee.would_fit(&keys2, cost2, !is_vote).is_ok());
assert!(testee.would_fit(&keys2, &cost2).is_err());
}
}
#[test]
fn test_cost_tracker_try_add_is_atomic() {
let (mint_keypair, start_hash) = test_setup();
let (_ctx, _keys, _cost) = build_simple_transaction(&mint_keypair, &start_hash);
let none_vote = false;
let (tx, _keys, _cost) = build_simple_transaction(&mint_keypair, &start_hash);
let acct1 = Pubkey::new_unique();
let acct2 = Pubkey::new_unique();
@ -385,7 +309,7 @@ mod tests {
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);
let mut testee = CostTracker::new(account_max, block_max);
// case 1: a tx writes to 3 accounts, should success, we will have:
// | acct1 | $cost |
@ -398,7 +322,7 @@ mod tests {
execution_cost: cost,
..TransactionCost::default()
};
assert!(testee.try_add(none_vote, &tx_cost).is_ok());
assert!(testee.try_add(&tx, &tx_cost).is_ok());
let (_costliest_account, costliest_account_cost) = testee.find_costliest_account();
assert_eq!(cost, testee.block_cost);
assert_eq!(3, testee.cost_by_writable_accounts.len());
@ -416,7 +340,7 @@ mod tests {
execution_cost: cost,
..TransactionCost::default()
};
assert!(testee.try_add(none_vote, &tx_cost).is_ok());
assert!(testee.try_add(&tx, &tx_cost).is_ok());
let (costliest_account, costliest_account_cost) = testee.find_costliest_account();
assert_eq!(cost * 2, testee.block_cost);
assert_eq!(3, testee.cost_by_writable_accounts.len());
@ -436,7 +360,7 @@ mod tests {
execution_cost: cost,
..TransactionCost::default()
};
assert!(testee.try_add(none_vote, &tx_cost).is_err());
assert!(testee.try_add(&tx, &tx_cost).is_err());
let (costliest_account, costliest_account_cost) = testee.find_costliest_account();
assert_eq!(cost * 2, testee.block_cost);
assert_eq!(3, testee.cost_by_writable_accounts.len());