RFC-002: non-zero genesis

[erikgrinaker]

Rendered

Initial draft to discuss non-zero genesis. Primary question to answer: should genesis include a link to the previous chain (e.g. block header), and should this link be verified and tied into the initial block of the new chain or only be advisory for users and integrators?

[ValarDragon]

I don't think any of the chain linking options differ in guarantees to any node types, but I strongly prefer weak chain linking.

The four different guarantees to consider are what does it do for a synced /unsynced full node, and synced / unsynced lite clients. Here is my understanding for why it doesn't affect security for them.

A synced full nodes guarantee state integrity by replaying all of the computation themselves. Correspondingly, they should replay the code to produce the new fork's genesis file and use that. There is then no need for them to reverify the new genesis, and chain linking didn't prove anything new.

A synced Lite clients guarantee state integrity by trusting 2/3rds of validators and then just tracking consensus. Even with strong chain linking, the lite client gets no guarantee that the newly derived state root is correct. The only way to actually efficiently check the new state root would be if you had a SNARK proof. (Moreover, one without preprocessing)

An unsynced full node/lite client needs to trust some genesis point. Maybe chain linking helps with this? (a bit hard to reason about, since this trust is based of a social consensus) But if so, imo weak chain linking should suffice. If they want stronger guarantees, they should just check the header using the prior TM version and some other tooling.

However, I do think weak chain linking is a good idea for overall clarity, and debuggability.

[erikgrinaker]

Thanks @ValarDragon! @cwgoes @melekes Do you share the assessment above? I.e. would ICS or light client security be improved by a strong link from a hard forked chain to the last block of the previous chain, or would it be useless without having the full block history on the new chain?

[cwgoes]

I do not think strong linking vs weak linking itself matters for IBC security - as @ValarDragon mentioned, the light client can't check the state root anyways - what absolutely does matter for IBC security (and light client security generally), however, is whether evidence will remain valid across an upgrade, e.g. if validators misbehave after an upgrade has occurred but within the unbonding period (double-signing block 49, if the upgrade was at block 50), they need to be held accountable after the upgrade just as if no upgrade had occurred. This will require persistence of state in the SDK (easy) and Tendermint (not sure how difficult). Is this proposal intended to address that problem (it had been discussed previously), or is that out of scope?

[cmwaters]

Just briefly looking into the evidence code, I don't think it will be too difficult to do. The evidence pool requires access to both the evidence db and the state db. If both of these persist after the upgrade i.e. have access to the validator sets at heights before the upgrade then the only problem lies with having access to the previous chainID and knowing at which heights to use the old chainID and which heights to use the new one.

[melekes]

What sort of benefits does strong chain-linking provide? I can't see any personally. Unless you put pre-upgrade header into LastBlockID (~ actually link them together), but I do not see us benefiting from that in any way. Only more complexity.

Weak chain-linking sounds good, but it feels weird. chainID and header's hash should be enough imho for a) external tools to see succession b) Tendermint to verify evidence from previous chain (we'll have to put pre-upgrade chainID into the state).

👍 for initial height.

[erikgrinaker]

Thanks for your feedback everyone. It sounds like we'll be using an initial_height genesis setting, in addition to weak chain linking (chain ID and either last block header or last block hash, ignored by Tendermint). I'll update the RFC with a concrete proposal.

@ebuchman In tendermint/tendermint#2543 you mentioned wanting to link the chains more strongly (e.g. "Current code expects all blocks with height > 1 to have a valid LastCommit, and we probably want to preserve this property"), do you want to weigh in here?

what absolutely does matter for IBC security (and light client security generally), however, is whether evidence will remain valid across an upgrade [...] Is this proposal intended to address that problem (it had been discussed previously), or is that out of scope?

I see, thank you for the clarification. State migration across upgrades is out of scope for this proposal.

The evidence pool requires access to both the evidence db and the state db. If both of these persist after the upgrade i.e. have access to the validator sets at heights before the upgrade then the only problem lies with having access to the previous chainID and knowing at which heights to use the old chainID and which heights to use the new one

Yes, ideally we should maintain backwards compatibility with old state across upgrades, in which case we may not even need to hard fork at all. However, that is a separate issue requiring more spec and implementation work - this proposal is a stopgap solution until then.

[erikgrinaker]

I've updated the RFC with a new (final?) proposal. The only outstanding question is whether we should include the entire last block header of the previous chain, or only the hash. My vote is yes, to remove a level of indirection for users.

