mining, ipc: omit dummy extraNonce from coinbase

Sjors commented at 12:01 pm on May 5, 2025: member

This PR changes the Mining IPC interface to stop including a dummy extraNonce in the coinbase scriptSig by default, exposing only the consensus-required BIP34 height. This simplifies downstream mining software (including Stratum v2), avoids forcing clients to strip or ignore data we generate, and reduces the risk of incompatibilities if future soft forks add required commitments to the scriptSig.

Existing behavior is preserved for RPCs, tests, regtest, and internal mining by explicitly opting in to the dummy extraNonce where needed (e.g. to satisfy bad-cb-length at low heights), so consensus rules and test coverage are unchanged. The remainder of the PR consists of small comment fixes, naming clarifications, and test cleanups to make the intent and behavior clearer.

DrahtBot commented at 12:01 pm on May 5, 2025: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32420.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	ryanofsky, sedited, achow101
Concept ACK	shahsb

If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#34452 (test: split interface_ipc.py by Sjors)
#34184 (mining: add cooldown to createNewBlock() immediately after IBD by Sjors)
#33966 (refactor: disentangle miner startup defaults from runtime options by Sjors)
#33922 (mining: add getMemoryLoad() and track template non-mempool memory footprint by Sjors)
#33421 (node: add BlockTemplateCache by ismaelsadeeq)
#32468 (rpc: generateblock to allow multiple outputs by polespinasa)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

DrahtBot added the label Mining on May 5, 2025

in src/node/miner.cpp:166 in 9a6dc0db85 outdated

