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

Merge bitcoin/bitcoin#33966: refactor: disentangle miner startup defaults from runtime options

Browse Source 
1e5d3b4f0d doc: add release note for mining option validation (Sjors Provoost)
0317f52022 ci: enforce iwyu for touched files (Sjors Provoost)
8c58f63578 refactor: have mining files include what they use (Sjors Provoost)
3bb6498fb0 mining: store block create options in NodeContext (Sjors Provoost)
4637cd157d mining: reject invalid block create options (Sjors Provoost)
8daac1d6eb mining: add block create option helpers (Sjors Provoost)
128da7c3ff miner: add block_max_weight to BlockCreateOptions (Sjors Provoost)
fa81e51eae mining: parse block creation args in mining_args (Sjors Provoost)
020166080c mining: use interface for tests, bench and fuzzers (Sjors Provoost)
44082bea47 interfaces: make Mining use const NodeContext (Sjors Provoost)
d4368e059c move-only: add node/mining_types.h (Sjors Provoost)
6aeb1fbea2 test: cover IPC blockmaxweight policy (Sjors Provoost)
63b23ea1e9 test: regression test for waitNext mining policy (Sjors Provoost)
24750f8b31 test: add createNewBlock failure helper (Sjors Provoost)
63ee9cd15b test: misc interface_ipc_mining.py improvements (Sjors Provoost)

Pull request description:

  Although this PR is primarily a refactor, _there are behavior changes_ documented in the release note:
  - the IPC mining interface now rejects out-of-range block template options instead of silently clamping them;
  - startup now rejects `-blockmaxweight` values lower than `-blockreservedweight`, instead of allowing them to be clamped later.

  The interaction between node startup options like `-blockreservedweight` and runtime options, especially those passed via IPC, is confusing.

  They're combined in `BlockAssembler::Options`, which this PR gets rid of in favour of `BlockCreateOptions`.

  `BlockCreateOptions` is used by interface clients. As before, IPC clients have access to a safe / sane subset, whereas RPC and test code can use all fields. The same type is also used to store mining defaults parsed once during node startup in `NodeContext`.

  The maximum block weight setting (`block_max_weight`) is optional. When read from startup options it matches `-blockmaxweight`; when provided by callers it is a runtime override. `Merge()` fills unset fields from startup defaults while preserving caller-provided values.

  This all happens in commits `mining: add block create option helpers` and `mining: store block create options in NodeContext`, and requires some preparation to keep things easy to review.

  We get rid of `BlockAssembler::Options` but this is used in many tests. Since large churn is inevitable, we might as well switch all tests, bench and fuzzers over to the Mining interface. The `mining: use interface for tests, bench and fuzzers` commit does that, dramatically reducing direct use of `BlockAssembler`. Two exceptions are documented in the commit message. Because `test_block_validity` wasn't available via the interface and the block_assemble benchmark needs it, it's moved from `BlockAssembler::Options` to `BlockCreateOptions` (still not exposed via IPC).

  We need access to mining related structs from both the miner and node initialization code. To avoid having to pull in all of `BlockAssembler` for the latter, the `move-only: add node/mining_types.h` commit introduces `node/mining_types.h` and moves `BlockCreateOptions`, `BlockWaitOptions` and `BlockCheckOptions` there from `src/node/types.h`.

  I considered also moving `DEFAULT_BLOCK_MAX_WEIGHT`, `DEFAULT_BLOCK_RESERVED_WEIGHT`, `MINIMUM_BLOCK_RESERVED_WEIGHT` and `DEFAULT_BLOCK_MIN_TX_FEE` there from `policy.h`, since they are distinct from relay policy and not needed by the kernel. But this seems more appropriate for a follow-up and requires additional discussion.

  ---

  I kept variable renaming and other formatting changes to a minimum to ease review with `--color-moved=dimmed-zebra`.

  ## Commit summary

  Tests and test cleanup:
  - `test: misc interface_ipc_mining.py improvements`
  - `test: add assert_create_fails helper`
  - `test: regression test for waitNext mining policy`
  - `test: cover IPC blockmaxweight policy`

  Refactoring test/bench/fuzz callers:
  - `interfaces: make Mining use const NodeContext`
  - `mining: use interface for tests, bench and fuzzers`

  Moving mining interface types:
  - `move-only: add node/mining_types.h`

  Separating startup defaults from runtime options:
  - `mining: parse block creation args in mining_args`: adds `node/mining_args.{h,cpp}` and moves mining option parsing out of `init.cpp`, without storing the parsed values yet.
  - `miner: add block_max_weight to BlockCreateOptions`: moves the runtime maximum block weight setting into `BlockCreateOptions` as an optional value, so it can later be defaulted from startup args when unset.
  - `mining: add block create option helpers`: centralizes block template option defaulting and merging, removes `BlockAssembler::Options`, and preserves behavior except for dropping the `Specified ` prefix from startup option error messages.
  - `mining: reject invalid block create options`: checks typed `BlockCreateOptions` before block template creation, so invalid runtime options are rejected instead of silently clamped. Startup validation also rejects `-blockmaxweight` values lower than `-blockreservedweight`.
  - `mining: store block create options in NodeContext`: stores the startup mining options in `NodeContext` as `BlockCreateOptions`, so startup defaults and runtime overrides can be merged with the same option type.

  Include hygiene, CI and release note:
  - `refactor: have mining files include what they use`
  - `ci: enforce iwyu for touched files`
  - `doc: add release note for mining option validation`

