Skip to content
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
30 changes: 29 additions & 1 deletion cost-model/src/cost_tracker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@
//! - add_transaction_cost(&tx_cost), mutable function to accumulate tx_cost to tracker.
//!
use {
crate::{block_cost_limits::*, transaction_cost::TransactionCost},
crate::{
block_cost_limits::*, cost_tracker_post_analysis::CostTrackerPostAnalysis,
transaction_cost::TransactionCost,
},
solana_metrics::datapoint_info,
solana_pubkey::Pubkey,
solana_runtime_transaction::transaction_with_meta::TransactionWithMeta,
Expand Down Expand Up @@ -433,6 +436,15 @@ impl CostTracker {
}
}

/// Implement the trait for the cost tracker
/// This is only used for post-analysis to avoid lock contention
/// Do not use in the hot path
impl CostTrackerPostAnalysis for CostTracker {
fn get_cost_by_writable_accounts(&self) -> &HashMap<Pubkey, u64, ahash::RandomState> {
&self.cost_by_writable_accounts
}
}

#[cfg(test)]
mod tests {
use {
Expand Down Expand Up @@ -998,4 +1010,20 @@ mod tests {
assert_eq!(0, cost_tracker.vote_cost);
assert_eq!(0, cost_tracker.allocated_accounts_data_size.0);
}

#[test]
fn test_get_cost_by_writable_accounts_post_analysis() {
let mut cost_tracker = CostTracker::default();
let cost = 100u64;
let transaction = WritableKeysTransaction(vec![Pubkey::new_unique()]);
let tx_cost = simple_transaction_cost(&transaction, cost);
cost_tracker.add_transaction_cost(&tx_cost);
let cost_by_writable_accounts = cost_tracker.get_cost_by_writable_accounts();
assert_eq!(1, cost_by_writable_accounts.len());
assert_eq!(cost, *cost_by_writable_accounts.values().next().unwrap());
assert_eq!(
*cost_by_writable_accounts,
cost_tracker.cost_by_writable_accounts
);
}
}
8 changes: 8 additions & 0 deletions cost-model/src/cost_tracker_post_analysis.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
use {solana_pubkey::Pubkey, std::collections::HashMap};

/// Trait to help with post-analysis of a given block
pub trait CostTrackerPostAnalysis {
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we already have the feature, is there a purpose in having the trait?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requiring manually use trait to access that function is an additional safety for dev.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it comes down to maintainability and making sure the get method is not used inadvertently.
The trait is kept behind a feature and the implementation as well.

So, if someone has to use it within agave code, the change would be very noticeable.
This should deter any such usage as it's not part of the vanilla struct and keeps it distinct.

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right but it's 2 things to do the same thing, prevent accidental usage. Just choose one I think

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can keep only the trait, if the general recommendation is not to use the feature gate.

Copy link
Copy Markdown
Author

@ebin-mathews ebin-mathews Sep 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reverted the feature.
@tao-stones

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'll also vote of using trait. Ok without feature gate

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right but it's 2 things to do the same thing, prevent accidental usage. Just choose one I think

if we actually cared about safety here, we'd use the type system and transform to a type that specifies the new method only after we've left the "hot path" not pray that dev reads a comment in a different file

/// Only use in post-analyze to avoid lock contention
/// Do not use in the hot path
fn get_cost_by_writable_accounts(&self) -> &HashMap<Pubkey, u64, ahash::RandomState>;
}
1 change: 1 addition & 0 deletions cost-model/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
pub mod block_cost_limits;
pub mod cost_model;
pub mod cost_tracker;
pub mod cost_tracker_post_analysis;
pub mod transaction_cost;

#[cfg_attr(feature = "frozen-abi", macro_use)]
Expand Down
Loading