Protocol clean up: version & transaction type

[scravy]

https://github.com/dtr-org/unit-e/pull/1004 changes the type which transaction type is encoded as from uint16_t to uint8_t, which gives a maximum of 256 possible transaction types. The structure we inherit from bitcoin supports uint32_t transaction versions.

I for one do not see the need to distinguish "version" and "type", as "regular transaction version 2" can as well be just the type "regular transaction version 2". Same with votes, etc. It also makes little sense to differentiate "type X of version Y" etc. (too granular). We can simply have types like "vote" and "new_vote" (should it ever come to this).

In the code we so far overloaded the uint32_t version field to mean both version: uint16_t and txtype: uint16_t. This was kind of a hack and does not clearly distinguish these fields. Which leads to strange effects given bitcoins little-endian serialization. For example we technically have a "dead byte" as https://github.com/dtr-org/unit-e/pull/1004 was merged.

The proper clean up would be to clean up the protocol and have the implementation follow suite. My proposal is:

• Re-label the existing version field as "txtype" (effectively drops notion of "version" in favor of "type")
• Reduce it to "uint8_t" (as has been done more-or-less already)
• In code: Drop GetVersion / SetVersion / etc.

Other benefits: This frees 3 byte per transaction :-)

Forwards compatibility: In case we need a higher range in the future we can use the then-last-remaining txtype and have the new type of transaction define a discriminator for additional versions, i.e. we can say "txtype 256 requires a tx layout which itself is versioned".

[cmihai]

In case we need a higher range in the future we can use the then-last-remaining txtype and have the new type of transaction define a discriminator for additional versions, i.e. we can say "txtype 256 requires a tx layout which itself is versioned".

So basically, make TxType a varint?

[scravy]

So basically, make TxType a varint?

Basically.

[kostyantyn]

I support dropping Version and use only types. As an intermediate approach (if we find cases that Version is convenient to have), would suggest separating txtype and version. Instead of having one uint32_t field, have two: txtype(uint8_t) and version(uint16_t)

About:

For example we technically have a "dead byte" as #1004 was merged

I confirm that now we have unused 1 byte in nVersion.