From e72186b0c98df5249e2fc93c246b956fb33e7a63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Fri, 15 Dec 2023 14:11:30 -0800 Subject: [PATCH 01/12] add migration for type values with intersection types with two or more interfaces --- migrations/type_value/migration.go | 140 +++++++ migrations/type_value/migration_test.go | 472 ++++++++++++++++++++++++ 2 files changed, 612 insertions(+) create mode 100644 migrations/type_value/migration.go create mode 100644 migrations/type_value/migration_test.go diff --git a/migrations/type_value/migration.go b/migrations/type_value/migration.go new file mode 100644 index 0000000000..943749ea44 --- /dev/null +++ b/migrations/type_value/migration.go @@ -0,0 +1,140 @@ +/* + * Cadence - The resource-oriented smart contract programming language + * + * Copyright Dapper Labs, Inc. + * + * 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 account_type + +import ( + "github.com/onflow/cadence/migrations" + "github.com/onflow/cadence/runtime/errors" + "github.com/onflow/cadence/runtime/interpreter" +) + +type TypeValueMigration struct{} + +var _ migrations.ValueMigration = TypeValueMigration{} + +func NewTypeValueMigration() TypeValueMigration { + return TypeValueMigration{} +} + +func (TypeValueMigration) Name() string { + return "TypeValueMigration" +} + +// Migrate migrates intersection types (formerly, restricted types) inside `TypeValue`s. +func (TypeValueMigration) Migrate( + _ interpreter.AddressPath, + value interpreter.Value, + inter *interpreter.Interpreter, +) (newValue interpreter.Value) { + switch value := value.(type) { + case interpreter.TypeValue: + convertedType := maybeConvertType(value.Type, inter) + if convertedType == nil { + return + } + return interpreter.NewTypeValue(nil, convertedType) + } + + return +} + +func maybeConvertType( + staticType interpreter.StaticType, + inter *interpreter.Interpreter, +) interpreter.StaticType { + + switch staticType := staticType.(type) { + case *interpreter.ConstantSizedStaticType: + convertedType := maybeConvertType(staticType.Type, inter) + if convertedType != nil { + return interpreter.NewConstantSizedStaticType(nil, convertedType, staticType.Size) + } + + case *interpreter.VariableSizedStaticType: + convertedType := maybeConvertType(staticType.Type, inter) + if convertedType != nil { + return interpreter.NewVariableSizedStaticType(nil, convertedType) + } + + case *interpreter.DictionaryStaticType: + convertedKeyType := maybeConvertType(staticType.KeyType, inter) + convertedValueType := maybeConvertType(staticType.ValueType, inter) + if convertedKeyType != nil && convertedValueType != nil { + return interpreter.NewDictionaryStaticType(nil, convertedKeyType, convertedValueType) + } + if convertedKeyType != nil { + return interpreter.NewDictionaryStaticType(nil, convertedKeyType, staticType.ValueType) + } + if convertedValueType != nil { + return interpreter.NewDictionaryStaticType(nil, staticType.KeyType, convertedValueType) + } + + case *interpreter.CapabilityStaticType: + convertedBorrowType := maybeConvertType(staticType.BorrowType, inter) + if convertedBorrowType != nil { + return interpreter.NewCapabilityStaticType(nil, convertedBorrowType) + } + + case *interpreter.OptionalStaticType: + convertedInnerType := maybeConvertType(staticType.Type, inter) + if convertedInnerType != nil { + return interpreter.NewOptionalStaticType(nil, convertedInnerType) + } + + case *interpreter.ReferenceStaticType: + // TODO: Reference of references must not be allowed? + convertedReferencedType := maybeConvertType(staticType.ReferencedType, inter) + if convertedReferencedType != nil { + return interpreter.NewReferenceStaticType(nil, staticType.Authorization, convertedReferencedType) + } + + case interpreter.FunctionStaticType: + // Non-storable + + case *interpreter.CompositeStaticType, + *interpreter.InterfaceStaticType, + interpreter.PrimitiveStaticType: + + // Nothing to do + + case *interpreter.IntersectionStaticType: + // No need to convert `staticType.Types` as they can only be interfaces. + + legacyType := staticType.LegacyType + if legacyType != nil { + convertedLegacyType := maybeConvertType(legacyType, inter) + if convertedLegacyType != nil { + intersectionType := interpreter.NewIntersectionStaticType(nil, staticType.Types) + intersectionType.LegacyType = convertedLegacyType + return intersectionType + } + } + + // If the set has at least two items, + // then force it to be re-stored/re-encoded + if len(staticType.Types) >= 2 { + return staticType + } + + default: + panic(errors.NewUnreachableError()) + } + + return nil +} diff --git a/migrations/type_value/migration_test.go b/migrations/type_value/migration_test.go new file mode 100644 index 0000000000..fa2ff14817 --- /dev/null +++ b/migrations/type_value/migration_test.go @@ -0,0 +1,472 @@ +/* + * Cadence - The resource-oriented smart contract programming language + * + * Copyright Dapper Labs, Inc. + * + * 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 account_type + +import ( + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + "github.com/onflow/cadence/migrations" + "github.com/onflow/cadence/runtime" + "github.com/onflow/cadence/runtime/common" + "github.com/onflow/cadence/runtime/interpreter" + "github.com/onflow/cadence/runtime/tests/runtime_utils" + "github.com/onflow/cadence/runtime/tests/utils" +) + +var _ migrations.Reporter = &testReporter{} + +type testReporter struct { + migratedPaths map[interpreter.AddressPath]struct{} +} + +func newTestReporter() *testReporter { + return &testReporter{ + migratedPaths: map[interpreter.AddressPath]struct{}{}, + } +} + +func (t *testReporter) Report( + addressPath interpreter.AddressPath, + _ string, +) { + t.migratedPaths[addressPath] = struct{}{} +} + +func TestTypeValueMigration(t *testing.T) { + t.Parallel() + + account := common.Address{0x42} + pathDomain := common.PathDomainPublic + + const stringType = interpreter.PrimitiveStaticTypeString + + const fooBarQualifiedIdentifier = "Foo.Bar" + const fooBazQualifiedIdentifier = "Foo.Baz" + + fooAddressLocation := common.NewAddressLocation(nil, account, "Foo") + + newIntersectionStaticTypeWithoutInterfaces := func() *interpreter.IntersectionStaticType { + return interpreter.NewIntersectionStaticType( + nil, + []*interpreter.InterfaceStaticType{}, + ) + } + + newIntersectionStaticTypeWithOneInterface := func() *interpreter.IntersectionStaticType { + return interpreter.NewIntersectionStaticType( + nil, + []*interpreter.InterfaceStaticType{ + interpreter.NewInterfaceStaticType( + nil, + nil, + fooBarQualifiedIdentifier, + common.NewTypeIDFromQualifiedName( + nil, + fooAddressLocation, + fooBarQualifiedIdentifier, + ), + ), + }, + ) + } + + newIntersectionStaticTypeWithTwoInterfaces := func() *interpreter.IntersectionStaticType { + return interpreter.NewIntersectionStaticType( + nil, + []*interpreter.InterfaceStaticType{ + interpreter.NewInterfaceStaticType( + nil, + nil, + fooBarQualifiedIdentifier, + common.NewTypeIDFromQualifiedName( + nil, + fooAddressLocation, + fooBarQualifiedIdentifier, + ), + ), + interpreter.NewInterfaceStaticType( + nil, + nil, + fooBazQualifiedIdentifier, + common.NewTypeIDFromQualifiedName( + nil, + fooAddressLocation, + fooBazQualifiedIdentifier, + ), + ), + }, + ) + } + + type testCase struct { + storedType interpreter.StaticType + expectedType interpreter.StaticType + } + + testCases := map[string]testCase{ + // base cases + "primitive": { + storedType: stringType, + expectedType: nil, + }, + "intersection_without_interfaces": { + storedType: newIntersectionStaticTypeWithoutInterfaces(), + expectedType: nil, + }, + "intersection_with_one_interface": { + storedType: newIntersectionStaticTypeWithOneInterface(), + expectedType: nil, + }, + "intersection_with_two_interfaces": { + storedType: newIntersectionStaticTypeWithTwoInterfaces(), + expectedType: newIntersectionStaticTypeWithTwoInterfaces(), + }, + // optional + "optional_primitive": { + storedType: interpreter.NewOptionalStaticType(nil, stringType), + expectedType: nil, + }, + "optional_intersection_without_interfaces": { + storedType: interpreter.NewOptionalStaticType( + nil, + newIntersectionStaticTypeWithoutInterfaces(), + ), + expectedType: nil, + }, + "optional_intersection_with_one_interface": { + storedType: interpreter.NewOptionalStaticType( + nil, + newIntersectionStaticTypeWithOneInterface(), + ), + expectedType: nil, + }, + "optional_intersection_with_two_interfaces": { + storedType: interpreter.NewOptionalStaticType( + nil, + newIntersectionStaticTypeWithTwoInterfaces(), + ), + expectedType: interpreter.NewOptionalStaticType( + nil, + newIntersectionStaticTypeWithTwoInterfaces(), + ), + }, + // constant-sized array + "constant_sized_array_of_primitive": { + storedType: interpreter.NewConstantSizedStaticType(nil, stringType, 3), + expectedType: nil, + }, + "constant_sized_array_of_intersection_without_interfaces": { + storedType: interpreter.NewConstantSizedStaticType( + nil, + newIntersectionStaticTypeWithoutInterfaces(), + 3, + ), + expectedType: nil, + }, + "constant_sized_array_of_intersection_with_one_interface": { + storedType: interpreter.NewConstantSizedStaticType( + nil, + newIntersectionStaticTypeWithOneInterface(), + 3, + ), + expectedType: nil, + }, + "constant_sized_array_of_intersection_with_two_interfaces": { + storedType: interpreter.NewConstantSizedStaticType( + nil, + newIntersectionStaticTypeWithTwoInterfaces(), + 3, + ), + expectedType: interpreter.NewConstantSizedStaticType( + nil, + newIntersectionStaticTypeWithTwoInterfaces(), + 3, + ), + }, + // variable-sized array + "variable_sized_array_of_primitive": { + storedType: interpreter.NewVariableSizedStaticType(nil, stringType), + expectedType: nil, + }, + "variable_sized_array_of_intersection_without_interfaces": { + storedType: interpreter.NewVariableSizedStaticType( + nil, + newIntersectionStaticTypeWithoutInterfaces(), + ), + expectedType: nil, + }, + "variable_sized_array_of_intersection_with_one_interface": { + storedType: interpreter.NewVariableSizedStaticType( + nil, + newIntersectionStaticTypeWithOneInterface(), + ), + expectedType: nil, + }, + "variable_sized_array_of_intersection_with_two_interfaces": { + storedType: interpreter.NewVariableSizedStaticType( + nil, + newIntersectionStaticTypeWithTwoInterfaces(), + ), + expectedType: interpreter.NewVariableSizedStaticType( + nil, + newIntersectionStaticTypeWithTwoInterfaces(), + ), + }, + // dictionary + "dictionary_of_primitive_key_and_value": { + storedType: interpreter.NewDictionaryStaticType(nil, stringType, stringType), + expectedType: nil, + }, + "dictionary_of_intersection_without_interfaces_key": { + storedType: interpreter.NewDictionaryStaticType( + nil, + newIntersectionStaticTypeWithoutInterfaces(), + stringType, + ), + expectedType: nil, + }, + "dictionary_of_intersection_without_interfaces_value": { + storedType: interpreter.NewDictionaryStaticType( + nil, + stringType, + newIntersectionStaticTypeWithoutInterfaces(), + ), + expectedType: nil, + }, + "dictionary_of_intersection_with_one_interface_key": { + storedType: interpreter.NewDictionaryStaticType( + nil, + newIntersectionStaticTypeWithOneInterface(), + stringType, + ), + expectedType: nil, + }, + "dictionary_of_intersection_with_one_interface_value": { + storedType: interpreter.NewDictionaryStaticType( + nil, + stringType, + newIntersectionStaticTypeWithOneInterface(), + ), + expectedType: nil, + }, + "dictionary_of_intersection_with_two_interfaces_key": { + storedType: interpreter.NewDictionaryStaticType( + nil, + newIntersectionStaticTypeWithTwoInterfaces(), + stringType, + ), + expectedType: interpreter.NewDictionaryStaticType( + nil, + newIntersectionStaticTypeWithTwoInterfaces(), + stringType, + ), + }, + "dictionary_of_intersection_with_two_interfaces_value": { + storedType: interpreter.NewDictionaryStaticType( + nil, + stringType, + newIntersectionStaticTypeWithTwoInterfaces(), + ), + expectedType: interpreter.NewDictionaryStaticType( + nil, + stringType, + newIntersectionStaticTypeWithTwoInterfaces(), + ), + }, + // capability + "capability_primitive": { + storedType: interpreter.NewCapabilityStaticType(nil, stringType), + expectedType: nil, + }, + "capability_intersection_without_interfaces": { + storedType: interpreter.NewCapabilityStaticType( + nil, + newIntersectionStaticTypeWithoutInterfaces(), + ), + expectedType: nil, + }, + "capability_intersection_with_one_interface": { + storedType: interpreter.NewCapabilityStaticType( + nil, + newIntersectionStaticTypeWithOneInterface(), + ), + expectedType: nil, + }, + "capability_intersection_with_two_interfaces": { + storedType: interpreter.NewCapabilityStaticType( + nil, + newIntersectionStaticTypeWithTwoInterfaces(), + ), + expectedType: interpreter.NewCapabilityStaticType( + nil, + newIntersectionStaticTypeWithTwoInterfaces(), + ), + }, + // interface + "interface": { + storedType: interpreter.NewInterfaceStaticType( + nil, + nil, + "Bar", + common.NewTypeIDFromQualifiedName( + nil, + fooAddressLocation, + fooBarQualifiedIdentifier, + ), + ), + }, + // composite + "composite": { + storedType: interpreter.NewCompositeStaticType( + nil, + nil, + "Bar", + common.NewTypeIDFromQualifiedName( + nil, + fooAddressLocation, + fooBarQualifiedIdentifier, + ), + ), + }, + } + + // Store values + + ledger := runtime_utils.NewTestLedger(nil, nil) + storage := runtime.NewStorage(ledger, nil) + + inter, err := interpreter.NewInterpreter( + nil, + utils.TestLocation, + &interpreter.Config{ + Storage: storage, + AtreeValueValidationEnabled: false, + AtreeStorageValidationEnabled: true, + }, + ) + require.NoError(t, err) + + for name, testCase := range testCases { + storeTypeValue( + inter, + account, + pathDomain, + name, + testCase.storedType, + ) + } + + err = storage.Commit(inter, true) + require.NoError(t, err) + + // Migrate + + migration := migrations.NewStorageMigration(inter, storage) + + reporter := newTestReporter() + + migration.Migrate( + &migrations.AddressSliceIterator{ + Addresses: []common.Address{ + account, + }, + }, + migration.NewValueMigrationsPathMigrator( + reporter, + NewTypeValueMigration(), + ), + ) + + migration.Commit() + + // Check reported migrated paths + for identifier, test := range testCases { + addressPath := interpreter.AddressPath{ + Address: account, + Path: interpreter.PathValue{ + Domain: pathDomain, + Identifier: identifier, + }, + } + + if test.expectedType == nil { + assert.NotContains(t, reporter.migratedPaths, addressPath) + } else { + assert.Contains(t, reporter.migratedPaths, addressPath) + } + } + + // Assert the migrated values. + // Traverse through the storage and see if the values are updated now. + + storageMap := storage.GetStorageMap(account, pathDomain.Identifier(), false) + require.NotNil(t, storageMap) + require.Greater(t, storageMap.Count(), uint64(0)) + + iterator := storageMap.Iterator(inter) + + for key, value := iterator.Next(); key != nil; key, value = iterator.Next() { + identifier := string(key.(interpreter.StringAtreeValue)) + + t.Run(identifier, func(t *testing.T) { + testCase, ok := testCases[identifier] + require.True(t, ok) + + var expectedValue interpreter.Value + if testCase.expectedType != nil { + expectedValue = interpreter.NewTypeValue(nil, testCase.expectedType) + + // `IntersectionType.LegacyType` is not considered in the `IntersectionType.Equal` method. + // Therefore, check for the legacy type equality manually. + typeValue := value.(interpreter.TypeValue) + if actualIntersectionType, ok := typeValue.Type.(*interpreter.IntersectionStaticType); ok { + expectedIntersectionType := testCase.expectedType.(*interpreter.IntersectionStaticType) + + if actualIntersectionType.LegacyType == nil { + assert.Nil(t, expectedIntersectionType.LegacyType) + } else { + assert.True(t, actualIntersectionType.LegacyType.Equal(expectedIntersectionType.LegacyType)) + } + } + } else { + expectedValue = interpreter.NewTypeValue(nil, testCase.storedType) + } + + utils.AssertValuesEqual(t, inter, expectedValue, value) + }) + } +} + +func storeTypeValue( + inter *interpreter.Interpreter, + address common.Address, + domain common.PathDomain, + pathIdentifier string, + staticType interpreter.StaticType, +) { + inter.WriteStored( + address, + domain.Identifier(), + interpreter.StringStorageMapKey(pathIdentifier), + interpreter.NewTypeValue(inter, staticType), + ) +} From d6f14250a778304625caefe054f8fb0223a98f06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Mon, 18 Dec 2023 10:17:41 -0800 Subject: [PATCH 02/12] fix package name Co-authored-by: Supun Setunga --- migrations/type_value/migration.go | 2 +- migrations/type_value/migration_test.go | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/migrations/type_value/migration.go b/migrations/type_value/migration.go index 943749ea44..31b496ffa8 100644 --- a/migrations/type_value/migration.go +++ b/migrations/type_value/migration.go @@ -16,7 +16,7 @@ * limitations under the License. */ -package account_type +package type_value import ( "github.com/onflow/cadence/migrations" diff --git a/migrations/type_value/migration_test.go b/migrations/type_value/migration_test.go index fa2ff14817..f59755bf02 100644 --- a/migrations/type_value/migration_test.go +++ b/migrations/type_value/migration_test.go @@ -16,7 +16,7 @@ * limitations under the License. */ -package account_type +package type_value import ( "testing" From 3936f9caef783fd76db7fdf435154451f3ec352e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Wed, 20 Dec 2023 15:29:22 -0800 Subject: [PATCH 03/12] insert directly, without wrapping in optional --- migrations/migration.go | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/migrations/migration.go b/migrations/migration.go index 7cb4dccbb5..b0a4ebc88a 100644 --- a/migrations/migration.go +++ b/migrations/migration.go @@ -253,10 +253,7 @@ func (m *StorageMigration) migrateNestedValue( valueToSet = newValue } - // Always wrap with an optional, when inserting to the dictionary. - valueToSet = interpreter.NewUnmeteredSomeValueNonCopying(valueToSet) - - dictionary.SetKey( + dictionary.Insert( m.interpreter, emptyLocationRange, keyToSet, From 5184797a4670022ac907651a1d7af4bf9ea02e6f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Wed, 20 Dec 2023 15:30:10 -0800 Subject: [PATCH 04/12] clean up --- migrations/account_type/migration_test.go | 8 ++++---- migrations/migration.go | 2 +- migrations/migration_test.go | 4 ++-- migrations/string_normalization/migration_test.go | 4 ++-- runtime/interpreter/value.go | 2 +- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/migrations/account_type/migration_test.go b/migrations/account_type/migration_test.go index 515b5f4018..6b65415d17 100644 --- a/migrations/account_type/migration_test.go +++ b/migrations/account_type/migration_test.go @@ -30,7 +30,7 @@ import ( "github.com/onflow/cadence/runtime" "github.com/onflow/cadence/runtime/common" "github.com/onflow/cadence/runtime/interpreter" - "github.com/onflow/cadence/runtime/tests/runtime_utils" + . "github.com/onflow/cadence/runtime/tests/runtime_utils" "github.com/onflow/cadence/runtime/tests/utils" ) @@ -321,7 +321,7 @@ func TestTypeValueMigration(t *testing.T) { // Store values - ledger := runtime_utils.NewTestLedger(nil, nil) + ledger := NewTestLedger(nil, nil) storage := runtime.NewStorage(ledger, nil) inter, err := interpreter.NewInterpreter( @@ -451,7 +451,7 @@ func TestNestedTypeValueMigration(t *testing.T) { expectedAccountTypeValue := interpreter.NewTypeValue(nil, unauthorizedAccountReferenceType) stringTypeValue := interpreter.NewTypeValue(nil, interpreter.PrimitiveStaticTypeString) - ledger := runtime_utils.NewTestLedger(nil, nil) + ledger := NewTestLedger(nil, nil) storage := runtime.NewStorage(ledger, nil) locationRange := interpreter.EmptyLocationRange @@ -728,7 +728,7 @@ func TestValuesWithStaticTypeMigration(t *testing.T) { expectedValue interpreter.Value } - ledger := runtime_utils.NewTestLedger(nil, nil) + ledger := NewTestLedger(nil, nil) storage := runtime.NewStorage(ledger, nil) locationRange := interpreter.EmptyLocationRange diff --git a/migrations/migration.go b/migrations/migration.go index b0a4ebc88a..4cb8e0668f 100644 --- a/migrations/migration.go +++ b/migrations/migration.go @@ -169,7 +169,7 @@ func (m *StorageMigration) migrateNestedValue( for _, fieldName := range fieldNames { existingValue := composite.GetField( m.interpreter, - interpreter.EmptyLocationRange, + emptyLocationRange, fieldName, ) diff --git a/migrations/migration_test.go b/migrations/migration_test.go index 3e3773efd9..3e563e34f6 100644 --- a/migrations/migration_test.go +++ b/migrations/migration_test.go @@ -29,7 +29,7 @@ import ( "github.com/onflow/cadence/runtime" "github.com/onflow/cadence/runtime/common" "github.com/onflow/cadence/runtime/interpreter" - "github.com/onflow/cadence/runtime/tests/runtime_utils" + . "github.com/onflow/cadence/runtime/tests/runtime_utils" "github.com/onflow/cadence/runtime/tests/utils" ) @@ -106,7 +106,7 @@ func TestMultipleMigrations(t *testing.T) { expectedValue interpreter.Value } - ledger := runtime_utils.NewTestLedger(nil, nil) + ledger := NewTestLedger(nil, nil) storage := runtime.NewStorage(ledger, nil) locationRange := interpreter.EmptyLocationRange diff --git a/migrations/string_normalization/migration_test.go b/migrations/string_normalization/migration_test.go index 16de687dee..c149fc1366 100644 --- a/migrations/string_normalization/migration_test.go +++ b/migrations/string_normalization/migration_test.go @@ -29,7 +29,7 @@ import ( "github.com/onflow/cadence/runtime" "github.com/onflow/cadence/runtime/common" "github.com/onflow/cadence/runtime/interpreter" - "github.com/onflow/cadence/runtime/tests/runtime_utils" + . "github.com/onflow/cadence/runtime/tests/runtime_utils" "github.com/onflow/cadence/runtime/tests/utils" ) @@ -44,7 +44,7 @@ func TestStringNormalizingMigration(t *testing.T) { expectedValue interpreter.Value } - ledger := runtime_utils.NewTestLedger(nil, nil) + ledger := NewTestLedger(nil, nil) storage := runtime.NewStorage(ledger, nil) locationRange := interpreter.EmptyLocationRange diff --git a/runtime/interpreter/value.go b/runtime/interpreter/value.go index d886fb9238..24c6e0947f 100644 --- a/runtime/interpreter/value.go +++ b/runtime/interpreter/value.go @@ -510,7 +510,7 @@ func (TypeValue) ChildStorables() []atree.Storable { // HashInput returns a byte slice containing: // - HashInputTypeType (1 byte) // - type id (n bytes) -func (v TypeValue) HashInput(interpreter *Interpreter, _ LocationRange, scratch []byte) []byte { +func (v TypeValue) HashInput(_ *Interpreter, _ LocationRange, scratch []byte) []byte { typeID := v.Type.ID() length := 1 + len(typeID) From 5b8a1478445c453205d8cc82421156e3ef4684e3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Wed, 20 Dec 2023 15:32:12 -0800 Subject: [PATCH 05/12] get values when getting keys --- migrations/migration.go | 26 ++++++++++++++++++-------- 1 file changed, 18 insertions(+), 8 deletions(-) diff --git a/migrations/migration.go b/migrations/migration.go index 4cb8e0668f..055c89e48f 100644 --- a/migrations/migration.go +++ b/migrations/migration.go @@ -197,19 +197,29 @@ func (m *StorageMigration) migrateNestedValue( case *interpreter.DictionaryValue: dictionary := value + type keyValuePair struct { + key, value interpreter.Value + } + // Read the keys first, so the iteration wouldn't be affected // by the modification of the nested values. - var existingKeys []interpreter.Value - dictionary.IterateKeys(m.interpreter, func(key interpreter.Value) (resume bool) { - existingKeys = append(existingKeys, key) + var existingKeysAndValues []keyValuePair + dictionary.Iterate(m.interpreter, func(key, value interpreter.Value) (resume bool) { + existingKeysAndValues = append( + existingKeysAndValues, + keyValuePair{ + key: key, + value: value, + }, + ) + + // continue iteration return true }) - for _, existingKey := range existingKeys { - existingValue, exist := dictionary.Get(nil, interpreter.EmptyLocationRange, existingKey) - if !exist { - panic(errors.NewUnreachableError()) - } + for _, existingKeyAndValue := range existingKeysAndValues { + existingKey := existingKeyAndValue.key + existingValue := existingKeyAndValue.value newKey := m.migrateNestedValue( addressPath, From fae34407d1aed55d9115ba7a5432ccf562044c41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Wed, 20 Dec 2023 15:33:51 -0800 Subject: [PATCH 06/12] clean up --- migrations/type_value/migration_test.go | 115 ++++++++++++------------ 1 file changed, 58 insertions(+), 57 deletions(-) diff --git a/migrations/type_value/migration_test.go b/migrations/type_value/migration_test.go index f59755bf02..93a0291736 100644 --- a/migrations/type_value/migration_test.go +++ b/migrations/type_value/migration_test.go @@ -28,7 +28,7 @@ import ( "github.com/onflow/cadence/runtime" "github.com/onflow/cadence/runtime/common" "github.com/onflow/cadence/runtime/interpreter" - "github.com/onflow/cadence/runtime/tests/runtime_utils" + . "github.com/onflow/cadence/runtime/tests/runtime_utils" "github.com/onflow/cadence/runtime/tests/utils" ) @@ -51,71 +51,72 @@ func (t *testReporter) Report( t.migratedPaths[addressPath] = struct{}{} } -func TestTypeValueMigration(t *testing.T) { - t.Parallel() +const fooBarQualifiedIdentifier = "Foo.Bar" +const fooBazQualifiedIdentifier = "Foo.Baz" - account := common.Address{0x42} - pathDomain := common.PathDomainPublic +var testAddress = common.Address{0x42} - const stringType = interpreter.PrimitiveStaticTypeString +var fooAddressLocation = common.NewAddressLocation(nil, testAddress, "Foo") - const fooBarQualifiedIdentifier = "Foo.Bar" - const fooBazQualifiedIdentifier = "Foo.Baz" - - fooAddressLocation := common.NewAddressLocation(nil, account, "Foo") - - newIntersectionStaticTypeWithoutInterfaces := func() *interpreter.IntersectionStaticType { - return interpreter.NewIntersectionStaticType( - nil, - []*interpreter.InterfaceStaticType{}, - ) - } +func newIntersectionStaticTypeWithoutInterfaces() *interpreter.IntersectionStaticType { + return interpreter.NewIntersectionStaticType( + nil, + []*interpreter.InterfaceStaticType{}, + ) +} - newIntersectionStaticTypeWithOneInterface := func() *interpreter.IntersectionStaticType { - return interpreter.NewIntersectionStaticType( - nil, - []*interpreter.InterfaceStaticType{ - interpreter.NewInterfaceStaticType( - nil, +func newIntersectionStaticTypeWithOneInterface() *interpreter.IntersectionStaticType { + return interpreter.NewIntersectionStaticType( + nil, + []*interpreter.InterfaceStaticType{ + interpreter.NewInterfaceStaticType( + nil, + nil, + fooBarQualifiedIdentifier, + common.NewTypeIDFromQualifiedName( nil, + fooAddressLocation, fooBarQualifiedIdentifier, - common.NewTypeIDFromQualifiedName( - nil, - fooAddressLocation, - fooBarQualifiedIdentifier, - ), ), - }, - ) - } + ), + }, + ) +} - newIntersectionStaticTypeWithTwoInterfaces := func() *interpreter.IntersectionStaticType { - return interpreter.NewIntersectionStaticType( - nil, - []*interpreter.InterfaceStaticType{ - interpreter.NewInterfaceStaticType( - nil, +func newIntersectionStaticTypeWithTwoInterfaces() *interpreter.IntersectionStaticType { + return interpreter.NewIntersectionStaticType( + nil, + []*interpreter.InterfaceStaticType{ + interpreter.NewInterfaceStaticType( + nil, + nil, + fooBarQualifiedIdentifier, + common.NewTypeIDFromQualifiedName( nil, + fooAddressLocation, fooBarQualifiedIdentifier, - common.NewTypeIDFromQualifiedName( - nil, - fooAddressLocation, - fooBarQualifiedIdentifier, - ), ), - interpreter.NewInterfaceStaticType( - nil, + ), + interpreter.NewInterfaceStaticType( + nil, + nil, + fooBazQualifiedIdentifier, + common.NewTypeIDFromQualifiedName( nil, + fooAddressLocation, fooBazQualifiedIdentifier, - common.NewTypeIDFromQualifiedName( - nil, - fooAddressLocation, - fooBazQualifiedIdentifier, - ), ), - }, - ) - } + ), + }, + ) +} + +func TestTypeValueMigration(t *testing.T) { + t.Parallel() + + pathDomain := common.PathDomainPublic + + const stringType = interpreter.PrimitiveStaticTypeString type testCase struct { storedType interpreter.StaticType @@ -351,7 +352,7 @@ func TestTypeValueMigration(t *testing.T) { // Store values - ledger := runtime_utils.NewTestLedger(nil, nil) + ledger := NewTestLedger(nil, nil) storage := runtime.NewStorage(ledger, nil) inter, err := interpreter.NewInterpreter( @@ -368,7 +369,7 @@ func TestTypeValueMigration(t *testing.T) { for name, testCase := range testCases { storeTypeValue( inter, - account, + testAddress, pathDomain, name, testCase.storedType, @@ -387,7 +388,7 @@ func TestTypeValueMigration(t *testing.T) { migration.Migrate( &migrations.AddressSliceIterator{ Addresses: []common.Address{ - account, + testAddress, }, }, migration.NewValueMigrationsPathMigrator( @@ -401,7 +402,7 @@ func TestTypeValueMigration(t *testing.T) { // Check reported migrated paths for identifier, test := range testCases { addressPath := interpreter.AddressPath{ - Address: account, + Address: testAddress, Path: interpreter.PathValue{ Domain: pathDomain, Identifier: identifier, @@ -418,7 +419,7 @@ func TestTypeValueMigration(t *testing.T) { // Assert the migrated values. // Traverse through the storage and see if the values are updated now. - storageMap := storage.GetStorageMap(account, pathDomain.Identifier(), false) + storageMap := storage.GetStorageMap(testAddress, pathDomain.Identifier(), false) require.NotNil(t, storageMap) require.Greater(t, storageMap.Count(), uint64(0)) From 0a7e31b8daa6b1d7787ce9fa2da2771355c6bb49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Wed, 20 Dec 2023 15:34:11 -0800 Subject: [PATCH 07/12] ensure removed key existed --- migrations/migration.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/migrations/migration.go b/migrations/migration.go index 055c89e48f..01027251bd 100644 --- a/migrations/migration.go +++ b/migrations/migration.go @@ -248,11 +248,15 @@ func (m *StorageMigration) migrateNestedValue( // Key was migrated. // Remove the old value at the old key. // This old value will be inserted again with the new key, unless the value is also migrated. - _ = dictionary.RemoveKey( + oldValue := dictionary.Remove( m.interpreter, emptyLocationRange, existingKey, ) + if _, ok := oldValue.(*interpreter.SomeValue); !ok { + panic(errors.NewUnreachableError()) + } + keyToSet = newKey } From bde964117f3cf440cbf56e10107326040aba447b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Wed, 20 Dec 2023 15:42:47 -0800 Subject: [PATCH 08/12] start adding "re-hashing" test --- migrations/type_value/migration_test.go | 173 ++++++++++++++++++++++++ 1 file changed, 173 insertions(+) diff --git a/migrations/type_value/migration_test.go b/migrations/type_value/migration_test.go index 93a0291736..41e531e143 100644 --- a/migrations/type_value/migration_test.go +++ b/migrations/type_value/migration_test.go @@ -19,8 +19,10 @@ package type_value import ( + "strings" "testing" + "github.com/onflow/atree" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -471,3 +473,174 @@ func storeTypeValue( interpreter.NewTypeValue(inter, staticType), ) } + +// testIntersectionType simulates the old, incorrect restricted type type ID generation, +// which did not sort the type IDs of the interface types. +type testIntersectionType struct { + *interpreter.IntersectionStaticType + generatedTypeID bool +} + +var _ interpreter.StaticType = &testIntersectionType{} + +func (t *testIntersectionType) ID() common.TypeID { + t.generatedTypeID = true + + interfaceTypeIDs := make([]string, 0, len(t.Types)) + for _, interfaceType := range t.Types { + interfaceTypeIDs = append( + interfaceTypeIDs, + string(interfaceType.ID()), + ) + } + + var result strings.Builder + result.WriteByte('{') + // NOTE: no sorting + for i, interfaceTypeID := range interfaceTypeIDs { + if i > 0 { + result.WriteByte(',') + } + result.WriteString(interfaceTypeID) + } + result.WriteByte('}') + return common.TypeID(result.String()) +} + +// TestRehash stores a dictionary in storage, +// which has a key that is a type value with a restricted type that has two interface types, +// runs the migration, and ensures the dictionary is still usable +func TestRehash(t *testing.T) { + + t.Parallel() + + locationRange := interpreter.EmptyLocationRange + + ledger := NewTestLedger(nil, nil) + + storageMapKey := interpreter.StringStorageMapKey("dict") + newTestValue := func() interpreter.Value { + return interpreter.NewUnmeteredStringValue("test") + } + + newStorageAndInterpreter := func(t *testing.T) (*runtime.Storage, *interpreter.Interpreter) { + storage := runtime.NewStorage(ledger, nil) + inter, err := interpreter.NewInterpreter( + nil, + utils.TestLocation, + &interpreter.Config{ + Storage: storage, + AtreeValueValidationEnabled: false, + AtreeStorageValidationEnabled: true, + }, + ) + require.NoError(t, err) + + return storage, inter + } + + t.Run("prepare", func(t *testing.T) { + + storage, inter := newStorageAndInterpreter(t) + + dictionaryStaticType := interpreter.NewDictionaryStaticType( + nil, + interpreter.PrimitiveStaticTypeMetaType, + interpreter.PrimitiveStaticTypeString, + ) + dictValue := interpreter.NewDictionaryValue(inter, locationRange, dictionaryStaticType) + + // TODO: wrap in &testIntersectionType + intersectionType := newIntersectionStaticTypeWithTwoInterfaces() + + typeValue := interpreter.NewUnmeteredTypeValue(intersectionType) + + dictValue.Insert( + inter, + locationRange, + typeValue, + newTestValue(), + ) + + // TODO: assert.True(t, intersectionType.generatedTypeID) + + storageMap := storage.GetStorageMap( + testAddress, + common.PathDomainStorage.Identifier(), + true, + ) + + storageMap.SetValue(inter, + storageMapKey, + dictValue.Transfer( + inter, + locationRange, + atree.Address(testAddress), + false, + nil, + nil, + ), + ) + + err := storage.Commit(inter, false) + require.NoError(t, err) + }) + + t.Run("migrate", func(t *testing.T) { + + storage, inter := newStorageAndInterpreter(t) + + migration := migrations.NewStorageMigration(inter, storage) + + reporter := newTestReporter() + + migration.Migrate( + &migrations.AddressSliceIterator{ + Addresses: []common.Address{ + testAddress, + }, + }, + migration.NewValueMigrationsPathMigrator( + reporter, + NewTypeValueMigration(), + ), + ) + + migration.Commit() + + require.Equal(t, + map[interpreter.AddressPath]struct{}{ + { + Address: testAddress, + Path: interpreter.PathValue{ + Domain: common.PathDomainStorage, + Identifier: string(storageMapKey), + }, + }: {}, + }, + reporter.migratedPaths, + ) + }) + + // TODO: re-enable + //t.Run("load", func(t *testing.T) { + // + // storage, inter := newStorageAndInterpreter(t) + // + // storageMap := storage.GetStorageMap(testAddress, common.PathDomainStorage.Identifier(), false) + // storedValue := storageMap.ReadValue(inter, storageMapKey) + // + // require.IsType(t, &interpreter.DictionaryValue{}, storedValue) + // + // dictValue := storedValue.(*interpreter.DictionaryValue) + // + // typeValue := interpreter.NewUnmeteredTypeValue(newIntersectionStaticTypeWithTwoInterfaces()) + // + // value, ok := dictValue.Get(inter, locationRange, typeValue) + // require.True(t, ok) + // + // require.IsType(t, &interpreter.StringValue{}, value) + // require.Equal(t, value.(*interpreter.StringValue), testValue) + //}) + +} From d3a8a45c0c35253b77d7237209bd3b5697605cdb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Wed, 20 Dec 2023 16:00:24 -0800 Subject: [PATCH 09/12] fix locations of interface types --- migrations/type_value/migration_test.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/migrations/type_value/migration_test.go b/migrations/type_value/migration_test.go index 41e531e143..111e4f95d3 100644 --- a/migrations/type_value/migration_test.go +++ b/migrations/type_value/migration_test.go @@ -73,7 +73,7 @@ func newIntersectionStaticTypeWithOneInterface() *interpreter.IntersectionStatic []*interpreter.InterfaceStaticType{ interpreter.NewInterfaceStaticType( nil, - nil, + fooAddressLocation, fooBarQualifiedIdentifier, common.NewTypeIDFromQualifiedName( nil, @@ -91,7 +91,7 @@ func newIntersectionStaticTypeWithTwoInterfaces() *interpreter.IntersectionStati []*interpreter.InterfaceStaticType{ interpreter.NewInterfaceStaticType( nil, - nil, + fooAddressLocation, fooBarQualifiedIdentifier, common.NewTypeIDFromQualifiedName( nil, @@ -101,7 +101,7 @@ func newIntersectionStaticTypeWithTwoInterfaces() *interpreter.IntersectionStati ), interpreter.NewInterfaceStaticType( nil, - nil, + fooAddressLocation, fooBazQualifiedIdentifier, common.NewTypeIDFromQualifiedName( nil, From 4478943b50d48adef61568da6acfd8b27fc050f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Wed, 20 Dec 2023 16:05:23 -0800 Subject: [PATCH 10/12] complete test --- migrations/type_value/migration_test.go | 48 +++++++++++++------------ 1 file changed, 25 insertions(+), 23 deletions(-) diff --git a/migrations/type_value/migration_test.go b/migrations/type_value/migration_test.go index 111e4f95d3..fb751d00b2 100644 --- a/migrations/type_value/migration_test.go +++ b/migrations/type_value/migration_test.go @@ -550,8 +550,9 @@ func TestRehash(t *testing.T) { ) dictValue := interpreter.NewDictionaryValue(inter, locationRange, dictionaryStaticType) - // TODO: wrap in &testIntersectionType - intersectionType := newIntersectionStaticTypeWithTwoInterfaces() + intersectionType := &testIntersectionType{ + IntersectionStaticType: newIntersectionStaticTypeWithTwoInterfaces(), + } typeValue := interpreter.NewUnmeteredTypeValue(intersectionType) @@ -562,7 +563,7 @@ func TestRehash(t *testing.T) { newTestValue(), ) - // TODO: assert.True(t, intersectionType.generatedTypeID) + assert.True(t, intersectionType.generatedTypeID) storageMap := storage.GetStorageMap( testAddress, @@ -622,25 +623,26 @@ func TestRehash(t *testing.T) { ) }) - // TODO: re-enable - //t.Run("load", func(t *testing.T) { - // - // storage, inter := newStorageAndInterpreter(t) - // - // storageMap := storage.GetStorageMap(testAddress, common.PathDomainStorage.Identifier(), false) - // storedValue := storageMap.ReadValue(inter, storageMapKey) - // - // require.IsType(t, &interpreter.DictionaryValue{}, storedValue) - // - // dictValue := storedValue.(*interpreter.DictionaryValue) - // - // typeValue := interpreter.NewUnmeteredTypeValue(newIntersectionStaticTypeWithTwoInterfaces()) - // - // value, ok := dictValue.Get(inter, locationRange, typeValue) - // require.True(t, ok) - // - // require.IsType(t, &interpreter.StringValue{}, value) - // require.Equal(t, value.(*interpreter.StringValue), testValue) - //}) + t.Run("load", func(t *testing.T) { + + storage, inter := newStorageAndInterpreter(t) + + storageMap := storage.GetStorageMap(testAddress, common.PathDomainStorage.Identifier(), false) + storedValue := storageMap.ReadValue(inter, storageMapKey) + + require.IsType(t, &interpreter.DictionaryValue{}, storedValue) + + dictValue := storedValue.(*interpreter.DictionaryValue) + typeValue := interpreter.NewUnmeteredTypeValue(newIntersectionStaticTypeWithTwoInterfaces()) + + value, ok := dictValue.Get(inter, locationRange, typeValue) + require.True(t, ok) + + require.IsType(t, &interpreter.StringValue{}, value) + require.Equal(t, + newTestValue(), + value.(*interpreter.StringValue), + ) + }) } From 7d90491a6f9ed240db43f0813a417893aabe40c7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Wed, 20 Dec 2023 16:20:04 -0800 Subject: [PATCH 11/12] ensure original type value's type ID is in reverse order --- migrations/type_value/migration_test.go | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/migrations/type_value/migration_test.go b/migrations/type_value/migration_test.go index fb751d00b2..d710957cda 100644 --- a/migrations/type_value/migration_test.go +++ b/migrations/type_value/migration_test.go @@ -19,6 +19,7 @@ package type_value import ( + "sort" "strings" "testing" @@ -494,6 +495,9 @@ func (t *testIntersectionType) ID() common.TypeID { ) } + // NOTE: ensure the interface set is in *reverse* order + sort.Sort(sort.Reverse(sort.StringSlice(interfaceTypeIDs))) + var result strings.Builder result.WriteByte('{') // NOTE: no sorting @@ -565,6 +569,12 @@ func TestRehash(t *testing.T) { assert.True(t, intersectionType.generatedTypeID) + // NOTE: intentionally in reverse order + assert.Equal(t, + common.TypeID("{A.4200000000000000.Foo.Baz,A.4200000000000000.Foo.Bar}"), + intersectionType.ID(), + ) + storageMap := storage.GetStorageMap( testAddress, common.PathDomainStorage.Identifier(), @@ -634,7 +644,14 @@ func TestRehash(t *testing.T) { dictValue := storedValue.(*interpreter.DictionaryValue) - typeValue := interpreter.NewUnmeteredTypeValue(newIntersectionStaticTypeWithTwoInterfaces()) + intersectionType := newIntersectionStaticTypeWithTwoInterfaces() + typeValue := interpreter.NewUnmeteredTypeValue(intersectionType) + + // NOTE: in *sorted* order + assert.Equal(t, + common.TypeID("{A.4200000000000000.Foo.Bar,A.4200000000000000.Foo.Baz}"), + intersectionType.ID(), + ) value, ok := dictValue.Get(inter, locationRange, typeValue) require.True(t, ok) From ef70f1daf7fab6a3275fed2de103a677539e80af Mon Sep 17 00:00:00 2001 From: Supun Setunga Date: Thu, 21 Dec 2023 15:30:53 -0800 Subject: [PATCH 12/12] Use old hashing/ID generation algorithm to remove keys --- migrations/legacy_intersection_type.go | 56 ++++ migrations/migration.go | 71 +++++ migrations/type_value/migration_test.go | 368 +++++++++++++++++++++--- 3 files changed, 453 insertions(+), 42 deletions(-) create mode 100644 migrations/legacy_intersection_type.go diff --git a/migrations/legacy_intersection_type.go b/migrations/legacy_intersection_type.go new file mode 100644 index 0000000000..7f452c81f8 --- /dev/null +++ b/migrations/legacy_intersection_type.go @@ -0,0 +1,56 @@ +/* + * Cadence - The resource-oriented smart contract programming language + * + * Copyright Dapper Labs, Inc. + * + * 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 migrations + +import ( + "strings" + + "github.com/onflow/cadence/runtime/common" + "github.com/onflow/cadence/runtime/interpreter" +) + +// LegacyIntersectionType simulates the old, incorrect restricted-type type-ID generation, +// which did not sort the type IDs of the interface types. +type LegacyIntersectionType struct { + *interpreter.IntersectionStaticType +} + +var _ interpreter.StaticType = &LegacyIntersectionType{} + +func (t *LegacyIntersectionType) ID() common.TypeID { + interfaceTypeIDs := make([]string, 0, len(t.Types)) + for _, interfaceType := range t.Types { + interfaceTypeIDs = append( + interfaceTypeIDs, + string(interfaceType.ID()), + ) + } + + var result strings.Builder + result.WriteByte('{') + // NOTE: no sorting + for i, interfaceTypeID := range interfaceTypeIDs { + if i > 0 { + result.WriteByte(',') + } + result.WriteString(interfaceTypeID) + } + result.WriteByte('}') + return common.TypeID(result.String()) +} diff --git a/migrations/migration.go b/migrations/migration.go index 01027251bd..bcf2e90d68 100644 --- a/migrations/migration.go +++ b/migrations/migration.go @@ -248,11 +248,13 @@ func (m *StorageMigration) migrateNestedValue( // Key was migrated. // Remove the old value at the old key. // This old value will be inserted again with the new key, unless the value is also migrated. + existingKey = legacyKey(existingKey) oldValue := dictionary.Remove( m.interpreter, emptyLocationRange, existingKey, ) + if _, ok := oldValue.(*interpreter.SomeValue); !ok { panic(errors.NewUnreachableError()) } @@ -300,3 +302,72 @@ func (m *StorageMigration) migrateNestedValue( return } } + +// legacyKey return the same type with the "old" hash/ID generation algo. +func legacyKey(key interpreter.Value) interpreter.Value { + typeValue, isTypeValue := key.(interpreter.TypeValue) + if !isTypeValue { + return key + } + + legacyType := legacyType(typeValue.Type) + if legacyType == nil { + return key + } + + return interpreter.NewUnmeteredTypeValue(legacyType) +} + +func legacyType(staticType interpreter.StaticType) interpreter.StaticType { + switch typ := staticType.(type) { + case *interpreter.IntersectionStaticType: + return &LegacyIntersectionType{ + IntersectionStaticType: typ, + } + + case *interpreter.ConstantSizedStaticType: + legacyType := legacyType(typ.Type) + if legacyType != nil { + return interpreter.NewConstantSizedStaticType(nil, legacyType, typ.Size) + } + + case *interpreter.VariableSizedStaticType: + legacyType := legacyType(typ.Type) + if legacyType != nil { + return interpreter.NewVariableSizedStaticType(nil, legacyType) + } + + case *interpreter.DictionaryStaticType: + legacyKeyType := legacyType(typ.KeyType) + legacyValueType := legacyType(typ.ValueType) + if legacyKeyType != nil && legacyValueType != nil { + return interpreter.NewDictionaryStaticType(nil, legacyKeyType, legacyValueType) + } + if legacyKeyType != nil { + return interpreter.NewDictionaryStaticType(nil, legacyKeyType, typ.ValueType) + } + if legacyValueType != nil { + return interpreter.NewDictionaryStaticType(nil, typ.KeyType, legacyValueType) + } + + case *interpreter.OptionalStaticType: + legacyInnerType := legacyType(typ.Type) + if legacyInnerType != nil { + return interpreter.NewOptionalStaticType(nil, legacyInnerType) + } + + case *interpreter.CapabilityStaticType: + legacyBorrowType := legacyType(typ.BorrowType) + if legacyBorrowType != nil { + return interpreter.NewCapabilityStaticType(nil, legacyBorrowType) + } + + case *interpreter.ReferenceStaticType: + legacyReferencedType := legacyType(typ.ReferencedType) + if legacyReferencedType != nil { + return interpreter.NewReferenceStaticType(nil, typ.Authorization, legacyReferencedType) + } + } + + return nil +} diff --git a/migrations/type_value/migration_test.go b/migrations/type_value/migration_test.go index d710957cda..6d8ed2e6ff 100644 --- a/migrations/type_value/migration_test.go +++ b/migrations/type_value/migration_test.go @@ -19,8 +19,6 @@ package type_value import ( - "sort" - "strings" "testing" "github.com/onflow/atree" @@ -114,6 +112,34 @@ func newIntersectionStaticTypeWithTwoInterfaces() *interpreter.IntersectionStati ) } +func newIntersectionStaticTypeWithTwoInterfacesReversed() *interpreter.IntersectionStaticType { + return interpreter.NewIntersectionStaticType( + nil, + []*interpreter.InterfaceStaticType{ + interpreter.NewInterfaceStaticType( + nil, + fooAddressLocation, + fooBazQualifiedIdentifier, + common.NewTypeIDFromQualifiedName( + nil, + fooAddressLocation, + fooBazQualifiedIdentifier, + ), + ), + interpreter.NewInterfaceStaticType( + nil, + fooAddressLocation, + fooBarQualifiedIdentifier, + common.NewTypeIDFromQualifiedName( + nil, + fooAddressLocation, + fooBarQualifiedIdentifier, + ), + ), + }, + ) +} + func TestTypeValueMigration(t *testing.T) { t.Parallel() @@ -475,42 +501,6 @@ func storeTypeValue( ) } -// testIntersectionType simulates the old, incorrect restricted type type ID generation, -// which did not sort the type IDs of the interface types. -type testIntersectionType struct { - *interpreter.IntersectionStaticType - generatedTypeID bool -} - -var _ interpreter.StaticType = &testIntersectionType{} - -func (t *testIntersectionType) ID() common.TypeID { - t.generatedTypeID = true - - interfaceTypeIDs := make([]string, 0, len(t.Types)) - for _, interfaceType := range t.Types { - interfaceTypeIDs = append( - interfaceTypeIDs, - string(interfaceType.ID()), - ) - } - - // NOTE: ensure the interface set is in *reverse* order - sort.Sort(sort.Reverse(sort.StringSlice(interfaceTypeIDs))) - - var result strings.Builder - result.WriteByte('{') - // NOTE: no sorting - for i, interfaceTypeID := range interfaceTypeIDs { - if i > 0 { - result.WriteByte(',') - } - result.WriteString(interfaceTypeID) - } - result.WriteByte('}') - return common.TypeID(result.String()) -} - // TestRehash stores a dictionary in storage, // which has a key that is a type value with a restricted type that has two interface types, // runs the migration, and ensures the dictionary is still usable @@ -554,8 +544,8 @@ func TestRehash(t *testing.T) { ) dictValue := interpreter.NewDictionaryValue(inter, locationRange, dictionaryStaticType) - intersectionType := &testIntersectionType{ - IntersectionStaticType: newIntersectionStaticTypeWithTwoInterfaces(), + intersectionType := &migrations.LegacyIntersectionType{ + IntersectionStaticType: newIntersectionStaticTypeWithTwoInterfacesReversed(), } typeValue := interpreter.NewUnmeteredTypeValue(intersectionType) @@ -567,8 +557,6 @@ func TestRehash(t *testing.T) { newTestValue(), ) - assert.True(t, intersectionType.generatedTypeID) - // NOTE: intentionally in reverse order assert.Equal(t, common.TypeID("{A.4200000000000000.Foo.Baz,A.4200000000000000.Foo.Bar}"), @@ -663,3 +651,299 @@ func TestRehash(t *testing.T) { ) }) } + +// TestRehashNestedIntersectionType stores a dictionary in storage, +// which has a key that is a type value with a restricted type that has two interface types, +// runs the migration, and ensures the dictionary is still usable +func TestRehashNestedIntersectionType(t *testing.T) { + + locationRange := interpreter.EmptyLocationRange + + storageMapKey := interpreter.StringStorageMapKey("dict") + newTestValue := func() interpreter.Value { + return interpreter.NewUnmeteredStringValue("test") + } + + newStorageAndInterpreter := func(t *testing.T, ledger atree.Ledger) (*runtime.Storage, *interpreter.Interpreter) { + storage := runtime.NewStorage(ledger, nil) + inter, err := interpreter.NewInterpreter( + nil, + utils.TestLocation, + &interpreter.Config{ + Storage: storage, + AtreeValueValidationEnabled: false, + AtreeStorageValidationEnabled: true, + }, + ) + require.NoError(t, err) + + return storage, inter + } + + t.Run("array type", func(t *testing.T) { + t.Parallel() + + ledger := NewTestLedger(nil, nil) + + t.Run("prepare", func(t *testing.T) { + + storage, inter := newStorageAndInterpreter(t, ledger) + + dictionaryStaticType := interpreter.NewDictionaryStaticType( + nil, + interpreter.PrimitiveStaticTypeMetaType, + interpreter.PrimitiveStaticTypeString, + ) + dictValue := interpreter.NewDictionaryValue(inter, locationRange, dictionaryStaticType) + + intersectionType := &migrations.LegacyIntersectionType{ + IntersectionStaticType: newIntersectionStaticTypeWithTwoInterfacesReversed(), + } + + typeValue := interpreter.NewUnmeteredTypeValue( + interpreter.NewVariableSizedStaticType( + nil, + intersectionType, + ), + ) + + dictValue.Insert( + inter, + locationRange, + typeValue, + newTestValue(), + ) + + // NOTE: intentionally in reverse order + assert.Equal(t, + common.TypeID("{A.4200000000000000.Foo.Baz,A.4200000000000000.Foo.Bar}"), + intersectionType.ID(), + ) + + storageMap := storage.GetStorageMap( + testAddress, + common.PathDomainStorage.Identifier(), + true, + ) + + storageMap.SetValue(inter, + storageMapKey, + dictValue.Transfer( + inter, + locationRange, + atree.Address(testAddress), + false, + nil, + nil, + ), + ) + + err := storage.Commit(inter, false) + require.NoError(t, err) + }) + + t.Run("migrate", func(t *testing.T) { + + storage, inter := newStorageAndInterpreter(t, ledger) + + migration := migrations.NewStorageMigration(inter, storage) + + reporter := newTestReporter() + + migration.Migrate( + &migrations.AddressSliceIterator{ + Addresses: []common.Address{ + testAddress, + }, + }, + migration.NewValueMigrationsPathMigrator( + reporter, + NewTypeValueMigration(), + ), + ) + + migration.Commit() + + require.Equal(t, + map[interpreter.AddressPath]struct{}{ + { + Address: testAddress, + Path: interpreter.PathValue{ + Domain: common.PathDomainStorage, + Identifier: string(storageMapKey), + }, + }: {}, + }, + reporter.migratedPaths, + ) + }) + + t.Run("load", func(t *testing.T) { + + storage, inter := newStorageAndInterpreter(t, ledger) + + storageMap := storage.GetStorageMap(testAddress, common.PathDomainStorage.Identifier(), false) + storedValue := storageMap.ReadValue(inter, storageMapKey) + + require.IsType(t, &interpreter.DictionaryValue{}, storedValue) + + dictValue := storedValue.(*interpreter.DictionaryValue) + + intersectionType := newIntersectionStaticTypeWithTwoInterfaces() + typeValue := interpreter.NewUnmeteredTypeValue( + interpreter.NewVariableSizedStaticType(nil, intersectionType), + ) + + // NOTE: in *sorted* order + assert.Equal(t, + common.TypeID("{A.4200000000000000.Foo.Bar,A.4200000000000000.Foo.Baz}"), + intersectionType.ID(), + ) + + value, ok := dictValue.Get(inter, locationRange, typeValue) + require.True(t, ok) + + require.IsType(t, &interpreter.StringValue{}, value) + require.Equal(t, + newTestValue(), + value.(*interpreter.StringValue), + ) + }) + }) + + t.Run("dictionary type", func(t *testing.T) { + t.Parallel() + + ledger := NewTestLedger(nil, nil) + + t.Run("prepare", func(t *testing.T) { + + storage, inter := newStorageAndInterpreter(t, ledger) + + dictionaryStaticType := interpreter.NewDictionaryStaticType( + nil, + interpreter.PrimitiveStaticTypeMetaType, + interpreter.PrimitiveStaticTypeString, + ) + dictValue := interpreter.NewDictionaryValue(inter, locationRange, dictionaryStaticType) + + intersectionType := &migrations.LegacyIntersectionType{ + IntersectionStaticType: newIntersectionStaticTypeWithTwoInterfacesReversed(), + } + + typeValue := interpreter.NewUnmeteredTypeValue( + interpreter.NewDictionaryStaticType( + nil, + interpreter.PrimitiveStaticTypeString, + intersectionType, + ), + ) + + dictValue.Insert( + inter, + locationRange, + typeValue, + newTestValue(), + ) + + // NOTE: intentionally in reverse order + assert.Equal(t, + common.TypeID("{A.4200000000000000.Foo.Baz,A.4200000000000000.Foo.Bar}"), + intersectionType.ID(), + ) + + storageMap := storage.GetStorageMap( + testAddress, + common.PathDomainStorage.Identifier(), + true, + ) + + storageMap.SetValue(inter, + storageMapKey, + dictValue.Transfer( + inter, + locationRange, + atree.Address(testAddress), + false, + nil, + nil, + ), + ) + + err := storage.Commit(inter, false) + require.NoError(t, err) + }) + + t.Run("migrate", func(t *testing.T) { + + storage, inter := newStorageAndInterpreter(t, ledger) + + migration := migrations.NewStorageMigration(inter, storage) + + reporter := newTestReporter() + + migration.Migrate( + &migrations.AddressSliceIterator{ + Addresses: []common.Address{ + testAddress, + }, + }, + migration.NewValueMigrationsPathMigrator( + reporter, + NewTypeValueMigration(), + ), + ) + + migration.Commit() + + require.Equal(t, + map[interpreter.AddressPath]struct{}{ + { + Address: testAddress, + Path: interpreter.PathValue{ + Domain: common.PathDomainStorage, + Identifier: string(storageMapKey), + }, + }: {}, + }, + reporter.migratedPaths, + ) + }) + + t.Run("load", func(t *testing.T) { + + storage, inter := newStorageAndInterpreter(t, ledger) + + storageMap := storage.GetStorageMap(testAddress, common.PathDomainStorage.Identifier(), false) + storedValue := storageMap.ReadValue(inter, storageMapKey) + + require.IsType(t, &interpreter.DictionaryValue{}, storedValue) + + dictValue := storedValue.(*interpreter.DictionaryValue) + + intersectionType := newIntersectionStaticTypeWithTwoInterfaces() + typeValue := interpreter.NewUnmeteredTypeValue( + interpreter.NewDictionaryStaticType( + nil, + interpreter.PrimitiveStaticTypeString, + intersectionType, + ), + ) + + // NOTE: in *sorted* order + assert.Equal(t, + common.TypeID("{A.4200000000000000.Foo.Bar,A.4200000000000000.Foo.Baz}"), + intersectionType.ID(), + ) + + value, ok := dictValue.Get(inter, locationRange, typeValue) + require.True(t, ok) + + require.IsType(t, &interpreter.StringValue{}, value) + require.Equal(t, + newTestValue(), + value.(*interpreter.StringValue), + ) + }) + }) +}