mining: add helper for block constraint checks

The maximum block size, reserved weight and additional sigops
constraints can be provided as startup arguments and/or IPC
options. Avoid duplication of these checks by adding helpers.

Have IPC check all of these, instead of only block_reserved_weight.
This removes the need for clamping options, so ClampOptions is
renamed to ApplyBlockCreateOptions.

Currently these exceptions can only be triggered by IPC (and test)
code, because for the RPC path the minimum -blockreservedweight is
enforced during node startup.

If in the future RPC methods like getblocktemplate allow a custom
value per request, they would trigger this exception and the RPC
call would return an error - consistent with IPC behavior.

Author: Sjors

src/node/interfaces.cpp

@@ -67,7 +67,6 @@
 #include <any>
 #include <memory>
 #include <optional>
-#include <stdexcept>
 #include <utility>
 
 #include <boost/signals2/signal.hpp>
@@ -980,16 +979,6 @@ class MinerImpl : public Mining
 
     std::unique_ptr<BlockTemplate> createNewBlock(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 {};
         ApplyMiningDefaults(m_node.mining_args, options);

src/node/miner.cpp

@@ -30,6 +30,7 @@
 #include <algorithm>
 #include <utility>
 #include <numeric>
+#include <stdexcept>
 
 namespace node {
 
@@ -76,16 +77,26 @@ void RegenerateCommitments(CBlock& block, ChainstateManager& chainman)
     block.hashMerkleRoot = BlockMerkleRoot(block);
 }
 
-static BlockCreateOptions ClampOptions(BlockCreateOptions options)
+static BlockCreateOptions ApplyBlockCreateOptions(BlockCreateOptions options)
 {
     // Typically block_reserved_weight and block_max_weight are set by
     // ApplyMiningDefaults before the constructor calls this; value_or(DEFAULT_...)
     // only affects (test) call sites that don't go through the Mining interface.
-    options.block_reserved_weight = std::clamp<size_t>(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<size_t>(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.block_max_weight = std::clamp<size_t>(options.block_max_weight.value_or(DEFAULT_BLOCK_MAX_WEIGHT), *options.block_reserved_weight, MAX_BLOCK_WEIGHT);
+    const size_t reserved_weight{options.block_reserved_weight.value_or(DEFAULT_BLOCK_RESERVED_WEIGHT)};
+    if (auto result{CheckBlockReservedWeight(reserved_weight)}; !result) {
+        throw std::runtime_error("block_reserved_weight " + util::ErrorString(result).original);
+    }
+    options.block_reserved_weight = reserved_weight;
+    if (auto result{CheckCoinbaseOutputMaxAdditionalSigops(options.coinbase_output_max_additional_sigops)}; !result) {
+        throw std::runtime_error("coinbase_output_max_additional_sigops " + util::ErrorString(result).original);
+    }
+    const size_t max_weight{options.block_max_weight.value_or(DEFAULT_BLOCK_MAX_WEIGHT)};
+    // block_reserved_weight can safely exceed block_max_weight, but the rest
+    // of the block template will be empty.
+    if (auto result{CheckBlockMaxWeight(max_weight)}; !result) {
+        throw std::runtime_error("block_max_weight " + util::ErrorString(result).original);
+    }
+    options.block_max_weight = max_weight;
     return options;
 }
 
@@ -97,7 +108,7 @@ BlockAssembler::BlockAssembler(Chainstate& chainstate,
       m_mempool{options.use_mempool ? mempool : nullptr},
       m_chainstate{chainstate},
       m_mining_args{mining_args},
-      m_options{ClampOptions(options)}
+      m_options{ApplyBlockCreateOptions(options)}
 {
 }
 

src/node/mining_args.cpp

@@ -6,9 +6,7 @@
 
 #include <common/args.h>
 #include <common/messages.h>
-#include <consensus/consensus.h>
 #include <node/mining_types.h>
-#include <tinyformat.h>
 #include <util/moneystr.h>
 #include <util/translation.h>
 
@@ -25,17 +23,14 @@ util::Result<void> ReadMiningArgs(const ArgsManager& args, MiningArgs& mining_ar
     }
 
     const size_t max_block_weight = args.GetIntArg("-blockmaxweight", DEFAULT_BLOCK_MAX_WEIGHT);
-    if (max_block_weight > MAX_BLOCK_WEIGHT) {
-        return util::Error{strprintf(_("Specified -blockmaxweight (%d) exceeds consensus maximum block weight (%d)"), max_block_weight, MAX_BLOCK_WEIGHT)};
+    if (auto result{CheckBlockMaxWeight(max_block_weight)}; !result) {
+        return util::Error{Untranslated("Specified -blockmaxweight " + util::ErrorString(result).original)};
     }
     mining_args.default_block_max_weight = max_block_weight;
 
     const size_t block_reserved_weight = args.GetIntArg("-blockreservedweight", DEFAULT_BLOCK_RESERVED_WEIGHT);
-    if (block_reserved_weight > MAX_BLOCK_WEIGHT) {
-        return util::Error{strprintf(_("Specified -blockreservedweight (%d) exceeds consensus maximum block weight (%d)"), block_reserved_weight, MAX_BLOCK_WEIGHT)};
-    }
-    if (block_reserved_weight < MINIMUM_BLOCK_RESERVED_WEIGHT) {
-        return util::Error{strprintf(_("Specified -blockreservedweight (%d) is lower than minimum safety value of (%d)"), block_reserved_weight, MINIMUM_BLOCK_RESERVED_WEIGHT)};
+    if (auto result{CheckBlockReservedWeight(block_reserved_weight)}; !result) {
+        return util::Error{Untranslated("Specified -blockreservedweight " + util::ErrorString(result).original)};
     }
     mining_args.default_block_reserved_weight = block_reserved_weight;
 

src/node/mining_types.h

@@ -16,8 +16,11 @@
 #include <policy/policy.h>
 #include <primitives/transaction.h>
 #include <script/script.h>
+#include <tinyformat.h>
 #include <uint256.h>
+#include <util/result.h>
 #include <util/time.h>
+#include <util/translation.h>
 
 #include <cstddef>
 #include <optional>
@@ -163,6 +166,40 @@ struct CoinbaseTx {
     uint32_t lock_time;
 };
 
+/** Check that block_max_weight does not exceed consensus limits. */
+[[nodiscard]] inline util::Result<void> CheckBlockMaxWeight(size_t block_max_weight)
+{
+    if (block_max_weight > MAX_BLOCK_WEIGHT) {
+        return util::Error{Untranslated(strprintf("(%zu) exceeds consensus maximum block weight (%u)",
+                                                  block_max_weight, MAX_BLOCK_WEIGHT))};
+    }
+    return {};
+}
+
+/** Check that block_reserved_weight is within allowed bounds. */
+[[nodiscard]] inline util::Result<void> CheckBlockReservedWeight(size_t block_reserved_weight)
+{
+    if (block_reserved_weight < MINIMUM_BLOCK_RESERVED_WEIGHT) {
+        return util::Error{Untranslated(strprintf("(%zu) is lower than minimum safety value of (%u)",
+                                                  block_reserved_weight, MINIMUM_BLOCK_RESERVED_WEIGHT))};
+    }
+    if (block_reserved_weight > MAX_BLOCK_WEIGHT) {
+        return util::Error{Untranslated(strprintf("(%zu) exceeds consensus maximum block weight (%u)",
+                                                  block_reserved_weight, MAX_BLOCK_WEIGHT))};
+    }
+    return {};
+}
+
+/** Check that coinbase_output_max_additional_sigops does not exceed consensus limits. */
+[[nodiscard]] inline util::Result<void> CheckCoinbaseOutputMaxAdditionalSigops(size_t sigops)
+{
+    if (sigops > MAX_BLOCK_SIGOPS_COST) {
+        return util::Error{Untranslated(strprintf("(%zu) exceeds consensus maximum block sigops cost (%d)",
+                                                  sigops, MAX_BLOCK_SIGOPS_COST))};
+    }
+    return {};
+}
+
 } // namespace node
 
 #endif // BITCOIN_NODE_MINING_TYPES_H

test/functional/interface_ipc_mining.py

@@ -8,13 +8,14 @@
 from io import BytesIO
 from test_framework.blocktools import NULL_OUTPOINT
 from test_framework.messages import (
-    MAX_BLOCK_WEIGHT,
     CTransaction,
     CTxIn,
     CTxOut,
     CTxInWitness,
     ser_uint256,
     COIN,
+    MAX_BLOCK_WEIGHT,
+    MAX_BLOCK_SIGOPS_COST,
 )
 from test_framework.script import (
     CScript,
@@ -122,6 +123,15 @@ async def make_mining_ctx(self):
         mining = init.makeMining(ctx).result
         return ctx, mining
 
+    async def assert_create_fails(self, mining, opts, expected_msg):
+        """Assert that createNewBlock raises a remote exception with the expected message."""
+        try:
+            await mining.createNewBlock(opts)
+            raise AssertionError("createNewBlock unexpectedly succeeded")
+        except capnp.lib.capnp.KjException as e:
+            assert_equal(e.description, f"remote exception: std::exception: {expected_msg}")
+            assert_equal(e.type, "FAILED")
+
     def run_mining_interface_test(self):
         """Test Mining interface methods."""
         self.log.info("Running Mining interface test")
@@ -260,12 +270,21 @@ async def async_routine():
 
                 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")
+                await self.assert_create_fails(mining, opts,
+                    "block_reserved_weight (0) is lower than minimum safety value of (2000)")
+
+                self.log.debug("Enforce maximum reserved weight for IPC clients too")
+                opts.blockReservedWeight = MAX_BLOCK_WEIGHT + 1
+                await self.assert_create_fails(mining, opts,
+                    f"block_reserved_weight ({MAX_BLOCK_WEIGHT + 1}) exceeds consensus maximum block weight ({MAX_BLOCK_WEIGHT})")
+
+                self.log.debug("Enforce sigops limit for IPC clients too")
+                opts.blockReservedWeight = 4000
+                opts.coinbaseOutputMaxAdditionalSigops = MAX_BLOCK_SIGOPS_COST + 1
+                await self.assert_create_fails(mining, opts,
+                    f"coinbase_output_max_additional_sigops ({MAX_BLOCK_SIGOPS_COST + 1}) exceeds consensus maximum block sigops cost ({MAX_BLOCK_SIGOPS_COST})")
+                opts.coinbaseOutputMaxAdditionalSigops = 0
+
 
         asyncio.run(capnp.run(async_routine()))
 

test/functional/test_framework/messages.py

@@ -36,6 +36,7 @@
 
 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