feat(vstorage): Handle explicit delete of path

golang/cosmos/x/swingset/keeper/keeper.go

@@ -17,6 +17,7 @@ import (
 	"github.com/Agoric/agoric-sdk/golang/cosmos/vm"
 	"github.com/Agoric/agoric-sdk/golang/cosmos/x/swingset/types"
 	vstoragekeeper "github.com/Agoric/agoric-sdk/golang/cosmos/x/vstorage/keeper"
+	vstoragetypes "github.com/Agoric/agoric-sdk/golang/cosmos/x/vstorage/types"
 )
 
 // Top-level paths for chain storage should remain synchronized with
@@ -103,19 +104,19 @@ func (k Keeper) PushAction(ctx sdk.Context, action vm.Jsonable) error {
 	}
 
 	// Set the vstorage corresponding to the queue entry for the current tail.
-	k.vstorageKeeper.SetStorage(ctx, fmt.Sprintf("actionQueue.%d", tail), string(bz))
+	k.vstorageKeeper.SetStorage(ctx, vstoragetypes.NewStorageEntry(fmt.Sprintf("actionQueue.%d", tail), string(bz)))
 
 	// Update the tail to point to the next available entry.
-	k.vstorageKeeper.SetStorage(ctx, "actionQueue.tail", fmt.Sprintf("%d", tail+1))
+	k.vstorageKeeper.SetStorage(ctx, vstoragetypes.NewStorageEntry("actionQueue.tail", fmt.Sprintf("%d", tail+1)))
 	return nil
 }
 
 func (k Keeper) actionQueueIndex(ctx sdk.Context, name string) (uint64, error) {
 	index := uint64(0)
 	var err error
-	indexStr := k.vstorageKeeper.GetData(ctx, "actionQueue."+name)
-	if indexStr != "" {
-		index, err = strconv.ParseUint(indexStr, 10, 64)
+	indexEntry := k.vstorageKeeper.GetEntry(ctx, "actionQueue."+name)
+	if indexEntry.HasData() {
+		index, err = strconv.ParseUint(indexEntry.StringValue(), 10, 64)
 	}
 	return index, err
 }
@@ -190,18 +191,18 @@ func getBeansOwingPathForAddress(addr sdk.AccAddress) string {
 // the FeeAccount but has not yet paid.
 func (k Keeper) GetBeansOwing(ctx sdk.Context, addr sdk.AccAddress) sdk.Uint {
 	path := getBeansOwingPathForAddress(addr)
-	value := k.vstorageKeeper.GetData(ctx, path)
-	if value == "" {
+	entry := k.vstorageKeeper.GetEntry(ctx, path)
+	if !entry.HasData() {
 		return sdk.ZeroUint()
 	}
-	return sdk.NewUintFromString(value)
+	return sdk.NewUintFromString(entry.StringValue())
 }
 
 // SetBeansOwing sets the number of beans that the given address owes to the
 // feeCollector but has not yet paid.
 func (k Keeper) SetBeansOwing(ctx sdk.Context, addr sdk.AccAddress, beans sdk.Uint) {
 	path := getBeansOwingPathForAddress(addr)
-	k.vstorageKeeper.SetStorage(ctx, path, beans.String())
+	k.vstorageKeeper.SetStorage(ctx, vstoragetypes.NewStorageEntry(path, beans.String()))
 }
 
 // ChargeBeans charges the given address the given number of beans.  It divides
@@ -304,13 +305,13 @@ func (k Keeper) ChargeForProvisioning(ctx sdk.Context, submitter, addr sdk.AccAd
 // GetEgress gets the entire egress struct for a peer
 func (k Keeper) GetEgress(ctx sdk.Context, addr sdk.AccAddress) types.Egress {
 	path := StoragePathEgress + "." + addr.String()
-	value := k.vstorageKeeper.GetData(ctx, path)
-	if value == "" {
+	entry := k.vstorageKeeper.GetEntry(ctx, path)
+	if !entry.HasData() {
 		return types.Egress{}
 	}
 
 	var egress types.Egress
-	err := json.Unmarshal([]byte(value), &egress)
+	err := json.Unmarshal([]byte(entry.StringValue()), &egress)
 	if err != nil {
 		panic(err)
 	}
@@ -328,7 +329,7 @@ func (k Keeper) SetEgress(ctx sdk.Context, egress *types.Egress) error {
 	}
 
 	// FIXME: We should use just SetStorageAndNotify here, but solo needs legacy for now.
-	k.vstorageKeeper.LegacySetStorageAndNotify(ctx, path, string(bz))
+	k.vstorageKeeper.LegacySetStorageAndNotify(ctx, vstoragetypes.NewStorageEntry(path, string(bz)))
 
 	// Now make sure the corresponding account has been initialised.
 	if acc := k.accountKeeper.GetAccount(ctx, egress.Peer); acc != nil {
@@ -354,14 +355,14 @@ func (k Keeper) Logger(ctx sdk.Context) log.Logger {
 // GetMailbox gets the entire mailbox struct for a peer
 func (k Keeper) GetMailbox(ctx sdk.Context, peer string) string {
 	path := StoragePathMailbox + "." + peer
-	return k.vstorageKeeper.GetData(ctx, path)
+	return k.vstorageKeeper.GetEntry(ctx, path).StringValue()
 }
 
 // SetMailbox sets the entire mailbox struct for a peer
 func (k Keeper) SetMailbox(ctx sdk.Context, peer string, mailbox string) {
 	path := StoragePathMailbox + "." + peer
 	// FIXME: We should use just SetStorageAndNotify here, but solo needs legacy for now.
-	k.vstorageKeeper.LegacySetStorageAndNotify(ctx, path, mailbox)
+	k.vstorageKeeper.LegacySetStorageAndNotify(ctx, vstoragetypes.NewStorageEntry(path, mailbox))
 }
 
 func (k Keeper) PathToEncodedKey(path string) []byte {

golang/cosmos/x/swingset/keeper/querier.go

@@ -79,12 +79,12 @@ func queryMailbox(ctx sdk.Context, path []string, req abci.RequestQuery, keeper
 
 // nolint: unparam
 func legacyQueryStorage(ctx sdk.Context, path string, req abci.RequestQuery, keeper Keeper, legacyQuerierCdc *codec.LegacyAmino) (res []byte, err error) {
-	value := keeper.vstorageKeeper.GetData(ctx, path)
-	if value == "" {
+	entry := keeper.vstorageKeeper.GetEntry(ctx, path)
+	if !entry.HasData() {
 		return []byte{}, sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "could not get swingset %+v", path)
 	}
 
-	bz, err2 := codec.MarshalJSONIndent(legacyQuerierCdc, vstoragetypes.Data{Value: value})
+	bz, err2 := codec.MarshalJSONIndent(legacyQuerierCdc, vstoragetypes.Data{Value: entry.StringValue()})
 	if err2 != nil {
 		return nil, sdkerrors.Wrap(sdkerrors.ErrJSONMarshal, err2.Error())
 	}

golang/cosmos/x/vstorage/keeper/grpc_query.go

@@ -23,10 +23,10 @@ func (k Querier) Data(c context.Context, req *types.QueryDataRequest) (*types.Qu
 	}
 	ctx := sdk.UnwrapSDKContext(c)
 
-	value := k.GetData(ctx, req.Path)
+	entry := k.GetEntry(ctx, req.Path)
 
 	return &types.QueryDataResponse{
-		Value: value,
+		Value: entry.StringValue(),
 	}, nil
 }
 

golang/cosmos/x/vstorage/keeper/keeper.go

@@ -3,6 +3,7 @@ package keeper
 import (
 	"bytes"
 	"encoding/json"
+	"fmt"
 	"sort"
 	"strconv"
 	"strings"
@@ -32,7 +33,7 @@ type ProposedChange struct {
 }
 
 type ChangeManager interface {
-	Track(ctx sdk.Context, k Keeper, path, value string, isLegacy bool)
+	Track(ctx sdk.Context, k Keeper, entry types.StorageEntry, isLegacy bool)
 	EmitEvents(ctx sdk.Context, k Keeper)
 	Rollback(ctx sdk.Context)
 }
@@ -44,14 +45,26 @@ type BatchingChangeManager struct {
 
 var _ ChangeManager = (*BatchingChangeManager)(nil)
 
+// TODO: Use bytes.CutPrefix once we can rely upon go >= 1.20.
+func cutPrefix(s, prefix []byte) (after []byte, found bool) {
+	if !bytes.HasPrefix(s, prefix) {
+		return s, false
+	}
+	return s[len(prefix):], true
+}
+
 // Keeper maintains the link to data storage and exposes getter/setter methods
 // for the various parts of the state machine
 type Keeper struct {
 	changeManager ChangeManager
 	storeKey      sdk.StoreKey
 }
 
-func (bcm *BatchingChangeManager) Track(ctx sdk.Context, k Keeper, path, value string, isLegacy bool) {
+func (bcm *BatchingChangeManager) Track(ctx sdk.Context, k Keeper, entry types.StorageEntry, isLegacy bool) {
+	path := entry.Path()
+	// TODO: differentiate between deletion and setting empty string?
+	// Using empty string for deletion for backwards compatibility
+	value := entry.StringValue()
 	if change, ok := bcm.changes[path]; ok {
 		change.NewValue = value
 		if isLegacy {
@@ -62,7 +75,7 @@ func (bcm *BatchingChangeManager) Track(ctx sdk.Context, k Keeper, path, value s
 	bcm.changes[path] = &ProposedChange{
 		Path:               path,
 		NewValue:           value,
-		ValueFromLastBlock: k.GetData(ctx, path),
+		ValueFromLastBlock: k.GetEntry(ctx, path).StringValue(),
 		LegacyEvents:       isLegacy,
 	}
 }
@@ -114,12 +127,19 @@ func (k Keeper) ExportStorage(ctx sdk.Context) []*types.DataEntry {
 	exported := []*types.DataEntry{}
 	defer iterator.Close()
 	for ; iterator.Valid(); iterator.Next() {
-		path := types.EncodedKeyToPath(iterator.Key())
-		value := string(bytes.TrimPrefix(iterator.Value(), types.EncodedDataPrefix))
-		if len(value) == 0 {
+		rawValue := iterator.Value()
+		if len(rawValue) == 0 {
+			continue
+		}
+		if bytes.Equal(rawValue, types.EncodedNoDataValue) {
 			continue
 		}
-		entry := types.DataEntry{Path: path, Value: value}
+		path := types.EncodedKeyToPath(iterator.Key())
+		value, hasPrefix := cutPrefix(rawValue, types.EncodedDataPrefix)
+		if !hasPrefix {
+			panic(fmt.Errorf("value at path %q starts with unexpected prefix", path))
+		}
+		entry := types.DataEntry{Path: path, Value: string(value)}
 		exported = append(exported, &entry)
 	}
 	return exported
@@ -129,7 +149,7 @@ func (k Keeper) ImportStorage(ctx sdk.Context, entries []*types.DataEntry) {
 	for _, entry := range entries {
 		// This set does the bookkeeping for us in case the entries aren't a
 		// complete tree.
-		k.SetStorage(ctx, entry.Path, entry.Value)
+		k.SetStorage(ctx, types.NewStorageEntry(entry.Path, entry.Value))
 	}
 }
 
@@ -156,14 +176,23 @@ func (k Keeper) EmitChange(ctx sdk.Context, change *ProposedChange) {
 	)
 }
 
-// GetData gets generic storage.  The default value is an empty string.
-func (k Keeper) GetData(ctx sdk.Context, path string) string {
-	//fmt.Printf("GetData(%s)\n", path);
+// GetEntry gets generic storage.  The default value is an empty string.
+func (k Keeper) GetEntry(ctx sdk.Context, path string) types.StorageEntry {
+	//fmt.Printf("GetEntry(%s)\n", path);
 	store := ctx.KVStore(k.storeKey)
 	encodedKey := types.PathToEncodedKey(path)
-	bz := bytes.TrimPrefix(store.Get(encodedKey), types.EncodedDataPrefix)
-	value := string(bz)
-	return value
+	rawValue := store.Get(encodedKey)
+	if len(rawValue) == 0 {
+		return types.NewStorageEntryWithNoData(path)
+	}
+	if bytes.Equal(rawValue, types.EncodedNoDataValue) {
+		return types.NewStorageEntryWithNoData(path)
+	}
+	value, hasPrefix := cutPrefix(rawValue, types.EncodedDataPrefix)
+	if !hasPrefix {
+		panic(fmt.Errorf("value at path %q starts with unexpected prefix", path))
+	}
+	return types.NewStorageEntry(path, string(value))
 }
 
 func (k Keeper) getKeyIterator(ctx sdk.Context, path string) db.Iterator {
@@ -192,7 +221,7 @@ func (k Keeper) GetChildren(ctx sdk.Context, path string) *types.Children {
 // (just an empty string) and exist only to provide linkage to subnodes with
 // data.
 func (k Keeper) HasStorage(ctx sdk.Context, path string) bool {
-	return k.GetData(ctx, path) != ""
+	return k.GetEntry(ctx, path).HasData()
 }
 
 // HasEntry tells if a given path has either subnodes or data.
@@ -221,22 +250,22 @@ func (k Keeper) FlushChangeEvents(ctx sdk.Context) {
 	k.changeManager.Rollback(ctx)
 }
 
-func (k Keeper) SetStorageAndNotify(ctx sdk.Context, path, value string) {
-	k.changeManager.Track(ctx, k, path, value, false)
-	k.SetStorage(ctx, path, value)
+func (k Keeper) SetStorageAndNotify(ctx sdk.Context, entry types.StorageEntry) {
+	k.changeManager.Track(ctx, k, entry, false)
+	k.SetStorage(ctx, entry)
 }
 
-func (k Keeper) LegacySetStorageAndNotify(ctx sdk.Context, path, value string) {
-	k.changeManager.Track(ctx, k, path, value, true)
-	k.SetStorage(ctx, path, value)
+func (k Keeper) LegacySetStorageAndNotify(ctx sdk.Context, entry types.StorageEntry) {
+	k.changeManager.Track(ctx, k, entry, true)
+	k.SetStorage(ctx, entry)
 }
 
 func (k Keeper) AppendStorageValueAndNotify(ctx sdk.Context, path, value string) error {
 	blockHeight := strconv.FormatInt(ctx.BlockHeight(), 10)
 
 	// Preserve correctly-formatted data within the current block,
 	// otherwise initialize a blank cell.
-	currentData := k.GetData(ctx, path)
+	currentData := k.GetEntry(ctx, path).StringValue()
 	var cell StreamCell
 	_ = json.Unmarshal([]byte(currentData), &cell)
 	if cell.BlockHeight != blockHeight {
@@ -251,7 +280,7 @@ func (k Keeper) AppendStorageValueAndNotify(ctx sdk.Context, path, value string)
 	if err != nil {
 		return err
 	}
-	k.SetStorageAndNotify(ctx, path, string(bz))
+	k.SetStorageAndNotify(ctx, types.NewStorageEntry(path, string(bz)))
 	return nil
 }
 
@@ -263,22 +292,27 @@ func componentsToPath(components []string) string {
 //
 // Maintains the invariant: path entries exist if and only if self or some
 // descendant has non-empty storage
-func (k Keeper) SetStorage(ctx sdk.Context, path, value string) {
+func (k Keeper) SetStorage(ctx sdk.Context, entry types.StorageEntry) {
 	store := ctx.KVStore(k.storeKey)
+	path := entry.Path()
 	encodedKey := types.PathToEncodedKey(path)
 
-	if value == "" && !k.HasChildren(ctx, path) {
-		// We have no children, can delete.
-		store.Delete(encodedKey)
+	if !entry.HasData() {
+		if !k.HasChildren(ctx, path) {
+			// We have no children, can delete.
+			store.Delete(encodedKey)
+		} else {
+			store.Set(encodedKey, types.EncodedNoDataValue)
+		}
 	} else {
 		// Update the value.
-		bz := bytes.Join([][]byte{types.EncodedDataPrefix, []byte(value)}, []byte{})
+		bz := bytes.Join([][]byte{types.EncodedDataPrefix, []byte(entry.StringValue())}, []byte{})
 		store.Set(encodedKey, bz)
 	}
 
 	// Update our other parent children.
 	pathComponents := strings.Split(path, types.PathSeparator)
-	if value == "" {
+	if !entry.HasData() {
 		// delete placeholder ancestors if they're no longer needed
 		for i := len(pathComponents) - 1; i >= 0; i-- {
 			ancestor := componentsToPath(pathComponents[0:i])
@@ -296,7 +330,7 @@ func (k Keeper) SetStorage(ctx sdk.Context, path, value string) {
 				// The ancestor exists, implying all further ancestors exist, so we can break.
 				break
 			}
-			store.Set(types.PathToEncodedKey(ancestor), types.EncodedDataPrefix)
+			store.Set(types.PathToEncodedKey(ancestor), types.EncodedNoDataValue)
 		}
 	}
 }