Skip to content

defer channels_sv2::client share acceptance to upstream acknowledgement #2090

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
GitGab19 merged 3 commits into from
Feb 19, 2026
Merged

defer channels_sv2::client share acceptance to upstream acknowledgement #2090

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
6356ee3
channels-sv2: defer client share acceptance to upstream acknowledgement
plebhash Feb 18, 2026
89e5841
bump channels_sv2 to 4.0.0
plebhash Feb 18, 2026
bebe82f
bump stratum-translation to 0.1.3
plebhash Feb 19, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion stratum-core/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ framing_sv2 = { path = "../sv2/framing-sv2", version = "^6.0.0" }
noise_sv2 = { path = "../sv2/noise-sv2", version = "^1.0.0" }
parsers_sv2 = { path = "../sv2/parsers-sv2", version = "^0.2.0" }
handlers_sv2 = { path = "../sv2/handlers-sv2", version = "^0.2.0" }
channels_sv2 = { path = "../sv2/channels-sv2", version = "^3.0.0" }
channels_sv2 = { path = "../sv2/channels-sv2", version = "^4.0.0" }
common_messages_sv2 = { path = "../sv2/subprotocols/common-messages", version = "^7.0.0" }
mining_sv2 = { path = "../sv2/subprotocols/mining", version = "^7.0.0" }
template_distribution_sv2 = { path = "../sv2/subprotocols/template-distribution", version = "^5.0.0" }
Expand Down
4 changes: 2 additions & 2 deletions stratum-core/stratum-translation/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "stratum_translation"
version = "0.1.2"
version = "0.1.3"
edition = "2021"
license = "MIT OR Apache-2.0"
description = "Stratum V1 ↔ Stratum V2 translation utilities for reuse across proxies, apps, and firmware"
Expand All @@ -12,7 +12,7 @@ path = "src/lib.rs"
[dependencies]
binary_sv2 = { path = "../../sv2/binary-sv2", version = "^5.0.0" }
mining_sv2 = { path = "../../sv2/subprotocols/mining", version = "^7.0.0" }
channels_sv2 = { path = "../../sv2/channels-sv2", version = "^3.0.0" }
channels_sv2 = { path = "../../sv2/channels-sv2", version = "^4.0.0" }
v1 = { path = "../../sv1", package = "sv1_api", version = "^2.0.0" }
tracing = { workspace = true }
bitcoin = { workspace = true }
2 changes: 1 addition & 1 deletion sv2/channels-sv2/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "channels_sv2"
version = "3.0.1"
version = "4.0.0"
authors = ["The Stratum V2 Developers"]
edition = "2021"
readme = "README.md"
Expand Down
25 changes: 15 additions & 10 deletions sv2/channels-sv2/src/client/extended.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,17 @@ impl<'a> ExtendedChannel<'a> {
&self.share_accounting
}

/// Updates share accounting based on a [`SubmitSharesSuccess`] message from the
/// upstream server. Delegates to [`ShareAccounting::on_share_acknowledgement`].
pub fn on_share_acknowledgement(
&mut self,
new_submits_accepted_count: u32,
new_shares_sum: f64,
Copy link
Copy Markdown
Member

@Shourya742 Shourya742 Feb 19, 2026

Choose a reason for hiding this comment

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

Yeah, we can make new_share_sum a u64, based on the comments in the sv2-apps PR: stratum-mining/sv2-apps#283 (comment) . There’s no real need to keep it as an f64, it looks like an inaccuracy that slipped through during the initial iteration.

Copy link
Copy Markdown
Member

@GitGab19 GitGab19 Feb 19, 2026

Choose a reason for hiding this comment

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

IIRC, the new_shares_sum represent the amount of work acknowledged by the server in a specific SubmitSharesSuccess, that's why it's a f64.

But I might be wrong here.

Copy link
Copy Markdown
Member Author

@plebhash plebhash Feb 19, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

I remember seeing work being reported as floating point on Braiins UI, and also contemplating the fact that strictly speaking, work is a unit of difficulty, and therefore it can (and maybe should) be tracked as floating point numbers (like rust bitcoin does)

this is specially true when we're operating at diff ranges 0 < diff < 1, where we have no relevant data unless we're working with floating point numbers

but as @Shourya742 correctly points out, this was kind of a failed attempt at retaining granular data, because SubmitSharesSuccess.new_shares_sum is U64, which forces everyone to truncate and makes an internal representation based on f64 pretty much useless

FWIW there's some extra context on stratum-mining/sv2-spec#126

with that said, I'd like to avoid expanding the scope of this PR to also cover for this, because that's not the problem #2089 we're trying to solve

created #2092 to keep track of it

Copy link
Copy Markdown
Member

@Shourya742 Shourya742 Feb 19, 2026

Choose a reason for hiding this comment

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

Acc to spec, its the sum of share acknowledged in the batch,

Copy link
Copy Markdown
Member

@Shourya742 Shourya742 Feb 19, 2026

Choose a reason for hiding this comment

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

I was not aware of that PR. Thanks for mentioning that.

Copy link
Copy Markdown
Member Author

@plebhash plebhash Feb 19, 2026

Choose a reason for hiding this comment

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

Acc to spec, its the sum of share acknowledged in the batch,

no, you're confusing it with new_submits_accepted_count

please see stratum-mining/sv2-spec#126, I drafted this while interacting with Braiins

) {
self.share_accounting
.on_share_acknowledgement(new_submits_accepted_count, new_shares_sum);
}

