kroma-network / go-ethereum

go-ethereum for Kroma
GNU Lesser General Public License v3.0
44 stars 23 forks source link

[ZKTRIE-003] `MerkleTree.Delete` can incorrectly update the root node if removing a leaf at level 1. #82

Closed this-is-chainlight closed 5 months ago

this-is-chainlight commented 6 months ago

Summary

MerkleTree.Delete can incorrectly update the root node if removing a leaf at level 1.

Description

MerkleTree.Delete can incorrectly update the root node if removing a leaf at level 1. If a LeafNode at level 1 is deleted, its sibling is being promoted to the root node. This behavior is incorrect when the sibling is a ParentNode, as it changes the path prefix of all nodes below the promoted ParentNode. Instead, the deleted LeafNode should be replaced by an EmptyNode.

Impact

High

  1. Some value on the state db can be removed.
  2. So that it can miscalculate the state root hash, and it leads to the fork.

Recommendation

diff --git a/trie/zk/merkle_tree.go b/trie/zk/merkle_tree.go
index 3ea84c98c..5c3cbd38b 100644
--- a/trie/zk/merkle_tree.go
+++ b/trie/zk/merkle_tree.go
@@ -275,10 +275,6 @@ func (t *MerkleTree) rmAndUpload(path TreePath, pathNodes []*ParentNode) {
        switch len(pathNodes) {
        case 0: // The leaf node you want to remove is root node.
                t.rootNode = EmptyNodeValue
-       case 1:
-               // root (ParentNode) --- LeafNode or ParentNode (promoted to root node)
-               //                    |- LeafNode (deleted)
-               t.rootNode = t.getChild(pathNodes[0], path.GetOther(0))
        default:
                lastSibling := t.getChild(pathNodes[len(pathNodes)-1], path.GetOther(len(pathNodes)-1))

Reference

Patch

Reported