zcash / zips

Zcash Improvement Proposals
https://zips.z.cash
MIT License
274 stars 156 forks source link

Consider removing version_group_id from V6 and future transactions. #497

Open nuttycom opened 3 years ago

nuttycom commented 3 years ago

https://github.com/zcash/zips/issues/494 identifies the need to include the consensus_branch_id value in the transaction format to avoid potential mismatches between transaction IDs computed before and after a network upgrade boundary.

After this change, the version_group_id field will be somewhat vestigial. It currently serves to allow parsing to short-circuit in the case of a poorly-implemented fork of Zcash, anticipating that the human-readable part of the version might be the same on different chain forks, but that the version group id would be changed by an entity performing a fork. However, the consensus branch ID can serve this purpose as well as the version group ID can, and removing the version group ID simplifies reasoning about the protocol as follows:

The parser for a transaction should be selected according to the transaction version number. As the first step in parsing, the consensus branch ID must be parsed, and parsing should halt if the consensus branch ID is not recognized. This provides a clean separation of roles:

str4d commented 3 years ago

The main problem with this proposal is it makes transaction parsing contextual: it becomes impossible to parse a transaction without knowing the full set of chain NUs. Obviously when a new transaction version is introduced this doesn't matter (in the current design the consensus branch ID and version group ID change together in that case). But e.g. from NU5 the v4 transaction format will still be valid; with this proposed change, v4 transaction parsers that worked perfectly fine before the NU would suddenly halt after.

nuttycom commented 3 years ago

I don't think that's true? https://github.com/zcash/zcash/blob/master/src/primitives/transaction.h#L610 produces a correct parse failure now. Post v6, a parser would first inspect the transaction version; for versions 5 and less, it would interpret the following bytes as a version group ID; for versions 6 and greater it would interpret the following bytes as a consensus branch ID.

I don't think that it's possible for a conformant V4 parser to misinterpret a (proposed) V6 transaction in any way that would introduce a parse ambiguity.