mining: always pad scriptSig at low heights, drop include_dummy

Sjors commented at 8:52 am on March 19, 2026: member

Blocks 0-16 on any new chain require mining code to be careful not to violate the bad-cb-length rule, which states the coinbase transaction scriptSig must be at least 2 bytes.

Our mining code deals with that by padding the scriptSig with a 0 extraNonce. It does this for every height. As a result IPC clients would get an unnecessary 0 in the scriptSigPrefix field of CoinbaseTx. #32420 fixed that by introducing a include_dummy_extranonce option in BlockCreateOptions and turning that off for IPC clients.

A minor issue was missed though: createNewBlock() now fails with bad-cb-length. An easy workaround is to use the generate RPC for the first 16 blocks, as demonstrated in the third commit.

The real fix is to have the miner code always pad the scriptSig at lower heights, but to not include that in the scriptSigPrefix field of CoinbaseTx (introduced in #33819). This is what the 4th commit implements.

Now that we set scriptSigPrefix independent of what our internal miner code does - to get past CheckBlock() - the original motivation for include_dummy_extranonce goes away and we can just drop it entirely. The last commit drops it and adjusts, adjusting the tests and hardcoded block and assume utxo hashes.

This last change does not break IPC clients, because include_dummy_extranonce was never exposed in mining.capnp.

Instead of adjusting the hardcoded hashes, an alternative approach would be to just always pad the scriptSig internally, since we exclude the padding from scriptSigPrefix anyway. However, IPC clients can also call getBlock() to get the raw block and might be confused about the difference. The miner code is also easier to understand if we limit the exception (coinbase_tx.script_sig_prefix != coinbaseTx.vin[0].scriptSig) to nHeight <= 16, where the explanation is based purely on consensus rules rather than historical test suite reasons.

The first two commits are preperation test changes:

extract assert_capnp_failed helper for macOS (also part of #34727)
use script_BIP34_coinbase_height in IPC mining test (existing code in interface_ipc_mining.py was incorrect for low height

test: extract assert_capnp_failed helper for macOS

Introduces the assert_capnp_failed helper in ipc_util.py to
cleanly handle macOS-specific Cap'n Proto exception strings, and
refactors an existing block weight test to use it.

Co-authored-by: Enoch Azariah <enirox001@gmail.com>

7881e298c7

test: use script_BIP34_coinbase_height in IPC mining test

Needed in a later commit to correctly derive the BIP34 prefix
for heights <= 16.

Add a padding parameter to script_BIP34_coinbase_height() that
controls whether the OP_0 dummy extranonce is appended for
heights <= 16.

Use this helper with padding=False in the IPC mining test's
build_coinbase_test().

f10d9690f0

test: bad-cb-length for createNewBlock() at low heights

On new regtest / signet chains (heights <= 16), createBlock() fails
internally with bad-cb-length. Since mining.capnp does not expose
include_dummy_extranonce, IPC clients can't work around this.

Add a functional test to illustrate this issue and use RPC to
work around it. The next commit introduces a fix.

51d748c431

DrahtBot added the label Mining on Mar 19, 2026

DrahtBot commented at 8:52 am on March 19, 2026: contributor

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

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept ACK	enirox001, w0xlt

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:

#34727 (test: Add IPC wake-up test and reuse mining context by enirox001)
#34644 (mining: add submitBlock to IPC Mining interface by w0xlt)
#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.

LLM Linter (✨ experimental)

Possible typos and grammar issues:

extranonce -> extraNonce [In test/functional/interface_ipc_mining.py comment: “createNewBlock pads the scriptSig with a dummy extranonce…”; this capitalization/word form looks like a typo and can momentarily confuse the intended term.]

2026-03-19 13:23:12

Sjors commented at 8:54 am on March 19, 2026: member

Given the easy workaround, I don’t think it’s necessary to backport this. The only downside of that workaround, and the reason I noticed the issue while working on another project, is that the generate RPC methods can’t be used to create a deterministic regtest chain. With createNewBlock() -> submitSolution() the IPC client can pick specific (cached) nonce and nTime values.

Historical note: the introduction of a CoinbaseTx struct for IPC clients in #33819 probably made the include_dummy_extranonce approach in #32420 unnecessary, since we can now give clients a different scriptSigPrefix than what we use in the block template in order to get past CheckBlock().

maflcko commented at 9:40 am on March 19, 2026: member

A nice benefit of this change is that it’s now possible to get rid of the include_dummy_extranonce option entirely, without breaking IPC clients because it’s not exposed in mining.capnp, but it would involve significant test churn

I don’t think test churn is a reason to leave deprecated/unused/confusing stuff in the real code. Test code should always follow the real code, not the other way round.

Sjors commented at 9:43 am on March 19, 2026: member

@maflcko I’ll push a commit to this PR to drop it entirely, updating the hashes where needed.

fanquake commented at 9:49 am on March 19, 2026: member

without breaking IPC clients

Until the interface stabilizes, and is marked as such, “breaking clients” should be much less of an issue? I don’t think we should have an interface that is marked as brand new/experimental, but then also not have the freedom to iterate/change it, because we are also half-pretending that it is meant to be stable/backwards compatible.

Sjors commented at 9:55 am on March 19, 2026: member

“breaking clients” should be much less of an issue?

Indeed, I just observed that it’s nice that we never exposed include_dummy_extranonce to clients in the first place, so dropping it is an internal change to Bitcoin Core that has no impact on IPC clients. But if it was a breaking change, we could indeed just do that as long as the interface is marked experimental.

Sjors force-pushed on Mar 19, 2026

Sjors renamed this:
~~mining: always set dummy_extranonce at low heights~~
mining: always pad scriptSig at low heights, drop include_dummy_extranonce
on Mar 19, 2026

Sjors commented at 12:33 pm on March 19, 2026: member

The ancestor commits failure seems unrelated, but the fuzzer failures seem relevant. Will investigate.

Sjors marked this as a draft on Mar 19, 2026

mining: always set dummy_extranonce at low heights

Unconditionally pad the coinbase transaction scriptSig with OP_0
for height up to 16, regardless of include_dummy_extranonce.

This avoids having to expose and document include_dummy_extranonce
to IPC clients.

The dummy OP_0 push is omitted from script_sig_prefix in the
getCoinbaseTx() result. Mining IPC clients are expected to construct
their own coinbase transaction and pass it to submitSolution().
They typically bring a real extraNonce or pool name which satisfies
the coinbase length requirement.

The next commit will drop include_dummy_extranonce entirely.

d6f9ab4d08

DrahtBot added the label CI failed on Mar 19, 2026

DrahtBot commented at 1:00 pm on March 19, 2026: contributor

🚧 At least one of the CI tasks failed. Task Windows native, fuzz, VS: https://github.com/bitcoin/bitcoin/actions/runs/23294168432/job/67737082288 LLM reason (✨ experimental): Fuzz test failure: fuzz.exe crashed on utxo_total_supply due to an assertion mismatch (scriptSig != duplicate_coinbase_script, utxo_total_supply.cpp:161).

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

mining: drop include_dummy_extranonce, update test hashes

Now that the prior commit makes OP_0 padding unconditional at
heights <= 16, include_dummy_extranonce is only used by tests.

Drop the option entirely and regenerate the hardcoded blocks
and transaction hashes throughout the test suite. Also update the
assume_utxo regtest snapshot in chainparams.

Additional side-effects:

- without the dummy extranonce, coinbase scriptSigs are 1 byte shorter
at heights > 16, making every block 1 byte smaller.  This shifts
where block files wrap and therefore where pruning boundaries land.

- feature_assumeutxo malleation cases
  - case 1: error message changes due to UTXO reordering, similar to
            8f2078af6a55448c003b3f7f3021955fbb351caa
  - case 4: the corruption byte is swapped from \x82 to \x83 because
            \x82 happened to be the actual value at that offset in
            the new snapshot.

d2af27bade

Sjors force-pushed on Mar 19, 2026

Sjors commented at 1:30 pm on March 19, 2026: member

The fuzzer was still adding the dummy extraNonce, should be fixed now.

DrahtBot removed the label CI failed on Mar 19, 2026

Sjors marked this as ready for review on Mar 19, 2026

in src/node/miner.cpp:192 in d6f9ab4d08

188+    // Set script_sig_prefix here, so IPC mining clients are not affected by
189+    // the optional scriptSig padding below. They provide their own extraNonce,
190+    // and in a typical setup a pool name or realistic extraNonce already makes
191+    // the scriptSig long enough.
192+    coinbase_tx.script_sig_prefix = coinbaseTx.vin[0].scriptSig;
193+    if (nHeight <= 16 || m_options.include_dummy_extranonce) {

enirox001 commented at 2:43 pm on March 21, 2026:

In commit “mining: always set dummy_extranonce at low heights” (https://github.com/bitcoin/bitcoin/pull/34860/changes/d6f9ab4d082dc12a9df976668689d7c75cfe34cd):

The condition nHeight <= 16 technically includes height 0. But height 0 is caught by the Assert below. Would it be cleaner to write if (nHeight <= 16 && nHeight > 0)? or move the Assert above the condition?

in src/node/miner.cpp:189 in d6f9ab4d08

183@@ -184,14 +184,19 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock()
184     // increasing its length would reduce the space they can use and may break
185     // existing clients.
186     coinbaseTx.vin[0].scriptSig = CScript() << nHeight;
187-    if (m_options.include_dummy_extranonce) {
188+    // Set script_sig_prefix here, so IPC mining clients are not affected by
189+    // the optional scriptSig padding below. They provide their own extraNonce,
190+    // and in a typical setup a pool name or realistic extraNonce already makes

enirox001 commented at 2:48 pm on March 21, 2026:

In commit “mining: always set dummy_extranonce at low heights” (https://github.com/bitcoin/bitcoin/pull/34860/changes/d6f9ab4d082dc12a9df976668689d7c75cfe34cd):

The comment says “in a typical setup a pool name or realistic extraNonce already makes the scriptSig long enough” but this puts the responsibility entirely on the client at heights ≤ 16. I tested this by removing the extra nonce in run_low_height_test() and it causes submitSolution() to silently return False with no explanation. Should we at minimum document in the code or the IPC interface that at heights ≤ 16 the client must provide at least 1 byte of extra nonce?

in src/node/miner.cpp:191 in d6f9ab4d08

183@@ -184,14 +184,19 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock()
184     // increasing its length would reduce the space they can use and may break
185     // existing clients.
186     coinbaseTx.vin[0].scriptSig = CScript() << nHeight;
187-    if (m_options.include_dummy_extranonce) {
188+    // Set script_sig_prefix here, so IPC mining clients are not affected by
189+    // the optional scriptSig padding below. They provide their own extraNonce,
190+    // and in a typical setup a pool name or realistic extraNonce already makes
191+    // the scriptSig long enough.
192+    coinbase_tx.script_sig_prefix = coinbaseTx.vin[0].scriptSig;

enirox001 commented at 3:02 pm on March 21, 2026:

In commit “mining: always set dummy_extranonce at low heights” (https://github.com/bitcoin/bitcoin/pull/34860/changes/d6f9ab4d082dc12a9df976668689d7c75cfe34cd):

At heights ≤ 16, getBlock() and getCoinbaseTx() return different scriptSig data since script_sig_prefix is snapshotted here before the OP_0 padding is added. I verified this in the test. getBlock() returns 5100 ([OP_1, OP_0]) while scriptSigPrefix returns 51 ([OP_1] only). Would an IPC client comparing both would be confused? Should this discrepancy be documented in the IPC interface or the code?

enirox001 commented at 3:19 pm on March 21, 2026: contributor

Concept ACK. Changes look good to me. Left a few questions about some of the changes made.

I also tested that the boundary works correctly across all heights up to 17. At heights 1-16 the discrepancy between getBlock() and scriptSigPrefix exists as expected, and disappears exactly at height 17

w0xlt commented at 8:07 am on March 22, 2026: contributor

Concept ACK

mining: always pad scriptSig at low heights, drop include_dummy_extranonce #34860

Reviews

Conflicts

LLM Linter (✨ experimental)