Skip to content

Commit

Permalink
fix: store key uniqueness (#9363)
Browse files Browse the repository at this point in the history
* fix: store key uniqueness

* gosimple: use copy instead of for loop

* Update types/store.go

Co-authored-by: Tyler <48813565+technicallyty@users.noreply.github.com>

* fix import

Co-authored-by: Tyler <48813565+technicallyty@users.noreply.github.com>
  • Loading branch information
robert-zaremba and technicallyty authored May 20, 2021
1 parent b1201f8 commit 8161b3a
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 14 deletions.
42 changes: 33 additions & 9 deletions types/store.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,10 @@
package types

import (
fmt "fmt"
"sort"
"strings"

"github.com/cosmos/cosmos-sdk/store/types"
"github.com/cosmos/cosmos-sdk/types/kv"
)
Expand Down Expand Up @@ -80,18 +84,32 @@ type (
MemoryStoreKey = types.MemoryStoreKey
)

// assertNoCommonPrefix will panic if there are two keys: k1 and k2 in keys, such that
// k1 is a prefix of k2
func assertNoPrefix(keys []string) {
sorted := make([]string, len(keys))
copy(sorted, keys)
sort.Strings(sorted)
for i := 1; i < len(sorted); i++ {
if strings.HasPrefix(sorted[i], sorted[i-1]) {
panic(fmt.Sprint("Potential key collision between KVStores:", sorted[i], " - ", sorted[i-1]))
}
}
}

// NewKVStoreKey returns a new pointer to a KVStoreKey.
// Use a pointer so keys don't collide.
func NewKVStoreKey(name string) *KVStoreKey {
return types.NewKVStoreKey(name)
}

// NewKVStoreKeys returns a map of new pointers to KVStoreKey's.
// Uses pointers so keys don't collide.
// The function will panic if there is a potential conflict in names (see `assertNoPrefix`
// function for more details).
func NewKVStoreKeys(names ...string) map[string]*KVStoreKey {
keys := make(map[string]*KVStoreKey)
for _, name := range names {
keys[name] = NewKVStoreKey(name)
assertNoPrefix(names)
keys := make(map[string]*KVStoreKey, len(names))
for _, n := range names {
keys[n] = NewKVStoreKey(n)
}

return keys
Expand All @@ -105,21 +123,27 @@ func NewTransientStoreKey(name string) *TransientStoreKey {

// NewTransientStoreKeys constructs a new map of TransientStoreKey's
// Must return pointers according to the ocap principle
// The function will panic if there is a potential conflict in names (see `assertNoPrefix`
// function for more details).
func NewTransientStoreKeys(names ...string) map[string]*TransientStoreKey {
assertNoPrefix(names)
keys := make(map[string]*TransientStoreKey)
for _, name := range names {
keys[name] = NewTransientStoreKey(name)
for _, n := range names {
keys[n] = NewTransientStoreKey(n)
}

return keys
}

// NewMemoryStoreKeys constructs a new map matching store key names to their
// respective MemoryStoreKey references.
// The function will panic if there is a potential conflict in names (see `assertNoPrefix`
// function for more details).
func NewMemoryStoreKeys(names ...string) map[string]*MemoryStoreKey {
assertNoPrefix(names)
keys := make(map[string]*MemoryStoreKey)
for _, name := range names {
keys[name] = types.NewMemoryStoreKey(name)
for _, n := range names {
keys[n] = types.NewMemoryStoreKey(n)
}

return keys
Expand Down
57 changes: 57 additions & 0 deletions types/store_internal_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
package types

import (
"testing"

"github.com/stretchr/testify/suite"
)

type storeIntSuite struct {
suite.Suite
}

func TestStoreIntSuite(t *testing.T) {
suite.Run(t, new(storeIntSuite))
}

func (s *storeIntSuite) TestAssertNoPrefix() {
var testCases = []struct {
keys []string
expectPanic bool
}{
{[]string{""}, false},
{[]string{"a"}, false},
{[]string{"a", "b"}, false},
{[]string{"a", "b1"}, false},
{[]string{"b2", "b1"}, false},
{[]string{"b1", "bb", "b2"}, false},

{[]string{"a", ""}, true},
{[]string{"a", "b", "a"}, true},
{[]string{"a", "b", "aa"}, true},
{[]string{"a", "b", "ab"}, true},
{[]string{"a", "b1", "bb", "b12"}, true},
}

require := s.Require()
for _, tc := range testCases {
if tc.expectPanic {
require.Panics(func() { assertNoPrefix(tc.keys) })
} else {
assertNoPrefix(tc.keys)
}
}
}

func (s *storeIntSuite) TestNewKVStoreKeys() {
require := s.Require()
require.Panics(func() { NewKVStoreKeys("a1", "a") }, "should fail one key is a prefix of another one")

require.Equal(map[string]*KVStoreKey{}, NewKVStoreKeys())
require.Equal(1, len(NewKVStoreKeys("one")))

key := "baca"
stores := NewKVStoreKeys(key, "a")
require.Len(stores, 2)
require.Equal(key, stores[key].Name())
}
5 changes: 0 additions & 5 deletions types/store_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,11 +54,6 @@ func (s *storeTestSuite) TestCommitID() {
s.Require().False(nonempty.IsZero())
}

func (s *storeTestSuite) TestNewKVStoreKeys() {
s.Require().Equal(map[string]*sdk.KVStoreKey{}, sdk.NewKVStoreKeys())
s.Require().Equal(1, len(sdk.NewKVStoreKeys("one")))
}

func (s *storeTestSuite) TestNewTransientStoreKeys() {
s.Require().Equal(map[string]*sdk.TransientStoreKey{}, sdk.NewTransientStoreKeys())
s.Require().Equal(1, len(sdk.NewTransientStoreKeys("one")))
Expand Down

0 comments on commit 8161b3a

Please sign in to comment.