160@@ -161,7 +161,12 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock()
161     coinbaseTx.vout.resize(1);
162     coinbaseTx.vout[0].scriptPubKey = m_options.coinbase_output_script;
163     coinbaseTx.vout[0].nValue = nFees + GetBlockSubsidy(nHeight, chainparams.GetConsensus());
164-    coinbaseTx.vin[0].scriptSig = CScript() << nHeight << OP_0;
165+    coinbaseTx.vin[0].scriptSig = CScript() << nHeight;
166+    // IsTestChain() can be dropped if hardcoded block hashes in tests are regenerated
167+    if (nHeight <= 16 || chainparams.IsTestChain()) {

maflcko commented at 12:12 pm on May 5, 2025:

nit: Would be good to limit this to regtest, assuming this is the only test network that “needs” it. Otherwise this can’t be tested outside of mainnet?

Sjors commented at 12:28 pm on May 5, 2025:

I’ll try to narrow it down to regtest.

ajtowns commented at 1:08 pm on May 5, 2025:

Match the comment to the updated code too?

Sjors commented at 1:33 pm on May 5, 2025:

Fixed

Sjors force-pushed on May 5, 2025

Sjors commented at 1:03 pm on May 5, 2025: member

I limited the exception to regtest. I also found two tests that for some reason implement their own coinbase construction code. I adjusted those for consistently with the first commit.

CreateBlockChain is used by the utxo_snapshot fuzzer which relies on hardcoded block hashes, so I just added a comment instead of changing it.

Sjors force-pushed on May 5, 2025

shahsb commented at 2:48 am on May 6, 2025: none

Concept ACK

DrahtBot added the label Needs rebase on May 9, 2025

ajtowns commented at 4:33 pm on May 10, 2025: contributor

Our miner code adds OP_0 to the coinbase scriptSig in order to avoid triggering the bad-cb-length consensus error on test networks.

Correct me if I’m wrong, please! I think what’s actually going on here is:

scriptSigs in the coinbase have had to be 2 bytes or more since before bitcoin’s git history began
this is perhaps because mainnet bitcoin blocks will usually require some extranonce stuff, and the coinbase tx’s scriptSig is a sensible place to do that
in any event, satoshi set the scriptSig up to contain a push of nBits and a push of the extranonce. this was changed to be time and extranonce (#505) and finally height and extranonce via BIP34 activation.
when BIP34 is applied to blocks 1-16, the coinbase is encoded as a 1-byte OP_1 through OP_16 (this is compliant with the “minimally encoded serialized CScript” part of the spec, though not the “first byte is number of bytes…” part).
this means the only blocks compliant with BIP34 that might potentially have a too short scriptSig are blocks 1 through 16
because mining regtest doesn’t require extranonce stuff, there’s no automatic reason to add extra bytes here, but the cb-length consensus check requires it anyway.
thus, when mining regtest blocks in the functional test framework, we explicitly add an OP_0 in blocktools.py:script_BIP34_coinbase_height for blocks 1 through 16. regtest was introduced a year after BIP 34, so there was never a question of “what did we do before BIP34 was enforced on regtest”
that code was introduced in #16363, at which point BIP34 wasn’t activated in regtest until block height 500; this was lowered to 2 in #16333 at which point the code was used

So I think it’s more fair to say that our miner code adds OP_0 as a dummy extraNonce, expecting it to be incremented as header nonce space runs out, which just never happens in unit tests and regtest.

In the current code, having a dummy extraNonce actually seems sensible to me – in real PoW contexts, we’d need a real extraNonce anyway, so this makes the test environment a little more similar to reality… So I feel a bit -0 on this as a consequence. I wonder if there isn’t a way to have this work for stratumv2 without changing the way the existing code works?

Would it work for the stratumv2 interface (the part of it inside bitcoin core?) to just recognise we supply a dummy extra nonce, and drop it? Even for the first 16 blocks, sv2 miners that supply a non-empty extraNonce of their own, or that include a pool-signature in the coinbase will pass the cb-length check. And after the first 16 blocks, it’s not an issue at all.

Sjors commented at 2:04 pm on May 12, 2025: member

Rebased after #32155.

So I think it’s more fair to say that our miner code adds OP_0 as a dummy extraNonce

Possibly, but extraNonce seems like an implementation detail that should be left to miners. E.g. the Stratum v2 spec defines how to use it here: https://github.com/stratum-mining/sv2-spec/blob/main/05-Mining-Protocol.md

It doesn’t belong in a block template imo.

I wonder if there isn’t a way to have this work for stratumv2 without changing the way the existing code works?

The Template Distribution protocol defines a message NewTemplate which has a coinbase_prefix field, described as:

Up to 8 bytes (not including the length byte) which are to be placed at the beginning of the coinbase field in the coinbase transaction

https://github.com/stratum-mining/sv2-spec/blob/main/07-Template-Distribution-Protocol.md#72-newtemplate-server---client

In the Job Declaration Protocol (which the node doesn’t play a role in) coinbase_tx_prefix is defined as:

Serialized bytes representing the initial part of the coinbase transaction (not including extranonce)

Would it work for the stratumv2 interface … to just recognise we supply a dummy extra nonce, and drop it

Yes but this would be a foot gun if a future soft fork requires an additional commitment. It’s also up to every consumer of our Mining interface to implement that (correctly), not just the one I wrote.

We could implement it somewhere between the mining code and interface, so at least interface users don’t have to deal with this. But currently BlockAssembler::CreateNewBlock() is called pretty much directly without further processing.

Sjors force-pushed on May 12, 2025

DrahtBot removed the label Needs rebase on May 12, 2025

ajtowns commented at 2:55 pm on May 12, 2025: contributor

It doesn’t belong in a block template imo.

Right, but we currently don’t include it in a block template either, so that’s (currently) fine.

Would it work for the stratumv2 interface … to just recognise we supply a dummy extra nonce, and drop it

Yes but this would be a foot gun if a future soft fork requires an additional commitment. It’s also up to every consumer of our Mining interface to implement that (correctly), not just the one I wrote.

I think it’s more likely that any additional commitments required by future soft forks will be in the coinbase tx’s outputs because the coinbase scriptSig’s limited to 100 bytes. The segwit commitment output is designed to allow for this, so additional outputs aren’t needed; signet makes use of this ability for the block signature.

We could implement it somewhere between the mining code and interface, so at least interface users don’t have to deal with this. But currently BlockAssembler::CreateNewBlock() is called pretty much directly without further processing.

Yeah, this was what I was thinking. Maybe adding something like:

 0struct CBlockTemplate
 1{
 2    CBlock block;
 3
 4    std::span<unsigned char> CoinbaseScriptSigPrefix()
 5    {
 6       if (block.vtx.size() == 0 || block.vtx[0].vin.size() == 0 || block.vtx[0].vin[0].scriptSig.size() == 0) return {};
 7       // our scriptSig includes a dummy extraNonce. Drop it here.
 8       std::span<unsigned char> r{block.vtx[0].vin[0].scriptSig};
 9       return r.first(r.size()-1);
10    }
11    ...
12};

and whatever getblocktemplate-ish api we introduce for stratumv2 calls that function to provide the information rather than dumping the scriptSig directly.

Sjors commented at 3:33 pm on May 12, 2025: member

I also don’t think CBlockTemplate should have an extraNonce. Instead it could be added by code that actually does the mining, such as the GenerateBlock block method in rpc/mining.cpp. That seems like a cleaner separation of concerns between template construction and mining.

Although CheckBlock() also needs it to be present for these early blocks, unless we pass an argument in to skip the bad-cb-length check.

achow101 commented at 2:47 pm on October 22, 2025: member

Are you still working on this?

Sjors requested review from ajtowns on Oct 31, 2025

ajtowns commented at 7:11 pm on October 31, 2025: contributor

I also don’t think CBlockTemplate should have an extraNonce.

I think a block should (almost) always have an extraNonce – whether for mainnet to provide sufficient work, or for early blocks on regtest/signet to avoid bad-cb-length, or for test net blocks to more closely simulate mainnet behaviour. Whether the template should also include an extraNonce depends on the user of the template – if they want a template they can just apply work to via nNonce, without further fussing about (like we do in our unit tests), then the template should also include an extraNonce, but otherwise it doesn’t need to.

The current commit descriptions in this PR claim “Our miner code adds OP_0 to the coinbase scriptSig in order to avoid triggering the bad-cb-length consensus error on test networks.” – I don’t think that’s really reflective of the intent behind that code though. It adds an OP_0 because in blocks (not templates) an extraNonce is expected, but only a dummy value is needed. That does avoid the bad-cb-length, but the motivation for it being there is just that the coinbase has always had pushes of two values in it; originally nBits and extraNonce, then nTime and extraNonce, now nHeight and extraNonce.

So rather than special casing early blocks and regtest, how about just making it an explicit option, whether defaulting to true (ie, current behaviour) or false? eg: https://github.com/ajtowns/bitcoin/commits/202510-miner-extranonce/

ajtowns removed review request from ajtowns on Oct 31, 2025

Sjors commented at 7:35 pm on November 1, 2025: member

Indeed the bad-cb-length check has been around since the first commit, of course without a documented reason.

https://github.com/bitcoin/bitcoin/blob/e071a3f6c06f41068ad17134189a4ac3073ef76b/main.h#L454-L457

There was also no documented reason for adding nBits to the coinbase scriptSig, before the extraNonce. I just asked: https://bitcoin.stackexchange.com/questions/129167/why-did-satoshi-put-nbits-in-the-coinbase-scriptsig

https://github.com/bitcoin/bitcoin/blob/e071a3f6c06f41068ad17134189a4ac3073ef76b/main.cpp#L2236-L2244

But as you point out in your historical overview, nBits was swapped for nTime, and then later for the height which became a consensus requirement.

So I agree it makes sense to call it extraNonce and not “dummy zero”. Adding an option include_dummy_extranonce seems a bit overkill, but I guess it’s fine.

Sjors force-pushed on Nov 3, 2025

Sjors renamed this:
~~miner: don't needlessly append dummy OP_0 to bip34~~
miner: drop dummy extraNonce in coinbase scriptSig for templates requested via IPC
on Nov 3, 2025

Sjors commented at 9:01 am on November 3, 2025: member

I switched to the approach of adding an include_dummy_extranonce, having the tests set it to true while IPC calls use the default false. Expanded test/functional/interface_ipc.py to demonstrate it.

Sjors force-pushed on Nov 3, 2025

Sjors commented at 10:10 am on November 3, 2025: member

Looks like fuzz/process_message.cpp and fuzz/process_messages.cpp were missing include_dummy_extranonce = true.

ajtowns requested review from ajtowns on Nov 8, 2025

DrahtBot added the label Needs rebase on Nov 12, 2025

ryanofsky approved

ryanofsky commented at 10:34 pm on November 12, 2025: contributor

Code review ACK b03feedadb411e15967659be83940d73ce977848. Assuming #33819 goes ahead, there should be less need for this change from the mining interface perspective. But not adding OP_0 seems like more sensible default behavior and all the changes here seem nice to make the test code and BlockAssembler code more explicit and clear about what they are doing.

DrahtBot requested review from shahsb on Nov 12, 2025

in src/node/types.h:67 in b03feedadb

62@@ -64,6 +63,10 @@ struct BlockCreateOptions {
63      * coinbase_max_additional_weight and coinbase_output_max_additional_sigops.
64      */
65     CScript coinbase_output_script{CScript() << OP_TRUE};
66+    /**
67+     * Whether to include and OP_0 as a dummy extraNonce in the template's coinbase

maflcko commented at 9:15 am on November 13, 2025:

    /**\n+ * Whether to include and OP_0 as a dummy extraNonce in the template's coinbase\n+ */\n -> "and" -> "an" [Correct article: "an OP_0" (singular noun) makes the sentence grammatical and clear.]

Sjors force-pushed on Nov 18, 2025

Sjors commented at 12:09 pm on November 18, 2025: member

Rebased after #33745 due to import conflict in the test. Also fixed comment typo.

DrahtBot removed the label Needs rebase on Nov 18, 2025

DrahtBot added the label Needs rebase on Dec 16, 2025

ryanofsky approved

ryanofsky commented at 5:59 pm on December 17, 2025: contributor

Code review ACK 9832a9aa2bffb5c0e14eb67ce22d9ce900106304. Just rebased since last review and tweaked comment

Sjors commented at 9:21 am on December 19, 2025: member

Trivial rebase after #34003.

Sjors force-pushed on Dec 19, 2025

DrahtBot removed the label Needs rebase on Dec 19, 2025

ryanofsky approved

ryanofsky commented at 6:50 pm on January 7, 2026: contributor

Code review ACK a3e887766459f850a9ff6f7c84139baa63da50c0, just fixing conflict in test since last review

DrahtBot added the label Needs rebase on Jan 13, 2026

Sjors force-pushed on Jan 14, 2026

DrahtBot removed the label Needs rebase on Jan 14, 2026

Sjors commented at 9:08 am on January 14, 2026: member

Rebased after:

#33819
adjusted the “trailing OP_0” comment in node/miner.cpp‎ introduced in that PR
modified the functional/interface_ipc.py check introduced by this PR to use the new getCoinbaseTx() introduced in that PR (we already check the result is identical to getCoinbaseRawTx())

in src/node/miner.cpp:186 in 75bce1e711

186+    // increasing its length would reduce the space they can use and may break
187+    // existing clients.
188+    coinbaseTx.vin[0].scriptSig = CScript() << nHeight;
189+    if (m_options.include_dummy_extranonce) {
190+        // Add trailing OP_0 (historically an extranonce). It's not consensus
191+        // required, but it prevents bad-cb-length for nHeight <= 16.

ryanofsky commented at 3:05 pm on January 14, 2026:

In commit “[miner] omit dummy extraNonce via IPC” (75bce1e711351e2d3e512c4a24db0275afe5a999)

I found this comment confusing because it’s not clear where the bad-cb-length error would occur. Maybe adding “in tests” to the end of this sentence could help and be a minimal fix. Chatgpt also suggested following comment based on discussion history in this PR which I do find clearer:

0// For blocks at heights <= 16, the BIP34-encoded height alone is only one byte.
1// Consensus requires coinbase scriptSigs to be at least two bytes long
2// (bad-cb-length), so tests and regtest include a dummy extraNonce (OP_0)

Sjors commented at 4:56 pm on January 14, 2026:

Not just in tests, also on new signets. And since we removed checkpoints, any aliens using mining code for a giant reorg back to genesis also need to be careful :-)

Sjors commented at 1:45 pm on January 27, 2026:

Took your comment. The signet miner uses the same technique, but it constructs the coinbase transaction using the Python test framework, so that’s not relevant for this codepath.

in test/functional/interface_ipc.py:237 in 75bce1e711

ryanofsky commented at 3:31 pm on January 14, 2026:

In commit “[miner] omit dummy extraNonce via IPC” (75bce1e711351e2d3e512c4a24db0275afe5a999)

It looks like the “we already check the result is identical to getCoinbaseRawTx()” check is happening" on line 239 here, but I think this comment should say getCoinbaseRawTx not getCoinbaseTx to be accurate. It would also help to rename parse_and_deserialize_coinbase_tx to parse_and_deserialize_coinbase_raw_tx, or something shorter like get_coinbase_raw_tx to be clear this is calling the deprecated raw method.

ryanofsky commented at 3:33 pm on January 14, 2026:

In commit “[miner] omit dummy extraNonce via IPC” (75bce1e711351e2d3e512c4a24db0275afe5a999)

It looks like this could be simplified by calling parse_and_deserialize_coinbase_tx. (Maybe in a cleanup commit)

Sjors commented at 1:45 pm on January 27, 2026:

I dropped this check, since it’s made redundant by the comparison between getCoinbaseRawTx and getCoinbaseTx().

Sjors commented at 1:45 pm on January 27, 2026:

Done

ryanofsky approved

ryanofsky commented at 3:40 pm on January 14, 2026: contributor

Code review ACK 75bce1e711351e2d3e512c4a24db0275afe5a999. Was rebased to avoid conflicts with #33819 since last review

[doc] Update comments on dummy extraNonces in tests 78df9003d6

test: clarify getCoinbaseRawTx() comparison

The code comment mistakingly referred to "the deprecated getCoinbaseTx()",
instead of getCoinbaseRawTx. This was missed in d59b4cdb5772917ee13e48552d51662160104b62.

Also rename parse_and_deserialize_coinbase_tx to make it more clear
it refers to the deprecated method.

Finally, this commit drops the getCoinbaseRawTx() call when testing
template inspectors. The coinbase input check here is already covered by
build_coinbase_test.

bf3b5d6d06

[miner] omit dummy extraNonce via IPC

Previously the coinbase transaction generated by our miner code was
not used downstream, because the getblocktemplate RPC excludes it.

Since the Mining IPC interface was introduced in #30200 we do expose
this dummy coinbase transaction. In Stratum v2 several parts of it
are communicated downstream, including the scriptSig.

This commit removes the dummy extraNonce from the coinbase scriptSig
in block templates requested via IPC. This limits the scriptSig
to what is essential for consensus (BIP34) and removes the need for
external mining software to remove the dummy, or even ignore
the scriptSig we provide and generate it some other way. This
could cause problems if a future soft fork requires additional
data to be committed here.

A test is added to verify the new IPC behavior.

It achieves this by introducing an include_dummy_extranonce
option which defaults to false with all test code updated to
set it to true. Because this option is not exposed via IPC,
callers will no longer see it.

The caller needs to ensure that for blocks 1 through 16
they pad the scriptSig in order to avoid bad-cb-length.

Co-authored-by: Anthony Towns <aj@erisian.com.au>

d511adb664

Sjors force-pushed on Jan 27, 2026

in test/functional/interface_ipc.py:147 in bf3b5d6d06

143@@ -144,7 +144,7 @@ async def parse_and_deserialize_block(self, block_template, ctx):
144         block.deserialize(block_data)
145         return block
146 
147-    async def parse_and_deserialize_coinbase_tx(self, block_template, ctx):
148+    async def get_coinbase_raw_tx(self, block_template, ctx):

ryanofsky commented at 1:05 pm on January 28, 2026:

In commit “test: clarify getCoinbaseRawTx() comparison” (bf3b5d6d069a0bbb39af0c487fd597257f862f31)

Looking around here it would be nice IMO to rename parse_and_deserialize_block to get_block and parse_and_deserialize_coinbase to get_coinbase as well. Existing names are hard to read and look like they are doing they are doing local parsing and deserializing when actually they are making remote calls.

Sjors commented at 10:52 am on January 29, 2026:

I’ll rename parse_and_deserialize_coinbase if I need to retouch, but will leave parse_and_deserialize_block as is for now, to not expand the scope of this PR.

ryanofsky approved

ryanofsky commented at 1:30 pm on January 28, 2026: contributor

Code review ACK d511adb664edcfb97be44bc0738f49b679240504. Just rebased since last review and make suggested tweaks. I’d really like to see this PR merged for the cleanups and sanity it brings to this code. Needs another reviewer though.

I feel like current PR description may be a hurdle to getting this reviewed since it is very technical and immediately goes into weeds with details about the history of the code and about stratum v2. All this information is useful but IMO it would be much better to put in the third commit message or in the IPC interface documentation where it will be more accessible anyway after this is merged. Then the PR description can just say why this change is good and what information is most useful to review it. Chatgpt suggested following which reads very well to me:

This PR changes the Mining IPC interface to stop including a dummy extraNonce in the coinbase scriptSig by default, exposing only the consensus-required BIP34 height. This simplifies downstream mining software (including Stratum v2), avoids forcing clients to strip or ignore data we generate, and reduces the risk of incompatibilities if future soft forks add required commitments to the scriptSig.

Existing behavior is preserved for RPCs, tests, regtest, and internal mining by explicitly opting in to the dummy extraNonce where needed (e.g. to satisfy bad-cb-length at low heights), so consensus rules and test coverage are unchanged. The remainder of the PR consists of small comment fixes, naming clarifications, and test cleanups to make the intent and behavior clearer.

It also suggested some shorter titles which may be good

mining ipc: omit dummy extraNonce from coinbase
mining ipc: omit dummy extraNonce (tests and RPC unchanged)
mining: make IPC coinbase scriptSig future-proof

Sjors renamed this:
~~miner: drop dummy extraNonce in coinbase scriptSig for templates requested via IPC~~
mining, ipc: omit dummy extraNonce from coinbase
on Jan 29, 2026

Sjors commented at 10:51 am on January 29, 2026: member

Alright, trying the simplified PR title and description.

in src/test/util/mining.cpp:52 in 78df9003d6 outdated

48@@ -49,6 +49,7 @@ std::vector<std::shared_ptr<CBlock>> CreateBlockChain(size_t total_height, const
49         coinbase_tx.vout.resize(1);
50         coinbase_tx.vout[0].scriptPubKey = P2WSH_OP_TRUE;
51         coinbase_tx.vout[0].nValue = GetBlockSubsidy(height + 1, params.GetConsensus());
52+        // Always include OP_0 as a dummy extraNonce.

sedited commented at 9:11 pm on January 30, 2026:

In commit 78df9003d63414e4a17b686af7647aeefd706ec5

Nit: I’m not sure how exhaustive you are trying to be here, but there are a few more cases in the benchmarks,

Sjors commented at 9:10 am on January 31, 2026:

In terms of adding the comment? I think a few places is enough.

Or do you mean in terms of actually adding the extraNonce? Only where it’s needed, in particular if it saves us from having to generate test blocks.

sedited commented at 9:13 am on January 31, 2026:

Yeah, I meant adding the comment. That’s fine.

sedited approved

sedited commented at 9:55 pm on January 30, 2026: contributor

ACK d511adb664edcfb97be44bc0738f49b679240504

achow101 commented at 11:09 pm on February 2, 2026: member

ACK d511adb664edcfb97be44bc0738f49b679240504

achow101 merged this on Feb 2, 2026

achow101 closed this on Feb 2, 2026

Sjors deleted the branch on Feb 3, 2026

mining, ipc: omit dummy extraNonce from coinbase #32420

Code Coverage & Benchmarks

Reviews

Conflicts