Fixes subsequence matching in deleteValueForIndices (#6071)

* fixes subsequence matching in deleteValueForIndices
* Merge branch 'master' into fix-delete-value-for-indices
* Merge refs/heads/master into fix-delete-value-for-indices

beacon-chain/db/kv/BUILD.bazel

@@ -75,6 +75,7 @@ go_test(
         "slashings_test.go",
         "state_summary_test.go",
         "state_test.go",
+        "utils_test.go",
     ],
     embed = [":go_default_library"],
     deps = [
@@ -92,5 +93,6 @@ go_test(
         "@com_github_prysmaticlabs_go_bitfield//:go_default_library",
         "@com_github_prysmaticlabs_go_ssz//:go_default_library",
         "@in_gopkg_d4l3k_messagediff_v1//:go_default_library",
+        "@io_etcd_go_bbolt//:go_default_library",
     ],
 )

beacon-chain/db/kv/utils.go

@@ -60,8 +60,9 @@ func deleteValueForIndices(indicesByBucket map[string][]byte, root []byte, tx *b
 		valuesAtIndex := bkt.Get(idx)
 		if valuesAtIndex != nil {
 			start := bytes.Index(valuesAtIndex, root)
-			// if the root was not found inside the values at index slice, we continue.
-			if start == -1 {
+			// If the root was not found inside the values at index slice, we continue.
+			// Root must be correctly aligned to avoid matching to subsequences of adjacent values.
+			if start == -1 || start%len(root) != 0 {
 				continue
 			}
 			// We clear out the root from the values at index slice. For example,

beacon-chain/db/kv/utils_test.go

@@ -0,0 +1,149 @@
+package kv
+
+import (
+	"crypto/rand"
+	"reflect"
+	"testing"
+
+	"github.com/prysmaticlabs/prysm/shared/bytesutil"
+	bolt "go.etcd.io/bbolt"
+)
+
+func Test_deleteValueForIndices(t *testing.T) {
+	db := setupDB(t)
+	blocks := make([]byte, 128)
+	if _, err := rand.Read(blocks); err != nil {
+		t.Fatal(err)
+	}
+	tests := []struct {
+		name          string
+		inputIndices  map[string][]byte
+		root          []byte
+		outputIndices map[string][]byte
+		wantErr       bool
+	}{
+		{
+			name:          "empty input, no root",
+			inputIndices:  map[string][]byte{},
+			root:          []byte{},
+			outputIndices: map[string][]byte{},
+			wantErr:       false,
+		},
+		{
+			name:          "empty input, root does not exist",
+			inputIndices:  map[string][]byte{},
+			root:          bytesutil.PadTo([]byte("not found"), 32),
+			outputIndices: map[string][]byte{},
+			wantErr:       false,
+		},
+		{
+			name: "non empty input, root does not exist",
+			inputIndices: map[string][]byte{
+				"blocks": bytesutil.PadTo([]byte{0xde, 0xad, 0xbe, 0xef}, 64),
+			},
+			root: bytesutil.PadTo([]byte("not found"), 32),
+			outputIndices: map[string][]byte{
+				"blocks": bytesutil.PadTo([]byte{0xde, 0xad, 0xbe, 0xef}, 64),
+			},
+			wantErr: false,
+		},
+		{
+			name: "removes value for a single bucket",
+			inputIndices: map[string][]byte{
+				"blocks": {0xde, 0xad, 0xbe, 0xef},
+			},
+			root: []byte{0xde},
+			outputIndices: map[string][]byte{
+				"blocks": {0xad, 0xbe, 0xef},
+			},
+			wantErr: false,
+		},
+		{
+			name: "removes multi-byte value for a single bucket (non-aligned)",
+			inputIndices: map[string][]byte{
+				"blocks": {0xde, 0xad, 0xbe, 0xef},
+			},
+			root: []byte{0xad, 0xbe},
+			outputIndices: map[string][]byte{
+				"blocks": {0xde, 0xad, 0xbe, 0xef},
+			},
+			wantErr: false,
+		},
+		{
+			name: "removes multi-byte value for a single bucket (non-aligned)",
+			inputIndices: map[string][]byte{
+				"blocks": {0xde, 0xad, 0xbe, 0xef, 0xff, 0x01},
+			},
+			root: []byte{0xbe, 0xef},
+			outputIndices: map[string][]byte{
+				"blocks": {0xde, 0xad, 0xff, 0x01},
+			},
+			wantErr: false,
+		},
+		{
+			name: "removes value from multiple buckets",
+			inputIndices: map[string][]byte{
+				"blocks":      {0xff, 0x32, 0x45, 0x25, 0xde, 0xad, 0xbe, 0xef, 0x24},
+				"state":       {0x01, 0x02, 0x03, 0x04},
+				"check-point": {0xde, 0xad, 0xbe, 0xef},
+				"powchain":    {0xba, 0xad, 0xb0, 0x00, 0xde, 0xad, 0xbe, 0xef, 0xff},
+			},
+			root: []byte{0xde, 0xad, 0xbe, 0xef},
+			outputIndices: map[string][]byte{
+				"blocks":      {0xff, 0x32, 0x45, 0x25, 0x24},
+				"state":       {0x01, 0x02, 0x03, 0x04},
+				"check-point": {},
+				"powchain":    {0xba, 0xad, 0xb0, 0x00, 0xff},
+			},
+			wantErr: false,
+		},
+		{
+			name: "root as subsequence of two values (preserve)",
+			inputIndices: map[string][]byte{
+				"blocks": blocks,
+			},
+			outputIndices: map[string][]byte{
+				"blocks": blocks,
+			},
+			root: blocks[48:80],
+		},
+		{
+			name: "root as subsequence of two values (remove)",
+			inputIndices: map[string][]byte{
+				"blocks": blocks,
+			},
+			outputIndices: map[string][]byte{
+				"blocks": append(blocks[0:64], blocks[96:]...),
+			},
+			root: blocks[64:96],
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			err := db.db.Update(func(tx *bolt.Tx) error {
+				for k, idx := range tt.inputIndices {
+					bkt := tx.Bucket([]byte(k))
+					if err := bkt.Put(idx, tt.inputIndices[k]); err != nil {
+						t.Fatal(err)
+					}
+				}
+				if err := deleteValueForIndices(tt.inputIndices, tt.root, tx); (err != nil) != tt.wantErr {
+					t.Errorf("deleteValueForIndices() error = %v, wantErr %v", err, tt.wantErr)
+				}
+				// Check updated indices.
+				for k, idx := range tt.inputIndices {
+					bkt := tx.Bucket([]byte(k))
+					valuesAtIndex := bkt.Get(idx)
+					if !reflect.DeepEqual(valuesAtIndex, tt.outputIndices[k]) {
+						t.Errorf("unexpected output at %q, want: %#v, got: %#v", k, tt.outputIndices[k], valuesAtIndex)
+					}
+				}
+				return nil
+			})
+			if err != nil {
+				t.Fatal(err)
+			}
+		})
+	}
+}