Problem: memiavl's root hash is not safe to retain in commit info (#1150

)

* Problem: memiavl's root hash is not safe to retain in commit info

Solution:
- clone the byte slices when nesserary

* Update CHANGELOG.md

Signed-off-by: yihuang <[email protected]>

* add unit test that reproduce the issue

---------

Signed-off-by: yihuang <[email protected]>

CHANGELOG.md

@@ -37,6 +37,7 @@
 - [#1123](https://github.com/crypto-org-chain/cronos/pull/1123) Fix memiavl snapshot switching
 - [#1125](https://github.com/crypto-org-chain/cronos/pull/1125) Fix genesis migrate for feeibc, evm, feemarket and gravity.
 - [#1130](https://github.com/crypto-org-chain/cronos/pull/1130) Fix lock issues when state-sync with memiavl.
+- [#1150](https://github.com/crypto-org-chain/cronos/pull/1150) Fix memiavl's unsafe retain of the root hashes.
 
 ### Features
 

memiavl/db_test.go

@@ -280,23 +280,36 @@ func TestLoadVersion(t *testing.T) {
 }
 
 func TestZeroCopy(t *testing.T) {
-	db, err := Load(t.TempDir(), Options{InitialStores: []string{"test"}, CreateIfMissing: true, ZeroCopy: true})
+	db, err := Load(t.TempDir(), Options{InitialStores: []string{"test", "test2"}, CreateIfMissing: true, ZeroCopy: true})
 	require.NoError(t, err)
-	db.Commit([]*NamedChangeSet{
+	_, _, err = db.Commit([]*NamedChangeSet{
 		{Name: "test", Changeset: ChangeSets[0]},
+		{Name: "test2"},
 	})
+	require.NoError(t, err)
 	require.NoError(t, errors.Join(
 		db.RewriteSnapshot(),
 		db.Reload(),
 	))
 
+	// the test tree's root hash will reference the zero-copy value
+	_, _, err = db.Commit([]*NamedChangeSet{
+		{Name: "test"},
+		{Name: "test2", Changeset: ChangeSets[0]},
+	})
+	require.NoError(t, err)
+
+	commitInfo := *db.LastCommitInfo()
+
 	value := db.TreeByName("test").Get([]byte("hello"))
 	require.Equal(t, []byte("world"), value)
 
 	db.SetZeroCopy(false)
 	valueCloned := db.TreeByName("test").Get([]byte("hello"))
 	require.Equal(t, []byte("world"), valueCloned)
 
+	_ = commitInfo.CommitID()
+
 	require.NoError(t, db.Close())
 
 	require.Equal(t, []byte("world"), valueCloned)
@@ -307,6 +320,9 @@ func TestZeroCopy(t *testing.T) {
 	require.Panics(t, func() {
 		require.Equal(t, []byte("world"), value)
 	})
+
+	// it's ok to access after db closed
+	_ = commitInfo.CommitID()
 }
 
 func TestWalIndexConversion(t *testing.T) {

memiavl/mem_node.go

@@ -70,6 +70,10 @@ func (node *MemNode) Mutate(version, cowVersion uint32) *MemNode {
 	return n
 }
 
+func (node *MemNode) SafeHash() []byte {
+	return node.Hash()
+}
+
 // Computes the hash of the node without computing its descendants. Must be
 // called on nodes which have descendant node hashes already computed.
 func (node *MemNode) Hash() []byte {

memiavl/node.go

@@ -20,6 +20,9 @@ type Node interface {
 	Right() Node
 	Hash() []byte
 
+	// SafeHash returns byte slice that's safe to retain
+	SafeHash() []byte
+
 	// PersistedNode clone a new node, MemNode modify in place
 	Mutate(version, cowVersion uint32) *MemNode
 

memiavl/persisted_node.go

@@ -131,6 +131,10 @@ func (node PersistedNode) Right() Node {
 	return PersistedNode{snapshot: node.snapshot, index: node.index - 1}
 }
 
+func (node PersistedNode) SafeHash() []byte {
+	return bytes.Clone(node.Hash())
+}
+
 func (node PersistedNode) Hash() []byte {
 	if node.isLeaf {
 		return node.leafNode().Hash()

memiavl/tree.go

@@ -162,12 +162,13 @@ func (t *Tree) Version() int64 {
 	return int64(t.version)
 }
 
-// RootHash updates the hashes and return the current root hash
+// RootHash updates the hashes and return the current root hash,
+// it clones the persisted node's bytes, so the returned bytes is safe to retain.
 func (t *Tree) RootHash() []byte {
 	if t.root == nil {
 		return emptyHash
 	}
-	return t.root.Hash()
+	return t.root.SafeHash()
 }
 
 func (t *Tree) GetWithIndex(key []byte) (int64, []byte) {