[josef-widder]

Does that mean that the epoch in the Heights defined in ICS007(Height) will not be used? Or it depends on the application whether they choose to increment the epoch or start with a fresh initial height?

[erikgrinaker]

Does that mean that the epoch in the Heights defined in ICS007(Height) will not be used? Or it depends on the application whether they choose to increment the epoch or start with a fresh initial height?

I'll let @cwgoes weigh in, but since networks can choose whether to restart from 0 or not during a hard fork it should probably still be supported. Since evidence is not kept across hard forks anyway, we may want to increment the epoch for hard forks even when the height is not reset.

[cwgoes]

I'll let @cwgoes weigh in, but since networks can choose whether to restart from 0 or not during a hard fork it should probably still be supported. Since evidence is not kept across hard forks anyway, we may want to increment the epoch for hard forks even when the height is not reset.

Currently, IBC expects the epoch to be encoded into the chain ID (this is a bit of a hack to ensure that the epoch is signed over, which it wouldn't be otherwise).

[melekes]

How would network governors verify it? I.e., imagine Alice has genesisA.json, Bob has genesisB.json. Since we do not hash a predecessor field, both Alice and Bob can have different predecessor blocks, and there's no way for me to find what is the "right" one. Then what's the reason for putting it in genesis in the first place.

[erikgrinaker]

Oh, that's a good point. I somehow assumed that the entire genesis file was hashed into the first block. Does this field even have any value if it can't be trusted or agreed upon?

[melekes]

no. I too thought we hash entire genesis into the first block or something like that a while ago, but no 😄

[alexanderbez]

I recall there was a proposal to do this long ago, but it never happened it seems.

[erikgrinaker]

Ok, in that case I think we should just remove it. What's your take @ValarDragon?

[ebuchman]

Isn't all the data in the genesis effectively hashed in to the header already? the header of the first block i think should use the genesis time since there's otherwise no commit to draw it from, and everything else should be hashed in two (consensus params, validators, app state).

we don't take a literal hash of the json file but we should be able to verify one. though I guess that requires computing the application's merkle state from the app_state_bytes in the genessis, which might be prohibitive if you're just trying to verify that a genesis file is correct.

Not sure the best way to do it but possible the first LastBlockID should include the hash of the header or maybe we introduce a block-0 that has the genesis as the first transaction or something

[ebuchman]

Huh I think the AppHash may not be currently making it into the first block because abci.InitChain does not return the app hash. It probably should?

[erikgrinaker]

Isn't all the data in the genesis effectively hashed in to the header already?

Yes, mostly. This appears sufficient to secure the current genesis file. However, we're considering adding a new predecessor field, but that field then won't be hashed into any blocks, and thus cannot be trusted.

Since it cannot be trusted it could be a potential social engineering attack vector, and I think we should err on the side of caution and drop this field until we find a way to secure the entire genesis file. The field is not actually used for anything anyway, so it's not like we need it.

maybe we introduce a block-0 that has the genesis as the first transaction or something

Would it be horrible to add a block field that's only used in the initial block?

the AppHash may not be currently making it into the first block because abci.InitChain does not return the app hash

Seems so. That probably means that the genesis file app hash isn't verified either. I opened tendermint/tendermint#5177 for this, we'll see if we can squeeze this in for 0.34 (the RC is already out).

[ValarDragon]

So should we open a separate issue in Tendermint to anchor the blockhash with the entire genesis file, and then do weak chain linking?

[erikgrinaker]

Yeah, I've added tendermint/tendermint#5183

[erikgrinaker]

Following discussion in #119 (review) I've removed the predecessor field for chain linking, since it won't be hashed into the genesis block and thus can't be trusted. We can consider adding it later, e.g. if and when we hash the entire genesis file into the genesis block (tendermint/tendermint#5183).

I believe this PR should be good to go then.

[erikgrinaker]

Prototype implementation (tendermint/tendermint#5191) was simplified greatly by including the initial height in the node state as State.InitialHeight, so I've updated the RFC with this.

[tac0turtle]

LGTM,

[ebuchman]

Haven't had a chance to review yet, sorry I'm late, will get to it next week (off the next few days). Just a note some potentially related work in https://github.com/tendermint/tendermint/blob/master/docs/architecture/adr-017-chain-versions.md incase it's helpful

[melekes]

lgtm