fix: store key uniqueness (#9363)

* 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>

types/store.go

@@ -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"
 )
@@ -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
@@ -105,10 +123,13 @@ 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
@@ -116,10 +137,13 @@ func NewTransientStoreKeys(names ...string) map[string]*TransientStoreKey {
 
 // 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

types/store_internal_test.go

@@ -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())
+}

types/store_test.go

@@ -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")))