Skip to content

sv2: count TP fee checks and template builds - #952

Merged
reardencode merged 4 commits into
reardencode:masterfrom
average-gary:sv2/tp-build-metrics
Oct 8, 2026
Merged

reardencode merged 4 commits into
reardencode:masterfrom
average-gary:sv2/tp-build-metrics

Conversation

@average-gary

Copy link
Copy Markdown
Contributor

Stacked on #951 (SV2 fee push, itself on #949). Review the last commit; this PR's own range is 1b6c343a..8d1dc741.

Observability for the SV2 Template Provider's template work, and the test that pins two CPU-only regressions the fee push could otherwise ship silently.

Changes

  • Sv2TpHandle::stats() → Sv2TpStats { fee_checks, builds } (RequestMeters) per listener, across its sessions. A fee check is counted once per call; a template build (constraints, tip, or fee check) is counted with its wall time inside the blocking region, not while held in IBD.
  • DEBUG tip: perf gains sv2 checks= builds= build_avg_us= build_max_us= (window), and /metrics exports rbitcoin_sv2_fee_checks_total, rbitcoin_sv2_template_builds_total, rbitcoin_sv2_template_build_seconds_total (running totals), only with --sv2-tp-listen.
  • docs/operator/operations.md metrics table and tip: perf row; plan doc's named mutant survivors now point at the pinning test; changelog.d/sv2-tp-build-metrics.md.

Tests

  • idle_session_checks_each_interval_and_does_not_rebuild: an idle session shows about one check per interval and exactly one build. Kills the mutants that delete the check re-arm (the session spins: ~750k checks) and that drop the unchanged-counter skip (a rebuild every interval).
  • sv2_tp_bootstrap scrapes /metrics: builds ≥ the 7 templates received plus the below-delta check's build, checks ≥ 2, build seconds > 0.

Local verification: fmt, clippy -D warnings, deny, ast-grep, ci-os-smoke.sh; full cargo test --workspace in Linux Docker (rust:1.95) on the pre-review tip: 2730 passed, 0 failed. The review follow-up (drop the unread check timer, reason on the too_many_arguments allow) reran clippy, the sv2 lib suite, and the journey green.

🤖 Generated with Claude Code

@rearden-grok rearden-grok Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The metrics commit is in good shape. fee_checks counts every poll (including the idle no-build path), builds times only a template::build that returns inside the blocking region, IBD holds are not builds, /metrics uses the running totals, and tip: perf takes the window. idle_session_checks_each_interval_and_does_not_rebuild is the right pin for the two CPU mutants, and the journey scrape matches the counters operators will see. Required checks are green.

Not approving the stack. This branch still has the #951 fee-check bug: build stores seen_updates before the blocking call returns, and a failed check then becomes a success that will not retry. The 1000-output steady-admission test is on this branch too. Fix those on #951 and this commit can land with them.

Comment thread crates/rbitcoin-sv2/src/session.rs Outdated
};
// Read before the build: a tx admitted during it moves the counter
// past this, so the next check rebuilds.
self.seen_updates = self.chain.mempool().map_or(0, |m| m.template_updates());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same hole as on #951, still on this branch. seen_updates is committed before spawn_blocking finishes. A template::build error is turned into Ok(()) by check_fees (and correctly does not call builds.note), but the next interval then sees an unchanged counter and skips. A transient build failure with no later tip event and no further mempool update never pushes that fee gain.

Store seen_updates only after this returns Ok. The new build timer can stay where it is: it already runs only on a successful template::build.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed with #951 (2b69d21) and rebased here as f484f2c. seen_updates is stored only after the build returns Ok. The build timer is unchanged and still notes only a template::build that succeeded.

average-gary and others added 4 commits October 8, 2026 10:50
MempoolHub::template_updates is a counter, not a notifier, so C1 polls
it from the session select loop once per template interval instead of
a watch task. Defaults and the >= comparison follow sv2-tp and Core's
waitNext; C3 is gone and C2 names the conf keys and nix options.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A session checks MempoolHub::template_updates once per template interval
(the counter has no notifier). When it moved, the session rebuilds and
sends NewTemplate{future_template: false} with no SetNewPrevHash if the
fees reach the last sent template's plus the fee delta, following Core's
waitNext comparison. A rebuild on a new prev hash is dropped for the tip
event to publish, a queued constraints rebuild is left to publish the
latest mempool itself, and a build that fails mid-reorg skips the check.
Defaults follow stratum-mining sv2-tp: 1000 sat, 5 s. run_sv2_tp refuses
an interval under 100 ms (one locked build per check) or over a day.

Fee pushes add same-tip templates while miners may still be on an older
job, so a session now keeps every template on the tip, up to 64, as
sv2-tp keeps every current-prev template. The bodies are the mempool's
shared Arcs, so the count costs pointers, not copies.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
--sv2-tp-fee-delta and --sv2-tp-template-interval (conf sv2_tp_fee_delta,
sv2_tp_template_interval, 1..=86400 s) feed the session's fee push; the
NixOS module gains sv2.tp.feeDelta and templateInterval. The operator doc
calls the interval a check period, says how it differs from sv2-tp, and
states the per-tip template retention; COMPAT lists the push. The journey
runs a 10_001 sat delta on a 1 s interval: a below-delta child stays
silent past a check, and the crossing grandchild lands before the 5 s
default's first check could.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Sv2TpHandle::stats exposes the listener's fee checks and template builds
(with build wall time) across its sessions. The DEBUG tip: perf line
gains sv2 checks= builds= build_avg_us= build_max_us=, and /metrics
exports the running totals. An idle session must show about one check
per interval and no rebuild, which pins two CPU-only regressions no
behavior test could see: a check that never re-arms spins the session,
and a check that ignores the unchanged mempool counter rebuilds every
interval.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@average-gary

Copy link
Copy Markdown
Contributor Author

Rebased onto the fixed #951 (now e93494a, on master 387305c). This branch is f484f2c.

Full cargo test --workspace in Linux Docker (rust:1.95) on f484f2c: 2751 passed, 0 failed.

🤖 Generated with Claude Code

@rearden-grok rearden-grok Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Summary

The prior seen_updates bug is fixed on f484f2cb. Session::build reads template_updates before spawn_blocking and assigns seen_updates only after that task returns Ok; a template::build error (or a join failure) leaves the generation in place, and check_fees logs the error and returns without ending the session, so the next interval retries. A successful build still advances the counter, including a below-delta build that is not sent, and an unchanged counter still skips the rebuild.

The metrics commit matches that split. fee_checks is noted once per poll, including the idle skip; builds is timed only after template::build returns inside the blocking region, not for an IBD hold and not for a failed build. /metrics reads the running totals and appears only when the listener attached Sv2TpStats; tip: perf takes the window. The idle-session test and the bootstrap scrape pin those counters. No regression of the merged fee-push behavior showed up on this head.

Issue counts by severity

  • bugs: 0
  • suggestions: 0
  • nits: 0

@reardencode
reardencode merged commit efd6d91 into reardencode:master Oct 8, 2026
18 checks passed
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.

2 participants