From 89bda6ab266108b3fe5dc3bb6fea2c8c8aee5c2c Mon Sep 17 00:00:00 2001 From: Michael FIG Date: Thu, 19 May 2022 21:37:04 -0600 Subject: [PATCH] feat(keeper): storage path key encoding --- golang/cosmos/x/swingset/keeper/keeper.go | 206 +++++++++++------- .../cosmos/x/swingset/keeper/keeper_test.go | 23 +- golang/cosmos/x/swingset/types/key.go | 5 +- 3 files changed, 140 insertions(+), 94 deletions(-) diff --git a/golang/cosmos/x/swingset/keeper/keeper.go b/golang/cosmos/x/swingset/keeper/keeper.go index d03e767896b..180f8b3a31e 100644 --- a/golang/cosmos/x/swingset/keeper/keeper.go +++ b/golang/cosmos/x/swingset/keeper/keeper.go @@ -1,13 +1,14 @@ package keeper import ( + "bytes" "encoding/json" "fmt" - "sort" "strconv" "strings" "github.com/tendermint/tendermint/libs/log" + db "github.com/tendermint/tm-db" "github.com/cosmos/cosmos-sdk/codec" "github.com/cosmos/cosmos-sdk/store/prefix" @@ -35,19 +36,71 @@ type Keeper struct { var _ types.SwingSetKeeper = &Keeper{} -// A prefix of bytes, since KVStores can't handle empty slices as keys. -var keyPrefix = []byte{':'} +// Keys are structured as: `${numberOfPathElements}\0${zeroSeparatedPath}`, such +// as `0\0` for the root element, and `1\0foo` for `foo`, and `3\0foo\0bar\0baz` +// for `foo.bar.baz`. The reason for this is space efficiency: +// - we can read a node's data with just a single store operation +// after mapping the key +// - we can scan for all of `foo.bar`'s children by iterating over the prefix +// `3\0foo\0bar\0`. +// We still need to iterate up the tree until we are sure the correct ancestor +// nodes are present or absent, but we don't need to fetch all an ancestor's +// keys to do so. +var ( + keySeparator = []byte{0} + keySeparatorRune = rune(0) + pathSeparatorRune = '.' + pathSeparator = string(pathSeparatorRune) + dataPrefix = ":" + emptyValue = []byte{1} +) + +// keyToPath converts a byte slice path to a string key +func keyToPath(key []byte) string { + // Split the key into its length and path components. + split := bytes.SplitN(key, keySeparator, 2) + pathBytes := split[1] + + pathStr := strings.Map(func(chr rune) rune { + switch chr { + case pathSeparatorRune: + panic(fmt.Sprintf("key %q cannot contain %q", key, pathSeparatorRune)) + case keySeparatorRune: + return pathSeparatorRune + default: + return chr + } + }, string(pathBytes)) + + return pathStr +} + +func pathToDataKey(keyStr string) []byte { + return append([]byte(dataPrefix), []byte(keyStr)...) +} -// keyToString converts a byte slice path to a string key -func keyToString(key []byte) string { - return string(key[len(keyPrefix):]) +// pathToKey converts a string key to a byte slice path +func pathToKey(keyStr string) []byte { + return pathWithLengthToKey(keyStr, 0) } -// stringToKey converts a string key to a byte slice path -func stringToKey(keyStr string) []byte { - key := keyPrefix - key = append(key, []byte(keyStr)...) - return key +func pathWithLengthToKey(keyStr string, initialPathLength int) []byte { + pathLength := initialPathLength + if len(keyStr) > 0 { + pathLength += 1 + } + pathStr := strings.Map(func(chr rune) rune { + switch chr { + case keySeparatorRune: + panic(fmt.Sprintf("key %q cannot contain %q", keyStr, keySeparatorRune)) + case pathSeparatorRune: + pathLength += 1 + return keySeparatorRune + default: + return chr + } + }, keyStr) + return []byte(fmt.Sprintf("%d%c%s", pathLength, keySeparatorRune, pathStr)) } // NewKeeper creates a new IBC transfer Keeper instance @@ -250,14 +303,14 @@ func (k Keeper) SetEgress(ctx sdk.Context, egress *types.Egress) error { // ExportStorage fetches all storage func (k Keeper) ExportStorage(ctx sdk.Context) []*types.StorageEntry { store := ctx.KVStore(k.storeKey) - dataStore := prefix.NewStore(store, types.DataPrefix) + dataStore := prefix.NewStore(store, types.DataKeyPrefix) iterator := sdk.KVStorePrefixIterator(dataStore, nil) exported := []*types.StorageEntry{} defer iterator.Close() for ; iterator.Valid(); iterator.Next() { - keyStr := keyToString(iterator.Key()) + keyStr := keyToPath(iterator.Key()) entry := types.StorageEntry{Key: keyStr, Value: string(iterator.Value())} exported = append(exported, &entry) } @@ -268,8 +321,8 @@ func (k Keeper) ExportStorage(ctx sdk.Context) []*types.StorageEntry { func (k Keeper) GetStorage(ctx sdk.Context, path string) string { //fmt.Printf("GetStorage(%s)\n", path); store := ctx.KVStore(k.storeKey) - dataStore := prefix.NewStore(store, types.DataPrefix) - dataKey := stringToKey(path) + dataStore := prefix.NewStore(store, types.DataKeyPrefix) + dataKey := pathToDataKey(path) if !dataStore.Has(dataKey) { return "" } @@ -278,91 +331,90 @@ func (k Keeper) GetStorage(ctx sdk.Context, path string) string { return value } -// GetKeys gets all storage child keys at a given path -func (k Keeper) GetKeys(ctx sdk.Context, path string) *types.Keys { +func (k Keeper) getKeyIterator(ctx sdk.Context, path string) db.Iterator { store := ctx.KVStore(k.storeKey) - keysStore := prefix.NewStore(store, types.KeysPrefix) - key := stringToKey(path) - if !keysStore.Has(key) { - return types.NewKeys() + pathStore := prefix.NewStore(store, types.PathKeyPrefix) + var keyPrefix []byte + if path == "" { + keyPrefix = pathWithLengthToKey("", 1) + } else { + keyPrefix = pathToKey(path + pathSeparator) } - bz := keysStore.Get(key) + + return sdk.KVStorePrefixIterator(pathStore, keyPrefix) +} + +// GetKeys gets all storage child keys at a given path +func (k Keeper) GetKeys(ctx sdk.Context, path string) *types.Keys { + iterator := k.getKeyIterator(ctx, path) + var keys types.Keys - k.cdc.MustUnmarshalLengthPrefixed(bz, &keys) + keys.Keys = []string{} + defer iterator.Close() + for ; iterator.Valid(); iterator.Next() { + parts := strings.Split(keyToPath(iterator.Key()), pathSeparator) + keyStr := parts[len(parts)-1] + keys.Keys = append(keys.Keys, keyStr) + } return &keys } -// SetStorage sets the entire generic storage for a path -func (k Keeper) SetStorage(ctx sdk.Context, path, value string) { +// HasStorage tells if a given path has data. +func (k Keeper) HasStorage(ctx sdk.Context, path string) bool { store := ctx.KVStore(k.storeKey) - dataStore := prefix.NewStore(store, types.DataPrefix) - keysStore := prefix.NewStore(store, types.KeysPrefix) + dataStore := prefix.NewStore(store, types.DataKeyPrefix) + dataKey := pathToDataKey(path) - // Update the value. - pathKey := stringToKey(path) - if value == "" { - dataStore.Delete(pathKey) - } else { - dataStore.Set(pathKey, []byte(value)) - } + // Check if we have data. + return dataStore.Has(dataKey) +} + +// HasKeys tells if a given path has child keys. +func (k Keeper) HasKeys(ctx sdk.Context, path string) bool { + // Check if we have children. + iterator := k.getKeyIterator(ctx, path) + defer iterator.Close() + return iterator.Valid() +} +// SetStorage sets the entire generic storage for a path +func (k Keeper) SetStorage(ctx sdk.Context, path, value string) { ctx.EventManager().EmitEvent( types.NewStorageEvent(path, value), ) - if value == "" && len(k.GetKeys(ctx, path).Keys) > 0 { - // If we're deleting and we had children, we need to keep our ancestor keys - // around. - return + store := ctx.KVStore(k.storeKey) + dataStore := prefix.NewStore(store, types.DataKeyPrefix) + pathStore := prefix.NewStore(store, types.PathKeyPrefix) + + // Update the value. + dataKey := pathToDataKey(path) + + if value == "" { + dataStore.Delete(dataKey) + } else { + dataStore.Set(dataKey, []byte(value)) } - // Update the parent keys. + // Update our and other parent keys. pathComponents := strings.Split(path, ".") - for i := len(pathComponents) - 1; i >= 0; i-- { + for i := len(pathComponents); i >= 0; i-- { ancestor := strings.Join(pathComponents[0:i], ".") - lastKeyStr := pathComponents[i] - - // Get a map corresponding to the parent's keys. - keyNode := k.GetKeys(ctx, ancestor) - keyList := keyNode.Keys - keyMap := make(map[string]bool, len(keyList)+1) - for _, keyStr := range keyList { - keyMap[keyStr] = true - } - // Decide if we need to add or remove the key. + // Decide if we need to add or remove the ancestor. if value == "" { - if _, ok := keyMap[lastKeyStr]; !ok { - // If the key is already gone, we don't need to remove from the key - // lists. + if k.HasStorage(ctx, ancestor) || k.HasKeys(ctx, ancestor) { + // If the key is needed, skip out. return } // Delete the key. - delete(keyMap, lastKeyStr) - } else { - if _, ok := keyMap[lastKeyStr]; ok { - // If the key already exists, we don't need to add to the key lists. - return - } - // Add the key. - keyMap[lastKeyStr] = true - } - - keyList = make([]string, 0, len(keyMap)) - for keyStr := range keyMap { - keyList = append(keyList, keyStr) - } - - // Update the list of keys - ancestorKey := stringToKey(ancestor) - if len(keyList) == 0 { - // No keys left, delete the parent. - keysStore.Delete(ancestorKey) + pathStore.Delete(pathToKey(ancestor)) + } else if i < len(pathComponents) && k.HasStorage(ctx, ancestor) { + // The key is present, so we can skip out. + return } else { - // Update the key node, ordering deterministically. - sort.Strings(keyList) - keyNode.Keys = keyList - keysStore.Set(ancestorKey, k.cdc.MustMarshalLengthPrefixed(keyNode)) + // Add the key as an empty value. + pathStore.Set(pathToKey(ancestor), emptyValue) } } } diff --git a/golang/cosmos/x/swingset/keeper/keeper_test.go b/golang/cosmos/x/swingset/keeper/keeper_test.go index c250e34f9c0..fbfb246713e 100644 --- a/golang/cosmos/x/swingset/keeper/keeper_test.go +++ b/golang/cosmos/x/swingset/keeper/keeper_test.go @@ -31,33 +31,28 @@ func Test_Key_Encoding(t *testing.T) { key []byte }{ { - name: "empty key matches prefix", + name: "empty key is prefixed", keyStr: "", - key: keyPrefix, - }, - { - name: "empty key string (actual)", - keyStr: "", - key: []byte{':'}, + key: []byte("0\x00"), }, { name: "some key string", keyStr: "some", - key: []byte{':', 's', 'o', 'm', 'e'}, + key: []byte("1\x00some"), }, { - name: "key prefix immutable", - keyStr: "", - key: keyPrefix, + name: "dot-separated", + keyStr: "some.child.grandchild", + key: []byte("3\x00some\x00child\x00grandchild"), }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if key := stringToKey(tt.keyStr); !bytes.Equal(key, tt.key) { - t.Errorf("stringToKey(%q) = %v, want %v", tt.keyStr, key, tt.key) + if key := pathToKey(tt.keyStr); !bytes.Equal(key, tt.key) { + t.Errorf("pathToKey(%q) = %v, want %v", tt.keyStr, key, tt.key) } - if keyStr := keyToString(tt.key); keyStr != tt.keyStr { + if keyStr := keyToPath(tt.key); keyStr != tt.keyStr { t.Errorf("keyToString(%v) = %q, want %q", tt.key, keyStr, tt.keyStr) } }) diff --git a/golang/cosmos/x/swingset/types/key.go b/golang/cosmos/x/swingset/types/key.go index 606265bbefb..e22e9dc0190 100644 --- a/golang/cosmos/x/swingset/types/key.go +++ b/golang/cosmos/x/swingset/types/key.go @@ -9,7 +9,6 @@ const ( ) var ( - DataPrefix = []byte(StoreKey + "/data") - KeysPrefix = []byte(StoreKey + "/keys") - EgressPrefix = []byte(StoreKey + "/egress") + DataKeyPrefix = []byte(ModuleName + "/data") + PathKeyPrefix = []byte(ModuleName + "/path") )