Calculate size and weight of block correctly in CreateNewBlock() #8838

1. 
   jnewbery commented at 2:34 pm on September 29, 2016: member

   bitcoind writes the following log when it’s created a new block:

   CreateNewBlock(): total size _ txs: _ fees: _ sigops _

   If this is post-segwit and the miner is counting weight and not block size, then the size field will be incorrect (it will always be set to 1000).

   This PR fixes the log in the following way:

   • if the miner is not accounting for size, then only log the weight of the block.
   • if the miner is accounting for size, then log the size and weight of the block.

   I’ve run all tests and manually verified that the log output is correct when using the blockmaxsize and blockmaxweight command line parameters.

2. laanwj added the label Mining on Sep 29, 2016
3. MarcoFalke added the label Docs and Output on Sep 29, 2016
4. 
   gmaxwell commented at 3:12 am on October 3, 2016: contributor

   Log entries that change their format are really bad for tooling. Is there any reason why it’s bad to log limits that are at their maximum because they’re not being applied?
5. 
   jnewbery commented at 1:15 pm on October 3, 2016: member

   The current code is not logging nBlockSize at its maximum when size is not being accounted. Under the current code, when a block is being created, the following happens:

   • set nBlockSize to 1000 (bytes) to reserve space for the coinbase transaction
   • set nBlockWeight to 4000 to reserve space for the coinbase transaction
   • for every transaction that is added to the block:
     • if size is being accounted update nBlockSize
     • update nBlockWeight
   • when the block is complete, log nBlockSize and not nBlockWeight

   That means that if size is not being accounted, CreateNewBlock always logs that a block of size 1000 (bytes) has been created.

   I don’t think we should just change the log to output size = 1000000 if size isn’t being accounted.

   I propose the following change:

   • add nBlockWeight to the log
   • if size is being accounted, both size and weight will be logged correctly
   • if size is not being accounted, set size to 0 and log weight correctly.
6. 
   sipa commented at 1:36 pm on October 3, 2016: member

   Maybe @gmaxwell’s concern can be addressed by logging ’total size ?'
7. 
   sipa commented at 1:38 pm on October 3, 2016: member

   Alternatively, you can always just compute the total block size using ::GetSerializeSize(block, SER_NETWORK, PROTOCOL_VERSION).
8. log block size and weight correctly. 5f274a1749
9. jnewbery force-pushed on Oct 3, 2016
10. 
    jnewbery commented at 6:09 pm on October 3, 2016: member

    Updated to always calculate size and weight correctly (whether or not size is being accounted). I’ll update the description of this PR.
11. jnewbery renamed this:
    Only log block size if block size is being accounted
    Calculate size and weight of block correctly in CreateNewBlock()
    on Oct 3, 2016
12. 
    sipa commented at 12:51 pm on October 4, 2016: member

    utACK
13. 
    jnewbery commented at 12:59 pm on November 17, 2016: member

    @laanwj - this PR fixes a bug where bitcoin logs incorrect sizes for new blocks. Is there anything I can do to help this get merged?
14. laanwj merged this on Nov 17, 2016
15. laanwj closed this on Nov 17, 2016

    ---

16. laanwj referenced this in commit f6db48ad1c on Nov 17, 2016
17. MarcoFalke added this to the milestone 0.13.2 on Nov 17, 2016
18. MarcoFalke added the label Needs backport on Nov 17, 2016
19. jnewbery deleted the branch on Nov 17, 2016
20. laanwj referenced this in commit 99dc2d7e23 on Dec 2, 2016
21. 
    laanwj commented at 7:22 am on December 2, 2016: member

    Backported in #9264, removing tag.
22. laanwj removed the label Needs backport on Dec 2, 2016
23. laanwj referenced this in commit 094848baf0 on Dec 2, 2016
24. MarcoFalke locked this on Sep 8, 2021

Contributors
jnewbery gmaxwell sipa laanwj

Labels
Docs Mining

Milestone
0.13.2