Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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 Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion sv2/channels-sv2/Cargo.toml
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "channels_sv2"
version = "5.0.0"
version = "5.1.0"
authors = ["The Stratum V2 Developers"]
edition = "2021"
readme = "README.md"
Expand Down
5 changes: 5 additions & 0 deletions sv2/channels-sv2/src/server/extended.rs
Original file line number Diff line number Diff line change
Expand Up @@ -435,6 +435,11 @@ where
&self.share_accounting
}

/// 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

self.share_accounting.increment_rejected_shares(error_code);
}

/// Updates the channel state with a new template.
///
/// If the template is a future template, the chain tip is not used.
Expand Down
64 changes: 60 additions & 4 deletions sv2/channels-sv2/src/server/share_accounting.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,16 @@
//! success, batch acknowledgment, and block discovery.
//! - **Share Validation Error**: Enumerates possible failure reasons when validating a share.
//! - **Share Accounting**: Tracks per-channel share statistics, acknowledges batches, detects
//! duplicate shares, and maintains best difficulty found.
//! duplicate shares, tracks rejected shares, and maintains best difficulty found.
//!
//! ## Usage
//!
//! Intended for use within mining server implementations that process SV2 share submissions and
//! issue `SubmitShares.Success` messages. Not intended for use by mining clients.
//! issue `SubmitShares.Success` or `SubmitShares.Error` messages. Not intended for use by mining
//! clients.

use bitcoin::hashes::sha256d::Hash;
use std::collections::HashSet;
use std::collections::{HashMap, HashSet};

/// The outcome of share validation, from the perspective of a Mining Server.
///
Expand Down Expand Up @@ -68,11 +69,12 @@ pub enum ShareValidationError {
/// Standard).
///
/// This struct manages per-channel share statistics, batch acknowledgment, duplicate detection,
/// and difficulty tracking. Only meant for usage on Mining Servers.
/// rejected-share accounting, and difficulty tracking. Only meant for usage on Mining Servers.
#[derive(Clone, Debug)]
pub struct ShareAccounting {
last_share_sequence_number: u32,
shares_accepted: u32,
rejected_shares: HashMap<String, u32>,
share_work_sum: f64,
last_batch_accepted: u32,
last_batch_work_sum: f64,
Expand All @@ -91,6 +93,7 @@ impl ShareAccounting {
Self {
last_share_sequence_number: 0,
shares_accepted: 0,
rejected_shares: HashMap::new(),
share_work_sum: 0.0,
last_batch_accepted: 0,
last_batch_work_sum: 0.0,
Expand All @@ -102,6 +105,19 @@ impl ShareAccounting {
}
}

/// Increments rejected-share accounting for an emitted `SubmitShares.Error`.
///
/// This should only be called by the application layer after it decides to send a
/// `SubmitShares.Error` downstream. Validation errors that are handled internally and do not
/// produce `SubmitShares.Error` messages should not be counted here.
pub fn increment_rejected_shares(&mut self, error_code: &str) {
if let Some(count) = self.rejected_shares.get_mut(error_code) {
*count += 1;
} else {
self.rejected_shares.insert(error_code.to_string(), 1);
}
}

/// Updates internal accounting for a newly accepted share.
///
/// - Increments total shares accepted and work sum.
Expand Down Expand Up @@ -160,6 +176,16 @@ impl ShareAccounting {
self.shares_accepted
}

/// Returns a reference to the map of rejected shares by error code.
pub fn get_rejected_shares(&self) -> &HashMap<String, u32> {
&self.rejected_shares
}

/// Returns the total number of rejected shares on this channel.
pub fn get_rejected_shares_total(&self) -> u32 {
self.rejected_shares.values().copied().sum()
}

/// Returns the sum of work contributed by all accepted shares.
///
/// Note: this is not what we use for `SubmitShares.Success` messages.
Expand Down Expand Up @@ -214,3 +240,33 @@ impl ShareAccounting {
self.blocks_found
}
}

#[cfg(test)]
mod tests {
use super::ShareAccounting;

#[test]
fn rejected_shares_are_tracked_by_error_code() {
let mut accounting = ShareAccounting::new(10);

accounting.increment_rejected_shares("difficulty-too-low");
accounting.increment_rejected_shares("duplicate-share");
accounting.increment_rejected_shares("difficulty-too-low");

assert_eq!(accounting.get_rejected_shares_total(), 3);
assert_eq!(
accounting
.get_rejected_shares()
.get("difficulty-too-low")
.copied(),
Some(2)
);
assert_eq!(
accounting
.get_rejected_shares()
.get("duplicate-share")
.copied(),
Some(1)
);
}
}
5 changes: 5 additions & 0 deletions sv2/channels-sv2/src/server/standard.rs
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,11 @@ where
&self.share_accounting
}

/// 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?

self.share_accounting.increment_rejected_shares(error_code);
}

/// Updates the channel state with a new job.
///
/// If the template is a future template, the chain tip is not used.
Expand Down
Loading