highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2026-04-27 15:28:49 +02:00
Code Issues Packages Projects Releases Wiki Activity

mining: ensure witness commitment check in submitBlock

Browse Source 
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.

Note that 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.
This commit is contained in:
Sjors Provoost
2025-10-31 11:53:12 +01:00
parent 00d1b6ef4b
commit 862bd43283
3 changed files with 43 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files

27
test/functional/interface_ipc.py
View File

@@ -8,7 +8,11 @@ from io import BytesIO
from pathlib import Path
import shutil
from test_framework.messages import (CBlock, CTransaction, ser_uint256, COIN)
from test_framework.test_framework import (BitcoinTestFramework, assert_equal)
from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import (
assert_equal,
assert_not_equal
)
from test_framework.wallet import MiniWallet
# Test may be skipped and not have capnp installed
@@ -49,10 +53,10 @@ class IPCInterfaceTest(BitcoinTestFramework):
}
def set_test_params(self):
self.num_nodes = 1
self.num_nodes = 2
def setup_nodes(self):
self.extra_init = [{"ipcbind": True}]
self.extra_init = [{"ipcbind": True}, {}]
super().setup_nodes()
# Use this function to also load the capnp modules (we cannot use set_test_params for this,
# as it is being called before knowing whether capnp is available).
@@ -206,11 +210,26 @@ class IPCInterfaceTest(BitcoinTestFramework):
self.log.debug("Submit a valid block")
block.nVersion = original_version
block.solve()
self.log.debug("First call checkBlock()")
res = await mining.result.checkBlock(block.serialize(), check_opts)
assert_equal(res.result, True)
self.log.debug("Submitted coinbase must include witness")
assert_not_equal(coinbase.serialize_without_witness().hex(), coinbase.serialize().hex())
res = await template.result.submitSolution(ctx, block.nVersion, block.nTime, block.nNonce, coinbase.serialize_without_witness())
assert_equal(res.result, False)
self.log.debug("Submit again, with the witness")
res = await template.result.submitSolution(ctx, block.nVersion, block.nTime, block.nNonce, coinbase.serialize())
assert_equal(res.result, True)
assert_equal(self.nodes[0].getchaintips()[0]["height"], current_block_height + 1)
self.log.debug("Block should propagate")
assert_equal(self.nodes[1].getchaintips()[0]["height"], current_block_height + 1)
# Stalls if a regression causes submitBlock() to accept an invalid block:
self.sync_all()
assert_equal(self.nodes[0].getchaintips()[0], self.nodes[1].getchaintips()[0])
miniwallet.rescan_utxos()
assert_equal(miniwallet.get_balance(), balance + 1)
self.log.debug("Check block should fail now, since it is a duplicate")