feat(scorer): add reason sub-label to profit_scored_total#149
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
b704f5b to
f2cb65c
Compare
aether_mempool_profit_scored_total used to be labelled only by
decision, so the dashboard could see "10 reverted" but not whether
they came from the V2 U256 walker, the f64 absurdity floor, the V3
revm verifier, or organic revm reverts. Add a `reason` sub-label
distinguishing those code paths.
Five wire labels (pinned by unit test against the constants):
- n/a non-reverted decisions, no_path, or any path with
no sub-source worth distinguishing
- u256_walker V2-only exact-U256 walker reached a verdict
(PR #136 path)
- absurdity_floor f64 fallback above MAX_PLAUSIBLE_F64_NET_WEI (1 ETH)
downgraded to reverted (PR #136 path)
- revm_verdict V3-touching revm sim ran to completion with a
non-reverting verdict (PR #144 path)
- revm_revert V3-touching revm sim explicitly reverted/halted
(PR #144 path)
The reason is Prometheus-only and NOT persisted to the
mempool_profitability table — the migration's CHECK constraint only
covers decision, and adding a reason column would force every
existing row to back-fill. `NewProfitabilityScore.reason` skips the
DB insert path; it only flows into the metric label.
revm_verdict_to_decision and f64_fallback_verdict now return a
4-tuple (net, realised, decision, reason). no_path_outcome carries
REASON_NA. The aggregating let in score_one destructures
(net, realised, decision, reason) and threads reason into
ScoreOutcome.
Dashboard panels in deploy/docker/grafana/dashboards/mempool.json
(panel IDs 19 and 20 from PR #146) updated to sum by
(decision, reason) and legend-format {{decision}} / {{reason}}.
Title and description updated to reflect the new dimension.
Stacks on feat/dedupe-replay-scorer-helpers (PR #148).
Verification:
- cargo clippy --workspace --all-targets -- -D warnings : clean
- cargo test --workspace --lib --bins : 528 passed, 0 failed
- new test: reason_constants_are_stable_wire_labels
- existing verdict-helper tests updated to assert reason value
- python3 -m json.tool mempool.json : parses cleanly
7b31a6f to
e39c717
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
`aether_mempool_profit_scored_total` was labelled only by `decision`, so
the dashboard could see "10 reverted" but not whether they came from
the V2 U256 walker (PR #136), the f64 absurdity floor (PR #136), the
V3 revm verifier (PR #144), or organic revm reverts. Add a `reason`
sub-label distinguishing those code paths so PR #146's decision-pie
Stacks on PR #148 (`feat/dedupe-replay-scorer-helpers`).
Five reason values
Wire labels pinned by `reason_constants_are_stable_wire_labels`:
Wire-format
cardinality bounded at ~10 `(decision, reason)` combinations.
the `mempool_profitability` Postgres table because the migration's
CHECK constraint only covers `decision`. `NewProfitabilityScore.reason`
is consumed by `PgProfitabilityWriter::insert_score` for the metric
label and dropped before the SQL bind. No migration needed.
Code changes
`crates/grpc-server/src/profitability_writer.rs` (+62 / −5):
`["decision", "reason"]`.
`serde(default = "default_reason")` shim returning `REASON_NA` so
external callers that haven't been updated still deserialise cleanly.
the `try_send` and uses it in the metric `with_label_values` call.
`crates/grpc-server/src/bin/aether_profit_scorer.rs` (+45 / −19):
4-tuple `(net, realised, decision, reason)`.
the U256-walker arm explicitly attaches `REASON_U256_WALKER`.
`NewProfitabilityScore` constructed for the writer.
`deploy/docker/grafana/dashboards/mempool.json` (+8 / −8):
legend `{{decision}} / {{reason}}`, title and description updated.
Verification
```
cargo clippy --workspace --all-targets -- -D warnings ✓ clean
cargo test --workspace --lib --bins ✓ 528 passed, 0 failed
python3 -m json.tool mempool.json ✓ parses
```
New unit test:
Existing verdict-helper tests gain `reason` assertions:
Operational notes
After this lands, the live scorer process (PID 2476 at handoff time)
needs to restart to pick up the new metric label set. Grafana caches
metric metadata; the panels will start splitting once the new metric
samples arrive. The pre-restart counter series (cardinality
`{decision}` only) coexists with the new `{decision, reason}` series
without breakage — Prometheus treats them as distinct families.
Out of scope
(pool absent, eth_call empty, optimiser bailed, tokens missing,
zero reserves) all collapse to `REASON_NA` today. If one proves
operationally interesting (e.g. sustained `pool_not_in_registry`
signals a V3 pool-discovery gap), it can be split out in a follow-up.
combinations, the cardinality is well below any Prometheus practical
limit. No `relabel_config` needed.