feat(internal): Add StorageNode delete method

golang/cosmos/x/vstorage/vstorage.go

@@ -31,14 +31,35 @@ func NewStorageHandler(keeper Keeper) vstorageHandler {
 	return vstorageHandler{keeper: keeper}
 }
 
-func unmarshalSinglePathFromArgs(args []json.RawMessage, path *string) error {
+func unmarshalString(jsonText json.RawMessage) (string, error) {
+	var str *string
+	if err := json.Unmarshal(jsonText, &str); err != nil {
+		return "", err
+	} else if str == nil {
+		return "", fmt.Errorf("cannot unmarshal `null` into string")
+	}
+	return *str, nil
+}
+
+func unmarshalSinglePathFromArgs(args []json.RawMessage) (string, error) {
 	if len(args) == 0 {
-		return fmt.Errorf("missing 'path' argument")
+		return "", fmt.Errorf("missing 'path' argument")
+	} else if len(args) != 1 {
+		return "", fmt.Errorf("extra arguments after 'path'")
 	}
-	if len(args) != 1 {
-		return fmt.Errorf("extra arguments after 'path'")
+	return unmarshalString(args[0])
+}
+
+func unmarshalPathsFromArgs(args []json.RawMessage) ([]string, error) {
+	paths := make([]string, len(args))
+	for i, arg := range args {
+		path, err := unmarshalString(arg)
+		if err != nil {
+			return nil, err
+		}
+		paths[i] = path
 	}
-	return json.Unmarshal(args[0], path)
+	return paths, nil
 }
 
 func (sh vstorageHandler) Receive(cctx *vm.ControllerContext, str string) (ret string, err error) {
@@ -104,6 +125,17 @@ func (sh vstorageHandler) Receive(cctx *vm.ControllerContext, str string) (ret s
 		}
 		return "true", nil
 
+	case "delete":
+		paths, err := unmarshalPathsFromArgs(msg.Args)
+		if err != nil {
+			return "", err
+		}
+		for _, path := range paths {
+			newEntry := types.NewStorageEntryWithNoData(path)
+			keeper.SetStorageAndNotify(cctx.Context, newEntry)
+		}
+		return "true", nil
+
 	case "append":
 		for _, arg := range msg.Args {
 			var entry types.StorageEntry
@@ -124,10 +156,9 @@ func (sh vstorageHandler) Receive(cctx *vm.ControllerContext, str string) (ret s
 
 	case "get":
 		// Note that "get" does not (currently) unwrap a StreamCell.
-		var path string
-		err = unmarshalSinglePathFromArgs(msg.Args, &path)
+		path, err := unmarshalSinglePathFromArgs(msg.Args)
 		if err != nil {
-			return
+			return "", err
 		}
 
 		entry := keeper.GetEntry(cctx.Context, path)
@@ -141,10 +172,9 @@ func (sh vstorageHandler) Receive(cctx *vm.ControllerContext, str string) (ret s
 		return string(bz), nil
 
 	case "getStoreKey":
-		var path string
-		err = unmarshalSinglePathFromArgs(msg.Args, &path)
+		path, err := unmarshalSinglePathFromArgs(msg.Args)
 		if err != nil {
-			return
+			return "", err
 		}
 		value := vstorageStoreKey{
 			StoreName:       keeper.GetStoreName(),
@@ -159,10 +189,9 @@ func (sh vstorageHandler) Receive(cctx *vm.ControllerContext, str string) (ret s
 		return string(bz), nil
 
 	case "has":
-		var path string
-		err = unmarshalSinglePathFromArgs(msg.Args, &path)
+		path, err := unmarshalSinglePathFromArgs(msg.Args)
 		if err != nil {
-			return
+			return "", err
 		}
 		value := keeper.HasStorage(cctx.Context, path)
 		if !value {
@@ -172,10 +201,9 @@ func (sh vstorageHandler) Receive(cctx *vm.ControllerContext, str string) (ret s
 
 	// TODO: "keys" is deprecated
 	case "children", "keys":
-		var path string
-		err = unmarshalSinglePathFromArgs(msg.Args, &path)
+		path, err := unmarshalSinglePathFromArgs(msg.Args)
 		if err != nil {
-			return
+			return "", err
 		}
 		children := keeper.GetChildren(cctx.Context, path)
 		if children.Children == nil {
@@ -188,10 +216,9 @@ func (sh vstorageHandler) Receive(cctx *vm.ControllerContext, str string) (ret s
 		return string(bytes), nil
 
 	case "entries":
-		var path string
-		err = unmarshalSinglePathFromArgs(msg.Args, &path)
+		path, err := unmarshalSinglePathFromArgs(msg.Args)
 		if err != nil {
-			return
+			return "", err
 		}
 		children := keeper.GetChildren(cctx.Context, path)
 		entries := make([][]interface{}, len(children.Children))
@@ -210,10 +237,9 @@ func (sh vstorageHandler) Receive(cctx *vm.ControllerContext, str string) (ret s
 		return string(bytes), nil
 
 	case "values":
-		var path string
-		err = unmarshalSinglePathFromArgs(msg.Args, &path)
+		path, err := unmarshalSinglePathFromArgs(msg.Args)
 		if err != nil {
-			return
+			return "", err
 		}
 		children := keeper.GetChildren(cctx.Context, path)
 		vals := make([]string, len(children.Children))
@@ -227,10 +253,9 @@ func (sh vstorageHandler) Receive(cctx *vm.ControllerContext, str string) (ret s
 		return string(bytes), nil
 
 	case "size":
-		var path string
-		err = unmarshalSinglePathFromArgs(msg.Args, &path)
+		path, err := unmarshalSinglePathFromArgs(msg.Args)
 		if err != nil {
-			return
+			return "", err
 		}
 		children := keeper.GetChildren(cctx.Context, path)
 		if children.Children == nil {

golang/cosmos/x/vstorage/vstorage_test.go

@@ -88,7 +88,8 @@ func TestGetAndHas(t *testing.T) {
 		{label: "empty non-terminal", args: []interface{}{"top.empty-non-terminal"}, want: `null`},
 		{label: "no entry", args: []interface{}{"nosuchpath"}, want: `null`},
 		{label: "empty args", args: []interface{}{}, errContains: ptr(`missing`)},
-		{label: "non-string arg", args: []interface{}{42}, errContains: ptr(`json`)},
+		{label: "number arg", args: []interface{}{42}, errContains: ptr(`json`)},
+		{label: "null arg", args: []interface{}{nil}, errContains: ptr(`null`)},
 		{label: "extra args", args: []interface{}{"foo", "bar"}, errContains: ptr(`extra`)},
 	}
 	for _, desc := range cases {
@@ -123,12 +124,13 @@ func TestGetAndHas(t *testing.T) {
 	}
 }
 
-func doTestSet(t *testing.T, method string, expectNotify bool) {
+func doTestSetAndDelete(t *testing.T, setMethod string, expectNotify bool) {
 	kit := makeTestKit()
 	keeper, handler, ctx, cctx := kit.keeper, kit.handler, kit.ctx, kit.cctx
 
 	type testCase struct {
 		label        string
+		method       string
 		args         []interface{}
 		errContains  *string
 		skipReadBack map[int]bool
@@ -144,23 +146,42 @@ func doTestSet(t *testing.T, method string, expectNotify bool) {
 			args: []interface{}{[]string{"baz.a", "qux"}, []string{"baz.b", "qux"}},
 		},
 		{label: "other multi-value",
-			args: []interface{}{[]string{"qux", "A"}, []string{"quux", "B"}},
+			args: []interface{}{[]string{"qux", "A"}, []string{"quux", "B"}, []string{"ephemeral", "C"}},
 		},
 		{label: "overwrites",
 			args: []interface{}{[]string{"bar"}, []string{"quux", "new"}},
 		},
-		{label: "non-string path",
+		{label: "deletes", method: "delete",
+			args: []interface{}{"final.final.corge", "baz.b", "baz.a"},
+		},
+		{label: "more deletes", method: "delete",
+			args: []interface{}{"ephemeral", "notfound", "alsonotfound"},
+		},
+		{label: "restorations",
+			args: []interface{}{[]string{"baz.b", "quux"}, []string{"baz.a", "quux"}},
+		},
+		{label: "number path",
 			// TODO: Fully validate input before making changes
 			// args:        []interface{}{[]string{"foo", "X"}, []interface{}{42, "new"}},
 			args:        []interface{}{[]interface{}{42, "new"}},
 			errContains: ptr("path"),
 		},
+		{label: "null path",
+			// TODO: Fully validate input before making changes
+			// args:        []interface{}{[]string{"foo", "Y"}, []interface{}{nil, "new"}},
+			args:        []interface{}{[]interface{}{nil, "new"}},
+			errContains: ptr("path"),
+		},
 		{label: "non-string value",
 			// TODO: Fully validate input before making changes
 			// args:        []interface{}{[]string{"foo", "X"}, []interface{}{"foo", true}},
 			args:        []interface{}{[]interface{}{"foo", true}},
 			errContains: ptr("value"),
 		},
+		{label: "null path", method: "delete",
+			args:        []interface{}{nil, "foo"},
+			errContains: ptr("null"),
+		},
 		{label: "self-overwrite",
 			args:         []interface{}{[]string{"final.final.corge", "grault"}, []string{"final.final.corge", "garply"}},
 			skipReadBack: map[int]bool{0: true},
@@ -175,14 +196,31 @@ func doTestSet(t *testing.T, method string, expectNotify bool) {
 	}
 	// Expect events to be alphabetized by key.
 	expectedFlushEvents := sdk.Events{
-		stateChangeEvent("baz.a", "qux"),
-		stateChangeEvent("baz.b", "qux"),
+		stateChangeEvent("baz.a", "quux"),
+		stateChangeEvent("baz.b", "quux"),
 		stateChangeEvent("final.final.corge", "garply"),
 		stateChangeEvent("foo", "bar"),
 		stateChangeEvent("quux", "new"),
 		stateChangeEvent("qux", "A"),
 	}
 	for _, desc := range cases {
+		method := desc.method
+		if method == "" {
+			method = setMethod
+		} else if method == "delete" && setMethod == "setWithoutNotify" {
+			// "delete" notifies, so replace it with set-empty when testing *without* notifies.
+			method = setMethod
+			for i, arg := range desc.args {
+				if str, ok := arg.(string); ok {
+					desc.args[i] = []string{str}
+				} else {
+					desc.args[i] = []interface{}{arg}
+					if desc.errContains != nil {
+						desc.errContains = ptr("path")
+					}
+				}
+			}
+		}
 		got, err := callReceive(handler, cctx, method, desc.args)
 
 		if desc.errContains == nil {
@@ -195,6 +233,10 @@ func doTestSet(t *testing.T, method string, expectNotify bool) {
 			// Read the data back.
 			for i, arg := range desc.args {
 				entry, ok := arg.([]string)
+				if method == "delete" {
+					path, pathOk := arg.(string)
+					entry, ok = []string{path}, pathOk
+				}
 				if desc.skipReadBack[i] {
 					continue
 				} else if !ok {
@@ -224,31 +266,31 @@ func doTestSet(t *testing.T, method string, expectNotify bool) {
 
 	// Verify corresponding events.
 	if got := ctx.EventManager().Events(); !reflect.DeepEqual(got, sdk.Events{}) {
-		t.Errorf("%s got unexpected events before flush %#v", method, got)
+		t.Errorf("%s got unexpected events before flush %#v", setMethod, got)
 	}
 	keeper.FlushChangeEvents(ctx)
 	if !expectNotify {
 		if got := ctx.EventManager().Events(); !reflect.DeepEqual(got, sdk.Events{}) {
-			t.Errorf("%s got unexpected events after flush %#v", method, got)
+			t.Errorf("%s got unexpected events after flush %#v", setMethod, got)
 		}
 	} else if got := ctx.EventManager().Events(); !reflect.DeepEqual(got, expectedFlushEvents) {
 		for _, evt := range got {
 			attrs := make([]string, len(evt.Attributes))
 			for i, attr := range evt.Attributes {
 				attrs[i] = fmt.Sprintf("%s=%q", attr.Key, attr.Value)
 			}
-			t.Logf("%s got event %s<%s>", method, evt.Type, strings.Join(attrs, ", "))
+			t.Logf("%s got event %s<%s>", setMethod, evt.Type, strings.Join(attrs, ", "))
 		}
-		t.Errorf("%s got after flush events %#v; want %#v", method, got, expectedFlushEvents)
+		t.Errorf("%s got after flush events %#v; want %#v", setMethod, got, expectedFlushEvents)
 	}
 }
 
-func TestSet(t *testing.T) {
-	doTestSet(t, "set", true)
+func TestSetAndDelete(t *testing.T) {
+	doTestSetAndDelete(t, "set", true)
 }
 
-func TestSetWithoutNotify(t *testing.T) {
-	doTestSet(t, "setWithoutNotify", false)
+func TestSetWithoutNotifyAndDelete(t *testing.T) {
+	doTestSetAndDelete(t, "setWithoutNotify", false)
 }
 
 // TODO: TestAppend

packages/internal/src/lib-chainStorage.js

@@ -36,14 +36,16 @@ const { Fail } = assert;
  * string-valued data for each node, defaulting to the empty string.
  *
  * @typedef {object} StorageNode
- * @property {(data: string) => Promise<void>} setValue publishes some data
+ * @property {(data: string) => Promise<void>} setValue publishes some data to chain storage
+ * @property {() => Promise<void>} delete removes the chain storage data
  * @property {() => string} getPath the chain storage path at which the node was constructed
  * @property {() => Promise<VStorageKey>} getStoreKey DEPRECATED use getPath
  * @property {(subPath: string, options?: {sequence?: boolean}) => StorageNode} makeChildNode
  */
 
 const ChainStorageNodeI = M.interface('StorageNode', {
   setValue: M.callWhen(M.string()).returns(),
+  delete: M.callWhen().returns(),
   getPath: M.call().returns(M.string()),
   getStoreKey: M.callWhen().returns(M.record()),
   makeChildNode: M.call(M.string())
@@ -107,18 +109,17 @@ harden(assertPathSegment);
  * Must match the switch in vstorage.go using `vstorageMessage` type
  *
  * @typedef { 'get' | 'getStoreKey' | 'has' | 'children' | 'entries' | 'values' |'size' } StorageGetByPathMessageMethod
+ * @typedef { 'delete' } StorageMultiPathMessageMethod
  * @typedef { 'set' | 'setWithoutNotify' | 'append' } StorageUpdateEntriesMessageMethod
- * @typedef {StorageGetByPathMessageMethod | StorageUpdateEntriesMessageMethod } StorageMessageMethod
+ * @typedef { StorageGetByPathMessageMethod | StorageMultiPathMessageMethod | StorageUpdateEntriesMessageMethod } StorageMessageMethod
  * @typedef { [path: string] } StorageGetByPathMessageArgs
+ * @typedef { [...paths: string[]] } StorageMultiPathMessageArgs
  * @typedef { [path: string, value?: string | null] } StorageEntry
  * @typedef { StorageEntry[] } StorageUpdateEntriesMessageArgs
- * @typedef {{
- *   method: StorageGetByPathMessageMethod;
- *   args: StorageGetByPathMessageArgs;
- *  } | {
- *   method: StorageUpdateEntriesMessageMethod;
- *   args: StorageUpdateEntriesMessageArgs;
- * }} StorageMessage
+ * @typedef {{ method: StorageGetByPathMessageMethod, args: StorageGetByPathMessageArgs }} StorageGetByPathMessage
+ * @typedef {{ method: StorageMultiPathMessageMethod, args: StorageMultiPathMessageArgs }} StorageMultiPathMessage
+ * @typedef {{ method: StorageUpdateEntriesMessageMethod, args: StorageUpdateEntriesMessageArgs }} StorageUpdateEntriesMessage
+ * @typedef { StorageGetByPathMessage | StorageMultiPathMessage | StorageUpdateEntriesMessage } StorageMessage
  */
 
 /**
@@ -195,6 +196,11 @@ export const prepareChainStorageNode = zone => {
           args: [entry],
         });
       },
+      /** @type {() => Promise<void>} */
+      async delete() {
+        const { path, messenger } = this.state;
+        await cb.callE(messenger, { method: 'delete', args: [path] });
+      },
       // Possible extensions:
       // * getValue()
       // * getChildNames() and/or makeChildNodes()

packages/internal/src/storage-test-utils.js

@@ -182,6 +182,15 @@ export const makeFakeStorageKit = (rootPath, rootOptions) => {
         }
         break;
       }
+      case 'delete': {
+        trace('toStorage delete', message);
+        /** @type {string[]} */
+        const keys = message.args;
+        for (const key of keys) {
+          data.delete(key);
+        }
+        break;
+      }
       case 'size':
         // Intentionally incorrect because it counts non-child descendants,
         // but nevertheless supports a "has children" test.