From e98021315a5df719aed828c9be86fb134d9a76ea Mon Sep 17 00:00:00 2001 From: Omar Sy Date: Thu, 2 Jul 2026 12:27:15 +0200 Subject: [PATCH] fix(gnovm): reclaim stored key object on delete() of object-keyed map MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit delete(m, key) on a realm map with array/struct keys detached the ARGUMENT key from the realm instead of the STORED key object the map held (an iv.Copy child). The stored key was therefore never DecRef'd, never marked deleted, and never removed from the store — a deterministic, unbounded on-chain storage leak (and unrefunded storage deposit) for any realm that add/deletes an array- or struct-keyed persistent map. DeleteForKey now returns the stored key TypedValue it removed; the delete builtin marks that stored key (not the argument key) in DidUpdate. Single caller; value path unchanged. Regression zrealm_map5.gno: init() persists a map[[1]int]int, main() deletes one entry; the finalize golden now emits d[keyOID](-213) for the reclaimed key. Verified red->green: buggy code omits the d[...] deletion, so the golden mismatches. Co-Authored-By: Claude Opus 4.8 --- gnovm/pkg/gnolang/uverse.go | 15 +++-- gnovm/pkg/gnolang/values.go | 10 +++- gnovm/tests/files/zrealm_map5.gno | 96 +++++++++++++++++++++++++++++++ 3 files changed, 113 insertions(+), 8 deletions(-) create mode 100644 gnovm/tests/files/zrealm_map5.gno diff --git a/gnovm/pkg/gnolang/uverse.go b/gnovm/pkg/gnolang/uverse.go index 1fe8e1ad3ae..9da2c1fa854 100644 --- a/gnovm/pkg/gnolang/uverse.go +++ b/gnovm/pkg/gnolang/uverse.go @@ -1013,12 +1013,15 @@ func makeUverseNode() { if !ok { return } - // delete - mv.DeleteForKey(m, m.Store, &itv) - - // mark key as deleted - keyObj := itv.GetFirstObject(m.Store) - m.Realm.DidUpdate(m, mv, keyObj, nil) + // delete; capture the STORED key that was removed. Detaching + // the argument key (itv) instead orphaned the stored key object + // in the store — it was never DecRef'd nor marked deleted. + delKey := mv.DeleteForKey(m, m.Store, &itv) + + // mark the STORED key object as deleted (not the argument key) + if delKey != nil { + m.Realm.DidUpdate(m, mv, delKey.GetFirstObject(m.Store), nil) + } // mark value as deleted valObj := val.GetFirstObject(m.Store) diff --git a/gnovm/pkg/gnolang/values.go b/gnovm/pkg/gnolang/values.go index 75252be54b5..77fd3bf4128 100644 --- a/gnovm/pkg/gnolang/values.go +++ b/gnovm/pkg/gnolang/values.go @@ -821,16 +821,22 @@ func (mv *MapValue) GetValueForKey(m *Machine, store Store, key *TypedValue) (va return } -func (mv *MapValue) DeleteForKey(m *Machine, store Store, key *TypedValue) { +// DeleteForKey removes the entry for key and returns the STORED key TypedValue +// that was removed (nil if the key was absent or NaN). Callers must dirty-mark / +// DecRef this stored key's object, not the (possibly transient) argument key — +// otherwise a non-primitive stored key object is orphaned in the store. +func (mv *MapValue) DeleteForKey(m *Machine, store Store, key *TypedValue) (deletedKey *TypedValue) { // if key is NaN, do nothing. kmk, isNaN := key.ComputeMapKey(m, store, false) if isNaN { - return + return nil } if mli, ok := mv.vmap[kmk]; ok { mv.List.Remove(mli) delete(mv.vmap, kmk) + return &mli.Key } + return nil } // ---------------------------------------- diff --git a/gnovm/tests/files/zrealm_map5.gno b/gnovm/tests/files/zrealm_map5.gno new file mode 100644 index 00000000000..8948f141ea5 --- /dev/null +++ b/gnovm/tests/files/zrealm_map5.gno @@ -0,0 +1,96 @@ +// PKGPATH: gno.land/r/test +package test + +// Regression: deleting an entry whose KEY is a non-primitive (array/struct) +// object from a persisted realm map must reclaim the stored key object — it +// must be emitted as a deleted object (d[...]) in the finalize diff, not +// orphaned. Before the fix the stored [1]int key object leaked (delete +// detached the argument key, not the stored key). +var m map[[1]int]int + +func init() { + m = map[[1]int]int{[1]int{1}: 10, [1]int{2}: 20} +} + +func main(cur realm) { + delete(m, [1]int{1}) + println("len:", len(m)) +} + +// Output: +// len: 1 + +// Realm: +// finalizerealm["gno.land/r/test"] +// u[08ada09dee16d791fd406d629fe29bb0ed084a30:7](-202)= +// @@ -1,31 +1,6 @@ +// { +// "List": { +// "List": [ +// - { +// - "Key": { +// - "T": { +// - "@type": "/gno.ArrayType", +// - "Elt": { +// - "@type": "/gno.PrimitiveType", +// - "value": "32" +// - }, +// - "Len": "1", +// - "Vrd": false +// - }, +// - "V": { +// - "@type": "/gno.RefValue", +// - "Hash": "983a2dfd65f409988f2165b8b8e31a3f0147a0d7", +// - "ObjectID": "08ada09dee16d791fd406d629fe29bb0ed084a30:8" +// - } +// - }, +// - "Value": { +// - "N": "CgAAAAAAAAA=", +// - "T": { +// - "@type": "/gno.PrimitiveType", +// - "value": "32" +// - } +// - } +// - }, +// { +// "Key": { +// "T": { +// @@ -56,7 +31,7 @@ +// "ObjectInfo": { +// "ID": "08ada09dee16d791fd406d629fe29bb0ed084a30:7", +// "LastObjectSize": "590", +// - "ModTime": "0", +// + "ModTime": "9", +// "OwnerID": "08ada09dee16d791fd406d629fe29bb0ed084a30:3", +// "RefCount": "1" +// } +// u[08ada09dee16d791fd406d629fe29bb0ed084a30:3](0)= +// @@ -2,7 +2,7 @@ +// "ObjectInfo": { +// "ID": "08ada09dee16d791fd406d629fe29bb0ed084a30:3", +// "LastObjectSize": "393", +// - "ModTime": "6", +// + "ModTime": "9", +// "OwnerID": "08ada09dee16d791fd406d629fe29bb0ed084a30:2", +// "RefCount": "1" +// }, +// @@ -25,7 +25,7 @@ +// }, +// "V": { +// "@type": "/gno.RefValue", +// - "Hash": "a669cab1a0b6e73131ef8ee77e2ed6bedcf482e3", +// + "Hash": "b8ca988c204aac98121d376916a114a3f0bee2cd", +// "ObjectID": "08ada09dee16d791fd406d629fe29bb0ed084a30:7" +// } +// } +// u[08ada09dee16d791fd406d629fe29bb0ed084a30:2](0)= +// @@ -34,7 +34,7 @@ +// }, +// "V": { +// "@type": "/gno.RefValue", +// - "Hash": "e000ab9359964c6da1719766969d91e0ab8de8dd", +// + "Hash": "b3f9131b15661e4bfa46d47af69a74439c92d91c", +// "ObjectID": "08ada09dee16d791fd406d629fe29bb0ed084a30:3" +// } +// }, +// d[08ada09dee16d791fd406d629fe29bb0ed084a30:8](-213)