From 4ed0e432e389a250e1ec8b1fb003f8dfd019a17f Mon Sep 17 00:00:00 2001 From: Victor Farazdagi Date: Mon, 1 Jun 2020 22:00:47 +0300 Subject: [PATCH] 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 | 2 + beacon-chain/db/kv/utils.go | 5 +- beacon-chain/db/kv/utils_test.go | 149 +++++++++++++++++++++++++++++++ 3 files changed, 154 insertions(+), 2 deletions(-) create mode 100644 beacon-chain/db/kv/utils_test.go diff --git a/beacon-chain/db/kv/BUILD.bazel b/beacon-chain/db/kv/BUILD.bazel index 018c3bf64..043f2f75e 100644 --- a/beacon-chain/db/kv/BUILD.bazel +++ b/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", ], ) diff --git a/beacon-chain/db/kv/utils.go b/beacon-chain/db/kv/utils.go index a244bad6f..dcc195a1e 100644 --- a/beacon-chain/db/kv/utils.go +++ b/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, diff --git a/beacon-chain/db/kv/utils_test.go b/beacon-chain/db/kv/utils_test.go new file mode 100644 index 000000000..d17704e78 --- /dev/null +++ b/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) + } + }) + } +}