Skip to content

client share accounting refinement#328

Merged
plebhash merged 4 commits intostratum-mining:mainfrom
plebhash:2026-03-12-client-share-accounting-refinement
Mar 19, 2026
Merged

client share accounting refinement#328
plebhash merged 4 commits intostratum-mining:mainfrom
plebhash:2026-03-12-client-share-accounting-refinement

Conversation

@plebhash
Copy link
Member

@plebhash plebhash commented Mar 12, 2026

@plebhash plebhash changed the title 2026 03 12 client share accounting refinement client share accounting refinement Mar 12, 2026
@plebhash plebhash force-pushed the 2026-03-12-client-share-accounting-refinement branch 2 times, most recently from 2909abb to 646cda9 Compare March 17, 2026 19:27
@plebhash plebhash marked this pull request as ready for review March 17, 2026 19:29
@plebhash plebhash force-pushed the 2026-03-12-client-share-accounting-refinement branch from 646cda9 to b943289 Compare March 18, 2026 01:34
@plebhash plebhash force-pushed the 2026-03-12-client-share-accounting-refinement branch 3 times, most recently from 39a3844 to 5c47e05 Compare March 18, 2026 17:44
@gimballock
Copy link

For when the monitoring/metrics commits come back (via #2119): the earlier rename of validated_shares to submitted_shares in the app layer collides with the existing shares_submitted field on ServerExtendedChannelInfo (which counts shares sent to upstream — a different concept). Recommend keeping validated_shares consistently across both layers to avoid confusion in dashboards and PromQL queries.

@plebhash plebhash force-pushed the 2026-03-12-client-share-accounting-refinement branch from 5c47e05 to 00c7741 Compare March 18, 2026 17:56
@gimballock
Copy link

Note for when the metrics rename commits come back: snapshot_cache.rs::update_metrics() also references sv2_server_shares_accepted_total / sv2_client_shares_accepted_total and has stale-label-removal logic for those metric names. Will need updating alongside prometheus_metrics.rs and http_server.rs. Happy to help with that integration.

@gimballock
Copy link

Once all three counters land in Prometheus, inflight shares (submitted but not yet resolved) can be monitored via the invariant: inflight = validated - acknowledged - rejected. Divergence from >= 0 would likely indicate some kind of issue.

@gimballock
Copy link

Beyond passing ci checks this LGTM

@plebhash
Copy link
Member Author

@gimballock thanks

I assume all the notes above are also covered by stratum-mining/stratum#2119 (comment)?

if not, I'd encourage you to move any missing info to that issue, because taking those notes here means they're much likely to be forgotten once the PR is merged

@gimballock
Copy link

Good call — I've already posted the server-side refinement guidance on stratum-mining/stratum#2119. Let me move the remaining notes there too:

  • the submitted_shares / validated_shares naming concern
  • the snapshot_cache.rs update needed when the metrics rename lands

@plebhash plebhash force-pushed the 2026-03-12-client-share-accounting-refinement branch 2 times, most recently from d9222c4 to 6474310 Compare March 19, 2026 12:19
@plebhash plebhash force-pushed the 2026-03-12-client-share-accounting-refinement branch 3 times, most recently from 9f88bb1 to df9a0ef Compare March 19, 2026 12:40
Copy link
Collaborator

@Shourya742 Shourya742 left a comment

Choose a reason for hiding this comment

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

ACK

@plebhash plebhash force-pushed the 2026-03-12-client-share-accounting-refinement branch from df9a0ef to 7c13f50 Compare March 19, 2026 13:19
@plebhash plebhash merged commit 0ba8328 into stratum-mining:main Mar 19, 2026
12 checks passed
@plebhash plebhash deleted the 2026-03-12-client-share-accounting-refinement branch March 19, 2026 13:54
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.

4 participants