Investigate app_hash / no_empty_block issue

[ethanfrey]

There is a problem in cosmos-sdk with constantly changing app_hash: cosmos/cosmos-sdk#520

Investigate if this also affects weave. We have different app logic, but also rely on the same underlying merkle store.

[ebuchman]

So, the new IAVL updates the version with each call to Commit, which is why the app hash changes with each block.

I'm not sure if we want to change this. Instead we're considering adding a mechanism to ResponseEndBlock to allow it control whether/when new empty blocks are produced. See tendermint/tendermint#1392

[ethanfrey]

Hey @ebuchman thanks for the comment.

I was looking into this a bit, and it seems that the iavl tree
returns an app hash based on data and the version number

// SaveVersion saves a new tree version to disk, based on the current state of
// the tree. Returns the hash and new version number.
func (tree *VersionedTree) SaveVersion() ([]byte, int64, error) {

If I understand that code correctly, if there are empty blocks, the hash returned from SaveVersion will remain constant, but the second value (the version) will increment. If I return the hash of the IAVL tree directly in Commit, then the app should be compatible with no empty blocks.

As to the cosmos-sdk issue, it seems:

• IAVLStore.Commit returns the version and hash directly from tendermint/iavl
• RootMultiStore, then calculates a hash of all substores, which embed the entire CommitInfo (version+hash) of each substore

And it seems the issue comes from the second, if I understand properly.

The store in weave looks similar to the IAVLStore in cosmos-sdk, and we only
return the hash to tendermint

Seems like this should pose no problem, as long only the version, not the hash, in the iavl tree changes on an empty block. As for cosmos-sdk, can't you just create the root hash by doing SimpleHashFromMap of a map of store-name to iavl hash (without version)? This would allow no empty blocks with no tendermint modifications.

I mean, the root multi store should ensure that all substores have the same version, right? If that is the case, there is no need to include the version in a hash, just check them all and error/panic if any are different.

[ethanfrey]

I ran this with tendermint v0.19.7 and bov v0.4.0, with the following config options:

create_empty_blocks = false
create_empty_blocks_interval = 300

I can confirm it produced two blocks on startup, and one 5 minutes later

I[06-21|14:32:31.650] Info synced                                  module=bov height=0 hash=
D[06-21|14:32:34.723] Commit synced                                module=bov height=1 hash=17DFE24D7D8A47D2298C87E4731718BC5DD27AAF
D[06-21|14:32:35.755] Commit synced                                module=bov height=2 hash=17DFE24D7D8A47D2298C87E4731718BC5DD27AAF
D[06-21|14:37:36.785] Commit synced                                module=bov height=3 hash=17DFE24D7D8A47D2298C87E4731718BC5DD27AAF

Looks like we are safe here... now, please convince Jae to fix up the multistore implementation and cosmos-sdk will fly again 😄