Skip to content

channels_sv2::server::share_accounting keeps track of rejected shares#2149

Draft
plebhash wants to merge 1 commit into
stratum-mining:mainfrom
plebhash:2026-05-08-refine-server-share-accounting
Draft

channels_sv2::server::share_accounting keeps track of rejected shares#2149
plebhash wants to merge 1 commit into
stratum-mining:mainfrom
plebhash:2026-05-08-refine-server-share-accounting

Conversation

@plebhash
Copy link
Copy Markdown
Member

@plebhash plebhash commented May 8, 2026

Comment thread sv2/channels-sv2/src/server/extended.rs Outdated
@plebhash plebhash force-pushed the 2026-05-08-refine-server-share-accounting branch from 533ca1e to 47163fb Compare May 11, 2026 22:00
Copy link
Copy Markdown
Member

@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

}

/// Updates rejected-share accounting for an emitted `SubmitShares.Error`.
pub fn increment_rejected_shares(&mut self, error_code: &str) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Shouldn't this be called internally by validate_share(), in case of an error?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

yeah I think this would be a more concise approach, since we would avoid the need for the app layer to explicitly call this

(or more importantly, we would eliminate the possibility of the app layer forgetting to call this while it should)

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

but while trying to implement this approach, I actually started to think that it might make sense to tackle #2142 first

that way, validate_share could return ShareValidationError variants that already carry the needed error_code embedded into itself

the app layer would then be able to use those error_code without having to think too much about it

Copy link
Copy Markdown
Member Author

@plebhash plebhash May 12, 2026

Choose a reason for hiding this comment

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

btw I don't think we need to wait for stratum-mining/sv2-spec#194 before we tackle #2142

as is, SRI is already diverging from the spec sentence discussed in stratum-mining/sv2-spec#165, so the doing #2142 will not change anything with regards to that

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

reverting this to draft while I work on #2142

}

/// Updates rejected-share accounting for an emitted `SubmitShares.Error`.
pub fn increment_rejected_shares(&mut self, error_code: &str) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Shouldn't this be called internally by validate_share(), in case of an error?

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.

channels_sv2::server::share_accounting::ShareAccounting needs refinement

4 participants