mining: move minimum block_reserved_weight check to miner

And also enforce maximum.

Currently the exception 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 {
 
@@ -81,7 +82,21 @@ static BlockCreateOptions ClampOptions(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);
+    const size_t reserved_weight{options.block_reserved_weight.value_or(DEFAULT_BLOCK_RESERVED_WEIGHT)};
+    // Reject too-small values instead of clamping so callers don't silently
+    // end up mining with different options than requested.
+    if (reserved_weight < MINIMUM_BLOCK_RESERVED_WEIGHT) {
+        throw std::runtime_error(strprintf("block_reserved_weight (%zu) must be at least %u weight units",
+                                           reserved_weight,
+                                           MINIMUM_BLOCK_RESERVED_WEIGHT));
+    }
+    // Reject too-large values too.
+    if (reserved_weight > MAX_BLOCK_WEIGHT) {
+        throw std::runtime_error(strprintf("block_reserved_weight (%zu) exceeds consensus maximum block weight (%u)",
+                                           reserved_weight,
+                                           MAX_BLOCK_WEIGHT));
+    }
+    options.block_reserved_weight = reserved_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.

test/functional/interface_ipc_mining.py

@@ -267,6 +267,16 @@ async def async_routine():
                     assert_equal(e.description, "remote exception: std::exception: block_reserved_weight (0) must be at least 2000 weight units")
                     assert_equal(e.type, "FAILED")
 
+                self.log.debug("Enforce maximum reserved weight for IPC clients too")
+                opts.blockReservedWeight = MAX_BLOCK_WEIGHT + 1
+                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: block_reserved_weight ({MAX_BLOCK_WEIGHT + 1}) exceeds consensus maximum block weight ({MAX_BLOCK_WEIGHT})")
+                    assert_equal(e.type, "FAILED")
+
+
         asyncio.run(capnp.run(async_routine()))
 
     def run_coinbase_and_submission_test(self):