diff --git a/sv2/channels-sv2/src/server/extended.rs b/sv2/channels-sv2/src/server/extended.rs index dabe5a6787..7fd76759b7 100644 --- a/sv2/channels-sv2/src/server/extended.rs +++ b/sv2/channels-sv2/src/server/extended.rs @@ -671,6 +671,8 @@ where let is_stale_job = self.job_store.get_stale_job(job_id).is_some(); if is_stale_job { + self.share_accounting + .increment_rejected_shares(ERROR_CODE_SUBMIT_SHARES_STALE_SHARE); return Err(ShareValidationError::Stale( ERROR_CODE_SUBMIT_SHARES_STALE_SHARE, )); @@ -678,6 +680,8 @@ where // if job_id is not active, past or stale, return error if !is_active_job && !is_past_job && !is_stale_job { + self.share_accounting + .increment_rejected_shares(ERROR_CODE_SUBMIT_SHARES_INVALID_JOB_ID); return Err(ShareValidationError::InvalidJobId( ERROR_CODE_SUBMIT_SHARES_INVALID_JOB_ID, )); @@ -696,7 +700,6 @@ where .get_stale_job(job_id) .expect("stale job must exist") }; - let job_target = self .job_id_to_target .get(&job_id) @@ -704,6 +707,8 @@ where let extranonce_size = share.extranonce.inner_as_ref().len(); if extranonce_size != self.rollable_extranonce_size as usize { + self.share_accounting + .increment_rejected_shares(ERROR_CODE_SUBMIT_SHARES_BAD_EXTRANONCE_SIZE); return Err(ShareValidationError::BadExtranonceSize( ERROR_CODE_SUBMIT_SHARES_BAD_EXTRANONCE_SIZE, )); @@ -745,6 +750,8 @@ where // This is done by checking if the version & 0x1fffe000 == 0 // ref: https://github.com/bitcoin/bips/blob/master/bip-0320.mediawiki if (share.version & 0x1fffe000) != 0 { + self.share_accounting + .increment_rejected_shares(ERROR_CODE_VERSION_ROLLING_NOT_ALLOWED); return Err(ShareValidationError::VersionRollingNotAllowed( ERROR_CODE_VERSION_ROLLING_NOT_ALLOWED, )); @@ -786,6 +793,8 @@ where .share_accounting .is_share_seen(share_hash.to_raw_hash()) { + self.share_accounting + .increment_rejected_shares(ERROR_CODE_SUBMIT_SHARES_DUPLICATE_SHARE); return Err(ShareValidationError::DuplicateShare( ERROR_CODE_SUBMIT_SHARES_DUPLICATE_SHARE, )); @@ -828,6 +837,8 @@ where .share_accounting .is_share_seen(share_hash.to_raw_hash()) { + self.share_accounting + .increment_rejected_shares(ERROR_CODE_SUBMIT_SHARES_DUPLICATE_SHARE); return Err(ShareValidationError::DuplicateShare( ERROR_CODE_SUBMIT_SHARES_DUPLICATE_SHARE, )); @@ -844,6 +855,8 @@ where Ok(ShareValidationResult::Valid(share_hash.to_raw_hash())) } else { + self.share_accounting + .increment_rejected_shares(ERROR_CODE_SUBMIT_SHARES_DIFFICULTY_TOO_LOW); Err(ShareValidationError::DoesNotMeetTarget( ERROR_CODE_SUBMIT_SHARES_DIFFICULTY_TOO_LOW, )) @@ -868,7 +881,10 @@ mod tests { }; use binary_sv2::{Sv2Option, U256}; use bitcoin::{transaction::TxOut, Amount, ScriptBuf, Target}; - use mining_sv2::{NewExtendedMiningJob, SetCustomMiningJob, SubmitSharesExtended}; + use mining_sv2::{ + NewExtendedMiningJob, SetCustomMiningJob, SubmitSharesExtended, + ERROR_CODE_SUBMIT_SHARES_DIFFICULTY_TOO_LOW, + }; use std::convert::TryInto; use template_distribution_sv2::{NewTemplate, SetNewPrevHash}; @@ -1448,6 +1464,13 @@ mod tests { res.unwrap_err(), ShareValidationError::DoesNotMeetTarget(_) )); + assert_eq!( + channel + .get_share_accounting() + .get_rejected_shares() + .get(ERROR_CODE_SUBMIT_SHARES_DIFFICULTY_TOO_LOW), + Some(&1) + ); } #[test] diff --git a/sv2/channels-sv2/src/server/share_accounting.rs b/sv2/channels-sv2/src/server/share_accounting.rs index 65858adad1..1795d302b6 100644 --- a/sv2/channels-sv2/src/server/share_accounting.rs +++ b/sv2/channels-sv2/src/server/share_accounting.rs @@ -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. /// @@ -74,11 +75,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, share_work_sum: f64, last_batch_accepted: u32, last_batch_work_sum: f64, @@ -97,6 +99,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, @@ -108,6 +111,19 @@ impl ShareAccounting { } } + /// Increments rejected-share accounting for a share-validation `error_code`. + /// + /// Intended to be called by channel validation paths when returning a + /// [`ShareValidationError`] variant that carries an `error_code`. + /// Validation errors that do not map to `SubmitShares.Error` 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. @@ -169,6 +185,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 { + &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. @@ -223,3 +249,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) + ); + } +} diff --git a/sv2/channels-sv2/src/server/standard.rs b/sv2/channels-sv2/src/server/standard.rs index 0653dbeabc..0ae310d141 100644 --- a/sv2/channels-sv2/src/server/standard.rs +++ b/sv2/channels-sv2/src/server/standard.rs @@ -591,6 +591,8 @@ where let is_stale_job = self.job_store.get_stale_job(job_id).is_some(); if is_stale_job { + self.share_accounting + .increment_rejected_shares(ERROR_CODE_SUBMIT_SHARES_STALE_SHARE); return Err(ShareValidationError::Stale( ERROR_CODE_SUBMIT_SHARES_STALE_SHARE, )); @@ -598,6 +600,8 @@ where // if job_id is not active, past or stale, return error if !is_active_job && !is_past_job && !is_stale_job { + self.share_accounting + .increment_rejected_shares(ERROR_CODE_SUBMIT_SHARES_INVALID_JOB_ID); return Err(ShareValidationError::InvalidJobId( ERROR_CODE_SUBMIT_SHARES_INVALID_JOB_ID, )); @@ -670,6 +674,8 @@ where .share_accounting .is_share_seen(share_hash.to_raw_hash()) { + self.share_accounting + .increment_rejected_shares(ERROR_CODE_SUBMIT_SHARES_DUPLICATE_SHARE); return Err(ShareValidationError::DuplicateShare( ERROR_CODE_SUBMIT_SHARES_DUPLICATE_SHARE, )); @@ -723,6 +729,8 @@ where .share_accounting .is_share_seen(share_hash.to_raw_hash()) { + self.share_accounting + .increment_rejected_shares(ERROR_CODE_SUBMIT_SHARES_DUPLICATE_SHARE); return Err(ShareValidationError::DuplicateShare( ERROR_CODE_SUBMIT_SHARES_DUPLICATE_SHARE, )); @@ -739,6 +747,8 @@ where Ok(ShareValidationResult::Valid(share_hash.to_raw_hash())) } else { + self.share_accounting + .increment_rejected_shares(ERROR_CODE_SUBMIT_SHARES_DIFFICULTY_TOO_LOW); Err(ShareValidationError::DoesNotMeetTarget( ERROR_CODE_SUBMIT_SHARES_DIFFICULTY_TOO_LOW, )) @@ -763,7 +773,9 @@ mod tests { }; use binary_sv2::Sv2Option; use bitcoin::{transaction::TxOut, Amount, ScriptBuf, Target}; - use mining_sv2::{NewMiningJob, SubmitSharesStandard}; + use mining_sv2::{ + NewMiningJob, SubmitSharesStandard, ERROR_CODE_SUBMIT_SHARES_DIFFICULTY_TOO_LOW, + }; use std::convert::TryInto; use template_distribution_sv2::{NewTemplate, SetNewPrevHash as SetNewPrevHashTdp}; @@ -1230,6 +1242,13 @@ mod tests { res.unwrap_err(), ShareValidationError::DoesNotMeetTarget(_) )); + assert_eq!( + standard_channel + .get_share_accounting() + .get_rejected_shares() + .get(ERROR_CODE_SUBMIT_SHARES_DIFFICULTY_TOO_LOW), + Some(&1) + ); } #[test]