Skip to content

improve share validation#1966

Merged
plebhash merged 3 commits into
stratum-mining:mainfrom
plebhash:2025-10-24-improve-share-validation
Oct 24, 2025
Merged

improve share validation#1966
plebhash merged 3 commits into
stratum-mining:mainfrom
plebhash:2025-10-24-improve-share-validation

Conversation

@plebhash
Copy link
Copy Markdown
Member

@Shourya742 raised some concerns about the direction #1902 is going #1902 (comment)

a proposed alternative is to do all the persistence at the application layer, which is currently missing some information

this PR improves ShareValidationResult so that the application layer has access to the share hash

every other information relevant for some kind of persistence on the application layer can be retrieved from:

  • the current channel state (e.g.: share_work is simply the current channel's Target, or user_identity can be retrieved via get_user_identity)
  • the context where we're calling validate_share() (e.g.: channel_id and sequence_number is already available on the share message itself)
  • the channel's internal ShareAccounting, which can be retrieved via get_share_accounting()

this also avoids some redundancies on the proposed solution of #1965

@plebhash plebhash force-pushed the 2025-10-24-improve-share-validation branch 2 times, most recently from 93e2475 to fb71e65 Compare October 24, 2025 15:26
info!("SubmitSharesStandard: {} ✅", success);
messages.push((downstream_id, Mining::SubmitSharesSuccess(success)).into());
} else {
let share_work = standard_channel.get_target().difficulty_float();
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.

wouldn't share_work be something related to the share and not the channel difficulty?

Copy link
Copy Markdown
Member Author

@plebhash plebhash Oct 24, 2025

Choose a reason for hiding this comment

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

a share should be rewarded because it meets the difficulty target established by the server, if the hash has more zeros than necessary, that doesn't make that share more valuable

that's the rationale we have always followed to update ShareAccounting::share_work_sum, which I got from an interaction with Braiins a while ago

btw, that's also the same rationale you're following on #1965, the only difference is that it's being returned inside ShareValidationResult::Valid, which as I pointed out is unnecessary, since that information is already available via get_target

https://github.com/stratum-mining/stratum/pull/1965/files#r2461063936

Copy link
Copy Markdown
Member Author

@plebhash plebhash Oct 24, 2025

Choose a reason for hiding this comment

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

please see stratum-mining/sv2-spec#126

for some reason the rendering of images on my fork is messed up, but I think this one summarizes things:

image

@plebhash plebhash force-pushed the 2025-10-24-improve-share-validation branch 2 times, most recently from 44bf12e to 130a3b3 Compare October 24, 2025 15:35
the application layer will handling ShareAccounting, so we don't need to inform it whether it's time to acknowledge shares

we also start returning the share hash after validation
@plebhash plebhash force-pushed the 2025-10-24-improve-share-validation branch from 130a3b3 to 0f73fa8 Compare October 24, 2025 15:43
This was referenced Oct 24, 2025
@plebhash plebhash merged commit 561cc2c into stratum-mining:main Oct 24, 2025
11 checks passed
@plebhash plebhash deleted the 2025-10-24-improve-share-validation branch October 24, 2025 17:18
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.

3 participants