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

Merge bitcoin/bitcoin#34568: mining: Break compatibility with existing IPC mining clients

Browse Source 
f700609e8a doc: Release notes for mining IPC interface bump (Ryan Ofsky)
9453c15361 ipc mining: break compatibility with existing clients (version bump) (Sjors Provoost)
70de5cc2d2 ipc mining: pass missing context to BlockTemplate methods (incompatible schema change) (Sjors Provoost)
2278f017af ipc mining: remove deprecated methods (incompatible schema change) (Ryan Ofsky)
c6638fa7c5 ipc mining: provide default option values (incompatible schema change) (Ryan Ofsky)
a4603ac774 ipc mining: declare constants for default field values (Ryan Ofsky)
ff995b50cf ipc test: add workaround to block_reserved_weight exception test (Ryan Ofsky)
b970cdf20f test framework: expand expected_stderr, expected_ret_code options (Ryan Ofsky)
df53a3e5ec rpc refactor: stop using deprecated getCoinbaseCommitment method (Ryan Ofsky)

Pull request description:

  This PR increments the field number of the `Init.makeMining` method and makes the old `makeMining` method return an error, so IPC mining clients not using the latest schema file will get an error and not be able to access the Mining interface.

  Normally, there shouldn't be a need to break compatibility this way, but the mining interface has evolved a lot since it was first introduced, with old clients using the original methods less stable and performant than newer clients. So now is a good time to introduce a cutoff, drop deprecated methods, and stop supporting old clients which can't function as well.

  Bumping the field number is also an opportunity to make other improvements that would be awkward to implement compatibly:
  - Making Cap'n Proto default parameter and field values match default values of corresponding C++ methods and structs.
  - Adding missing Context parameters to Mining.createNewBlock and checkBlock methods so these methods will be executed on separate execution threads and not block the Cap'n Proto event loop thread.

  More details about these changes are in the commit messages.

ACKs for top commit:
  Sjors:
    ACK f700609e8a
  enirox001:
    ACK f700609e8a
  ismaelsadeeq:
    ACK f700609e8a
  sedited:
    ACK f700609e8a

Tree-SHA512: 0901886af00214c138643b33cec21647de5671dfff2021afe06d78dfd970664a844cde9a1e28f685bb27edccaf6e0c3f2d1e6bb4164bde6b84f42955946e366d
This commit is contained in:
merge-script
2026-02-20 11:06:06 +01:00
parent 641a1954f7 f700609e8a
commit cb3473a680
19 changed files with 103 additions and 122 deletions
Download Patch File Download Diff File Expand all files Collapse all files

14
test/functional/interface_ipc.py
View File

@@ -68,9 +68,23 @@ class IPCInterfaceTest(BitcoinTestFramework):
asyncio.run(capnp.run(async_routine()))
def run_deprecated_mining_test(self):
self.log.info("Running deprecated mining interface test")
async def async_routine():
ctx, init = await make_capnp_init_ctx(self)
self.log.debug("Calling deprecated makeMiningOld2 should raise an error")
try:
await init.makeMiningOld2()
raise AssertionError("makeMiningOld2 unexpectedly succeeded")
except capnp.KjException as e:
assert_equal(e.description, "remote exception: std::exception: Old mining interface (@2) not supported. Please update your client!")
assert_equal(e.type, "FAILED")
asyncio.run(capnp.run(async_routine()))
def run_test(self):
self.run_echo_test()
self.run_mining_test()
self.run_deprecated_mining_test()
if __name__ == '__main__':
IPCInterfaceTest(__file__).main()

58
test/functional/interface_ipc_mining.py
View File

