feat: Deep tree structure with Updates

[Manav-Aggarwal]

This PR is just a reference to #5 which adds a deep tree structure with updates, however, it was forked off of master which uses cosmos-db as the underlying DB, however, the default branch in cosmos-sdk-rollmint: release/v0.46.x-rollmint uses iavl 0.19.4 which corresponds to release/v0.19.x in this repo and uses tm-db. Also, it was opened by @tzdybal so he could not approve it. We'll use this PR as a proxy for that PR in order for approval.

Please look at #5 for relevant comments.

[codecov-commenter]

Codecov Report

❗ No coverage uploaded for pull request base (deepsubtrees@f4499a9). Click here to learn what that means.
The diff coverage is n/a.

@@               Coverage Diff               @@
##             deepsubtrees      #13   +/-   ##
===============================================
  Coverage                ?   64.24%           
===============================================
  Files                   ?       28           
  Lines                   ?     4645           
  Branches                ?        0           
===============================================
  Hits                    ?     2984           
  Misses                  ?     1278           
  Partials                ?      383           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[MSevey]

I don't think this is a valid reason to ignore the error because this doesn't handles the case of a corrupt hash and nil node, only the case when node is nil and hash is nil, so GetNode is returning nil, ErrNodeMissingHash. But we would never hit that case because we are checking for > 0 at the start. So we know that if we are calling GetNode we are calling it with a non empty hash.

Therefore the errors we should be getting from GetNode should be an error getting the node or making the node, both of which seem like errors we should care about.

[Manav-Aggarwal]

Updated the comment. The error starting with Value missing for hash is expected with a deep subtree since it does not contain all nodes but can contain left hashes. ErrMissingHash will never be true since we are checking > 0 like you said.

[MSevey]

Building off of my other comment, we should either explain why it is ok to return without an error and still a nil node.leftNode in this case, or we should update this to the following as an additional protection against mismatches.

Suggested change

	} else if !bytes.Equal(node.leftNode.hash, node.leftHash) {
	}
	if !bytes.Equal(node.leftNode.hash, node.leftHash) {

[Manav-Aggarwal]

This is an invalid code since if node.leftNode == nil is true, then node.leftNode.hash will crash. GetNode can return a nil value for node.leftNode and that's completely fine.

Code suggested would cause crashes.