When an IPC client requests a new block template via the Mining interface, we hold on to its CBlock. That way when they call submitSolution() we can modify it in place, rather than having to reconstruct the full block like the submitblock RPC does.
Before this commit however we forgot to invalidate m_checked_witness_commitment, which we should since the client brings a new coinbase.
This would cause us to accept an invalid chaintip.
Fix this and add a test to confirm that we now reject such a block. As a sanity check, we add a second node to the test and confirm that will accept our mined block.
As first noticed in #33374 the IPC code takes the coinbase as provided, unlike the submitblock RPC which calls UpdateUncommittedBlockStructures() and adds witness commitment to the coinbase if it was missing.
Although that could have been an alternative fix, we instead document that IPC clients are expected to provide the full coinbase including witness commitment.
Patch to produce the original issue:
0diff --git a/src/node/miner.cpp b/src/node/miner.cpp
1index b988e28a3f..28e9048a4d 100644
2--- a/src/node/miner.cpp
3+++ b/src/node/miner.cpp
4@@ -450,15 +450,10 @@ void AddMerkleRootAndCoinbase(CBlock& block, CTransactionRef coinbase, uint32_t
5 }
6 block.nVersion = version;
7 block.nTime = timestamp;
8 block.nNonce = nonce;
9 block.hashMerkleRoot = BlockMerkleRoot(block);
10-
11- // Reset cached checks
12- block.m_checked_witness_commitment = false;
13- block.m_checked_merkle_root = false;
14- block.fChecked = false;
15 }
16
17 std::unique_ptr<CBlockTemplate> WaitAndCreateNewBlock(ChainstateManager& chainman,
18 KernelNotifications& kernel_notifications,
19 CTxMemPool* mempool,
20diff --git a/test/functional/interface_ipc.py b/test/functional/interface_ipc.py
21index cce56e3294..bf1b7048ab 100755
22--- a/test/functional/interface_ipc.py
23+++ b/test/functional/interface_ipc.py
24@@ -216,22 +216,22 @@ class IPCInterfaceTest(BitcoinTestFramework):
25 assert_equal(res.result, True)
26
27 # The remote template block will be mutated, capture the original:
28 remote_block_before = await self.parse_and_deserialize_block(template, ctx)
29
30- self.log.debug("Submitted coinbase must include witness")
31+ self.log.debug("Submitted coinbase with missing witness is accepted")
32 assert_not_equal(coinbase.serialize_without_witness().hex(), coinbase.serialize().hex())
33 res = await template.result.submitSolution(ctx, block.nVersion, block.nTime, block.nNonce, coinbase.serialize_without_witness())
34- assert_equal(res.result, False)
35+ assert_equal(res.result, True)
36
37 self.log.debug("Even a rejected submitBlock() mutates the template's block")
38 # Can be used by clients to download and inspect the (rejected)
39 # reconstructed block.
40 remote_block_after = await self.parse_and_deserialize_block(template, ctx)
41 assert_not_equal(remote_block_before.serialize().hex(), remote_block_after.serialize().hex())
42
43- self.log.debug("Submit again, with the witness")
44+ self.log.debug("Submit again, with the witness - does not replace the invalid block")
45 res = await template.result.submitSolution(ctx, block.nVersion, block.nTime, block.nNonce, coinbase.serialize())
46 assert_equal(res.result, True)
47
48 self.log.debug("Block should propagate")
49 assert_equal(self.nodes[1].getchaintips()[0]["height"], current_block_height + 1)
