From 5b37352a5f5ecb2c1b35f0dbc973caf875f2bff2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Fri, 29 May 2026 11:55:39 -0700 Subject: [PATCH 01/16] add MutationCount --- array.go | 37 ++ array_data_slab.go | 8 + array_metadata_slab.go | 8 + array_slab.go | 14 + map.go | 38 +++ map_data_slab.go | 8 + map_metadata_slab.go | 8 + map_slab.go | 13 + mutation_count_test.go | 747 +++++++++++++++++++++++++++++++++++++++++ 9 files changed, 881 insertions(+) create mode 100644 mutation_count_test.go diff --git a/array.go b/array.go index 18dd7fe..44a2336 100644 --- a/array.go +++ b/array.go @@ -589,6 +589,11 @@ func (a *Array) PopIterate(fn ArrayPopIterationFunc) error { size = inlinedArrayDataSlabPrefixSize } + // Bump the old root's mutation counter before swapping a.root + // so that sibling wrappers whose .root still points to this orphaned slab + // can detect the root swap. See ArraySlab.MutationCount. + a.root.BumpMutationCount() + // Set root to empty data slab a.root = &ArrayDataSlab{ header: ArraySlabHeader{ @@ -637,6 +642,12 @@ func (a *Array) splitRoot() error { oldRoot := a.root oldRoot.SetSlabID(sID) + // Intentionally NOT calling oldRoot.BumpMutationCount() here: + // ArraySlab.Split reuses the receiver as the LEFT child + // (see ArrayDataSlab.Split / ArrayMetaDataSlab.Split — both return (receiver, rightSlab)), + // so the "old root" is not orphaned — it stays in the tree as the left child. + // If a later promoteChildAsNewRoot picks this slab, + // MutationCount() on the live root would falsely report staleness for the initiating wrapper. // Split old root leftSlab, rightSlab, err := oldRoot.Split(a.Storage) @@ -645,6 +656,15 @@ func (a *Array) splitRoot() error { return err } + // Invariant: ArraySlab.Split must return the receiver as the LEFT child. + // The decision to skip BumpMutationCount above relies on this — + // if Split ever returns a fresh struct for the left side, + // the receiver becomes orphaned and splitRoot must behave like + // promoteChildAsNewRoot (bump). + if Slab(oldRoot) != leftSlab { + panic(NewUnreachableError()) + } + left := leftSlab.(ArraySlab) right := rightSlab.(ArraySlab) @@ -695,6 +715,14 @@ func (a *Array) promoteChildAsNewRoot(childID SlabID) error { rootID := a.root.SlabID() + // Bump the old root's mutation counter before swapping a.root + // so that sibling wrappers whose .root still points to this orphaned slab + // can detect the root swap. + // Promote does not perturb the orphaned old root's SlabID, + // so this counter is the only signal sibling wrappers have. + // See ArraySlab.MutationCount. + a.root.BumpMutationCount() + a.root = child a.root.SetSlabID(rootID) @@ -1325,6 +1353,15 @@ func (a *Array) ValueID() ValueID { return slabIDToValueID(a.root.SlabID()) } +// MutationCount returns the root slab's mutation counter. +// It is bumped on root replacement, +// and not on element-level or non-root structural changes. +// Callers cache the value to detect staleness later. +// See ArraySlab.MutationCount. +func (a *Array) MutationCount() uint64 { + return a.root.MutationCount() +} + func (a *Array) Type() TypeInfo { if extraData := a.root.ExtraData(); extraData != nil { return extraData.TypeInfo diff --git a/array_data_slab.go b/array_data_slab.go index 4be704e..df1bee6 100644 --- a/array_data_slab.go +++ b/array_data_slab.go @@ -555,6 +555,14 @@ func (a *ArrayDataSlab) Header() ArraySlabHeader { return a.header } +func (a *ArrayDataSlab) MutationCount() uint64 { + return a.header.mutationCount +} + +func (a *ArrayDataSlab) BumpMutationCount() { + a.header.mutationCount++ +} + func (a *ArrayDataSlab) IsData() bool { return true } diff --git a/array_metadata_slab.go b/array_metadata_slab.go index eaa29c9..acf6b71 100644 --- a/array_metadata_slab.go +++ b/array_metadata_slab.go @@ -863,6 +863,14 @@ func (a *ArrayMetaDataSlab) Header() ArraySlabHeader { return a.header } +func (a *ArrayMetaDataSlab) MutationCount() uint64 { + return a.header.mutationCount +} + +func (a *ArrayMetaDataSlab) BumpMutationCount() { + a.header.mutationCount++ +} + func (a *ArrayMetaDataSlab) ByteSize() uint32 { return a.header.size } diff --git a/array_slab.go b/array_slab.go index c8cf6b0..1522b2e 100644 --- a/array_slab.go +++ b/array_slab.go @@ -22,6 +22,11 @@ type ArraySlabHeader struct { slabID SlabID // id is used to retrieve slab from storage size uint32 // size is used to split and merge; leaf: size of all element; internal: size of all headers count uint32 // count is used to lookup element; leaf: number of elements; internal: number of elements in all its headers + // mutationCount is a per-slab counter that is bumped when this slab is detached as root. + // It enables callers to detect that their cached view of the root is stale. + // Element-level mutations and non-root structural changes do NOT bump the counter. + // In-memory only (NOT encoded). + mutationCount uint64 } type ArraySlab interface { @@ -43,6 +48,15 @@ type ArraySlab interface { Header() ArraySlabHeader + // MutationCount returns a per-slab counter that is bumped when this slab is detached as root. + // It enables callers to detect that their cached view of the root is stale. + // Element-level mutations and non-root structural changes do NOT bump the counter. + // In-memory only (NOT encoded). + MutationCount() uint64 + // BumpMutationCount bumps the mutation count, which should be called when this slab is detached as root. + // See MutationCount() for details. + BumpMutationCount() + ExtraData() *ArrayExtraData RemoveExtraData() *ArrayExtraData SetExtraData(*ArrayExtraData) diff --git a/map.go b/map.go index 2f2ec41..db8ebe7 100644 --- a/map.go +++ b/map.go @@ -821,6 +821,12 @@ func (m *OrderedMap) PopIterate(fn MapPopIterationFunc) error { prefixSize = uint32(inlinedMapDataSlabPrefixSize) } + // Bump the old root's mutation counter before swapping m.root + // so that sibling wrappers whose .root still points to this orphaned slab + // can detect the root swap. + // See MapSlab.MutationCount. + m.root.BumpMutationCount() + // Set root to empty data slab m.root = &MapDataSlab{ header: MapSlabHeader{ @@ -868,6 +874,12 @@ func (m *OrderedMap) splitRoot() error { oldRoot := m.root oldRoot.SetSlabID(sID) + // Intentionally NOT calling oldRoot.BumpMutationCount() here: + // MapSlab.Split reuses the receiver as the LEFT child + // (see MapDataSlab.Split / MapMetaDataSlab.Split — both return (receiver, rightSlab)), + // so the "old root" is not orphaned — it stays in the tree as the left child. + // If a later promoteChildAsNewRoot picks this slab, + // MutationCount() on the live root would falsely report staleness for the initiating wrapper. // Split old root leftSlab, rightSlab, err := oldRoot.Split(m.Storage) @@ -876,6 +888,15 @@ func (m *OrderedMap) splitRoot() error { return err } + // Invariant: MapSlab.Split must return the receiver as the LEFT child. + // The decision to skip BumpMutationCount above relies on this — + // if Split ever returns a fresh struct for the left side, + // the receiver becomes orphaned and splitRoot must behave like + // promoteChildAsNewRoot (bump). + if Slab(oldRoot) != leftSlab { + panic(NewUnreachableError()) + } + left := leftSlab.(MapSlab) right := rightSlab.(MapSlab) @@ -923,6 +944,14 @@ func (m *OrderedMap) promoteChildAsNewRoot(childID SlabID) error { rootID := m.root.SlabID() + // Bump the old root's mutation counter before swapping m.root + // so that sibling wrappers whose .root still points to this orphaned slab + // can detect the root swap. + // Promote does not perturb the orphaned old root's SlabID, + // so this counter is the only signal sibling wrappers have. + // See MapSlab.MutationCount. + m.root.BumpMutationCount() + m.root = child m.root.SetSlabID(rootID) @@ -1530,6 +1559,15 @@ func (m *OrderedMap) ValueID() ValueID { return slabIDToValueID(m.root.SlabID()) } +// MutationCount returns the root slab's mutation counter. +// It is bumped on root replacement, +// and not on element-level or non-root structural changes. +// Callers cache the value to detect staleness later. +// See MapSlab.MutationCount. +func (m *OrderedMap) MutationCount() uint64 { + return m.root.MutationCount() +} + // CanCopyNonRefSimple returns true if the map can be copied // as a container with only non-reference and simple storables. func (m *OrderedMap) CanCopyNonRefSimple() bool { diff --git a/map_data_slab.go b/map_data_slab.go index 3c68fb3..65fe24c 100644 --- a/map_data_slab.go +++ b/map_data_slab.go @@ -449,6 +449,14 @@ func (m *MapDataSlab) Header() MapSlabHeader { return m.header } +func (m *MapDataSlab) MutationCount() uint64 { + return m.header.mutationCount +} + +func (m *MapDataSlab) BumpMutationCount() { + m.header.mutationCount++ +} + func (m *MapDataSlab) IsData() bool { return true } diff --git a/map_metadata_slab.go b/map_metadata_slab.go index bdbecfe..bcb0a1f 100644 --- a/map_metadata_slab.go +++ b/map_metadata_slab.go @@ -785,6 +785,14 @@ func (m *MapMetaDataSlab) Header() MapSlabHeader { return m.header } +func (m *MapMetaDataSlab) MutationCount() uint64 { + return m.header.mutationCount +} + +func (m *MapMetaDataSlab) BumpMutationCount() { + m.header.mutationCount++ +} + func (m *MapMetaDataSlab) ByteSize() uint32 { return m.header.size } diff --git a/map_slab.go b/map_slab.go index 31f19f2..6e4ee32 100644 --- a/map_slab.go +++ b/map_slab.go @@ -24,6 +24,11 @@ type MapSlabHeader struct { slabID SlabID // id is used to retrieve slab from storage size uint32 // size is used to split and merge; leaf: size of all element; internal: size of all headers firstKey Digest // firstKey (first hashed key) is used to lookup value + // mutationCount is a per-slab counter that is bumped when this slab is detached as root. + // It enables callers to detect that their cached view of the root is stale. + // Element-level mutations and non-root structural changes do NOT bump the counter. + // In-memory only (NOT encoded). + mutationCount uint64 } type MapSlab interface { @@ -79,6 +84,14 @@ type MapSlab interface { Header() MapSlabHeader + // MutationCount returns a per-slab counter that is bumped when this slab is detached as root. + // It enables callers to detect that their cached view of the root is stale. + // Element-level mutations and non-root structural changes do NOT bump the counter. + MutationCount() uint64 + // BumpMutationCount bumps the mutation count, which should be called when this slab is detached as root. + // See MutationCount() for details. + BumpMutationCount() + // canCopyWithoutSlabID returns true if // - All elements can be copied, and // - Slab doesn't have any references to other slabs (such as next slab ID). diff --git a/mutation_count_test.go b/mutation_count_test.go new file mode 100644 index 0000000..a468c14 --- /dev/null +++ b/mutation_count_test.go @@ -0,0 +1,747 @@ +/* + * Atree - Scalable Arrays and Ordered Maps + * + * Copyright Flow Foundation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package atree_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/onflow/atree" + testutils "github.com/onflow/atree/test_utils" +) + +// MutationCount is exposed on *Array / *OrderedMap so that callers caching +// the value at wrapper construction can detect when their .root pointer has +// been orphaned by a structural mutation that preserves the orphaned slab's +// SlabID — namely promoteChildAsNewRoot and PopIterate. The counter lives on +// the slab struct so that two Go-level container structs that share an +// orphaned slab pointer both observe the same bumped value. +// +// splitRoot is NOT a bump site: ArraySlab.Split / MapSlab.Split return the +// receiver as the LEFT child, so the "old root" stays in the tree as a child +// slab. If a subsequent promote picks that slab, *Array.MutationCount() +// would otherwise erroneously report staleness for the wrapper that +// initiated the operations. splitRoot already perturbs the old root's +// SlabID, which is the signal sibling wrappers observe via the ValueID-based +// staleness check. +// +// These tests pin the contract: +// 1. Counter starts at 0 for a freshly constructed container. +// 2. Element-level operations that do not detach a root leave the counter +// unchanged (Get / Set / Append/Insert below split threshold / Remove +// that does not promote). +// 3. splitRoot does NOT bump the counter (regression test: bumping there +// would corrupt the live counter for the initiating wrapper if the +// demoted left child is later re-promoted to root). +// 4. promoteChildAsNewRoot bumps the OLD root's counter exactly once. +// 5. PopIterate bumps the OLD root's counter exactly once. +// 6. A pointer to the OLD root captured before promote/PopIterate sees the +// bumped value; the live *Array.MutationCount() reads the NEW root's +// counter, which is fresh (= 0) and therefore matches what the +// initiating wrapper would have cached. +// 7. The wrapper that initiated a full grow+collapse cycle does not see a +// false-positive staleness on the live counter. + +// --- Invariant tests: ArraySlab.Split / MapSlab.Split return the receiver +// as the LEFT child --- +// +// The decision NOT to bump MutationCount at splitRoot rests on this +// invariant: if Split started returning a fresh struct for the left side +// instead, the old root would become orphaned and splitRoot would need to +// behave like promote/PopIterate (bump). These tests pin the contract so a +// future Split refactor breaks loudly here rather than silently +// re-introducing the bug. + +func TestSplitRoot_Array_DemotedRootIsReusedAsLeftChild(t *testing.T) { + atree.SetThreshold(256) + defer atree.SetThreshold(1024) + + typeInfo := testutils.NewSimpleTypeInfo(42) + storage := newTestPersistentStorage(t) + address := atree.Address{1, 2, 3, 4, 5, 6, 7, 8} + + array, err := atree.NewArray(storage, address, typeInfo) + require.NoError(t, err) + + // Capture the initial root by Go pointer. + oldRoot := atree.GetArrayRootSlab(array) + require.True(t, oldRoot.IsData(), "initial root is a data slab") + + // Force splitRoot. Stop as soon as the first split fires so that the + // data-slab root we captured stays at the topmost child level — any + // further splits would demote it deeper into the tree. + for i := uint64(0); ; i++ { + require.NoError(t, array.Append(testutils.Uint64Value(i))) + if !atree.GetArrayRootSlab(array).IsData() { + break + } + } + require.False(t, atree.GetArrayRootSlab(array).IsData(), + "prerequisite: a split must have occurred") + + // The new root is a metadata slab. + metaRoot, ok := atree.GetArrayRootSlab(array).(*atree.ArrayMetaDataSlab) + require.True(t, ok) + + // The left child SlabID, as recorded by splitRoot, must point back to + // the same Go struct as the captured old root. + childSlabIDs, _ := atree.GetArrayMetaDataSlabChildInfo(metaRoot) + require.GreaterOrEqual(t, len(childSlabIDs), 2, + "split must produce at least two children") + + leftChild, found, err := storage.Retrieve(childSlabIDs[0]) + require.NoError(t, err) + require.True(t, found, "left child must be retrievable from storage") + + require.Same(t, oldRoot, leftChild, + "ArraySlab.Split must return the receiver as the LEFT child; if "+ + "this fails, splitRoot's old root becomes orphaned and "+ + "MutationCount must be bumped there too (currently it is not)") + + // Bonus: the SlabID on the demoted slab is the fresh one splitRoot + // generated via SetSlabID(sID), which is also what's recorded in the + // new root's left header. + require.Equal(t, leftChild.SlabID(), oldRoot.SlabID(), + "SetSlabID side effect is observable on the captured pointer") +} + +func TestSplitRoot_Map_DemotedRootIsReusedAsLeftChild(t *testing.T) { + atree.SetThreshold(256) + defer atree.SetThreshold(1024) + + typeInfo := testutils.NewSimpleTypeInfo(42) + storage := newTestPersistentStorage(t) + address := atree.Address{1, 2, 3, 4, 5, 6, 7, 8} + + m, err := atree.NewMap(storage, address, atree.NewDefaultDigesterBuilder(), typeInfo) + require.NoError(t, err) + + oldRoot := atree.GetMapRootSlab(m) + require.True(t, oldRoot.IsData(), "initial root is a data slab") + + cmp := testutils.CompareValue + hip := testutils.GetHashInput + // Stop at the first split so the captured root stays at the topmost + // child level rather than being demoted deeper by subsequent splits. + for i := uint64(0); ; i++ { + _, err := m.Set(cmp, hip, testutils.Uint64Value(i), testutils.Uint64Value(i)) + require.NoError(t, err) + if !atree.GetMapRootSlab(m).IsData() { + break + } + } + require.False(t, atree.GetMapRootSlab(m).IsData(), + "prerequisite: a split must have occurred") + + metaRoot, ok := atree.GetMapRootSlab(m).(*atree.MapMetaDataSlab) + require.True(t, ok) + + childSlabIDs, _, _ := atree.GetMapMetaDataSlabChildInfo(metaRoot) + require.GreaterOrEqual(t, len(childSlabIDs), 2, + "split must produce at least two children") + + leftChild, found, err := storage.Retrieve(childSlabIDs[0]) + require.NoError(t, err) + require.True(t, found, "left child must be retrievable from storage") + + require.Same(t, oldRoot, leftChild, + "MapSlab.Split must return the receiver as the LEFT child; if "+ + "this fails, splitRoot's old root becomes orphaned and "+ + "MutationCount must be bumped there too (currently it is not)") + + require.Equal(t, leftChild.SlabID(), oldRoot.SlabID(), + "SetSlabID side effect is observable on the captured pointer") +} + +// --- Array --- + +func TestMutationCount_Array_StartsAtZero(t *testing.T) { + t.Parallel() + + typeInfo := testutils.NewSimpleTypeInfo(42) + storage := newTestPersistentStorage(t) + address := atree.Address{1, 2, 3, 4, 5, 6, 7, 8} + + array, err := atree.NewArray(storage, address, typeInfo) + require.NoError(t, err) + + require.Equal(t, uint64(0), array.MutationCount()) + require.Equal(t, uint64(0), atree.GetArrayRootSlab(array).MutationCount()) +} + +func TestMutationCount_Array_GetDoesNotBump(t *testing.T) { + t.Parallel() + + typeInfo := testutils.NewSimpleTypeInfo(42) + storage := newTestPersistentStorage(t) + address := atree.Address{1, 2, 3, 4, 5, 6, 7, 8} + + array, err := atree.NewArray(storage, address, typeInfo) + require.NoError(t, err) + + require.NoError(t, array.Append(testutils.Uint64Value(0))) + require.NoError(t, array.Append(testutils.Uint64Value(1))) + require.NoError(t, array.Append(testutils.Uint64Value(2))) + + for i := uint64(0); i < array.Count(); i++ { + _, err := array.Get(i) + require.NoError(t, err) + } + + require.Equal(t, uint64(0), array.MutationCount()) +} + +func TestMutationCount_Array_ElementOpsBelowSplitDoNotBump(t *testing.T) { + t.Parallel() + + // Keep the default threshold so a handful of appends stays in one slab. + typeInfo := testutils.NewSimpleTypeInfo(42) + storage := newTestPersistentStorage(t) + address := atree.Address{1, 2, 3, 4, 5, 6, 7, 8} + + array, err := atree.NewArray(storage, address, typeInfo) + require.NoError(t, err) + + for i := uint64(0); i < 10; i++ { + require.NoError(t, array.Append(testutils.Uint64Value(i))) + } + for i := uint64(0); i < 10; i++ { + _, err := array.Set(i, testutils.Uint64Value(i*2)) + require.NoError(t, err) + } + for i := uint64(0); i < 5; i++ { + _, err := array.Remove(0) + require.NoError(t, err) + } + for i := uint64(0); i < 3; i++ { + require.NoError(t, array.Insert(0, testutils.Uint64Value(100+i))) + } + + require.True(t, atree.GetArrayRootSlab(array).IsData(), + "tree should still be a single data slab") + require.Equal(t, uint64(0), array.MutationCount()) +} + +func TestMutationCount_Array_PopIterateBumpsOldRoot(t *testing.T) { + t.Parallel() + + typeInfo := testutils.NewSimpleTypeInfo(42) + storage := newTestPersistentStorage(t) + address := atree.Address{1, 2, 3, 4, 5, 6, 7, 8} + + array, err := atree.NewArray(storage, address, typeInfo) + require.NoError(t, err) + + for i := uint64(0); i < 5; i++ { + require.NoError(t, array.Append(testutils.Uint64Value(i))) + } + + oldRoot := atree.GetArrayRootSlab(array) + require.Equal(t, uint64(0), oldRoot.MutationCount()) + + err = array.PopIterate(func(_ atree.Storable) {}) + require.NoError(t, err) + + require.NotSame(t, oldRoot, atree.GetArrayRootSlab(array), + "PopIterate must replace the root with a fresh empty data slab") + require.Equal(t, uint64(1), oldRoot.MutationCount(), + "PopIterate must bump the orphaned old root exactly once") + require.Equal(t, uint64(0), array.MutationCount(), + "live MutationCount reads the fresh new root") +} + +// TestMutationCount_Array_SplitRootDoesNotBumpCounter is a regression guard: +// the obvious "bump on every root swap" reading of the design would bump +// here too, but ArraySlab.Split returns the receiver as the left child, so +// the old root is not orphaned — it remains in the tree. Bumping it would +// pollute a later promote. +func TestMutationCount_Array_SplitRootDoesNotBumpCounter(t *testing.T) { + atree.SetThreshold(256) + defer atree.SetThreshold(1024) + + typeInfo := testutils.NewSimpleTypeInfo(42) + storage := newTestPersistentStorage(t) + address := atree.Address{1, 2, 3, 4, 5, 6, 7, 8} + + array, err := atree.NewArray(storage, address, typeInfo) + require.NoError(t, err) + + oldRoot := atree.GetArrayRootSlab(array) + originalSlabID := oldRoot.SlabID() + require.Equal(t, uint64(0), oldRoot.MutationCount()) + + for i := uint64(0); i < 200; i++ { + require.NoError(t, array.Append(testutils.Uint64Value(i))) + } + + require.False(t, atree.GetArrayRootSlab(array).IsData(), + "prerequisite: a split must have occurred") + require.NotSame(t, oldRoot, atree.GetArrayRootSlab(array)) + + // splitRoot reuses oldRoot as the LEFT child. Its counter must remain + // 0 — otherwise a later promoteChildAsNewRoot that re-promotes this + // slab would make the live root carry a non-zero counter, producing a + // false stale signal to the wrapper that initiated the operations. + require.Equal(t, uint64(0), oldRoot.MutationCount(), + "splitRoot must not bump the demoted slab") + + // Sibling wrappers detect splitRoot via the ValueID change instead: + // splitRoot called SetSlabID on the demoted slab. + require.NotEqual(t, originalSlabID, oldRoot.SlabID(), + "splitRoot must change the demoted slab's SlabID — that is the "+ + "sibling-detection signal for splits") +} + +func TestMutationCount_Array_RemoveUntilPromoteBumpsOldRoot(t *testing.T) { + atree.SetThreshold(256) + defer atree.SetThreshold(1024) + + typeInfo := testutils.NewSimpleTypeInfo(42) + storage := newTestPersistentStorage(t) + address := atree.Address{1, 2, 3, 4, 5, 6, 7, 8} + + array, err := atree.NewArray(storage, address, typeInfo) + require.NoError(t, err) + + // Grow to multi-level. + for i := uint64(0); i < 200; i++ { + require.NoError(t, array.Append(testutils.Uint64Value(i))) + } + require.False(t, atree.GetArrayRootSlab(array).IsData(), + "prerequisite: tree must be multi-level before testing promote") + + // Capture the metadata-slab root that promote will displace. + oldMetaRoot := atree.GetArrayRootSlab(array) + require.Equal(t, uint64(0), oldMetaRoot.MutationCount(), + "the post-split root starts with a fresh counter (splitRoot does not bump)") + + // Remove until the tree collapses back to a single data slab. + for array.Count() > 0 && !atree.GetArrayRootSlab(array).IsData() { + _, err := array.Remove(0) + require.NoError(t, err) + } + + // Promote happened: root is now a data slab, with a different struct + // identity than the metadata-slab root we captured. + require.True(t, atree.GetArrayRootSlab(array).IsData(), + "tree must have collapsed back to a single data slab via promote") + require.NotSame(t, oldMetaRoot, atree.GetArrayRootSlab(array), + "root struct must have changed identity after promoteChildAsNewRoot") + + // The orphaned metadata-slab root must have its counter bumped exactly + // once. Cascading promotes (multi-level collapse) each bump their own + // orphaned slab — but oldMetaRoot is the topmost, displaced by the + // first promote in the cascade. + require.Equal(t, uint64(1), oldMetaRoot.MutationCount(), + "promoteChildAsNewRoot must bump the orphaned old root exactly once") + + // The live root is the freshly promoted slab — counter 0. + require.Equal(t, uint64(0), array.MutationCount(), + "live MutationCount reads the freshly promoted child") +} + +// TestMutationCount_Array_SiblingObservesPromoteWithStableValueID is the +// canonical regression test for the gap promoteChildAsNewRoot leaves in the +// SlabID-based detection. +func TestMutationCount_Array_SiblingObservesPromoteWithStableValueID(t *testing.T) { + atree.SetThreshold(256) + defer atree.SetThreshold(1024) + + typeInfo := testutils.NewSimpleTypeInfo(42) + storage := newTestPersistentStorage(t) + address := atree.Address{1, 2, 3, 4, 5, 6, 7, 8} + + array, err := atree.NewArray(storage, address, typeInfo) + require.NoError(t, err) + + // Grow to multi-level FIRST. + for i := uint64(0); i < 200; i++ { + require.NoError(t, array.Append(testutils.Uint64Value(i))) + } + require.False(t, atree.GetArrayRootSlab(array).IsData()) + + // "Sibling" snapshot: a wrapper constructed at this point would cache + // the live ValueID and MutationCount, both reflecting the multi-level + // tree state. + siblingRoot := atree.GetArrayRootSlab(array) + cachedValueID := array.ValueID() + cachedMutationCount := array.MutationCount() + + // Now collapse the tree via removes — triggers promoteChildAsNewRoot. + for array.Count() > 0 && !atree.GetArrayRootSlab(array).IsData() { + _, err := array.Remove(0) + require.NoError(t, err) + } + require.True(t, atree.GetArrayRootSlab(array).IsData(), + "prerequisite: a promote must have occurred") + + // The value ID is stable across promote — promote assigns the original + // rootID to the promoted child and never perturbs the orphaned old + // root's SlabID. This is the gap a SlabID-based check misses. + require.Equal(t, cachedValueID, array.ValueID(), + "ValueID is preserved across promote — this is the gap that "+ + "SlabID-based staleness checks miss") + + // What detects the gap: the orphaned old root's counter is bumped, and + // a "sibling" view via siblingRoot.MutationCount() observes the bump. + require.Greater(t, siblingRoot.MutationCount(), cachedMutationCount, + "the orphaned old root's counter must diverge from the cached value") +} + +// TestMutationCount_Array_NoFalseStaleForInitiatingWrapper covers the +// regression that motivated removing the splitRoot bump: a wrapper that +// itself initiates a grow+collapse cycle must not see a divergent live +// counter from its cached value. +func TestMutationCount_Array_NoFalseStaleForInitiatingWrapper(t *testing.T) { + atree.SetThreshold(256) + defer atree.SetThreshold(1024) + + typeInfo := testutils.NewSimpleTypeInfo(42) + storage := newTestPersistentStorage(t) + address := atree.Address{1, 2, 3, 4, 5, 6, 7, 8} + + array, err := atree.NewArray(storage, address, typeInfo) + require.NoError(t, err) + + primaryCachedCount := array.MutationCount() + require.Equal(t, uint64(0), primaryCachedCount) + + // Full grow + collapse cycle (multiple splits + multiple promotes). + for i := uint64(0); i < 500; i++ { + require.NoError(t, array.Append(testutils.Uint64Value(i))) + } + for array.Count() > 0 { + _, err := array.Remove(0) + require.NoError(t, err) + } + + require.Equal(t, primaryCachedCount, array.MutationCount(), + "the wrapper that initiated all operations must not see a false stale: "+ + "each swap bumps only the OLD root being detached, and the live "+ + "root is always a fresh slab with counter 0") +} + +// TestMutationCount_Array_OrphanedRootIsNotFurtherBumped pins down the +// monotonicity contract: once a slab is detached, future operations through +// the *Array must not retroactively touch its counter. +func TestMutationCount_Array_OrphanedRootIsNotFurtherBumped(t *testing.T) { + typeInfo := testutils.NewSimpleTypeInfo(42) + storage := newTestPersistentStorage(t) + address := atree.Address{1, 2, 3, 4, 5, 6, 7, 8} + + array, err := atree.NewArray(storage, address, typeInfo) + require.NoError(t, err) + + for i := uint64(0); i < 5; i++ { + require.NoError(t, array.Append(testutils.Uint64Value(i))) + } + + // First PopIterate orphans the initial root. + root0 := atree.GetArrayRootSlab(array) + err = array.PopIterate(func(_ atree.Storable) {}) + require.NoError(t, err) + require.Equal(t, uint64(1), root0.MutationCount()) + + // Now run more operations through *Array. root0 is no longer accessed + // by *Array.root, so its counter must remain 1. + for i := uint64(0); i < 5; i++ { + require.NoError(t, array.Append(testutils.Uint64Value(i))) + } + err = array.PopIterate(func(_ atree.Storable) {}) + require.NoError(t, err) + + require.Equal(t, uint64(1), root0.MutationCount(), + "a slab no longer referenced by *Array must not receive further bumps") +} + +// --- OrderedMap --- + +func TestMutationCount_Map_StartsAtZero(t *testing.T) { + t.Parallel() + + typeInfo := testutils.NewSimpleTypeInfo(42) + storage := newTestPersistentStorage(t) + address := atree.Address{1, 2, 3, 4, 5, 6, 7, 8} + + m, err := atree.NewMap(storage, address, atree.NewDefaultDigesterBuilder(), typeInfo) + require.NoError(t, err) + + require.Equal(t, uint64(0), m.MutationCount()) + require.Equal(t, uint64(0), atree.GetMapRootSlab(m).MutationCount()) +} + +func TestMutationCount_Map_GetDoesNotBump(t *testing.T) { + t.Parallel() + + typeInfo := testutils.NewSimpleTypeInfo(42) + storage := newTestPersistentStorage(t) + address := atree.Address{1, 2, 3, 4, 5, 6, 7, 8} + + m, err := atree.NewMap(storage, address, atree.NewDefaultDigesterBuilder(), typeInfo) + require.NoError(t, err) + + cmp := testutils.CompareValue + hip := testutils.GetHashInput + + for i := uint64(0); i < 5; i++ { + _, err := m.Set(cmp, hip, testutils.Uint64Value(i), testutils.Uint64Value(i*10)) + require.NoError(t, err) + } + for i := uint64(0); i < 5; i++ { + _, err := m.Get(cmp, hip, testutils.Uint64Value(i)) + require.NoError(t, err) + } + + require.Equal(t, uint64(0), m.MutationCount()) +} + +func TestMutationCount_Map_ElementOpsBelowSplitDoNotBump(t *testing.T) { + t.Parallel() + + typeInfo := testutils.NewSimpleTypeInfo(42) + storage := newTestPersistentStorage(t) + address := atree.Address{1, 2, 3, 4, 5, 6, 7, 8} + + m, err := atree.NewMap(storage, address, atree.NewDefaultDigesterBuilder(), typeInfo) + require.NoError(t, err) + + cmp := testutils.CompareValue + hip := testutils.GetHashInput + + for i := uint64(0); i < 5; i++ { + _, err := m.Set(cmp, hip, testutils.Uint64Value(i), testutils.Uint64Value(i)) + require.NoError(t, err) + } + for i := uint64(0); i < 5; i++ { + _, err := m.Set(cmp, hip, testutils.Uint64Value(i), testutils.Uint64Value(i*100)) + require.NoError(t, err) + } + for i := uint64(0); i < 2; i++ { + _, _, err := m.Remove(cmp, hip, testutils.Uint64Value(i)) + require.NoError(t, err) + } + + require.True(t, atree.GetMapRootSlab(m).IsData(), + "tree should still be a single data slab") + require.Equal(t, uint64(0), m.MutationCount()) +} + +func TestMutationCount_Map_PopIterateBumpsOldRoot(t *testing.T) { + t.Parallel() + + typeInfo := testutils.NewSimpleTypeInfo(42) + storage := newTestPersistentStorage(t) + address := atree.Address{1, 2, 3, 4, 5, 6, 7, 8} + + m, err := atree.NewMap(storage, address, atree.NewDefaultDigesterBuilder(), typeInfo) + require.NoError(t, err) + + cmp := testutils.CompareValue + hip := testutils.GetHashInput + + for i := uint64(0); i < 5; i++ { + _, err := m.Set(cmp, hip, testutils.Uint64Value(i), testutils.Uint64Value(i)) + require.NoError(t, err) + } + + oldRoot := atree.GetMapRootSlab(m) + require.Equal(t, uint64(0), oldRoot.MutationCount()) + + err = m.PopIterate(func(_, _ atree.Storable) {}) + require.NoError(t, err) + + require.NotSame(t, oldRoot, atree.GetMapRootSlab(m), + "PopIterate must replace the root with a fresh empty data slab") + require.Equal(t, uint64(1), oldRoot.MutationCount(), + "PopIterate must bump the orphaned old root exactly once") + require.Equal(t, uint64(0), m.MutationCount(), + "live MutationCount reads the fresh new root") +} + +func TestMutationCount_Map_SplitRootDoesNotBumpCounter(t *testing.T) { + atree.SetThreshold(256) + defer atree.SetThreshold(1024) + + typeInfo := testutils.NewSimpleTypeInfo(42) + storage := newTestPersistentStorage(t) + address := atree.Address{1, 2, 3, 4, 5, 6, 7, 8} + + m, err := atree.NewMap(storage, address, atree.NewDefaultDigesterBuilder(), typeInfo) + require.NoError(t, err) + + cmp := testutils.CompareValue + hip := testutils.GetHashInput + + oldRoot := atree.GetMapRootSlab(m) + require.Equal(t, uint64(0), oldRoot.MutationCount()) + + for i := uint64(0); i < 300; i++ { + _, err := m.Set(cmp, hip, testutils.Uint64Value(i), testutils.Uint64Value(i)) + require.NoError(t, err) + } + + require.False(t, atree.GetMapRootSlab(m).IsData(), + "prerequisite: a split must have occurred") + require.NotSame(t, oldRoot, atree.GetMapRootSlab(m)) + require.Equal(t, uint64(0), oldRoot.MutationCount(), + "splitRoot must not bump the demoted slab") +} + +func TestMutationCount_Map_RemoveUntilPromoteBumpsOldRoot(t *testing.T) { + atree.SetThreshold(256) + defer atree.SetThreshold(1024) + + typeInfo := testutils.NewSimpleTypeInfo(42) + storage := newTestPersistentStorage(t) + address := atree.Address{1, 2, 3, 4, 5, 6, 7, 8} + + m, err := atree.NewMap(storage, address, atree.NewDefaultDigesterBuilder(), typeInfo) + require.NoError(t, err) + + cmp := testutils.CompareValue + hip := testutils.GetHashInput + + for i := uint64(0); i < 300; i++ { + _, err := m.Set(cmp, hip, testutils.Uint64Value(i), testutils.Uint64Value(i)) + require.NoError(t, err) + } + require.False(t, atree.GetMapRootSlab(m).IsData()) + + oldMetaRoot := atree.GetMapRootSlab(m) + require.Equal(t, uint64(0), oldMetaRoot.MutationCount()) + + for i := uint64(0); i < 300; i++ { + if atree.GetMapRootSlab(m).IsData() { + break + } + _, _, err := m.Remove(cmp, hip, testutils.Uint64Value(i)) + require.NoError(t, err) + } + + require.True(t, atree.GetMapRootSlab(m).IsData(), + "map must have collapsed back to a single data slab via promote") + require.NotSame(t, oldMetaRoot, atree.GetMapRootSlab(m), + "root struct must have changed identity after promoteChildAsNewRoot") + require.Equal(t, uint64(1), oldMetaRoot.MutationCount(), + "promoteChildAsNewRoot must bump exactly once") + require.Equal(t, uint64(0), m.MutationCount(), + "live MutationCount reads the fresh promoted child") +} + +func TestMutationCount_Map_SiblingObservesPromoteWithStableValueID(t *testing.T) { + atree.SetThreshold(256) + defer atree.SetThreshold(1024) + + typeInfo := testutils.NewSimpleTypeInfo(42) + storage := newTestPersistentStorage(t) + address := atree.Address{1, 2, 3, 4, 5, 6, 7, 8} + + m, err := atree.NewMap(storage, address, atree.NewDefaultDigesterBuilder(), typeInfo) + require.NoError(t, err) + + cmp := testutils.CompareValue + hip := testutils.GetHashInput + + for i := uint64(0); i < 300; i++ { + _, err := m.Set(cmp, hip, testutils.Uint64Value(i), testutils.Uint64Value(i)) + require.NoError(t, err) + } + require.False(t, atree.GetMapRootSlab(m).IsData()) + + siblingRoot := atree.GetMapRootSlab(m) + cachedValueID := m.ValueID() + cachedMutationCount := m.MutationCount() + + for i := uint64(0); i < 300; i++ { + if atree.GetMapRootSlab(m).IsData() { + break + } + _, _, err := m.Remove(cmp, hip, testutils.Uint64Value(i)) + require.NoError(t, err) + } + require.True(t, atree.GetMapRootSlab(m).IsData()) + + require.Equal(t, cachedValueID, m.ValueID(), + "ValueID is preserved across promote — this is the gap the counter closes") + require.Greater(t, siblingRoot.MutationCount(), cachedMutationCount, + "the orphaned old root's counter must diverge from the cached value") +} + +func TestMutationCount_Map_NoFalseStaleForInitiatingWrapper(t *testing.T) { + atree.SetThreshold(256) + defer atree.SetThreshold(1024) + + typeInfo := testutils.NewSimpleTypeInfo(42) + storage := newTestPersistentStorage(t) + address := atree.Address{1, 2, 3, 4, 5, 6, 7, 8} + + m, err := atree.NewMap(storage, address, atree.NewDefaultDigesterBuilder(), typeInfo) + require.NoError(t, err) + + cmp := testutils.CompareValue + hip := testutils.GetHashInput + + primaryCachedCount := m.MutationCount() + require.Equal(t, uint64(0), primaryCachedCount) + + for i := uint64(0); i < 500; i++ { + _, err := m.Set(cmp, hip, testutils.Uint64Value(i), testutils.Uint64Value(i)) + require.NoError(t, err) + } + for i := uint64(0); i < 500; i++ { + _, _, err := m.Remove(cmp, hip, testutils.Uint64Value(i)) + require.NoError(t, err) + } + + require.Equal(t, primaryCachedCount, m.MutationCount(), + "the wrapper that initiated the operations must not see a false stale") +} + +func TestMutationCount_Map_OrphanedRootIsNotFurtherBumped(t *testing.T) { + typeInfo := testutils.NewSimpleTypeInfo(42) + storage := newTestPersistentStorage(t) + address := atree.Address{1, 2, 3, 4, 5, 6, 7, 8} + + m, err := atree.NewMap(storage, address, atree.NewDefaultDigesterBuilder(), typeInfo) + require.NoError(t, err) + + cmp := testutils.CompareValue + hip := testutils.GetHashInput + + for i := uint64(0); i < 5; i++ { + _, err := m.Set(cmp, hip, testutils.Uint64Value(i), testutils.Uint64Value(i)) + require.NoError(t, err) + } + + root0 := atree.GetMapRootSlab(m) + err = m.PopIterate(func(_, _ atree.Storable) {}) + require.NoError(t, err) + require.Equal(t, uint64(1), root0.MutationCount()) + + for i := uint64(0); i < 5; i++ { + _, err := m.Set(cmp, hip, testutils.Uint64Value(i), testutils.Uint64Value(i)) + require.NoError(t, err) + } + err = m.PopIterate(func(_, _ atree.Storable) {}) + require.NoError(t, err) + + require.Equal(t, uint64(1), root0.MutationCount(), + "a slab no longer referenced by *OrderedMap must not receive further bumps") +} From a7dd4c21f9335c4a73a25618447a39afd2a75e7e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Mon, 1 Jun 2026 14:24:42 -0700 Subject: [PATCH 02/16] share container state across *Array and *OrderedMap sibling instances Multiple Go-level *Array or *OrderedMap instances created over the same logical container held independent root slab pointers. A structural change (splitRoot, promoteChildAsNewRoot) performed through one instance left siblings observing stale roots. Hoist the mutable state onto a shared *arrayState / *orderedMapState that lives in a per-SlabStorage registry keyed by root SlabID. All instances over the same logical container share one state pointer, so structural changes propagate automatically. parentUpdater stays per-instance: a regular Get installs a real parent-notification callback while a readonly iterator installs a trap callback; two siblings can legitimately have different parentUpdaters and a mutation must fire only the originating instance's callback. SlabStorage gains a state registry. External implementations can embed *BaseStateRegistry to satisfy them with zero boilerplate. BasicSlabStorage and PersistentSlabStorage do exactly this. --- array.go | 204 ++++++++++--------- array_conversion.go | 4 +- array_data_slab.go | 21 +- array_metadata_slab.go | 21 +- array_serialization_verify.go | 6 +- array_sibling_consistency_test.go | 301 ++++++++++++++++++++++++++++ array_state.go | 71 +++++++ array_verify.go | 32 +-- map.go | 172 +++++++++------- map_data_slab.go | 22 +- map_metadata_slab.go | 22 +- map_serialization_verify.go | 2 +- map_sibling_consistency_test.go | 320 ++++++++++++++++++++++++++++++ map_state.go | 50 +++++ map_verify.go | 16 +- state_registry.go | 105 ++++++++++ storage.go | 40 ++-- 17 files changed, 1198 insertions(+), 211 deletions(-) create mode 100644 array_sibling_consistency_test.go create mode 100644 array_state.go create mode 100644 map_sibling_consistency_test.go create mode 100644 map_state.go create mode 100644 state_registry.go diff --git a/array.go b/array.go index 18dd7fe..8b50498 100644 --- a/array.go +++ b/array.go @@ -43,25 +43,26 @@ const ( // parent container's element size limit. Specifically, array with one segment // which fits in size limit can be inlined, while arrays with multiple segments // can't be inlined. +// +// Multiple *Array Go instances can exist for the same logical container; +// they all share the same *arrayState via the SlabStorage-backed registry, +// so structural mutations through any one of them are observed by all of them. +// See array_state.go for the rationale. type Array struct { Storage SlabStorage - root ArraySlab - // parentUpdater is a callback that notifies parent container when this array is modified. - // If this callback is nil, this array has no parent. Otherwise, this array has parent - // and this callback must be used when this array is changed by Append, Insert, Set, Remove, etc. - // - // parentUpdater acts like "parent pointer". It is not stored physically and is only in memory. - // It is setup when child array is returned from parent's Get. It is also setup when - // new child is added to parent through Set or Insert. + // state holds the mutable per-logical-container state + // (root pointer, mutableElementIndex). + // Shared across siblings so structural changes propagate. + state *arrayState + + // parentUpdater is the callback notifying the parent container + // when this *Array instance triggers a mutation. + // It is per-instance, not shared: + // a Get-loaded instance has a real parent updater, + // while a readonly-iterator-loaded instance has a trap callback. + // A mutation through one instance must fire only its own callback. parentUpdater parentUpdater - - // mutableElementIndex tracks index of mutable element, such as Array and OrderedMap. - // This is needed by mutable element to properly update itself through parentUpdater. - // WARNING: since mutableElementIndex is created lazily, we need to create mutableElementIndex - // if it is nil before adding/updating elements. Range, delete, and read are no-ops on nil Go map. - // TODO: maybe optimize by replacing map to get faster updates. - mutableElementIndex map[ValueID]uint64 } var _ Value = &Array{} @@ -94,9 +95,12 @@ func NewArray(storage SlabStorage, address Address, typeInfo TypeInfo) (*Array, return nil, err } + state := newArrayState(root) + storage.SetArrayState(sID, state) + return &Array{ Storage: storage, - root: root, + state: state, }, nil } @@ -105,6 +109,15 @@ func NewArrayWithRootID(storage SlabStorage, rootID SlabID) (*Array, error) { return nil, NewSlabIDErrorf("cannot create Array from undefined slab ID") } + // If another *Array instance for this container already exists, reuse + // its shared state so structural changes propagate. + if existing := storage.ArrayState(rootID); existing != nil { + return &Array{ + Storage: storage, + state: existing, + }, nil + } + root, err := getArraySlab(storage, rootID) if err != nil { // Don't need to wrap error as external error because err is already categorized by getArraySlab(). @@ -116,9 +129,12 @@ func NewArrayWithRootID(storage SlabStorage, rootID SlabID) (*Array, error) { return nil, NewNotValueError(rootID) } + state := newArrayState(root) + storage.SetArrayState(rootID, state) + return &Array{ Storage: storage, - root: root, + state: state, }, nil } @@ -277,9 +293,12 @@ func NewArrayFromBatchData(storage SlabStorage, address Address, typeInfo TypeIn return nil, err } + state := newArrayState(root) + storage.SetArrayState(root.SlabID(), state) + return &Array{ Storage: storage, - root: root, + state: state, }, nil } @@ -351,7 +370,7 @@ func nextLevelArraySlabs(storage SlabStorage, address Address, slabs []ArraySlab // Array operations (get, set, insert, remove, and pop iterate) func (a *Array) Get(i uint64) (Value, error) { - storable, err := a.root.Get(a.Storage, i) + storable, err := a.state.root.Get(a.Storage, i) if err != nil { // Don't need to wrap error as external error because err is already categorized by ArraySlab.Get(). return nil, err @@ -393,7 +412,7 @@ func (a *Array) Set(index uint64, value Value) (Storable, error) { unwrappedValue, _ := unwrapValue(value) newValue, ok := unwrappedValue.(mutableValueNotifier) if !ok || existingValueID != newValue.ValueID() { - delete(a.mutableElementIndex, existingValueID) + delete(a.state.mutableElementIndex, existingValueID) } } @@ -401,13 +420,13 @@ func (a *Array) Set(index uint64, value Value) (Storable, error) { } func (a *Array) set(index uint64, value Value) (Storable, error) { - existingStorable, err := a.root.Set(a.Storage, a.Address(), index, value) + existingStorable, err := a.state.root.Set(a.Storage, a.Address(), index, value) if err != nil { // Don't need to wrap error as external error because err is already categorized by ArraySlab.Set(). return nil, err } - if a.root.IsFull() { + if a.state.root.IsFull() { err = a.splitRoot() if err != nil { // Don't need to wrap error as external error because err is already categorized by Array.splitRoot(). @@ -415,8 +434,8 @@ func (a *Array) set(index uint64, value Value) (Storable, error) { } } - if !a.root.IsData() { - root := a.root.(*ArrayMetaDataSlab) + if !a.state.root.IsData() { + root := a.state.root.(*ArrayMetaDataSlab) if len(root.childrenHeaders) == 1 { err = a.promoteChildAsNewRoot(root.childrenHeaders[0].slabID) if err != nil { @@ -464,13 +483,13 @@ func (a *Array) Insert(index uint64, value Value) error { return NewArrayElementCannotExceedMaxElementCountError(maxArrayElementCount) } - err := a.root.Insert(a.Storage, a.Address(), index, value) + err := a.state.root.Insert(a.Storage, a.Address(), index, value) if err != nil { // Don't need to wrap error as external error because err is already categorized by ArraySlab.Insert(). return err } - if a.root.IsFull() { + if a.state.root.IsFull() { err = a.splitRoot() if err != nil { // Don't need to wrap error as external error because err is already categorized by Array.splitRoot(). @@ -526,22 +545,22 @@ func (a *Array) Remove(index uint64) (Storable, error) { // Delete removed element ValueID from mutableElementIndex if removedValueID != emptyValueID { - delete(a.mutableElementIndex, removedValueID) + delete(a.state.mutableElementIndex, removedValueID) } return removedStorable, nil } func (a *Array) remove(index uint64) (Storable, error) { - storable, err := a.root.Remove(a.Storage, index) + storable, err := a.state.root.Remove(a.Storage, index) if err != nil { // Don't need to wrap error as external error because err is already categorized by ArraySlab.Remove(). return nil, err } - if !a.root.IsData() { + if !a.state.root.IsData() { // Set root to its child slab if root has one child slab. - root := a.root.(*ArrayMetaDataSlab) + root := a.state.root.(*ArrayMetaDataSlab) if len(root.childrenHeaders) == 1 { err = a.promoteChildAsNewRoot(root.childrenHeaders[0].slabID) if err != nil { @@ -572,17 +591,17 @@ type ArrayPopIterationFunc func(Storable) // Each element is passed to ArrayPopIterationFunc callback before removal. func (a *Array) PopIterate(fn ArrayPopIterationFunc) error { - err := a.root.PopIterate(a.Storage, fn) + err := a.state.root.PopIterate(a.Storage, fn) if err != nil { // Don't need to wrap error as external error because err is already categorized by ArraySlab.PopIterate(). return err } - rootID := a.root.SlabID() + rootID := a.state.root.SlabID() - extraData := a.root.ExtraData() + extraData := a.state.root.ExtraData() - inlined := a.root.Inlined() + inlined := a.state.root.Inlined() size := uint32(arrayRootDataSlabPrefixSize) if inlined { @@ -590,7 +609,7 @@ func (a *Array) PopIterate(fn ArrayPopIterationFunc) error { } // Set root to empty data slab - a.root = &ArrayDataSlab{ + a.state.root = &ArrayDataSlab{ header: ArraySlabHeader{ slabID: rootID, size: size, @@ -601,7 +620,7 @@ func (a *Array) PopIterate(fn ArrayPopIterationFunc) error { // Save root slab if !a.Inlined() { - err = storeSlab(a.Storage, a.root) + err = storeSlab(a.Storage, a.state.root) if err != nil { return err } @@ -614,17 +633,17 @@ func (a *Array) PopIterate(fn ArrayPopIterationFunc) error { func (a *Array) splitRoot() error { - if a.root.IsData() { + if a.state.root.IsData() { // Adjust root data slab size before splitting - dataSlab := a.root.(*ArrayDataSlab) + dataSlab := a.state.root.(*ArrayDataSlab) dataSlab.header.size = dataSlab.header.size - arrayRootDataSlabPrefixSize + arrayDataSlabPrefixSize } // Get old root's extra data and reset it to nil in old root - extraData := a.root.RemoveExtraData() + extraData := a.state.root.RemoveExtraData() // Save root node id - rootID := a.root.SlabID() + rootID := a.state.root.SlabID() // Assign a new slab ID to old root before splitting it. sID, err := a.Storage.GenerateSlabID(a.Address()) @@ -635,7 +654,7 @@ func (a *Array) splitRoot() error { fmt.Sprintf("failed to generate slab ID for address 0x%x", a.Address())) } - oldRoot := a.root + oldRoot := a.state.root oldRoot.SetSlabID(sID) // Split old root @@ -662,7 +681,7 @@ func (a *Array) splitRoot() error { extraData: extraData, } - a.root = newRoot + a.state.root = newRoot err = storeSlab(a.Storage, left) if err != nil { @@ -674,7 +693,7 @@ func (a *Array) splitRoot() error { return err } - return storeSlab(a.Storage, a.root) + return storeSlab(a.Storage, a.state.root) } func (a *Array) promoteChildAsNewRoot(childID SlabID) error { @@ -691,17 +710,17 @@ func (a *Array) promoteChildAsNewRoot(childID SlabID) error { dataSlab.header.size = dataSlab.header.size - arrayDataSlabPrefixSize + arrayRootDataSlabPrefixSize } - extraData := a.root.RemoveExtraData() + extraData := a.state.root.RemoveExtraData() - rootID := a.root.SlabID() + rootID := a.state.root.SlabID() - a.root = child + a.state.root = child - a.root.SetSlabID(rootID) + a.state.root.SetSlabID(rootID) - a.root.SetExtraData(extraData) + a.state.root.SetExtraData(extraData) - err = storeSlab(a.Storage, a.root) + err = storeSlab(a.Storage, a.state.root) if err != nil { return err } @@ -722,12 +741,12 @@ func (a *Array) incrementIndexFrom(index uint64) error { // Although range loop over Go map is not deterministic, it is OK // to use here because this operation is free of side-effect and // leads to the same results independent of map order. - for id, i := range a.mutableElementIndex { + for id, i := range a.state.mutableElementIndex { if i >= index { - if a.mutableElementIndex[id]+1 >= a.Count() { + if a.state.mutableElementIndex[id]+1 >= a.Count() { return NewFatalError(fmt.Errorf("failed to increment index of ValueID %s in array %s: new index exceeds array count", id, a.ValueID())) } - a.mutableElementIndex[id]++ + a.state.mutableElementIndex[id]++ } } return nil @@ -738,19 +757,19 @@ func (a *Array) decrementIndexFrom(index uint64) error { // Although range loop over Go map is not deterministic, it is OK // to use here because this operation is free of side-effect and // leads to the same results independent of map order. - for id, i := range a.mutableElementIndex { + for id, i := range a.state.mutableElementIndex { if i > index { - if a.mutableElementIndex[id] <= 0 { + if a.state.mutableElementIndex[id] <= 0 { return NewFatalError(fmt.Errorf("failed to decrement index of ValueID %s in array %s: new index < 0", id, a.ValueID())) } - a.mutableElementIndex[id]-- + a.state.mutableElementIndex[id]-- } } return nil } func (a *Array) getIndexByValueID(id ValueID) (uint64, bool) { - index, exist := a.mutableElementIndex[id] + index, exist := a.state.mutableElementIndex[id] return index, exist } @@ -778,12 +797,12 @@ func (a *Array) setCallbackWithChild(i uint64, child Value, maxInlineSize uint32 vid := c.ValueID() // mutableElementIndex is lazily initialized. - if a.mutableElementIndex == nil { - a.mutableElementIndex = make(map[ValueID]uint64) + if a.state.mutableElementIndex == nil { + a.state.mutableElementIndex = make(map[ValueID]uint64) } // Index i will be updated with array operations, which affects element index. - a.mutableElementIndex[vid] = i + a.state.mutableElementIndex[vid] = i c.setParentUpdater(func() (found bool, err error) { @@ -801,7 +820,7 @@ func (a *Array) setCallbackWithChild(i uint64, child Value, maxInlineSize uint32 return false, nil } - storable, err := a.root.Get(a.Storage, adjustedIndex) + storable, err := a.state.root.Get(a.Storage, adjustedIndex) if err != nil { // Don't need to wrap error as external error because err is already categorized by ArraySlab.Get(). return false, err @@ -897,11 +916,11 @@ func (a *Array) notifyParentIfNeeded() error { } func (a *Array) Inlined() bool { - return a.root.Inlined() + return a.state.root.Inlined() } func (a *Array) Inlinable(maxInlineSize uint32) bool { - return a.root.Inlinable(maxInlineSize) + return a.state.root.Inlinable(maxInlineSize) } func (a *Array) hasParentUpdater() bool { @@ -909,11 +928,11 @@ func (a *Array) hasParentUpdater() bool { } func (a *Array) getMutableElementIndexCount() uint64 { - return uint64(len(a.mutableElementIndex)) + return uint64(len(a.state.mutableElementIndex)) } func (a *Array) getMutableElementIndex() map[ValueID]uint64 { - return a.mutableElementIndex + return a.state.mutableElementIndex } // Value operations @@ -923,14 +942,14 @@ func (a *Array) getMutableElementIndex() map[ValueID]uint64 { // - inlined data slab storable func (a *Array) Storable(_ SlabStorage, _ Address, maxInlineSize uint32) (Storable, error) { - inlined := a.root.Inlined() - inlinable := a.root.Inlinable(maxInlineSize) + inlined := a.state.root.Inlined() + inlinable := a.state.root.Inlinable(maxInlineSize) switch { case inlinable && inlined: // Root slab is inlinable and was inlined. // Return root slab as storable, no size adjustment and change to storage. - return a.root, nil + return a.state.root, nil case !inlinable && !inlined: // Root slab is not inlinable and was not inlined. @@ -941,19 +960,19 @@ func (a *Array) Storable(_ SlabStorage, _ Address, maxInlineSize uint32) (Storab // Root slab is inlinable and was NOT inlined. // Inline root data slab. - err := a.root.Inline(a.Storage) + err := a.state.root.Inline(a.Storage) if err != nil { return nil, err } - return a.root, nil + return a.state.root, nil case !inlinable && inlined: // Root slab is NOT inlinable and was previously inlined. // Uninline root slab. - err := a.root.Uninline(a.Storage) + err := a.state.root.Uninline(a.Storage) if err != nil { return nil, err } @@ -1014,7 +1033,7 @@ func (a *Array) ReadOnlyIteratorWithMutationCallback( return emptyReadOnlyArrayIterator, nil } - slab, err := firstArrayDataSlab(a.Storage, a.root) + slab, err := firstArrayDataSlab(a.Storage, a.state.root) if err != nil { // Don't need to wrap error as external error because err is already categorized by firstArrayDataSlab(). return nil, err @@ -1104,11 +1123,11 @@ func (a *Array) ReadOnlyRangeIteratorWithMutationCallback( var dataSlab *ArrayDataSlab index := startIndex - if a.root.IsData() { - dataSlab = a.root.(*ArrayDataSlab) + if a.state.root.IsData() { + dataSlab = a.state.root.(*ArrayDataSlab) } else if startIndex == 0 { var err error - dataSlab, err = firstArrayDataSlab(a.Storage, a.root) + dataSlab, err = firstArrayDataSlab(a.Storage, a.state.root) if err != nil { // Don't need to wrap error as external error because err is already categorized by firstArrayDataSlab(). return nil, err @@ -1118,7 +1137,7 @@ func (a *Array) ReadOnlyRangeIteratorWithMutationCallback( // getArrayDataSlabWithIndex returns data slab containing element at startIndex, // getArrayDataSlabWithIndex also returns adjusted index for this element at returned data slab. // Adjusted index must be used as index when creating ArrayIterator. - dataSlab, index, err = getArrayDataSlabWithIndex(a.Storage, a.root, startIndex) + dataSlab, index, err = getArrayDataSlabWithIndex(a.Storage, a.state.root, startIndex) if err != nil { // Don't need to wrap error as external error because err is already categorized by getArrayDataSlabWithIndex(). return nil, err @@ -1140,7 +1159,7 @@ func (a *Array) ReadOnlyRangeIteratorWithMutationCallback( // ReadOnlyLoadedValueIterator returns iterator to iterate loaded array elements. func (a *Array) ReadOnlyLoadedValueIterator() (*ArrayLoadedValueIterator, error) { - switch slab := a.root.(type) { + switch slab := a.state.root.(type) { case *ArrayDataSlab: // Create a data iterator from root slab. @@ -1303,40 +1322,40 @@ func (a *Array) IterateReadOnlyLoadedValues(fn ArrayIterationFunc) error { // Other operations func (a *Array) rootSlab() ArraySlab { - return a.root + return a.state.root } func (a *Array) Address() Address { - return a.root.SlabID().address + return a.state.root.SlabID().address } func (a *Array) Count() uint64 { - return uint64(a.root.Header().count) + return uint64(a.state.root.Header().count) } func (a *Array) SlabID() SlabID { - if a.root.Inlined() { + if a.state.root.Inlined() { return SlabIDUndefined } - return a.root.SlabID() + return a.state.root.SlabID() } func (a *Array) ValueID() ValueID { - return slabIDToValueID(a.root.SlabID()) + return slabIDToValueID(a.state.root.SlabID()) } func (a *Array) Type() TypeInfo { - if extraData := a.root.ExtraData(); extraData != nil { + if extraData := a.state.root.ExtraData(); extraData != nil { return extraData.TypeInfo } return nil } func (a *Array) SetType(typeInfo TypeInfo) error { - extraData := a.root.ExtraData() + extraData := a.state.root.ExtraData() extraData.TypeInfo = typeInfo - a.root.SetExtraData(extraData) + a.state.root.SetExtraData(extraData) if a.Inlined() { // Array is inlined. @@ -1348,7 +1367,7 @@ func (a *Array) SetType(typeInfo TypeInfo) error { // Array is standalone. // Store modified root slab in storage since typeInfo is part of extraData stored in root slab. - return storeSlab(a.Storage, a.root) + return storeSlab(a.Storage, a.state.root) } func (a *Array) String() string { @@ -1375,7 +1394,7 @@ func (a *Array) String() string { // CanCopyNonRefSimple returns true if the array can be copied // as a container with only non-reference and simple storables. func (a *Array) CanCopyNonRefSimple() bool { - return a.root.canCopyWithoutSlabID() + return a.state.root.canCopyWithoutSlabID() } // CopyNonRefSimple returns a copy of the array that only @@ -1383,7 +1402,7 @@ func (a *Array) CanCopyNonRefSimple() bool { // NOTE: Please call CanCopyNonRefSimple() to confirm the copy operation // is feasible for the array before calling CopyNonRefSimple(). func (a *Array) CopyNonRefSimple(address Address) (*Array, error) { - if !a.root.IsData() { + if !a.state.root.IsData() { return nil, newCopyArrayErrorf("can't copy multi-slab array") } @@ -1395,7 +1414,7 @@ func (a *Array) CopyNonRefSimple(address Address) (*Array, error) { fmt.Sprintf("failed to generate slab ID for address 0x%x", address)) } - copiedRootSlab, err := a.root.copyWithNewSlabID(newID) + copiedRootSlab, err := a.state.root.copyWithNewSlabID(newID) if err != nil { return nil, newCopyArrayError(err) } @@ -1405,13 +1424,16 @@ func (a *Array) CopyNonRefSimple(address Address) (*Array, error) { return nil, err } + state := newArrayState(copiedRootSlab) + a.Storage.SetArrayState(copiedRootSlab.SlabID(), state) + return &Array{ Storage: a.Storage, - root: copiedRootSlab, + state: state, }, nil } // IsWithinSingleSlab returns true if the array is stored in a single slab. func (a *Array) IsWithinSingleSlab() bool { - return a.root.IsData() + return a.state.root.IsData() } diff --git a/array_conversion.go b/array_conversion.go index 4c20aec..8b2bdab 100644 --- a/array_conversion.go +++ b/array_conversion.go @@ -37,7 +37,7 @@ func ByteArrayToByteSlice[T ByteStorable](array *Array) ([]byte, error) { } // Get first array data slab for traversal. - slab, err := firstArrayDataSlab(array.Storage, array.root) + slab, err := firstArrayDataSlab(array.Storage, array.state.root) if err != nil { // Don't need to wrap error as external error because err is already categorized by firstArrayDataSlab(). return nil, err @@ -152,7 +152,7 @@ func newArrayWithElements( } // Modify array root slab to include elements. - root := array.root.(*ArrayDataSlab) + root := array.state.root.(*ArrayDataSlab) root.elements = elements root.header.count = uint32(len(elements)) // This addition is safe from overflow because elementSize was already diff --git a/array_data_slab.go b/array_data_slab.go index 4be704e..797f868 100644 --- a/array_data_slab.go +++ b/array_data_slab.go @@ -599,9 +599,28 @@ func (a *ArrayDataSlab) StoredValue(storage SlabStorage) (Value, error) { if a.extraData == nil { return nil, NewNotValueError(a.SlabID()) } + + rootID := a.SlabID() + + // Share state with any existing *Array instance for this container. + // See array_state.go for rationale. + if existing := storage.ArrayState(rootID); existing != nil { + // Adopt the freshly-loaded slab into the shared state. + // atree wires the parentUpdater on the *atree.Array instance it just returned, + // so the state must point at this instance + // for parent notifications from any sibling to fire correctly. + existing.root = a + return &Array{ + Storage: storage, + state: existing, + }, nil + } + + state := newArrayState(a) + storage.SetArrayState(rootID, state) return &Array{ Storage: storage, - root: a, + state: state, }, nil } diff --git a/array_metadata_slab.go b/array_metadata_slab.go index eaa29c9..4ee1400 100644 --- a/array_metadata_slab.go +++ b/array_metadata_slab.go @@ -903,9 +903,28 @@ func (a *ArrayMetaDataSlab) StoredValue(storage SlabStorage) (Value, error) { if a.extraData == nil { return nil, NewNotValueError(a.SlabID()) } + + rootID := a.SlabID() + + // Share state with any existing *Array instance for this container. + // See array_state.go for rationale. + if existing := storage.ArrayState(rootID); existing != nil { + // Adopt the freshly-loaded slab into the shared state. + // atree wires the parentUpdater on the *atree.Array instance it just returned, + // so the state must point at this instance + // for parent notifications from any sibling to fire correctly. + existing.root = a + return &Array{ + Storage: storage, + state: existing, + }, nil + } + + state := newArrayState(a) + storage.SetArrayState(rootID, state) return &Array{ Storage: storage, - root: a, + state: state, }, nil } diff --git a/array_serialization_verify.go b/array_serialization_verify.go index 913da0a..1bfa639 100644 --- a/array_serialization_verify.go +++ b/array_serialization_verify.go @@ -51,7 +51,7 @@ func VerifyArraySerialization( decodeTypeInfo: decodeTypeInfo, compare: compare, } - return v.verifyArraySlab(a.root) + return v.verifyArraySlab(a.state.root) } type serializationVerifier struct { @@ -300,10 +300,10 @@ func (v *serializationVerifier) verifyValue(value Value) error { switch value := value.(type) { case *Array: - return v.verifyArraySlab(value.root) + return v.verifyArraySlab(value.state.root) case *OrderedMap: - return v.verifyMapSlab(value.root) + return v.verifyMapSlab(value.state.root) } return nil } diff --git a/array_sibling_consistency_test.go b/array_sibling_consistency_test.go new file mode 100644 index 0000000..e413365 --- /dev/null +++ b/array_sibling_consistency_test.go @@ -0,0 +1,301 @@ +/* + * Atree - Scalable Arrays and Ordered Maps + * + * Copyright Flow Foundation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package atree_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/onflow/atree" + testutils "github.com/onflow/atree/test_utils" +) + +// TestArraySiblingConsistencyAfterSplitRoot verifies that +// two *Array instances obtained for the same inlined inner array +// observe the same canonical state +// after a structural change (splitRoot) initiated through one of them. +// +// Without shared state, +// sibling[1] would retain a pointer to the pre-split root slab +// whose own SlabID has been reassigned by splitRoot — +// sibling[1].Count() would return a child-slab count +// rather than the canonical post-split count. +func TestArraySiblingConsistencyAfterSplitRoot(t *testing.T) { + + atree.SetThreshold(256) + defer atree.SetThreshold(1024) + + typeInfo := testutils.NewSimpleTypeInfo(42) + storage := newTestPersistentStorage(t) + address := atree.Address{1, 2, 3, 4, 5, 6, 7, 8} + + // Outer array holding one inner array. Both stored at the same address. + outer, err := atree.NewArray(storage, address, typeInfo) + require.NoError(t, err) + + inner, err := atree.NewArray(storage, address, typeInfo) + require.NoError(t, err) + require.NoError(t, inner.Append(testutils.NewUint64ValueFromInteger(0))) + + require.NoError(t, outer.Append(inner)) + + // Obtain two sibling *Array instances for the same inner container by + // calling outer.Get(0) twice. Pre-refactor these would have been two + // distinct *atree.Array Go objects with their own root pointers. + a, err := outer.Get(0) + require.NoError(t, err) + sibling1 := a.(*atree.Array) + + a, err = outer.Get(0) + require.NoError(t, err) + sibling2 := a.(*atree.Array) + + // Sanity: same ValueID, single-slab to start. + require.Equal(t, sibling1.ValueID(), sibling2.ValueID()) + require.True(t, sibling1.IsWithinSingleSlab(), + "initial inner array must be in a single slab") + + // Mutate through sibling1 enough to force a slab split. + // Threshold is 256 bytes; + // appending 200 uint64s definitely exceeds it. + const appendCount = 200 + for i := uint64(0); i < appendCount; i++ { + require.NoError(t, sibling1.Append(testutils.NewUint64ValueFromInteger(int(i)))) + } + + // Confirm splitRoot actually fired: + // root is now a MetaDataSlab, not a DataSlab. + // Without this assertion the test could pass trivially + // if atree's sizing kept the data in one slab + // (in which case there'd be no sibling-divergence opportunity to test). + require.False(t, sibling1.IsWithinSingleSlab(), + "splitRoot must have fired during the appends") + + // Both siblings must observe the post-split state. + require.Equal(t, uint64(1+appendCount), sibling1.Count(), + "sibling1 (mutated) must see appended count") + require.Equal(t, uint64(1+appendCount), sibling2.Count(), + "sibling2 (untouched) must see post-split count through shared state") + + // Mutate through sibling2; sibling1 must see it. + require.NoError(t, sibling2.Append(testutils.NewUint64ValueFromInteger(9999))) + require.Equal(t, uint64(2+appendCount), sibling1.Count(), + "sibling1 must observe sibling2's append through shared state") + require.Equal(t, uint64(2+appendCount), sibling2.Count()) + + // Spot-check element accessibility through both siblings. + last1, err := sibling1.Get(sibling1.Count() - 1) + require.NoError(t, err) + last2, err := sibling2.Get(sibling2.Count() - 1) + require.NoError(t, err) + require.Equal(t, testutils.NewUint64ValueFromInteger(9999), last1) + require.Equal(t, testutils.NewUint64ValueFromInteger(9999), last2) +} + +// TestArraySiblingConsistencyAfterPromoteRoot exercises the dual of the split case: +// enough removals to trigger promoteChildAsNewRoot. +// The previous Cadence-side staleness check +// (cached valueID vs live ValueID()) +// did NOT detect this case +// because promoteChildAsNewRoot keeps the root slab ID stable on `a`; +// sibling instances would retain a pointer to the orphaned old root struct. +// With shared state, both siblings observe the new root via state.root. +func TestArraySiblingConsistencyAfterPromoteRoot(t *testing.T) { + + atree.SetThreshold(256) + defer atree.SetThreshold(1024) + + typeInfo := testutils.NewSimpleTypeInfo(42) + storage := newTestPersistentStorage(t) + address := atree.Address{1, 2, 3, 4, 5, 6, 7, 8} + + outer, err := atree.NewArray(storage, address, typeInfo) + require.NoError(t, err) + + inner, err := atree.NewArray(storage, address, typeInfo) + require.NoError(t, err) + + // Populate enough elements to require multi-slab structure. + const initialCount = 200 + for i := uint64(0); i < initialCount; i++ { + require.NoError(t, inner.Append(testutils.NewUint64ValueFromInteger(int(i)))) + } + + require.NoError(t, outer.Append(inner)) + + // Two sibling instances. + a, err := outer.Get(0) + require.NoError(t, err) + sibling1 := a.(*atree.Array) + + a, err = outer.Get(0) + require.NoError(t, err) + sibling2 := a.(*atree.Array) + + // Sanity: the 200-element inner must be multi-slab after population. + // If this fails, the test setup didn't create a structure that can undergo promote. + require.False(t, sibling1.IsWithinSingleSlab(), + "populated inner array must span multiple slabs") + + // Remove enough elements through sibling1 to trigger root promotion + // (when meta slab shrinks to one child, + // atree promotes the child to be the new root). + for sibling1.Count() > 1 { + _, err := sibling1.Remove(sibling1.Count() - 1) + require.NoError(t, err) + } + + // Confirm promoteChildAsNewRoot actually fired: + // root is back to a DataSlab. + // The only path from MetaDataSlab back to DataSlab under removals is promote, + // so this assertion proves the structural op happened during the shrink. + require.True(t, sibling1.IsWithinSingleSlab(), + "promoteChildAsNewRoot must have fired during the removals") + + require.Equal(t, uint64(1), sibling1.Count()) + require.Equal(t, uint64(1), sibling2.Count(), + "sibling2 must observe post-promote count through shared state") + + // The remaining element must be readable through both siblings. + v1, err := sibling1.Get(0) + require.NoError(t, err) + v2, err := sibling2.Get(0) + require.NoError(t, err) + require.Equal(t, v1, v2) +} + +// TestArraySiblingConsistencyAcrossSplitAndPromote exercises a sequence +// that drives both structural operations through different siblings: +// - grow via sibling1 until splitRoot fires +// - shrink via sibling2 until promoteChildAsNewRoot fires +// +// Each transition must leave both siblings observing the same live canonical state. +// Without shared state, +// the second sibling's first touch after either transition would silently use a stale root. +func TestArraySiblingConsistencyAcrossSplitAndPromote(t *testing.T) { + + atree.SetThreshold(256) + defer atree.SetThreshold(1024) + + typeInfo := testutils.NewSimpleTypeInfo(42) + storage := newTestPersistentStorage(t) + address := atree.Address{1, 2, 3, 4, 5, 6, 7, 8} + + outer, err := atree.NewArray(storage, address, typeInfo) + require.NoError(t, err) + + inner, err := atree.NewArray(storage, address, typeInfo) + require.NoError(t, err) + require.NoError(t, inner.Append(testutils.NewUint64ValueFromInteger(0))) + + require.NoError(t, outer.Append(inner)) + + a, err := outer.Get(0) + require.NoError(t, err) + sibling1 := a.(*atree.Array) + + a, err = outer.Get(0) + require.NoError(t, err) + sibling2 := a.(*atree.Array) + + require.Equal(t, sibling1.ValueID(), sibling2.ValueID()) + require.True(t, sibling1.IsWithinSingleSlab(), + "initial inner array must be in a single slab") + + // Grow through sibling1 → splitRoot. + const grow = 200 + for i := uint64(0); i < grow; i++ { + require.NoError(t, sibling1.Append(testutils.NewUint64ValueFromInteger(int(i)))) + } + require.False(t, sibling1.IsWithinSingleSlab(), + "splitRoot must have fired during the grow phase") + require.Equal(t, uint64(1+grow), sibling2.Count(), + "sibling2 must see post-split count") + require.Equal(t, sibling1.ValueID(), sibling2.ValueID(), + "split must preserve matching ValueIDs through shared state") + + // Shrink through sibling2 → promoteChildAsNewRoot. + for sibling2.Count() > 1 { + _, err := sibling2.Remove(sibling2.Count() - 1) + require.NoError(t, err) + } + require.True(t, sibling1.IsWithinSingleSlab(), + "promoteChildAsNewRoot must have fired during the shrink phase") + require.Equal(t, uint64(1), sibling1.Count(), + "sibling1 must see post-promote count") + require.Equal(t, sibling1.ValueID(), sibling2.ValueID(), + "promote must preserve matching ValueIDs through shared state") + + // Final cross-check: append via sibling1, observe via sibling2. + require.NoError(t, sibling1.Append(testutils.NewUint64ValueFromInteger(42))) + require.Equal(t, uint64(2), sibling2.Count()) + v, err := sibling2.Get(1) + require.NoError(t, err) + require.Equal(t, testutils.NewUint64ValueFromInteger(42), v) +} + +// TestArraySiblingTestStructuralAssertionsAreMeaningful is a meta-test +// that validates the structural assertions used in the sibling tests above +// (`require.False(sibling.IsWithinSingleSlab(), ...)`) actually do real work. +// The risk we're guarding against: +// a future atree change +// that increases the slab threshold or shrinks element encodings +// could silently make the sibling tests' 200-element grow loop fit in a single slab — +// splitRoot never fires, +// no sibling divergence is possible, +// and the consistency assertions pass trivially. +// +// This test runs the same grow pattern under two thresholds: +// - 256 bytes (what the sibling tests use): split must fire +// - 16 KiB (large): split must NOT fire +// +// If either case behaves wrong, this test fails — +// and the sibling tests' structural assertions are confirmed to be a real guard. +func TestArraySiblingTestStructuralAssertionsAreMeaningful(t *testing.T) { + + typeInfo := testutils.NewSimpleTypeInfo(42) + address := atree.Address{1, 2, 3, 4, 5, 6, 7, 8} + + growAndCheck := func(threshold uint32) (singleSlabAtEnd bool) { + atree.SetThreshold(threshold) + defer atree.SetThreshold(1024) + + storage := newTestPersistentStorage(t) + arr, err := atree.NewArray(storage, address, typeInfo) + require.NoError(t, err) + + const grow = 200 + for i := uint64(0); i < grow; i++ { + require.NoError(t, arr.Append(testutils.NewUint64ValueFromInteger(int(i)))) + } + return arr.IsWithinSingleSlab() + } + + require.False(t, growAndCheck(256), + "with threshold=256 (what the sibling tests use), 200 uint64s "+ + "must overflow a single slab and force splitRoot — if this "+ + "passes (i.e. returns single-slab), the sibling tests' structural "+ + "assertions would pass trivially without a real split") + + require.True(t, growAndCheck(16*1024), + "with threshold=16KiB, 200 uint64s must fit in a single slab — "+ + "if this fails, our 'no-split' baseline doesn't hold and the "+ + "meta-test can't distinguish the two configurations") +} diff --git a/array_state.go b/array_state.go new file mode 100644 index 0000000..d9daf71 --- /dev/null +++ b/array_state.go @@ -0,0 +1,71 @@ +/* + * Atree - Scalable Arrays and Ordered Maps + * + * Copyright Flow Foundation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package atree + +// arrayState holds the mutable state shared by all *Array instances +// over the same logical container (same root SlabID / ValueID). +// +// Multiple *Array Go instances can exist for the same logical container +// when a caller obtains a handle through different paths +// (e.g., two Get calls on the same parent slot, +// or constructing via NewArrayWithRootID while another instance already exists). +// All such instances point at the same *arrayState, +// so structural changes +// (splitRoot, promoteChildAsNewRoot, root replacement on merge, etc.) +// performed through any one instance are observed by all the others. +// This is what eliminates the "stale sibling root" hazard +// that previously required workarounds at the Cadence layer. +// +// Lifetime: +// states are registered on the SlabStorage by SlabID. +// State entries live for the lifetime of the storage; +// see StateRegistry for why they must survive SlabStorage.Remove. +// Live *Array instances pointing to a state keep the state alive via the Go pointer; +// the storage registry holds a strong reference until the storage is dropped. +// +// parentUpdater is intentionally NOT on the shared state. +// It is set per *Array instance based on HOW that instance was obtained: +// a regular Get from a parent installs a real parent-notification callback; +// a readonly iterator installs a "trap callback" that errors on mutation. +// Two siblings over the same slab can therefore legitimately have different parentUpdaters, +// and a mutation must invoke only the originating instance's callback. +type arrayState struct { + root ArraySlab + + // mutableElementIndex tracks index of mutable element, such as Array and OrderedMap. + // This is needed by mutable element to properly update itself through parentUpdater. + // WARNING: since mutableElementIndex is created lazily, we need to create mutableElementIndex + // if it is nil before adding/updating elements. Range, delete, and read are no-ops on nil Go map. + // TODO: maybe optimize by replacing map to get faster updates. + mutableElementIndex map[ValueID]uint64 +} + +// ArrayState is an opaque handle to atree's shared per-container state for an *Array. +// SlabStorage implementations store and return these values +// via the ArrayState / SetArrayState methods on SlabStorage. +// +// External code should treat values of this type as opaque: +// they should not be constructed (its zero value is meaningless) +// or introspected (all fields are unexported). +// Pass them verbatim between SlabStorage method calls. +type ArrayState = arrayState + +func newArrayState(root ArraySlab) *arrayState { + return &arrayState{root: root} +} diff --git a/array_verify.go b/array_verify.go index 82af593..ecc23a5 100644 --- a/array_verify.go +++ b/array_verify.go @@ -71,16 +71,16 @@ func verifyArray( } // Verify array extra data - extraData := a.root.ExtraData() + extraData := a.state.root.ExtraData() if extraData == nil { - return NewFatalError(fmt.Errorf("root slab %d doesn't have extra data", a.root.SlabID())) + return NewFatalError(fmt.Errorf("root slab %d doesn't have extra data", a.state.root.SlabID())) } // Verify that extra data has correct type information if typeInfo != nil && !tic(extraData.TypeInfo, typeInfo) { return NewFatalError(fmt.Errorf( "root slab %d type information %v is wrong, want %v", - a.root.SlabID(), + a.state.root.SlabID(), extraData.TypeInfo, typeInfo, )) @@ -95,7 +95,7 @@ func verifyArray( } // Verify array slabs - computedCount, dataSlabIDs, nextDataSlabIDs, err := v.verifySlab(a.root, 0, nil, []SlabID{}, []SlabID{}, slabIDs) + computedCount, dataSlabIDs, nextDataSlabIDs, err := v.verifySlab(a.state.root, 0, nil, []SlabID{}, []SlabID{}, slabIDs) if err != nil { // Don't need to wrap error as external error because err is already categorized by verifySlab(). return err @@ -103,7 +103,7 @@ func verifyArray( // Verify array count if computedCount != uint32(a.Count()) { - return NewFatalError(fmt.Errorf("root slab %d count %d is wrong, want %d", a.root.SlabID(), a.Count(), computedCount)) + return NewFatalError(fmt.Errorf("root slab %d count %d is wrong, want %d", a.state.root.SlabID(), a.Count(), computedCount)) } // Verify next data slab ids @@ -404,7 +404,7 @@ func (v *arrayVerifier) verifyMetaDataSlab( // verifyArrayValueID verifies array ValueID is always the same as // root slab's SlabID indepedent of array's inlined status. func verifyArrayValueID(a *Array) error { - rootSlabID := a.root.Header().slabID + rootSlabID := a.state.root.Header().slabID vid := a.ValueID() @@ -444,7 +444,7 @@ func verifyArraySlabID(a *Array) error { return nil } - rootSlabID := a.root.Header().slabID + rootSlabID := a.state.root.Header().slabID if sid == SlabIDUndefined { return NewFatalError( @@ -469,20 +469,20 @@ func verifyNotInlinedValueStatusAndSize(v Value, maxInlineSize uint32) error { switch v := v.(type) { case *Array: // Verify not-inlined array's inlined status - if v.root.Inlined() { + if v.state.root.Inlined() { return NewFatalError( fmt.Errorf( "not-inlined array %s has inlined status", - v.root.Header().slabID)) + v.state.root.Header().slabID)) } // Verify not-inlined array size. - if v.root.IsData() { - inlinableSize := v.root.ByteSize() - arrayRootDataSlabPrefixSize + inlinedArrayDataSlabPrefixSize + if v.state.root.IsData() { + inlinableSize := v.state.root.ByteSize() - arrayRootDataSlabPrefixSize + inlinedArrayDataSlabPrefixSize if inlinableSize <= maxInlineSize { return NewFatalError( fmt.Errorf("not-inlined array root slab %s can be inlined, inlinable size %d <= max inline size %d", - v.root.Header().slabID, + v.state.root.Header().slabID, inlinableSize, maxInlineSize)) } @@ -494,16 +494,16 @@ func verifyNotInlinedValueStatusAndSize(v Value, maxInlineSize uint32) error { return NewFatalError( fmt.Errorf( "not-inlined map %s has inlined status", - v.root.Header().slabID)) + v.state.root.Header().slabID)) } // Verify not-inlined map size. - if v.root.IsData() { - inlinableSize := v.root.ByteSize() - mapRootDataSlabPrefixSize + inlinedMapDataSlabPrefixSize + if v.state.root.IsData() { + inlinableSize := v.state.root.ByteSize() - mapRootDataSlabPrefixSize + inlinedMapDataSlabPrefixSize if inlinableSize <= maxInlineSize { return NewFatalError( fmt.Errorf("not-inlined map root slab %s can be inlined, inlinable size %d <= max inline size %d", - v.root.Header().slabID, + v.state.root.Header().slabID, inlinableSize, maxInlineSize)) } diff --git a/map.go b/map.go index 2f2ec41..949e2fa 100644 --- a/map.go +++ b/map.go @@ -51,18 +51,21 @@ const ( // parent container's element size limit. Specifically, OrderedMap with one segment // which fits in size limit can be inlined, while OrderedMap with multiple segments // can't be inlined. +// +// Multiple *OrderedMap Go instances can exist for the same logical container; +// they share an *orderedMapState via the SlabStorage-backed registry, +// so structural mutations are observed by all siblings. +// See map_state.go for rationale. type OrderedMap struct { Storage SlabStorage - root MapSlab digesterBuilder DigesterBuilder - // parentUpdater is a callback that notifies parent container when this map is modified. - // If this callback is nil, this map has no parent. Otherwise, this map has parent - // and this callback must be used when this map is changed by Set and Remove. - // - // parentUpdater acts like "parent pointer". It is not stored physically and is only in memory. - // It is setup when child map is returned from parent's Get. It is also setup when - // new child is added to parent through Set or Insert. + // state holds the mutable per-logical-container state (root pointer). + // Shared across siblings. + state *orderedMapState + + // parentUpdater is per-instance, + // see Array.parentUpdater for rationale. parentUpdater parentUpdater } @@ -117,9 +120,12 @@ func NewMap(storage SlabStorage, address Address, digestBuilder DigesterBuilder, return nil, err } + state := newOrderedMapState(root) + storage.SetOrderedMapState(sID, state) + return &OrderedMap{ Storage: storage, - root: root, + state: state, digesterBuilder: digestBuilder, }, nil } @@ -129,6 +135,21 @@ func NewMapWithRootID(storage SlabStorage, rootID SlabID, digestBuilder Digester return nil, NewSlabIDErrorf("cannot create OrderedMap from undefined slab ID") } + // If another *OrderedMap instance for this container already exists, + // reuse its shared state so structural changes propagate. + if existing := storage.OrderedMapState(rootID); existing != nil { + // Re-seed the caller's digester from the existing extra data so + // hkeys match the canonical map. + if extraData := existing.root.ExtraData(); extraData != nil { + digestBuilder.SetSeed(extraData.Seed, typicalRandomConstant) + } + return &OrderedMap{ + Storage: storage, + state: existing, + digesterBuilder: digestBuilder, + }, nil + } + root, err := getMapSlab(storage, rootID) if err != nil { // Don't need to wrap error as external error because err is already categorized by getMapSlab(). @@ -142,9 +163,12 @@ func NewMapWithRootID(storage SlabStorage, rootID SlabID, digestBuilder Digester digestBuilder.SetSeed(extraData.Seed, typicalRandomConstant) + state := newOrderedMapState(root) + storage.SetOrderedMapState(rootID, state) + return &OrderedMap{ Storage: storage, - root: root, + state: state, digesterBuilder: digestBuilder, }, nil } @@ -406,9 +430,12 @@ func NewMapFromBatchData( return nil, err } + state := newOrderedMapState(root) + storage.SetOrderedMapState(root.SlabID(), state) + return &OrderedMap{ Storage: storage, - root: root, + state: state, digesterBuilder: digesterBuilder, }, nil } @@ -536,7 +563,7 @@ func (m *OrderedMap) get(comparator ValueComparator, hip HashInputProvider, key } // Don't need to wrap error as external error because err is already categorized by MapSlab.Get(). - return m.root.Get(m.Storage, keyDigest, level, hkey, comparator, key) + return m.state.root.Get(m.Storage, keyDigest, level, hkey, comparator, key) } func (m *OrderedMap) getElementAndNextKey(comparator ValueComparator, hip HashInputProvider, key Value) (Value, Value, Value, error) { @@ -556,7 +583,7 @@ func (m *OrderedMap) getElementAndNextKey(comparator ValueComparator, hip HashIn return nil, nil, nil, wrapErrorfAsExternalErrorIfNeeded(err, fmt.Sprintf("failed to get map key digest at level %d", level)) } - keyStorable, valueStorable, nextKeyStorable, err := m.root.getElementAndNextKey(m.Storage, keyDigest, level, hkey, comparator, key) + keyStorable, valueStorable, nextKeyStorable, err := m.state.root.getElementAndNextKey(m.Storage, keyDigest, level, hkey, comparator, key) if err != nil { return nil, nil, nil, err } @@ -607,7 +634,7 @@ func (m *OrderedMap) getNextKey(comparator ValueComparator, hip HashInputProvide return nil, wrapErrorfAsExternalErrorIfNeeded(err, fmt.Sprintf("failed to get map key digest at level %d", level)) } - _, _, nextKeyStorable, err := m.root.getElementAndNextKey(m.Storage, keyDigest, level, hkey, comparator, key) + _, _, nextKeyStorable, err := m.state.root.getElementAndNextKey(m.Storage, keyDigest, level, hkey, comparator, key) if err != nil { return nil, err } @@ -660,19 +687,19 @@ func (m *OrderedMap) set(comparator ValueComparator, hip HashInputProvider, key return nil, wrapErrorfAsExternalErrorIfNeeded(err, fmt.Sprintf("failed to get map key digest at level %d", level)) } - keyStorable, existingMapValueStorable, err := m.root.Set(m.Storage, m.digesterBuilder, keyDigest, level, hkey, comparator, hip, key, value) + keyStorable, existingMapValueStorable, err := m.state.root.Set(m.Storage, m.digesterBuilder, keyDigest, level, hkey, comparator, hip, key, value) if err != nil { // Don't need to wrap error as external error because err is already categorized by MapSlab.Set(). return nil, err } if existingMapValueStorable == nil { - m.root.ExtraData().incrementCount() + m.state.root.ExtraData().incrementCount() } - if !m.root.IsData() { + if !m.state.root.IsData() { // Set root to its child slab if root has one child slab. - root := m.root.(*MapMetaDataSlab) + root := m.state.root.(*MapMetaDataSlab) if len(root.childrenHeaders) == 1 { err := m.promoteChildAsNewRoot(root.childrenHeaders[0].slabID) if err != nil { @@ -682,7 +709,7 @@ func (m *OrderedMap) set(comparator ValueComparator, hip HashInputProvider, key } } - if m.root.IsFull() { + if m.state.root.IsFull() { err := m.splitRoot() if err != nil { // Don't need to wrap error as external error because err is already categorized by OrderedMap.splitRoot(). @@ -758,17 +785,17 @@ func (m *OrderedMap) remove(comparator ValueComparator, hip HashInputProvider, k return nil, nil, wrapErrorfAsExternalErrorIfNeeded(err, fmt.Sprintf("failed to create map key digest at level %d", level)) } - k, v, err := m.root.Remove(m.Storage, keyDigest, level, hkey, comparator, key) + k, v, err := m.state.root.Remove(m.Storage, keyDigest, level, hkey, comparator, key) if err != nil { // Don't need to wrap error as external error because err is already categorized by MapSlab.Remove(). return nil, nil, err } - m.root.ExtraData().decrementCount() + m.state.root.ExtraData().decrementCount() - if !m.root.IsData() { + if !m.state.root.IsData() { // Set root to its child slab if root has one child slab. - root := m.root.(*MapMetaDataSlab) + root := m.state.root.(*MapMetaDataSlab) if len(root.childrenHeaders) == 1 { err := m.promoteChildAsNewRoot(root.childrenHeaders[0].slabID) if err != nil { @@ -778,7 +805,7 @@ func (m *OrderedMap) remove(comparator ValueComparator, hip HashInputProvider, k } } - if m.root.IsFull() { + if m.state.root.IsFull() { err := m.splitRoot() if err != nil { // Don't need to wrap error as external error because err is already categorized by OrderedMap.splitRoot(). @@ -802,19 +829,19 @@ type MapPopIterationFunc func(Storable, Storable) // Each element is passed to MapPopIterationFunc callback before removal. func (m *OrderedMap) PopIterate(fn MapPopIterationFunc) error { - err := m.root.PopIterate(m.Storage, fn) + err := m.state.root.PopIterate(m.Storage, fn) if err != nil { // Don't need to wrap error as external error because err is already categorized by MapSlab.PopIterate(). return err } - rootID := m.root.SlabID() + rootID := m.state.root.SlabID() // Set map count to 0 in extraData - extraData := m.root.ExtraData() + extraData := m.state.root.ExtraData() extraData.Count = 0 - inlined := m.root.Inlined() + inlined := m.state.root.Inlined() prefixSize := uint32(mapRootDataSlabPrefixSize) if inlined { @@ -822,7 +849,7 @@ func (m *OrderedMap) PopIterate(fn MapPopIterationFunc) error { } // Set root to empty data slab - m.root = &MapDataSlab{ + m.state.root = &MapDataSlab{ header: MapSlabHeader{ slabID: rootID, size: prefixSize + hkeyElementsPrefixSize, @@ -834,7 +861,7 @@ func (m *OrderedMap) PopIterate(fn MapPopIterationFunc) error { if !m.Inlined() { // Save root slab - err = storeSlab(m.Storage, m.root) + err = storeSlab(m.Storage, m.state.root) if err != nil { return err } @@ -847,17 +874,17 @@ func (m *OrderedMap) PopIterate(fn MapPopIterationFunc) error { func (m *OrderedMap) splitRoot() error { - if m.root.IsData() { + if m.state.root.IsData() { // Adjust root data slab size before splitting - dataSlab := m.root.(*MapDataSlab) + dataSlab := m.state.root.(*MapDataSlab) dataSlab.header.size = dataSlab.header.size - mapRootDataSlabPrefixSize + mapDataSlabPrefixSize } // Get old root's extra data and reset it to nil in old root - extraData := m.root.RemoveExtraData() + extraData := m.state.root.RemoveExtraData() // Save root node id - rootID := m.root.SlabID() + rootID := m.state.root.SlabID() // Assign a new slab ID to old root before splitting it. sID, err := m.Storage.GenerateSlabID(m.Address()) @@ -866,7 +893,7 @@ func (m *OrderedMap) splitRoot() error { return wrapErrorfAsExternalErrorIfNeeded(err, fmt.Sprintf("failed to generate slab ID for address 0x%x", m.Address())) } - oldRoot := m.root + oldRoot := m.state.root oldRoot.SetSlabID(sID) // Split old root @@ -890,7 +917,7 @@ func (m *OrderedMap) splitRoot() error { extraData: extraData, } - m.root = newRoot + m.state.root = newRoot err = storeSlab(m.Storage, left) if err != nil { @@ -902,7 +929,7 @@ func (m *OrderedMap) splitRoot() error { return err } - return storeSlab(m.Storage, m.root) + return storeSlab(m.Storage, m.state.root) } func (m *OrderedMap) promoteChildAsNewRoot(childID SlabID) error { @@ -919,17 +946,17 @@ func (m *OrderedMap) promoteChildAsNewRoot(childID SlabID) error { dataSlab.header.size = dataSlab.header.size - mapDataSlabPrefixSize + mapRootDataSlabPrefixSize } - extraData := m.root.RemoveExtraData() + extraData := m.state.root.RemoveExtraData() - rootID := m.root.SlabID() + rootID := m.state.root.SlabID() - m.root = child + m.state.root = child - m.root.SetSlabID(rootID) + m.state.root.SetSlabID(rootID) - m.root.SetExtraData(extraData) + m.state.root.SetExtraData(extraData) - err = storeSlab(m.Storage, m.root) + err = storeSlab(m.Storage, m.state.root) if err != nil { return err } @@ -945,11 +972,11 @@ func (m *OrderedMap) promoteChildAsNewRoot(childID SlabID) error { // mutableValue operations (parent updater callback, mutableElementIndex, etc) func (m *OrderedMap) Inlined() bool { - return m.root.Inlined() + return m.state.root.Inlined() } func (m *OrderedMap) Inlinable(maxInlineSize uint32) bool { - return m.root.Inlinable(maxInlineSize) + return m.state.root.Inlinable(maxInlineSize) } func (m *OrderedMap) setParentUpdater(f parentUpdater) { @@ -1100,15 +1127,15 @@ func (m *OrderedMap) notifyParentIfNeeded() error { // - inlined data slab storable func (m *OrderedMap) Storable(_ SlabStorage, _ Address, maxInlineSize uint32) (Storable, error) { - inlined := m.root.Inlined() - inlinable := m.root.Inlinable(maxInlineSize) + inlined := m.state.root.Inlined() + inlinable := m.state.root.Inlinable(maxInlineSize) switch { case inlinable && inlined: // Root slab is inlinable and was inlined. // Return root slab as storable, no size adjustment and change to storage. - return m.root, nil + return m.state.root, nil case !inlinable && !inlined: // Root slab is not inlinable and was not inlined. @@ -1119,18 +1146,18 @@ func (m *OrderedMap) Storable(_ SlabStorage, _ Address, maxInlineSize uint32) (S // Root slab is inlinable and was NOT inlined. // Inline root data slab. - err := m.root.Inline(m.Storage) + err := m.state.root.Inline(m.Storage) if err != nil { return nil, err } - return m.root, nil + return m.state.root, nil case !inlinable && inlined: // Root slab is NOT inlinable and was inlined. // Uninline root slab. - err := m.root.Uninline(m.Storage) + err := m.state.root.Uninline(m.Storage) if err != nil { return nil, err } @@ -1157,7 +1184,7 @@ func (m *OrderedMap) Iterator(comparator ValueComparator, hip HashInputProvider) return emptyMutableMapIterator, nil } - keyStorable, err := firstKeyInMapSlab(m.Storage, m.root) + keyStorable, err := firstKeyInMapSlab(m.Storage, m.state.root) if err != nil { // Don't need to wrap error as external error because err is already categorized by firstKeyInMapSlab(). return nil, err @@ -1211,7 +1238,7 @@ func (m *OrderedMap) ReadOnlyIteratorWithMutationCallback( return emptyReadOnlyMapIterator, nil } - dataSlab, err := firstMapDataSlab(m.Storage, m.root) + dataSlab, err := firstMapDataSlab(m.Storage, m.state.root) if err != nil { // Don't need to wrap error as external error because err is already categorized by firstMapDataSlab(). return nil, err @@ -1239,7 +1266,7 @@ func (m *OrderedMap) ReadOnlyIteratorWithMutationCallback( // ReadOnlyLoadedValueIterator returns iterator to iterate loaded map elements. func (m *OrderedMap) ReadOnlyLoadedValueIterator() (*MapLoadedValueIterator, error) { - switch slab := m.root.(type) { + switch slab := m.state.root.(type) { case *MapDataSlab: // Create a data iterator from root slab. @@ -1446,29 +1473,29 @@ func (m *OrderedMap) IterateReadOnlyLoadedValues(fn MapEntryIterationFunc) error // Other operations func (m *OrderedMap) Seed() uint64 { - return m.root.ExtraData().Seed + return m.state.root.ExtraData().Seed } func (m *OrderedMap) Count() uint64 { - return m.root.ExtraData().Count + return m.state.root.ExtraData().Count } func (m *OrderedMap) Address() Address { - return m.root.SlabID().address + return m.state.root.SlabID().address } func (m *OrderedMap) Type() TypeInfo { - if extraData := m.root.ExtraData(); extraData != nil { + if extraData := m.state.root.ExtraData(); extraData != nil { return extraData.TypeInfo } return nil } func (m *OrderedMap) SetType(typeInfo TypeInfo) error { - extraData := m.root.ExtraData() + extraData := m.state.root.ExtraData() extraData.TypeInfo = typeInfo - m.root.SetExtraData(extraData) + m.state.root.SetExtraData(extraData) if m.Inlined() { // Map is inlined. @@ -1480,7 +1507,7 @@ func (m *OrderedMap) SetType(typeInfo TypeInfo) error { // Map is standalone. // Store modified root slab in storage since typeInfo is part of extraData stored in root slab. - return storeSlab(m.Storage, m.root) + return storeSlab(m.Storage, m.state.root) } func (m *OrderedMap) String() string { @@ -1512,7 +1539,7 @@ func (m *MapExtraData) decrementCount() { m.Count-- } func (m *OrderedMap) rootSlab() MapSlab { - return m.root + return m.state.root } func (m *OrderedMap) getDigesterBuilder() DigesterBuilder { @@ -1520,20 +1547,20 @@ func (m *OrderedMap) getDigesterBuilder() DigesterBuilder { } func (m *OrderedMap) SlabID() SlabID { - if m.root.Inlined() { + if m.state.root.Inlined() { return SlabIDUndefined } - return m.root.SlabID() + return m.state.root.SlabID() } func (m *OrderedMap) ValueID() ValueID { - return slabIDToValueID(m.root.SlabID()) + return slabIDToValueID(m.state.root.SlabID()) } // CanCopyNonRefSimple returns true if the map can be copied // as a container with only non-reference and simple storables. func (m *OrderedMap) CanCopyNonRefSimple() bool { - return m.root.canCopyWithoutSlabID() + return m.state.root.canCopyWithoutSlabID() } // CopyNonRefSimple returns a copy of the map that only @@ -1541,11 +1568,11 @@ func (m *OrderedMap) CanCopyNonRefSimple() bool { // NOTE: Please call CanCopyNonRefSimple() to confirm the copy operation // is feasible for the map before calling CopyNonRefSimple(). func (m *OrderedMap) CopyNonRefSimple(address Address, digestBuilder DigesterBuilder) (*OrderedMap, error) { - if !m.root.IsData() { + if !m.state.root.IsData() { return nil, newCopyMapErrorf("can't copy multi-slab map") } - seed := m.root.ExtraData().Seed + seed := m.state.root.ExtraData().Seed // Seed digester digestBuilder.SetSeed(seed, typicalRandomConstant) @@ -1557,7 +1584,7 @@ func (m *OrderedMap) CopyNonRefSimple(address Address, digestBuilder DigesterBui return nil, wrapErrorfAsExternalErrorIfNeeded(err, fmt.Sprintf("failed to generate slab ID for address 0x%x", address)) } - copiedRoot, err := m.root.copyWithNewSlabID(newID) + copiedRoot, err := m.state.root.copyWithNewSlabID(newID) if err != nil { return nil, newCopyMapError(err) } @@ -1567,14 +1594,17 @@ func (m *OrderedMap) CopyNonRefSimple(address Address, digestBuilder DigesterBui return nil, err } + state := newOrderedMapState(copiedRoot) + m.Storage.SetOrderedMapState(copiedRoot.SlabID(), state) + return &OrderedMap{ Storage: m.Storage, digesterBuilder: digestBuilder, - root: copiedRoot, + state: state, }, nil } // IsWithinSingleSlab returns true if the map is stored in a single slab. func (m *OrderedMap) IsWithinSingleSlab() bool { - return m.root.IsData() + return m.state.root.IsData() } diff --git a/map_data_slab.go b/map_data_slab.go index 3c68fb3..bcaff33 100644 --- a/map_data_slab.go +++ b/map_data_slab.go @@ -431,12 +431,30 @@ func (m *MapDataSlab) StoredValue(storage SlabStorage) (Value, error) { } digestBuilder := NewDefaultDigesterBuilder() - digestBuilder.SetSeed(m.extraData.Seed, typicalRandomConstant) + rootID := m.SlabID() + + // Share state with any existing *OrderedMap instance for this container. + // See map_state.go for rationale. + if existing := storage.OrderedMapState(rootID); existing != nil { + // Adopt the freshly-loaded slab into the shared state. + // atree wires the parentUpdater on the *atree.OrderedMap instance it just returned, + // so the state must point at this instance + // for parent notifications from any sibling to fire correctly. + existing.root = m + return &OrderedMap{ + Storage: storage, + state: existing, + digesterBuilder: digestBuilder, + }, nil + } + + state := newOrderedMapState(m) + storage.SetOrderedMapState(rootID, state) return &OrderedMap{ Storage: storage, - root: m, + state: state, digesterBuilder: digestBuilder, }, nil } diff --git a/map_metadata_slab.go b/map_metadata_slab.go index bdbecfe..5a353d6 100644 --- a/map_metadata_slab.go +++ b/map_metadata_slab.go @@ -753,12 +753,30 @@ func (m *MapMetaDataSlab) StoredValue(storage SlabStorage) (Value, error) { } digestBuilder := NewDefaultDigesterBuilder() - digestBuilder.SetSeed(m.extraData.Seed, typicalRandomConstant) + rootID := m.SlabID() + + // Share state with any existing *OrderedMap instance for this container. + // See map_state.go for rationale. + if existing := storage.OrderedMapState(rootID); existing != nil { + // Adopt the freshly-loaded slab into the shared state. + // atree wires the parentUpdater on the *atree.OrderedMap instance it just returned, + // so the state must point at this instance + // for parent notifications from any sibling to fire correctly. + existing.root = m + return &OrderedMap{ + Storage: storage, + state: existing, + digesterBuilder: digestBuilder, + }, nil + } + + state := newOrderedMapState(m) + storage.SetOrderedMapState(rootID, state) return &OrderedMap{ Storage: storage, - root: m, + state: state, digesterBuilder: digestBuilder, }, nil } diff --git a/map_serialization_verify.go b/map_serialization_verify.go index ff7117a..8df74b6 100644 --- a/map_serialization_verify.go +++ b/map_serialization_verify.go @@ -51,7 +51,7 @@ func VerifyMapSerialization( decodeTypeInfo: decodeTypeInfo, compare: compare, } - return v.verifyMapSlab(m.root) + return v.verifyMapSlab(m.state.root) } func (v *serializationVerifier) verifyMapSlab(slab MapSlab) error { diff --git a/map_sibling_consistency_test.go b/map_sibling_consistency_test.go new file mode 100644 index 0000000..f59d6f3 --- /dev/null +++ b/map_sibling_consistency_test.go @@ -0,0 +1,320 @@ +/* + * Atree - Scalable Arrays and Ordered Maps + * + * Copyright Flow Foundation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package atree_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/onflow/atree" + testutils "github.com/onflow/atree/test_utils" +) + +// TestMapSiblingConsistencyAfterSplitRoot is the OrderedMap counterpart +// to TestArraySiblingConsistencyAfterSplitRoot: +// two *OrderedMap instances obtained for the same inner map +// must observe the same canonical state +// after a structural change initiated through one of them. +func TestMapSiblingConsistencyAfterSplitRoot(t *testing.T) { + + atree.SetThreshold(256) + defer atree.SetThreshold(1024) + + typeInfo := testutils.NewSimpleTypeInfo(42) + storage := newTestPersistentStorage(t) + address := atree.Address{1, 2, 3, 4, 5, 6, 7, 8} + + outer, err := atree.NewMap(storage, address, atree.NewDefaultDigesterBuilder(), typeInfo) + require.NoError(t, err) + + inner, err := atree.NewMap(storage, address, atree.NewDefaultDigesterBuilder(), typeInfo) + require.NoError(t, err) + + k0 := testutils.NewUint64ValueFromInteger(0) + v0 := testutils.NewUint64ValueFromInteger(0) + prev, err := inner.Set(testutils.CompareValue, testutils.GetHashInput, k0, v0) + require.NoError(t, err) + require.Nil(t, prev) + + prev, err = outer.Set(testutils.CompareValue, testutils.GetHashInput, testutils.NewUint64ValueFromInteger(0), inner) + require.NoError(t, err) + require.Nil(t, prev) + + // Two sibling instances over the same inner map. + innerVal, err := outer.Get(testutils.CompareValue, testutils.GetHashInput, testutils.NewUint64ValueFromInteger(0)) + require.NoError(t, err) + sibling1 := innerVal.(*atree.OrderedMap) + + innerVal, err = outer.Get(testutils.CompareValue, testutils.GetHashInput, testutils.NewUint64ValueFromInteger(0)) + require.NoError(t, err) + sibling2 := innerVal.(*atree.OrderedMap) + + require.Equal(t, sibling1.ValueID(), sibling2.ValueID()) + require.True(t, sibling1.IsWithinSingleSlab(), + "initial inner map must be in a single slab") + + // Insert through sibling1 enough to force a split. + const insertCount = 200 + for i := uint64(1); i <= insertCount; i++ { + k := testutils.NewUint64ValueFromInteger(int(i)) + v := testutils.NewUint64ValueFromInteger(int(i)) + prev, err := sibling1.Set(testutils.CompareValue, testutils.GetHashInput, k, v) + require.NoError(t, err) + require.Nil(t, prev) + } + + // Confirm splitRoot actually fired. + require.False(t, sibling1.IsWithinSingleSlab(), + "splitRoot must have fired during the inserts") + + require.Equal(t, uint64(1+insertCount), sibling1.Count()) + require.Equal(t, uint64(1+insertCount), sibling2.Count(), + "sibling2 must observe post-split count through shared state") + + // Mutate through sibling2; sibling1 must see it. + prev, err = sibling2.Set(testutils.CompareValue, testutils.GetHashInput, + testutils.NewUint64ValueFromInteger(99999), + testutils.NewUint64ValueFromInteger(99999)) + require.NoError(t, err) + require.Nil(t, prev) + require.Equal(t, uint64(2+insertCount), sibling1.Count(), + "sibling1 must observe sibling2's insert") +} + +// TestMapSiblingConsistencyAfterPromoteRoot exercises the dual of +// TestMapSiblingConsistencyAfterSplitRoot for OrderedMap: +// enough removals through one sibling to trigger promoteChildAsNewRoot. +// The previous Cadence-side staleness check +// (cached valueID vs live ValueID()) +// would NOT detect this case for either Array or Map +// because promoteChildAsNewRoot keeps the root's slab ID stable; +// shared state is what makes the post-promote count visible to the other sibling. +func TestMapSiblingConsistencyAfterPromoteRoot(t *testing.T) { + + atree.SetThreshold(256) + defer atree.SetThreshold(1024) + + typeInfo := testutils.NewSimpleTypeInfo(42) + storage := newTestPersistentStorage(t) + address := atree.Address{1, 2, 3, 4, 5, 6, 7, 8} + + outer, err := atree.NewMap(storage, address, atree.NewDefaultDigesterBuilder(), typeInfo) + require.NoError(t, err) + + inner, err := atree.NewMap(storage, address, atree.NewDefaultDigesterBuilder(), typeInfo) + require.NoError(t, err) + + // Populate enough entries to require multi-slab structure + // (will force the root to become a meta slab with multiple children). + const initialCount = 200 + for i := uint64(0); i < initialCount; i++ { + k := testutils.NewUint64ValueFromInteger(int(i)) + v := testutils.NewUint64ValueFromInteger(int(i)) + prev, err := inner.Set(testutils.CompareValue, testutils.GetHashInput, k, v) + require.NoError(t, err) + require.Nil(t, prev) + } + + prev, err := outer.Set(testutils.CompareValue, testutils.GetHashInput, + testutils.NewUint64ValueFromInteger(0), inner) + require.NoError(t, err) + require.Nil(t, prev) + + // Two sibling instances. + innerVal, err := outer.Get(testutils.CompareValue, testutils.GetHashInput, testutils.NewUint64ValueFromInteger(0)) + require.NoError(t, err) + sibling1 := innerVal.(*atree.OrderedMap) + + innerVal, err = outer.Get(testutils.CompareValue, testutils.GetHashInput, testutils.NewUint64ValueFromInteger(0)) + require.NoError(t, err) + sibling2 := innerVal.(*atree.OrderedMap) + + require.Equal(t, sibling1.ValueID(), sibling2.ValueID()) + require.False(t, sibling1.IsWithinSingleSlab(), + "populated inner map must span multiple slabs") + + // Remove through sibling1 until the meta slab is forced to promote. + for i := uint64(1); i < initialCount; i++ { + k := testutils.NewUint64ValueFromInteger(int(i)) + _, _, err := sibling1.Remove(testutils.CompareValue, testutils.GetHashInput, k) + require.NoError(t, err) + } + + // Confirm promoteChildAsNewRoot actually fired. + require.True(t, sibling1.IsWithinSingleSlab(), + "promoteChildAsNewRoot must have fired during the removals") + + require.Equal(t, uint64(1), sibling1.Count()) + require.Equal(t, uint64(1), sibling2.Count(), + "sibling2 must observe post-promote count through shared state") + + // The remaining entry must be readable through both siblings. + k0 := testutils.NewUint64ValueFromInteger(0) + v1, err := sibling1.Get(testutils.CompareValue, testutils.GetHashInput, k0) + require.NoError(t, err) + v2, err := sibling2.Get(testutils.CompareValue, testutils.GetHashInput, k0) + require.NoError(t, err) + require.Equal(t, v1, v2) + + // Insert through sibling2; sibling1 must see it + // (and the live ValueID must match — + // both siblings share the same state.root). + prev, err = sibling2.Set(testutils.CompareValue, testutils.GetHashInput, + testutils.NewUint64ValueFromInteger(99999), + testutils.NewUint64ValueFromInteger(99999)) + require.NoError(t, err) + require.Nil(t, prev) + require.Equal(t, uint64(2), sibling1.Count(), + "sibling1 must observe sibling2's post-promote insert") + require.Equal(t, sibling1.ValueID(), sibling2.ValueID(), + "siblings must keep matching ValueIDs after every structural op") +} + +// TestMapSiblingConsistencyAcrossSplitAndPromote is the OrderedMap +// counterpart to TestArraySiblingConsistencyAcrossSplitAndPromote: +// a sequence that drives both structural operations through different siblings. +// - grow via sibling1 until splitRoot fires +// - shrink via sibling2 until promoteChildAsNewRoot fires +// +// Each transition must leave both siblings observing the same live canonical state. +func TestMapSiblingConsistencyAcrossSplitAndPromote(t *testing.T) { + + atree.SetThreshold(256) + defer atree.SetThreshold(1024) + + typeInfo := testutils.NewSimpleTypeInfo(42) + storage := newTestPersistentStorage(t) + address := atree.Address{1, 2, 3, 4, 5, 6, 7, 8} + + outer, err := atree.NewMap(storage, address, atree.NewDefaultDigesterBuilder(), typeInfo) + require.NoError(t, err) + + inner, err := atree.NewMap(storage, address, atree.NewDefaultDigesterBuilder(), typeInfo) + require.NoError(t, err) + + k0 := testutils.NewUint64ValueFromInteger(0) + v0 := testutils.NewUint64ValueFromInteger(0) + prev, err := inner.Set(testutils.CompareValue, testutils.GetHashInput, k0, v0) + require.NoError(t, err) + require.Nil(t, prev) + + prev, err = outer.Set(testutils.CompareValue, testutils.GetHashInput, + testutils.NewUint64ValueFromInteger(0), inner) + require.NoError(t, err) + require.Nil(t, prev) + + innerVal, err := outer.Get(testutils.CompareValue, testutils.GetHashInput, testutils.NewUint64ValueFromInteger(0)) + require.NoError(t, err) + sibling1 := innerVal.(*atree.OrderedMap) + + innerVal, err = outer.Get(testutils.CompareValue, testutils.GetHashInput, testutils.NewUint64ValueFromInteger(0)) + require.NoError(t, err) + sibling2 := innerVal.(*atree.OrderedMap) + + require.Equal(t, sibling1.ValueID(), sibling2.ValueID()) + require.True(t, sibling1.IsWithinSingleSlab(), + "initial inner map must be in a single slab") + + // Grow through sibling1 → splitRoot. + const grow = 200 + for i := uint64(1); i <= grow; i++ { + k := testutils.NewUint64ValueFromInteger(int(i)) + v := testutils.NewUint64ValueFromInteger(int(i)) + prev, err := sibling1.Set(testutils.CompareValue, testutils.GetHashInput, k, v) + require.NoError(t, err) + require.Nil(t, prev) + } + require.False(t, sibling1.IsWithinSingleSlab(), + "splitRoot must have fired during the grow phase") + require.Equal(t, uint64(1+grow), sibling2.Count(), + "sibling2 must see post-split count") + require.Equal(t, sibling1.ValueID(), sibling2.ValueID(), + "split must preserve matching ValueIDs through shared state") + + // Shrink through sibling2 → promoteChildAsNewRoot. + for i := uint64(1); i <= grow; i++ { + k := testutils.NewUint64ValueFromInteger(int(i)) + _, _, err := sibling2.Remove(testutils.CompareValue, testutils.GetHashInput, k) + require.NoError(t, err) + } + require.True(t, sibling1.IsWithinSingleSlab(), + "promoteChildAsNewRoot must have fired during the shrink phase") + require.Equal(t, uint64(1), sibling1.Count(), + "sibling1 must see post-promote count") + require.Equal(t, sibling1.ValueID(), sibling2.ValueID(), + "promote must preserve matching ValueIDs through shared state") + + // Final cross-check: insert via sibling1, observe via sibling2. + k42 := testutils.NewUint64ValueFromInteger(42) + v42 := testutils.NewUint64ValueFromInteger(42) + prev, err = sibling1.Set(testutils.CompareValue, testutils.GetHashInput, k42, v42) + require.NoError(t, err) + require.Nil(t, prev) + require.Equal(t, uint64(2), sibling2.Count()) + v, err := sibling2.Get(testutils.CompareValue, testutils.GetHashInput, k42) + require.NoError(t, err) + require.Equal(t, v42, v) +} + +// TestMapSiblingTestStructuralAssertionsAreMeaningful is the OrderedMap +// counterpart to TestArraySiblingTestStructuralAssertionsAreMeaningful. +// It validates that the structural assertions used in the map sibling tests +// actually distinguish "split fired" from "split skipped" — +// if a future atree change made the test setup fit in a single slab, +// the sibling-consistency assertions would pass trivially. +// +// This test runs the same insert pattern under two thresholds: +// - 256 bytes (what the sibling tests use): split must fire +// - 16 KiB (large): split must NOT fire +func TestMapSiblingTestStructuralAssertionsAreMeaningful(t *testing.T) { + + typeInfo := testutils.NewSimpleTypeInfo(42) + address := atree.Address{1, 2, 3, 4, 5, 6, 7, 8} + + growAndCheck := func(threshold uint32) (singleSlabAtEnd bool) { + atree.SetThreshold(threshold) + defer atree.SetThreshold(1024) + + storage := newTestPersistentStorage(t) + m, err := atree.NewMap(storage, address, atree.NewDefaultDigesterBuilder(), typeInfo) + require.NoError(t, err) + + const grow = 200 + for i := uint64(0); i < grow; i++ { + k := testutils.NewUint64ValueFromInteger(int(i)) + v := testutils.NewUint64ValueFromInteger(int(i)) + prev, err := m.Set(testutils.CompareValue, testutils.GetHashInput, k, v) + require.NoError(t, err) + require.Nil(t, prev) + } + return m.IsWithinSingleSlab() + } + + require.False(t, growAndCheck(256), + "with threshold=256 (what the sibling tests use), 200 entries "+ + "must overflow a single slab and force splitRoot — if this "+ + "passes (single-slab), the sibling tests' structural assertions "+ + "would pass trivially without a real split") + + require.True(t, growAndCheck(16*1024), + "with threshold=16KiB, 200 entries must fit in a single slab — "+ + "if this fails, our 'no-split' baseline doesn't hold and the "+ + "meta-test can't distinguish the two configurations") +} diff --git a/map_state.go b/map_state.go new file mode 100644 index 0000000..1dca4cd --- /dev/null +++ b/map_state.go @@ -0,0 +1,50 @@ +/* + * Atree - Scalable Arrays and Ordered Maps + * + * Copyright Flow Foundation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package atree + +// orderedMapState holds the mutable state shared by all *OrderedMap instances +// over the same logical container. +// See arrayState for the general rationale; +// this is the symmetric type for maps. +// +// parentUpdater is intentionally NOT on the shared state +// (see arrayState for the same reasoning — set per-instance, may be a trap callback). +// +// digesterBuilder is per-instance +// because it is provided by the caller of NewMapWithRootID; +// siblings constructed with different builders (rare but legal) keep their own. +// The seed they configure is encoded in the map's ExtraData, +// so siblings using different builders still compute the same hkeys. +type orderedMapState struct { + root MapSlab +} + +// OrderedMapState is an opaque handle to atree's shared per-container state for an *OrderedMap. +// SlabStorage implementations store and return these values +// via the OrderedMapState / SetOrderedMapState methods on SlabStorage. +// +// External code should treat values of this type as opaque: +// they should not be constructed (its zero value is meaningless) +// or introspected (all fields are unexported). +// Pass them verbatim between SlabStorage method calls. +type OrderedMapState = orderedMapState + +func newOrderedMapState(root MapSlab) *orderedMapState { + return &orderedMapState{root: root} +} diff --git a/map_verify.go b/map_verify.go index 6bd9b97..a843166 100644 --- a/map_verify.go +++ b/map_verify.go @@ -65,9 +65,9 @@ func verifyMap( } // Verify map extra data - extraData := m.root.ExtraData() + extraData := m.state.root.ExtraData() if extraData == nil { - return NewFatalError(fmt.Errorf("root slab %d doesn't have extra data", m.root.SlabID())) + return NewFatalError(fmt.Errorf("root slab %d doesn't have extra data", m.state.root.SlabID())) } // Verify that extra data has correct type information @@ -75,7 +75,7 @@ func verifyMap( return NewFatalError( fmt.Errorf( "root slab %d type information %v, want %v", - m.root.SlabID(), + m.state.root.SlabID(), extraData.TypeInfo, typeInfo, )) @@ -83,7 +83,7 @@ func verifyMap( // Verify that extra data has seed if extraData.Seed == 0 { - return NewFatalError(fmt.Errorf("root slab %d seed is uninitialized", m.root.SlabID())) + return NewFatalError(fmt.Errorf("root slab %d seed is uninitialized", m.state.root.SlabID())) } v := &mapVerifier{ @@ -96,7 +96,7 @@ func verifyMap( } computedCount, dataSlabIDs, nextDataSlabIDs, firstKeys, err := v.verifySlab( - m.root, 0, nil, []SlabID{}, []SlabID{}, []Digest{}, slabIDs) + m.state.root, 0, nil, []SlabID{}, []SlabID{}, []Digest{}, slabIDs) if err != nil { // Don't need to wrap error as external error because err is already categorized by verifySlab(). return err @@ -107,7 +107,7 @@ func verifyMap( return NewFatalError( fmt.Errorf( "root slab %d count %d is wrong, want %d", - m.root.SlabID(), + m.state.root.SlabID(), extraData.Count, computedCount, )) @@ -728,7 +728,7 @@ func verifyValue(value Value, address Address, typeInfo TypeInfo, tic TypeInfoCo // verifyMapValueID verifies map ValueID is always the same as // root slab's SlabID indepedent of map's inlined status. func verifyMapValueID(m *OrderedMap) error { - rootSlabID := m.root.Header().slabID + rootSlabID := m.state.root.Header().slabID vid := m.ValueID() @@ -768,7 +768,7 @@ func verifyMapSlabID(m *OrderedMap) error { return nil } - rootSlabID := m.root.Header().slabID + rootSlabID := m.state.root.Header().slabID if sid == SlabIDUndefined { return NewFatalError( diff --git a/state_registry.go b/state_registry.go new file mode 100644 index 0000000..2685948 --- /dev/null +++ b/state_registry.go @@ -0,0 +1,105 @@ +/* + * Atree - Scalable Arrays and Ordered Maps + * + * Copyright Flow Foundation + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package atree + +// StateRegistry is the interface for managing shared state associated with slabs. +// Implementations may embed *BaseStateRegistry to inherit working defaults; +// atree's BasicSlabStorage and PersistentSlabStorage do so. +// Remove(SlabID) is also expected to clear any state registered under that slab ID. +type StateRegistry interface { + ArrayState(rootID SlabID) *ArrayState + SetArrayState(rootID SlabID, state *ArrayState) + OrderedMapState(rootID SlabID) *OrderedMapState + SetOrderedMapState(rootID SlabID, state *OrderedMapState) +} + +// BaseStateRegistry is an embeddable helper +// providing default implementations of the four state-registry methods on SlabStorage +// (ArrayState, SetArrayState, OrderedMapState, SetOrderedMapState). +// +// SlabStorage implementations should embed *BaseStateRegistry +// to get a working registry without writing the boilerplate themselves: +// +// type MyStorage struct { +// *atree.BaseStateRegistry +// // other fields ... +// } +// +// func NewMyStorage() *MyStorage { +// return &MyStorage{BaseStateRegistry: atree.NewBaseStateRegistry()} +// } +// +// Implementations must NOT eagerly drop state from their Remove(SlabID) method; +// see the note on StateRegistry for why. +// atree's own BasicSlabStorage and PersistentSlabStorage do not clear state on Remove, deliberately. +// Callers that want explicit cleanup +// (e.g. at the end of a long-lived storage's lifecycle) +// can call RemoveStateForSlab directly. +type BaseStateRegistry struct { + arrayStates map[SlabID]*ArrayState + orderedMapStates map[SlabID]*OrderedMapState +} + +var _ StateRegistry = &BaseStateRegistry{} + +// NewBaseStateRegistry returns an empty registry. +// Registry maps are lazily initialized on first use, +// so a zero-valued *BaseStateRegistry also works; +// this constructor is provided for explicitness. +func NewBaseStateRegistry() *BaseStateRegistry { + return &BaseStateRegistry{} +} + +// ArrayState returns the registered *ArrayState for the given root slab ID, +// or nil if none. +func (r *BaseStateRegistry) ArrayState(rootID SlabID) *ArrayState { + return r.arrayStates[rootID] +} + +// SetArrayState registers state under the given root slab ID. +// Lazily initializes the underlying map. +func (r *BaseStateRegistry) SetArrayState(rootID SlabID, state *ArrayState) { + if r.arrayStates == nil { + r.arrayStates = make(map[SlabID]*ArrayState) + } + r.arrayStates[rootID] = state +} + +// OrderedMapState returns the registered *OrderedMapState for the given root slab ID, +// or nil if none. +func (r *BaseStateRegistry) OrderedMapState(rootID SlabID) *OrderedMapState { + return r.orderedMapStates[rootID] +} + +// SetOrderedMapState registers state under the given root slab ID. +// Lazily initializes the underlying map. +func (r *BaseStateRegistry) SetOrderedMapState(rootID SlabID, state *OrderedMapState) { + if r.orderedMapStates == nil { + r.orderedMapStates = make(map[SlabID]*OrderedMapState) + } + r.orderedMapStates[rootID] = state +} + +// RemoveStateForSlab clears any registered shared state under the given +// root slab ID. SlabStorage implementations should call this from their +// Remove(SlabID) method so state lifetime tracks slab lifetime. +func (r *BaseStateRegistry) RemoveStateForSlab(rootID SlabID) { + delete(r.arrayStates, rootID) + delete(r.orderedMapStates, rootID) +} diff --git a/storage.go b/storage.go index af1c8f2..3de3512 100644 --- a/storage.go +++ b/storage.go @@ -173,6 +173,7 @@ func (s *LedgerBaseStorage) ResetReporter() { type SlabIterator func() (SlabID, Slab) type SlabStorage interface { + StateRegistry Store(SlabID, Slab) error Retrieve(SlabID) (Slab, bool, error) RetrieveIfLoaded(SlabID) Slab @@ -185,6 +186,10 @@ type SlabStorage interface { // BasicSlabStorage type BasicSlabStorage struct { + // Shared-state registry methods (ArrayState, SetArrayState, + // OrderedMapState, SetOrderedMapState) are inherited via embedding. + *BaseStateRegistry + Slabs map[SlabID]Slab slabIndex map[Address]SlabIndex DecodeStorable StorableDecoder @@ -202,12 +207,13 @@ func NewBasicSlabStorage( decodeTypeInfo TypeInfoDecoder, ) *BasicSlabStorage { return &BasicSlabStorage{ - Slabs: make(map[SlabID]Slab), - slabIndex: make(map[Address]SlabIndex), - cborEncMode: cborEncMode, - cborDecMode: cborDecMode, - DecodeStorable: decodeStorable, - DecodeTypeInfo: decodeTypeInfo, + BaseStateRegistry: NewBaseStateRegistry(), + Slabs: make(map[SlabID]Slab), + slabIndex: make(map[Address]SlabIndex), + cborEncMode: cborEncMode, + cborDecMode: cborDecMode, + DecodeStorable: decodeStorable, + DecodeTypeInfo: decodeTypeInfo, } } @@ -235,6 +241,8 @@ func (s *BasicSlabStorage) Store(id SlabID, slab Slab) error { func (s *BasicSlabStorage) Remove(id SlabID) error { delete(s.Slabs, id) + // Clean up any container shared state registered under this root slab ID. + s.RemoveStateForSlab(id) return nil } @@ -299,6 +307,9 @@ func (s *BasicSlabStorage) SlabIterator() (SlabIterator, error) { // PersistentSlabStorage type PersistentSlabStorage struct { + // Shared-state registry methods inherited via embedding. + *BaseStateRegistry + baseStorage BaseStorage cache map[SlabID]Slab deltas map[SlabID]Slab @@ -322,13 +333,14 @@ func NewPersistentSlabStorage( opts ...StorageOption, ) *PersistentSlabStorage { storage := &PersistentSlabStorage{ - baseStorage: base, - cache: make(map[SlabID]Slab), - deltas: make(map[SlabID]Slab), - cborEncMode: cborEncMode, - cborDecMode: cborDecMode, - DecodeStorable: decodeStorable, - DecodeTypeInfo: decodeTypeInfo, + BaseStateRegistry: NewBaseStateRegistry(), + baseStorage: base, + cache: make(map[SlabID]Slab), + deltas: make(map[SlabID]Slab), + cborEncMode: cborEncMode, + cborDecMode: cborDecMode, + DecodeStorable: decodeStorable, + DecodeTypeInfo: decodeTypeInfo, } for _, applyOption := range opts { @@ -965,6 +977,8 @@ func (s *PersistentSlabStorage) Remove(id SlabID) error { } // add to nil to deltas under that id s.deltas[id] = nil + // Clean up any container shared state registered under this root slab ID. + s.RemoveStateForSlab(id) return nil } From 8f68792aa774ebddae587b916c005dc085c45e56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Mon, 1 Jun 2026 14:59:48 -0700 Subject: [PATCH 03/16] fix handling of inlined slabs --- array_sibling_consistency_test.go | 107 ++++++++++++++++++++++++++++++ map_sibling_consistency_test.go | 86 ++++++++++++++++++++++++ state_registry.go | 26 ++++++-- storage.go | 20 ++++-- 4 files changed, 231 insertions(+), 8 deletions(-) diff --git a/array_sibling_consistency_test.go b/array_sibling_consistency_test.go index e413365..9f8676e 100644 --- a/array_sibling_consistency_test.go +++ b/array_sibling_consistency_test.go @@ -299,3 +299,110 @@ func TestArraySiblingTestStructuralAssertionsAreMeaningful(t *testing.T) { "if this fails, our 'no-split' baseline doesn't hold and the "+ "meta-test can't distinguish the two configurations") } + +// TestArraySiblingConsistencyAcrossInlineTransition guards against a subtle state-lifetime hazard: +// when an uninlined child container shrinks enough to fit inside its parent slab, +// atree calls ArrayDataSlab.Inline, +// which internally calls storage.Remove(slabID) to remove the child slab from storage +// (its data now lives embedded in the parent). +// +// The container itself is NOT destroyed — +// it continues to exist inlined. +// But if SlabStorage.Remove eagerly drops the shared state registry entry, +// every live sibling *Array pointing at that state +// would silently lose canonical state on the next structural change. +// +// The test forces the inline transition while holding sibling instances, +// then verifies they continue to observe consistent state. +func TestArraySiblingConsistencyAcrossInlineTransition(t *testing.T) { + + atree.SetThreshold(256) + defer atree.SetThreshold(1024) + + typeInfo := testutils.NewSimpleTypeInfo(42) + storage := newTestPersistentStorage(t) + address := atree.Address{1, 2, 3, 4, 5, 6, 7, 8} + + outer, err := atree.NewArray(storage, address, typeInfo) + require.NoError(t, err) + + inner, err := atree.NewArray(storage, address, typeInfo) + require.NoError(t, err) + + // Grow inner past the inline threshold BEFORE attaching to outer, + // so that when it's added to outer + // it remains uninlined. + const growSize = 100 + for i := uint64(0); i < growSize; i++ { + require.NoError(t, inner.Append(testutils.NewUint64ValueFromInteger(int(i)))) + } + + require.NoError(t, outer.Append(inner)) + + // Two sibling instances of the inner, while it's uninlined. + a, err := outer.Get(0) + require.NoError(t, err) + sibling1 := a.(*atree.Array) + + a, err = outer.Get(0) + require.NoError(t, err) + sibling2 := a.(*atree.Array) + + require.Equal(t, sibling1.ValueID(), sibling2.ValueID()) + require.False(t, sibling1.Inlined()) + + // Shrink inner through sibling1 until atree re-inlines it. + // This triggers ArrayDataSlab.Inline which calls storage.Remove on the inner's slab ID — + // exactly the path that would drop our registry entry if we cleaned up state on Remove. + for sibling1.Count() > 0 && !sibling1.Inlined() { + _, err := sibling1.Remove(sibling1.Count() - 1) + require.NoError(t, err) + } + require.True(t, sibling1.Inlined(), + "inner must be inlined after the shrink so the test exercises the Inline path") + + // Sibling2 must observe the post-inline state. + // If state was dropped, + // sibling2 still holds a pointer to the old state struct, + // and a fresh Get on outer would build a new state — + // siblings would diverge. + require.Equal(t, sibling1.Count(), sibling2.Count(), + "sibling2 must observe sibling1's removals across the inline transition") + require.Equal(t, sibling1.ValueID(), sibling2.ValueID()) + require.True(t, sibling2.Inlined(), + "sibling2 must see the inlined state through shared state") + + // Cross-check: append through sibling2, observe through sibling1. + require.NoError(t, sibling2.Append(testutils.NewUint64ValueFromInteger(42))) + require.Equal(t, sibling2.Count(), sibling1.Count(), + "sibling1 must observe sibling2's append") + + // Critical assertion: + // a FRESH load via outer.Get(0) after the inline transition + // must return a *Array sharing the SAME state as the pre-inline siblings — + // not a freshly-allocated state. + // + // If storage.Remove (triggered by Inline) dropped the registry entry, + // this Get would create a new *arrayState and register it. + // The fresh state would initially point at the same slab struct, + // but the moment a structural change happens through any instance, + // the two states diverge silently. + a, err = outer.Get(0) + require.NoError(t, err) + sibling3 := a.(*atree.Array) + + // Trigger a structural change through sibling3: + // grow it back to multi-slab, forcing splitRoot. + // With a shared state, sibling1's view updates too. + // With dropped state, sibling1 keeps reading from a stale root. + for i := uint64(0); i < 200; i++ { + require.NoError(t, sibling3.Append(testutils.NewUint64ValueFromInteger(int(i)))) + } + require.False(t, sibling3.IsWithinSingleSlab(), + "sibling3 must have triggered splitRoot during the regrowth") + require.Equal(t, sibling3.Count(), sibling1.Count(), + "sibling1 must see sibling3's post-split count via shared state — "+ + "if this fails, the inline transition dropped the registry "+ + "entry and sibling3 got an independent state") + require.Equal(t, sibling1.ValueID(), sibling3.ValueID()) +} diff --git a/map_sibling_consistency_test.go b/map_sibling_consistency_test.go index f59d6f3..b23c7ac 100644 --- a/map_sibling_consistency_test.go +++ b/map_sibling_consistency_test.go @@ -318,3 +318,89 @@ func TestMapSiblingTestStructuralAssertionsAreMeaningful(t *testing.T) { "if this fails, our 'no-split' baseline doesn't hold and the "+ "meta-test can't distinguish the two configurations") } + +// TestMapSiblingConsistencyAcrossInlineTransition is the OrderedMap +// counterpart to TestArraySiblingConsistencyAcrossInlineTransition. +// It exercises the same state-lifetime hazard: +// MapDataSlab.Inline internally calls storage.Remove(slabID) on the child, +// and our shared-state registry must survive that call +// so future *OrderedMap instances for the now-inlined container +// share state with any existing siblings. +func TestMapSiblingConsistencyAcrossInlineTransition(t *testing.T) { + + atree.SetThreshold(256) + defer atree.SetThreshold(1024) + + typeInfo := testutils.NewSimpleTypeInfo(42) + storage := newTestPersistentStorage(t) + address := atree.Address{1, 2, 3, 4, 5, 6, 7, 8} + + outer, err := atree.NewMap(storage, address, atree.NewDefaultDigesterBuilder(), typeInfo) + require.NoError(t, err) + + inner, err := atree.NewMap(storage, address, atree.NewDefaultDigesterBuilder(), typeInfo) + require.NoError(t, err) + + // Grow inner past the inline threshold before attaching it. + const growSize = 100 + for i := uint64(0); i < growSize; i++ { + k := testutils.NewUint64ValueFromInteger(int(i)) + v := testutils.NewUint64ValueFromInteger(int(i)) + prev, err := inner.Set(testutils.CompareValue, testutils.GetHashInput, k, v) + require.NoError(t, err) + require.Nil(t, prev) + } + + prev, err := outer.Set(testutils.CompareValue, testutils.GetHashInput, + testutils.NewUint64ValueFromInteger(0), inner) + require.NoError(t, err) + require.Nil(t, prev) + + // Two sibling instances while inner is uninlined. + innerVal, err := outer.Get(testutils.CompareValue, testutils.GetHashInput, testutils.NewUint64ValueFromInteger(0)) + require.NoError(t, err) + sibling1 := innerVal.(*atree.OrderedMap) + + innerVal, err = outer.Get(testutils.CompareValue, testutils.GetHashInput, testutils.NewUint64ValueFromInteger(0)) + require.NoError(t, err) + sibling2 := innerVal.(*atree.OrderedMap) + + require.Equal(t, sibling1.ValueID(), sibling2.ValueID()) + require.False(t, sibling1.Inlined()) + + // Shrink inner through sibling1 until atree re-inlines it. + // This triggers MapDataSlab.Inline → storage.Remove. + for i := uint64(0); sibling1.Count() > 0 && !sibling1.Inlined(); i++ { + k := testutils.NewUint64ValueFromInteger(int(i)) + _, _, err := sibling1.Remove(testutils.CompareValue, testutils.GetHashInput, k) + require.NoError(t, err) + } + require.True(t, sibling1.Inlined(), + "inner must be inlined after the shrink so the test exercises the Inline path") + + require.Equal(t, sibling1.Count(), sibling2.Count(), + "sibling2 must observe sibling1's removals across the inline transition") + + // Critical: a fresh load after the inline transition must share the same state + // as the pre-inline siblings. + // Trigger a structural change through this fresh load + // and verify pre-existing siblings see it. + innerVal, err = outer.Get(testutils.CompareValue, testutils.GetHashInput, testutils.NewUint64ValueFromInteger(0)) + require.NoError(t, err) + sibling3 := innerVal.(*atree.OrderedMap) + + for i := uint64(0); i < 200; i++ { + k := testutils.NewUint64ValueFromInteger(int(i + 1000)) + v := testutils.NewUint64ValueFromInteger(int(i + 1000)) + prev, err := sibling3.Set(testutils.CompareValue, testutils.GetHashInput, k, v) + require.NoError(t, err) + require.Nil(t, prev) + } + require.False(t, sibling3.IsWithinSingleSlab(), + "sibling3 must have triggered splitRoot during the regrowth") + require.Equal(t, sibling3.Count(), sibling1.Count(), + "sibling1 must see sibling3's post-split count via shared state — "+ + "if this fails, the inline transition dropped the registry "+ + "entry and sibling3 got an independent state") + require.Equal(t, sibling1.ValueID(), sibling3.ValueID()) +} diff --git a/state_registry.go b/state_registry.go index 2685948..c336539 100644 --- a/state_registry.go +++ b/state_registry.go @@ -21,7 +21,20 @@ package atree // StateRegistry is the interface for managing shared state associated with slabs. // Implementations may embed *BaseStateRegistry to inherit working defaults; // atree's BasicSlabStorage and PersistentSlabStorage do so. -// Remove(SlabID) is also expected to clear any state registered under that slab ID. +// +// NOTE: state must SURVIVE SlabStorage.Remove. +// atree internally calls Remove not only on container destruction +// but also when a child slab is inlined into its parent +// (ArrayDataSlab.Inline / MapDataSlab.Inline). +// In the inline case the container continues to exist logically (embedded in the parent), +// and its state must remain +// so future *Array / *OrderedMap instances for that container +// share the same canonical view as any pre-existing siblings. +// Implementations must NOT eagerly drop state in their Remove method. +// +// State entries therefore live for the lifetime of the storage. +// For per-transaction storage (the common case) this is bounded. +// Callers that want explicit cleanup can use RemoveStateForSlab on *BaseStateRegistry directly. type StateRegistry interface { ArrayState(rootID SlabID) *ArrayState SetArrayState(rootID SlabID, state *ArrayState) @@ -96,9 +109,14 @@ func (r *BaseStateRegistry) SetOrderedMapState(rootID SlabID, state *OrderedMapS r.orderedMapStates[rootID] = state } -// RemoveStateForSlab clears any registered shared state under the given -// root slab ID. SlabStorage implementations should call this from their -// Remove(SlabID) method so state lifetime tracks slab lifetime. +// RemoveStateForSlab clears any registered shared state under the given root slab ID. +// This is an escape hatch for callers that want to bound memory growth for a long-lived storage. +// +// SlabStorage implementations should NOT call this from Remove(SlabID): +// atree's Inline path uses storage.Remove for slabs whose containers remain logically alive +// (just inlined), +// so dropping their state there causes sibling-divergence bugs. +// See the note on StateRegistry. func (r *BaseStateRegistry) RemoveStateForSlab(rootID SlabID) { delete(r.arrayStates, rootID) delete(r.orderedMapStates, rootID) diff --git a/storage.go b/storage.go index 3de3512..6b2d4a0 100644 --- a/storage.go +++ b/storage.go @@ -241,8 +241,18 @@ func (s *BasicSlabStorage) Store(id SlabID, slab Slab) error { func (s *BasicSlabStorage) Remove(id SlabID) error { delete(s.Slabs, id) - // Clean up any container shared state registered under this root slab ID. - s.RemoveStateForSlab(id) + // NOTE: do NOT drop the container shared state here. + // SlabStorage.Remove is called both when a container is destroyed + // AND when it is inlined into its parent (ArrayDataSlab.Inline / MapDataSlab.Inline). + // In the inline case the container continues to exist logically + // (embedded inside the parent), + // and its state must survive + // so future *Array / *OrderedMap instances for this container + // share the same canonical view as any pre-existing siblings. + // + // State entries therefore live for the lifetime of the storage. + // For per-transaction storage (the common case) this is fine. + // Callers that want explicit cleanup can use BaseStateRegistry.RemoveStateForSlab. return nil } @@ -977,8 +987,10 @@ func (s *PersistentSlabStorage) Remove(id SlabID) error { } // add to nil to deltas under that id s.deltas[id] = nil - // Clean up any container shared state registered under this root slab ID. - s.RemoveStateForSlab(id) + // NOTE: do NOT drop the container shared state here. + // See BasicSlabStorage.Remove for the rationale: + // Remove is called both on destruction and on Inline, + // and state must survive the inline case. return nil } From 52144d6abebdd0f36e2b05b15fecc96cda4be607 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Mon, 1 Jun 2026 19:30:16 -0700 Subject: [PATCH 04/16] remove incorrect root replacement --- array_data_slab.go | 5 -- array_metadata_slab.go | 5 -- array_sibling_consistency_test.go | 84 +++++++++++++++++++++++++++++++ map_data_slab.go | 5 -- map_metadata_slab.go | 5 -- map_sibling_consistency_test.go | 79 +++++++++++++++++++++++++++++ 6 files changed, 163 insertions(+), 20 deletions(-) diff --git a/array_data_slab.go b/array_data_slab.go index 797f868..96cff55 100644 --- a/array_data_slab.go +++ b/array_data_slab.go @@ -605,11 +605,6 @@ func (a *ArrayDataSlab) StoredValue(storage SlabStorage) (Value, error) { // Share state with any existing *Array instance for this container. // See array_state.go for rationale. if existing := storage.ArrayState(rootID); existing != nil { - // Adopt the freshly-loaded slab into the shared state. - // atree wires the parentUpdater on the *atree.Array instance it just returned, - // so the state must point at this instance - // for parent notifications from any sibling to fire correctly. - existing.root = a return &Array{ Storage: storage, state: existing, diff --git a/array_metadata_slab.go b/array_metadata_slab.go index 4ee1400..df3c6c5 100644 --- a/array_metadata_slab.go +++ b/array_metadata_slab.go @@ -909,11 +909,6 @@ func (a *ArrayMetaDataSlab) StoredValue(storage SlabStorage) (Value, error) { // Share state with any existing *Array instance for this container. // See array_state.go for rationale. if existing := storage.ArrayState(rootID); existing != nil { - // Adopt the freshly-loaded slab into the shared state. - // atree wires the parentUpdater on the *atree.Array instance it just returned, - // so the state must point at this instance - // for parent notifications from any sibling to fire correctly. - existing.root = a return &Array{ Storage: storage, state: existing, diff --git a/array_sibling_consistency_test.go b/array_sibling_consistency_test.go index 9f8676e..9b25407 100644 --- a/array_sibling_consistency_test.go +++ b/array_sibling_consistency_test.go @@ -406,3 +406,87 @@ func TestArraySiblingConsistencyAcrossInlineTransition(t *testing.T) { "entry and sibling3 got an independent state") require.Equal(t, sibling1.ValueID(), sibling3.ValueID()) } + +// TestArrayBatchBuildWithDistinctInlinedMaps verifies that constructing +// an array from three distinct inlined OrderedMaps produces an array +// whose elements retain their original distinguishing entries. +// +// Scenario (mirrors Cadence's `NewArrayValue([struct1, struct2, struct3])` +// which Transfers each value via `CopyNonRefSimple` before adding to the array): +// - Construct three *OrderedMap values m1, m2, m3, +// each with a single distinguishing entry. +// - Copy each via CopyNonRefSimple to simulate Cadence's transfer step. +// - Build a new *Array via NewArrayFromBatchData, +// supplying the copies as its elements. +// +// All values must live in a single storage: +// atree's shared-state design assumes SlabIDs are unique within a storage, +// but each storage has its own monotonic SlabID counter starting from zero, +// so mixing values across multiple storages can produce SlabID collisions +// that the shared-state registry cannot disambiguate. +func TestArrayBatchBuildWithDistinctInlinedMaps(t *testing.T) { + + typeInfo := testutils.NewSimpleTypeInfo(42) + storage := newTestBasicStorage(t) + var address atree.Address + + const mapCount = 3 + copies := make([]*atree.OrderedMap, mapCount) + for i := range copies { + m, err := atree.NewMap(storage, address, atree.NewDefaultDigesterBuilder(), typeInfo) + require.NoError(t, err) + + prev, err := m.Set( + testutils.CompareValue, testutils.GetHashInput, + testutils.NewUint64ValueFromInteger(0), + testutils.NewUint64ValueFromInteger(i+1), + ) + require.NoError(t, err) + require.Nil(t, prev) + + copied, err := m.CopyNonRefSimple(address, atree.NewDefaultDigesterBuilder()) + require.NoError(t, err) + + copies[i] = copied + } + + idx := 0 + arr, err := atree.NewArrayFromBatchData( + storage, + address, + typeInfo, + func() (atree.Value, error) { + if idx >= mapCount { + return nil, nil + } + m := copies[idx] + idx++ + return m, nil + }, + ) + require.NoError(t, err) + require.Equal(t, uint64(mapCount), arr.Count()) + + // Iterate the array and verify each element retained its distinct content. + iter, err := arr.ReadOnlyIterator() + require.NoError(t, err) + + for i := 0; i < mapCount; i++ { + v, err := iter.Next() + require.NoError(t, err) + require.NotNil(t, v, "iterator must produce an element at index %d", i) + + gotMap, ok := v.(*atree.OrderedMap) + require.True(t, ok, "element %d must be an *OrderedMap", i) + + gotValue, err := gotMap.Get( + testutils.CompareValue, testutils.GetHashInput, + testutils.NewUint64ValueFromInteger(0), + ) + require.NoError(t, err) + + expected := testutils.NewUint64ValueFromInteger(i + 1) + require.Equal(t, expected, gotValue, + "element %d must retain its distinguishing entry", i) + } +} diff --git a/map_data_slab.go b/map_data_slab.go index bcaff33..4e26c38 100644 --- a/map_data_slab.go +++ b/map_data_slab.go @@ -438,11 +438,6 @@ func (m *MapDataSlab) StoredValue(storage SlabStorage) (Value, error) { // Share state with any existing *OrderedMap instance for this container. // See map_state.go for rationale. if existing := storage.OrderedMapState(rootID); existing != nil { - // Adopt the freshly-loaded slab into the shared state. - // atree wires the parentUpdater on the *atree.OrderedMap instance it just returned, - // so the state must point at this instance - // for parent notifications from any sibling to fire correctly. - existing.root = m return &OrderedMap{ Storage: storage, state: existing, diff --git a/map_metadata_slab.go b/map_metadata_slab.go index 5a353d6..0676616 100644 --- a/map_metadata_slab.go +++ b/map_metadata_slab.go @@ -760,11 +760,6 @@ func (m *MapMetaDataSlab) StoredValue(storage SlabStorage) (Value, error) { // Share state with any existing *OrderedMap instance for this container. // See map_state.go for rationale. if existing := storage.OrderedMapState(rootID); existing != nil { - // Adopt the freshly-loaded slab into the shared state. - // atree wires the parentUpdater on the *atree.OrderedMap instance it just returned, - // so the state must point at this instance - // for parent notifications from any sibling to fire correctly. - existing.root = m return &OrderedMap{ Storage: storage, state: existing, diff --git a/map_sibling_consistency_test.go b/map_sibling_consistency_test.go index b23c7ac..b40f62a 100644 --- a/map_sibling_consistency_test.go +++ b/map_sibling_consistency_test.go @@ -404,3 +404,82 @@ func TestMapSiblingConsistencyAcrossInlineTransition(t *testing.T) { "entry and sibling3 got an independent state") require.Equal(t, sibling1.ValueID(), sibling3.ValueID()) } + +// TestMapBuildWithDistinctInlinedMaps is the OrderedMap counterpart to +// TestArrayBatchBuildWithDistinctInlinedMaps: +// inserting three distinct inlined OrderedMap values into a parent OrderedMap +// produces a parent whose entries retain their original distinguishing content. +// +// Scenario: +// - Construct three inner *OrderedMap values m1, m2, m3, +// each with a single distinguishing entry. +// - Copy each via CopyNonRefSimple to simulate a transfer. +// - Insert each copy as the value of an entry in a parent *OrderedMap. +// +// All values must live in a single storage: +// atree's shared-state design assumes SlabIDs are unique within a storage, +// but each storage has its own monotonic SlabID counter starting from zero, +// so mixing values across multiple storages can produce SlabID collisions +// that the shared-state registry cannot disambiguate. +func TestMapBuildWithDistinctInlinedMaps(t *testing.T) { + + typeInfo := testutils.NewSimpleTypeInfo(42) + storage := newTestBasicStorage(t) + var address atree.Address + + const innerCount = 3 + copies := make([]*atree.OrderedMap, innerCount) + for i := range copies { + m, err := atree.NewMap(storage, address, atree.NewDefaultDigesterBuilder(), typeInfo) + require.NoError(t, err) + + prev, err := m.Set( + testutils.CompareValue, testutils.GetHashInput, + testutils.NewUint64ValueFromInteger(0), + testutils.NewUint64ValueFromInteger(i+1), + ) + require.NoError(t, err) + require.Nil(t, prev) + + copied, err := m.CopyNonRefSimple(address, atree.NewDefaultDigesterBuilder()) + require.NoError(t, err) + + copies[i] = copied + } + + outer, err := atree.NewMap(storage, address, atree.NewDefaultDigesterBuilder(), typeInfo) + require.NoError(t, err) + + for i, copied := range copies { + prev, err := outer.Set( + testutils.CompareValue, testutils.GetHashInput, + testutils.NewUint64ValueFromInteger(i), + copied, + ) + require.NoError(t, err) + require.Nil(t, prev) + } + require.Equal(t, uint64(innerCount), outer.Count()) + + // Look up each entry and verify the inner map retained its distinct content. + for i := 0; i < innerCount; i++ { + v, err := outer.Get( + testutils.CompareValue, testutils.GetHashInput, + testutils.NewUint64ValueFromInteger(i), + ) + require.NoError(t, err) + + gotMap, ok := v.(*atree.OrderedMap) + require.True(t, ok, "entry %d's value must be an *OrderedMap", i) + + gotValue, err := gotMap.Get( + testutils.CompareValue, testutils.GetHashInput, + testutils.NewUint64ValueFromInteger(0), + ) + require.NoError(t, err) + + expected := testutils.NewUint64ValueFromInteger(i + 1) + require.Equal(t, expected, gotValue, + "entry %d's inner map must retain its distinguishing entry", i) + } +} From 3f65b5776116309f6b35231482b63287735e4601 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Tue, 2 Jun 2026 11:35:43 -0700 Subject: [PATCH 05/16] add HasParentUpdater and HasReadOnlyMutationCallback --- .gitignore | 2 ++ array.go | 36 +++++++++++++++++++++++++++++++++++- array_iterator.go | 2 +- map.go | 30 ++++++++++++++++++++++++++++++ map_iterator.go | 4 ++-- value.go | 5 +++++ 6 files changed, 75 insertions(+), 4 deletions(-) diff --git a/.gitignore b/.gitignore index 66fd13c..398baf2 100644 --- a/.gitignore +++ b/.gitignore @@ -13,3 +13,5 @@ # Dependency directories (remove the comment below to include it) # vendor/ + +.idea diff --git a/array.go b/array.go index 8b50498..8038d73 100644 --- a/array.go +++ b/array.go @@ -60,9 +60,18 @@ type Array struct { // when this *Array instance triggers a mutation. // It is per-instance, not shared: // a Get-loaded instance has a real parent updater, - // while a readonly-iterator-loaded instance has a trap callback. + // while a readonly-iterator-loaded instance has a trap callback + // (see parentUpdaterIsReadOnlyMutationCallback). // A mutation through one instance must fire only its own callback. parentUpdater parentUpdater + + // parentUpdaterIsReadOnlyMutationCallback is true + // when parentUpdater is a trap callback set by a read-only iterator + // (rather than a real parent-notification callback set by setCallbackWithChild). + // Callers that cache or alias this *Array + // use this to avoid promoting a trap-bearing instance to a shared/canonical wrapper: + // mutations through such a wrapper would trip the trap. + parentUpdaterIsReadOnlyMutationCallback bool } var _ Value = &Array{} @@ -775,6 +784,30 @@ func (a *Array) getIndexByValueID(id ValueID) (uint64, bool) { func (a *Array) setParentUpdater(f parentUpdater) { a.parentUpdater = f + a.parentUpdaterIsReadOnlyMutationCallback = false +} + +// setReadOnlyMutationCallback installs a trap callback that fires +// when the *Array is mutated through this instance, +// indicating the instance was loaded via a read-only iterator. +func (a *Array) setReadOnlyMutationCallback(f parentUpdater) { + a.parentUpdater = f + a.parentUpdaterIsReadOnlyMutationCallback = true +} + +// HasParentUpdater reports whether a parent-notification (or read-only trap) callback is installed. +// Use HasReadOnlyMutationCallback to distinguish the two cases. +func (a *Array) HasParentUpdater() bool { + return a.parentUpdater != nil +} + +// HasReadOnlyMutationCallback reports whether the installed parentUpdater +// is a trap callback set by a read-only iterator +// (as opposed to a real parent-notification callback). +// Callers that want to share or canonicalize the *Array should consult this +// to avoid caching a trap-bearing instance. +func (a *Array) HasReadOnlyMutationCallback() bool { + return a.parentUpdaterIsReadOnlyMutationCallback } // setCallbackWithChild sets up callback function with child value (child) @@ -911,6 +944,7 @@ func (a *Array) notifyParentIfNeeded() error { } if !found { a.parentUpdater = nil + a.parentUpdaterIsReadOnlyMutationCallback = false } return nil } diff --git a/array_iterator.go b/array_iterator.go index d07f25b..e5be52c 100644 --- a/array_iterator.go +++ b/array_iterator.go @@ -100,7 +100,7 @@ func (i *readOnlyArrayIterator) setMutationCallback(value Value) { unwrappedChild, _ := unwrapValue(value) if v, ok := unwrappedChild.(mutableValueNotifier); ok { - v.setParentUpdater(func() (found bool, err error) { + v.setReadOnlyMutationCallback(func() (found bool, err error) { i.valueMutationCallback(value) return true, NewReadOnlyIteratorElementMutationError(i.array.ValueID(), v.ValueID()) }) diff --git a/map.go b/map.go index 949e2fa..3ba7023 100644 --- a/map.go +++ b/map.go @@ -67,6 +67,11 @@ type OrderedMap struct { // parentUpdater is per-instance, // see Array.parentUpdater for rationale. parentUpdater parentUpdater + + // parentUpdaterIsReadOnlyMutationCallback indicates parentUpdater is a + // read-only iterator's trap callback rather than a real parent-notification callback. + // See Array.parentUpdaterIsReadOnlyMutationCallback for rationale. + parentUpdaterIsReadOnlyMutationCallback bool } var _ Value = &OrderedMap{} @@ -981,6 +986,30 @@ func (m *OrderedMap) Inlinable(maxInlineSize uint32) bool { func (m *OrderedMap) setParentUpdater(f parentUpdater) { m.parentUpdater = f + m.parentUpdaterIsReadOnlyMutationCallback = false +} + +// setReadOnlyMutationCallback installs a trap callback that fires +// when the *OrderedMap is mutated through this instance, +// indicating the instance was loaded via a read-only iterator. +func (m *OrderedMap) setReadOnlyMutationCallback(f parentUpdater) { + m.parentUpdater = f + m.parentUpdaterIsReadOnlyMutationCallback = true +} + +// HasParentUpdater reports whether a parent-notification (or read-only trap) callback is installed. +// Use HasReadOnlyMutationCallback to distinguish the two cases. +func (m *OrderedMap) HasParentUpdater() bool { + return m.parentUpdater != nil +} + +// HasReadOnlyMutationCallback reports whether the installed parentUpdater +// is a trap callback set by a read-only iterator +// (as opposed to a real parent-notification callback). +// Callers that want to share or canonicalize the *OrderedMap should consult this +// to avoid caching a trap-bearing instance. +func (m *OrderedMap) HasReadOnlyMutationCallback() bool { + return m.parentUpdaterIsReadOnlyMutationCallback } // setCallbackWithChild sets up callback function with child value (child) @@ -1116,6 +1145,7 @@ func (m *OrderedMap) notifyParentIfNeeded() error { } if !found { m.parentUpdater = nil + m.parentUpdaterIsReadOnlyMutationCallback = false } return nil } diff --git a/map_iterator.go b/map_iterator.go index 5b2b984..bdfc077 100644 --- a/map_iterator.go +++ b/map_iterator.go @@ -145,7 +145,7 @@ func (i *readOnlyMapIterator) setMutationCallback(key, value Value) { unwrappedKey, _ := unwrapValue(key) if k, ok := unwrappedKey.(mutableValueNotifier); ok { - k.setParentUpdater(func() (found bool, err error) { + k.setReadOnlyMutationCallback(func() (found bool, err error) { i.keyMutationCallback(key) return true, NewReadOnlyIteratorElementMutationError(i.m.ValueID(), k.ValueID()) }) @@ -154,7 +154,7 @@ func (i *readOnlyMapIterator) setMutationCallback(key, value Value) { unwrappedValue, _ := unwrapValue(value) if v, ok := unwrappedValue.(mutableValueNotifier); ok { - v.setParentUpdater(func() (found bool, err error) { + v.setReadOnlyMutationCallback(func() (found bool, err error) { i.valueMutationCallback(value) return true, NewReadOnlyIteratorElementMutationError(i.m.ValueID(), v.ValueID()) }) diff --git a/value.go b/value.go index 21061fc..ae89b2f 100644 --- a/value.go +++ b/value.go @@ -45,6 +45,11 @@ type mutableValueNotifier interface { Value ValueID() ValueID setParentUpdater(parentUpdater) + // setReadOnlyMutationCallback installs a trap callback used by read-only iterators. + // Distinct from setParentUpdater + // so callers (and HasReadOnlyMutationCallback on *Array / *OrderedMap) + // can tell a trap apart from a real parent-notification callback. + setReadOnlyMutationCallback(parentUpdater) Inlined() bool // Inlinable returns true if a mutable value can be inlined by fitting within the given maxInlineSize. Inlinable(maxInlineSize uint32) bool From 6f8e5cfd3534cf4c5b8261e488ec8d6c32c975e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Tue, 2 Jun 2026 16:09:26 -0700 Subject: [PATCH 06/16] clean up --- array.go | 30 +++++++++++++----------------- array_data_slab.go | 12 ++++-------- array_metadata_slab.go | 12 ++++-------- map.go | 35 ++++++++++++----------------------- map_data_slab.go | 13 ++++--------- map_metadata_slab.go | 13 ++++--------- 6 files changed, 41 insertions(+), 74 deletions(-) diff --git a/array.go b/array.go index 8038d73..285efdc 100644 --- a/array.go +++ b/array.go @@ -120,27 +120,23 @@ func NewArrayWithRootID(storage SlabStorage, rootID SlabID) (*Array, error) { // If another *Array instance for this container already exists, reuse // its shared state so structural changes propagate. - if existing := storage.ArrayState(rootID); existing != nil { - return &Array{ - Storage: storage, - state: existing, - }, nil - } + state := storage.ArrayState(rootID) + if state == nil { + root, err := getArraySlab(storage, rootID) + if err != nil { + // Don't need to wrap error as external error because err is already categorized by getArraySlab(). + return nil, err + } - root, err := getArraySlab(storage, rootID) - if err != nil { - // Don't need to wrap error as external error because err is already categorized by getArraySlab(). - return nil, err - } + extraData := root.ExtraData() + if extraData == nil { + return nil, NewNotValueError(rootID) + } - extraData := root.ExtraData() - if extraData == nil { - return nil, NewNotValueError(rootID) + state = newArrayState(root) + storage.SetArrayState(rootID, state) } - state := newArrayState(root) - storage.SetArrayState(rootID, state) - return &Array{ Storage: storage, state: state, diff --git a/array_data_slab.go b/array_data_slab.go index 96cff55..e389747 100644 --- a/array_data_slab.go +++ b/array_data_slab.go @@ -604,15 +604,11 @@ func (a *ArrayDataSlab) StoredValue(storage SlabStorage) (Value, error) { // Share state with any existing *Array instance for this container. // See array_state.go for rationale. - if existing := storage.ArrayState(rootID); existing != nil { - return &Array{ - Storage: storage, - state: existing, - }, nil + state := storage.ArrayState(rootID) + if state == nil { + state = newArrayState(a) + storage.SetArrayState(rootID, state) } - - state := newArrayState(a) - storage.SetArrayState(rootID, state) return &Array{ Storage: storage, state: state, diff --git a/array_metadata_slab.go b/array_metadata_slab.go index df3c6c5..0b324c1 100644 --- a/array_metadata_slab.go +++ b/array_metadata_slab.go @@ -908,15 +908,11 @@ func (a *ArrayMetaDataSlab) StoredValue(storage SlabStorage) (Value, error) { // Share state with any existing *Array instance for this container. // See array_state.go for rationale. - if existing := storage.ArrayState(rootID); existing != nil { - return &Array{ - Storage: storage, - state: existing, - }, nil + state := storage.ArrayState(rootID) + if state == nil { + state = newArrayState(a) + storage.SetArrayState(rootID, state) } - - state := newArrayState(a) - storage.SetArrayState(rootID, state) return &Array{ Storage: storage, state: state, diff --git a/map.go b/map.go index 3ba7023..1c14450 100644 --- a/map.go +++ b/map.go @@ -142,35 +142,24 @@ func NewMapWithRootID(storage SlabStorage, rootID SlabID, digestBuilder Digester // If another *OrderedMap instance for this container already exists, // reuse its shared state so structural changes propagate. - if existing := storage.OrderedMapState(rootID); existing != nil { - // Re-seed the caller's digester from the existing extra data so - // hkeys match the canonical map. - if extraData := existing.root.ExtraData(); extraData != nil { - digestBuilder.SetSeed(extraData.Seed, typicalRandomConstant) + state := storage.OrderedMapState(rootID) + if state == nil { + root, err := getMapSlab(storage, rootID) + if err != nil { + // Don't need to wrap error as external error because err is already categorized by getMapSlab(). + return nil, err } - return &OrderedMap{ - Storage: storage, - state: existing, - digesterBuilder: digestBuilder, - }, nil - } - root, err := getMapSlab(storage, rootID) - if err != nil { - // Don't need to wrap error as external error because err is already categorized by getMapSlab(). - return nil, err + state = newOrderedMapState(root) + storage.SetOrderedMapState(rootID, state) } - extraData := root.ExtraData() - if extraData == nil { - return nil, NewNotValueError(rootID) + // Re-seed the caller's digester from the canonical extra data so + // hkeys match the canonical map. + if extraData := state.root.ExtraData(); extraData != nil { + digestBuilder.SetSeed(extraData.Seed, typicalRandomConstant) } - digestBuilder.SetSeed(extraData.Seed, typicalRandomConstant) - - state := newOrderedMapState(root) - storage.SetOrderedMapState(rootID, state) - return &OrderedMap{ Storage: storage, state: state, diff --git a/map_data_slab.go b/map_data_slab.go index 4e26c38..ac5f1a7 100644 --- a/map_data_slab.go +++ b/map_data_slab.go @@ -437,16 +437,11 @@ func (m *MapDataSlab) StoredValue(storage SlabStorage) (Value, error) { // Share state with any existing *OrderedMap instance for this container. // See map_state.go for rationale. - if existing := storage.OrderedMapState(rootID); existing != nil { - return &OrderedMap{ - Storage: storage, - state: existing, - digesterBuilder: digestBuilder, - }, nil + state := storage.OrderedMapState(rootID) + if state == nil { + state = newOrderedMapState(m) + storage.SetOrderedMapState(rootID, state) } - - state := newOrderedMapState(m) - storage.SetOrderedMapState(rootID, state) return &OrderedMap{ Storage: storage, state: state, diff --git a/map_metadata_slab.go b/map_metadata_slab.go index 0676616..d146feb 100644 --- a/map_metadata_slab.go +++ b/map_metadata_slab.go @@ -759,16 +759,11 @@ func (m *MapMetaDataSlab) StoredValue(storage SlabStorage) (Value, error) { // Share state with any existing *OrderedMap instance for this container. // See map_state.go for rationale. - if existing := storage.OrderedMapState(rootID); existing != nil { - return &OrderedMap{ - Storage: storage, - state: existing, - digesterBuilder: digestBuilder, - }, nil + state := storage.OrderedMapState(rootID) + if state == nil { + state = newOrderedMapState(m) + storage.SetOrderedMapState(rootID, state) } - - state := newOrderedMapState(m) - storage.SetOrderedMapState(rootID, state) return &OrderedMap{ Storage: storage, state: state, From 5a5276105b9b77c3452c4ce06258512c5fcb60c4 Mon Sep 17 00:00:00 2001 From: "Leo Zhang (zhangchiqing)" Date: Tue, 2 Jun 2026 21:34:20 -0700 Subject: [PATCH 07/16] add more tests for sibling --- array_sibling_consistency_test.go | 246 ++++++++++++++++++++++++++ map_sibling_consistency_test.go | 285 ++++++++++++++++++++++++++++++ 2 files changed, 531 insertions(+) diff --git a/array_sibling_consistency_test.go b/array_sibling_consistency_test.go index 9b25407..aaafbdd 100644 --- a/array_sibling_consistency_test.go +++ b/array_sibling_consistency_test.go @@ -490,3 +490,249 @@ func TestArrayBatchBuildWithDistinctInlinedMaps(t *testing.T) { "element %d must retain its distinguishing entry", i) } } + +// TestArraySiblingConsistencyAfterPopIterate verifies that +// PopIterate, which replaces state.root with a brand-new empty *ArrayDataSlab, +// propagates that replacement to every sibling Go handle. +// +// PopIterate is the only operation that swaps state.root out for a freshly +// allocated slab (array.go: `a.state.root = &ArrayDataSlab{...}`). +// If state were not shared, the second sibling would keep pointing at the +// pre-pop root and continue reporting the old count. +// Mutating through sibling2 after the pop must also be observed by sibling1. +func TestArraySiblingConsistencyAfterPopIterate(t *testing.T) { + + atree.SetThreshold(256) + defer atree.SetThreshold(1024) + + typeInfo := testutils.NewSimpleTypeInfo(42) + storage := newTestPersistentStorage(t) + address := atree.Address{1, 2, 3, 4, 5, 6, 7, 8} + + outer, err := atree.NewArray(storage, address, typeInfo) + require.NoError(t, err) + + inner, err := atree.NewArray(storage, address, typeInfo) + require.NoError(t, err) + + // Populate enough to force multi-slab so PopIterate exercises the + // non-trivial case (an inlined single-slab case wouldn't change state.root). + const initialCount = 200 + for i := uint64(0); i < initialCount; i++ { + require.NoError(t, inner.Append(testutils.NewUint64ValueFromInteger(int(i)))) + } + + require.NoError(t, outer.Append(inner)) + + a, err := outer.Get(0) + require.NoError(t, err) + sibling1 := a.(*atree.Array) + + a, err = outer.Get(0) + require.NoError(t, err) + sibling2 := a.(*atree.Array) + + require.Equal(t, sibling1.ValueID(), sibling2.ValueID()) + require.False(t, sibling1.IsWithinSingleSlab(), + "populated inner array must span multiple slabs") + + rootIDBeforePop := sibling1.SlabID() + + // Pop through sibling1. After this, state.root has been replaced with + // a freshly allocated empty *ArrayDataSlab carrying the original root SlabID. + require.NoError(t, sibling1.PopIterate(func(atree.Storable) {})) + + require.Equal(t, uint64(0), sibling1.Count(), "sibling1 must observe empty array post-pop") + require.Equal(t, uint64(0), sibling2.Count(), + "sibling2 must observe the new empty root through shared state") + require.Equal(t, rootIDBeforePop, sibling2.SlabID(), + "PopIterate must preserve the canonical root SlabID") + require.Equal(t, sibling1.ValueID(), sibling2.ValueID()) + require.True(t, sibling1.IsWithinSingleSlab(), + "post-pop root must be a single empty data slab") + require.True(t, sibling2.IsWithinSingleSlab()) + + // Mutate via sibling2; sibling1 must see it through shared state. + require.NoError(t, sibling2.Append(testutils.NewUint64ValueFromInteger(7))) + require.Equal(t, uint64(1), sibling1.Count(), + "sibling1 must observe sibling2's post-pop append") + v, err := sibling1.Get(0) + require.NoError(t, err) + require.Equal(t, testutils.NewUint64ValueFromInteger(7), v) +} + +// TestArraySiblingConsistencyAcrossUninlineTransition is the reverse direction +// of TestArraySiblingConsistencyAcrossInlineTransition: +// start with an inlined inner, mutate enough through one sibling to force +// the inner to be uninlined, and verify every sibling observes the transition. +// +// The uninline transition runs through *Array.Storable when the parent re-stores +// the child: once the inner's root becomes too large to inline (or becomes a +// MetaDataSlab, which is never inlinable), `state.root.Uninline` flips `inlined` +// to false on the shared slab and stores it in storage. A second sibling holding +// only its own root pointer (pre-PR) would still report Inlined() == true. +func TestArraySiblingConsistencyAcrossUninlineTransition(t *testing.T) { + + atree.SetThreshold(256) + defer atree.SetThreshold(1024) + + typeInfo := testutils.NewSimpleTypeInfo(42) + storage := newTestPersistentStorage(t) + address := atree.Address{1, 2, 3, 4, 5, 6, 7, 8} + + outer, err := atree.NewArray(storage, address, typeInfo) + require.NoError(t, err) + + // Build an inlined inner: empty array attached to outer is inlined by default. + inner, err := atree.NewArray(storage, address, typeInfo) + require.NoError(t, err) + require.NoError(t, outer.Append(inner)) + + a, err := outer.Get(0) + require.NoError(t, err) + sibling1 := a.(*atree.Array) + + a, err = outer.Get(0) + require.NoError(t, err) + sibling2 := a.(*atree.Array) + + require.Equal(t, sibling1.ValueID(), sibling2.ValueID()) + require.True(t, sibling1.Inlined(), + "freshly attached empty inner must be inlined") + require.True(t, sibling2.Inlined()) + + // Grow via sibling1 until it can no longer be inlined. + // Each Append eventually triggers parent re-store → inner.Storable() → + // Uninline when the slab exceeds the inline size or becomes a meta slab. + for i := uint64(0); sibling1.Inlined(); i++ { + require.NoError(t, sibling1.Append(testutils.NewUint64ValueFromInteger(int(i)))) + + // Guard against an infinite loop if a future change quietly raises + // the inline threshold above what 1000 Appends can exceed. + require.Less(t, i, uint64(1000), + "sibling1 must transition to uninlined within a bounded number of appends") + } + + require.False(t, sibling1.Inlined(), + "sibling1 must observe the uninline transition") + require.False(t, sibling2.Inlined(), + "sibling2 must observe the uninline transition through shared state") + require.Equal(t, sibling1.Count(), sibling2.Count()) + require.Equal(t, sibling1.ValueID(), sibling2.ValueID()) + require.NotEqual(t, atree.SlabIDUndefined, sibling2.SlabID(), + "uninlined sibling must expose a real SlabID") + + // Cross-check: append through sibling2; sibling1 must see it. + require.NoError(t, sibling2.Append(testutils.NewUint64ValueFromInteger(9999))) + require.Equal(t, sibling2.Count(), sibling1.Count()) + last, err := sibling1.Get(sibling1.Count() - 1) + require.NoError(t, err) + require.Equal(t, testutils.NewUint64ValueFromInteger(9999), last) +} + +// TestArrayTrapCallbackDoesNotFireOnSiblingMutation pins down the contract +// introduced by HasReadOnlyMutationCallback / setReadOnlyMutationCallback: +// a per-instance trap callback on one sibling must NOT fire when an +// unrelated sibling triggers a structural change through the shared state. +// +// Two *Array Go handles for the same inner exist: +// - sibling1, obtained via outer.Get(0): real parent-notification callback. +// - sibling2, obtained via outer.ReadOnlyIterator().Next(): trap callback. +// +// Mutations through sibling1 must succeed and propagate state to sibling2 +// without firing sibling2's trap (state propagation does not invoke +// sibling.parentUpdater on uninvolved siblings). +// Mutations through sibling2 must trip the trap and return +// ReadOnlyIteratorElementMutationError. +func TestArrayTrapCallbackDoesNotFireOnSiblingMutation(t *testing.T) { + + typeInfo := testutils.NewSimpleTypeInfo(42) + storage := newTestPersistentStorage(t) + address := atree.Address{1, 2, 3, 4, 5, 6, 7, 8} + + outer, err := atree.NewArray(storage, address, typeInfo) + require.NoError(t, err) + + inner, err := atree.NewArray(storage, address, typeInfo) + require.NoError(t, err) + require.NoError(t, inner.Append(testutils.NewUint64ValueFromInteger(0))) + + require.NoError(t, outer.Append(inner)) + + // sibling1: real parent-notification callback. + a, err := outer.Get(0) + require.NoError(t, err) + sibling1 := a.(*atree.Array) + require.True(t, sibling1.HasParentUpdater()) + require.False(t, sibling1.HasReadOnlyMutationCallback(), + "Get-loaded sibling must carry a real callback") + + // sibling2: trap callback installed by the readonly iterator. + iter, err := outer.ReadOnlyIterator() + require.NoError(t, err) + v, err := iter.Next() + require.NoError(t, err) + sibling2 := v.(*atree.Array) + require.True(t, sibling2.HasParentUpdater()) + require.True(t, sibling2.HasReadOnlyMutationCallback(), + "iterator-loaded sibling must carry a trap callback") + + // Sanity: siblings share state. + require.Equal(t, sibling1.ValueID(), sibling2.ValueID()) + + // Mutate through sibling1 (real callback). Must succeed. + require.NoError(t, sibling1.Append(testutils.NewUint64ValueFromInteger(1))) + require.Equal(t, uint64(2), sibling1.Count()) + require.Equal(t, uint64(2), sibling2.Count(), + "sibling2 must observe sibling1's mutation through shared state") + + // sibling2's trap must still be installed; sibling1's must still be real. + require.False(t, sibling1.HasReadOnlyMutationCallback(), + "sibling1's real callback must not be replaced by sibling2's trap") + require.True(t, sibling2.HasReadOnlyMutationCallback(), + "sibling2's trap must survive sibling1's mutation "+ + "(state propagation must not invoke uninvolved siblings' updaters)") + + // Mutate through sibling2 (trap callback). Must return the trap error. + err = sibling2.Append(testutils.NewUint64ValueFromInteger(2)) + var mutationError *atree.ReadOnlyIteratorElementMutationError + require.ErrorAs(t, err, &mutationError, + "mutating through a trap-bearing sibling must return ReadOnlyIteratorElementMutationError") +} + +// TestNewArrayWithRootIDReturnsSameState verifies: +// two calls to NewArrayWithRootID for the same rootID must return *Array +// instances backed by the same shared state — i.e. the same Go pointer for +// `state.root`. Tested indirectly via GetArrayRootSlab pointer equality. +func TestNewArrayWithRootIDReturnsSameState(t *testing.T) { + + typeInfo := testutils.NewSimpleTypeInfo(42) + storage := newTestPersistentStorage(t) + address := atree.Address{1, 2, 3, 4, 5, 6, 7, 8} + + arr, err := atree.NewArray(storage, address, typeInfo) + require.NoError(t, err) + require.NoError(t, arr.Append(testutils.NewUint64ValueFromInteger(0))) + + rootID := arr.SlabID() + require.NotEqual(t, atree.SlabIDUndefined, rootID, + "standalone array must have a real SlabID for NewArrayWithRootID to succeed") + + a1, err := atree.NewArrayWithRootID(storage, rootID) + require.NoError(t, err) + + a2, err := atree.NewArrayWithRootID(storage, rootID) + require.NoError(t, err) + + require.NotSame(t, a1, a2, + "each NewArrayWithRootID call must return a distinct *Array Go object") + require.Same(t, atree.GetArrayRootSlab(a1), atree.GetArrayRootSlab(a2), + "two NewArrayWithRootID calls for the same rootID must back the *Array "+ + "instances with the same shared state.root pointer") + require.Equal(t, a1.ValueID(), a2.ValueID()) + + // Functional cross-check: mutate through a1, observe through a2. + require.NoError(t, a1.Append(testutils.NewUint64ValueFromInteger(42))) + require.Equal(t, a1.Count(), a2.Count(), + "a2 must observe a1's mutation through the shared state") +} diff --git a/map_sibling_consistency_test.go b/map_sibling_consistency_test.go index b40f62a..2d2a691 100644 --- a/map_sibling_consistency_test.go +++ b/map_sibling_consistency_test.go @@ -483,3 +483,288 @@ func TestMapBuildWithDistinctInlinedMaps(t *testing.T) { "entry %d's inner map must retain its distinguishing entry", i) } } + +// TestMapSiblingConsistencyAfterPopIterate is the OrderedMap counterpart +// to TestArraySiblingConsistencyAfterPopIterate. +// PopIterate replaces state.root with a freshly allocated empty *MapDataSlab +// (map.go: `m.state.root = &MapDataSlab{...}`); the replacement must be +// observed by every sibling Go handle through the shared state. +func TestMapSiblingConsistencyAfterPopIterate(t *testing.T) { + + atree.SetThreshold(256) + defer atree.SetThreshold(1024) + + typeInfo := testutils.NewSimpleTypeInfo(42) + storage := newTestPersistentStorage(t) + address := atree.Address{1, 2, 3, 4, 5, 6, 7, 8} + + outer, err := atree.NewMap(storage, address, atree.NewDefaultDigesterBuilder(), typeInfo) + require.NoError(t, err) + + inner, err := atree.NewMap(storage, address, atree.NewDefaultDigesterBuilder(), typeInfo) + require.NoError(t, err) + + // Populate enough to force multi-slab so PopIterate exercises the + // non-trivial case. + const initialCount = 200 + for i := uint64(0); i < initialCount; i++ { + k := testutils.NewUint64ValueFromInteger(int(i)) + v := testutils.NewUint64ValueFromInteger(int(i)) + prev, err := inner.Set(testutils.CompareValue, testutils.GetHashInput, k, v) + require.NoError(t, err) + require.Nil(t, prev) + } + + prev, err := outer.Set(testutils.CompareValue, testutils.GetHashInput, + testutils.NewUint64ValueFromInteger(0), inner) + require.NoError(t, err) + require.Nil(t, prev) + + innerVal, err := outer.Get(testutils.CompareValue, testutils.GetHashInput, testutils.NewUint64ValueFromInteger(0)) + require.NoError(t, err) + sibling1 := innerVal.(*atree.OrderedMap) + + innerVal, err = outer.Get(testutils.CompareValue, testutils.GetHashInput, testutils.NewUint64ValueFromInteger(0)) + require.NoError(t, err) + sibling2 := innerVal.(*atree.OrderedMap) + + require.Equal(t, sibling1.ValueID(), sibling2.ValueID()) + require.False(t, sibling1.IsWithinSingleSlab(), + "populated inner map must span multiple slabs") + + rootIDBeforePop := sibling1.SlabID() + + // Pop through sibling1. State.root is replaced with a new empty *MapDataSlab + // carrying the original root SlabID. + require.NoError(t, sibling1.PopIterate(func(atree.Storable, atree.Storable) {})) + + require.Equal(t, uint64(0), sibling1.Count(), "sibling1 must observe empty map post-pop") + require.Equal(t, uint64(0), sibling2.Count(), + "sibling2 must observe the new empty root through shared state") + require.Equal(t, rootIDBeforePop, sibling2.SlabID(), + "PopIterate must preserve the canonical root SlabID") + require.Equal(t, sibling1.ValueID(), sibling2.ValueID()) + require.True(t, sibling1.IsWithinSingleSlab(), + "post-pop root must be a single empty data slab") + require.True(t, sibling2.IsWithinSingleSlab()) + + // Mutate via sibling2; sibling1 must see it through shared state. + k7 := testutils.NewUint64ValueFromInteger(7) + v7 := testutils.NewUint64ValueFromInteger(7) + prev, err = sibling2.Set(testutils.CompareValue, testutils.GetHashInput, k7, v7) + require.NoError(t, err) + require.Nil(t, prev) + require.Equal(t, uint64(1), sibling1.Count(), + "sibling1 must observe sibling2's post-pop insert") + got, err := sibling1.Get(testutils.CompareValue, testutils.GetHashInput, k7) + require.NoError(t, err) + require.Equal(t, v7, got) +} + +// TestMapSiblingConsistencyAcrossUninlineTransition is the OrderedMap counterpart +// to TestArraySiblingConsistencyAcrossUninlineTransition: start with an inlined +// inner map, mutate enough through one sibling to force uninlining, and verify +// every sibling observes the transition. +func TestMapSiblingConsistencyAcrossUninlineTransition(t *testing.T) { + + atree.SetThreshold(256) + defer atree.SetThreshold(1024) + + typeInfo := testutils.NewSimpleTypeInfo(42) + storage := newTestPersistentStorage(t) + address := atree.Address{1, 2, 3, 4, 5, 6, 7, 8} + + outer, err := atree.NewMap(storage, address, atree.NewDefaultDigesterBuilder(), typeInfo) + require.NoError(t, err) + + // An empty inner attached to outer is inlined by default. + inner, err := atree.NewMap(storage, address, atree.NewDefaultDigesterBuilder(), typeInfo) + require.NoError(t, err) + + prev, err := outer.Set(testutils.CompareValue, testutils.GetHashInput, + testutils.NewUint64ValueFromInteger(0), inner) + require.NoError(t, err) + require.Nil(t, prev) + + innerVal, err := outer.Get(testutils.CompareValue, testutils.GetHashInput, testutils.NewUint64ValueFromInteger(0)) + require.NoError(t, err) + sibling1 := innerVal.(*atree.OrderedMap) + + innerVal, err = outer.Get(testutils.CompareValue, testutils.GetHashInput, testutils.NewUint64ValueFromInteger(0)) + require.NoError(t, err) + sibling2 := innerVal.(*atree.OrderedMap) + + require.Equal(t, sibling1.ValueID(), sibling2.ValueID()) + require.True(t, sibling1.Inlined(), + "freshly attached empty inner must be inlined") + require.True(t, sibling2.Inlined()) + + // Grow via sibling1 until the inner can no longer be inlined. + for i := uint64(0); sibling1.Inlined(); i++ { + k := testutils.NewUint64ValueFromInteger(int(i)) + v := testutils.NewUint64ValueFromInteger(int(i)) + prev, err := sibling1.Set(testutils.CompareValue, testutils.GetHashInput, k, v) + require.NoError(t, err) + require.Nil(t, prev) + + require.Less(t, i, uint64(1000), + "sibling1 must transition to uninlined within a bounded number of inserts") + } + + require.False(t, sibling1.Inlined(), + "sibling1 must observe the uninline transition") + require.False(t, sibling2.Inlined(), + "sibling2 must observe the uninline transition through shared state") + require.Equal(t, sibling1.Count(), sibling2.Count()) + require.Equal(t, sibling1.ValueID(), sibling2.ValueID()) + require.NotEqual(t, atree.SlabIDUndefined, sibling2.SlabID(), + "uninlined sibling must expose a real SlabID") + + // Cross-check: insert through sibling2; sibling1 must see it. + kX := testutils.NewUint64ValueFromInteger(99999) + vX := testutils.NewUint64ValueFromInteger(99999) + prev, err = sibling2.Set(testutils.CompareValue, testutils.GetHashInput, kX, vX) + require.NoError(t, err) + require.Nil(t, prev) + require.Equal(t, sibling2.Count(), sibling1.Count()) + got, err := sibling1.Get(testutils.CompareValue, testutils.GetHashInput, kX) + require.NoError(t, err) + require.Equal(t, vX, got) +} + +// TestMapTrapCallbackDoesNotFireOnSiblingMutation is the OrderedMap counterpart +// to TestArrayTrapCallbackDoesNotFireOnSiblingMutation. A trap callback on a +// readonly-iterator-loaded sibling must not fire when a separate sibling +// initiates a structural change, but must fire when the trap-bearing sibling +// itself is mutated. +func TestMapTrapCallbackDoesNotFireOnSiblingMutation(t *testing.T) { + + typeInfo := testutils.NewSimpleTypeInfo(42) + storage := newTestPersistentStorage(t) + address := atree.Address{1, 2, 3, 4, 5, 6, 7, 8} + + outer, err := atree.NewMap(storage, address, atree.NewDefaultDigesterBuilder(), typeInfo) + require.NoError(t, err) + + inner, err := atree.NewMap(storage, address, atree.NewDefaultDigesterBuilder(), typeInfo) + require.NoError(t, err) + + k0 := testutils.NewUint64ValueFromInteger(0) + v0 := testutils.NewUint64ValueFromInteger(0) + prev, err := inner.Set(testutils.CompareValue, testutils.GetHashInput, k0, v0) + require.NoError(t, err) + require.Nil(t, prev) + + outerKey := testutils.NewUint64ValueFromInteger(0) + prev, err = outer.Set(testutils.CompareValue, testutils.GetHashInput, outerKey, inner) + require.NoError(t, err) + require.Nil(t, prev) + + // sibling1: real parent-notification callback. + v, err := outer.Get(testutils.CompareValue, testutils.GetHashInput, outerKey) + require.NoError(t, err) + sibling1 := v.(*atree.OrderedMap) + require.True(t, sibling1.HasParentUpdater()) + require.False(t, sibling1.HasReadOnlyMutationCallback(), + "Get-loaded sibling must carry a real callback") + + // sibling2: trap callback installed by the readonly iterator. + // Use a key-value iterator so the value (the inner *OrderedMap) carries the trap. + iter, err := outer.ReadOnlyIterator() + require.NoError(t, err) + _, v, err = iter.Next() + require.NoError(t, err) + sibling2 := v.(*atree.OrderedMap) + require.True(t, sibling2.HasParentUpdater()) + require.True(t, sibling2.HasReadOnlyMutationCallback(), + "iterator-loaded sibling must carry a trap callback") + + require.Equal(t, sibling1.ValueID(), sibling2.ValueID()) + + // Mutate through sibling1 (real callback). Must succeed. + k1 := testutils.NewUint64ValueFromInteger(1) + v1 := testutils.NewUint64ValueFromInteger(1) + prev, err = sibling1.Set(testutils.CompareValue, testutils.GetHashInput, k1, v1) + require.NoError(t, err) + require.Nil(t, prev) + require.Equal(t, uint64(2), sibling1.Count()) + require.Equal(t, uint64(2), sibling2.Count(), + "sibling2 must observe sibling1's mutation through shared state") + + // sibling2's trap must still be installed; sibling1's must still be real. + require.False(t, sibling1.HasReadOnlyMutationCallback(), + "sibling1's real callback must not be replaced by sibling2's trap") + require.True(t, sibling2.HasReadOnlyMutationCallback(), + "sibling2's trap must survive sibling1's mutation "+ + "(state propagation must not invoke uninvolved siblings' updaters)") + + // Mutate through sibling2 (trap callback). Must return the trap error. + k2 := testutils.NewUint64ValueFromInteger(2) + v2 := testutils.NewUint64ValueFromInteger(2) + _, err = sibling2.Set(testutils.CompareValue, testutils.GetHashInput, k2, v2) + var mutationError *atree.ReadOnlyIteratorElementMutationError + require.ErrorAs(t, err, &mutationError, + "mutating through a trap-bearing sibling must return ReadOnlyIteratorElementMutationError") +} + +// TestNewMapWithRootIDReturnsSameState pins the idempotence contract for +// OrderedMap analogous to TestNewArrayWithRootIDReturnsSameState: +// two calls to NewMapWithRootID for the same rootID must return *OrderedMap +// instances backed by the same shared state.root pointer, AND each call must +// re-seed the caller's digester from the canonical extraData.Seed so all +// siblings compute consistent hkeys. +func TestNewMapWithRootIDReturnsSameState(t *testing.T) { + + typeInfo := testutils.NewSimpleTypeInfo(42) + storage := newTestPersistentStorage(t) + address := atree.Address{1, 2, 3, 4, 5, 6, 7, 8} + + m, err := atree.NewMap(storage, address, atree.NewDefaultDigesterBuilder(), typeInfo) + require.NoError(t, err) + + k0 := testutils.NewUint64ValueFromInteger(0) + v0 := testutils.NewUint64ValueFromInteger(0) + prev, err := m.Set(testutils.CompareValue, testutils.GetHashInput, k0, v0) + require.NoError(t, err) + require.Nil(t, prev) + + rootID := m.SlabID() + require.NotEqual(t, atree.SlabIDUndefined, rootID) + canonicalSeed := m.Seed() + + // Each NewMapWithRootID call passes a fresh DigesterBuilder; the function + // must seed it from the canonical extra data so hkeys match. + db1 := atree.NewDefaultDigesterBuilder() + m1, err := atree.NewMapWithRootID(storage, rootID, db1) + require.NoError(t, err) + + db2 := atree.NewDefaultDigesterBuilder() + m2, err := atree.NewMapWithRootID(storage, rootID, db2) + require.NoError(t, err) + + require.NotSame(t, m1, m2, + "each NewMapWithRootID call must return a distinct *OrderedMap Go object") + require.Same(t, atree.GetMapRootSlab(m1), atree.GetMapRootSlab(m2), + "two NewMapWithRootID calls for the same rootID must back the "+ + "*OrderedMap instances with the same shared state.root pointer") + require.Equal(t, m1.ValueID(), m2.ValueID()) + require.Equal(t, canonicalSeed, m1.Seed(), + "NewMapWithRootID must report the canonical seed") + require.Equal(t, canonicalSeed, m2.Seed()) + + // Functional cross-check: insert with m1's digester, look up with m2's digester. + // If either digester wasn't re-seeded from the canonical extra data, the + // hkeys would diverge and the lookup would miss. + kX := testutils.NewUint64ValueFromInteger(123) + vX := testutils.NewUint64ValueFromInteger(456) + prev, err = m1.Set(testutils.CompareValue, testutils.GetHashInput, kX, vX) + require.NoError(t, err) + require.Nil(t, prev) + + got, err := m2.Get(testutils.CompareValue, testutils.GetHashInput, kX) + require.NoError(t, err) + require.Equal(t, vX, got, + "m2 must find the key inserted via m1 — proves both digesters "+ + "were re-seeded with the canonical seed and the *OrderedMap instances "+ + "share the same state.root") +} From 6a539fb9e73a72aade381db701d2a7b3c794b787 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Wed, 3 Jun 2026 10:08:48 -0700 Subject: [PATCH 08/16] also run CI for PRs against -rc branches --- .github/workflows/ci.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 71b4330..573b673 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -10,11 +10,13 @@ on: push: branches: - main + - '*-rc' - 'feature/**' - 'release-**' pull_request: branches: - main + - '*-rc' - 'feature/**' - 'release-**' @@ -24,7 +26,7 @@ concurrency: jobs: - # Test on various OS with default Go version. + # Test on various OS with default Go version. tests: name: Test on ${{matrix.os}} runs-on: ${{ matrix.os }} From aab139e9f747c5e988fd8e1e70fcf4fc798f803e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Wed, 3 Jun 2026 10:08:48 -0700 Subject: [PATCH 09/16] also run CI for PRs against -rc branches --- .github/workflows/ci.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 71b4330..573b673 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -10,11 +10,13 @@ on: push: branches: - main + - '*-rc' - 'feature/**' - 'release-**' pull_request: branches: - main + - '*-rc' - 'feature/**' - 'release-**' @@ -24,7 +26,7 @@ concurrency: jobs: - # Test on various OS with default Go version. + # Test on various OS with default Go version. tests: name: Test on ${{matrix.os}} runs-on: ${{ matrix.os }} From 253b82e1ad2c26c1937a3be7d96c2face9fdd707 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Wed, 10 Jun 2026 12:47:26 -0700 Subject: [PATCH 10/16] use HasParentUpdater, remove Array.hasParentUpdater --- array.go | 4 ---- array_test.go | 8 ++++---- export_test.go | 1 - map_test.go | 8 ++++---- 4 files changed, 8 insertions(+), 13 deletions(-) diff --git a/array.go b/array.go index d1d87ac..dcbf7c4 100644 --- a/array.go +++ b/array.go @@ -981,10 +981,6 @@ func (a *Array) Inlinable(maxInlineSize uint32) bool { return a.state.root.Inlinable(maxInlineSize) } -func (a *Array) hasParentUpdater() bool { - return a.parentUpdater != nil -} - func (a *Array) getMutableElementIndexCount() uint64 { return uint64(len(a.state.mutableElementIndex)) } diff --git a/array_test.go b/array_test.go index f6ab315..ce6061a 100644 --- a/array_test.go +++ b/array_test.go @@ -9039,14 +9039,14 @@ func TestArrayWithOutdatedCallback(t *testing.T) { expectedValues[0] = testutils.Uint64Value(0) // childArray.parentUpdater isn't nil before callback is invoked. - require.True(t, atree.ArrayHasParentUpdater(childArray)) + require.True(t, childArray.HasParentUpdater()) // modify overwritten child array err = childArray.Append(testutils.Uint64Value(0)) require.NoError(t, err) // childArray.parentUpdater is nil after callback is invoked. - require.False(t, atree.ArrayHasParentUpdater(childArray)) + require.False(t, childArray.HasParentUpdater()) // No-op on parent testValueEqual(t, expectedValues, parentArray) @@ -9094,14 +9094,14 @@ func TestArrayWithOutdatedCallback(t *testing.T) { expectedValues = testutils.ExpectedArrayValue{} // childArray.parentUpdater isn't nil before callback is invoked. - require.True(t, atree.ArrayHasParentUpdater(childArray)) + require.True(t, childArray.HasParentUpdater()) // modify removed child array err = childArray.Append(testutils.Uint64Value(0)) require.NoError(t, err) // childArray.parentUpdater is nil after callback is invoked. - require.False(t, atree.ArrayHasParentUpdater(childArray)) + require.False(t, childArray.HasParentUpdater()) // No-op on parent testValueEqual(t, expectedValues, parentArray) diff --git a/export_test.go b/export_test.go index 866d6e5..00fe9a5 100644 --- a/export_test.go +++ b/export_test.go @@ -41,7 +41,6 @@ var ( // Exported function of Array for testing. var ( GetArrayRootSlab = (*Array).rootSlab - ArrayHasParentUpdater = (*Array).hasParentUpdater GetArrayMutableElementIndexCount = (*Array).getMutableElementIndexCount GetArrayMutableElementIndex = (*Array).getMutableElementIndex ) diff --git a/map_test.go b/map_test.go index e99d4fd..5349b3f 100644 --- a/map_test.go +++ b/map_test.go @@ -19862,14 +19862,14 @@ func TestMapWithOutdatedCallback(t *testing.T) { expectedKeyValues[k] = testutils.Uint64Value(0) // childArray.parentUpdater isn't nil before callback is invoked. - require.True(t, atree.ArrayHasParentUpdater(childArray)) + require.True(t, childArray.HasParentUpdater()) // modify overwritten child array err = childArray.Append(testutils.Uint64Value(0)) require.NoError(t, err) // childArray.parentUpdater is nil after callback is invoked. - require.False(t, atree.ArrayHasParentUpdater(childArray)) + require.False(t, childArray.HasParentUpdater()) // No-op on parent testValueEqual(t, expectedKeyValues, parentMap) @@ -19922,14 +19922,14 @@ func TestMapWithOutdatedCallback(t *testing.T) { delete(expectedKeyValues, k) // childArray.parentUpdater isn't nil before callback is invoked. - require.True(t, atree.ArrayHasParentUpdater(childArray)) + require.True(t, childArray.HasParentUpdater()) // modify removed child array err = childArray.Append(testutils.Uint64Value(0)) require.NoError(t, err) // childArray.parentUpdater is nil after callback is invoked. - require.False(t, atree.ArrayHasParentUpdater(childArray)) + require.False(t, childArray.HasParentUpdater()) // No-op on parent testValueEqual(t, expectedKeyValues, parentMap) From 90cf75333712d2b333349b04f27a7b1265ceb085 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Wed, 10 Jun 2026 12:51:38 -0700 Subject: [PATCH 11/16] detect destroyed containers in NewArrayWithRootID / NewMapWithRootID MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A registered container state deliberately survives storage.Remove because Remove is also called when a container is inlined while still alive. The side effect: after a container was genuinely destroyed, the constructors served the leftover state as a live container (a zombie), where they previously returned SlabNotFoundError. Distinguish the two cases on lookup: a non-inlined root must still have its slab in storage. If the slab is gone, the container was destroyed — clear the leftover state and return SlabNotFoundError, restoring pre-shared-state behavior. Inlined roots are exempt since they legitimately have no standalone slab. The existence check only fires on a registry hit with a non-inlined root; a cold-cache hit costs one base-storage read + decode, which is what the constructors paid on every call before the shared-state registry existed. Adds StateRegistry.RemoveStateForSlab so the constructors can clear the stale entry through SlabStorage; BaseStateRegistry already implements it, so embedders are unaffected. --- array.go | 24 +++++++++++++++++ array_sibling_consistency_test.go | 43 ++++++++++++++++++++++++++++++ map.go | 24 +++++++++++++++++ map_sibling_consistency_test.go | 44 +++++++++++++++++++++++++++++++ 4 files changed, 135 insertions(+) diff --git a/array.go b/array.go index dcbf7c4..9b51501 100644 --- a/array.go +++ b/array.go @@ -121,6 +121,30 @@ func NewArrayWithRootID(storage SlabStorage, rootID SlabID) (*Array, error) { // If another *Array instance for this container already exists, reuse // its shared state so structural changes propagate. state := storage.ArrayState(rootID) + + // A registered state can outlive its container: + // storage.Remove is called both when a container is inlined (still alive) + // and when it is destroyed, + // and the registry deliberately survives Remove for the inline case. + // A non-inlined root must still have its slab in storage — + // if it doesn't, the container was destroyed, + // and returning the leftover state would resurrect it as a zombie. + // An inlined root legitimately has no standalone slab, so it is not checked + // (a container destroyed WHILE inlined never had a slab to remove, + // so it cannot be detected here; its root slab ID is not referenced + // by any remaining storable, so nothing should dereference it). + if state != nil && !state.root.Inlined() { + _, found, err := storage.Retrieve(rootID) + if err != nil { + // Wrap err as external error (if needed) because err is returned by SlabStorage interface. + return nil, wrapErrorfAsExternalErrorIfNeeded(err, fmt.Sprintf("failed to retrieve slab %s", rootID)) + } + if !found { + storage.RemoveStateForSlab(rootID) + return nil, NewSlabNotFoundErrorf(rootID, "array slab not found") + } + } + if state == nil { root, err := getArraySlab(storage, rootID) if err != nil { diff --git a/array_sibling_consistency_test.go b/array_sibling_consistency_test.go index aaafbdd..b25278a 100644 --- a/array_sibling_consistency_test.go +++ b/array_sibling_consistency_test.go @@ -736,3 +736,46 @@ func TestNewArrayWithRootIDReturnsSameState(t *testing.T) { require.Equal(t, a1.Count(), a2.Count(), "a2 must observe a1's mutation through the shared state") } + +// TestNewArrayWithRootIDAfterDestroy verifies that a registered state +// does not outlive its container's destruction: +// after the root slab is removed from storage, +// NewArrayWithRootID must return SlabNotFoundError — +// not a zombie *Array served from the leftover registry state — +// and must clear the leftover state from the registry. +// +// (The registry deliberately survives storage.Remove +// because Remove is also called when a container is inlined while still alive; +// the constructors distinguish the two cases +// by checking slab existence for non-inlined roots.) +func TestNewArrayWithRootIDAfterDestroy(t *testing.T) { + + typeInfo := testutils.NewSimpleTypeInfo(42) + storage := newTestPersistentStorage(t) + address := atree.Address{1, 2, 3, 4, 5, 6, 7, 8} + + arr, err := atree.NewArray(storage, address, typeInfo) + require.NoError(t, err) + require.NoError(t, arr.Append(testutils.NewUint64ValueFromInteger(0))) + + rootID := arr.SlabID() + require.NotEqual(t, atree.SlabIDUndefined, rootID) + require.NotNil(t, storage.ArrayState(rootID), + "constructing the array must have registered its state") + + // Destroy the standalone single-slab container + // by removing its root slab. + require.NoError(t, storage.Remove(rootID)) + + _, err = atree.NewArrayWithRootID(storage, rootID) + var slabNotFoundError *atree.SlabNotFoundError + require.ErrorAs(t, err, &slabNotFoundError, + "NewArrayWithRootID on a destroyed container must return SlabNotFoundError") + + require.Nil(t, storage.ArrayState(rootID), + "detecting the destroyed container must clear the leftover registry state") + + // A second call goes down the no-state path and must fail the same way. + _, err = atree.NewArrayWithRootID(storage, rootID) + require.ErrorAs(t, err, &slabNotFoundError) +} diff --git a/map.go b/map.go index 78b7564..458dcf3 100644 --- a/map.go +++ b/map.go @@ -143,6 +143,30 @@ func NewMapWithRootID(storage SlabStorage, rootID SlabID, digestBuilder Digester // If another *OrderedMap instance for this container already exists, // reuse its shared state so structural changes propagate. state := storage.OrderedMapState(rootID) + + // A registered state can outlive its container: + // storage.Remove is called both when a container is inlined (still alive) + // and when it is destroyed, + // and the registry deliberately survives Remove for the inline case. + // A non-inlined root must still have its slab in storage — + // if it doesn't, the container was destroyed, + // and returning the leftover state would resurrect it as a zombie. + // An inlined root legitimately has no standalone slab, so it is not checked + // (a container destroyed WHILE inlined never had a slab to remove, + // so it cannot be detected here; its root slab ID is not referenced + // by any remaining storable, so nothing should dereference it). + if state != nil && !state.root.Inlined() { + _, found, err := storage.Retrieve(rootID) + if err != nil { + // Wrap err as external error (if needed) because err is returned by SlabStorage interface. + return nil, wrapErrorfAsExternalErrorIfNeeded(err, fmt.Sprintf("failed to retrieve slab %s", rootID)) + } + if !found { + storage.RemoveStateForSlab(rootID) + return nil, NewSlabNotFoundErrorf(rootID, "map slab not found") + } + } + if state == nil { root, err := getMapSlab(storage, rootID) if err != nil { diff --git a/map_sibling_consistency_test.go b/map_sibling_consistency_test.go index 2d2a691..812ec07 100644 --- a/map_sibling_consistency_test.go +++ b/map_sibling_consistency_test.go @@ -768,3 +768,47 @@ func TestNewMapWithRootIDReturnsSameState(t *testing.T) { "were re-seeded with the canonical seed and the *OrderedMap instances "+ "share the same state.root") } + +// TestNewMapWithRootIDAfterDestroy is the OrderedMap counterpart to +// TestNewArrayWithRootIDAfterDestroy: +// after the root slab is removed from storage, +// NewMapWithRootID must return SlabNotFoundError +// instead of a zombie *OrderedMap served from the leftover registry state, +// and must clear the leftover state from the registry. +func TestNewMapWithRootIDAfterDestroy(t *testing.T) { + + typeInfo := testutils.NewSimpleTypeInfo(42) + storage := newTestPersistentStorage(t) + address := atree.Address{1, 2, 3, 4, 5, 6, 7, 8} + + m, err := atree.NewMap(storage, address, atree.NewDefaultDigesterBuilder(), typeInfo) + require.NoError(t, err) + prev, err := m.Set( + testutils.CompareValue, testutils.GetHashInput, + testutils.NewUint64ValueFromInteger(0), + testutils.NewUint64ValueFromInteger(0), + ) + require.NoError(t, err) + require.Nil(t, prev) + + rootID := m.SlabID() + require.NotEqual(t, atree.SlabIDUndefined, rootID) + require.NotNil(t, storage.OrderedMapState(rootID), + "constructing the map must have registered its state") + + // Destroy the standalone single-slab container + // by removing its root slab. + require.NoError(t, storage.Remove(rootID)) + + _, err = atree.NewMapWithRootID(storage, rootID, atree.NewDefaultDigesterBuilder()) + var slabNotFoundError *atree.SlabNotFoundError + require.ErrorAs(t, err, &slabNotFoundError, + "NewMapWithRootID on a destroyed container must return SlabNotFoundError") + + require.Nil(t, storage.OrderedMapState(rootID), + "detecting the destroyed container must clear the leftover registry state") + + // A second call goes down the no-state path and must fail the same way. + _, err = atree.NewMapWithRootID(storage, rootID, atree.NewDefaultDigesterBuilder()) + require.ErrorAs(t, err, &slabNotFoundError) +} From e60baf6df8d466604ba2a0156f82882ffefc7ace Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Wed, 10 Jun 2026 12:54:42 -0700 Subject: [PATCH 12/16] restore NotValueError check in NewMapWithRootID MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When the shared-state registry was introduced, NewMapWithRootID lost the check that the slab carries extra data; its array counterpart kept it. Only root slabs carry extra data — a slab without it is an interior slab, not a value. Without the check, passing an interior slab ID silently produced a broken *OrderedMap with an unseeded digester, where it previously returned NotValueError. Worse, the bogus state was registered in the registry under that slab ID, poisoning every later lookup for it. Reject the slab before registering state, so a failed call leaves the registry untouched. The new tests cover both container types: - the map test is the regression test, - the array test pins the existing check against the same mistake. --- array_sibling_consistency_test.go | 60 +++++++++++++++++++++++++ map.go | 8 ++++ map_sibling_consistency_test.go | 73 +++++++++++++++++++++++++++++++ 3 files changed, 141 insertions(+) diff --git a/array_sibling_consistency_test.go b/array_sibling_consistency_test.go index b25278a..f9bf3f6 100644 --- a/array_sibling_consistency_test.go +++ b/array_sibling_consistency_test.go @@ -737,6 +737,66 @@ func TestNewArrayWithRootIDReturnsSameState(t *testing.T) { "a2 must observe a1's mutation through the shared state") } +// TestNewArrayWithRootIDRejectsNonRootSlabID pins the array counterpart of +// TestNewMapWithRootIDRejectsNonRootSlabID: +// NewArrayWithRootID must return NotValueError for interior (non-root) slab IDs, +// and the failed call must not register a bogus state in the registry. +func TestNewArrayWithRootIDRejectsNonRootSlabID(t *testing.T) { + + atree.SetThreshold(256) + defer atree.SetThreshold(1024) + + typeInfo := testutils.NewSimpleTypeInfo(42) + storage := newTestPersistentStorage(t) + address := atree.Address{1, 2, 3, 4, 5, 6, 7, 8} + + arr, err := atree.NewArray(storage, address, typeInfo) + require.NoError(t, err) + + // Grow the array until it spans multiple slabs, + // so interior slab IDs exist. + const count = 200 + for i := 0; i < count; i++ { + require.NoError(t, arr.Append(testutils.NewUint64ValueFromInteger(i))) + } + require.False(t, arr.IsWithinSingleSlab(), + "array must span multiple slabs so interior slab IDs exist") + + rootID := arr.SlabID() + + iterator, err := storage.SlabIterator() + require.NoError(t, err) + + interiorSlabCount := 0 + for { + id, _ := iterator() + if id == atree.SlabIDUndefined { + break + } + if id == rootID { + continue + } + interiorSlabCount++ + + // Calling twice proves the failed call did not register + // a bogus state in the registry: + // a poisoned registry would make the second call succeed. + for i := 0; i < 2; i++ { + _, err := atree.NewArrayWithRootID(storage, id) + var notValueError *atree.NotValueError + require.ErrorAs(t, err, ¬ValueError, + "NewArrayWithRootID with interior slab ID %s must return NotValueError (call %d)", id, i+1) + } + } + require.Positive(t, interiorSlabCount, + "test must have exercised at least one interior slab") + + // The real root must still work. + arr2, err := atree.NewArrayWithRootID(storage, rootID) + require.NoError(t, err) + require.Equal(t, uint64(count), arr2.Count()) +} + // TestNewArrayWithRootIDAfterDestroy verifies that a registered state // does not outlive its container's destruction: // after the root slab is removed from storage, diff --git a/map.go b/map.go index 458dcf3..102b756 100644 --- a/map.go +++ b/map.go @@ -174,6 +174,14 @@ func NewMapWithRootID(storage SlabStorage, rootID SlabID, digestBuilder Digester return nil, err } + // Only root slabs carry extra data; + // a slab without it is an interior slab, not a value. + // Reject it before registering state, + // so the registry never holds a state for a non-root slab ID. + if root.ExtraData() == nil { + return nil, NewNotValueError(rootID) + } + state = newOrderedMapState(root) storage.SetOrderedMapState(rootID, state) } diff --git a/map_sibling_consistency_test.go b/map_sibling_consistency_test.go index 812ec07..3c5f947 100644 --- a/map_sibling_consistency_test.go +++ b/map_sibling_consistency_test.go @@ -769,6 +769,79 @@ func TestNewMapWithRootIDReturnsSameState(t *testing.T) { "share the same state.root") } +// TestNewMapWithRootIDRejectsNonRootSlabID verifies that NewMapWithRootID +// returns NotValueError when given the ID of an interior (non-root) slab. +// Only root slabs carry extra data; interior slabs are not values. +// +// Regression test: when the shared-state registry was introduced, +// the extra-data check was accidentally dropped from NewMapWithRootID +// (its array counterpart kept it), +// so a non-root slab ID silently produced a broken *OrderedMap +// with an unseeded digester — +// and registered that bogus state in the registry, +// poisoning all later lookups for that slab ID. +func TestNewMapWithRootIDRejectsNonRootSlabID(t *testing.T) { + + atree.SetThreshold(256) + defer atree.SetThreshold(1024) + + typeInfo := testutils.NewSimpleTypeInfo(42) + storage := newTestPersistentStorage(t) + address := atree.Address{1, 2, 3, 4, 5, 6, 7, 8} + + m, err := atree.NewMap(storage, address, atree.NewDefaultDigesterBuilder(), typeInfo) + require.NoError(t, err) + + // Grow the map until it spans multiple slabs, + // so interior slab IDs exist. + const count = 200 + for i := 0; i < count; i++ { + prev, err := m.Set( + testutils.CompareValue, testutils.GetHashInput, + testutils.NewUint64ValueFromInteger(i), + testutils.NewUint64ValueFromInteger(i), + ) + require.NoError(t, err) + require.Nil(t, prev) + } + require.False(t, m.IsWithinSingleSlab(), + "map must span multiple slabs so interior slab IDs exist") + + rootID := m.SlabID() + + iterator, err := storage.SlabIterator() + require.NoError(t, err) + + interiorSlabCount := 0 + for { + id, _ := iterator() + if id == atree.SlabIDUndefined { + break + } + if id == rootID { + continue + } + interiorSlabCount++ + + // Calling twice proves the failed call did not register + // a bogus state in the registry: + // a poisoned registry would make the second call succeed. + for i := 0; i < 2; i++ { + _, err := atree.NewMapWithRootID(storage, id, atree.NewDefaultDigesterBuilder()) + var notValueError *atree.NotValueError + require.ErrorAs(t, err, ¬ValueError, + "NewMapWithRootID with interior slab ID %s must return NotValueError (call %d)", id, i+1) + } + } + require.Positive(t, interiorSlabCount, + "test must have exercised at least one interior slab") + + // The real root must still work. + m2, err := atree.NewMapWithRootID(storage, rootID, atree.NewDefaultDigesterBuilder()) + require.NoError(t, err) + require.Equal(t, uint64(count), m2.Count()) +} + // TestNewMapWithRootIDAfterDestroy is the OrderedMap counterpart to // TestNewArrayWithRootIDAfterDestroy: // after the root slab is removed from storage, From 6c887cb4bafb94807f5d1d886dd1d908b0d5e75c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Wed, 10 Jun 2026 12:57:19 -0700 Subject: [PATCH 13/16] clear state registry on DropDeltas DropDeltas rolls storage back to the last committed state, but registered container states point at in-memory root slabs that still reflect the discarded mutations. An *Array / *OrderedMap instance created after the rollback would be served such a state from the registry and resurrect the discarded writes. Add BaseStateRegistry.RemoveAllStates and call it from DropDeltas, so post-rollback instances re-decode the committed slabs. Note that DropDeltas alone still leaves in-place-mutated slab structs in the read cache; callers wanting a full reset to committed state pair it with DropCache (as cmd/smoke does), and the new test follows that pattern. --- state_registry.go | 26 +++++++++++++++++-- storage.go | 7 +++++ storage_test.go | 66 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 97 insertions(+), 2 deletions(-) diff --git a/state_registry.go b/state_registry.go index c336539..5627a31 100644 --- a/state_registry.go +++ b/state_registry.go @@ -40,6 +40,11 @@ type StateRegistry interface { SetArrayState(rootID SlabID, state *ArrayState) OrderedMapState(rootID SlabID) *OrderedMapState SetOrderedMapState(rootID SlabID, state *OrderedMapState) + // RemoveStateForSlab clears any registered state under the given root slab ID. + // atree calls it when it detects that a registered state + // outlived its container's destruction + // (NewArrayWithRootID / NewMapWithRootID on a destroyed container's root slab ID). + RemoveStateForSlab(rootID SlabID) } // BaseStateRegistry is an embeddable helper @@ -73,8 +78,13 @@ var _ StateRegistry = &BaseStateRegistry{} // NewBaseStateRegistry returns an empty registry. // Registry maps are lazily initialized on first use, -// so a zero-valued *BaseStateRegistry also works; -// this constructor is provided for explicitness. +// so a zero-valued BaseStateRegistry struct (i.e. &BaseStateRegistry{}) also works. +// A nil *BaseStateRegistry does NOT work: +// the methods dereference the receiver and panic. +// Embedders like BasicSlabStorage and PersistentSlabStorage embed the pointer, +// so they must be created via their constructors +// (or otherwise have the embedded pointer set) — +// a struct-literal zero value panics on first container creation. func NewBaseStateRegistry() *BaseStateRegistry { return &BaseStateRegistry{} } @@ -121,3 +131,15 @@ func (r *BaseStateRegistry) RemoveStateForSlab(rootID SlabID) { delete(r.arrayStates, rootID) delete(r.orderedMapStates, rootID) } + +// RemoveAllStates clears the entire registry. +// +// This must be called whenever in-memory container state is discarded wholesale, +// e.g. PersistentSlabStorage.DropDeltas (rollback to last commit): +// registered states point at in-memory root slabs that reflect uncommitted mutations, +// so after a rollback they would resurrect the discarded writes +// for any container instance created afterwards. +func (r *BaseStateRegistry) RemoveAllStates() { + r.arrayStates = nil + r.orderedMapStates = nil +} diff --git a/storage.go b/storage.go index 6b2d4a0..a0be206 100644 --- a/storage.go +++ b/storage.go @@ -910,6 +910,13 @@ func (s *PersistentSlabStorage) NondeterministicFastCommit(numWorkers int) error func (s *PersistentSlabStorage) DropDeltas() { s.deltas = make(map[SlabID]Slab) + + // Dropping deltas rolls storage back to the last committed state, + // but registered container states point at in-memory root slabs + // that still reflect the discarded mutations. + // Clear the registry so container instances created after the rollback + // re-decode the committed slabs instead of resurrecting discarded writes. + s.RemoveAllStates() } func (s *PersistentSlabStorage) DropCache() { diff --git a/storage_test.go b/storage_test.go index f5f4eb3..7856af8 100644 --- a/storage_test.go +++ b/storage_test.go @@ -3353,3 +3353,69 @@ func TestStorageBatchPreloadNotFoundSlabs(t *testing.T) { } }) } + +// TestPersistentStorageDropDeltasClearsContainerStates verifies that +// DropDeltas (rollback to the last commit) clears the shared-state registry. +// +// Registered container states point at in-memory root slabs that reflect +// uncommitted mutations. If the registry survived DropDeltas, any *Array / +// *OrderedMap instance created after the rollback would observe the +// discarded writes instead of the committed state. +func TestPersistentStorageDropDeltasClearsContainerStates(t *testing.T) { + + typeInfo := testutils.NewSimpleTypeInfo(42) + storage := newTestPersistentStorage(t) + address := atree.Address{1, 2, 3, 4, 5, 6, 7, 8} + + arr, err := atree.NewArray(storage, address, typeInfo) + require.NoError(t, err) + require.NoError(t, arr.Append(testutils.NewUint64ValueFromInteger(0))) + arrayRootID := arr.SlabID() + + m, err := atree.NewMap(storage, address, atree.NewDefaultDigesterBuilder(), typeInfo) + require.NoError(t, err) + prev, err := m.Set( + testutils.CompareValue, testutils.GetHashInput, + testutils.NewUint64ValueFromInteger(0), + testutils.NewUint64ValueFromInteger(0), + ) + require.NoError(t, err) + require.Nil(t, prev) + mapRootID := m.SlabID() + + // Commit the one-element state of both containers. + require.NoError(t, storage.FastCommit(1)) + + // Mutate both containers without committing. + require.NoError(t, arr.Append(testutils.NewUint64ValueFromInteger(1))) + require.Equal(t, uint64(2), arr.Count()) + + prev, err = m.Set( + testutils.CompareValue, testutils.GetHashInput, + testutils.NewUint64ValueFromInteger(1), + testutils.NewUint64ValueFromInteger(1), + ) + require.NoError(t, err) + require.Nil(t, prev) + require.Equal(t, uint64(2), m.Count()) + + // Roll back to the last commit. + // DropCache is paired with DropDeltas to also discard committed-but-cached + // slab structs, matching the full reset pattern used by cmd/smoke. + storage.DropDeltas() + storage.DropCache() + + // Fresh instances must observe the committed state, + // not the discarded in-memory mutations. + arr2, err := atree.NewArrayWithRootID(storage, arrayRootID) + require.NoError(t, err) + require.Equal(t, uint64(1), arr2.Count(), + "post-rollback array must observe the committed state — "+ + "a stale registry entry would resurrect the discarded append") + + m2, err := atree.NewMapWithRootID(storage, mapRootID, atree.NewDefaultDigesterBuilder()) + require.NoError(t, err) + require.Equal(t, uint64(1), m2.Count(), + "post-rollback map must observe the committed state — "+ + "a stale registry entry would resurrect the discarded insert") +} From 3113df49a84eaa6df1a1f55affe0984b82de97e4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Fri, 12 Jun 2026 12:17:28 -0700 Subject: [PATCH 14/16] invalidate cached mutated slabs on DropDeltas MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DropDeltas cleared deltas and the state registry, but slabs are mutated in place and the same mutated structs also sit in the read cache (commit places stored slab structs in the cache, and cached reads can be mutated afterwards). A fresh container load after the rollback would read the mutated struct from the cache and observe — and on a later commit re-persist — the supposedly discarded mutations, so DropDeltas alone was not a complete rollback and silently required pairing with DropCache. Every mutated slab has a delta entry, so invalidating the cache for all delta'd IDs makes subsequent reads decode the committed bytes again. DropDeltas alone is now a complete rollback for everything storage serves afterwards. One vector remains and cannot be closed in atree: container instances obtained before the rollback still hold their mutated in-memory roots. Document on DropDeltas that callers must discard retained container references across a rollback. The test now deliberately omits DropCache and asserts mutated slabs are no longer served from memory. --- storage.go | 26 +++++++++++++++++++++----- storage_test.go | 38 +++++++++++++++++++++++++++----------- 2 files changed, 48 insertions(+), 16 deletions(-) diff --git a/storage.go b/storage.go index a0be206..e248e6e 100644 --- a/storage.go +++ b/storage.go @@ -908,14 +908,30 @@ func (s *PersistentSlabStorage) NondeterministicFastCommit(numWorkers int) error return nil } +// DropDeltas discards all uncommitted changes, rolling storage back to the last committed state. +// +// WARNING: the rollback only affects what storage serves from now on. +// Container instances (*Array / *OrderedMap) obtained BEFORE the rollback still hold their mutated in-memory roots, +// and using them can re-introduce the discarded writes. +// Callers MUST discard all container references they hold across a rollback +// (atree cannot invalidate handles it has already returned on its own). func (s *PersistentSlabStorage) DropDeltas() { + + // Slabs are mutated in place, so a mutated slab struct can also sit in the read cache + // (commit places stored slabs in the cache, and Retrieve caches reads whose structs are mutated later). + // Dropping only the deltas would leave those mutated structs in the cache: + // subsequent reads would observe (and a later commit would re-persist) the supposedly discarded mutations. + // Every mutated slab has a delta entry, so invalidating the cache for all delta'd IDs + // makes subsequent reads decode the committed bytes again. + for id := range s.deltas { + delete(s.cache, id) + } + s.deltas = make(map[SlabID]Slab) - // Dropping deltas rolls storage back to the last committed state, - // but registered container states point at in-memory root slabs - // that still reflect the discarded mutations. - // Clear the registry so container instances created after the rollback - // re-decode the committed slabs instead of resurrecting discarded writes. + // Registered container states point at in-memory root slabs that still reflect the discarded mutations. + // Clear the registry so container instances created after the rollback re-decode the committed slabs + // instead of resurrecting discarded writes. s.RemoveAllStates() } diff --git a/storage_test.go b/storage_test.go index 7856af8..299e8b6 100644 --- a/storage_test.go +++ b/storage_test.go @@ -3354,13 +3354,22 @@ func TestStorageBatchPreloadNotFoundSlabs(t *testing.T) { }) } -// TestPersistentStorageDropDeltasClearsContainerStates verifies that -// DropDeltas (rollback to the last commit) clears the shared-state registry. +// TestPersistentStorageDropDeltasClearsContainerStates verifies +// that DropDeltas ALONE is a complete rollback to the last commit +// for everything storage serves afterwards. // -// Registered container states point at in-memory root slabs that reflect -// uncommitted mutations. If the registry survived DropDeltas, any *Array / -// *OrderedMap instance created after the rollback would observe the -// discarded writes instead of the committed state. +// Two stale sources must both be invalidated: +// - the shared-state registry: registered states point at +// in-memory root slabs that reflect the discarded mutations; +// - the read cache: slabs are mutated in place, +// and commit places stored slab structs in the cache, +// so the cache holds the same mutated structs the deltas did. +// +// If either survived, a fresh container instance created after the rollback +// would observe — and a later commit would re-persist — the discarded writes. +// +// (Container instances obtained BEFORE the rollback still hold their mutated +// roots; discarding those is the caller's responsibility, see DropDeltas doc.) func TestPersistentStorageDropDeltasClearsContainerStates(t *testing.T) { typeInfo := testutils.NewSimpleTypeInfo(42) @@ -3400,10 +3409,17 @@ func TestPersistentStorageDropDeltasClearsContainerStates(t *testing.T) { require.Equal(t, uint64(2), m.Count()) // Roll back to the last commit. - // DropCache is paired with DropDeltas to also discard committed-but-cached - // slab structs, matching the full reset pattern used by cmd/smoke. + // Deliberately NOT paired with DropCache: + // DropDeltas itself must invalidate the cache entries of mutated slabs, + // otherwise the state == nil load path below would read the in-place-mutated + // slab structs from the cache and observe the discarded mutations. storage.DropDeltas() - storage.DropCache() + + // The mutated root slabs must no longer be served from memory. + require.Nil(t, storage.RetrieveIfLoaded(arrayRootID), + "DropDeltas must invalidate the cache entry of a mutated slab") + require.Nil(t, storage.RetrieveIfLoaded(mapRootID), + "DropDeltas must invalidate the cache entry of a mutated slab") // Fresh instances must observe the committed state, // not the discarded in-memory mutations. @@ -3411,11 +3427,11 @@ func TestPersistentStorageDropDeltasClearsContainerStates(t *testing.T) { require.NoError(t, err) require.Equal(t, uint64(1), arr2.Count(), "post-rollback array must observe the committed state — "+ - "a stale registry entry would resurrect the discarded append") + "a stale registry entry or cache entry would resurrect the discarded append") m2, err := atree.NewMapWithRootID(storage, mapRootID, atree.NewDefaultDigesterBuilder()) require.NoError(t, err) require.Equal(t, uint64(1), m2.Count(), "post-rollback map must observe the committed state — "+ - "a stale registry entry would resurrect the discarded insert") + "a stale registry entry or cache entry would resurrect the discarded insert") } From d3e52943760c199c8387de5eeb3eeb062ff245e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Fri, 26 Jun 2026 11:32:01 -0700 Subject: [PATCH 15/16] reject root-ID access to inlined containers NewArrayWithRootID and NewMapWithRootID only prove access to standalone root slabs. After a container is inlined into its parent, the old root ID no longer names a live standalone slab, even though the shared-state registry must keep the state alive for parent-loaded siblings. Reject constructor calls that hit an inlined registered state without clearing that state. Also mark root-ID-loaded wrappers and reject later mutations through them if another sibling has since inlined the container and the wrapper has no parent updater. This prevents silent in-memory mutations that cannot be written back to the parent while leaving normal parent-loaded and standalone Cadence paths intact. Add array and map regressions covering existing root-ID handles, fresh root-ID loads after inline, registry retention, and continued mutation through the parent-loaded handle. --- array.go | 64 +++++++++++++++-- array_sibling_consistency_test.go | 73 ++++++++++++++++++++ map.go | 59 ++++++++++++++-- map_sibling_consistency_test.go | 110 ++++++++++++++++++++++++++++++ 4 files changed, 298 insertions(+), 8 deletions(-) diff --git a/array.go b/array.go index 9b51501..e73aeee 100644 --- a/array.go +++ b/array.go @@ -72,6 +72,13 @@ type Array struct { // use this to avoid promoting a trap-bearing instance to a shared/canonical wrapper: // mutations through such a wrapper would trip the trap. parentUpdaterIsReadOnlyMutationCallback bool + + // loadedWithRootID is true when this wrapper was created by NewArrayWithRootID. + // If such a wrapper later observes an inlined state, + // it cannot safely mutate without a parentUpdater: + // root-ID loading only proves access to a standalone root, + // while an inlined value needs a parent write-back path. + loadedWithRootID bool } var _ Value = &Array{} @@ -122,6 +129,16 @@ func NewArrayWithRootID(storage SlabStorage, rootID SlabID) (*Array, error) { // its shared state so structural changes propagate. state := storage.ArrayState(rootID) + // NewArrayWithRootID only loads standalone roots. + // If the registry says this logical container is currently inlined, + // rootID is no longer a live standalone slab ID. + // Do not clear the state here: + // the inlined container may still be alive inside its parent, + // and dropping the registry would reintroduce sibling divergence. + if state != nil && state.root.Inlined() { + return nil, NewSlabNotFoundErrorf(rootID, "array slab is inlined") + } + // A registered state can outlive its container: // storage.Remove is called both when a container is inlined (still alive) // and when it is destroyed, @@ -162,8 +179,9 @@ func NewArrayWithRootID(storage SlabStorage, rootID SlabID) (*Array, error) { } return &Array{ - Storage: storage, - state: state, + Storage: storage, + state: state, + loadedWithRootID: true, }, nil } @@ -449,6 +467,11 @@ func (a *Array) Set(index uint64, value Value) (Storable, error) { } func (a *Array) set(index uint64, value Value) (Storable, error) { + err := a.ensureRootIDLoadedInlinedMutationAllowed() + if err != nil { + return nil, err + } + existingStorable, err := a.state.root.Set(a.Storage, a.Address(), index, value) if err != nil { // Don't need to wrap error as external error because err is already categorized by ArraySlab.Set(). @@ -512,7 +535,12 @@ func (a *Array) Insert(index uint64, value Value) error { return NewArrayElementCannotExceedMaxElementCountError(maxArrayElementCount) } - err := a.state.root.Insert(a.Storage, a.Address(), index, value) + err := a.ensureRootIDLoadedInlinedMutationAllowed() + if err != nil { + return err + } + + err = a.state.root.Insert(a.Storage, a.Address(), index, value) if err != nil { // Don't need to wrap error as external error because err is already categorized by ArraySlab.Insert(). return err @@ -581,6 +609,11 @@ func (a *Array) Remove(index uint64) (Storable, error) { } func (a *Array) remove(index uint64) (Storable, error) { + err := a.ensureRootIDLoadedInlinedMutationAllowed() + if err != nil { + return nil, err + } + storable, err := a.state.root.Remove(a.Storage, index) if err != nil { // Don't need to wrap error as external error because err is already categorized by ArraySlab.Remove(). @@ -620,7 +653,12 @@ type ArrayPopIterationFunc func(Storable) // Each element is passed to ArrayPopIterationFunc callback before removal. func (a *Array) PopIterate(fn ArrayPopIterationFunc) error { - err := a.state.root.PopIterate(a.Storage, fn) + err := a.ensureRootIDLoadedInlinedMutationAllowed() + if err != nil { + return err + } + + err = a.state.root.PopIterate(a.Storage, fn) if err != nil { // Don't need to wrap error as external error because err is already categorized by ArraySlab.PopIterate(). return err @@ -997,6 +1035,19 @@ func (a *Array) notifyParentIfNeeded() error { return nil } +func (a *Array) ensureRootIDLoadedInlinedMutationAllowed() error { + if !a.loadedWithRootID || !a.state.root.Inlined() || a.parentUpdater != nil { + return nil + } + + return NewFatalError( + fmt.Errorf( + "cannot mutate inlined array %s loaded by root ID without parent updater", + a.ValueID(), + ), + ) +} + func (a *Array) Inlined() bool { return a.state.root.Inlined() } @@ -1439,6 +1490,11 @@ func (a *Array) Type() TypeInfo { } func (a *Array) SetType(typeInfo TypeInfo) error { + err := a.ensureRootIDLoadedInlinedMutationAllowed() + if err != nil { + return err + } + extraData := a.state.root.ExtraData() extraData.TypeInfo = typeInfo diff --git a/array_sibling_consistency_test.go b/array_sibling_consistency_test.go index f9bf3f6..9a5d558 100644 --- a/array_sibling_consistency_test.go +++ b/array_sibling_consistency_test.go @@ -839,3 +839,76 @@ func TestNewArrayWithRootIDAfterDestroy(t *testing.T) { _, err = atree.NewArrayWithRootID(storage, rootID) require.ErrorAs(t, err, &slabNotFoundError) } + +// TestNewArrayWithRootIDRejectsCurrentlyInlinedState verifies that +// NewArrayWithRootID only serves standalone roots. +// +// Once a child array is inlined into its parent, +// its old root slab ID no longer names a live standalone slab. +// A root-ID load must fail instead of returning a no-parent handle +// for an in-parent value. +// +// The registry entry must still survive: +// the inlined child is alive inside its parent, +// and parent-loaded siblings must keep sharing the same state. +func TestNewArrayWithRootIDRejectsCurrentlyInlinedState(t *testing.T) { + + atree.SetThreshold(256) + defer atree.SetThreshold(1024) + + typeInfo := testutils.NewSimpleTypeInfo(42) + storage := newTestPersistentStorage(t) + address := atree.Address{1, 2, 3, 4, 5, 6, 7, 8} + + outer, err := atree.NewArray(storage, address, typeInfo) + require.NoError(t, err) + + inner, err := atree.NewArray(storage, address, typeInfo) + require.NoError(t, err) + + for i := uint64(0); i < 100; i++ { + require.NoError(t, inner.Append(testutils.NewUint64ValueFromInteger(int(i)))) + } + require.False(t, inner.Inlined(), + "inner must start as a standalone root") + + rootID := inner.SlabID() + require.NotEqual(t, atree.SlabIDUndefined, rootID) + + direct, err := atree.NewArrayWithRootID(storage, rootID) + require.NoError(t, err) + require.False(t, direct.HasParentUpdater()) + + require.NoError(t, outer.Append(inner)) + + v, err := outer.Get(0) + require.NoError(t, err) + fromParent := v.(*atree.Array) + require.True(t, fromParent.HasParentUpdater()) + + for fromParent.Count() > 0 && !fromParent.Inlined() { + _, err := fromParent.Remove(fromParent.Count() - 1) + require.NoError(t, err) + } + require.True(t, fromParent.Inlined(), + "inner must be inlined to exercise the root-ID rejection") + require.True(t, direct.Inlined(), + "root-ID-loaded sibling must observe the inline transition") + + err = direct.Append(testutils.NewUint64ValueFromInteger(41)) + var fatalError *atree.FatalError + require.ErrorAs(t, err, &fatalError, + "root-ID-loaded inlined sibling must not mutate without a parent updater") + + _, err = atree.NewArrayWithRootID(storage, rootID) + var slabNotFoundError *atree.SlabNotFoundError + require.ErrorAs(t, err, &slabNotFoundError) + + require.NotNil(t, storage.ArrayState(rootID), + "failed root-ID load must not clear live inlined state") + + count := fromParent.Count() + require.NoError(t, fromParent.Append(testutils.NewUint64ValueFromInteger(42))) + require.Equal(t, uint64(1), outer.Count()) + require.Equal(t, count+1, fromParent.Count()) +} diff --git a/map.go b/map.go index 102b756..e88e51e 100644 --- a/map.go +++ b/map.go @@ -72,6 +72,13 @@ type OrderedMap struct { // read-only iterator's trap callback rather than a real parent-notification callback. // See Array.parentUpdaterIsReadOnlyMutationCallback for rationale. parentUpdaterIsReadOnlyMutationCallback bool + + // loadedWithRootID is true when this wrapper was created by NewMapWithRootID. + // If such a wrapper later observes an inlined state, + // it cannot safely mutate without a parentUpdater: + // root-ID loading only proves access to a standalone root, + // while an inlined value needs a parent write-back path. + loadedWithRootID bool } var _ Value = &OrderedMap{} @@ -144,6 +151,16 @@ func NewMapWithRootID(storage SlabStorage, rootID SlabID, digestBuilder Digester // reuse its shared state so structural changes propagate. state := storage.OrderedMapState(rootID) + // NewMapWithRootID only loads standalone roots. + // If the registry says this logical container is currently inlined, + // rootID is no longer a live standalone slab ID. + // Do not clear the state here: + // the inlined container may still be alive inside its parent, + // and dropping the registry would reintroduce sibling divergence. + if state != nil && state.root.Inlined() { + return nil, NewSlabNotFoundErrorf(rootID, "map slab is inlined") + } + // A registered state can outlive its container: // storage.Remove is called both when a container is inlined (still alive) // and when it is destroyed, @@ -193,9 +210,10 @@ func NewMapWithRootID(storage SlabStorage, rootID SlabID, digestBuilder Digester } return &OrderedMap{ - Storage: storage, - state: state, - digesterBuilder: digestBuilder, + Storage: storage, + state: state, + digesterBuilder: digestBuilder, + loadedWithRootID: true, }, nil } @@ -698,6 +716,11 @@ func (m *OrderedMap) Set(comparator ValueComparator, hip HashInputProvider, key func (m *OrderedMap) set(comparator ValueComparator, hip HashInputProvider, key Value, value Value) (Storable, error) { + err := m.ensureRootIDLoadedInlinedMutationAllowed() + if err != nil { + return nil, err + } + keyDigest, err := m.digesterBuilder.Digest(hip, key) if err != nil { // Wrap err as external error (if needed) because err is returned by DigesterBuilder interface. @@ -796,6 +819,11 @@ func (m *OrderedMap) Remove(comparator ValueComparator, hip HashInputProvider, k func (m *OrderedMap) remove(comparator ValueComparator, hip HashInputProvider, key Value) (Storable, Storable, error) { + err := m.ensureRootIDLoadedInlinedMutationAllowed() + if err != nil { + return nil, nil, err + } + keyDigest, err := m.digesterBuilder.Digest(hip, key) if err != nil { // Wrap err as external error (if needed) because err is returned by DigesterBuilder interface. @@ -855,7 +883,12 @@ type MapPopIterationFunc func(Storable, Storable) // Each element is passed to MapPopIterationFunc callback before removal. func (m *OrderedMap) PopIterate(fn MapPopIterationFunc) error { - err := m.state.root.PopIterate(m.Storage, fn) + err := m.ensureRootIDLoadedInlinedMutationAllowed() + if err != nil { + return err + } + + err = m.state.root.PopIterate(m.Storage, fn) if err != nil { // Don't need to wrap error as external error because err is already categorized by MapSlab.PopIterate(). return err @@ -1200,6 +1233,19 @@ func (m *OrderedMap) notifyParentIfNeeded() error { return nil } +func (m *OrderedMap) ensureRootIDLoadedInlinedMutationAllowed() error { + if !m.loadedWithRootID || !m.state.root.Inlined() || m.parentUpdater != nil { + return nil + } + + return NewFatalError( + fmt.Errorf( + "cannot mutate inlined map %s loaded by root ID without parent updater", + m.ValueID(), + ), + ) +} + // Value operations // Storable returns OrderedMap m as either: @@ -1572,6 +1618,11 @@ func (m *OrderedMap) Type() TypeInfo { } func (m *OrderedMap) SetType(typeInfo TypeInfo) error { + err := m.ensureRootIDLoadedInlinedMutationAllowed() + if err != nil { + return err + } + extraData := m.state.root.ExtraData() extraData.TypeInfo = typeInfo diff --git a/map_sibling_consistency_test.go b/map_sibling_consistency_test.go index 3c5f947..e913f4f 100644 --- a/map_sibling_consistency_test.go +++ b/map_sibling_consistency_test.go @@ -885,3 +885,113 @@ func TestNewMapWithRootIDAfterDestroy(t *testing.T) { _, err = atree.NewMapWithRootID(storage, rootID, atree.NewDefaultDigesterBuilder()) require.ErrorAs(t, err, &slabNotFoundError) } + +// TestNewMapWithRootIDRejectsCurrentlyInlinedState verifies that +// NewMapWithRootID only serves standalone roots. +// +// Once a child map is inlined into its parent, +// its old root slab ID no longer names a live standalone slab. +// A root-ID load must fail instead of returning a no-parent handle +// for an in-parent value. +// +// The registry entry must still survive: +// the inlined child is alive inside its parent, +// and parent-loaded siblings must keep sharing the same state. +func TestNewMapWithRootIDRejectsCurrentlyInlinedState(t *testing.T) { + + atree.SetThreshold(256) + defer atree.SetThreshold(1024) + + typeInfo := testutils.NewSimpleTypeInfo(42) + storage := newTestPersistentStorage(t) + address := atree.Address{1, 2, 3, 4, 5, 6, 7, 8} + + outer, err := atree.NewMap( + storage, + address, + atree.NewDefaultDigesterBuilder(), + typeInfo, + ) + require.NoError(t, err) + + inner, err := atree.NewMap( + storage, + address, + atree.NewDefaultDigesterBuilder(), + typeInfo, + ) + require.NoError(t, err) + + for i := uint64(0); i < 100; i++ { + k := testutils.NewUint64ValueFromInteger(int(i)) + v := testutils.NewUint64ValueFromInteger(int(i)) + prev, err := inner.Set(testutils.CompareValue, testutils.GetHashInput, k, v) + require.NoError(t, err) + require.Nil(t, prev) + } + require.False(t, inner.Inlined(), + "inner must start as a standalone root") + + rootID := inner.SlabID() + require.NotEqual(t, atree.SlabIDUndefined, rootID) + + direct, err := atree.NewMapWithRootID( + storage, + rootID, + atree.NewDefaultDigesterBuilder(), + ) + require.NoError(t, err) + require.False(t, direct.HasParentUpdater()) + + outerKey := testutils.NewUint64ValueFromInteger(0) + prev, err := outer.Set( + testutils.CompareValue, + testutils.GetHashInput, + outerKey, + inner, + ) + require.NoError(t, err) + require.Nil(t, prev) + + v, err := outer.Get(testutils.CompareValue, testutils.GetHashInput, outerKey) + require.NoError(t, err) + fromParent := v.(*atree.OrderedMap) + require.True(t, fromParent.HasParentUpdater()) + + for fromParent.Count() > 0 && !fromParent.Inlined() { + k := testutils.NewUint64ValueFromInteger(int(fromParent.Count() - 1)) + _, _, err := fromParent.Remove(testutils.CompareValue, testutils.GetHashInput, k) + require.NoError(t, err) + } + require.True(t, fromParent.Inlined(), + "inner must be inlined to exercise the root-ID rejection") + require.True(t, direct.Inlined(), + "root-ID-loaded sibling must observe the inline transition") + + k := testutils.NewUint64ValueFromInteger(999) + value := testutils.NewUint64ValueFromInteger(999) + prev, err = direct.Set(testutils.CompareValue, testutils.GetHashInput, k, value) + var fatalError *atree.FatalError + require.ErrorAs(t, err, &fatalError, + "root-ID-loaded inlined sibling must not mutate without a parent updater") + require.Nil(t, prev) + + _, err = atree.NewMapWithRootID( + storage, + rootID, + atree.NewDefaultDigesterBuilder(), + ) + var slabNotFoundError *atree.SlabNotFoundError + require.ErrorAs(t, err, &slabNotFoundError) + + require.NotNil(t, storage.OrderedMapState(rootID), + "failed root-ID load must not clear live inlined state") + + count := fromParent.Count() + k = testutils.NewUint64ValueFromInteger(1000) + value = testutils.NewUint64ValueFromInteger(1000) + prev, err = fromParent.Set(testutils.CompareValue, testutils.GetHashInput, k, value) + require.NoError(t, err) + require.Nil(t, prev) + require.Equal(t, count+1, fromParent.Count()) +} From 0ad3c29fa427ad7345afa22d7758c19785063ab1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Mon, 29 Jun 2026 09:14:37 -0700 Subject: [PATCH 16/16] add docstrings for ensureRootIDLoadedInlinedMutationAllowed Co-authored-by: Leo Zhang --- array.go | 11 +++++++++++ map.go | 12 ++++++++++++ 2 files changed, 23 insertions(+) diff --git a/array.go b/array.go index e73aeee..2c44d77 100644 --- a/array.go +++ b/array.go @@ -1035,6 +1035,17 @@ func (a *Array) notifyParentIfNeeded() error { return nil } +// ensureRootIDLoadedInlinedMutationAllowed rejects mutations through a wrapper +// that was created by NewArrayWithRootID but whose container has since been +// inlined into a parent, when this wrapper has no parentUpdater to write the +// change back. Such a mutation would only change in-memory state that can never +// be persisted into the parent, silently diverging memory from storage. +// +// INVARIANT: every method that mutates the array's elements, structure, or +// extra data (currently set, Insert, remove, PopIterate, and SetType) MUST call +// this and return early on error before touching a.state.root. The protection +// only holds if all mutation entry points are guarded; any new mutating method +// that skips this check silently reopens the divergence hole described above. func (a *Array) ensureRootIDLoadedInlinedMutationAllowed() error { if !a.loadedWithRootID || !a.state.root.Inlined() || a.parentUpdater != nil { return nil diff --git a/map.go b/map.go index e88e51e..86c1f43 100644 --- a/map.go +++ b/map.go @@ -1233,6 +1233,18 @@ func (m *OrderedMap) notifyParentIfNeeded() error { return nil } +// ensureRootIDLoadedInlinedMutationAllowed rejects mutations through a wrapper +// that was created by NewMapWithRootID but whose container has since been +// inlined into a parent, when this wrapper has no parentUpdater to write the +// change back. Such a mutation would only change in-memory state that can never +// be persisted into the parent, silently diverging memory from storage. +// +// INVARIANT: every method that mutates the map's elements, structure, or extra +// data (currently set, remove, PopIterate, and SetType) MUST call this and +// return early on error before touching m.state.root. The protection only holds +// if all mutation entry points are guarded; any new mutating method that skips +// this check silently reopens the divergence hole described above. + func (m *OrderedMap) ensureRootIDLoadedInlinedMutationAllowed() error { if !m.loadedWithRootID || !m.state.root.Inlined() || m.parentUpdater != nil { return nil