Strictly speaking the bip isn't wrong. It just happens that an empty byte vector is serialized identical to an empty vector of any other type, but it takes the reader an extra step. Thus, the minor fixup suggestion.
BIP 325: Clarify that scriptWitness is a stack, not a byte string #978
pull maflcko wants to merge 1 commits into bitcoin:master from maflcko:patch-1 changing 1 files +1 −2-
maflcko commented at 3:51 PM on August 22, 2020: member
-
gmaxwell commented at 4:46 PM on August 22, 2020: contributor
Is it leaking Bitcoin Core implementation details into the document to bring up vectors here? The word 'vector' isn't otherwise in this BIP.
Wouldn't it just be accurate to make it say that it's encoded just like BIP141? The compactsize count followed by the items (if there are any) is just how vectors in the software get encoded.
-
ff2833e6d6
Update bip-0325.mediawiki
It just happens that an empty byte vector is serialized identical to an empty vector of any other type, but it takes the reader an extra step. Thus, the minor fixup suggestion.
-
maflcko commented at 5:27 PM on August 22, 2020: member
Wouldn't it just be accurate to make it say that it's encoded just like BIP141?
Jup. Thx & done
- benthecarman approved
-
maflcko commented at 6:02 AM on August 25, 2020: member
cc @kallewoof
-
-
ajtowns commented at 6:36 AM on September 3, 2020: contributor
Huh? An empty vector of any type is serialised as a 1-byte block where that byte is
\x00(indicating the size of the vector is zero), the text of the BIP and the corresponding code currently has it serialized as a 0-byte block. Unless I'm missing something, this is a change, not a clarification -
maflcko commented at 6:54 AM on September 3, 2020: member
Currently the serializer can choose whether to serialize it as a 0-byte-block or a 1-byte-block (
\x00). Is there any reason to allow this freedom and require implementations to deal with the edge case? -
ajtowns commented at 7:19 AM on September 3, 2020: contributor
Hmm, I thought there was code that would error out in the 1-byte-
\x00case, but doesn't look like there ever was. Don't think there's any reason for the freedom; the intent was just to save a byte in every block when you don't need witness data. Changing this would require a chain restart of the current default signet I believe. -
kallewoof commented at 7:39 AM on September 3, 2020: contributor
~I'm not sure why it would require a reset since the default signet doesn't accept empty witnesses.~ (It does; I thought we used the witness, not the scriptSig.)
-
maflcko commented at 7:42 AM on September 3, 2020: member
If a reset is needed can be tested by running a reindex with the following diff:
- if (!v.empty()) v >> tx_spending.vin[0].scriptWitness.stack; + v >> tx_spending.vin[0].scriptWitness.stack; -
kallewoof commented at 7:52 AM on September 3, 2020: contributor
Thanks - yeah, it fails as we're using the scriptSig, not the scriptWitness. Alas.
-
ajtowns commented at 1:58 PM on September 5, 2020: contributor
I think there's a problem with signet's difficulty adjustments that requires a chain restart, so I think that makes the code simplification (at the cost of an extra byte per block) worthwile.
-
ajtowns commented at 4:10 AM on September 8, 2020: contributor
ACK, the code PR has been updated to match this change
-
- luke-jr merged this on Oct 5, 2020
- luke-jr closed this on Oct 5, 2020
- maflcko deleted the branch on Nov 17, 2021
Contributors
