go/storage/mkvs: Don't forget to include siblings in SyncGet proof

.changelog/2775.bugfix.1.md

@@ -0,0 +1 @@
+go/storage/mkvs: Don't forget to include siblings in SyncGet proof

go/storage/mkvs/urkel/lookup.go

@@ -122,11 +122,29 @@ func (t *tree) doGet(
 		bitLength := bitDepth + n.LabelBitLength
 
 		// Does lookup key end here? Look into LeafNode.
+		var value []byte
 		if key.BitLength() == bitLength {
+			// Include siblings before disabling the proof builder for the leaf node.
+			if opts.includeSiblings {
+				// Also fetch the left and right siblings.
+				_, err = t.doGet(ctx, n.Left, bitLength, key, opts, true)
+				if err != nil {
+					return nil, err
+				}
+				_, err = t.doGet(ctx, n.Right, bitLength, key, opts, true)
+				if err != nil {
+					return nil, err
+				}
+			}
+
 			// Omit the proof builder as the leaf node is always included with
 			// the internal node itself.
 			opts.proofBuilder = nil
-			return t.doGet(ctx, n.LeafNode, bitLength, key, opts, false)
+			value, err = t.doGet(ctx, n.LeafNode, bitLength, key, opts, false)
+			if err != nil {
+				return nil, err
+			}
+			return value, nil
 		}
 
 		// Lookup key is too short for the current n.Label. It's not stored.
@@ -135,7 +153,6 @@ func (t *tree) doGet(
 		}
 
 		// Continue recursively based on a bit value.
-		var value []byte
 		if key.GetBit(bitLength) {
 			value, err = t.doGet(ctx, n.Right, bitLength, key, opts, false)
 			if err != nil {

go/storage/mkvs/urkel/testdata/case-4.json

@@ -0,0 +1,16 @@
+[
+    {
+        "key": "NTlE",
+        "value": "QjVD",
+        "op": "Insert"
+    },
+    {
+        "value": "QzQ1QTQxNEQ2OEM0OTI=",
+        "key": "NTk=",
+        "op": "Insert"
+    },
+    {
+        "key": "NTk=",
+        "op": "Remove"
+    }
+]

go/storage/mkvs/urkel/urkel_test.go

@@ -1795,20 +1795,49 @@ func testSpecialCaseFromJSON(t *testing.T, ndb db.NodeDB, fixture string) {
 	ctx := context.Background()
 	tree := New(nil, ndb)
 
+	// Also test all operations against a "remote" tree to test sync operations.
+	var root node.Root
+	var remoteTree Tree
+	var value []byte
+
 	for _, o := range ops {
 		switch o.Op {
 		case mkvsTests.OpInsert:
+			if remoteTree != nil {
+				err = remoteTree.Insert(ctx, o.Key, o.Value)
+				require.NoError(t, err, "Insert")
+			}
+
 			err = tree.Insert(ctx, o.Key, o.Value)
 			require.NoError(t, err, "Insert")
 		case mkvsTests.OpRemove:
+			if remoteTree != nil {
+				err = remoteTree.Remove(ctx, o.Key)
+				require.NoError(t, err, "Remove")
+			}
+
 			err = tree.Remove(ctx, o.Key)
 			require.NoError(t, err, "Remove")
 		case mkvsTests.OpGet:
-			var value []byte
+			if remoteTree != nil {
+				value, err = remoteTree.Get(ctx, o.Key)
+				require.NoError(t, err, "Get")
+				require.EqualValues(t, o.Value, value, "Get should return the correct value")
+			}
+
 			value, err = tree.Get(ctx, o.Key)
 			require.NoError(t, err, "Get")
 			require.EqualValues(t, o.Value, value, "Get should return the correct value")
 		case mkvsTests.OpIteratorSeek:
+			if remoteTree != nil {
+				it := remoteTree.NewIterator(ctx)
+				it.Seek(o.Key)
+				require.NoError(t, it.Err(), "Seek")
+				require.EqualValues(t, o.ExpectedKey, it.Key(), "iterator should be at correct key")
+				require.EqualValues(t, o.Value, it.Value(), "iterator should be at correct value")
+				it.Close()
+			}
+
 			it := tree.NewIterator(ctx)
 			it.Seek(o.Key)
 			require.NoError(t, it.Err(), "Seek")
@@ -1818,6 +1847,12 @@ func testSpecialCaseFromJSON(t *testing.T, ndb db.NodeDB, fixture string) {
 		default:
 			require.Fail(t, "unknown operation: %s", o.Op)
 		}
+
+		// Commit everything and create a new remote tree at the root.
+		_, rootHash, err := tree.Commit(ctx, testNs, 0)
+		require.NoError(t, err, "Commit")
+		root = node.Root{Namespace: testNs, Hash: rootHash}
+		remoteTree = NewWithRoot(tree, nil, root, Capacity(0, 0))
 	}
 }
 
@@ -1833,6 +1868,10 @@ func testSpecialCase3(t *testing.T, ndb db.NodeDB, factory NodeDBFactory) {
 	testSpecialCaseFromJSON(t, ndb, "case-3.json")
 }
 
+func testSpecialCase4(t *testing.T, ndb db.NodeDB, factory NodeDBFactory) {
+	testSpecialCaseFromJSON(t, ndb, "case-4.json")
+}
+
 func testBackend(
 	t *testing.T,
 	initBackend func(t *testing.T) (NodeDBFactory, func()),
@@ -1871,6 +1910,7 @@ func testBackend(
 		{"SpecialCase1", testSpecialCase1},
 		{"SpecialCase2", testSpecialCase2},
 		{"SpecialCase3", testSpecialCase3},
+		{"SpecialCase4", testSpecialCase4},
 		{"Errors", testErrors},
 	}