Shelley Spec maxBlockSize is incorrect

[lehins]

trafficstars

There is confusion in the spec between two terms:

• maxBlockSize, which is defined as "max block body size" (Figure 7)
• and maxBBSize, which is used in the chainChecks function. (Firgure 74)

Both of those refer to the maximum size of the block body, not the whole block, which is apparent from one of the names, definition and the check in the chainChecks funciton: hBbsize (bhbody bh) ≤ maxBBSize, where hBbsize (bhbody bh) is defined as the total size of all transactions in the block (i.e. block body) in Figure 72:

hBbsize in the spec corresponds to bhviewBSize in the Ledger implementation and refers to the block body size, not the size of the whole block with the header. Here is the predicate check in the BBODY rule:

https://github.com/IntersectMBO/cardano-ledger/blob/315e78d1baed5efcd18206f3d5113ffefc81de74/eras/shelley/impl/src/Cardano/Ledger/Shelley/Rules/Bbody.hs#L181-L186

With all that context in mind we now look at the bug in the spec in the PPUP rule (Figure 43):

Block header size has nothing to do with the block body size, so there is no reason for block header size to be included in the above formula. This computation should be maxTxSize pp < maxBlockSize pp instead, because we need to ensure that at least one transaction can fit into a block.

This is not a dangerous bug by any stretch of imagination, but it is misleading, because it makes one believe that maxBlockSize has the meaning of the block size that also includes the header together with all of the transactions.

[lehins]

I'd also recommend renaming maxBlockSize to maxBlockBodySize, to improve clarity

[WhatisRT]

The name of this parameter is somewhat inconsistent. I can find at least maxBlockSize, maxBBSize and maxBlockBodySize. For maxHeaderSize, I can also find maxBHSize which makes me think that a better long form would be maxBlockHeaderSize.

So maybe we should first answer some questions:

• Do we want short or long forms of the names, or both depending on the situation?
• Do we want consistency (maxBlock(Header|Body)Size)? I vote yes.

The other issue is the check you've mentioned. It's somewhat stupid but I'm not sure we should change it now. It is what's been implemented and it doesn't harm anything. Maybe the better option would be to just put in an erratum like we already have for a whole bunch of other things.

[lehins]

Do we want short or long forms of the names, or both depending on the situation?

My preference would be for longer and consistent names across the spec

Do we want consistency (maxBlock(Header|Body)Size)? I vote yes.

I vote yes as well.

It is what's been implemented and it doesn't harm anything.

I'll be removing this check from the implementation, because HFC did not have this logic which is an actual bug that I was describing in the meeting. So, I suggest we have consistent story across the spec and implementation and remove this nonsense.