Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix specification of genesis coinbase height #540

Closed
teor2345 opened this issue Aug 4, 2021 · 6 comments
Closed

Fix specification of genesis coinbase height #540

teor2345 opened this issue Aug 4, 2021 · 6 comments

Comments

@teor2345
Copy link
Contributor

teor2345 commented Aug 4, 2021

Recently, we updated the spec with the coinbase height encodings for:

  • heights 1-16: 0x50 + height
  • other heights: a minimal signed little-endian encoding, prefixed with a length

However, the genesis block does not use either of these encoding schemes. And it starts with a 0x04 byte, which is a valid length under the "other heights" encoding scheme. (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. 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 transaction version. (Or any fields that depend on that version, or the new NU5 coinbase expiry height rule.)

The exact genesis coinbase data is:
https://github.com/ZcashFoundation/zebra/blob/24bf952e982bde28eb384b211659159d46150f63/zebra-chain/src/transparent/serialize.rs#L20-L25

@daira
Copy link
Collaborator

daira commented Aug 4, 2021

I don't think there is a spec bug here. The genesis block (at height 0 of any chain) is explicitly excluded from coinbase height checks in section 7.1.2 of the protocol spec:

image

and in section 7.12:

image

Thanks for pointing out the potential ambiguity at height 520617983, but I think this is resolved by just not trying to parse the coinbase data at height 0. (The spec doesn't claim that you can!)

@daira daira closed this as completed Aug 4, 2021
@teor2345
Copy link
Contributor Author

teor2345 commented Aug 4, 2021

How can implementations distinguish between a genesis block, and a block at height 520,617,983 which has the exact same coinbase data as a genesis block?

Or alternately, how can implementations calculate or verify the height of a genesis block?

Edit:

If Zcash chooses not to specify how to identify genesis blocks, that's ok. Zebra can just keep checking the entire genesis coinbase data. And assuming that the far future block at height 520,617,983 won't have that coinbase data.

@daira
Copy link
Collaborator

daira commented Aug 5, 2021

I think the assumption is that the genesis block for a given chain is baked-in, so you never need to get it from other nodes. In particular you know the hash of the genesis block, so you could filter it out if another node did send it to you. Also the genesis block has an all-zeros prevHash, which no other block can have assuming preimage-resistance of SHA-256d. In any case, the behaviour here hasn't changed from Bitcoin 0.11.2.

@teor2345
Copy link
Contributor Author

teor2345 commented Aug 6, 2021

In particular you know the hash of the genesis block, so you could filter it out if another node did send it to you. Also the genesis block has an all-zeros prevHash, which no other block can have assuming preimage-resistance of SHA-256d.

Zebra checks the genesis previous block hash and also the genesis block hash. But we don't have that information available during coinbase height parsing.

So for now, we'll just check the coinbase data during height parsing.

@daira
Copy link
Collaborator

daira commented Aug 6, 2021

I'm a little confused as to why you don't have the block header, since you need merkleHash in order to know you have the correct transactions for that block, including the coinbase tx.

@teor2345
Copy link
Contributor Author

teor2345 commented Aug 9, 2021

It's just not passed to the method that does the parsing, because of how our serialization is implemented.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants