Number format(s) specification

[A60AB5450353F40E]

Reading on Bitcoin Cash number formats I found something which confused me...

Transaction format references a "variable length integer", and in the spec. it's defined here: https://reference.cash/protocol/formats/variable-length-integer

However, the actual code calls that same format "compact size": https://gitlab.com/bitcoin-cash-node/bitcoin-cash-node/-/blob/master/src/serialize.h#L353

and the "variable-length integer" is something else, not found in the spec: https://gitlab.com/bitcoin-cash-node/bitcoin-cash-node/-/blob/master/src/serialize.h#L423

[A60AB5450353F40E]

For example, BTC docs clearly states "compactSize uint" where our docs say "variable length integer" for the same field (output script length).

The BTC docs clearly disambiguate the two: https://btcinformation.org/en/glossary/compactsize

https://btcinformation.org/en/developer-reference#compactsize-unsigned-integers

[A60AB5450353F40E]

I hurried a bit to make that PR sorry for the spam, maybe your idea was to have only consensus stuff in there, and the other VarInt doesn't belong there as it's used for local storage while CompactInt is in the TX raw format so is consensus. I feel it needs to be disambiguated from the other VarInt encoding... simplest could be to rename this to "Compact Variable Length Integer"

[joshmg]

No worries about the multiple comments! Better to get something posted and iterate than to spend too long perfecting and never get anything out there. We appreciate the feedback. :)

@thesquaregroot and I talked a little about #32 this morning. Our intention isn't to match any particular implementation's verbiage (a lot of it are C++ colloquialisms which is inappropriate for a larger audience, i.e. "uint" or "size_t", etc), but at the same time we're not looking to rename things simply for the sake of it. In this particular case, the name collision is unfortunate and our inclination is to fix that in some capacity.

I think we're inclined to take your most recent suggestion and rename what the specification calls "variable length integer" to "compact variable length integer". It's similar enough to the C++ -ism such that anyone with that context will understand the intended parallel, while also avoiding the name collision of BCHN/Core's internal VarInt.

Ultimately, we want to be as clear and concise to as wide an audience as possible, so if you have other suggestions I'd be happy to discuss them here. If you still agree "compact variable length integer" is a sufficiently clear and concise terminology for this then let's go with that. If you submit a PR with that change I'd gladly merge it, otherwise I'll put the change in our backlog and get to it in the coming weeks.

[A60AB5450353F40E]

Great, ok so I updated that PR just to change the title to "Compact Variable Length Integer".

We don't have an entry in the docs for the other one, though. So, any ideas what to call these? Chained variable length integers? High bit terminated variable length integers?

[thesquaregroot]

@A60AB5450353F40E After looking into it a bit, the other VarInt format you're referring to looks like it is purely used internally in the C++ codebases for on-disk serialization. If that's correct, this format is effectively just an implementation detail of those nodes and not actually a part of the protocol, which would explain why it doesn't appear in the spec.

Let me know if I'm missing something though and we can investigate further.

[A60AB5450353F40E]

@thesquaregroot that's right, the other one is internal and implementation specific so no reason to be in the spec at all. With https://github.com/BitcoinUnlimited/BitcoinCashSpecification/pull/34 being merged I think we can close this.