From 418b7995ddfbc88764f1f0ceabf8993808b08bd8 Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Fri, 19 Dec 2025 15:13:21 -0600 Subject: [PATCH 1/3] test: have mining template helpers return None Refactor the mining_create_block_template and mining_wait_next_template helpers in ipc_util.py to return None if they time out or fail. It makes the test easier to read and provides a more clear error message in case of a regression. There were a few spots that didn't use mining_wait_next_template yet, which now do. --- test/functional/interface_ipc_mining.py | 17 +++++++++++------ test/functional/test_framework/ipc_util.py | 10 ++++++++-- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/test/functional/interface_ipc_mining.py b/test/functional/interface_ipc_mining.py index 20f90490514..876a9231a0f 100755 --- a/test/functional/interface_ipc_mining.py +++ b/test/functional/interface_ipc_mining.py @@ -160,6 +160,7 @@ class IPCMiningTest(BitcoinTestFramework): async with AsyncExitStack() as stack: self.log.debug("Create a template") template = await mining_create_block_template(mining, stack, ctx, self.default_block_create_options) + assert template is not None self.log.debug("Test some inspectors of Template") header = (await template.getBlockHeader(ctx)).result @@ -179,23 +180,26 @@ class IPCMiningTest(BitcoinTestFramework): template2 = await wait_and_do( mining_wait_next_template(template, stack, ctx, waitoptions), lambda: self.generate(self.nodes[0], 1)) + assert template2 is not None block2 = await mining_get_block(template2, ctx) assert_equal(len(block2.vtx), 1) self.log.debug("Wait for another, but time out") - template3 = await template2.waitNext(ctx, waitoptions) - assert_equal(template3._has("result"), False) + template3 = await mining_wait_next_template(template2, stack, ctx, waitoptions) + assert template3 is None self.log.debug("Wait for another, get one after increase in fees in the mempool") template4 = await wait_and_do( mining_wait_next_template(template2, stack, ctx, waitoptions), lambda: self.miniwallet.send_self_transfer(fee_rate=10, from_node=self.nodes[0])) + assert template4 is not None block3 = await mining_get_block(template4, ctx) assert_equal(len(block3.vtx), 2) self.log.debug("Wait again, this should return the same template, since the fee threshold is zero") waitoptions.feeThreshold = 0 template5 = await mining_wait_next_template(template4, stack, ctx, waitoptions) + assert template5 is not None block4 = await mining_get_block(template5, ctx) assert_equal(len(block4.vtx), 2) waitoptions.feeThreshold = 1 @@ -204,20 +208,21 @@ class IPCMiningTest(BitcoinTestFramework): template6 = await wait_and_do( mining_wait_next_template(template5, stack, ctx, waitoptions), lambda: self.miniwallet.send_self_transfer(fee_rate=10, from_node=self.nodes[0])) + assert template6 is not None block4 = await mining_get_block(template6, ctx) assert_equal(len(block4.vtx), 3) self.log.debug("Wait for another, but time out, since the fee threshold is set now") - template7 = await template6.waitNext(ctx, waitoptions) - assert_equal(template7._has("result"), False) + template7 = await mining_wait_next_template(template6, stack, ctx, waitoptions) + assert template7 is None self.log.debug("interruptWait should abort the current wait") async def wait_for_block(): new_waitoptions = self.capnp_modules['mining'].BlockWaitOptions() new_waitoptions.timeout = timeout * 60 # 1 minute wait new_waitoptions.feeThreshold = 1 - template7 = await template6.waitNext(ctx, new_waitoptions) - assert_equal(template7._has("result"), False) + template7 = await mining_wait_next_template(template6, stack, ctx, new_waitoptions) + assert template7 is None await wait_and_do(wait_for_block(), template6.interruptWait()) asyncio.run(capnp.run(async_routine())) diff --git a/test/functional/test_framework/ipc_util.py b/test/functional/test_framework/ipc_util.py index fc451faabc2..1b1ad7a55aa 100644 --- a/test/functional/test_framework/ipc_util.py +++ b/test/functional/test_framework/ipc_util.py @@ -108,12 +108,18 @@ 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.""" - return await stack.enter_async_context(destroying((await mining.createNewBlock(opts)).result, ctx)) + response = await mining.createNewBlock(opts) + if not response._has("result"): + return None + return await stack.enter_async_context(destroying(response.result, ctx)) async def mining_wait_next_template(template, stack, ctx, opts): """Call template.waitNext() and return template, then call template.destroy() when stack exits.""" - return await stack.enter_async_context(destroying((await template.waitNext(ctx, opts)).result, ctx)) + response = await template.waitNext(ctx, opts) + if not response._has("result"): + return None + return await stack.enter_async_context(destroying(response.result, ctx)) async def mining_get_block(block_template, ctx): From d3e49528d479613ebe50088d530a621861463fa4 Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Fri, 19 Dec 2025 15:18:33 -0600 Subject: [PATCH 2/3] mining: fix -blockreservedweight shadows IPC option The -blockreservedweight startup option should only affect RPC code, because IPC clients (currently) do not have a way to signal their intent to use the node default (the BlockCreateOptions struct defaults merely document a recommendation for client software). Before this commit however, if the user set -blockreservedweight then ApplyArgsManOptions would cause the block_reserved_weight option passed by IPC clients to be ignored. Users who don't set this value were not affected. Fix this by making BlockCreateOptions::block_reserved_weight an std::optional. Internal interface users, such as the RPC call sites, don't set a value so -blockreservedweight is used. Whereas IPC clients do set a value which is no longer ignored. Test coverage is added. mining_basic.py already ensured -blockreservedweight is enforced by mining RPC methods. This commit adds coverage for Mining interface IPC clients. It also verifies that -blockreservedweight has no effect on them. Co-Authored-By: Russell Yanofsky --- src/init.cpp | 2 +- src/node/miner.cpp | 11 ++++++--- src/node/types.h | 6 ++++- test/functional/interface_ipc_mining.py | 33 +++++++++++++++++++++++++ 4 files changed, 46 insertions(+), 6 deletions(-) diff --git a/src/init.cpp b/src/init.cpp index a44cdf807e0..e6cc2045b4a 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -685,7 +685,7 @@ void SetupServerArgs(ArgsManager& argsman, bool can_listen_ipc) argsman.AddArg("-blockmaxweight=", strprintf("Set maximum BIP141 block weight (default: %d)", DEFAULT_BLOCK_MAX_WEIGHT), ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::BLOCK_CREATION); - argsman.AddArg("-blockreservedweight=", strprintf("Reserve space for the fixed-size block header plus the largest coinbase transaction the mining software may add to the block. (default: %d).", DEFAULT_BLOCK_RESERVED_WEIGHT), ArgsManager::ALLOW_ANY, OptionsCategory::BLOCK_CREATION); + argsman.AddArg("-blockreservedweight=", strprintf("Reserve space for the fixed-size block header plus the largest coinbase transaction the mining software may add to the block. Only affects mining RPC clients, not IPC clients. (default: %d).", DEFAULT_BLOCK_RESERVED_WEIGHT), ArgsManager::ALLOW_ANY, OptionsCategory::BLOCK_CREATION); argsman.AddArg("-blockmintxfee=", strprintf("Set lowest fee rate (in %s/kvB) for transactions to be included in block creation. (default: %s)", CURRENCY_UNIT, FormatMoney(DEFAULT_BLOCK_MIN_TX_FEE)), ArgsManager::ALLOW_ANY, OptionsCategory::BLOCK_CREATION); argsman.AddArg("-blockversion=", "Override block version to test forking scenarios", ArgsManager::ALLOW_ANY | ArgsManager::DEBUG_ONLY, OptionsCategory::BLOCK_CREATION); diff --git a/src/node/miner.cpp b/src/node/miner.cpp index b5073054eb2..7bdd8b690d7 100644 --- a/src/node/miner.cpp +++ b/src/node/miner.cpp @@ -78,11 +78,12 @@ void RegenerateCommitments(CBlock& block, ChainstateManager& chainman) static BlockAssembler::Options ClampOptions(BlockAssembler::Options options) { - options.block_reserved_weight = std::clamp(options.block_reserved_weight, MINIMUM_BLOCK_RESERVED_WEIGHT, MAX_BLOCK_WEIGHT); + // Apply DEFAULT_BLOCK_RESERVED_WEIGHT when the caller left it unset. + options.block_reserved_weight = std::clamp(options.block_reserved_weight.value_or(DEFAULT_BLOCK_RESERVED_WEIGHT), MINIMUM_BLOCK_RESERVED_WEIGHT, MAX_BLOCK_WEIGHT); options.coinbase_output_max_additional_sigops = std::clamp(options.coinbase_output_max_additional_sigops, 0, MAX_BLOCK_SIGOPS_COST); // Limit weight to between block_reserved_weight and MAX_BLOCK_WEIGHT for sanity: // block_reserved_weight can safely exceed -blockmaxweight, but the rest of the block template will be empty. - options.nBlockMaxWeight = std::clamp(options.nBlockMaxWeight, options.block_reserved_weight, MAX_BLOCK_WEIGHT); + options.nBlockMaxWeight = std::clamp(options.nBlockMaxWeight, *options.block_reserved_weight, MAX_BLOCK_WEIGHT); return options; } @@ -102,13 +103,15 @@ void ApplyArgsManOptions(const ArgsManager& args, BlockAssembler::Options& optio if (const auto parsed{ParseMoney(*blockmintxfee)}) options.blockMinFeeRate = CFeeRate{*parsed}; } options.print_modified_fee = args.GetBoolArg("-printpriority", options.print_modified_fee); - options.block_reserved_weight = args.GetIntArg("-blockreservedweight", options.block_reserved_weight); + if (!options.block_reserved_weight) { + options.block_reserved_weight = args.GetIntArg("-blockreservedweight"); + } } void BlockAssembler::resetBlock() { // Reserve space for fixed-size block header, txs count, and coinbase tx. - nBlockWeight = m_options.block_reserved_weight; + nBlockWeight = *Assert(m_options.block_reserved_weight); nBlockSigOpsCost = m_options.coinbase_output_max_additional_sigops; // These counters do not include coinbase tx diff --git a/src/node/types.h b/src/node/types.h index deab1faacb8..1eea9460535 100644 --- a/src/node/types.h +++ b/src/node/types.h @@ -44,8 +44,12 @@ struct BlockCreateOptions { /** * The default reserved weight for the fixed-size block header, * transaction count and coinbase transaction. + * + * Providing a value overrides the `-blockreservedweight` startup setting. + * Cap'n Proto IPC clients currently cannot leave this field unset, so they + * always provide a value. */ - size_t block_reserved_weight{DEFAULT_BLOCK_RESERVED_WEIGHT}; + std::optional block_reserved_weight{}; /** * The maximum additional sigops which the pool will add in coinbase * transaction outputs. diff --git a/test/functional/interface_ipc_mining.py b/test/functional/interface_ipc_mining.py index 876a9231a0f..d1bdf609ad1 100755 --- a/test/functional/interface_ipc_mining.py +++ b/test/functional/interface_ipc_mining.py @@ -8,6 +8,7 @@ from contextlib import AsyncExitStack from io import BytesIO from test_framework.blocktools import NULL_OUTPOINT from test_framework.messages import ( + MAX_BLOCK_WEIGHT, CTransaction, CTxIn, CTxOut, @@ -227,6 +228,37 @@ class IPCMiningTest(BitcoinTestFramework): asyncio.run(capnp.run(async_routine())) + 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 + # 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}"]) + + async def async_routine(): + ctx, mining = await self.make_mining_ctx() + self.miniwallet.send_self_transfer(fee_rate=10, from_node=self.nodes[0]) + + 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) + assert_equal(len(block.vtx), 2) + + self.log.debug("Use absurdly large reserved weight to force an empty template") + opts.blockReservedWeight = MAX_BLOCK_WEIGHT + empty_template = await mining_create_block_template(mining, stack, ctx, opts) + assert empty_template is not None + empty_block = await mining_get_block(empty_template, ctx) + assert_equal(len(empty_block.vtx), 1) + + asyncio.run(capnp.run(async_routine())) + def run_coinbase_and_submission_test(self): """Test coinbase construction (getCoinbaseTx, getCoinbaseCommitment) and block submission (submitSolution).""" self.log.info("Running coinbase construction and submission test") @@ -309,6 +341,7 @@ class IPCMiningTest(BitcoinTestFramework): self.run_mining_interface_test() self.run_block_template_test() self.run_coinbase_and_submission_test() + self.run_ipc_option_override_test() if __name__ == '__main__': From b623fab1ba87ea93dd7e302b8c927e55f2036173 Mon Sep 17 00:00:00 2001 From: Sjors Provoost Date: Sat, 7 Feb 2026 13:50:18 +0100 Subject: [PATCH 3/3] mining: enforce minimum reserved weight for IPC Previously a lower value was silently clamped to MINIMUM_BLOCK_RESERVED_WEIGHT. --- src/node/interfaces.cpp | 11 +++++++++++ src/node/types.h | 3 ++- test/functional/interface_ipc_mining.py | 9 +++++++++ 3 files changed, 22 insertions(+), 1 deletion(-) diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 37524176e20..af9388d4b9f 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -67,6 +67,7 @@ #include #include #include +#include #include #include @@ -969,6 +970,16 @@ public: std::unique_ptr createNewBlock(const BlockCreateOptions& options) override { + // Reject too-small values instead of clamping so callers don't silently + // end up mining with different options than requested. This matches the + // behavior of the `-blockreservedweight` startup option, which rejects + // values below MINIMUM_BLOCK_RESERVED_WEIGHT. + if (options.block_reserved_weight && options.block_reserved_weight < MINIMUM_BLOCK_RESERVED_WEIGHT) { + throw std::runtime_error(strprintf("block_reserved_weight (%zu) must be at least %u weight units", + *options.block_reserved_weight, + MINIMUM_BLOCK_RESERVED_WEIGHT)); + } + // Ensure m_tip_block is set so consumers of BlockTemplate can rely on that. if (!waitTipChanged(uint256::ZERO, MillisecondsDouble::max())) return {}; diff --git a/src/node/types.h b/src/node/types.h index 1eea9460535..e3ee05dd0d9 100644 --- a/src/node/types.h +++ b/src/node/types.h @@ -43,7 +43,8 @@ struct BlockCreateOptions { bool use_mempool{true}; /** * The default reserved weight for the fixed-size block header, - * transaction count and coinbase transaction. + * transaction count and coinbase transaction. Minimum: 2000 weight units + * (MINIMUM_BLOCK_RESERVED_WEIGHT). * * Providing a value overrides the `-blockreservedweight` startup setting. * Cap'n Proto IPC clients currently cannot leave this field unset, so they diff --git a/test/functional/interface_ipc_mining.py b/test/functional/interface_ipc_mining.py index d1bdf609ad1..61c050e74fe 100755 --- a/test/functional/interface_ipc_mining.py +++ b/test/functional/interface_ipc_mining.py @@ -257,6 +257,15 @@ 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: + 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):