ACKs for top commit:
  w0xlt:
    reACK 1e5d3b4f0d
  sedited:
    ACK 1e5d3b4f0d
  ryanofsky:
    Code review ACK 1e5d3b4f0d. Looks good, thanks for the updates!

Tree-SHA512: 28c715023cb78f02775caa787b243c994bd0f8ce4559afc8db9301e93400ebbc74963626a4afe65ae15bcc16b9192d051a745839f4c804848d50746ea5a224b4
This commit is contained in:
Ryan Ofsky
2026-05-26 08:14:01 -04:00
parent ac9424fdc6 1e5d3b4f0d
commit a4157fc24a
40 changed files with 1118 additions and 505 deletions
Download Patch File Download Diff File Expand all files Collapse all files

169
test/functional/interface_ipc_mining.py
View File

@@ -6,38 +6,42 @@
import asyncio
import time
from contextlib import AsyncExitStack
from decimal import Decimal
from io import BytesIO
from test_framework.blocktools import NULL_OUTPOINT, script_BIP34_coinbase_height
from test_framework.messages import (
MAX_BLOCK_WEIGHT,
CBlockHeader,
COIN,
CTransaction,
CTxIn,
CTxOut,
CTxInWitness,
ser_uint256,
COIN,
CTxOut,
DEFAULT_BLOCK_RESERVED_WEIGHT,
MAX_BLOCK_SIGOPS_COST,
MAX_BLOCK_WEIGHT,
from_hex,
msg_headers,
ser_uint256,
)
from test_framework.test_framework import BitcoinTestFramework
from test_framework.util import (
assert_equal,
assert_greater_than_or_equal,
assert_not_equal
assert_not_equal,
)
from test_framework.wallet import MiniWallet
from test_framework.p2p import P2PInterface
from test_framework.ipc_util import (
assert_capnp_failed,
assert_create_new_block_fails,
destroying,
mining_create_block_template,
load_capnp_modules,
make_mining_ctx,
mining_create_block_template,
mining_get_block,
mining_get_coinbase_tx,
mining_wait_next_template,
wait_and_do,
make_mining_ctx,
assert_capnp_failed
)
# Test may be skipped and not have capnp installed
@@ -291,8 +295,9 @@ class IPCMiningTest(BitcoinTestFramework):
def run_ipc_option_override_test(self):
self.log.info("Running IPC option override test")
# Set an absurd reserved weight. `-blockreservedweight` is RPC-only, so
# with this setting RPC templates would be empty. IPC clients set
# Confirm that BlockCreateOptions.blockReservedWeight takes precedence
# over -blockreservedweight. Set an absurdly high -blockreservedweight
# value that would result in empty blocks to verify this. IPC clients set
# blockReservedWeight per template request and are unaffected; later in
# the test the IPC template includes a mempool transaction.
self.restart_node(0, extra_args=[f"-blockreservedweight={MAX_BLOCK_WEIGHT}"])
@@ -317,11 +322,136 @@ class IPCMiningTest(BitcoinTestFramework):
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:
assert_capnp_failed(e, "remote exception: std::exception: block_reserved_weight (0) must be at least 2000 weight units")
await assert_create_new_block_fails(ctx, mining, opts,
"block_reserved_weight (0) is lower than minimum safety value of (2000)")
async def async_routine_check_max_reserved_weight():
self.log.debug("Enforce maximum reserved weight for IPC clients too")
ctx, mining = await make_mining_ctx(self)
opts = self.capnp_modules['mining'].BlockCreateOptions()
opts.blockReservedWeight = MAX_BLOCK_WEIGHT + 1
await assert_create_new_block_fails(ctx, mining, opts,
f"block_reserved_weight ({MAX_BLOCK_WEIGHT + 1}) exceeds consensus maximum block weight ({MAX_BLOCK_WEIGHT})")
async def async_routine_check_sigops_limit():
self.log.debug("Enforce sigops limit for IPC clients too")
ctx, mining = await make_mining_ctx(self)
opts = self.capnp_modules['mining'].BlockCreateOptions()
opts.coinbaseOutputMaxAdditionalSigops = MAX_BLOCK_SIGOPS_COST + 1
await assert_create_new_block_fails(ctx, mining, opts,
f"coinbase_output_max_additional_sigops ({MAX_BLOCK_SIGOPS_COST + 1}) exceeds consensus maximum block sigops cost ({MAX_BLOCK_SIGOPS_COST})")
asyncio.run(capnp.run(async_routine()))
asyncio.run(capnp.run(async_routine_check_max_reserved_weight()))
asyncio.run(capnp.run(async_routine_check_sigops_limit()))
def run_waitnext_mining_policy_test(self):
"""Verify that waitNext() preserves the mining policy from -blockmintxfee
instead of falling back to defaults."""
self.log.info("Running waitNext mining policy test")
block_min_tx_fee = Decimal("0.00002000")
below_block_min_tx_fee = Decimal("0.00001000")
above_block_min_tx_fee = Decimal("0.00003000")
self.restart_node(0, extra_args=[
f"-blockmintxfee={block_min_tx_fee:.8f}",
"-minrelaytxfee=0",
"-persistmempool=0",
])
async def async_routine():
ctx, mining = await make_mining_ctx(self)
self.log.debug("Create a below -blockmintxfee transaction")
low_fee_tx = self.miniwallet.send_self_transfer(
fee_rate=below_block_min_tx_fee,
from_node=self.nodes[0],
confirmed_only=True,
)
assert low_fee_tx["txid"] in self.nodes[0].getrawmempool()
async with AsyncExitStack() as stack:
self.log.debug("createNewBlock should respect -blockmintxfee")
template = await mining_create_block_template(mining, stack, ctx, self.default_block_create_options)
assert template is not None
block = await mining_get_block(template, ctx)
assert low_fee_tx["txid"] not in {tx.txid_hex for tx in block.vtx[1:]}
self.log.debug("waitNext should preserve the same mining policy")
high_fee_tx = self.miniwallet.send_self_transfer(
fee_rate=above_block_min_tx_fee,
from_node=self.nodes[0],
confirmed_only=True,
)
mempool_txids = self.nodes[0].getrawmempool()
assert high_fee_tx["txid"] in mempool_txids
assert low_fee_tx["txid"] in mempool_txids
template_next = await mining_wait_next_template(template, stack, ctx, self.default_block_wait_options)
assert template_next is not None
block_next = await mining_get_block(template_next, ctx)
block_next_txids = {tx.txid_hex for tx in block_next.vtx[1:]}
assert high_fee_tx["txid"] in block_next_txids
assert low_fee_tx["txid"] not in block_next_txids
asyncio.run(capnp.run(async_routine()))
def run_block_max_weight_test(self):
"""Verify IPC createNewBlock() and waitNext() preserve the -blockmaxweight policy."""
self.log.info("Running block_max_weight test")
# Cap that leaves room for only a handful of mempool transactions
# above DEFAULT_BLOCK_RESERVED_WEIGHT (8000). Well below MAX_BLOCK_WEIGHT
# (4_000_000), so any truncation observed here is attributable to the
# cap, not to consensus limits or wallet chain limits.
small_cap = DEFAULT_BLOCK_RESERVED_WEIGHT + 4000
NUM_TXS = 20
self.restart_node(0, extra_args=[
f"-blockmaxweight={small_cap}",
"-minrelaytxfee=0",
"-persistmempool=0",
])
# Refresh miniwallet's UTXO view from the chain after restart.
self.miniwallet.rescan_utxos()
# Fill the mempool enough that the configured block weight cap forces
# template truncation.
for _ in range(NUM_TXS):
self.miniwallet.send_self_transfer(from_node=self.nodes[0], confirmed_only=True)
assert_equal(self.nodes[0].getmempoolinfo()["size"], NUM_TXS)
async def async_routine():
ctx, mining = await make_mining_ctx(self)
async with AsyncExitStack() as stack:
template = await mining_create_block_template(mining, stack, ctx, self.default_block_create_options)
assert template is not None
block = await mining_get_block(template, ctx)
assert_greater_than_or_equal(small_cap, block.get_weight())
# Exclude the coinbase; the cap must have forced truncation.
initial_included = len(block.vtx) - 1
assert initial_included < NUM_TXS, (
f"Expected -blockmaxweight={small_cap} to truncate; "
f"included {initial_included}/{NUM_TXS} mempool txs"
)
self.log.debug("waitNext should preserve -blockmaxweight")
high_fee_tx = self.miniwallet.send_self_transfer(
from_node=self.nodes[0],
confirmed_only=True,
fee_rate=10,
)
template_next = await mining_wait_next_template(template, stack, ctx, self.default_block_wait_options)
assert template_next is not None
block_next = await mining_get_block(template_next, ctx)
assert_greater_than_or_equal(small_cap, block_next.get_weight())
assert high_fee_tx["txid"] in {tx.txid_hex for tx in block_next.vtx[1:]}
next_included = len(block_next.vtx) - 1
assert next_included < NUM_TXS + 1, (
f"Expected -blockmaxweight={small_cap} to remain capped after waitNext; "
f"included {next_included}/{NUM_TXS + 1} mempool txs"
)
asyncio.run(capnp.run(async_routine()))
@@ -421,8 +551,6 @@ 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()
@@ -437,7 +565,7 @@ class IPCMiningTest(BitcoinTestFramework):
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)
coinbase = await self.build_coinbase_test(template, ctx, self.miniwallet, extra_nonce=extra_nonce)
block.vtx[0] = coinbase
block.hashMerkleRoot = block.calc_merkle_root()
block.solve()
@@ -450,10 +578,15 @@ class IPCMiningTest(BitcoinTestFramework):
def run_test(self):
self.miniwallet = MiniWallet(self.nodes[0])
self.default_block_create_options = self.capnp_modules['mining'].BlockCreateOptions()
self.default_block_wait_options = self.capnp_modules['mining'].BlockWaitOptions()
self.default_block_wait_options.timeout = 1000.0 * self.options.timeout_factor
self.default_block_wait_options.feeThreshold = 1
self.run_mining_interface_test()
self.run_early_startup_test()
self.run_block_template_test()
self.run_coinbase_and_submission_test()
self.run_waitnext_mining_policy_test()
self.run_block_max_weight_test()
self.run_ipc_option_override_test()
# Needs to run last because it resets the chain.

