`channels_sv2`: fix coinbase `scriptSig` size/serialization defects that yield consensus-invalid coinbases by plebhash · Pull Request #2243 · stratum-mining/stratum

plebhash · 2026-08-02T02:43:32Z

bit-aloo

One suggestion, and rest looks ok

bit-aloo · 2026-08-03T09:10:23Z

+        // standard channels have no rollable extranonce, so the prefix is the full extranonce
+        if job_factory.script_sig_size(MAX_COINBASE_PREFIX_SIZE, extranonce_prefix.len())
+            > MAX_SCRIPT_SIG_SIZE


This check we can move inside the job_factory and call with extranonce_prefix size? with methods like these:

impl JobFactory { pub fn check_script_sig_budget(&self, coinbase_prefix_size: usize, full_extranonce_size: usize) -> Result<(), JobFactoryError>; pub fn check_worst_case_script_sig_budget(&self, full_extranonce_size: usize) -> Result<(), JobFactoryError>; }

thanks for the suggestion, incorporating in a slightly different (more concise) way

adding pub fn fits_script_sig_budget(&self, full_extranonce_size: usize) -> bool to JobFactory

the following diff is being rebased into commit history:

diff --git a/sv2/channels-sv2/src/server/extended.rs b/sv2/channels-sv2/src/server/extended.rs index 75ed7f19..972fbe10 100644 --- a/sv2/channels-sv2/src/server/extended.rs +++ b/sv2/channels-sv2/src/server/extended.rs @@ -45,12 +45,7 @@ use crate::{ merkle_root::merkle_root_from_path, server::{ error::ExtendedChannelError, - jobs::{ - extended::ExtendedJob, - factory::{JobFactory, MAX_COINBASE_PREFIX_SIZE, MAX_SCRIPT_SIG_SIZE}, - job_store::JobStore, - JobOrigin, - }, + jobs::{extended::ExtendedJob, factory::JobFactory, job_store::JobStore, JobOrigin}, share_accounting::{ShareAccounting, ShareValidationError, ShareValidationResult}, }, target::{bytes_to_hex, hash_rate_to_target, u256_to_block_hash}, @@ -123,8 +118,8 @@ impl ExtendedChannel { /// and `//` delimiters: `/pool_tag_string//` /// /// Returns [`ExtendedChannelError::ScriptSigSizeTooLarge`] if the tags, the delimiters, the - /// full extranonce and a worst-case [`MAX_COINBASE_PREFIX_SIZE`] coinbase prefix do not fit - /// within [`MAX_SCRIPT_SIG_SIZE`]. + /// full extranonce and a worst-case coinbase prefix do not fit within the coinbase `scriptSig` + /// budget, see [`JobFactory::fits_script_sig_budget`]. #[allow(clippy::too_many_arguments)] pub fn new_for_pool( channel_id: u32, @@ -165,8 +160,8 @@ impl ExtendedChannel { /// `/` delimiters: `/pool_tag_string/miner_tag_string/` /// /// Returns [`ExtendedChannelError::ScriptSigSizeTooLarge`] if the tags, the delimiters, the - /// full extranonce and a worst-case [`MAX_COINBASE_PREFIX_SIZE`] coinbase prefix do not fit - /// within [`MAX_SCRIPT_SIG_SIZE`]. + /// full extranonce and a worst-case coinbase prefix do not fit within the coinbase `scriptSig` + /// budget, see [`JobFactory::fits_script_sig_budget`]. #[allow(clippy::too_many_arguments)] pub fn new_for_job_declaration_client( channel_id: u32, @@ -234,10 +229,8 @@ impl ExtendedChannel { // conservative check against the spec's worst-case `NewTemplate::coinbase_prefix`. // the exact size is re-checked against each actual template in `JobFactory::coinbase` - if job_factory.script_sig_size( - MAX_COINBASE_PREFIX_SIZE, - extranonce_prefix.len() + rollable_extranonce_size as usize, - ) > MAX_SCRIPT_SIG_SIZE + if !job_factory + .fits_script_sig_budget(extranonce_prefix.len() + rollable_extranonce_size as usize) { return Err(ExtendedChannelError::ScriptSigSizeTooLarge); } @@ -298,7 +291,8 @@ impl ExtendedChannel { /// and share validation will be performed accordingly. /// /// Returns an error if the new extranonce prefix is too large, or if it would push the - /// assembled coinbase `scriptSig` past [`MAX_SCRIPT_SIG_SIZE`]. The channel is left unchanged + /// assembled coinbase `scriptSig` past its budget (see + /// [`JobFactory::fits_script_sig_budget`]). The channel is left unchanged /// in both error cases. pub fn set_extranonce_prefix( &mut self, @@ -310,11 +304,9 @@ impl ExtendedChannel { // re-run the constructor's invariant: a prefix that is individually valid can still push // the assembled scriptSig past the consensus cap - if self.job_factory.script_sig_size( - MAX_COINBASE_PREFIX_SIZE, + if !self.job_factory.fits_script_sig_budget( extranonce_prefix.len() + self.rollable_extranonce_size as usize, - ) > MAX_SCRIPT_SIG_SIZE - { + ) { return Err(ExtendedChannelError::ScriptSigSizeTooLarge); } @@ -481,8 +473,9 @@ impl ExtendedChannel { /// Returns [`ExtendedChannelError::JobFactoryError`] wrapping /// [`JobFactoryError::ScriptSigSizeTooLarge`](crate::server::jobs::error::JobFactoryError::ScriptSigSizeTooLarge) /// if the template's `coinbase_prefix` pushes the assembled coinbase `scriptSig` past - /// [`MAX_SCRIPT_SIG_SIZE`]. The constructor can only check against the spec's worst-case - /// [`MAX_COINBASE_PREFIX_SIZE`], so this is where an out-of-spec Template Provider is caught. + /// its budget. The constructor can only check against the spec's worst-case prefix (see + /// [`JobFactory::fits_script_sig_budget`]), so this is where an out-of-spec Template Provider + /// is caught. pub fn on_new_template( &mut self, template: NewTemplateOwned, diff --git a/sv2/channels-sv2/src/server/group.rs b/sv2/channels-sv2/src/server/group.rs index 27217d84..d6ed3423 100644 --- a/sv2/channels-sv2/src/server/group.rs +++ b/sv2/channels-sv2/src/server/group.rs @@ -34,11 +34,7 @@ use crate::{ chain_tip::ChainTip, server::{ error::GroupChannelError, - jobs::{ - extended::ExtendedJob, - factory::{JobFactory, MAX_COINBASE_PREFIX_SIZE, MAX_SCRIPT_SIG_SIZE}, - job_store::JobStore, - }, + jobs::{extended::ExtendedJob, factory::JobFactory, job_store::JobStore}, }, }; use bitcoin::transaction::TxOut; @@ -82,8 +78,8 @@ impl GroupChannel { /// and `//` delimiters: `/pool_tag_string//` /// /// Returns [`GroupChannelError::ScriptSigSizeTooLarge`] if the tags, the delimiters, the - /// extranonce and a worst-case [`MAX_COINBASE_PREFIX_SIZE`] coinbase prefix do not fit within - /// [`MAX_SCRIPT_SIG_SIZE`]. + /// extranonce and a worst-case coinbase prefix do not fit within the coinbase `scriptSig` + /// budget, see [`JobFactory::fits_script_sig_budget`]. pub fn new_for_pool( group_channel_id: u32, full_extranonce_size: usize, @@ -110,8 +106,8 @@ impl GroupChannel { /// `/` delimiters: `/pool_tag_string/miner_tag_string/` /// /// Returns [`GroupChannelError::ScriptSigSizeTooLarge`] if the tags, the delimiters, the - /// extranonce and a worst-case [`MAX_COINBASE_PREFIX_SIZE`] coinbase prefix do not fit within - /// [`MAX_SCRIPT_SIG_SIZE`]. + /// extranonce and a worst-case coinbase prefix do not fit within the coinbase `scriptSig` + /// budget, see [`JobFactory::fits_script_sig_budget`]. pub fn new_for_job_declaration_client( group_channel_id: u32, full_extranonce_size: usize, @@ -138,9 +134,7 @@ impl GroupChannel { // conservative check against the spec's worst-case `NewTemplate::coinbase_prefix`. // the exact size is re-checked against each actual template in `JobFactory::coinbase` - if job_factory.script_sig_size(MAX_COINBASE_PREFIX_SIZE, full_extranonce_size) - > MAX_SCRIPT_SIG_SIZE - { + if !job_factory.fits_script_sig_budget(full_extranonce_size) { return Err(GroupChannelError::ScriptSigSizeTooLarge); } @@ -184,17 +178,16 @@ impl GroupChannel { /// Also clears all channel IDs, as no channels can belong to the same group while having different `full_extranonce_size`s. /// /// Returns [`GroupChannelError::ScriptSigSizeTooLarge`] if the new size would push the - /// assembled coinbase `scriptSig` past [`MAX_SCRIPT_SIG_SIZE`], leaving the group channel - /// unchanged. + /// assembled coinbase `scriptSig` past its budget (see + /// [`JobFactory::fits_script_sig_budget`]), leaving the group channel unchanged. pub fn set_full_extranonce_size( &mut self, full_extranonce_size: usize, ) -> Result<(), GroupChannelError> { // re-run the constructor's invariant, before touching any state - if self + if !self .job_factory - .script_sig_size(MAX_COINBASE_PREFIX_SIZE, full_extranonce_size) - > MAX_SCRIPT_SIG_SIZE + .fits_script_sig_budget(full_extranonce_size) { return Err(GroupChannelError::ScriptSigSizeTooLarge); } @@ -267,9 +260,10 @@ impl GroupChannel { /// /// Returns [`GroupChannelError::JobFactoryError`] wrapping /// [`JobFactoryError::ScriptSigSizeTooLarge`](crate::server::jobs::error::JobFactoryError::ScriptSigSizeTooLarge) - /// if the template's `coinbase_prefix` pushes the assembled coinbase `scriptSig` past - /// [`MAX_SCRIPT_SIG_SIZE`]. The constructor can only check against the spec's worst-case - /// [`MAX_COINBASE_PREFIX_SIZE`], so this is where an out-of-spec Template Provider is caught. + /// if the template's `coinbase_prefix` pushes the assembled coinbase `scriptSig` past its + /// budget. The constructor can only check against the spec's worst-case prefix (see + /// [`JobFactory::fits_script_sig_budget`]), so this is where an out-of-spec Template Provider + /// is caught. pub fn on_new_template( &mut self, template: NewTemplateOwned, diff --git a/sv2/channels-sv2/src/server/jobs/factory.rs b/sv2/channels-sv2/src/server/jobs/factory.rs index 4d7eeb57..6215a681 100644 --- a/sv2/channels-sv2/src/server/jobs/factory.rs +++ b/sv2/channels-sv2/src/server/jobs/factory.rs @@ -46,8 +46,8 @@ use template_distribution_sv2::NewTemplateOwned; /// consensus rules. pub const MAX_SCRIPT_SIG_SIZE: usize = 100; -/// Maximum number of bytes that [`NewTemplate::coinbase_prefix`] is allowed to contribute to the -/// coinbase `scriptSig`, as mandated by the Sv2 Template Distribution Protocol spec. +/// Maximum number of bytes that [`NewTemplateOwned::coinbase_prefix`] is allowed to contribute to +/// the coinbase `scriptSig`, as mandated by the Sv2 Template Distribution Protocol spec. /// /// The spec phrase "up to 8 bytes (not including the length byte)" refers to the `B0255` wire /// length prefix. The BIP34 script push opcode is already part of these 8 bytes. @@ -149,8 +149,8 @@ impl JobFactory { } /// Returns the number of bytes of the coinbase `scriptSig` assembled by this factory, for a - /// [`NewTemplate::coinbase_prefix`] of `coinbase_prefix_size` bytes and a full extranonce of - /// `full_extranonce_size` bytes. + /// [`NewTemplateOwned::coinbase_prefix`] of `coinbase_prefix_size` bytes and a full extranonce + /// of `full_extranonce_size` bytes. /// /// The assembled layout is: /// `coinbase_prefix || OP_PUSHBYTES_N || /pool_tag/miner_tag/ || OP_PUSHBYTES_M || extranonce` @@ -168,6 +168,19 @@ impl JobFactory { + full_extranonce_size } + /// Returns whether the coinbase `scriptSig` assembled by this factory fits within + /// [`MAX_SCRIPT_SIG_SIZE`], for a full extranonce of `full_extranonce_size` bytes and the + /// worst-case [`NewTemplateOwned::coinbase_prefix`] allowed by the spec + /// ([`MAX_COINBASE_PREFIX_SIZE`]). + /// + /// Meant for callers that do not have a template at hand, such as channel constructors and + /// extranonce setters. Since it assumes the largest in-spec prefix, a `true` here guarantees + /// that no in-spec template can overflow the budget; the exact size is still re-checked + /// against each actual template in `JobFactory::coinbase`. + pub fn fits_script_sig_budget(&self, full_extranonce_size: usize) -> bool { + self.script_sig_size(MAX_COINBASE_PREFIX_SIZE, full_extranonce_size) <= MAX_SCRIPT_SIG_SIZE + } + /// Creates a new job from a template. /// /// This job (and related shares) is fully committed to: diff --git a/sv2/channels-sv2/src/server/standard.rs b/sv2/channels-sv2/src/server/standard.rs index 90af70c3..043a4efa 100644 --- a/sv2/channels-sv2/src/server/standard.rs +++ b/sv2/channels-sv2/src/server/standard.rs @@ -39,10 +39,7 @@ use crate::{ server::{ error::StandardChannelError, jobs::{ - extended::ExtendedJob, - factory::{JobFactory, MAX_COINBASE_PREFIX_SIZE, MAX_SCRIPT_SIG_SIZE}, - job_store::JobStore, - standard::StandardJob, + extended::ExtendedJob, factory::JobFactory, job_store::JobStore, standard::StandardJob, }, share_accounting::{ShareAccounting, ShareValidationError, ShareValidationResult}, }, @@ -116,8 +113,8 @@ impl StandardChannel { /// and `//` delimiters: `/pool_tag_string//` /// /// Returns [`StandardChannelError::ScriptSigSizeTooLarge`] if the tags, the delimiters, the - /// extranonce prefix and a worst-case [`MAX_COINBASE_PREFIX_SIZE`] coinbase prefix do not fit - /// within [`MAX_SCRIPT_SIG_SIZE`]. + /// extranonce prefix and a worst-case coinbase prefix do not fit within the coinbase + /// `scriptSig` budget, see [`JobFactory::fits_script_sig_budget`]. #[allow(clippy::too_many_arguments)] pub fn new_for_pool( channel_id: u32, @@ -154,8 +151,8 @@ impl StandardChannel { /// `/` delimiters: `/pool_tag_string/miner_tag_string/` /// /// Returns [`StandardChannelError::ScriptSigSizeTooLarge`] if the tags, the delimiters, the - /// extranonce prefix and a worst-case [`MAX_COINBASE_PREFIX_SIZE`] coinbase prefix do not fit - /// within [`MAX_SCRIPT_SIG_SIZE`]. + /// extranonce prefix and a worst-case coinbase prefix do not fit within the coinbase + /// `scriptSig` budget, see [`JobFactory::fits_script_sig_budget`]. #[allow(clippy::too_many_arguments)] pub fn new_for_job_declaration_client( channel_id: u32, @@ -218,9 +215,7 @@ impl StandardChannel { // conservative check against the spec's worst-case `NewTemplate::coinbase_prefix`. // the exact size is re-checked against each actual template in `JobFactory::coinbase`. // standard channels have no rollable extranonce, so the prefix is the full extranonce - if job_factory.script_sig_size(MAX_COINBASE_PREFIX_SIZE, extranonce_prefix.len()) - > MAX_SCRIPT_SIG_SIZE - { + if !job_factory.fits_script_sig_budget(extranonce_prefix.len()) { return Err(StandardChannelError::ScriptSigSizeTooLarge); } @@ -259,7 +254,8 @@ impl StandardChannel { /// Sets a new extranonce prefix for this channel. /// /// Returns an error if the new prefix is too large, or if it would push the assembled coinbase - /// `scriptSig` past [`MAX_SCRIPT_SIG_SIZE`]. The channel is left unchanged in both error cases. + /// `scriptSig` past its budget (see [`JobFactory::fits_script_sig_budget`]). The channel is + /// left unchanged in both error cases. pub fn set_extranonce_prefix( &mut self, extranonce_prefix: AllocatedExtranoncePrefix, @@ -270,10 +266,9 @@ impl StandardChannel { // re-run the constructor's invariant: a prefix that is individually valid can still push // the assembled scriptSig past the consensus cap - if self + if !self .job_factory - .script_sig_size(MAX_COINBASE_PREFIX_SIZE, extranonce_prefix.len()) - > MAX_SCRIPT_SIG_SIZE + .fits_script_sig_budget(extranonce_prefix.len()) { return Err(StandardChannelError::ScriptSigSizeTooLarge); } @@ -451,8 +446,9 @@ impl StandardChannel { /// Returns [`StandardChannelError::JobFactoryError`] wrapping /// [`JobFactoryError::ScriptSigSizeTooLarge`](crate::server::jobs::error::JobFactoryError::ScriptSigSizeTooLarge) /// if the template's `coinbase_prefix` pushes the assembled coinbase `scriptSig` past - /// [`MAX_SCRIPT_SIG_SIZE`]. The constructor can only check against the spec's worst-case - /// [`MAX_COINBASE_PREFIX_SIZE`], so this is where an out-of-spec Template Provider is caught. + /// its budget. The constructor can only check against the spec's worst-case prefix (see + /// [`JobFactory::fits_script_sig_budget`]), so this is where an out-of-spec Template Provider + /// is caught. pub fn on_new_template( &mut self, template: NewTemplateOwned,

`StandardChannel::validate_share` rebuilds the coinbase from scratch when a share meets the network target. It wrote the `OP_PUSHBYTES` opcode from `self.extranonce_prefix.len()` (the channel's *current* prefix) but then pushed `job.get_extranonce_prefix()` (the prefix the job was created with). `set_extranonce_prefix` may rotate the channel to a prefix of a different length at any point after a job is created. When that happens the opcode disagrees with the number of bytes that follow, so the reconstructed coinbase deserializes into a different `scriptSig` and its txid diverges from the `merkle_root` the job (and the winning share's header) is committed to. The block is then rejected by Bitcoin nodes and the reward is silently lost. The job's prefix is the only correct source here: the job's `merkle_root` is committed to it, and jobs created before a rotation intentionally keep serving the old prefix for share validation. Refs stratum-mining#2242

The three server channel constructors enforced the 100-byte `scriptSig` cap using a hardcoded `5 // BIP34` allowance for the template's `coinbase_prefix`, and nothing re-checked the budget once the actual `NewTemplate` arrived. A config sitting at the edge of the budget therefore assembled an oversized, consensus-invalid `scriptSig` as soon as a template carried a longer prefix. For a group channel that means the pool distributes unmineable work to every channel in the group. The template's `coinbase_prefix` is only known inside `JobFactory::coinbase`, which is where the `scriptSig` is actually assembled, so that is where the authoritative check belongs. Placing it there covers every `NewTemplate`-derived path at once (`new_standard_job`, `new_extended_job` and `new_coinbase_tx_prefix_and_suffix`), hence all three channels' `on_new_template`. The `SetCustomMiningJob` path is left alone: length validation there is delegated to upstream callers. The constructors keep a conservative pre-flight check, so an unusable config still fails at startup rather than at every job creation, but the allowance is raised from 5 to `MAX_COINBASE_PREFIX_SIZE` (8) — the cap the Template Distribution Protocol spec puts on `NewTemplate::coinbase_prefix`. The spec's "not including the length byte" refers to the `B0255` wire length prefix, not the BIP34 script push opcode, which is already part of those 8 bytes. This costs 3 bytes of tag/extranonce budget relative to the old estimate, in exchange for no in-spec template ever overflowing a channel that was accepted at construction. Both checks now share `JobFactory::script_sig_size`, which also removes the tag length arithmetic that was duplicated across `coinbase_tx_prefix` and `coinbase_tx_suffix`. Finally, `op_pushbytes_pool_miner_tag` capped the tag blob at 61 bytes, a number derived from yet another budget model (it assumed a 5-byte BIP34 prefix and a 32-byte extranonce), so it disagreed with the constructors in both directions. Now that `MAX_SCRIPT_SIG_SIZE` is enforced on the real assembled size, that match only needs to enforce what it is actually about: `OP_PUSHBYTES_N` is a valid single-byte push opcode only for N in 1..=75. Refs stratum-mining#2242

The channel constructors enforce that the tags, the extranonce and a worst-case coinbase prefix fit within the 100-byte `scriptSig` cap, but the setters that can change the extranonce configuration afterwards dropped that invariant: - `ExtendedChannel::set_extranonce_prefix` and `StandardChannel::set_extranonce_prefix` only re-checked `MAX_EXTRANONCE_LEN`, so swapping in a prefix that is individually valid but longer than the one the channel was built with silently pushed the total past the cap; - `GroupChannel::set_full_extranonce_size` checked nothing at all. Every job created after such a call carries a consensus-invalid coinbase, so a block found on the channel is rejected and the reward is lost. All three now re-run the same invariant through `JobFactory::script_sig_size`, before touching any state, so a rejected update leaves the channel exactly as it was. `set_full_extranonce_size` therefore becomes fallible; its only caller is in-crate. Refs stratum-mining#2242

plebhash changed the title ~~channels_sv2: fix coinbase scriptSig size/serialization defects yield consensus-invalid coinbases~~ channels_sv2: fix coinbase scriptSig size/serialization defects that yield consensus-invalid coinbases Aug 2, 2026

plebhash force-pushed the 2026-08-01-scriptsig-serialization-defects branch from cec5c34 to a26f3a5 Compare August 2, 2026 03:07

plebhash mentioned this pull request Aug 2, 2026

JDC: propagate GroupChannel::set_full_extranonce_size error stratum-mining/sv2-apps#658

Draft

bit-aloo reviewed Aug 3, 2026

View reviewed changes

plebhash force-pushed the 2026-08-01-scriptsig-serialization-defects branch from a26f3a5 to 5eb1f94 Compare August 3, 2026 14:39

plebhash added 3 commits August 3, 2026 14:58

plebhash force-pushed the 2026-08-01-scriptsig-serialization-defects branch from 5eb1f94 to fa12779 Compare August 3, 2026 17:59

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

`channels_sv2`: fix coinbase `scriptSig` size/serialization defects that yield consensus-invalid coinbases - #2243

`channels_sv2`: fix coinbase `scriptSig` size/serialization defects that yield consensus-invalid coinbases#2243
plebhash wants to merge 3 commits into
stratum-mining:mainfrom
plebhash:2026-08-01-scriptsig-serialization-defects

plebhash commented Aug 2, 2026 •

edited

Loading

Uh oh!

bit-aloo left a comment

Uh oh!

bit-aloo Aug 3, 2026

Uh oh!

plebhash Aug 3, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

plebhash commented Aug 2, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bit-aloo left a comment

Choose a reason for hiding this comment

Uh oh!

bit-aloo Aug 3, 2026

Choose a reason for hiding this comment

Uh oh!

plebhash Aug 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

plebhash commented Aug 2, 2026 •

edited

Loading

plebhash Aug 3, 2026 •

edited

Loading