mining: log failed blocks in submitSolution() #33372

1. 
   Sjors commented at 9:31 am on September 12, 2025: member

   Unlike the submitblock RPC which takes a fully serialized block, when a block solution is received via IPC the client only provides the nonce, coinbase and a few other fields to submitSolution(). It may not have all the information it needs to reconstruct the block. This makes debugging difficult when the block is invalid.

   This commit therefore logs the fully serialized block if it’s rejected. This is in addition to the failure reason logged by ProcessNewBlock().

   Example on a custom signet:

   02025-09-12T09:18:32Z [error] ProcessNewBlock: AcceptBlock FAILED (bad-signet-blksig, signet block signature validation failure)
   12025-09-12T09:18:32Z 000000204c002429741856d602d981fc9b8868d4cfe53d26f10ec5bed90186dbe2000000be72ddf01d23f7f1c3c165276c457b7a41bc0bac7b67096be4dd48e42d41612e6be5c368ae77031ef134b9a401020000000001010000000000000000000000000000000000000000000000000000000000000000ffffffff36016200112f536a6f72736e61646f20506f6f6c2f2f20000100000000000000000000000000000000000000000000000000000000000100000000010000000000000000225120f140a01a8eed90d0d410af4e25eeca9c313915d2245e0f12321493d63b8be8550120000000000000000000000000000000000000000000000000000000000000000000000000
   

   On mainnet this would be a huge message, but the IPC connection is trusted.

2. mining: log failed blocks in submitSolution()

   Unlike the submitblock RPC which takes a fully serialized block,
   when a block solution is received via IPC the client only provides
   the nonce, coinbase and a few other fields. It may not have all
   the information it needs to reconstruct the block. This makes
   debugging difficult when the block is invalid.
   
   This commit therefore logs the fully serialized block if it's
   rejected. This is in addition to the failure reason logged
   by ProcessNewBlock().

   067fa33daa
3. DrahtBot added the label Mining on Sep 12, 2025
4. 
   DrahtBot commented at 9:31 am on September 12, 2025: contributor

   The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

   Code Coverage & Benchmarks

   For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33372.

   Reviews

   See the guideline for information on the review process. A summary of reviews will appear here.

5. 
   ajtowns commented at 9:54 am on September 12, 2025: contributor

   On mainnet this would be a huge message, but the IPC connection is trusted.

   If this is for IPC, wouldn’t it make more sense to return the value over IPC as part of an error response? Otherwise, having it be a LogDebug or even LogTrace seems more sensible?

6. 
   Sjors commented at 10:13 am on September 12, 2025: member

   @ajtowns currently the return value is a boolean. I could indeed change it to return both a boolean and a string. But ideally I’d like to back port this to v30, so keeping it as simple as possible without changing the interface.

   (changing the interface would also break clients, but that’s fine at this early testing stage)

   I picked LogInfo because, especially on mainnet, the information seems critical enough to log by default.

7. 
   Sjors commented at 11:49 am on September 12, 2025: member

   I opened #33374 as an alternative or complimentary approach.
8. 
   Sjors commented at 5:06 pm on September 12, 2025: member

   The other PR has the additional benefit that the client can broadcast the block in additional ways, and/or inspect it with checkBlock().
9. Sjors closed this on Sep 12, 2025

   ---

10. 
    ajtowns commented at 6:42 am on September 15, 2025: contributor

    I think logging failed blocks would run afoul of rate limiting (#32604) unless it were special-cased similarly to the UpdateTip log message.

Contributors
Sjors DrahtBot ajtowns

Labels
Mining