Fix BIP-119 Typo + Clarify the reasons for 32 bit lengths #1268

pull JeremyRubin wants to merge 1 commits into bitcoin:master from JeremyRubin:patch-1 changing 1 files +11 −7

JeremyRubin commented at 12:12 AM on January 5, 2022: contributor

when drafting the BIP I attempted to triangulate what the width of the fields for number of inputs/outputs should be from various sources in the codebase, and made an error in looking at the signing and encoding logic, and not the decoding logic which restricts vectors (vin, vout) to MAX_SIZE which is 33554432. This fully justifies not using a wider type (uint64_t).

Also clarified arguments for not using a narrower type (uint16_t) which would be possible just for the vIn based on block size, because it's a leaky abstraction (you can still encode and decode such a transaction, just not mine it).

thanks to @roconnor-blockstream for pointing this out

Fix BIP-119 Typo + Clarify the reasons for 32 bit lengths

when drafting the BIP I attempted to triangulate what the width of the fields for number of inputs/outputs should be from various sources in the codebase, and made an error in looking at the signing and encoding logic, and not the _decoding_ logic which restricts vectors (vin, vout) to MAX_SIZE which is 33554432. This fully justifies not using a wider type (uint64_t).

Also clarified arguments for not using a narrower type (uint16_t) which would be possible just for the vIn based on block size, because it's a leaky abstraction (you can still encode and decode such a transaction,  just not mine it).


thanks to @roconnor-blockstream for pointing this out

b96b1712fc

roconnor-blockstream commented at 1:51 AM on January 5, 2022: contributor

LGTM.

in bip-0119.mediawiki:355 in b96b1712fc

 349 | @@ -348,12 +350,14 @@ script.
 350 |  
 351 |  =====Committing to the Number of Outputs=====
 352 |  
 353 | -In principal, committing to the Outputs Hash (below) implicitly commits to the number of outputs,
 354 | +In principle, committing to the Outputs Hash (below) implicitly commits to the number of outputs,
 355 |  making this field strictly redundant. However, separately committing to this number makes it easier
 356 |  to construct DefaultCheckTemplateVerifyHash from script.
 357 |  
 358 | -We treat the number of outputs as a `uint32_t` because a `COutpoint` index is a `uint32_t`, even

f3rnando commented at 3:21 AM on January 5, 2022:

¬¬

in bip-0119.mediawiki:361 in b96b1712fc

 359 | -though in principal a transaction could encode more outputs.
 360 | +We treat the number of outputs as a `uint32_t` because a `COutpoint` index is a `uint32_t`.
 361 | +Further, Bitcoin's consensus decoding logic limits vectors to `MAX_SIZE=33554432` and that is
 362 | +larger than `uint16_t` and smaller than `uint32_t`. 32 bits is also friendly for manipulation using
 363 | +Bitcoin's current math opcodes, should `OP_CAT` be added.
 364 |

f3rnando commented at 3:21 AM on January 5, 2022:

luke-jr merged this on Jan 15, 2022
luke-jr closed this on Jan 15, 2022

Contributors

JeremyRubin

roconnor-blockstream

f3rnando