@@ -6,6 +6,7 @@
import asyncio
from contextlib import AsyncExitStack
from io import BytesIO
import re
from test_framework.blocktools import NULL_OUTPOINT
from test_framework.messages import (
MAX_BLOCK_WEIGHT,
@@ -33,7 +34,6 @@ from test_framework.ipc_util import (
make_capnp_init_ctx,
mining_get_block,
mining_get_coinbase_tx,
mining_get_coinbase_raw_tx,
mining_wait_next_template,
wait_and_do,
)
@@ -92,27 +92,12 @@ class IPCMiningTest(BitcoinTestFramework):
coinbase_tx.vout[0].nValue = coinbase_res.blockRewardRemaining
# Add SegWit OP_RETURN. This is currently always present even for
# empty blocks, but this may change.
found_witness_op_return = False
# Compare SegWit OP_RETURN to getCoinbaseCommitment()
coinbase_commitment = (await template.getCoinbaseCommitment(ctx)).result
for output_data in coinbase_res.requiredOutputs:
output = CTxOut()
output.deserialize(BytesIO(output_data))
coinbase_tx.vout.append(output)
if output.scriptPubKey == coinbase_commitment:
found_witness_op_return = True
assert_equal(has_witness, found_witness_op_return)
coinbase_tx.nLockTime = coinbase_res.lockTime
# Compare to dummy coinbase transaction provided by the deprecated
# getCoinbaseRawTx()
coinbase_legacy = await mining_get_coinbase_raw_tx(template, ctx)
assert_equal(coinbase_legacy.vout[0].nValue, coinbase_res.blockRewardRemaining)
# Swap dummy output for our own
coinbase_legacy.vout[0].scriptPubKey = coinbase_tx.vout[0].scriptPubKey
assert_equal(coinbase_tx.serialize().hex(), coinbase_legacy.serialize().hex())
return coinbase_tx
async def make_mining_ctx(self):
@@ -242,9 +227,6 @@ class IPCMiningTest(BitcoinTestFramework):
async with AsyncExitStack() as stack:
opts = self.capnp_modules['mining'].BlockCreateOptions()
opts.useMempool = True
opts.blockReservedWeight = 4000
opts.coinbaseOutputMaxAdditionalSigops = 0
template = await mining_create_block_template(mining, stack, ctx, opts)
assert template is not None
block = await mining_get_block(template, ctx)
@@ -257,19 +239,28 @@ class IPCMiningTest(BitcoinTestFramework):
empty_block = await mining_get_block(empty_template, ctx)
assert_equal(len(empty_block.vtx), 1)
self.log.debug("Enforce minimum reserved weight for IPC clients too")
opts.blockReservedWeight = 0
try:
await mining.createNewBlock(opts)
raise AssertionError("createNewBlock unexpectedly succeeded")
except capnp.lib.capnp.KjException as e:
self.log.debug("Enforce minimum reserved weight for IPC clients too")
opts.blockReservedWeight = 0
try:
await mining.createNewBlock(ctx, opts)
raise AssertionError("createNewBlock unexpectedly succeeded")
except capnp.lib.capnp.KjException as e:
if e.type == "DISCONNECTED":
# The remote exception isn't caught currently and leads to a
# std::terminate call. Just detect and restart in this case.
# This bug is fixed with
# https://github.com/bitcoin-core/libmultiprocess/pull/218
assert_equal(e.description, "Peer disconnected.")
self.nodes[0].wait_until_stopped(expected_ret_code=(-11, -6, 1, 66), expected_stderr=re.compile(""))
self.start_node(0)
else:
assert_equal(e.description, "remote exception: std::exception: block_reserved_weight (0) must be at least 2000 weight units")
assert_equal(e.type, "FAILED")
asyncio.run(capnp.run(async_routine()))
def run_coinbase_and_submission_test(self):
"""Test coinbase construction (getCoinbaseTx, getCoinbaseCommitment) and block submission (submitSolution)."""
"""Test coinbase construction (getCoinbaseTx) and block submission (submitSolution)."""
self.log.info("Running coinbase construction and submission test")
async def async_routine():
@@ -278,7 +269,7 @@ class IPCMiningTest(BitcoinTestFramework):
current_block_height = self.nodes[0].getchaintips()[0]["height"]
check_opts = self.capnp_modules['mining'].BlockCheckOptions()
async with destroying((await mining.createNewBlock(self.default_block_create_options)).result, ctx) as template:
async with destroying((await mining.createNewBlock(ctx, self.default_block_create_options)).result, ctx) as template:
block = await mining_get_block(template, ctx)
balance = self.miniwallet.get_balance()
coinbase = await self.build_coinbase_test(template, ctx, self.miniwallet)
@@ -291,7 +282,7 @@ class IPCMiningTest(BitcoinTestFramework):
self.log.debug("Submit a block with a bad version")
block.nVersion = 0
block.solve()
check = await mining.checkBlock(block.serialize(), check_opts)
check = await mining.checkBlock(ctx, block.serialize(), check_opts)
assert_equal(check.result, False)
assert_equal(check.reason, "bad-version(0x00000000)")
submitted = (await template.submitSolution(ctx, block.nVersion, block.nTime, block.nNonce, coinbase.serialize())).result
@@ -301,7 +292,7 @@ class IPCMiningTest(BitcoinTestFramework):
block.solve()
self.log.debug("First call checkBlock()")
block_valid = (await mining.checkBlock(block.serialize(), check_opts)).result
block_valid = (await mining.checkBlock(ctx, block.serialize(), check_opts)).result
assert_equal(block_valid, True)
# The remote template block will be mutated, capture the original:
@@ -333,7 +324,7 @@ class IPCMiningTest(BitcoinTestFramework):
self.miniwallet.rescan_utxos()
assert_equal(self.miniwallet.get_balance(), balance + 1)
self.log.debug("Check block should fail now, since it is a duplicate")
check = await mining.checkBlock(block.serialize(), check_opts)
check = await mining.checkBlock(ctx, block.serialize(), check_opts)
assert_equal(check.result, False)
assert_equal(check.reason, "inconclusive-not-best-prevblk")
@@ -341,12 +332,7 @@ class IPCMiningTest(BitcoinTestFramework):
def run_test(self):
self.miniwallet = MiniWallet(self.nodes[0])
self.default_block_create_options = self.capnp_modules['mining'].BlockCreateOptions(
useMempool=True,
blockReservedWeight=4000,
coinbaseOutputMaxAdditionalSigops=0
)
self.default_block_create_options = self.capnp_modules['mining'].BlockCreateOptions()
self.run_mining_interface_test()
self.run_block_template_test()
self.run_coinbase_and_submission_test()

12
test/functional/test_framework/ipc_util.py
View File

@@ -12,7 +12,7 @@ from pathlib import Path
import shutil
from typing import Optional
from test_framework.messages import CBlock, CTransaction
from test_framework.messages import CBlock
# Test may be skipped and not have capnp installed
try:
@@ -108,7 +108,7 @@ async def make_capnp_init_ctx(self):
async def mining_create_block_template(mining, stack, ctx, opts):
"""Call mining.createNewBlock() and return template, then call template.destroy() when stack exits."""
response = await mining.createNewBlock(opts)
response = await mining.createNewBlock(ctx, opts)
if not response._has("result"):
return None
return await stack.enter_async_context(destroying(response.result, ctx))
@@ -129,14 +129,6 @@ async def mining_get_block(block_template, ctx):
return block
async def mining_get_coinbase_raw_tx(block_template, ctx):
assert block_template is not None
coinbase_data = BytesIO((await block_template.getCoinbaseRawTx(ctx)).result)
tx = CTransaction()
tx.deserialize(coinbase_data)
return tx
async def mining_get_coinbase_tx(block_template, ctx) -> CoinbaseTxData:
assert block_template is not None
# Note: the template_capnp struct will be garbage-collected when this

16
test/functional/test_framework/test_node.py
View File

@@ -22,6 +22,7 @@ import collections
import shlex
import shutil
import sys
from collections.abc import Iterable
from pathlib import Path
from .authproxy import (
@@ -469,6 +470,12 @@ class TestNode():
"""Checks whether the node has stopped.
Returns True if the node has stopped. False otherwise.
If the process has exited, asserts that the exit code matches
`expected_ret_code` (which may be a single value or an iterable of values),
and that stderr matches `expected_stderr` exactly or, if a regex pattern is
provided, contains the pattern.
This method is responsible for freeing resources (self.process)."""
if not self.running:
return True
@@ -477,12 +484,17 @@ class TestNode():
return False
# process has stopped. Assert that it didn't return an error code.
assert return_code == expected_ret_code, self._node_msg(
if not isinstance(expected_ret_code, Iterable):
expected_ret_code = (expected_ret_code,)
assert return_code in expected_ret_code, self._node_msg(
f"Node returned unexpected exit code ({return_code}) vs ({expected_ret_code}) when stopping")
# Check that stderr is as expected
self.stderr.seek(0)
stderr = self.stderr.read().decode('utf-8').strip()
if stderr != expected_stderr:
if isinstance(expected_stderr, re.Pattern):
if not expected_stderr.search(stderr):
raise AssertionError(f"Unexpected stderr {stderr!r} does not contain {expected_stderr.pattern!r}")
elif stderr != expected_stderr:
raise AssertionError("Unexpected stderr {} != {}".format(stderr, expected_stderr))
self.stdout.close()