From 042eb686fc19f48fe3e78a69b3cbe6de979a4001 Mon Sep 17 00:00:00 2001 From: Faye Amacker <33205765+fxamacker@users.noreply.github.com> Date: Sun, 1 Oct 2023 18:51:29 -0500 Subject: [PATCH] Uninline slab when it is overwritten or removed Currently, Set() and Remove() return overwritten or removed storable. If storable is inlined slab, it is not stored in storage (because it is removed from its parent slab which is in storage), so any future changes to it would be lost. On the other hand, if overwritten or removed storable is SlabIDStorable, any future changes to it can still be persisted because it is in its own slab in storage. This inconsistency (not merged or deployed yet) can cause potential data loss or unexpected behavior if deployed. This commit uninlines inlined slabs that is overwritten or removed, stores them in storage, and returns their SlabID as storable to caller. --- array.go | 146 +++++++++++--- array_test.go | 478 +++++++++++++++++++++++++++++++++++++++++++++ map.go | 132 ++++++++++--- map_test.go | 528 ++++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 1229 insertions(+), 55 deletions(-) diff --git a/array.go b/array.go index 841c38b6..55ddb93a 100644 --- a/array.go +++ b/array.go @@ -906,6 +906,56 @@ func (a *ArrayDataSlab) Inlinable(maxInlineSize uint64) bool { return uint64(inlinedSize) <= maxInlineSize } +// inline converts not-inlined ArrayDataSlab to inlined ArrayDataSlab and removes it from storage. +func (a *ArrayDataSlab) inline(storage SlabStorage) error { + if a.inlined { + return NewFatalError(fmt.Errorf("failed to inline ArrayDataSlab %s: it is inlined already", a.header.slabID)) + } + + id := a.header.slabID + + // Remove slab from storage because it is going to be inlined. + err := storage.Remove(id) + if err != nil { + // Wrap err as external error (if needed) because err is returned by SlabStorage interface. + return wrapErrorfAsExternalErrorIfNeeded(err, fmt.Sprintf("failed to remove slab %s", id)) + } + + // Update data slab size as inlined slab. + a.header.size = a.header.size - + arrayRootDataSlabPrefixSize + + inlinedArrayDataSlabPrefixSize + + // Update data slab inlined status. + a.inlined = true + + return nil +} + +// uninline converts an inlined ArrayDataSlab to uninlined ArrayDataSlab and stores it in storage. +func (a *ArrayDataSlab) uninline(storage SlabStorage) error { + if !a.inlined { + return NewFatalError(fmt.Errorf("failed to un-inline ArrayDataSlab %s: it is not inlined", a.header.slabID)) + } + + // Update data slab size + a.header.size = a.header.size - + inlinedArrayDataSlabPrefixSize + + arrayRootDataSlabPrefixSize + + // Update data slab inlined status + a.inlined = false + + // Store slab in storage + err := storage.Store(a.header.slabID, a) + if err != nil { + // Wrap err as external error (if needed) because err is returned by SlabStorage interface. + return wrapErrorfAsExternalErrorIfNeeded(err, fmt.Sprintf("failed to store slab %s", a.header.slabID)) + } + + return nil +} + func (a *ArrayDataSlab) hasPointer() bool { for _, e := range a.elements { if hasPointer(e) { @@ -2740,7 +2790,7 @@ func (a *Array) setCallbackWithChild(i uint64, child Value, maxInlineSize uint64 // Set child value with parent array using updated index. // Set() calls c.Storable() which returns inlined or not-inlined child storable. - existingValueStorable, err := a.Set(index, c) + existingValueStorable, err := a.set(index, c) if err != nil { return err } @@ -2813,6 +2863,34 @@ func (a *Array) Get(i uint64) (Value, error) { } func (a *Array) Set(index uint64, value Value) (Storable, error) { + existingStorable, err := a.set(index, value) + if err != nil { + return nil, err + } + + // If overwritten storable is an inlined slab, uninline the slab and store it in storage. + // This is to prevent potential data loss because the overwritten inlined slab was not in + // storage and any future changes to it would have been lost. + switch s := existingStorable.(type) { + case *ArrayDataSlab: + err = s.uninline(a.Storage) + if err != nil { + return nil, err + } + existingStorable = SlabIDStorable(s.header.slabID) + + case *MapDataSlab: + err = s.uninline(a.Storage) + if err != nil { + return nil, err + } + existingStorable = SlabIDStorable(s.header.slabID) + } + + return existingStorable, nil +} + +func (a *Array) set(index uint64, value Value) (Storable, error) { existingStorable, err := a.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(). @@ -2825,7 +2903,6 @@ func (a *Array) Set(index uint64, value Value) (Storable, error) { // Don't need to wrap error as external error because err is already categorized by Array.splitRoot(). return nil, err } - return existingStorable, nil } if !a.root.IsData() { @@ -2879,8 +2956,11 @@ func (a *Array) Insert(index uint64, value Value) error { } if a.root.IsFull() { - // Don't need to wrap error as external error because err is already categorized by Array.splitRoot(). - return a.splitRoot() + err = a.splitRoot() + if err != nil { + // Don't need to wrap error as external error because err is already categorized by Array.splitRoot(). + return err + } } a.incrementIndexFrom(index) @@ -2913,6 +2993,34 @@ func (a *Array) Insert(index uint64, value Value) error { } func (a *Array) Remove(index uint64) (Storable, error) { + storable, err := a.remove(index) + if err != nil { + return nil, err + } + + // If overwritten storable is an inlined slab, uninline the slab and store it in storage. + // This is to prevent potential data loss because the overwritten inlined slab was not in + // storage and any future changes to it would have been lost. + switch s := storable.(type) { + case *ArrayDataSlab: + err = s.uninline(a.Storage) + if err != nil { + return nil, err + } + storable = SlabIDStorable(s.header.slabID) + + case *MapDataSlab: + err = s.uninline(a.Storage) + if err != nil { + return nil, err + } + storable = SlabIDStorable(s.header.slabID) + } + + return storable, nil +} + +func (a *Array) remove(index uint64) (Storable, error) { storable, err := a.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(). @@ -3088,23 +3196,11 @@ func (a *Array) Storable(_ SlabStorage, _ Address, maxInlineSize uint64) (Storab return nil, NewFatalError(fmt.Errorf("unexpected inlinable array slab type %T", a.root)) } - rootID := rootDataSlab.header.slabID - - // Remove root slab from storage because it is going to be inlined. - err := a.Storage.Remove(rootID) + err := rootDataSlab.inline(a.Storage) 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 remove slab %s", rootID)) + return nil, err } - // Update root data slab size as inlined slab. - rootDataSlab.header.size = rootDataSlab.header.size - - arrayRootDataSlabPrefixSize + - inlinedArrayDataSlabPrefixSize - - // Update root data slab inlined status. - rootDataSlab.inlined = true - return rootDataSlab, nil case !inlinable && inlined: @@ -3119,19 +3215,9 @@ func (a *Array) Storable(_ SlabStorage, _ Address, maxInlineSize uint64) (Storab return nil, NewFatalError(fmt.Errorf("unexpected inlined array slab type %T", a.root)) } - // Update root data slab size - rootDataSlab.header.size = rootDataSlab.header.size - - inlinedArrayDataSlabPrefixSize + - arrayRootDataSlabPrefixSize - - // Update root data slab inlined status. - rootDataSlab.inlined = false - - // Store root slab in storage - err := a.Storage.Store(rootDataSlab.header.slabID, a.root) + err := rootDataSlab.uninline(a.Storage) 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 store slab %s", a.SlabID())) + return nil, err } return SlabIDStorable(a.SlabID()), nil diff --git a/array_test.go b/array_test.go index 4bd1b6fa..fd7c8d44 100644 --- a/array_test.go +++ b/array_test.go @@ -6520,3 +6520,481 @@ func getStoredDeltas(storage *PersistentSlabStorage) int { } return count } + +func TestArraySetReturnedValue(t *testing.T) { + typeInfo := testTypeInfo{42} + address := Address{1, 2, 3, 4, 5, 6, 7, 8} + + t.Run("child array is not inlined", func(t *testing.T) { + const arraySize = 2 + + storage := newTestPersistentStorage(t) + + // Create parent array + parentArray, err := NewArray(storage, address, typeInfo) + require.NoError(t, err) + + var expectedValues arrayValue + + for i := 0; i < arraySize; i++ { + // Create child array + childArray, err := NewArray(storage, address, typeInfo) + require.NoError(t, err) + + err = parentArray.Append(childArray) + require.NoError(t, err) + + var expectedChildValues arrayValue + for { + v := NewStringValue(strings.Repeat("a", 10)) + + err = childArray.Append(v) + require.NoError(t, err) + + expectedChildValues = append(expectedChildValues, v) + + if !childArray.Inlined() { + break + } + } + + expectedValues = append(expectedValues, expectedChildValues) + } + + verifyArray(t, storage, typeInfo, address, parentArray, expectedValues, true) + + // Overwrite existing child array value + for i := 0; i < arraySize; i++ { + existingStorable, err := parentArray.Set(uint64(i), Uint64Value(0)) + require.NoError(t, err) + require.NotNil(t, existingStorable) + + id, ok := existingStorable.(SlabIDStorable) + require.True(t, ok) + + child, err := id.StoredValue(storage) + require.NoError(t, err) + + valueEqual(t, expectedValues[i], child) + + err = storage.Remove(SlabID(id)) + require.NoError(t, err) + + expectedValues[i] = Uint64Value(0) + } + + verifyArray(t, storage, typeInfo, address, parentArray, expectedValues, true) + }) + + t.Run("child array is inlined", func(t *testing.T) { + const arraySize = 2 + + storage := newTestPersistentStorage(t) + + // Create parent array + parentArray, err := NewArray(storage, address, typeInfo) + require.NoError(t, err) + + var expectedValues arrayValue + + for i := 0; i < arraySize; i++ { + // Create child array + childArray, err := NewArray(storage, address, typeInfo) + require.NoError(t, err) + + err = parentArray.Append(childArray) + require.NoError(t, err) + + // Insert one element to child array + v := NewStringValue(strings.Repeat("a", 10)) + + err = childArray.Append(v) + require.NoError(t, err) + require.True(t, childArray.Inlined()) + + expectedValues = append(expectedValues, arrayValue{v}) + } + + verifyArray(t, storage, typeInfo, address, parentArray, expectedValues, true) + + // Overwrite existing child array value + for i := 0; i < arraySize; i++ { + existingStorable, err := parentArray.Set(uint64(i), Uint64Value(0)) + require.NoError(t, err) + require.NotNil(t, existingStorable) + + id, ok := existingStorable.(SlabIDStorable) + require.True(t, ok) + + child, err := id.StoredValue(storage) + require.NoError(t, err) + + valueEqual(t, expectedValues[i], child) + + expectedValues[i] = Uint64Value(0) + + err = storage.Remove(SlabID(id)) + require.NoError(t, err) + } + + verifyArray(t, storage, typeInfo, address, parentArray, expectedValues, true) + }) + + t.Run("child map is not inlined", func(t *testing.T) { + const arraySize = 2 + + storage := newTestPersistentStorage(t) + + // Create parent array + parentArray, err := NewArray(storage, address, typeInfo) + require.NoError(t, err) + + var expectedValues arrayValue + + for i := 0; i < arraySize; i++ { + // Create child map + childMap, err := NewMap(storage, address, newBasicDigesterBuilder(), typeInfo) + require.NoError(t, err) + + err = parentArray.Append(childMap) + require.NoError(t, err) + + expectedChildValues := make(mapValue) + expectedValues = append(expectedValues, expectedChildValues) + + // Insert into child map until child map is not inlined + j := 0 + for { + k := Uint64Value(j) + v := NewStringValue(strings.Repeat("a", 10)) + j++ + + existingStorable, err := childMap.Set(compare, hashInputProvider, k, v) + require.NoError(t, err) + require.Nil(t, existingStorable) + + expectedChildValues[k] = v + + if !childMap.Inlined() { + break + } + } + } + + verifyArray(t, storage, typeInfo, address, parentArray, expectedValues, true) + + // Overwrite existing child map value + for i := 0; i < arraySize; i++ { + existingStorable, err := parentArray.Set(uint64(i), Uint64Value(0)) + require.NoError(t, err) + require.NotNil(t, existingStorable) + + id, ok := existingStorable.(SlabIDStorable) + require.True(t, ok) + + child, err := id.StoredValue(storage) + require.NoError(t, err) + + valueEqual(t, expectedValues[i], child) + + expectedValues[i] = Uint64Value(0) + + err = storage.Remove(SlabID(id)) + require.NoError(t, err) + } + + verifyArray(t, storage, typeInfo, address, parentArray, expectedValues, true) + }) + + t.Run("child map is inlined", func(t *testing.T) { + const arraySize = 2 + + storage := newTestPersistentStorage(t) + + // Create parent array + parentArray, err := NewArray(storage, address, typeInfo) + require.NoError(t, err) + + var expectedValues arrayValue + + for i := 0; i < arraySize; i++ { + // Create child map + childMap, err := NewMap(storage, address, newBasicDigesterBuilder(), typeInfo) + require.NoError(t, err) + + k := Uint64Value(i) + + err = parentArray.Append(childMap) + require.NoError(t, err) + + expectedChildValues := make(mapValue) + expectedValues = append(expectedValues, expectedChildValues) + + // Insert into child map until child map is not inlined + v := NewStringValue(strings.Repeat("a", 10)) + + existingStorable, err := childMap.Set(compare, hashInputProvider, k, v) + require.NoError(t, err) + require.Nil(t, existingStorable) + + expectedChildValues[k] = v + } + + verifyArray(t, storage, typeInfo, address, parentArray, expectedValues, true) + + // Overwrite existing child map value + for i := 0; i < arraySize; i++ { + existingStorable, err := parentArray.Set(uint64(i), Uint64Value(0)) + require.NoError(t, err) + require.NotNil(t, existingStorable) + + id, ok := existingStorable.(SlabIDStorable) + require.True(t, ok) + + child, err := id.StoredValue(storage) + require.NoError(t, err) + + valueEqual(t, expectedValues[i], child) + + expectedValues[i] = Uint64Value(0) + + err = storage.Remove(SlabID(id)) + require.NoError(t, err) + } + + verifyArray(t, storage, typeInfo, address, parentArray, expectedValues, true) + }) +} + +func TestArrayRemoveReturnedValue(t *testing.T) { + typeInfo := testTypeInfo{42} + address := Address{1, 2, 3, 4, 5, 6, 7, 8} + + t.Run("child array is not inlined", func(t *testing.T) { + const arraySize = 2 + + storage := newTestPersistentStorage(t) + + // Create parent array + parentArray, err := NewArray(storage, address, typeInfo) + require.NoError(t, err) + + var expectedValues arrayValue + + for i := 0; i < arraySize; i++ { + // Create child array + childArray, err := NewArray(storage, address, typeInfo) + require.NoError(t, err) + + err = parentArray.Append(childArray) + require.NoError(t, err) + + var expectedChildValues arrayValue + for { + v := NewStringValue(strings.Repeat("a", 10)) + + err = childArray.Append(v) + require.NoError(t, err) + + expectedChildValues = append(expectedChildValues, v) + + if !childArray.Inlined() { + break + } + } + + expectedValues = append(expectedValues, expectedChildValues) + } + + verifyArray(t, storage, typeInfo, address, parentArray, expectedValues, true) + + // Remove child array value + for i := 0; i < arraySize; i++ { + valueStorable, err := parentArray.Remove(uint64(0)) + require.NoError(t, err) + + id, ok := valueStorable.(SlabIDStorable) + require.True(t, ok) + + child, err := id.StoredValue(storage) + require.NoError(t, err) + + valueEqual(t, expectedValues[i], child) + + err = storage.Remove(SlabID(id)) + require.NoError(t, err) + } + + verifyEmptyArray(t, storage, typeInfo, address, parentArray) + }) + + t.Run("child array is inlined", func(t *testing.T) { + const arraySize = 2 + + storage := newTestPersistentStorage(t) + + // Create parent array + parentArray, err := NewArray(storage, address, typeInfo) + require.NoError(t, err) + + var expectedValues arrayValue + + for i := 0; i < arraySize; i++ { + // Create child array + childArray, err := NewArray(storage, address, typeInfo) + require.NoError(t, err) + + err = parentArray.Append(childArray) + require.NoError(t, err) + + // Insert one element to child array + v := NewStringValue(strings.Repeat("a", 10)) + + err = childArray.Append(v) + require.NoError(t, err) + require.True(t, childArray.Inlined()) + + expectedValues = append(expectedValues, arrayValue{v}) + } + + verifyArray(t, storage, typeInfo, address, parentArray, expectedValues, true) + + // Remove child array value + for i := 0; i < arraySize; i++ { + valueStorable, err := parentArray.Remove(uint64(0)) + require.NoError(t, err) + + id, ok := valueStorable.(SlabIDStorable) + require.True(t, ok) + + child, err := id.StoredValue(storage) + require.NoError(t, err) + + valueEqual(t, expectedValues[i], child) + + err = storage.Remove(SlabID(id)) + require.NoError(t, err) + } + + verifyEmptyArray(t, storage, typeInfo, address, parentArray) + }) + + t.Run("child map is not inlined", func(t *testing.T) { + const arraySize = 2 + + storage := newTestPersistentStorage(t) + + // Create parent array + parentArray, err := NewArray(storage, address, typeInfo) + require.NoError(t, err) + + var expectedValues arrayValue + + for i := 0; i < arraySize; i++ { + // Create child map + childMap, err := NewMap(storage, address, newBasicDigesterBuilder(), typeInfo) + require.NoError(t, err) + + err = parentArray.Append(childMap) + require.NoError(t, err) + + expectedChildValues := make(mapValue) + expectedValues = append(expectedValues, expectedChildValues) + + // Insert into child map until child map is not inlined + j := 0 + for { + k := Uint64Value(j) + v := NewStringValue(strings.Repeat("a", 10)) + j++ + + existingStorable, err := childMap.Set(compare, hashInputProvider, k, v) + require.NoError(t, err) + require.Nil(t, existingStorable) + + expectedChildValues[k] = v + + if !childMap.Inlined() { + break + } + } + } + + verifyArray(t, storage, typeInfo, address, parentArray, expectedValues, true) + + // Remove child map value + for i := 0; i < arraySize; i++ { + valueStorable, err := parentArray.Remove(uint64(0)) + require.NoError(t, err) + + id, ok := valueStorable.(SlabIDStorable) + require.True(t, ok) + + child, err := id.StoredValue(storage) + require.NoError(t, err) + + valueEqual(t, expectedValues[i], child) + + err = storage.Remove(SlabID(id)) + require.NoError(t, err) + } + + verifyEmptyArray(t, storage, typeInfo, address, parentArray) + }) + + t.Run("child map is inlined", func(t *testing.T) { + const arraySize = 2 + + storage := newTestPersistentStorage(t) + + // Create parent array + parentArray, err := NewArray(storage, address, typeInfo) + require.NoError(t, err) + + var expectedValues arrayValue + + for i := 0; i < arraySize; i++ { + // Create child map + childMap, err := NewMap(storage, address, newBasicDigesterBuilder(), typeInfo) + require.NoError(t, err) + + k := Uint64Value(i) + + err = parentArray.Append(childMap) + require.NoError(t, err) + + expectedChildValues := make(mapValue) + expectedValues = append(expectedValues, expectedChildValues) + + // Insert into child map until child map is not inlined + v := NewStringValue(strings.Repeat("a", 10)) + + existingStorable, err := childMap.Set(compare, hashInputProvider, k, v) + require.NoError(t, err) + require.Nil(t, existingStorable) + + expectedChildValues[k] = v + } + + verifyArray(t, storage, typeInfo, address, parentArray, expectedValues, true) + + // Remove child map value + for i := 0; i < arraySize; i++ { + valueStorable, err := parentArray.Remove(uint64(0)) + require.NoError(t, err) + + id, ok := valueStorable.(SlabIDStorable) + require.True(t, ok) + + child, err := id.StoredValue(storage) + require.NoError(t, err) + + valueEqual(t, expectedValues[i], child) + + err = storage.Remove(SlabID(id)) + require.NoError(t, err) + } + + verifyEmptyArray(t, storage, typeInfo, address, parentArray) + }) +} diff --git a/map.go b/map.go index 27a485ab..848442f1 100644 --- a/map.go +++ b/map.go @@ -3060,6 +3060,52 @@ func (m *MapDataSlab) Inlinable(maxInlineSize uint64) bool { return uint64(inlinedSize) <= maxInlineSize } +// inline converts not-inlined MapDataSlab to inlined MapDataSlab and removes it from storage. +func (m *MapDataSlab) inline(storage SlabStorage) error { + if m.inlined { + return NewFatalError(fmt.Errorf("failed to inline MapDataSlab %s: it is inlined already", m.header.slabID)) + } + + id := m.header.slabID + + // Remove slab from storage because it is going to be inlined. + err := storage.Remove(id) + if err != nil { + // Wrap err as external error (if needed) because err is returned by SlabStorage interface. + return wrapErrorfAsExternalErrorIfNeeded(err, fmt.Sprintf("failed to remove slab %s", id)) + } + + // Update data slab size from not inlined to inlined + m.header.size = inlinedMapDataSlabPrefixSize + m.elements.Size() + + // Update data slab inlined status. + m.inlined = true + + return nil +} + +// uninline converts an inlined MapDataSlab to uninlined MapDataSlab and stores it in storage. +func (m *MapDataSlab) uninline(storage SlabStorage) error { + if !m.inlined { + return NewFatalError(fmt.Errorf("failed to uninline MapDataSlab %s: it is not inlined", m.header.slabID)) + } + + // Update data slab size from inlined to not inlined. + m.header.size = mapRootDataSlabPrefixSize + m.elements.Size() + + // Update data slab inlined status. + m.inlined = false + + // Store slab in storage + err := storage.Store(m.header.slabID, m) + if err != nil { + // Wrap err as external error (if needed) because err is returned by SlabStorage interface. + return wrapErrorfAsExternalErrorIfNeeded(err, fmt.Sprintf("failed to store slab %s", m.header.slabID)) + } + + return nil +} + func elementsStorables(elems elements, childStorables []Storable) []Storable { switch v := elems.(type) { @@ -4646,7 +4692,7 @@ func (m *OrderedMap) setCallbackWithChild( // Set child value with parent map using same key. // Set() calls c.Storable() which returns inlined or not-inlined child storable. - existingValueStorable, err := m.Set(comparator, hip, key, c) + existingValueStorable, err := m.set(comparator, hip, key, c) if err != nil { return err } @@ -4755,6 +4801,34 @@ func (m *OrderedMap) get(comparator ValueComparator, hip HashInputProvider, key } func (m *OrderedMap) Set(comparator ValueComparator, hip HashInputProvider, key Value, value Value) (Storable, error) { + storable, err := m.set(comparator, hip, key, value) + if err != nil { + return nil, err + } + + // If overwritten storable is an inlined slab, uninline the slab and store it in storage. + // This is to prevent potential data loss because the overwritten inlined slab was not in + // storage and any future changes to it would have been lost. + switch s := storable.(type) { + case *ArrayDataSlab: + err = s.uninline(m.Storage) + if err != nil { + return nil, err + } + storable = SlabIDStorable(s.header.slabID) + + case *MapDataSlab: + err = s.uninline(m.Storage) + if err != nil { + return nil, err + } + storable = SlabIDStorable(s.header.slabID) + } + + return storable, nil +} + +func (m *OrderedMap) set(comparator ValueComparator, hip HashInputProvider, key Value, value Value) (Storable, error) { keyDigest, err := m.digesterBuilder.Digest(hip, key) if err != nil { @@ -4790,7 +4864,6 @@ func (m *OrderedMap) Set(comparator ValueComparator, hip HashInputProvider, key // Don't need to wrap error as external error because err is already categorized by OrderedMap.promoteChildAsNewRoot(). return nil, err } - return existingValue, nil } } @@ -4831,6 +4904,34 @@ func (m *OrderedMap) Set(comparator ValueComparator, hip HashInputProvider, key } func (m *OrderedMap) Remove(comparator ValueComparator, hip HashInputProvider, key Value) (Storable, Storable, error) { + keyStorable, valueStorable, err := m.remove(comparator, hip, key) + if err != nil { + return nil, nil, err + } + + // If overwritten storable is an inlined slab, uninline the slab and store it in storage. + // This is to prevent potential data loss because the overwritten inlined slab was not in + // storage and any future changes to it would have been lost. + switch s := valueStorable.(type) { + case *ArrayDataSlab: + err = s.uninline(m.Storage) + if err != nil { + return nil, nil, err + } + valueStorable = SlabIDStorable(s.header.slabID) + + case *MapDataSlab: + err = s.uninline(m.Storage) + if err != nil { + return nil, nil, err + } + valueStorable = SlabIDStorable(s.header.slabID) + } + + return keyStorable, valueStorable, nil +} + +func (m *OrderedMap) remove(comparator ValueComparator, hip HashInputProvider, key Value) (Storable, Storable, error) { keyDigest, err := m.digesterBuilder.Digest(hip, key) if err != nil { @@ -4864,7 +4965,6 @@ func (m *OrderedMap) Remove(comparator ValueComparator, hip HashInputProvider, k // Don't need to wrap error as external error because err is already categorized by OrderedMap.promoteChildAsNewRoot(). return nil, nil, err } - return k, v, nil } } @@ -5031,21 +5131,11 @@ func (m *OrderedMap) Storable(_ SlabStorage, _ Address, maxInlineSize uint64) (S return nil, NewFatalError(fmt.Errorf("unexpected inlinable map slab type %T", m.root)) } - rootID := rootDataSlab.header.slabID - - // Remove root slab from storage because it is going to be inlined. - err := m.Storage.Remove(rootID) + err := rootDataSlab.inline(m.Storage) 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 remove slab %s", rootID)) + return nil, err } - // Update root data slab size from not inlined to inlined - rootDataSlab.header.size = inlinedMapDataSlabPrefixSize + rootDataSlab.elements.Size() - - // Update root data slab inlined status. - rootDataSlab.inlined = true - return rootDataSlab, nil case !inlinable && inlined: @@ -5060,17 +5150,9 @@ func (m *OrderedMap) Storable(_ SlabStorage, _ Address, maxInlineSize uint64) (S return nil, NewFatalError(fmt.Errorf("unexpected inlined map slab type %T", m.root)) } - // Update root data slab size from inlined to not inlined. - rootDataSlab.header.size = mapRootDataSlabPrefixSize + rootDataSlab.elements.Size() - - // Update root data slab inlined status. - rootDataSlab.inlined = false - - // Store root slab in storage - err := m.Storage.Store(m.SlabID(), m.root) + err := rootDataSlab.uninline(m.Storage) 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 store slab %s", m.SlabID())) + return nil, err } return SlabIDStorable(m.SlabID()), nil diff --git a/map_test.go b/map_test.go index 2b0c5ec7..095f9f00 100644 --- a/map_test.go +++ b/map_test.go @@ -12478,6 +12478,24 @@ func TestNestedThreeLevelChildMapInlinabilityInParentMap(t *testing.T) { require.Equal(t, k, existingKey) require.NotNil(t, existingValue) + // Grand child map is returned as SlabIDStorable, even if it was stored inlined in the parent. + id, ok := existingValue.(SlabIDStorable) + require.True(t, ok) + + v, err := id.StoredValue(storage) + require.NoError(t, err) + + gchildMap, ok := v.(*OrderedMap) + require.True(t, ok) + + expectedGChildMapValues, ok := expectedChildMapValues[k].(mapValue) + require.True(t, ok) + + valueEqual(t, expectedGChildMapValues, gchildMap) + + err = storage.Remove(SlabID(id)) + require.NoError(t, err) + delete(expectedChildMapValues, k) // Child map is inlined @@ -12845,3 +12863,513 @@ func getInlinedChildMapsFromParentMap(t *testing.T, address Address, parentMap * return children } + +func TestMapSetReturnedValue(t *testing.T) { + typeInfo := testTypeInfo{42} + address := Address{1, 2, 3, 4, 5, 6, 7, 8} + + t.Run("child array is not inlined", func(t *testing.T) { + const mapSize = 2 + + storage := newTestPersistentStorage(t) + + // Create parent map + parentMap, err := NewMap(storage, address, newBasicDigesterBuilder(), typeInfo) + require.NoError(t, err) + + expectedKeyValues := make(map[Value]Value) + + for i := 0; i < mapSize; i++ { + // Create child array + childArray, err := NewArray(storage, address, typeInfo) + require.NoError(t, err) + + k := Uint64Value(i) + + existingStorable, err := parentMap.Set(compare, hashInputProvider, k, childArray) + require.NoError(t, err) + require.Nil(t, existingStorable) + + var expectedChildValues arrayValue + for { + v := NewStringValue(strings.Repeat("a", 10)) + + err = childArray.Append(v) + require.NoError(t, err) + + expectedChildValues = append(expectedChildValues, v) + + if !childArray.Inlined() { + break + } + } + + expectedKeyValues[k] = expectedChildValues + } + + verifyMap(t, storage, typeInfo, address, parentMap, expectedKeyValues, nil, true) + + // Overwrite existing child array value + for k := range expectedKeyValues { + existingStorable, err := parentMap.Set(compare, hashInputProvider, k, Uint64Value(0)) + require.NoError(t, err) + require.NotNil(t, existingStorable) + + id, ok := existingStorable.(SlabIDStorable) + require.True(t, ok) + + child, err := id.StoredValue(storage) + require.NoError(t, err) + + valueEqual(t, expectedKeyValues[k], child) + + err = storage.Remove(SlabID(id)) + require.NoError(t, err) + + expectedKeyValues[k] = Uint64Value(0) + } + + verifyMap(t, storage, typeInfo, address, parentMap, expectedKeyValues, nil, true) + }) + + t.Run("child array is inlined", func(t *testing.T) { + const mapSize = 2 + + storage := newTestPersistentStorage(t) + + // Create parent map + parentMap, err := NewMap(storage, address, newBasicDigesterBuilder(), typeInfo) + require.NoError(t, err) + + expectedKeyValues := make(map[Value]Value) + + for i := 0; i < mapSize; i++ { + // Create child array + childArray, err := NewArray(storage, address, typeInfo) + require.NoError(t, err) + + k := Uint64Value(i) + + existingStorable, err := parentMap.Set(compare, hashInputProvider, k, childArray) + require.NoError(t, err) + require.Nil(t, existingStorable) + + // Insert one element to child array + v := NewStringValue(strings.Repeat("a", 10)) + + err = childArray.Append(v) + require.NoError(t, err) + require.True(t, childArray.Inlined()) + + expectedKeyValues[k] = arrayValue{v} + } + + verifyMap(t, storage, typeInfo, address, parentMap, expectedKeyValues, nil, true) + + // Overwrite existing child array value + for k := range expectedKeyValues { + existingStorable, err := parentMap.Set(compare, hashInputProvider, k, Uint64Value(0)) + require.NoError(t, err) + require.NotNil(t, existingStorable) + + id, ok := existingStorable.(SlabIDStorable) + require.True(t, ok) + + child, err := id.StoredValue(storage) + require.NoError(t, err) + + valueEqual(t, expectedKeyValues[k], child) + + expectedKeyValues[k] = Uint64Value(0) + + err = storage.Remove(SlabID(id)) + require.NoError(t, err) + } + + verifyMap(t, storage, typeInfo, address, parentMap, expectedKeyValues, nil, true) + }) + + t.Run("child map is not inlined", func(t *testing.T) { + const mapSize = 2 + + storage := newTestPersistentStorage(t) + + // Create parent map + parentMap, err := NewMap(storage, address, newBasicDigesterBuilder(), typeInfo) + require.NoError(t, err) + + expectedKeyValues := make(map[Value]Value) + + for i := 0; i < mapSize; i++ { + // Create child map + childMap, err := NewMap(storage, address, newBasicDigesterBuilder(), typeInfo) + require.NoError(t, err) + + k := Uint64Value(i) + + existingStorable, err := parentMap.Set(compare, hashInputProvider, k, childMap) + require.NoError(t, err) + require.Nil(t, existingStorable) + + expectedChildValues := make(mapValue) + expectedKeyValues[k] = expectedChildValues + + // Insert into child map until child map is not inlined + j := 0 + for { + k := Uint64Value(j) + v := NewStringValue(strings.Repeat("a", 10)) + j++ + + existingStorable, err := childMap.Set(compare, hashInputProvider, k, v) + require.NoError(t, err) + require.Nil(t, existingStorable) + + expectedChildValues[k] = v + + if !childMap.Inlined() { + break + } + } + } + + verifyMap(t, storage, typeInfo, address, parentMap, expectedKeyValues, nil, true) + + // Overwrite existing child map value + for k := range expectedKeyValues { + existingStorable, err := parentMap.Set(compare, hashInputProvider, k, Uint64Value(0)) + require.NoError(t, err) + require.NotNil(t, existingStorable) + + id, ok := existingStorable.(SlabIDStorable) + require.True(t, ok) + + child, err := id.StoredValue(storage) + require.NoError(t, err) + + valueEqual(t, expectedKeyValues[k], child) + + expectedKeyValues[k] = Uint64Value(0) + + err = storage.Remove(SlabID(id)) + require.NoError(t, err) + } + + verifyMap(t, storage, typeInfo, address, parentMap, expectedKeyValues, nil, true) + }) + + t.Run("child map is inlined", func(t *testing.T) { + const mapSize = 2 + + storage := newTestPersistentStorage(t) + + // Create parent map + parentMap, err := NewMap(storage, address, newBasicDigesterBuilder(), typeInfo) + require.NoError(t, err) + + expectedKeyValues := make(map[Value]Value) + + for i := 0; i < mapSize; i++ { + // Create child map + childMap, err := NewMap(storage, address, newBasicDigesterBuilder(), typeInfo) + require.NoError(t, err) + + k := Uint64Value(i) + + existingStorable, err := parentMap.Set(compare, hashInputProvider, k, childMap) + require.NoError(t, err) + require.Nil(t, existingStorable) + + expectedChildValues := make(mapValue) + expectedKeyValues[k] = expectedChildValues + + // Insert into child map until child map is not inlined + v := NewStringValue(strings.Repeat("a", 10)) + + existingStorable, err = childMap.Set(compare, hashInputProvider, k, v) + require.NoError(t, err) + require.Nil(t, existingStorable) + + expectedChildValues[k] = v + } + + verifyMap(t, storage, typeInfo, address, parentMap, expectedKeyValues, nil, true) + + // Overwrite existing child map value + for k := range expectedKeyValues { + existingStorable, err := parentMap.Set(compare, hashInputProvider, k, Uint64Value(0)) + require.NoError(t, err) + require.NotNil(t, existingStorable) + + id, ok := existingStorable.(SlabIDStorable) + require.True(t, ok) + + child, err := id.StoredValue(storage) + require.NoError(t, err) + + valueEqual(t, expectedKeyValues[k], child) + + expectedKeyValues[k] = Uint64Value(0) + + err = storage.Remove(SlabID(id)) + require.NoError(t, err) + } + + verifyMap(t, storage, typeInfo, address, parentMap, expectedKeyValues, nil, true) + }) +} + +func TestMapRemoveReturnedValue(t *testing.T) { + typeInfo := testTypeInfo{42} + address := Address{1, 2, 3, 4, 5, 6, 7, 8} + + t.Run("child array is not inlined", func(t *testing.T) { + const mapSize = 2 + + storage := newTestPersistentStorage(t) + + // Create parent map + parentMap, err := NewMap(storage, address, newBasicDigesterBuilder(), typeInfo) + require.NoError(t, err) + + expectedKeyValues := make(map[Value]Value) + + for i := 0; i < mapSize; i++ { + // Create child array + childArray, err := NewArray(storage, address, typeInfo) + require.NoError(t, err) + + k := Uint64Value(i) + + existingStorable, err := parentMap.Set(compare, hashInputProvider, k, childArray) + require.NoError(t, err) + require.Nil(t, existingStorable) + + var expectedChildValues arrayValue + for { + v := NewStringValue(strings.Repeat("a", 10)) + + err = childArray.Append(v) + require.NoError(t, err) + + expectedChildValues = append(expectedChildValues, v) + + if !childArray.Inlined() { + break + } + } + + expectedKeyValues[k] = expectedChildValues + } + + verifyMap(t, storage, typeInfo, address, parentMap, expectedKeyValues, nil, true) + + // Remove child array value + for k := range expectedKeyValues { + keyStorable, valueStorable, err := parentMap.Remove(compare, hashInputProvider, k) + require.NoError(t, err) + require.Equal(t, keyStorable, k) + + id, ok := valueStorable.(SlabIDStorable) + require.True(t, ok) + + child, err := id.StoredValue(storage) + require.NoError(t, err) + + valueEqual(t, expectedKeyValues[k], child) + + err = storage.Remove(SlabID(id)) + require.NoError(t, err) + + delete(expectedKeyValues, k) + } + + verifyMap(t, storage, typeInfo, address, parentMap, expectedKeyValues, nil, true) + }) + + t.Run("child array is inlined", func(t *testing.T) { + const mapSize = 2 + + storage := newTestPersistentStorage(t) + + // Create parent map + parentMap, err := NewMap(storage, address, newBasicDigesterBuilder(), typeInfo) + require.NoError(t, err) + + expectedKeyValues := make(map[Value]Value) + + for i := 0; i < mapSize; i++ { + // Create child array + childArray, err := NewArray(storage, address, typeInfo) + require.NoError(t, err) + + k := Uint64Value(i) + + existingStorable, err := parentMap.Set(compare, hashInputProvider, k, childArray) + require.NoError(t, err) + require.Nil(t, existingStorable) + + // Insert one element to child array + v := NewStringValue(strings.Repeat("a", 10)) + + err = childArray.Append(v) + require.NoError(t, err) + require.True(t, childArray.Inlined()) + + expectedKeyValues[k] = arrayValue{v} + } + + verifyMap(t, storage, typeInfo, address, parentMap, expectedKeyValues, nil, true) + + // Remove child array value + for k := range expectedKeyValues { + keyStorable, valueStorable, err := parentMap.Remove(compare, hashInputProvider, k) + require.NoError(t, err) + require.Equal(t, keyStorable, k) + + id, ok := valueStorable.(SlabIDStorable) + require.True(t, ok) + + child, err := id.StoredValue(storage) + require.NoError(t, err) + + valueEqual(t, expectedKeyValues[k], child) + + delete(expectedKeyValues, k) + + err = storage.Remove(SlabID(id)) + require.NoError(t, err) + } + + verifyMap(t, storage, typeInfo, address, parentMap, expectedKeyValues, nil, true) + }) + + t.Run("child map is not inlined", func(t *testing.T) { + const mapSize = 2 + + storage := newTestPersistentStorage(t) + + // Create parent map + parentMap, err := NewMap(storage, address, newBasicDigesterBuilder(), typeInfo) + require.NoError(t, err) + + expectedKeyValues := make(map[Value]Value) + + for i := 0; i < mapSize; i++ { + // Create child map + childMap, err := NewMap(storage, address, newBasicDigesterBuilder(), typeInfo) + require.NoError(t, err) + + k := Uint64Value(i) + + existingStorable, err := parentMap.Set(compare, hashInputProvider, k, childMap) + require.NoError(t, err) + require.Nil(t, existingStorable) + + expectedChildValues := make(mapValue) + expectedKeyValues[k] = expectedChildValues + + // Insert into child map until child map is not inlined + j := 0 + for { + k := Uint64Value(j) + v := NewStringValue(strings.Repeat("a", 10)) + j++ + + existingStorable, err := childMap.Set(compare, hashInputProvider, k, v) + require.NoError(t, err) + require.Nil(t, existingStorable) + + expectedChildValues[k] = v + + if !childMap.Inlined() { + break + } + } + } + + verifyMap(t, storage, typeInfo, address, parentMap, expectedKeyValues, nil, true) + + // Remove child map value + for k := range expectedKeyValues { + keyStorable, valueStorable, err := parentMap.Remove(compare, hashInputProvider, k) + require.NoError(t, err) + require.Equal(t, keyStorable, k) + + id, ok := valueStorable.(SlabIDStorable) + require.True(t, ok) + + child, err := id.StoredValue(storage) + require.NoError(t, err) + + valueEqual(t, expectedKeyValues[k], child) + + delete(expectedKeyValues, k) + + err = storage.Remove(SlabID(id)) + require.NoError(t, err) + } + + verifyMap(t, storage, typeInfo, address, parentMap, expectedKeyValues, nil, true) + }) + + t.Run("child map is inlined", func(t *testing.T) { + const mapSize = 2 + + storage := newTestPersistentStorage(t) + + // Create parent map + parentMap, err := NewMap(storage, address, newBasicDigesterBuilder(), typeInfo) + require.NoError(t, err) + + expectedKeyValues := make(map[Value]Value) + + for i := 0; i < mapSize; i++ { + // Create child map + childMap, err := NewMap(storage, address, newBasicDigesterBuilder(), typeInfo) + require.NoError(t, err) + + k := Uint64Value(i) + + existingStorable, err := parentMap.Set(compare, hashInputProvider, k, childMap) + require.NoError(t, err) + require.Nil(t, existingStorable) + + expectedChildValues := make(mapValue) + expectedKeyValues[k] = expectedChildValues + + // Insert into child map until child map is not inlined + v := NewStringValue(strings.Repeat("a", 10)) + + existingStorable, err = childMap.Set(compare, hashInputProvider, k, v) + require.NoError(t, err) + require.Nil(t, existingStorable) + + expectedChildValues[k] = v + } + + verifyMap(t, storage, typeInfo, address, parentMap, expectedKeyValues, nil, true) + + // Remove child map value + for k := range expectedKeyValues { + keyStorable, valueStorable, err := parentMap.Remove(compare, hashInputProvider, k) + require.NoError(t, err) + require.Equal(t, keyStorable, k) + + id, ok := valueStorable.(SlabIDStorable) + require.True(t, ok) + + child, err := id.StoredValue(storage) + require.NoError(t, err) + + valueEqual(t, expectedKeyValues[k], child) + + delete(expectedKeyValues, k) + + err = storage.Remove(SlabID(id)) + require.NoError(t, err) + } + + verifyMap(t, storage, typeInfo, address, parentMap, expectedKeyValues, nil, true) + }) +}