/// Handles a [`NewExtendedMiningJob`] message received from upstream.
///
/// The message could be either directed at this channel, or at a group channel it belongs to.
Expand Down Expand Up @@ -555,11 +566,8 @@ impl<'a> ExtendedChannel<'a> {

// check if a block was found
if network_target.is_met_by(share_hash) {
self.share_accounting.update_share_accounting(
job_target.difficulty_float(),
share.sequence_number,
share_hash.to_raw_hash(),
);
self.share_accounting
.track_validated_share(share.sequence_number, share_hash.to_raw_hash());
return Ok(ShareValidationResult::BlockFound(share_hash.to_raw_hash()));
}

Expand All @@ -572,11 +580,8 @@ impl<'a> ExtendedChannel<'a> {
return Err(ShareValidationError::DuplicateShare);
}

self.share_accounting.update_share_accounting(
job_target.difficulty_float(),
share.sequence_number,
share_hash.to_raw_hash(),
);
self.share_accounting
.track_validated_share(share.sequence_number, share_hash.to_raw_hash());

// update the best diff
self.share_accounting.update_best_diff(share_hash_as_diff);
Expand Down
50 changes: 34 additions & 16 deletions sv2/channels-sv2/src/client/share_accounting.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,15 +40,23 @@ pub enum ShareValidationError {
BadExtranonceSize,
}

/// Tracks share validation state for a specific channel (Extended or Standard).
/// Tracks share validation and acceptance state for a specific channel (Extended or Standard).
///
/// Used only on Mining Clients.
/// Keeps statistics and state for shares submitted through the channel:
/// Used only on Mining Clients. Share accounting is split into two phases:
///
/// **Validation phase** (updated by [`validate_share`] via [`track_validated_share`]):
/// - hashes of seen shares (for duplicate detection)
/// - last received share's sequence number
/// - total accepted shares
/// - highest difficulty seen in validated shares
///
/// **Acceptance phase** (updated by the application layer via [`on_share_acknowledgement`]):
/// - total accepted shares (confirmed by upstream [`SubmitSharesSuccess`])
/// - cumulative work from accepted shares
/// - hashes of seen shares (for duplicate detection)
/// - highest difficulty seen in accepted shares
///
/// [`validate_share`]: super::extended::ExtendedChannel::validate_share
/// [`track_validated_share`]: ShareAccounting::track_validated_share
/// [`on_share_acknowledgement`]: ShareAccounting::on_share_acknowledgement
/// [`SubmitSharesSuccess`]: mining_sv2::SubmitSharesSuccess
#[derive(Clone, Debug)]
pub struct ShareAccounting {
last_share_sequence_number: u32,
Expand Down Expand Up @@ -76,20 +84,30 @@ impl ShareAccounting {
}
}

/// Updates the accounting state with a newly accepted share.
/// Updates acceptance accounting based on a [`SubmitSharesSuccess`] message from the
/// upstream server.
///
/// - Increments share count and total work.
/// - Updates last share sequence number.
/// - Records share hash to detect duplicates.
pub fn update_share_accounting(
/// This should be called by the application layer when it receives upstream confirmation
/// that shares were accepted. It is intentionally **not** called from [`validate_share`] —
/// local validation only tracks the share for duplicate detection (via
/// [`track_validated_share`]).
pub fn on_share_acknowledgement(
&mut self,
share_work: f64,
share_sequence_number: u32,
share_hash: Hash,
new_submits_accepted_count: u32,
new_shares_sum: f64,
) {
self.shares_accepted += new_submits_accepted_count;
self.share_work_sum += new_shares_sum;
}

/// Records a share that passed local validation.
///
/// Adds the hash to the seen set for duplicate detection and updates the last sequence
/// number. Called from [`validate_share`] — does **not** count the share as accepted.
/// Acceptance accounting is deferred to [`on_share_acknowledgement`], which should be
/// called when the upstream server confirms via [`SubmitSharesSuccess`].
pub fn track_validated_share(&mut self, share_sequence_number: u32, share_hash: Hash) {
self.last_share_sequence_number = share_sequence_number;
self.shares_accepted += 1;
self.share_work_sum += share_work;
self.seen_shares.insert(share_hash);
}

Expand Down
25 changes: 15 additions & 10 deletions sv2/channels-sv2/src/client/standard.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,17 @@ impl<'a> StandardChannel<'a> {
&self.share_accounting
}

/// Updates share accounting based on a [`SubmitSharesSuccess`] message from the
/// upstream server. Delegates to [`ShareAccounting::on_share_acknowledgement`].
pub fn on_share_acknowledgement(
&mut self,
new_submits_accepted_count: u32,
new_shares_sum: f64,
) {
self.share_accounting
.on_share_acknowledgement(new_submits_accepted_count, new_shares_sum);
}

/// Handles a new group channel job by converting it into a standard job
/// and activating it in this channel's context.
///
Expand Down Expand Up @@ -336,11 +347,8 @@ impl<'a> StandardChannel<'a> {

// check if a block was found
if network_target.is_met_by(share_hash) {
self.share_accounting.update_share_accounting(
job_target.difficulty_float(),
share.sequence_number,
share_hash.to_raw_hash(),
);
self.share_accounting
.track_validated_share(share.sequence_number, share_hash.to_raw_hash());
return Ok(ShareValidationResult::BlockFound(share_hash.to_raw_hash()));
}

Expand All @@ -353,11 +361,8 @@ impl<'a> StandardChannel<'a> {
return Err(ShareValidationError::DuplicateShare);
}

self.share_accounting.update_share_accounting(
job_target.difficulty_float(),
share.sequence_number,
share_hash.to_raw_hash(),
);
self.share_accounting
.track_validated_share(share.sequence_number, share_hash.to_raw_hash());

// update the best diff
self.share_accounting.update_best_diff(share_hash_as_diff);
Expand Down
Loading