From 63483e7e082bc6fa5d9f2f3bc719f4a353924518 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      | 14 ++++++++
 internal/trie/node/subvalue_test.go | 54 +++++++++++++++++++++++++++++
 lib/trie/trie.go                    |  6 ++--
 3 files changed, 71 insertions(+), 3 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..a3b23e9e8f3
--- /dev/null
+++ b/internal/trie/node/subvalue.go
@@ -0,0 +1,14 @@
+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..85771c913cd
--- /dev/null
+++ b/internal/trie/node/subvalue_test.go
@@ -0,0 +1,54 @@
+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 443e0aed20d..d15f30b790e 100644
--- a/lib/trie/trie.go
+++ b/lib/trie/trie.go
@@ -351,7 +351,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
 		}
@@ -431,7 +431,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
 		}
@@ -463,7 +463,7 @@ func (t *Trie) insertInBranch(parentBranch *Node, key, value []byte) (
 			return parentBranch, mutated, nodesCreated
 		}
 
-		child, mutated, nodesCreated := t.insert(child, remainingKey, value)
+		child, mutated, nodesCreated = t.insert(child, remainingKey, value)
 		if !mutated {
 			return parentBranch, mutated, 0
 		}