Stop parsing a block height from the genesis block coinbase data

[teor2345]

Motivation

Zebra parses the genesis block coinbase data as a coinbase height, but that's not permitted by the spec.

The genesis block coinbase data starts with a 0x04 byte, which is also valid under the "other heights" encoding scheme in the spec. (The first 1+4 bytes correspond to height 520,617,983 under that scheme.) So it's not actually possible to specify an unambiguous encoding for the genesis coinbase data. Because a miner could theoretically insert the exact same coinbase data at block height 520,617,983.

However, the genesis and 520,617,983 coinbase transactions can be distinguished by:

• the genesis block header hash, or previous block hash field
• the transaction version (or any fields that depend on that version)
• the new NU5 coinbase expiry height rule

See zcash/zips#540 for some background to the spec, and Zebra's current behaviour.

Scheduling

This fix isn't required for the stable release. We should review the risks as part of the lightwalletd work.

This rule won't actually matter for 1000 years, and we already check the genesis block hash right after downloading (before parsing its height).

Zebra also rejects blocks that are too high after downloading and parsing the height, but before sending them to the verifier. So this issue can't cause any verifier bugs.

Specifications

A coinbase transaction for a block at block height greater than 0 MUST have a script that, as its first item, encodes the block height height as follows.
For height in the range {1 .. 16}, the encoding is a single byte of value 0x50 + height.
Otherwise, let heightBytes be the signed little-endian representation of height, using the minimum nonzero number of bytes such that the most significant byte is < 0x80. The length of heightBytes MUST be in the range {1 .. 5}. Then the encoding is the length of heightBytes encoded as one byte, followed by heightBytes itself.
This matches the encoding used by Bitcoin in the implementation of [BIP-34] (but the description here is to be considered normative).

https://zips.z.cash/protocol/protocol.pdf#txnconsensus

Suggested Solution

• Skip parsing the coinbase height in the script if the previous block hash is zero. Instead, return block height zero.

Rejected Solutions

Pass the transaction version to the coinbase height parser. Rejected because it's more complicated.

After NU5 or for v5 coinbase transactions, use the coinbase transaction expiry height. For earlier blocks, parse the coinbase height in the script. Rejected because it is hard to identify NU5 blocks using the block header or transaction data, and coinbase transactions could be v4 transactions.

Use the block header hash instead of the previous block hash. Rejected because it is more expensive to calculate the hash.

Related Work

• Require minimal height encodings during deserialization #2226

[ftm1000]

redistributing issues that can be separately worked from Epic #2311

[teor2345]

This is a consensus rule edge case that will never happen.