Skip to content

Conversation

jnewbery
Copy link
Contributor

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.

@laanwj laanwj added the Mining label Sep 29, 2016
@maflcko maflcko added the Docs label Sep 29, 2016
@gmaxwell
Copy link
Contributor

gmaxwell commented Oct 3, 2016

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?

@jnewbery
Copy link
Contributor Author

jnewbery commented Oct 3, 2016

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.

@sipa
Copy link
Member

sipa commented Oct 3, 2016

Maybe @gmaxwell's concern can be addressed by logging 'total size ?'

@sipa
Copy link
Member

sipa commented Oct 3, 2016

Alternatively, you can always just compute the total block size using ::GetSerializeSize(block, SER_NETWORK, PROTOCOL_VERSION).

@jnewbery
Copy link
Contributor Author

jnewbery commented Oct 3, 2016

Updated to always calculate size and weight correctly (whether or not size is being accounted). I'll update the description of this PR.

@jnewbery jnewbery changed the title Only log block size if block size is being accounted Calculate size and weight of block correctly in CreateNewBlock() Oct 3, 2016
@sipa
Copy link
Member

sipa commented Oct 4, 2016

utACK

@jnewbery
Copy link
Contributor Author

@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?

@laanwj laanwj merged commit 5f274a1 into bitcoin:master Nov 17, 2016
laanwj added a commit that referenced this pull request Nov 17, 2016
…wBlock()

5f274a1 log block size and weight correctly. (jnewbery)
@maflcko maflcko added this to the 0.13.2 milestone Nov 17, 2016
@jnewbery jnewbery deleted the dont_log_size branch November 17, 2016 13:54
@laanwj
Copy link
Member

laanwj commented Dec 2, 2016

Backported in #9264, removing tag.

laanwj pushed a commit to laanwj/bitcoin that referenced this pull request Dec 2, 2016
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants