Skip to content

metric: replace 10 duplicated bucket literals with leanSpec-named constants (#303)#304

Open
mananuf wants to merge 1 commit into
devnet-4from
spec/bucket-constants-alignment
Open

metric: replace 10 duplicated bucket literals with leanSpec-named constants (#303)#304
mananuf wants to merge 1 commit into
devnet-4from
spec/bucket-constants-alignment

Conversation

@mananuf
Copy link
Copy Markdown
Collaborator

@mananuf mananuf commented May 22, 2026

Summary

Ten of gean's 21 histograms in `node/metrics.go` had inline `[]float64` bucket literals whose values already matched one of the four named bucket constants leanSpec defines in `subspecs/metrics/registry.py:36-45`. Same bucket boundaries copy-pasted 2-6 times per latency shape. Replaces them with 4 named package-private vars that mirror the spec.

Closes #303.

What changed

Single file, `node/metrics.go`: +25 / -15.

Added (4 vars at top of histograms block)

```go
// Bucket sets mirror leanSpec's named constants in
// lean_spec/subspecs/metrics/registry.py:36-45. Defined here once so the
// histograms that share these shapes don't each carry a duplicated literal
// that can drift out of spec alignment silently. Histograms whose latency
// shape doesn't fit any of the four spec constants use their own hand-rolled
// bucket array inline and note that fact in a comment.
var (
forkChoiceBlockBuckets = []float64{0.005, 0.01, 0.025, 0.05, 0.1, 1, 1.25, 1.5, 2, 4}
attestationValidationBuckets = []float64{0.005, 0.01, 0.025, 0.05, 0.1, 1}
stateTransitionBuckets = []float64{0.25, 0.5, 0.75, 1, 1.25, 1.5, 2, 2.5, 3, 4}
reorgDepthBuckets = []float64{1, 2, 3, 5, 7, 10, 20, 30, 50, 100}
)
```

Replaced (10 histograms)

Histogram Now uses
`lean_fork_choice_block_processing_time_seconds` `forkChoiceBlockBuckets`
`lean_attestation_validation_time_seconds` `attestationValidationBuckets`
`lean_committee_signatures_aggregation_time_seconds` `stateTransitionBuckets`
`lean_pq_sig_attestation_signing_time_seconds` `attestationValidationBuckets`
`lean_pq_sig_attestation_verification_time_seconds` `attestationValidationBuckets`
`lean_fork_choice_reorg_depth` `reorgDepthBuckets`
`lean_state_transition_time_seconds` `stateTransitionBuckets`
`lean_state_transition_slots_processing_time_seconds` `attestationValidationBuckets`
`lean_state_transition_block_processing_time_seconds` `attestationValidationBuckets`
`lean_state_transition_attestations_processing_time_seconds` `attestationValidationBuckets`

Out of scope (11 histograms left hand-rolled)

These either don't measure time at all (byte sizes, payload counts, coverage ratios) or measure a time shape no spec bucket constant covers (`block_building`, `attestations_production`, aggregated-sig building/verification, full-block sig verification). Left alone.

Zero behavior change

Each replaced literal had the same values byte-for-byte as the named constant it now points to. Verified by reading the leanSpec file side-by-side and by:

  • `go test -count=1 ./node/...` green
  • No new buckets, no removed buckets, no reordering

A `/metrics` scrape post-merge will produce byte-identical bucket boundaries for the 10 affected histograms.

Spec compliance

Addresses the standing ⚠️ from the previous `/spec-compliant devnet3 head` audit:

"Histogram bucket boundaries should be cross-checked vs spec constants. Possible drift — worth verifying."

After this PR + #301, all gean histograms whose latency shape matches a spec-named bucket constant traceably reference it. The 11 hand-rolled ones are honestly hand-rolled (no spec constant covers them) and don't pretend otherwise.

Lean-review

Single subtraction-oriented pass during implementation:

  • `duplicates-existing-util`: ✅ collapses 10 duplicated bucket arrays into 4 named refs
  • `comment-bloat`: header comment is 6 lines but earns its keep (explains the spec cross-reference + the policy for hand-rolled histograms — both non-obvious to a future reader)
  • `premature-abstraction`: 4 named vars used 2-6 times each; threshold for naming clearly met
  • All other categories: 0 findings

Related

…stants (#303)

Ten of gean's histograms in node/metrics.go used inline []float64 literals
whose values already matched one of the four named bucket constants leanSpec
defines in subspecs/metrics/registry.py:36-45. The literals were copy-pasted
across the file — same bucket boundaries duplicated 2-6 times per shape.

Add 4 package-private vars (forkChoiceBlockBuckets,
attestationValidationBuckets, stateTransitionBuckets, reorgDepthBuckets)
at the top of the Histograms block, with a cross-reference comment to the
spec line numbers. Replace the 10 matching Buckets: lines with references
to the named vars.

Zero behavior change: each replaced literal had the same values as the
named constant it now points to. Verified by reading the spec file
side-by-side and by go test ./node/... passing.

Spec-compliance: addresses the standing ⚠️ flag from /spec-compliant
devnet3 head ("Histogram bucket boundaries should be cross-checked vs
spec constants"). Bucket boundaries used in gean are now traceable to a
spec constant via grep.

11 histograms remain hand-rolled — mostly non-time metrics (byte sizes,
payload counts, coverage ratios) where the spec's four time-bucket
constants don't semantically apply. Left as-is.

Closes #303.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant