Merge bitcoin/bitcoin#33222: miner: clamp options instead of asserting

7392b8b084 miner: clamp options instead of asserting (Pieter Wuille)

Pull request description:

  The `BlockAssembler::ClampOptions` function currently doesn't actually clamp most of the provided settings, but asserts that some are in range. This made sense while it was a purely internal interface.

  However, with the mining IPC interface exposed in #30510, these options are now externally accessible, and it is not entirely intuitive how to set them. In particular, calling `Mining::createNewBlock` with a default-constructed `BlockCreateOptions` will right now instantly crash the bitcoin node.

  This isn't a security issue, as the IPC interface is considered trusted, but it is highly unexpected I think, and rather unergonomical to have the node crash while developing against the interface.

  An alternative would be exposing a way for the interface to return a failure, but I think in this case, just correcting to reasonable values is acceptable.

ACKs for top commit:
  Sjors:
    ACK 7392b8b084
  achow101:
    ACK 7392b8b084
  stickies-v:
    ACK 7392b8b084
  ryanofsky:
    Code review ACK 7392b8b084. I think ideally this would throw an exception and return a clear error to the caller, or maybe log as stickies suggested, but clamping is much better than crashing.

Tree-SHA512: 7a1e05b68edbf57beb682ee63e27666f42af6a2b70a81874d368a2cb10d107a589e0a388658c1039330b8cc9f6048479870095a9d552ca387a250ac118c1abf2
This commit is contained in:
Ava Chow
2025-08-21 13:58:23 -07:00

View File

@@ -77,9 +77,8 @@ void RegenerateCommitments(CBlock& block, ChainstateManager& chainman)
static BlockAssembler::Options ClampOptions(BlockAssembler::Options options) static BlockAssembler::Options ClampOptions(BlockAssembler::Options options)
{ {
Assert(options.block_reserved_weight <= MAX_BLOCK_WEIGHT); options.block_reserved_weight = std::clamp<size_t>(options.block_reserved_weight, MINIMUM_BLOCK_RESERVED_WEIGHT, MAX_BLOCK_WEIGHT);
Assert(options.block_reserved_weight >= MINIMUM_BLOCK_RESERVED_WEIGHT); options.coinbase_output_max_additional_sigops = std::clamp<size_t>(options.coinbase_output_max_additional_sigops, 0, MAX_BLOCK_SIGOPS_COST);
Assert(options.coinbase_output_max_additional_sigops <= MAX_BLOCK_SIGOPS_COST);
// Limit weight to between block_reserved_weight and MAX_BLOCK_WEIGHT for sanity: // 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. // block_reserved_weight can safely exceed -blockmaxweight, but the rest of the block template will be empty.
options.nBlockMaxWeight = std::clamp<size_t>(options.nBlockMaxWeight, options.block_reserved_weight, MAX_BLOCK_WEIGHT); options.nBlockMaxWeight = std::clamp<size_t>(options.nBlockMaxWeight, options.block_reserved_weight, MAX_BLOCK_WEIGHT);