From 967494771456b4b5c8e0dd42124b12f0e6615d71 Mon Sep 17 00:00:00 2001
From: Quentin McGaw <quentin.mcgaw@gmail.com>
Date: Thu, 15 Sep 2022 17:59:29 +0000
Subject: [PATCH] Fix differentiating nil and empty subvalues

---
 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..d00aab788f6
--- /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"
+
+// SubValueEqual returns true if the node subvalue is equal to the
+// subvalue given as argument. In particular, it returns false
+// if one subvalue is nil and the other subvalue is the empty slice.
+func (n Node) SubValueEqual(subValue []byte) (equal bool) {
+	if len(subValue) == 0 && len(n.SubValue) == 0 {
+		return (subValue == nil && n.SubValue == nil) ||
+			(subValue != nil && n.SubValue != nil)
+	}
+	return bytes.Equal(n.SubValue, subValue)
+}
diff --git a/internal/trie/node/subvalue_test.go b/internal/trie/node/subvalue_test.go
new file mode 100644
index 00000000000..3d190acc63c
--- /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_SubValueEqual(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{SubValue: []byte{}},
+			subValue: []byte{},
+			equal:    true,
+		},
+		"nil node subvalue and empty subvalue": {
+			subValue: []byte{},
+		},
+		"empty node subvalue and nil subvalue": {
+			node: Node{SubValue: []byte{}},
+		},
+		"equal non empty values": {
+			node:     Node{SubValue: []byte{1, 2}},
+			subValue: []byte{1, 2},
+			equal:    true,
+		},
+		"not equal non empty values": {
+			node:     Node{SubValue: []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.SubValueEqual(testCase.subValue)
+
+			assert.Equal(t, testCase.equal, equal)
+		})
+	}
+}
diff --git a/lib/trie/trie.go b/lib/trie/trie.go
index e81b10b2f29..d5a4ef8d7e6 100644
--- a/lib/trie/trie.go
+++ b/lib/trie/trie.go
@@ -353,7 +353,7 @@ func (t *Trie) insertInLeaf(parentLeaf *Node, key, value []byte) (
 	newParent *Node, mutated bool, nodesCreated uint32) {
 	if bytes.Equal(parentLeaf.Key, key) {
 		nodesCreated = 0
-		if bytes.Equal(value, parentLeaf.SubValue) {
+		if parentLeaf.SubValueEqual(value) {
 			const mutated = false
 			return parentLeaf, mutated, nodesCreated
 		}
@@ -433,7 +433,7 @@ func (t *Trie) insertInBranch(parentBranch *Node, key, value []byte) (
 	copySettings := node.DefaultCopySettings
 
 	if bytes.Equal(key, parentBranch.Key) {
-		if bytes.Equal(parentBranch.SubValue, value) {
+		if parentBranch.SubValueEqual(value) {
 			const mutated = false
 			return parentBranch, mutated, 0
 		}