Cosmos-sdk: Return app hash from `InitChain`

Created on 12 Aug 2020  路  11Comments  路  Source: cosmos/cosmos-sdk

Summary

In https://github.com/tendermint/tendermint/pull/5227 (will be released in 0.34.0-rc3), Tendermint added ResponseInitChain.app_hash which replaces the value in the genesis file and is recorded in the genesis block. The SDK should probably return the app hash here instead.


For Admin Use

  • [ ] Not duplicate issue
  • [ ] Appropriate labels applied
  • [ ] Appropriate contributors tagged
  • [ ] Contributor assigned/self-assigned
ABCI

Most helpful comment

Perhaps yeah, but in InitChain, I think using the last commit ID should work.

All 11 comments

The genesis docs that I've thus far seen typically don't have an AppHash. How is this typically constructed or evaluated? I hope this won't be difficult to implement because, in order to get an app hash during InitChain, we have to commit the root multi-store which bumps the version.

In that case I guess you would just return an empty hash from InitChain. This is application-specific, the motivation was to record the initial app hash in the genesis block and validate it against what's specified in the genesis file. Note that after https://github.com/cosmos/iavl/pull/304 IAVL will return a hash for an empty tree with no versions.

Ok I guess, we can just return an empty hash for now?

That will work, although the hash of an empty input would probably be more consistent with what IAVL and Tendermint does.

Not sure how this affects upgrades - I believe the genesis file then contains some app state and probably an app hash as well, so InitChain would have to return the same app hash as specified in the genesis file.

I believe there might be some overlap or common bits to shared with https://github.com/cosmos/cosmos-sdk/issues/7018.

Following user feedback I've changed the behavior such that ResponseInitChain.app_hash replaces the value from the genesis file instead of having to match it. We may want to e.g. remove the genesis file app hash completely before the final 0.34 release, added https://github.com/tendermint/tendermint/issues/5238 to discuss this and would appreciate the SDK team's input on this.

Just thinking out loud, I think the app_hash only has any semantic meaning when starting from a fork/ugprade, otherwise, there is no pre-existing state and we don't commit on InitChain. With that in mind, maybe we can just return LastCommitID.Hash? In the case of a new chain, it'll be empty. During an upgrade, it'll be the hash of the last committed block.

Thoughts @zmanian?

I don't know the details of the SDK's state and hashing, but in general, identical states should give identical hashes. If height 2 is empty (contains no user-submitted transaction data), presumably height 1 should have the same hash as height 2?

Not quite, even if there is no new state between 1 and 2, the height/version is increased which gives you a new hash.

That must be because the SDK changes its internal state then. IAVL returns the same hash for version 1 and 2 if there were no changes.

Either way, this is application-specific, so the SDK is free to choose whatever hashing scheme it finds appropriate. Not having to special-case version 1 for e.g. proofs and stuff might be helpful to users though, but probably not a big deal.

Perhaps yeah, but in InitChain, I think using the last commit ID should work.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

ValarDragon picture ValarDragon  路  3Comments

ValarDragon picture ValarDragon  路  3Comments

rigelrozanski picture rigelrozanski  路  3Comments

ValarDragon picture ValarDragon  路  3Comments

rigelrozanski picture rigelrozanski  路  3Comments