mining: pad coinbase to fix createNewBlock at heights <=16

diff --git a/src/interfaces/mining.h b/src/interfaces/mining.h
index f4c42e20..943d34e7 100644
--- a/src/interfaces/mining.h
+++ b/src/interfaces/mining.h
@@ -34,7 +34,8 @@ public:
     virtual ~BlockTemplate() = default;
 
     virtual CBlockHeader getBlockHeader() = 0;
-    // Block contains a dummy coinbase transaction that should not be used.
+    // Block contains a dummy coinbase transaction that should not be used and
+    // it may not match a transaction constructed from getCoinbaseTx().
     virtual CBlock getBlock() = 0;
 
     // Fees per transaction, not including coinbase transaction.
@@ -64,6 +65,10 @@ public:
      * @note unlike the submitblock RPC, this method does NOT add the
      *       coinbase witness automatically.
      *
+     * @note for heights <= 16, the BIP34 height push in getCoinbaseTx().script_sig_prefix
+     *       is only one byte long, so the coinbase scriptSig needs at least
+     *       one additional byte of data to avoid bad-cb-length.
+     *
      * @returns if the block was processed, does not necessarily indicate validity.
      *
      * @note Returns true if the block is already known, which can happen if
diff --git a/src/node/miner.cpp b/src/node/miner.cpp
index 836b828b..574e293a 100644
--- a/src/node/miner.cpp
+++ b/src/node/miner.cpp
@@ -184,14 +184,19 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock()
     // increasing its length would reduce the space they can use and may break
     // existing clients.
     coinbaseTx.vin[0].scriptSig = CScript() << nHeight;
