block-prioritization-fee metrics update by tao-stones · Pull Request #6967 · anza-xyz/agave

tao-stones · 2025-07-14T23:22:32Z

Problem

block-prioritization-fee metrics reports data incorrectly:

min_prioritization_fee is actually min of compute_unit_price, denominated in micro-lamports
max_prioritization_fee is actually max of compute_unit_price, denominated in micro-lamports
total_prioritization_fee is the sum of all non-vote transaction's compute_unit_price, which is misleading.

Summary of Changes

Replace [min|max]-prioritization-fee with [min|max]-compute-unit-price to truly reflect the data points are raw cu price user set in micro-lamports;
total-prioritization-fee is updated to report the sum of all landed transactions' prioritization fee (eg compute_unit_price * compute_unit_limit).
Added an additional counter datapoint block_prioritization_fee_counters.total_calculate_priotitization_fee_elapsed_us to monitor time spent on calculate prioritization fees.

Fixes #6948

codecov-commenter · 2025-07-15T00:06:47Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.2%. Comparing base (2abe5ce) to head (b7c6fa7).
Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff            @@
##           master    #6967     +/-   ##
=========================================
- Coverage    83.2%    83.2%   -0.1%     
=========================================
  Files         853      853             
  Lines      375634   375678     +44     
=========================================
+ Hits       312647   312679     +32     
- Misses      62987    62999     +12

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

tao-stones · 2025-07-15T00:38:54Z

+                            .prioritization_fee,
+                        solana_fee::FeeFeatures::from(bank.feature_set.as_ref()),
+                    )
+                });


Making additional fee calculation in hot path might be an issue, reporting its us, testing in testnet

metrics reported from devbox indicates near 0 us

Yeah I don't like the additional work here.

It's also worth noting that I've had a WIP PR to make PrioritizationFeeCache rpc-only - we don't need to do any of this on a block-producing node; but then we'd lose the metrics.

See my other comment on reporting all fees.

I felt the same. Was focusing on Trent's particular use case, when one is curious about how cu-price is bidding up, it helps to put total fee collected as ref point. But yea, I don't love to have to calculate fee again here just for reporting.

One benefit of reporting here vs from cost-tracking is, here only report when block is opportunistically confirmed. Wondering if adding fee to transaction meta is a good idea - I chatted about it before, don't remember why we didn't do it.

but we won't report this on building nodes if we make prio-fee-cache optional (we should).

it's not separate infra, but the cost tracking is currently where we report other objective block facts around the cost. I feel it makes sense to just lop it near there for now so we have semething without reinventing a metric system.

i mostly want the separate infra 'cause 5000 identical datapoints is kinda useless load on the metrics db

agreed, but i feel we should have this metric asap even if repeated. then can figure out how we want to report going forward separately, but don't lose data time

Agree. Do we have objection of reporting total_priority_fee at cost of one multiplying (eg FeeBudgetLimits::from(compute_budget_limits))?

Can add total transaction fee reporting at Cost Tracker separately.

that's fine

tao-stones · 2025-07-15T00:40:54Z

-    // Total prioritization fees included in this slot.
-    total_prioritization_fee: Saturating<u64>,
+    // Total transaction fees of non-vote transactions included in this slot.
+    total_non_vote_transaction_fee: Saturating<u64>,


vote transactions are filtered out early, so only non-vote-transaction-fees are accumulated for confirmed blocks, which probably is the right thing to look at when analyzing prioritization fee behaviors

I'd actually be in favor of removing this field of the metric entirely.
We should not re-calculate the costs just to get a metric.

Each bank has the fee-details on it. We should report those somewhere, though it doesn't necessarily have to be in this datapoint.

A good place might be in the same loop as cost-reporting. and we report with a tag is_leader in a similar way.

wdyt?

tao-stones · 2025-07-15T14:49:13Z

-    // The minimum prioritization fee of prioritized transactions in this slot.
-    min_prioritization_fee: Option<u64>,
+    // The minimum compute unit price of prioritized transactions in this slot.
+    min_compute_unit_price: Option<u64>,


the min|max_compute_unit_prices are reported as sanitized raw value, in micro-lamports; vs above total_non_vote_transaction_fee is reported in lamports

tao-stones · 2025-07-15T14:56:32Z

testnet reports on internal-metrics:

tao-stones · 2025-07-15T14:59:05Z

note: block_min_prioritization_fee is obsoleted, removing it in separate pr #6973

t-nelson · 2025-07-15T21:18:22Z

assuming 992b0f8 is purely cosmetic?

tao-stones · 2025-07-15T21:22:24Z

assuming 992b0f8 is purely cosmetic?

Yes, only renaming "_prioritization_fee" to "_compute_unit_price"

tao-stones · 2025-07-18T16:39:43Z

Here’s a snapshot of a few minutes of data from mainnet-beta:

Orange line shows the total prioritization fees collected (in lamports).
Blue line represents the max compute unit price (in micro-lamports).

Observed a spike in max-cu-price that did not have similar effect in total prioritization fees. This suggests the spike may have been driven by outlier transactions, rather than a broad fee market up-bidding.

* clean up naming * accumulates and reports included transaction fees * only collect total prioritization fee

tao-stones commented Jul 15, 2025

View reviewed changes

tao-stones force-pushed the fix-prioritization-fee-metrics branch from f0e4505 to 3b2c3a7 Compare July 15, 2025 00:45

tao-stones commented Jul 15, 2025

View reviewed changes

tao-stones marked this pull request as ready for review July 15, 2025 14:56

tao-stones requested review from apfitzge and t-nelson July 15, 2025 14:56

tao-stones added 3 commits July 15, 2025 16:49

clean up naming

8583b99

accumulates and reports included transaction fees

784161a

only collect total prioritization fee

b7c6fa7

tao-stones force-pushed the fix-prioritization-fee-metrics branch from 3b2c3a7 to b7c6fa7 Compare July 15, 2025 21:52

apfitzge approved these changes Jul 18, 2025

View reviewed changes

t-nelson approved these changes Jul 18, 2025

View reviewed changes

tao-stones merged commit de80e15 into anza-xyz:master Jul 18, 2025
41 checks passed

tao-stones deleted the fix-prioritization-fee-metrics branch July 18, 2025 19:50

puhtaytow pushed a commit to puhtaytow/agave that referenced this pull request Jul 24, 2025

block-prioritization-fee metrics update (anza-xyz#6967)

dad4468

* clean up naming * accumulates and reports included transaction fees * only collect total prioritization fee

Conversation

tao-stones commented Jul 14, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Problem

Summary of Changes

Uh oh!

codecov-commenter commented Jul 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tao-stones Jul 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tao-stones commented Jul 15, 2025

Uh oh!

tao-stones commented Jul 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

t-nelson commented Jul 15, 2025

Uh oh!

tao-stones commented Jul 15, 2025

Uh oh!

tao-stones commented Jul 18, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

tao-stones commented Jul 14, 2025 •

edited

Loading

codecov-commenter commented Jul 15, 2025 •

edited

Loading

tao-stones Jul 15, 2025 •

edited

Loading

tao-stones commented Jul 15, 2025 •

edited

Loading