Fix CSizeComputer and remove witness size constants
The size and vsize of transactions, and the size and strippedsize of blocks are displayed identically even with witness transactions. When PROTOCOL_VERSION | SERIALIZE_TRANSACTION_NO_WITNESS is passed to GetSerializeSize, it has no effect because it is incorrectly passed as the nType and not the nVersion.
This is because nType was removed in eb696d7c66 but re-added in 195b0fac3c without reverting the function calls to include SER_NETWORK.
As witness bytes are no longer discounted due to this bug, the code should be altered to remove references to the WITNESS_SCALE_FACTOR. The nType in CSizeComputer and GetSerializeSize should be removed and all calls to these should be fixed.
My proposed fix:
- [ ] Ensure there is a test to require that blocks with and without witness data cannot exceed 1MB (current and desired behaviour since this bug).
- [ ] Change
CSizeComputerto match how it was originally intended in commit eb696d7c66 and currently is in Bitcoin withnTyperemoved. - [ ] Remove
WITNESS_SCALE_FACTOR,MAX_BLOCK_WEIGHTandMAX_BLOCK_SERIALIZED_SIZE. - [ ] Revert to using
MAX_BLOCK_SIZEas 1000000. Block size checks should be done against that. - [ ]
MIN_TRANSACTION_WEIGHTandMIN_SERIALIZABLE_TRANSACTION_WEIGHTshould reference size and not weight. - [ ] Remove
GetTransactionWeightand useGetSerializeSizewithoutSERIALIZE_TRANSACTION_NO_WITNESSfor bothsize,weightandvsize. - [ ] Any other references to weight and vsize should be removed or made equivalent to size.
- [ ]
strippedsizein blocks should useSERIALIZE_TRANSACTION_NO_WITNESS.stippedsize,weightandvsizeare only kept for RPC compatibility.
nType is currently used to send SER_POSMARKER in block headers:
https://github.com/peercoin/peercoin/blob/master/src/primitives/block.h#L46
If nType is removed, this part should be also somehow adapted.
In Bitcoin Core 26.0, nType has been removed in many more serialization-related classes (BufferedFile, CVectorWriter, CAutoFile...), so in order to keep maximum compatibility with the Bitcoin codebase, its removal has to be considered.
...and in Bitcoin Core 27.0 everything has been deeply refactored. See https://github.com/bitcoin/bitcoin/pull/28438 for reference.