-    if (m_options.include_dummy_extranonce) {
+    // Set script_sig_prefix here, so IPC mining clients are not affected by
+    // the optional scriptSig padding below. They provide their own extraNonce,
+    // and in a typical setup a pool name or realistic extraNonce already makes
+    // the scriptSig long enough.
+    coinbase_tx.script_sig_prefix = coinbaseTx.vin[0].scriptSig;
+    if (nHeight <= 16 || m_options.include_dummy_extranonce) {
         // For blocks at heights <= 16, the BIP34-encoded height alone is only
         // one byte. Consensus requires coinbase scriptSigs to be at least two
-        // bytes long (bad-cb-length), so tests and regtest include a dummy
-        // extraNonce (OP_0)
+        // bytes long (bad-cb-length), so an OP_0 is always appended at those
+        // heights. At greater heights it is only added when the caller
+        // requests it (e.g. RPC and test code that rely on stable hashes).
         coinbaseTx.vin[0].scriptSig << OP_0;
     }
-    coinbase_tx.script_sig_prefix = coinbaseTx.vin[0].scriptSig;
     Assert(nHeight > 0);
     coinbaseTx.nLockTime = static_cast<uint32_t>(nHeight - 1);
     coinbase_tx.lock_time = coinbaseTx.nLockTime;
@@ -221,7 +226,6 @@ std::unique_ptr<CBlockTemplate> BlockAssembler::CreateNewBlock()
     pblock->nNonce         = 0;
 
     if (m_options.test_block_validity) {
-        // if nHeight <= 16, and include_dummy_extranonce=false this will fail due to bad-cb-length.
         if (BlockValidationState state{TestBlockValidity(m_chainstate, *pblock, /*check_pow=*/false, /*check_merkle_root=*/false)}; !state.IsValid()) {
             throw std::runtime_error(strprintf("TestBlockValidity failed: %s", state.ToString()));
         }
diff --git a/src/node/types.h b/src/node/types.h
index 01e74528..cc0c4048 100644
--- a/src/node/types.h
+++ b/src/node/types.h
@@ -74,6 +74,9 @@ struct BlockCreateOptions {
     CScript coinbase_output_script{CScript() << OP_TRUE};
     /**
      * Whether to include an OP_0 as a dummy extraNonce in the template's coinbase
+     *
+     * TODO: this can be dropped after regenerating hardcoded block and transaction
+     *       hashes in the test suite.
      */
     bool include_dummy_extranonce{false};
 };
@@ -125,7 +128,9 @@ struct CoinbaseTx {
      * Prefix which needs to be placed at the beginning of the scriptSig.
      * Clients may append extra data to this as long as the overall scriptSig
      * size is 100 bytes or less, to avoid the block being rejected with
-     * "bad-cb-length" error.
+     * "bad-cb-length" error. At heights <= 16 the BIP 34 height push is only
+     * one byte long, so clients must append at least one additional byte to
+     * meet the consensus minimum scriptSig length of two bytes.
      *
      * Currently with BIP 34, the prefix is guaranteed to be less than 8 bytes,
      * but future soft forks could require longer prefixes.
diff --git a/test/functional/interface_ipc_mining.py b/test/functional/interface_ipc_mining.py
index 0f54bb92..2740662f 100755
--- a/test/functional/interface_ipc_mining.py
+++ b/test/functional/interface_ipc_mining.py
@@ -63,7 +63,7 @@ class IPCMiningTest(BitcoinTestFramework):
         # as it is being called before knowing whether capnp is available).
         self.capnp_modules = load_capnp_modules(self.config)
 
-    async def build_coinbase_test(self, template, ctx, miniwallet):
+    async def build_coinbase_test(self, template, ctx, miniwallet, extra_nonce=b""):
         self.log.debug("Build coinbase transaction using getCoinbaseTx()")
         assert template is not None
         coinbase_res = await mining_get_coinbase_tx(template, ctx)
@@ -79,7 +79,7 @@ class IPCMiningTest(BitcoinTestFramework):
         assert_equal(coinbase_res.scriptSigPrefix, bip34_prefix)
 
         # Typically a mining pool appends its name and an extraNonce
-        coinbase_tx.vin[0].scriptSig = coinbase_res.scriptSigPrefix
+        coinbase_tx.vin[0].scriptSig = coinbase_res.scriptSigPrefix + extra_nonce
 
         # We currently always provide a coinbase witness, even for empty
         # blocks, but this may change, so always check:
@@ -405,7 +405,12 @@ class IPCMiningTest(BitcoinTestFramework):
 
     def run_low_height_test(self):
         """Test that IPC createNewBlock() works at low block heights on a
-        clean chain. Currently fails with bad-cb-length and falls back to RPC."""
+        clean chain, in particular with regard to bad-cb-length.
+
+        createNewBlock pads the scriptSig with a dummy extranonce to pass
+        its internal CheckBlock(). This dummy is omitted from the
+        getCoinbaseTx() script_sig_prefix field. The client provides its
+        own extraNonce via submitSolution()."""
         self.log.info("Running low block height test")
 
         node = self.nodes[0]
@@ -416,29 +421,29 @@ class IPCMiningTest(BitcoinTestFramework):
         node.wait_for_rpc_connection()
         assert_equal(node.getblockcount(), 0)
 
+        miniwallet = MiniWallet(node)
+
         async def async_routine():
             ctx, mining = await make_mining_ctx(self)
             opts = self.capnp_modules['mining'].BlockCreateOptions()
 
-            # IPC createNewBlock() currently fails with bad-cb-length at heights
-            # <= 16, so use the generate RPC as a workaround. The next commit
-            # fixes this and replaces the loop body with a createNewBlock() /
-            # submitSolution() flow.
-            for i in range(17):
+            # Mine blocks 1-17 to exercise the boundary at height 16, where the
+            # internal scriptSig padding is no longer needed.
+            for height in range(1, 18):
                 async with AsyncExitStack() as stack:
-                    try:
-                        # Disable cooldown to avoid hanging in the IBD loop on a fresh chain
-                        template = await mining_create_block_template(mining, stack, ctx, opts, cooldown=False)
-                        assert template is not None
-                        # createNewBlock() only succeeds once the BIP34 height
-                        # encoding is >= 2 bytes, i.e. for height >= 17.
-                        if i != 16:
-                            raise AssertionError(f"createNewBlock() should have failed at height {i + 1}")
-                    except capnp.lib.capnp.KjException as e:
-                        assert i < 16
-                        assert_capnp_failed(e, "remote exception: std::exception: TestBlockValidity failed: bad-cb-length")
-                    self.generate(node, 1, sync_fun=self.no_op)
-                    assert_equal(node.getblockcount(), i + 1)
+                    # Disable cooldown to avoid hanging in the IBD loop on a fresh chain
+                    template = await mining_create_block_template(mining, stack, ctx, opts, cooldown=False)
+                    assert template is not None
+                    block = await mining_get_block(template, ctx)
+                    # Heights <= 16 need extra nonce padding.
+                    extra_nonce = b'\xaa\xbb\xcc\xdd' if height <= 16 else b""
+                    coinbase = await self.build_coinbase_test(template, ctx, miniwallet, extra_nonce=extra_nonce)
+                    block.vtx[0] = coinbase
+                    block.hashMerkleRoot = block.calc_merkle_root()
+                    block.solve()
+                    submitted = (await template.submitSolution(ctx, block.nVersion, block.nTime, block.nNonce, coinbase.serialize())).result
+                    assert_equal(submitted, True)
+                    assert_equal(node.getblockcount(), height)
 
         asyncio.run(capnp.run(async_routine()))