BIP 325: Clarify that scriptWitness is a stack, not a byte string

[maflcko]

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.

[gmaxwell]

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.

[maflcko]

Wouldn't it just be accurate to make it say that it's encoded just like BIP141?

Jup. Thx & done

[maflcko]

cc @kallewoof

[kallewoof]

ACK

Please merge at your convenience (ping @luke-jr).

[ajtowns]

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]

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]

Hmm, I thought there was code that would error out in the 1-byte-\x00 case, 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]

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]

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]

Thanks - yeah, it fails as we're using the scriptSig, not the scriptWitness. Alas.

[ajtowns]

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]

ACK, the code PR has been updated to match this change

[kallewoof]

ACK, ping @luke-jr