13
test/functional/mining_basic.py
View File

@@ -357,21 +357,28 @@ class MiningTest(BitcoinTestFramework):
self.stop_node(0)
self.nodes[0].assert_start_raises_init_error(
extra_args=[f"-blockreservedweight={MAX_BLOCK_WEIGHT + 1}"],
expected_msg=f"Error: Specified -blockreservedweight ({MAX_BLOCK_WEIGHT + 1}) exceeds consensus maximum block weight ({MAX_BLOCK_WEIGHT})",
expected_msg=f"Error: -blockreservedweight ({MAX_BLOCK_WEIGHT + 1}) exceeds consensus maximum block weight ({MAX_BLOCK_WEIGHT})",
)
self.log.info(f"Test that node will fail to start when user provide -blockreservedweight below {MINIMUM_BLOCK_RESERVED_WEIGHT}")
self.stop_node(0)
self.nodes[0].assert_start_raises_init_error(
extra_args=[f"-blockreservedweight={MINIMUM_BLOCK_RESERVED_WEIGHT - 1}"],
expected_msg=f"Error: Specified -blockreservedweight ({MINIMUM_BLOCK_RESERVED_WEIGHT - 1}) is lower than minimum safety value of ({MINIMUM_BLOCK_RESERVED_WEIGHT})",
expected_msg=f"Error: -blockreservedweight ({MINIMUM_BLOCK_RESERVED_WEIGHT - 1}) is lower than minimum safety value of ({MINIMUM_BLOCK_RESERVED_WEIGHT})",
)
self.log.info("Test that node will fail to start when user provide invalid -blockmaxweight")
self.stop_node(0)
self.nodes[0].assert_start_raises_init_error(
extra_args=[f"-blockmaxweight={MAX_BLOCK_WEIGHT + 1}"],
expected_msg=f"Error: Specified -blockmaxweight ({MAX_BLOCK_WEIGHT + 1}) exceeds consensus maximum block weight ({MAX_BLOCK_WEIGHT})",
expected_msg=f"Error: -blockmaxweight ({MAX_BLOCK_WEIGHT + 1}) exceeds consensus maximum block weight ({MAX_BLOCK_WEIGHT})",
)
self.log.info("Test that node will fail to start when -blockmaxweight is lower than -blockreservedweight")
self.stop_node(0)
self.nodes[0].assert_start_raises_init_error(
extra_args=[f"-blockmaxweight={DEFAULT_BLOCK_RESERVED_WEIGHT - 1}"],
expected_msg=f"Error: -blockreservedweight ({DEFAULT_BLOCK_RESERVED_WEIGHT}) exceeds -blockmaxweight ({DEFAULT_BLOCK_RESERVED_WEIGHT - 1})",
)
def test_height_in_locktime(self):

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

@@ -162,3 +162,12 @@ async def make_mining_ctx(self):
def assert_capnp_failed(e, description_prefix):
assert e.description.startswith(description_prefix), f"Expected description starting with '{description_prefix}', got '{e.description}'"
assert_equal(e.type, "FAILED")
async def assert_create_new_block_fails(ctx, mining, opts, expected_msg):
"""Assert that mining.createNewBlock fails with the expected remote exception."""
try:
await mining.createNewBlock(ctx, opts)
raise AssertionError("createNewBlock unexpectedly succeeded")
except capnp.lib.capnp.KjException as e:
assert_capnp_failed(e, f"remote exception: std::exception: {expected_msg}")

1
test/functional/test_framework/messages.py
View File

@@ -36,6 +36,7 @@ from test_framework.util import (
MAX_LOCATOR_SZ = 101
MAX_BLOCK_WEIGHT = 4000000
MAX_BLOCK_SIGOPS_COST = 80000
DEFAULT_BLOCK_RESERVED_WEIGHT = 8000
MINIMUM_BLOCK_RESERVED_WEIGHT = 2000
MAX_BLOOM_FILTER_SIZE = 36000