Follow-up to #2262 (client rejected_shares bound) and sibling of #2289 (job-storage retention).
The same unbounded-collection-under-attacker-controlled-messages pattern exists in the duplicate-share detection of both ShareAccounting implementations. seen_shares inserts every distinct accepted share hash and is only cleared on a chain-tip transition, which the peer controls the timing of (long-lived tip, or a stalled/withheld SetNewPrevHash). Memory grows linearly with accepted shares for the lifetime of a tip.
Unlike the job-storage caps in #2289, this is not a matter of picking a number and evicting past it — and the two ShareAccounting implementations do not want the same bound:
- On the server, evicting is not an option at all: forgetting a still-valid hash re-admits its duplicate, re-enabling the replay/accounting fraud the set exists to prevent. Overflow has to be an explicit failure, which in turn means the bound must be provably unreachable for any honestly-configured channel.
- On the client, eviction is harmless (it costs one double-counted local statistic), but the size is the hard constraint: the
client module is documented as no_std-compatible for embedded use, and an adversarial upstream can drive the cache to its bound at message speed — so the bound is memory the smallest supported device must have spare, not a theoretical ceiling.
So this needs a design decision, not just a bound: a bound whose size is derived from peer-supplied inputs is not a bound at all on the side where the peer is the threat.
1. Server seen_shares has no bound
|
pub fn update_share_accounting( |
|
&mut self, |
|
share_work: f64, |
|
share_sequence_number: u32, |
|
share_hash: Hash, |
|
) { |
|
self.last_share_sequence_number = share_sequence_number; |
|
self.shares_accepted = self.shares_accepted.saturating_add(1); |
|
self.share_work_sum += share_work; |
|
self.seen_shares.insert(share_hash); |
update_share_accounting inserts unconditionally; the only clear path is flush_seen_shares, called on chain-tip transitions:
|
self.share_accounting.flush_seen_shares(); |
|
self.share_accounting.flush_seen_shares(); |
|
self.share_accounting.flush_seen_shares(); |
Trust boundary: a malicious downstream miner. Server channel targets derive from downstream-supplied nominal hashrate and max target; a zero-hashrate channel at the maximum target makes essentially every unique header pass validate_share, so the set grows at message speed with no PoW cost. Affects standard and extended server channels (validate_share → is_share_seen / update_share_accounting in both).
2. Client seen_shares has the same defect
|
pub fn track_validated_share( |
|
&mut self, |
|
share_sequence_number: u32, |
|
share_hash: Hash, |
|
share_work: f64, |
|
) { |
|
self.last_share_sequence_number = share_sequence_number; |
|
self.validated_shares = self.validated_shares.saturating_add(1); |
|
self.validated_work_sum += share_work; |
|
self.seen_shares.insert(share_hash); |
|
} |
track_validated_share mirrors the server path; flushed only via flush_seen_shares:
|
self.share_accounting.flush_seen_shares(); |
|
self.share_accounting.flush_seen_shares(); |
|
self.share_accounting.flush_seen_shares(); |
Trust boundary: an adversarial upstream advertises an extremely easy channel target while holding one tip active; the attached share source then grows the validating client/proxy at message speed. Especially relevant for the documented no_std/embedded use case where memory is constrained.
fix (two bounds, one per trust boundary)
The two sides do not want the same bound. They differ in what overflow costs (fatal vs. harmless), in who controls the inputs a bound could be derived from (pool operator vs. peer), and in how much memory the host has (a pool server vs. a documented no_std/embedded client). Each of those points the same way: derive on the server, use a flat constant on the client.
1. Server — derived budget, hard limit.
The channel already stores expected_share_per_minute:
|
expected_share_per_minute: f32, |
budget = clamp(expected_shares_per_minute × WORST_CASE_TIP_MINUTES × MARGIN,
MIN_SEEN_SHARES_CAP, MAX_SEEN_SHARES_CAP)
WORST_CASE_TIP_MINUTES = 600 (10 h): seen_shares only needs to hold shares for the lifetime of one chain tip (it is flushed on every tip transition). 10 h is far beyond any realistic block interval, so the budget is unreachable by a well-behaved channel.MARGIN = 2: Poisson variance over thousands of expected shares is ~√N, so 2× is already generous; its real job is covering the window before vardiff converges.MIN_SEEN_SHARES_CAP = 4 096 / MAX_SEEN_SHARES_CAP = 2²⁰: the derived value is only as sane as the configured rate. Without the lower clamp, a near-zero configured rate yields a budget of 1 and closes a legitimate channel on its second accepted share; without the upper one, nothing caps a single channel's cache. Only reachable via absurd configuration, but free to bound.
Compute at construction. expected_share_per_minute is fixed for the channel's lifetime — update_channel recomputes the target from a new nominal hashrate precisely to preserve that expected rate — so the budget never needs refreshing.
When the budget is exhausted, validate_share returns a new ShareValidationError::SeenSharesBudgetExhausted — deliberately without an error_code string, so the existing "SHOULD lead to a client disconnection or application shutdown" documentation applies and the embedding application closes the channel. Evicting is not an option here: forgetting a still-valid hash re-admits its duplicate, re-enabling the replay/accounting fraud (double-counted shares are payout fraud) the set exists to prevent.
Deriving is worth it on this side precisely because the limit is load-bearing: it must stay unreachable for any honestly-configured rate, and honest rates span orders of magnitude. And the input is pool-configured, not peer-supplied, so the derivation cannot be steered by an attacker. Document the per-channel worst-case memory next to the constant (at 6 shares/min the budget is 7 200 hashes; at 100 shares/min, 120 000).
2. Client — flat constant bound, FIFO eviction.
/// Maximum number of accepted-share hashes a client channel retains for duplicate detection.
pub const MAX_SEEN_SHARES: usize = 4_096; // one 128 KB allocation
Held in a single VecDeque<Hash>, oldest evicted before each insert so the queue never exceeds the bound even transiently and its backing allocation settles at exactly MAX_SEEN_SHARES entries.
Why a constant sized for the smallest host, and not a derived cap. The client module is documented as no_std-compatible for embedded use, and an adversarial upstream advertising a trivial target can drive this cache to its bound at message speed — so the bound is memory the smallest supported device must have spare, not a theoretical ceiling. That rules out anything tip-sized: a cap derived from target and nominal hashrate (the approach this issue originally proposed) measured 137 MB peak RSS for a single channel at full occupancy, which is fatal on any MCU and unpleasant anywhere.
The derivation is also unsound on this side independently of its size. Its inputs are upstream-controlled, and a bound computed from attacker-supplied parameters cannot bound the attacker; it would need clamping into an absolute range regardless, at which point the clamp is the real bound and the derivation only picks where inside it eviction begins. Worse, the inverse relation (hash_rate_from_target) overflows for any target easier than ~2²⁴³ — including the maximum target — which is exactly the adversarial case, so the derivation degenerates precisely where it is needed.
Why 4 096 is enough. A client cache is not a tip-reconciliation structure. It exists to catch a share source re-submitting work it already sent — a retransmit or a buggy loop, arriving within seconds. 4 096 hashes covers ~11 h of history for a typical 6 shares/min channel and ~7 min for a very busy 600 shares/min proxy channel, both far beyond any realistic duplicate window.
Why one VecDeque and not a set plus a companion order queue. At this size a linear scan beats a hash lookup (random hashes diverge in the first byte, so contains early-exits immediately), and storing each hash once instead of twice halves the footprint — the quantity that actually matters on the constrained target.
Overflow evicts rather than fails because the consequence is local only: an evicted-then-replayed hash double-counts one statistic (validated_shares / validated_work_sum); nothing is paid out, and nothing is forwarded as newly-validated that the upstream will not independently dedup.
Both server constants, MAX_SEEN_SHARES, and the rationale above should live in rustdoc next to their definitions, and is_share_seen / flush_seen_shares docs updated to state each side's bound and overflow behavior.
notes
regression tests
- client: flood past
MAX_SEEN_SHARES with unique hashes and assert retention is bounded and oldest-first — the evicted hashes are the oldest, the resident ones the newest, and a chain-tip transition clears the queue. The audit PoCs' 4 097-unique-hash flood exercises exactly one eviction at this bound.
- server: fill to the budget and assert (a) every hash accepted along the way is still resident — nothing is evicted, so no duplicate can be re-admitted — and (b) the next
validate_share fails with SeenSharesBudgetExhausted rather than growing the cache. Note this replaces the audit PoC's "the full 10 000-share history stays resident" expectation: with a bounded budget the 10 000th share is refused, not retained.
PoCs available on:
Follow-up to #2262 (client
rejected_sharesbound) and sibling of #2289 (job-storage retention).The same unbounded-collection-under-attacker-controlled-messages pattern exists in the duplicate-share detection of both
ShareAccountingimplementations.seen_sharesinserts every distinct accepted share hash and is only cleared on a chain-tip transition, which the peer controls the timing of (long-lived tip, or a stalled/withheldSetNewPrevHash). Memory grows linearly with accepted shares for the lifetime of a tip.Unlike the job-storage caps in #2289, this is not a matter of picking a number and evicting past it — and the two
ShareAccountingimplementations do not want the same bound:clientmodule is documented asno_std-compatible for embedded use, and an adversarial upstream can drive the cache to its bound at message speed — so the bound is memory the smallest supported device must have spare, not a theoretical ceiling.So this needs a design decision, not just a bound: a bound whose size is derived from peer-supplied inputs is not a bound at all on the side where the peer is the threat.
1. Server
seen_shareshas no boundstratum/sv2/channels-sv2/src/server/share_accounting.rs
Lines 136 to 145 in 905cf73
update_share_accountinginserts unconditionally; the only clear path isflush_seen_shares, called on chain-tip transitions:stratum/sv2/channels-sv2/src/server/standard.rs
Line 584 in 905cf73
stratum/sv2/channels-sv2/src/server/extended.rs
Line 628 in 905cf73
stratum/sv2/channels-sv2/src/server/extended.rs
Line 678 in 905cf73
Trust boundary: a malicious downstream miner. Server channel targets derive from downstream-supplied nominal hashrate and max target; a zero-hashrate channel at the maximum target makes essentially every unique header pass
validate_share, so the set grows at message speed with no PoW cost. Affects standard and extended server channels (validate_share→is_share_seen/update_share_accountingin both).2. Client
seen_shareshas the same defectstratum/sv2/channels-sv2/src/client/share_accounting.rs
Lines 193 to 203 in 905cf73
track_validated_sharemirrors the server path; flushed only viaflush_seen_shares:stratum/sv2/channels-sv2/src/client/standard.rs
Line 364 in 905cf73
stratum/sv2/channels-sv2/src/client/extended.rs
Line 512 in 905cf73
stratum/sv2/channels-sv2/src/client/extended.rs
Line 562 in 905cf73
Trust boundary: an adversarial upstream advertises an extremely easy channel target while holding one tip active; the attached share source then grows the validating client/proxy at message speed. Especially relevant for the documented
no_std/embedded use case where memory is constrained.fix (two bounds, one per trust boundary)
The two sides do not want the same bound. They differ in what overflow costs (fatal vs. harmless), in who controls the inputs a bound could be derived from (pool operator vs. peer), and in how much memory the host has (a pool server vs. a documented
no_std/embedded client). Each of those points the same way: derive on the server, use a flat constant on the client.1. Server — derived budget, hard limit.
The channel already stores
expected_share_per_minute:stratum/sv2/channels-sv2/src/server/standard.rs
Line 99 in 905cf73
WORST_CASE_TIP_MINUTES = 600(10 h):seen_sharesonly needs to hold shares for the lifetime of one chain tip (it is flushed on every tip transition). 10 h is far beyond any realistic block interval, so the budget is unreachable by a well-behaved channel.MARGIN = 2: Poisson variance over thousands of expected shares is ~√N, so 2× is already generous; its real job is covering the window before vardiff converges.MIN_SEEN_SHARES_CAP = 4 096/MAX_SEEN_SHARES_CAP = 2²⁰: the derived value is only as sane as the configured rate. Without the lower clamp, a near-zero configured rate yields a budget of 1 and closes a legitimate channel on its second accepted share; without the upper one, nothing caps a single channel's cache. Only reachable via absurd configuration, but free to bound.Compute at construction.
expected_share_per_minuteis fixed for the channel's lifetime —update_channelrecomputes the target from a new nominal hashrate precisely to preserve that expected rate — so the budget never needs refreshing.When the budget is exhausted,
validate_sharereturns a newShareValidationError::SeenSharesBudgetExhausted— deliberately without anerror_codestring, so the existing "SHOULD lead to a client disconnection or application shutdown" documentation applies and the embedding application closes the channel. Evicting is not an option here: forgetting a still-valid hash re-admits its duplicate, re-enabling the replay/accounting fraud (double-counted shares are payout fraud) the set exists to prevent.Deriving is worth it on this side precisely because the limit is load-bearing: it must stay unreachable for any honestly-configured rate, and honest rates span orders of magnitude. And the input is pool-configured, not peer-supplied, so the derivation cannot be steered by an attacker. Document the per-channel worst-case memory next to the constant (at 6 shares/min the budget is 7 200 hashes; at 100 shares/min, 120 000).
2. Client — flat constant bound, FIFO eviction.
Held in a single
VecDeque<Hash>, oldest evicted before each insert so the queue never exceeds the bound even transiently and its backing allocation settles at exactlyMAX_SEEN_SHARESentries.Why a constant sized for the smallest host, and not a derived cap. The
clientmodule is documented asno_std-compatible for embedded use, and an adversarial upstream advertising a trivial target can drive this cache to its bound at message speed — so the bound is memory the smallest supported device must have spare, not a theoretical ceiling. That rules out anything tip-sized: a cap derived from target and nominal hashrate (the approach this issue originally proposed) measured 137 MB peak RSS for a single channel at full occupancy, which is fatal on any MCU and unpleasant anywhere.The derivation is also unsound on this side independently of its size. Its inputs are upstream-controlled, and a bound computed from attacker-supplied parameters cannot bound the attacker; it would need clamping into an absolute range regardless, at which point the clamp is the real bound and the derivation only picks where inside it eviction begins. Worse, the inverse relation (
hash_rate_from_target) overflows for any target easier than ~2²⁴³ — including the maximum target — which is exactly the adversarial case, so the derivation degenerates precisely where it is needed.Why 4 096 is enough. A client cache is not a tip-reconciliation structure. It exists to catch a share source re-submitting work it already sent — a retransmit or a buggy loop, arriving within seconds. 4 096 hashes covers ~11 h of history for a typical 6 shares/min channel and ~7 min for a very busy 600 shares/min proxy channel, both far beyond any realistic duplicate window.
Why one
VecDequeand not a set plus a companion order queue. At this size a linear scan beats a hash lookup (random hashes diverge in the first byte, socontainsearly-exits immediately), and storing each hash once instead of twice halves the footprint — the quantity that actually matters on the constrained target.Overflow evicts rather than fails because the consequence is local only: an evicted-then-replayed hash double-counts one statistic (
validated_shares/validated_work_sum); nothing is paid out, and nothing is forwarded as newly-validated that the upstream will not independently dedup.Both server constants,
MAX_SEEN_SHARES, and the rationale above should live in rustdoc next to their definitions, andis_share_seen/flush_seen_sharesdocs updated to state each side's bound and overflow behavior.notes
channels_sv2: need to bound job-storage retention (future templates, past jobs, replaced group jobs) #2289.regression tests
MAX_SEEN_SHARESwith unique hashes and assert retention is bounded and oldest-first — the evicted hashes are the oldest, the resident ones the newest, and a chain-tip transition clears the queue. The audit PoCs' 4 097-unique-hash flood exercises exactly one eviction at this bound.validate_sharefails withSeenSharesBudgetExhaustedrather than growing the cache. Note this replaces the audit PoC's "the full 10 000-share history stays resident" expectation: with a bounded budget the 10 000th share is refused, not retained.PoCs available on: