Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(sidecar): gross tip revenue metric #339

Merged
merged 4 commits into from
Oct 29, 2024
+126 −4
Merged

feat(sidecar): gross tip revenue metric #339

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
7dfc323
feat(sidecar): define gross tip revenue metric
mempirate Oct 29, 2024
30a6683
feat(sidecar): define gross tip revenue metric
mempirate Oct 29, 2024
759ab52
fix(ci): fix sidecar ci
mempirate Oct 29, 2024
dde39ae
feat(sidecar): calculate and record gross tip revenue on new head
mempirate Oct 29, 2024
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
5 changes: 4 additions & 1 deletion .github/workflows/bolt_sidecar_ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@ name: Bolt Sidecar CI

on:
push:
branches:
- unstable
- main
Copy link
Contributor

Choose a reason for hiding this comment

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

The main branch is unused. Any specific reason for adding it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We might use it in the future

paths:
- "bolt-sidecar/**"
pull_request:
Expand All @@ -18,7 +21,7 @@ concurrency:
jobs:
cargo-tests:
runs-on: ubuntu-latest
timeout-minutes: 10
timeout-minutes: 20
env:
RUST_BACKTRACE: 1

Expand Down
24 changes: 24 additions & 0 deletions bolt-sidecar/src/telemetry/metrics.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,10 @@ const INCLUSION_COMMITMENTS_ACCEPTED: &str = "bolt_sidecar_inclusion_commitments
const TRANSACTIONS_PRECONFIRMED: &str = "bolt_sidecar_transactions_preconfirmed";
/// Counter for the number of validation errors; to spot most the most common ones
const VALIDATION_ERRORS: &str = "bolt_sidecar_validation_errors";
/// Counter that tracks the gross tip revenue. Effective tip per gas * gas used.
/// We call it "gross" because in the case of PBS, it doesn't mean the proposer will
/// get all of this as revenue.
const GROSS_TIP_REVENUE: &str = "bolt_sidecar_gross_tip_revenue";

// Gauges ------------------------------------------------------------------
/// Gauge for the latest slot number
Expand All @@ -44,6 +48,7 @@ impl ApiMetrics {
describe_counter!(INCLUSION_COMMITMENTS_ACCEPTED, "Inclusion commitments accepted");
describe_counter!(TRANSACTIONS_PRECONFIRMED, "Transactions preconfirmed");
describe_counter!(VALIDATION_ERRORS, "Validation errors");
describe_counter!(GROSS_TIP_REVENUE, "Gross tip revenue");

// Gauges
describe_gauge!(LATEST_HEAD, "Latest slot number");
Expand Down Expand Up @@ -81,6 +86,25 @@ impl ApiMetrics {
counter!(INCLUSION_COMMITMENTS_ACCEPTED).increment(1);
}

pub fn increment_gross_tip_revenue(mut tip: u128) {
// If the tip is too large, we need to split it into multiple u64 parts
if tip > u64::MAX as u128 {
let mut parts = Vec::new();
while tip > u64::MAX as u128 {
parts.push(u64::MAX);
tip -= u64::MAX as u128;
}

parts.push(tip as u64);

for part in parts {
counter!(GROSS_TIP_REVENUE).increment(part);
}
} else {
counter!(GROSS_TIP_REVENUE).increment(tip as u64);
}
Comment on lines +90 to +105
Copy link
Contributor

Choose a reason for hiding this comment

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

In my opinion this is a bit simpler/succint and doesn't distinguish between edge cases and regular cases which is nice.

Suggested change
// If the tip is too large, we need to split it into multiple u64 parts
if tip > u64::MAX as u128 {
let mut parts = Vec::new();
while tip > u64::MAX as u128 {
parts.push(u64::MAX);
tip -= u64::MAX as u128;
}
parts.push(tip as u64);
for part in parts {
counter!(GROSS_TIP_REVENUE).increment(part);
}
} else {
counter!(GROSS_TIP_REVENUE).increment(tip as u64);
}
// Takes also into account tips larger than a u128 by splitting them into u64 parts
let mut parts = Vec::new();
let u64_max = u64::MAX as u128;
while tip > 0 {
// We can only add a max tip of u64::MAX.
let min = tip.min(u64_max) as u64;
parts.push(min);
// Subtract the part we just added, if something is left then tip is going to be
// greater than zero.
tip = tip.saturating_sub(u64_max)
}
parts.push(tip as u64);
for part in parts {
counter!(GROSS_TIP_REVENUE).increment(part);
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

True, but this always does an allocation

Copy link
Contributor

Choose a reason for hiding this comment

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

That's fair, also because with an u64 you can express a tip up to 18.44 ETH :D

}

pub fn increment_transactions_preconfirmed(tx_type: TxType) {
counter!(TRANSACTIONS_PRECONFIRMED, &[("type", tx_type_str(tx_type))]).increment(1);
}
Expand Down