Merge bitcoin/bitcoin#30356: refactor: add coinbase constraints to BlockAssembler::Options

c504b6997b1acc9771ad1f52efaa4be2b4966c6c refactor: add coinbase constraints to BlockCreateOptions (Sjors Provoost)
6b4c817d4b978adf69738677c74855ef0675f333 refactor: pass BlockCreateOptions to createNewBlock (Sjors Provoost)
323cfed5959b25c98235ec988b408fc5e3391e3c refactor: use CHECK_NONFATAL to avoid single-use symbol (Sjors Provoost)

Pull request description:

  When generating a block template through e.g. getblocktemplate RPC, we reserve 4000 weight units and 400 sigops. Pools use this space for their coinbase outputs.

  At least one pool patched their Bitcoin Core node to adjust these hardcoded values. They eventually [produced an invalid block](https://bitcoin.stackexchange.com/questions/117837/how-many-sigops-are-in-the-invalid-block-783426) which exceeded the sigops limit.

  The existince of such patches suggests it may be useful to make this value configurable. This PR would make such a change easier. However, the main motivation is that in the Stratum v2 spec requires the pool to communicate the maximum bytes they intend
  to add to the coinbase outputs.

  Specifically the `CoinbaseOutputDataSize` message which is part of the [Template Distribution Protocol](https://github.com/stratum-mining/sv2-spec/blob/main/07-Template-Distribution-Protocol.md#71-coinbaseoutputdatasize-client---server) has a field `coinbase_output_max_additional_size`.

  A proposed change to the spec adds the max additional sigops as well: https://github.com/stratum-mining/sv2-spec/pull/86. Whether that change makes it into the spec is not important though, as adding both to `BlockAssembler::Options` makes sense.

  The first commit is a test refactor followup for #30335, related to the code that's changed here, but not required.

  The second commit introduces BlockCreateOptions, with just `use_mempool`.

  The thirds commit adds `coinbase_max_additional_weight` and `coinbase_output_max_additional_sigops` to  `BlockCreateOptions`. They use the originally hardcoded values, and no existing caller overrides these defaults. This changes in #29432.

ACKs for top commit:
  itornaza:
    tested ACK c504b6997b1acc9771ad1f52efaa4be2b4966c6c
  ryanofsky:
    Code review ACK c504b6997b1acc9771ad1f52efaa4be2b4966c6c
  ismaelsadeeq:
    Code review ACK c504b6997b1acc9771ad1f52efaa4be2b4966c6c

Tree-SHA512: de2fa085f47048c91d95524e03f909f6f27f175c1fefa3d6106445e7eb5cf5b710eda6ea5b641cf3b4704a4e4e0181a0c829003b9fd35465f2a46167e5d64487

1 files changed, 2 insertions, 4 deletions

diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp
index 7e420dcd9b..8482ce6eb2 100644
--- a/src/rpc/mining.cpp
+++ b/src/rpc/mining.cpp
@@ -371,7 +371,7 @@ static RPCHelpMan generateblock()
 
     ChainstateManager& chainman = EnsureChainman(node);
     {
-        std::unique_ptr<CBlockTemplate> blocktemplate{miner.createNewBlock(coinbase_script, /*use_mempool=*/false)};
+        std::unique_ptr<CBlockTemplate> blocktemplate{miner.createNewBlock(coinbase_script, {.use_mempool = false})};
         if (!blocktemplate) {
             throw JSONRPCError(RPC_INTERNAL_ERROR, "Couldn't create new block");
         }
@@ -778,9 +778,7 @@ static RPCHelpMan getblocktemplate()
         }
         ENTER_CRITICAL_SECTION(cs_main);
 
-        std::optional<uint256> maybe_tip{miner.getTipHash()};
-        CHECK_NONFATAL(maybe_tip);
-        tip = maybe_tip.value();
+        tip = CHECK_NONFATAL(miner.getTipHash()).value();
 
         if (!IsRPCRunning())
             throw JSONRPCError(RPC_CLIENT_NOT_CONNECTED, "Shutting down");