From b8615797c7cfff7e6f30c94f8705bc809d1d433f Mon Sep 17 00:00:00 2001
From: Quentin McGaw <quentin.mcgaw@gmail.com>
Date: Tue, 22 Nov 2022 15:11:08 +0000
Subject: [PATCH] fix(trie): equality differentiate nil and empty storage
 values

---
 internal/trie/node/subvalue.go      | 17 +++++++++
 internal/trie/node/subvalue_test.go | 57 +++++++++++++++++++++++++++++
 lib/trie/trie.go                    |  4 +-
 3 files changed, 76 insertions(+), 2 deletions(-)
 create mode 100644 internal/trie/node/subvalue.go
 create mode 100644 internal/trie/node/subvalue_test.go

diff --git a/internal/trie/node/subvalue.go b/internal/trie/node/subvalue.go
new file mode 100644
index 00000000000..e868b2394f5
--- /dev/null
+++ b/internal/trie/node/subvalue.go
@@ -0,0 +1,17 @@
+// Copyright 2022 ChainSafe Systems (ON)
+// SPDX-License-Identifier: LGPL-3.0-only
+
+package node
+
+import "bytes"
+
+// StorageValueEqual returns true if the node storage value is equal to the
+// storage value given as argument. In particular, it returns false
+// if one storage value is nil and the other storage value is the empty slice.
+func (n Node) StorageValueEqual(stoageValue []byte) (equal bool) {
+	if len(stoageValue) == 0 && len(n.StorageValue) == 0 {
+		return (stoageValue == nil && n.StorageValue == nil) ||
+			(stoageValue != nil && n.StorageValue != nil)
+	}
+	return bytes.Equal(n.StorageValue, stoageValue)
+}
diff --git a/internal/trie/node/subvalue_test.go b/internal/trie/node/subvalue_test.go
new file mode 100644
index 00000000000..ad59251864d
--- /dev/null
+++ b/internal/trie/node/subvalue_test.go
@@ -0,0 +1,57 @@
+// Copyright 2022 ChainSafe Systems (ON)
+// SPDX-License-Identifier: LGPL-3.0-only
+
+package node
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func Test_Node_StorageValueEqual(t *testing.T) {
+	t.Parallel()
+
+	testCases := map[string]struct {
+		node     Node
+		subValue []byte
+		equal    bool
+	}{
+		"nil node subvalue and nil subvalue": {
+			equal: true,
+		},
+		"empty node subvalue and empty subvalue": {
+			node:     Node{StorageValue: []byte{}},
+			subValue: []byte{},
+			equal:    true,
+		},
+		"nil node subvalue and empty subvalue": {
+			subValue: []byte{},
+		},
+		"empty node subvalue and nil subvalue": {
+			node: Node{StorageValue: []byte{}},
+		},
+		"equal non empty values": {
+			node:     Node{StorageValue: []byte{1, 2}},
+			subValue: []byte{1, 2},
+			equal:    true,
+		},
+		"not equal non empty values": {
+			node:     Node{StorageValue: []byte{1, 2}},
+			subValue: []byte{1, 3},
+		},
+	}
+
+	for name, testCase := range testCases {
+		testCase := testCase
+		t.Run(name, func(t *testing.T) {
+			t.Parallel()
+
+			node := testCase.node
+
+			equal := node.StorageValueEqual(testCase.subValue)
+
+			assert.Equal(t, testCase.equal, equal)
+		})
+	}
+}
diff --git a/lib/trie/trie.go b/lib/trie/trie.go
index 16063b0a969..bf77acfc64c 100644
--- a/lib/trie/trie.go
+++ b/lib/trie/trie.go
@@ -370,7 +370,7 @@ func (t *Trie) insertInLeaf(parentLeaf *Node, key, value []byte,
 	newParent *Node, mutated bool, nodesCreated uint32) {
 	if bytes.Equal(parentLeaf.PartialKey, key) {
 		nodesCreated = 0
-		if bytes.Equal(parentLeaf.StorageValue, value) {
+		if parentLeaf.StorageValueEqual(value) {
 			mutated = false
 			return parentLeaf, mutated, nodesCreated
 		}
@@ -451,7 +451,7 @@ func (t *Trie) insertInBranch(parentBranch *Node, key, value []byte,
 	copySettings := node.DefaultCopySettings
 
 	if bytes.Equal(key, parentBranch.PartialKey) {
-		if bytes.Equal(parentBranch.StorageValue, value) {
+		if parentBranch.StorageValueEqual(value) {
 			mutated = false
 			return parentBranch, mutated, 0
 		}