Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix Cadence 1.0 migration of dictionary values when using atree inlined data #3316

Merged
merged 1 commit into from
May 7, 2024
Merged

Fix Cadence 1.0 migration of dictionary values when using atree inlined data #3316

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
142 changes: 97 additions & 45 deletions migrations/migration.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -318,31 +318,23 @@ func (m *StorageMigration) MigrateNestedValue(
key, value interpreter.Value
}

// Read the keys first, so the iteration wouldn't be affected
// by the modification of the nested values.
var existingKeysAndValues []keyValuePair
// Migrate keys first.

var existingKeys []interpreter.Value

dictionary.IterateReadOnly(
inter,
emptyLocationRange,
func(key, value interpreter.Value) (resume bool) {
func(key, _ interpreter.Value) (resume bool) {

existingKeysAndValues = append(
existingKeysAndValues,
keyValuePair{
key: key,
value: value,
},
)
existingKeys = append(existingKeys, key)

// Continue iteration
return true
},
)

for _, existingKeyAndValue := range existingKeysAndValues {
existingKey := existingKeyAndValue.key
existingValue := existingKeyAndValue.value
for _, existingKey := range existingKeys {

newKey := m.MigrateNestedValue(
storageKey,
Expand All @@ -352,26 +344,11 @@ func (m *StorageMigration) MigrateNestedValue(
reporter,
)

newValue := m.MigrateNestedValue(
storageKey,
storageMapKey,
existingValue,
valueMigrations,
reporter,
)

if newKey == nil && newValue == nil {
if newKey == nil {
continue
}

// We only reach here at least one of key or value has been migrated.
var keyToSet, valueToSet interpreter.Value

if newKey == nil {
keyToSet = existingKey
} else {
keyToSet = newKey
}
// We only reach here because key needs to be migrated.

// Remove the old key-value pair

Expand All @@ -388,16 +365,12 @@ func (m *StorageMigration) MigrateNestedValue(
))
}

if newValue == nil {
valueToSet = existingValue
} else {
// Value was migrated
valueToSet = newValue
// Remove existing key since old key is migrated
interpreter.StoredValue(inter, existingKeyStorable, m.storage).DeepRemove(inter, false)
inter.RemoveReferencedSlab(existingKeyStorable)

interpreter.StoredValue(inter, existingValueStorable, m.storage).
DeepRemove(inter, false)
inter.RemoveReferencedSlab(existingValueStorable)
}
// Convert removed value storable to Value.
existingValue := interpreter.StoredValue(inter, existingValueStorable, m.storage)

// Handle dictionary key conflicts.
//
Expand All @@ -417,8 +390,28 @@ func (m *StorageMigration) MigrateNestedValue(
if dictionary.ContainsKey(
inter,
emptyLocationRange,
keyToSet,
newKey,
) {

newValue := m.MigrateNestedValue(
storageKey,
storageMapKey,
existingValue,
valueMigrations,
reporter,
)

var valueToSet interpreter.Value
if newValue == nil {
valueToSet = existingValue
} else {
valueToSet = newValue

// Remove existing value since value is migrated.
existingValue.DeepRemove(inter, false)
inter.RemoveReferencedSlab(existingValueStorable)
}

owner := dictionary.GetOwner()

pathDomain := common.PathDomainStorage
Expand All @@ -433,7 +426,7 @@ func (m *StorageMigration) MigrateNestedValue(
conflictDictionary.InsertWithoutTransfer(
inter,
emptyLocationRange,
keyToSet,
newKey,
valueToSet,
)

Expand Down Expand Up @@ -463,17 +456,76 @@ func (m *StorageMigration) MigrateNestedValue(

} else {

// No conflict, insert the new key-value pair
// No conflict, insert the new key and existing value pair
// Don't migrate value here because we are going to migrate all values in the dictionary next.

dictionary.InsertWithoutTransfer(
inter,
emptyLocationRange,
keyToSet,
valueToSet,
newKey,
existingValue,
)
}
}

// Migrate values next.

var existingKeysAndValues []keyValuePair

dictionary.Iterate(
inter,
emptyLocationRange,
func(key, value interpreter.Value) (resume bool) {

existingKeysAndValues = append(
existingKeysAndValues,
keyValuePair{
key: key,
value: value,
},
)

// Continue iteration
return true
},
)

for _, existingKeyAndValue := range existingKeysAndValues {
existingKey := existingKeyAndValue.key
existingValue := existingKeyAndValue.value

newValue := m.MigrateNestedValue(
storageKey,
storageMapKey,
existingValue,
valueMigrations,
reporter,
)

if newValue == nil {
continue
}

// Set new value with existing key in the dictionary.
existingValueStorable := dictionary.InsertWithoutTransfer(
inter,
emptyLocationRange,
existingKey,
newValue,
)
if existingValueStorable == nil {
panic(errors.NewUnexpectedError(
"failed to set migrated value for key: %s",
existingKey,
))
}

// Remove existing value since value is migrated
interpreter.StoredValue(inter, existingValueStorable, m.storage).
DeepRemove(inter, false)
inter.RemoveReferencedSlab(existingValueStorable)
}

case *interpreter.PublishedValue:
publishedValue := typedValue
newInnerValue := m.MigrateNestedValue(
Expand Down