Use simple merkle proof for commit info

[ethanfrey]

Description

Pulling out one part of the logic from #6178 and simplifying the changes

One step towards #5842

Currently we serialize the entire commitInfo in the root multistore proofs. This is not even a merkle proof, just all data. Before we can move to ics23 proofs, we need to use merkle proofs that can be transformed to that format (it is just an encoding, not the logic).

As discussed a few times with @cwgoes and now @AdityaSripal we use merkle.SimpleProofsFromMap to produce the commitment of the root multi store. This can later be converted into ics23 format, but this is a valid, testable step that can be merged to master and is an improvement over current state.

closes: #XXXX

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

---

For contributor use:

• Targeted PR against correct branch (see CONTRIBUTING.md)
• Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
• Code follows the module structure standards.
• Wrote unit and integration tests
• Updated relevant documentation (docs/) or specification (x/<module>/spec/)
• Added relevant godoc comments.
• Added a relevant changelog entry to the Unreleased section in CHANGELOG.md
• Re-reviewed Files changed in the Github PR explorer

[codecov]

Codecov Report

Merging #6323 into master will decrease coverage by 0.02%.
The diff coverage is 70.49%.

@@            Coverage Diff             @@
##           master    #6323      +/-   ##
==========================================
- Coverage   55.71%   55.69%   -0.03%     
==========================================
  Files         448      448              
  Lines       26978    26941      -37     
==========================================
- Hits        15032    15005      -27     
+ Misses      10869    10862       -7     
+ Partials     1077     1074       -3     

[ethanfrey]

Some comments for the reviewers

[ethanfrey]

Note to review: This was for handling the serialized StoreInfos[] as a proof, which is now removed in favor of a merkle commitment (which scales much better for dozens or more of substores)

[ethanfrey]

The storeInfo contains a name and a commitment (hash and version).
This method is not really used to "hash" it in a normal means, but rather pull out the hash that was already inside of it.

In effect, to make a chained proof op work, you would have to take the root hash from the iavlstore and then hash it before feeding it as a value to the merkle simple proof, which makes no sense. We want the simple proof to commit directly to the root hash of the sub stores (not a hash of a hash of a hash... - we are not bitcoin mining).

If anyone intends to use this as a real hashing op (which it never was used as), it must contain the name as well before feeding it to the hasher. In such a case we could rename this to eg. Commitment, which we would use to feed into commitInfo.Hash() and commitInfo.ProofOp().

[AdityaSripal]

Yes, believe this was why the hashing in TM had to be removed. This approach works better. Could you please put the cruz of this comment in the godoc. Namely that this does not actually hash the storeInfo, rather it returns the embedded store hash

[ethanfrey]

Yes, I can definitely add it to the GoDoc. I can also rename the function to make it more explicit that we are not trying to implement the Hasher interface but just access the substore hash.

Actually I can do both (rename an explain the purpose of the function). Any preferences for names?

[AdityaSripal]

Quick doc request, otherwise lgtm 👍

[AdityaSripal]

Yes, believe this was why the hashing in TM had to be removed. This approach works better. Could you please put the cruz of this comment in the godoc. Namely that this does not actually hash the storeInfo, rather it returns the embedded store hash

[cwgoes]

utACK

[ethanfrey]

Nice.
When this is merged, I can make another cleanup PR that builds on top.

[alexanderbez]

ACK

[alexanderbez]

Great work @ethanfrey -- please merge latest master and GH will automatically merge this for us.

[ethanfrey]

Thanks for the quick reply everyone 😄

I just rebased on top of latest master, waiting for the CI to do it's magic

[ethanfrey]

How does automerge work?

I rebased and passed all CI tests, but now it says: "Merging is blocked. The base branch restricts merging to authorized users". Looks like someone with write access will have to do so manually.