From e21ea848605a6f102c080fdc96ca6259e0530250 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Thu, 29 Feb 2024 14:36:21 -0800 Subject: [PATCH 01/17] check storage health after migrating --- migrations/capcons/migration_test.go | 16 ++++- migrations/entitlements/migration_test.go | 63 ++++++++++++++++--- migrations/migration_test.go | 34 +++++++++- .../account_type_migration_test.go | 24 ++++++- .../composite_type_migration_test.go | 3 + .../intersection_type_migration_test.go | 18 ++++++ .../statictypes/statictype_migration_test.go | 10 ++- .../string_normalization/migration_test.go | 9 +++ 8 files changed, 159 insertions(+), 18 deletions(-) diff --git a/migrations/capcons/migration_test.go b/migrations/capcons/migration_test.go index 5905865fe7..649e970c56 100644 --- a/migrations/capcons/migration_test.go +++ b/migrations/capcons/migration_test.go @@ -514,6 +514,9 @@ func testPathCapabilityValueMigration( err = migration.Commit() require.NoError(t, err) + err = storage.CheckHealth() + require.NoError(t, err) + // Check migrated capabilities assert.Equal(t, @@ -1348,13 +1351,16 @@ func testLinkMigration( err = migration.Commit() require.NoError(t, err) + err = storage.CheckHealth() + require.NoError(t, err) + // Assert assert.Equal(t, expectedMigrations, reporter.migrations, ) - assert.Empty(t, + assert.Equal(t, expectedErrors, reporter.errors, ) @@ -2073,6 +2079,9 @@ func TestPublishedPathCapabilityValueMigration(t *testing.T) { err = migration.Commit() require.NoError(t, err) + err = storage.CheckHealth() + require.NoError(t, err) + // Check migrated capabilities assert.Equal(t, @@ -2320,18 +2329,21 @@ func TestUntypedPathCapabilityValueMigration(t *testing.T) { err = migration.Commit() require.NoError(t, err) + err = storage.CheckHealth() + require.NoError(t, err) + // Check migrated capabilities assert.Equal(t, expectedMigrations, reporter.migrations, ) + assert.Empty(t, reporter.errors) assert.Equal(t, expectedPathMigrations, reporter.pathCapabilityMigrations, ) require.Nil(t, reporter.missingCapabilityIDs) - require.Empty(t, reporter.errors) // Check diff --git a/migrations/entitlements/migration_test.go b/migrations/entitlements/migration_test.go index a46ae9fe58..113dce93fc 100644 --- a/migrations/entitlements/migration_test.go +++ b/migrations/entitlements/migration_test.go @@ -719,9 +719,12 @@ func convertEntireTestValue( err := migration.Commit() require.NoError(t, err) + err = storage.CheckHealth() + require.NoError(t, err) + // Assert - assert.Len(t, reporter.errors, 0) + assert.Empty(t, reporter.errors) if migratedValue == nil { return v @@ -1313,8 +1316,12 @@ func TestConvertToEntitledValue(t *testing.T) { }, } - test := func(testCase testCase, valueGenerator valueGenerator, typeGenerator typeGenerator) { - + test := func( + t *testing.T, + testCase testCase, + valueGenerator valueGenerator, + typeGenerator typeGenerator, + ) { input := valueGenerator.wrap(typeGenerator.wrap(testCase.Input)) if input == nil { return @@ -1324,6 +1331,9 @@ func TestConvertToEntitledValue(t *testing.T) { convertedValue := convertEntireTestValue(t, inter, storage, testAddress, input) + err := storage.CheckHealth() + require.NoError(t, err) + switch convertedValue := convertedValue.(type) { case interpreter.EquatableValue: require.True(t, @@ -1346,7 +1356,7 @@ func TestConvertToEntitledValue(t *testing.T) { for _, typeGenerator := range typeGenerators { t.Run(typeGenerator.name, func(t *testing.T) { - test(testCase, valueGenerator, typeGenerator) + test(t, testCase, valueGenerator, typeGenerator) }) } }) @@ -1517,7 +1527,10 @@ func TestMigrateSimpleContract(t *testing.T) { // Assert - assert.Len(t, reporter.errors, 0) + err = storage.CheckHealth() + require.NoError(t, err) + + assert.Empty(t, reporter.errors) storageMap := storage.GetStorageMap(account, storageIdentifier, false) require.NotNil(t, storageMap) @@ -1701,7 +1714,11 @@ func TestMigratePublishedValue(t *testing.T) { // Assert - assert.Len(t, reporter.errors, 0) + err = storage.CheckHealth() + require.NoError(t, err) + + assert.Empty(t, reporter.errors) + assert.Equal(t, map[struct { interpreter.StorageKey @@ -1955,7 +1972,11 @@ func TestMigratePublishedValueAcrossTwoAccounts(t *testing.T) { // Assert - assert.Len(t, reporter.errors, 0) + err = storage.CheckHealth() + require.NoError(t, err) + + assert.Empty(t, reporter.errors) + assert.Equal(t, map[struct { interpreter.StorageKey @@ -2404,7 +2425,11 @@ func TestMigrateArrayOfValues(t *testing.T) { // Assert - assert.Len(t, reporter.errors, 0) + err = storage.CheckHealth() + require.NoError(t, err) + + assert.Empty(t, reporter.errors) + assert.Equal(t, map[struct { interpreter.StorageKey @@ -2651,7 +2676,11 @@ func TestMigrateDictOfValues(t *testing.T) { // Assert - assert.Len(t, reporter.errors, 0) + err = storage.CheckHealth() + require.NoError(t, err) + + assert.Empty(t, reporter.errors) + assert.Equal(t, map[struct { interpreter.StorageKey @@ -2970,7 +2999,10 @@ func TestMigrateCapConsAcrossTwoAccounts(t *testing.T) { // Assert - assert.Len(t, reporter.errors, 0) + err = storage.CheckHealth() + require.NoError(t, err) + + assert.Empty(t, reporter.errors) assert.Len(t, reporter.migrated, 1) // TODO: assert @@ -3150,6 +3182,9 @@ func TestRehash(t *testing.T) { err := storage.Commit(inter, false) require.NoError(t, err) + + err = storage.CheckHealth() + require.NoError(t, err) }) t.Run("migrate", func(t *testing.T) { @@ -3196,6 +3231,11 @@ func TestRehash(t *testing.T) { err := migration.Commit() require.NoError(t, err) + err = storage.CheckHealth() + require.NoError(t, err) + + assert.Empty(t, reporter.errors) + require.Equal(t, map[struct { interpreter.StorageKey @@ -3217,6 +3257,9 @@ func TestRehash(t *testing.T) { storage, inter := newStorageAndInterpreter(t) + err := storage.CheckHealth() + require.NoError(t, err) + storageMap := storage.GetStorageMap( testAddress, common.PathDomainStorage.Identifier(), diff --git a/migrations/migration_test.go b/migrations/migration_test.go index e759fde504..34375f39b7 100644 --- a/migrations/migration_test.go +++ b/migrations/migration_test.go @@ -518,6 +518,11 @@ func TestMultipleMigrations(t *testing.T) { err = migration.Commit() require.NoError(t, err) + err = storage.CheckHealth() + require.NoError(t, err) + + assert.Empty(t, reporter.errored) + // Assert: Traverse through the storage and see if the values are updated now. for _, testCase := range testCases { @@ -660,6 +665,25 @@ func TestMigrationError(t *testing.T) { err = migration.Commit() require.NoError(t, err) + err = storage.CheckHealth() + require.NoError(t, err) + + assert.Equal(t, + map[struct { + interpreter.StorageKey + interpreter.StorageMapKey + }][]string{ + { + StorageKey: interpreter.StorageKey{ + Address: account, + Key: pathDomain.Identifier(), + }, + StorageMapKey: interpreter.StringStorageMapKey("int8_value"), + }: {"testInt8Migration"}, + }, + reporter.errored, + ) + // Assert: Traverse through the storage and see if the values are updated now. storageMap := storage.GetStorageMap(account, pathDomain.Identifier(), false) @@ -804,9 +828,12 @@ func TestCapConMigration(t *testing.T) { err = migration.Commit() require.NoError(t, err) + err = storage.CheckHealth() + require.NoError(t, err) + // Assert - assert.Len(t, reporter.errored, 0) + assert.Empty(t, reporter.errored) assert.Len(t, reporter.migrated, 2) storageMap = storage.GetStorageMap( @@ -919,9 +946,12 @@ func TestContractMigration(t *testing.T) { err = migration.Commit() require.NoError(t, err) + err = storage.CheckHealth() + require.NoError(t, err) + // Assert - assert.Len(t, reporter.errored, 0) + assert.Empty(t, reporter.errored) assert.Len(t, reporter.migrated, 1) value, err := rt.ExecuteScript( diff --git a/migrations/statictypes/account_type_migration_test.go b/migrations/statictypes/account_type_migration_test.go index 0dadb8bffc..9c78c7c3f1 100644 --- a/migrations/statictypes/account_type_migration_test.go +++ b/migrations/statictypes/account_type_migration_test.go @@ -485,6 +485,9 @@ func TestAccountTypeInTypeValueMigration(t *testing.T) { err = migration.Commit() require.NoError(t, err) + err = storage.CheckHealth() + require.NoError(t, err) + require.Empty(t, reporter.errors) storageMapKey := interpreter.StringStorageMapKey(name) @@ -812,6 +815,8 @@ func TestAccountTypeInNestedTypeValueMigration(t *testing.T) { migration := migrations.NewStorageMigration(inter, storage) + reporter := newTestReporter() + migration.Migrate( &migrations.AddressSliceIterator{ Addresses: []common.Address{ @@ -819,7 +824,7 @@ func TestAccountTypeInNestedTypeValueMigration(t *testing.T) { }, }, migration.NewValueMigrationsPathMigrator( - nil, + reporter, NewStaticTypeMigration(), ), ) @@ -827,6 +832,11 @@ func TestAccountTypeInNestedTypeValueMigration(t *testing.T) { err = migration.Commit() require.NoError(t, err) + err = storage.CheckHealth() + require.NoError(t, err) + + require.Empty(t, reporter.errors) + // Assert: Traverse through the storage and see if the values are updated now. storageMap := storage.GetStorageMap(account, pathDomain.Identifier(), false) @@ -1064,6 +1074,8 @@ func TestMigratingValuesWithAccountStaticType(t *testing.T) { migration := migrations.NewStorageMigration(inter, storage) + reporter := newTestReporter() + migration.Migrate( &migrations.AddressSliceIterator{ Addresses: []common.Address{ @@ -1071,7 +1083,7 @@ func TestMigratingValuesWithAccountStaticType(t *testing.T) { }, }, migration.NewValueMigrationsPathMigrator( - nil, + reporter, NewStaticTypeMigration(), ), ) @@ -1079,6 +1091,11 @@ func TestMigratingValuesWithAccountStaticType(t *testing.T) { err = migration.Commit() require.NoError(t, err) + err = storage.CheckHealth() + require.NoError(t, err) + + require.Empty(t, reporter.errors) + // Assert: Traverse through the storage and see if the values are updated now. storageMap := storage.GetStorageMap(account, pathDomain.Identifier(), false) @@ -1220,6 +1237,9 @@ func TestAccountTypeRehash(t *testing.T) { err := migration.Commit() require.NoError(t, err) + err = storage.CheckHealth() + require.NoError(t, err) + require.Empty(t, reporter.errors) require.Equal(t, diff --git a/migrations/statictypes/composite_type_migration_test.go b/migrations/statictypes/composite_type_migration_test.go index da57c92c1e..99031703a3 100644 --- a/migrations/statictypes/composite_type_migration_test.go +++ b/migrations/statictypes/composite_type_migration_test.go @@ -185,6 +185,9 @@ func TestCompositeAndInterfaceTypeMigration(t *testing.T) { err = migration.Commit() require.NoError(t, err) + err = storage.CheckHealth() + require.NoError(t, err) + require.Empty(t, reporter.errors) // Check reported migrated paths diff --git a/migrations/statictypes/intersection_type_migration_test.go b/migrations/statictypes/intersection_type_migration_test.go index 60fdd9acd2..0347c00e09 100644 --- a/migrations/statictypes/intersection_type_migration_test.go +++ b/migrations/statictypes/intersection_type_migration_test.go @@ -417,6 +417,9 @@ func TestIntersectionTypeMigration(t *testing.T) { err = migration.Commit() require.NoError(t, err) + err = storage.CheckHealth() + require.NoError(t, err) + require.Empty(t, reporter.errors) // Assert the migrated values. @@ -584,6 +587,9 @@ func TestIntersectionTypeRehash(t *testing.T) { err := migration.Commit() require.NoError(t, err) + err = storage.CheckHealth() + require.NoError(t, err) + require.Empty(t, reporter.errors) require.Equal(t, @@ -750,6 +756,9 @@ func TestRehashNestedIntersectionType(t *testing.T) { err := migration.Commit() require.NoError(t, err) + err = storage.CheckHealth() + require.NoError(t, err) + require.Empty(t, reporter.errors) require.Equal(t, @@ -891,6 +900,9 @@ func TestRehashNestedIntersectionType(t *testing.T) { err := migration.Commit() require.NoError(t, err) + err = storage.CheckHealth() + require.NoError(t, err) + require.Empty(t, reporter.errors) require.Equal(t, @@ -1065,6 +1077,9 @@ func TestIntersectionTypeMigrationWithInterfaceTypeConverter(t *testing.T) { err = migration.Commit() require.NoError(t, err) + err = storage.CheckHealth() + require.NoError(t, err) + require.Empty(t, reporter.errors) // Assert the migrated value. @@ -1437,6 +1452,9 @@ func TestIntersectionTypeMigrationWithTypeConverters(t *testing.T) { err = migration.Commit() require.NoError(t, err) + err = storage.CheckHealth() + require.NoError(t, err) + require.Empty(t, reporter.errors) key := struct { diff --git a/migrations/statictypes/statictype_migration_test.go b/migrations/statictypes/statictype_migration_test.go index a16194e8a5..99b16a1aef 100644 --- a/migrations/statictypes/statictype_migration_test.go +++ b/migrations/statictypes/statictype_migration_test.go @@ -93,7 +93,10 @@ func TestStaticTypeMigration(t *testing.T) { err = migration.Commit() require.NoError(t, err) - require.Empty(t, reporter.errors) + err = storage.CheckHealth() + require.NoError(t, err) + + assert.Empty(t, reporter.errors) storageMap := storage.GetStorageMap( testAddress, @@ -639,7 +642,10 @@ func TestMigratingNestedContainers(t *testing.T) { err = migration.Commit() require.NoError(t, err) - require.Empty(t, reporter.errors) + err = storage.CheckHealth() + require.NoError(t, err) + + assert.Empty(t, reporter.errors) storageMap := storage.GetStorageMap( testAddress, diff --git a/migrations/string_normalization/migration_test.go b/migrations/string_normalization/migration_test.go index b46621fba8..26cc028855 100644 --- a/migrations/string_normalization/migration_test.go +++ b/migrations/string_normalization/migration_test.go @@ -277,6 +277,9 @@ func TestStringNormalizingMigration(t *testing.T) { err = migration.Commit() require.NoError(t, err) + err = storage.CheckHealth() + require.NoError(t, err) + // Assert: Traverse through the storage and see if the values are updated now. storageMap := storage.GetStorageMap(account, pathDomain.Identifier(), false) @@ -415,6 +418,9 @@ func TestStringValueRehash(t *testing.T) { storage, inter := newStorageAndInterpreter(t) + err := storage.CheckHealth() + require.NoError(t, err) + storageMap := storage.GetStorageMap(testAddress, common.PathDomainStorage.Identifier(), false) storedValue := storageMap.ReadValue(inter, storageMapKey) @@ -554,6 +560,9 @@ func TestCharacterValueRehash(t *testing.T) { storage, inter := newStorageAndInterpreter(t) + err := storage.CheckHealth() + require.NoError(t, err) + storageMap := storage.GetStorageMap(testAddress, common.PathDomainStorage.Identifier(), false) storedValue := storageMap.ReadValue(inter, storageMapKey) From f9b1a87591343b1ee050952897f66532d4cd0a6b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Thu, 29 Feb 2024 14:41:10 -0800 Subject: [PATCH 02/17] implement AccountLinkValue.ChildStorables for atree storage health check --- runtime/interpreter/value_link.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runtime/interpreter/value_link.go b/runtime/interpreter/value_link.go index a32e29a42a..fc05142b49 100644 --- a/runtime/interpreter/value_link.go +++ b/runtime/interpreter/value_link.go @@ -278,7 +278,7 @@ func (v AccountLinkValue) StoredValue(_ atree.SlabStorage) (atree.Value, error) } func (v AccountLinkValue) ChildStorables() []atree.Storable { - panic(errors.NewUnreachableError()) + return nil } // NOTE: NEVER change, only add/increment; ensure uint64 From 1700a85ac3d8b2e09a12eed4e0fb8a754b0a15be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Mon, 4 Mar 2024 14:46:05 -0800 Subject: [PATCH 03/17] fix test case: transfer value to account before storing in storage map --- migrations/entitlements/migration_test.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/migrations/entitlements/migration_test.go b/migrations/entitlements/migration_test.go index 113dce93fc..0ec7d547e1 100644 --- a/migrations/entitlements/migration_test.go +++ b/migrations/entitlements/migration_test.go @@ -1494,11 +1494,20 @@ func TestMigrateSimpleContract(t *testing.T) { } for name, testCase := range testCases { + transferredValue := testCase.storedValue.Transfer( + inter, + interpreter.EmptyLocationRange, + atree.Address(account), + false, + nil, + nil, + ) + inter.WriteStored( account, storageIdentifier, interpreter.StringStorageMapKey(name), - testCase.storedValue, + transferredValue, ) } From 3f7853880897770dccdf027e479e57d9d9d9d507 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Mon, 4 Mar 2024 17:01:48 -0800 Subject: [PATCH 04/17] enable atree value and storage validation where possible --- migrations/entitlements/migration_test.go | 3 +- migrations/migration_test.go | 8 +++--- .../account_type_migration_test.go | 23 +++++++-------- .../composite_type_migration_test.go | 10 +++---- .../intersection_type_migration_test.go | 28 +++++++++---------- .../string_normalization/migration_test.go | 11 +++++--- 6 files changed, 44 insertions(+), 39 deletions(-) diff --git a/migrations/entitlements/migration_test.go b/migrations/entitlements/migration_test.go index 0ec7d547e1..f8fb2b61f1 100644 --- a/migrations/entitlements/migration_test.go +++ b/migrations/entitlements/migration_test.go @@ -3100,7 +3100,8 @@ func TestRehash(t *testing.T) { nil, utils.TestLocation, &interpreter.Config{ - Storage: storage, + Storage: storage, + // NOTE: disabled, because encoded and decoded values are expected to not match AtreeValueValidationEnabled: false, AtreeStorageValidationEnabled: true, }, diff --git a/migrations/migration_test.go b/migrations/migration_test.go index 34375f39b7..80bacae52a 100644 --- a/migrations/migration_test.go +++ b/migrations/migration_test.go @@ -242,8 +242,8 @@ func TestMultipleMigrations(t *testing.T) { utils.TestLocation, &interpreter.Config{ Storage: storage, - AtreeValueValidationEnabled: false, - AtreeStorageValidationEnabled: false, + AtreeValueValidationEnabled: true, + AtreeStorageValidationEnabled: true, }, ) require.NoError(t, err) @@ -597,8 +597,8 @@ func TestMigrationError(t *testing.T) { utils.TestLocation, &interpreter.Config{ Storage: storage, - AtreeValueValidationEnabled: false, - AtreeStorageValidationEnabled: false, + AtreeValueValidationEnabled: true, + AtreeStorageValidationEnabled: true, }, ) require.NoError(t, err) diff --git a/migrations/statictypes/account_type_migration_test.go b/migrations/statictypes/account_type_migration_test.go index 9c78c7c3f1..da9ae924e3 100644 --- a/migrations/statictypes/account_type_migration_test.go +++ b/migrations/statictypes/account_type_migration_test.go @@ -236,7 +236,7 @@ func TestAccountTypeInTypeValueMigration(t *testing.T) { []*interpreter.InterfaceStaticType{ interpreter.NewInterfaceStaticType( nil, - nil, + fooAddressLocation, fooBarQualifiedIdentifier, common.NewTypeIDFromQualifiedName( nil, @@ -251,7 +251,7 @@ func TestAccountTypeInTypeValueMigration(t *testing.T) { []*interpreter.InterfaceStaticType{ interpreter.NewInterfaceStaticType( nil, - nil, + fooAddressLocation, fooBarQualifiedIdentifier, common.NewTypeIDFromQualifiedName( nil, @@ -267,7 +267,7 @@ func TestAccountTypeInTypeValueMigration(t *testing.T) { Types: []*interpreter.InterfaceStaticType{}, LegacyType: interpreter.NewCompositeStaticType( nil, - nil, + fooAddressLocation, fooBarQualifiedIdentifier, common.NewTypeIDFromQualifiedName( nil, @@ -278,7 +278,7 @@ func TestAccountTypeInTypeValueMigration(t *testing.T) { }, expectedType: interpreter.NewCompositeStaticType( nil, - nil, + fooAddressLocation, fooBarQualifiedIdentifier, common.NewTypeIDFromQualifiedName( nil, @@ -341,7 +341,7 @@ func TestAccountTypeInTypeValueMigration(t *testing.T) { "non_intersection_interface": { storedType: interpreter.NewInterfaceStaticType( nil, - nil, + fooAddressLocation, fooBarQualifiedIdentifier, common.NewTypeIDFromQualifiedName( nil, @@ -354,7 +354,7 @@ func TestAccountTypeInTypeValueMigration(t *testing.T) { []*interpreter.InterfaceStaticType{ interpreter.NewInterfaceStaticType( nil, - nil, + fooAddressLocation, fooBarQualifiedIdentifier, common.NewTypeIDFromQualifiedName( nil, @@ -371,7 +371,7 @@ func TestAccountTypeInTypeValueMigration(t *testing.T) { []*interpreter.InterfaceStaticType{ interpreter.NewInterfaceStaticType( nil, - nil, + fooAddressLocation, fooBarQualifiedIdentifier, common.NewTypeIDFromQualifiedName( nil, @@ -386,7 +386,7 @@ func TestAccountTypeInTypeValueMigration(t *testing.T) { []*interpreter.InterfaceStaticType{ interpreter.NewInterfaceStaticType( nil, - nil, + fooAddressLocation, fooBarQualifiedIdentifier, common.NewTypeIDFromQualifiedName( nil, @@ -400,7 +400,7 @@ func TestAccountTypeInTypeValueMigration(t *testing.T) { "composite": { storedType: interpreter.NewCompositeStaticType( nil, - nil, + fooAddressLocation, fooBarQualifiedIdentifier, common.NewTypeIDFromQualifiedName( nil, @@ -447,7 +447,7 @@ func TestAccountTypeInTypeValueMigration(t *testing.T) { utils.TestLocation, &interpreter.Config{ Storage: storage, - AtreeValueValidationEnabled: false, + AtreeValueValidationEnabled: true, AtreeStorageValidationEnabled: true, }, ) @@ -1142,7 +1142,8 @@ func TestAccountTypeRehash(t *testing.T) { nil, utils.TestLocation, &interpreter.Config{ - Storage: storage, + Storage: storage, + // NOTE: atree value validation is disabled AtreeValueValidationEnabled: false, AtreeStorageValidationEnabled: true, }, diff --git a/migrations/statictypes/composite_type_migration_test.go b/migrations/statictypes/composite_type_migration_test.go index 99031703a3..d1ebfe25c0 100644 --- a/migrations/statictypes/composite_type_migration_test.go +++ b/migrations/statictypes/composite_type_migration_test.go @@ -46,8 +46,8 @@ func TestCompositeAndInterfaceTypeMigration(t *testing.T) { newCompositeType := func() *interpreter.CompositeStaticType { return interpreter.NewCompositeStaticType( nil, - nil, - "Bar", + fooAddressLocation, + fooBarQualifiedIdentifier, common.NewTypeIDFromQualifiedName( nil, fooAddressLocation, @@ -59,8 +59,8 @@ func TestCompositeAndInterfaceTypeMigration(t *testing.T) { newInterfaceType := func() *interpreter.InterfaceStaticType { return interpreter.NewInterfaceStaticType( nil, - nil, - "Baz", + fooAddressLocation, + fooBazQualifiedIdentifier, common.NewTypeIDFromQualifiedName( nil, fooAddressLocation, @@ -124,7 +124,7 @@ func TestCompositeAndInterfaceTypeMigration(t *testing.T) { utils.TestLocation, &interpreter.Config{ Storage: storage, - AtreeValueValidationEnabled: false, + AtreeValueValidationEnabled: true, AtreeStorageValidationEnabled: true, }, ) diff --git a/migrations/statictypes/intersection_type_migration_test.go b/migrations/statictypes/intersection_type_migration_test.go index 0347c00e09..eff4e7483c 100644 --- a/migrations/statictypes/intersection_type_migration_test.go +++ b/migrations/statictypes/intersection_type_migration_test.go @@ -295,8 +295,8 @@ func TestIntersectionTypeMigration(t *testing.T) { "non_intersection_interface": { storedType: interpreter.NewInterfaceStaticType( nil, - nil, - "Foo.Bar", + fooAddressLocation, + fooBarQualifiedIdentifier, common.NewTypeIDFromQualifiedName( nil, fooAddressLocation, @@ -308,8 +308,8 @@ func TestIntersectionTypeMigration(t *testing.T) { []*interpreter.InterfaceStaticType{ interpreter.NewInterfaceStaticType( nil, - nil, - "Foo.Bar", + fooAddressLocation, + fooBarQualifiedIdentifier, common.NewTypeIDFromQualifiedName( nil, fooAddressLocation, @@ -325,8 +325,8 @@ func TestIntersectionTypeMigration(t *testing.T) { []*interpreter.InterfaceStaticType{ interpreter.NewInterfaceStaticType( nil, - nil, - "Foo.Bar", + fooAddressLocation, + fooBarQualifiedIdentifier, common.NewTypeIDFromQualifiedName( nil, fooAddressLocation, @@ -340,8 +340,8 @@ func TestIntersectionTypeMigration(t *testing.T) { []*interpreter.InterfaceStaticType{ interpreter.NewInterfaceStaticType( nil, - nil, - "Foo.Bar", + fooAddressLocation, + fooBarQualifiedIdentifier, common.NewTypeIDFromQualifiedName( nil, fooAddressLocation, @@ -355,7 +355,7 @@ func TestIntersectionTypeMigration(t *testing.T) { "composite": { storedType: interpreter.NewCompositeStaticType( nil, - nil, + fooAddressLocation, "Foo.Bar", common.NewTypeIDFromQualifiedName( nil, @@ -377,7 +377,7 @@ func TestIntersectionTypeMigration(t *testing.T) { utils.TestLocation, &interpreter.Config{ Storage: storage, - AtreeValueValidationEnabled: false, + AtreeValueValidationEnabled: true, AtreeStorageValidationEnabled: true, }, ) @@ -503,7 +503,7 @@ func TestIntersectionTypeRehash(t *testing.T) { utils.TestLocation, &interpreter.Config{ Storage: storage, - AtreeValueValidationEnabled: false, + AtreeValueValidationEnabled: true, AtreeStorageValidationEnabled: true, }, ) @@ -659,7 +659,7 @@ func TestRehashNestedIntersectionType(t *testing.T) { utils.TestLocation, &interpreter.Config{ Storage: storage, - AtreeValueValidationEnabled: false, + AtreeValueValidationEnabled: true, AtreeStorageValidationEnabled: true, }, ) @@ -1014,7 +1014,7 @@ func TestIntersectionTypeMigrationWithInterfaceTypeConverter(t *testing.T) { utils.TestLocation, &interpreter.Config{ Storage: storage, - AtreeValueValidationEnabled: false, + AtreeValueValidationEnabled: true, AtreeStorageValidationEnabled: true, }, ) @@ -1411,7 +1411,7 @@ func TestIntersectionTypeMigrationWithTypeConverters(t *testing.T) { utils.TestLocation, &interpreter.Config{ Storage: storage, - AtreeValueValidationEnabled: false, + AtreeValueValidationEnabled: true, AtreeStorageValidationEnabled: true, }, ) diff --git a/migrations/string_normalization/migration_test.go b/migrations/string_normalization/migration_test.go index 26cc028855..06f280e60b 100644 --- a/migrations/string_normalization/migration_test.go +++ b/migrations/string_normalization/migration_test.go @@ -53,9 +53,10 @@ func TestStringNormalizingMigration(t *testing.T) { nil, utils.TestLocation, &interpreter.Config{ - Storage: storage, + Storage: storage, + // NOTE: disabled, because encoded and decoded values are expected to not match AtreeValueValidationEnabled: false, - AtreeStorageValidationEnabled: false, + AtreeStorageValidationEnabled: true, }, ) require.NoError(t, err) @@ -329,7 +330,8 @@ func TestStringValueRehash(t *testing.T) { nil, utils.TestLocation, &interpreter.Config{ - Storage: storage, + Storage: storage, + // NOTE: disabled, because encoded and decoded values are expected to not match AtreeValueValidationEnabled: false, AtreeStorageValidationEnabled: true, }, @@ -470,7 +472,8 @@ func TestCharacterValueRehash(t *testing.T) { nil, utils.TestLocation, &interpreter.Config{ - Storage: storage, + Storage: storage, + // NOTE: disabled, because encoded and decoded values are expected to not match AtreeValueValidationEnabled: false, AtreeStorageValidationEnabled: true, }, From d3717935268e2b752ff80e5580c1ad6b3c095d21 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Tue, 5 Mar 2024 09:54:27 -0800 Subject: [PATCH 05/17] improve tests: enable atree storage health and value validation where possible --- migrations/capcons/migration_test.go | 52 +- migrations/entitlements/migration_test.go | 33 +- migrations/migration_test.go | 21 +- .../account_type_migration_test.go | 938 ++++++++++-------- .../composite_type_migration_test.go | 6 +- .../intersection_type_migration_test.go | 36 +- .../statictypes/statictype_migration_test.go | 37 +- 7 files changed, 631 insertions(+), 492 deletions(-) diff --git a/migrations/capcons/migration_test.go b/migrations/capcons/migration_test.go index 649e970c56..03f517d318 100644 --- a/migrations/capcons/migration_test.go +++ b/migrations/capcons/migration_test.go @@ -514,19 +514,12 @@ func testPathCapabilityValueMigration( err = migration.Commit() require.NoError(t, err) - err = storage.CheckHealth() - require.NoError(t, err) - - // Check migrated capabilities + // Assert assert.Equal(t, expectedMigrations, reporter.migrations, ) - assert.Equal(t, - expectedErrors, - reporter.errors, - ) assert.Equal(t, expectedPathMigrations, reporter.pathCapabilityMigrations, @@ -535,6 +528,13 @@ func testPathCapabilityValueMigration( expectedMissingCapabilityIDs, reporter.missingCapabilityIDs, ) + require.Equal(t, + expectedErrors, + reporter.errors, + ) + + err = storage.CheckHealth() + require.NoError(t, err) if len(expectedMissingCapabilityIDs) == 0 { @@ -1351,19 +1351,12 @@ func testLinkMigration( err = migration.Commit() require.NoError(t, err) - err = storage.CheckHealth() - require.NoError(t, err) - // Assert assert.Equal(t, expectedMigrations, reporter.migrations, ) - assert.Equal(t, - expectedErrors, - reporter.errors, - ) assert.Equal(t, expectedLinkMigrations, reporter.linkMigrations, @@ -1376,6 +1369,13 @@ func testLinkMigration( expectedMissingTargets, reporter.missingTargets, ) + require.Equal(t, + expectedErrors, + reporter.errors, + ) + + err = storage.CheckHealth() + require.NoError(t, err) } func TestLinkMigration(t *testing.T) { @@ -2079,23 +2079,22 @@ func TestPublishedPathCapabilityValueMigration(t *testing.T) { err = migration.Commit() require.NoError(t, err) - err = storage.CheckHealth() - require.NoError(t, err) - - // Check migrated capabilities + // Assert assert.Equal(t, expectedMigrations, reporter.migrations, ) - assert.Empty(t, reporter.errors) assert.Equal(t, expectedPathMigrations, reporter.pathCapabilityMigrations, ) require.Nil(t, reporter.missingCapabilityIDs) - // Check + require.Empty(t, reporter.errors) + + err = storage.CheckHealth() + require.NoError(t, err) // language=cadence checkScript := ` @@ -2329,22 +2328,23 @@ func TestUntypedPathCapabilityValueMigration(t *testing.T) { err = migration.Commit() require.NoError(t, err) - err = storage.CheckHealth() - require.NoError(t, err) - - // Check migrated capabilities + // Assert assert.Equal(t, expectedMigrations, reporter.migrations, ) - assert.Empty(t, reporter.errors) assert.Equal(t, expectedPathMigrations, reporter.pathCapabilityMigrations, ) require.Nil(t, reporter.missingCapabilityIDs) + require.Empty(t, reporter.errors) + + err = storage.CheckHealth() + require.NoError(t, err) + // Check // language=cadence diff --git a/migrations/entitlements/migration_test.go b/migrations/entitlements/migration_test.go index f8fb2b61f1..ffd1c8629a 100644 --- a/migrations/entitlements/migration_test.go +++ b/migrations/entitlements/migration_test.go @@ -719,12 +719,12 @@ func convertEntireTestValue( err := migration.Commit() require.NoError(t, err) - err = storage.CheckHealth() - require.NoError(t, err) - // Assert - assert.Empty(t, reporter.errors) + require.Empty(t, reporter.errors) + + err = storage.CheckHealth() + require.NoError(t, err) if migratedValue == nil { return v @@ -1536,11 +1536,11 @@ func TestMigrateSimpleContract(t *testing.T) { // Assert + require.Empty(t, reporter.errors) + err = storage.CheckHealth() require.NoError(t, err) - assert.Empty(t, reporter.errors) - storageMap := storage.GetStorageMap(account, storageIdentifier, false) require.NotNil(t, storageMap) require.Greater(t, storageMap.Count(), uint64(0)) @@ -1723,11 +1723,11 @@ func TestMigratePublishedValue(t *testing.T) { // Assert + require.Empty(t, reporter.errors) + err = storage.CheckHealth() require.NoError(t, err) - assert.Empty(t, reporter.errors) - assert.Equal(t, map[struct { interpreter.StorageKey @@ -1981,11 +1981,11 @@ func TestMigratePublishedValueAcrossTwoAccounts(t *testing.T) { // Assert + require.Empty(t, reporter.errors) + err = storage.CheckHealth() require.NoError(t, err) - assert.Empty(t, reporter.errors) - assert.Equal(t, map[struct { interpreter.StorageKey @@ -2434,11 +2434,11 @@ func TestMigrateArrayOfValues(t *testing.T) { // Assert + require.Empty(t, reporter.errors) + err = storage.CheckHealth() require.NoError(t, err) - assert.Empty(t, reporter.errors) - assert.Equal(t, map[struct { interpreter.StorageKey @@ -2685,11 +2685,11 @@ func TestMigrateDictOfValues(t *testing.T) { // Assert + require.Empty(t, reporter.errors) + err = storage.CheckHealth() require.NoError(t, err) - assert.Empty(t, reporter.errors) - assert.Equal(t, map[struct { interpreter.StorageKey @@ -3008,10 +3008,11 @@ func TestMigrateCapConsAcrossTwoAccounts(t *testing.T) { // Assert + require.Empty(t, reporter.errors) + err = storage.CheckHealth() require.NoError(t, err) - assert.Empty(t, reporter.errors) assert.Len(t, reporter.migrated, 1) // TODO: assert @@ -3241,6 +3242,8 @@ func TestRehash(t *testing.T) { err := migration.Commit() require.NoError(t, err) + // Assert + err = storage.CheckHealth() require.NoError(t, err) diff --git a/migrations/migration_test.go b/migrations/migration_test.go index 80bacae52a..02d3f4232e 100644 --- a/migrations/migration_test.go +++ b/migrations/migration_test.go @@ -518,12 +518,12 @@ func TestMultipleMigrations(t *testing.T) { err = migration.Commit() require.NoError(t, err) - err = storage.CheckHealth() - require.NoError(t, err) + // Assert - assert.Empty(t, reporter.errored) + require.Empty(t, reporter.errored) - // Assert: Traverse through the storage and see if the values are updated now. + err = storage.CheckHealth() + require.NoError(t, err) for _, testCase := range testCases { @@ -665,9 +665,6 @@ func TestMigrationError(t *testing.T) { err = migration.Commit() require.NoError(t, err) - err = storage.CheckHealth() - require.NoError(t, err) - assert.Equal(t, map[struct { interpreter.StorageKey @@ -684,6 +681,9 @@ func TestMigrationError(t *testing.T) { reporter.errored, ) + err = storage.CheckHealth() + require.NoError(t, err) + // Assert: Traverse through the storage and see if the values are updated now. storageMap := storage.GetStorageMap(account, pathDomain.Identifier(), false) @@ -946,12 +946,13 @@ func TestContractMigration(t *testing.T) { err = migration.Commit() require.NoError(t, err) + // Assert + + require.Empty(t, reporter.errored) + err = storage.CheckHealth() require.NoError(t, err) - // Assert - - assert.Empty(t, reporter.errored) assert.Len(t, reporter.migrated, 1) value, err := rt.ExecuteScript( diff --git a/migrations/statictypes/account_type_migration_test.go b/migrations/statictypes/account_type_migration_test.go index da9ae924e3..5e518a2b24 100644 --- a/migrations/statictypes/account_type_migration_test.go +++ b/migrations/statictypes/account_type_migration_test.go @@ -485,11 +485,13 @@ func TestAccountTypeInTypeValueMigration(t *testing.T) { err = migration.Commit() require.NoError(t, err) - err = storage.CheckHealth() - require.NoError(t, err) + // Assert require.Empty(t, reporter.errors) + err = storage.CheckHealth() + require.NoError(t, err) + storageMapKey := interpreter.StringStorageMapKey(name) if testCase.expectedType == nil { @@ -581,285 +583,328 @@ func TestAccountTypeInNestedTypeValueMigration(t *testing.T) { pathDomain := common.PathDomainPublic type testCase struct { - storedValue interpreter.Value - expectedValue interpreter.Value + storedValue func(inter *interpreter.Interpreter) interpreter.Value + expectedValue func(inter *interpreter.Interpreter) interpreter.Value + validateValue bool } storedAccountTypeValue := interpreter.NewTypeValue(nil, interpreter.PrimitiveStaticTypePublicAccount) //nolint:staticcheck expectedAccountTypeValue := interpreter.NewTypeValue(nil, unauthorizedAccountReferenceType) stringTypeValue := interpreter.NewTypeValue(nil, interpreter.PrimitiveStaticTypeString) - ledger := NewTestLedger(nil, nil) - storage := runtime.NewStorage(ledger, nil) - locationRange := interpreter.EmptyLocationRange - - inter, err := interpreter.NewInterpreter( - nil, - utils.TestLocation, - &interpreter.Config{ - Storage: storage, - AtreeValueValidationEnabled: false, - AtreeStorageValidationEnabled: false, - }, - ) - require.NoError(t, err) - fooAddressLocation := common.NewAddressLocation(nil, account, "Foo") const fooBarQualifiedIdentifier = "Foo.Bar" testCases := map[string]testCase{ "account_some_value": { - storedValue: interpreter.NewUnmeteredSomeValueNonCopying(storedAccountTypeValue), - expectedValue: interpreter.NewUnmeteredSomeValueNonCopying(expectedAccountTypeValue), + storedValue: func(_ *interpreter.Interpreter) interpreter.Value { + return interpreter.NewUnmeteredSomeValueNonCopying(storedAccountTypeValue) + }, + expectedValue: func(_ *interpreter.Interpreter) interpreter.Value { + return interpreter.NewUnmeteredSomeValueNonCopying(expectedAccountTypeValue) + }, + validateValue: true, }, "int8_some_value": { - storedValue: interpreter.NewUnmeteredSomeValueNonCopying(stringTypeValue), + storedValue: func(_ *interpreter.Interpreter) interpreter.Value { + return interpreter.NewUnmeteredSomeValueNonCopying(stringTypeValue) + }, + expectedValue: nil, + validateValue: true, }, "account_array": { - storedValue: interpreter.NewArrayValue( - inter, - locationRange, - interpreter.NewVariableSizedStaticType(nil, interpreter.PrimitiveStaticTypeAnyStruct), - common.ZeroAddress, - stringTypeValue, - storedAccountTypeValue, - stringTypeValue, - stringTypeValue, - storedAccountTypeValue, - ), - expectedValue: interpreter.NewArrayValue( - inter, - locationRange, - interpreter.NewVariableSizedStaticType(nil, interpreter.PrimitiveStaticTypeAnyStruct), - common.ZeroAddress, - stringTypeValue, - expectedAccountTypeValue, - stringTypeValue, - stringTypeValue, - expectedAccountTypeValue, - ), + storedValue: func(inter *interpreter.Interpreter) interpreter.Value { + return interpreter.NewArrayValue( + inter, + interpreter.EmptyLocationRange, + interpreter.NewVariableSizedStaticType(nil, interpreter.PrimitiveStaticTypeAnyStruct), + common.ZeroAddress, + stringTypeValue, + storedAccountTypeValue, + stringTypeValue, + stringTypeValue, + storedAccountTypeValue, + ) + }, + expectedValue: func(inter *interpreter.Interpreter) interpreter.Value { + return interpreter.NewArrayValue( + inter, + interpreter.EmptyLocationRange, + interpreter.NewVariableSizedStaticType(nil, interpreter.PrimitiveStaticTypeAnyStruct), + common.ZeroAddress, + stringTypeValue, + expectedAccountTypeValue, + stringTypeValue, + stringTypeValue, + expectedAccountTypeValue, + ) + }, + validateValue: true, }, "non_account_array": { - storedValue: interpreter.NewArrayValue( - inter, - locationRange, - interpreter.NewVariableSizedStaticType(nil, interpreter.PrimitiveStaticTypeAnyStruct), - common.ZeroAddress, - stringTypeValue, - stringTypeValue, - stringTypeValue, - ), + storedValue: func(inter *interpreter.Interpreter) interpreter.Value { + return interpreter.NewArrayValue( + inter, + interpreter.EmptyLocationRange, + interpreter.NewVariableSizedStaticType(nil, interpreter.PrimitiveStaticTypeAnyStruct), + common.ZeroAddress, + stringTypeValue, + stringTypeValue, + stringTypeValue, + ) + }, + expectedValue: nil, + validateValue: true, }, "dictionary_with_account_type_value": { - storedValue: interpreter.NewDictionaryValue( - inter, - locationRange, - interpreter.NewDictionaryStaticType( - nil, - interpreter.PrimitiveStaticTypeInt8, - interpreter.PrimitiveStaticTypeAnyStruct, - ), - interpreter.NewUnmeteredInt8Value(4), - storedAccountTypeValue, - interpreter.NewUnmeteredInt8Value(5), - interpreter.NewUnmeteredStringValue("hello"), - ), - expectedValue: interpreter.NewDictionaryValue( - inter, - locationRange, - interpreter.NewDictionaryStaticType( - nil, - interpreter.PrimitiveStaticTypeInt8, - interpreter.PrimitiveStaticTypeAnyStruct, - ), - interpreter.NewUnmeteredInt8Value(4), - expectedAccountTypeValue, - interpreter.NewUnmeteredInt8Value(5), - interpreter.NewUnmeteredStringValue("hello"), - ), + storedValue: func(inter *interpreter.Interpreter) interpreter.Value { + return interpreter.NewDictionaryValue( + inter, + interpreter.EmptyLocationRange, + interpreter.NewDictionaryStaticType( + nil, + interpreter.PrimitiveStaticTypeInt8, + interpreter.PrimitiveStaticTypeAnyStruct, + ), + interpreter.NewUnmeteredInt8Value(4), + storedAccountTypeValue, + interpreter.NewUnmeteredInt8Value(5), + interpreter.NewUnmeteredStringValue("hello"), + ) + }, + expectedValue: func(inter *interpreter.Interpreter) interpreter.Value { + return interpreter.NewDictionaryValue( + inter, + interpreter.EmptyLocationRange, + interpreter.NewDictionaryStaticType( + nil, + interpreter.PrimitiveStaticTypeInt8, + interpreter.PrimitiveStaticTypeAnyStruct, + ), + interpreter.NewUnmeteredInt8Value(4), + expectedAccountTypeValue, + interpreter.NewUnmeteredInt8Value(5), + interpreter.NewUnmeteredStringValue("hello"), + ) + }, + validateValue: true, }, "dictionary_with_optional_account_type_value": { - storedValue: interpreter.NewDictionaryValue( - inter, - locationRange, - interpreter.NewDictionaryStaticType( - nil, - interpreter.PrimitiveStaticTypeInt8, - interpreter.NewOptionalStaticType(nil, interpreter.PrimitiveStaticTypeMetaType), - ), - interpreter.NewUnmeteredInt8Value(4), - interpreter.NewUnmeteredSomeValueNonCopying(storedAccountTypeValue), - ), - expectedValue: interpreter.NewDictionaryValue( - inter, - locationRange, - interpreter.NewDictionaryStaticType( - nil, - interpreter.PrimitiveStaticTypeInt8, - interpreter.NewOptionalStaticType(nil, interpreter.PrimitiveStaticTypeMetaType), - ), - interpreter.NewUnmeteredInt8Value(4), - interpreter.NewUnmeteredSomeValueNonCopying(expectedAccountTypeValue), - ), + storedValue: func(inter *interpreter.Interpreter) interpreter.Value { + return interpreter.NewDictionaryValue( + inter, + interpreter.EmptyLocationRange, + interpreter.NewDictionaryStaticType( + nil, + interpreter.PrimitiveStaticTypeInt8, + interpreter.NewOptionalStaticType(nil, interpreter.PrimitiveStaticTypeMetaType), + ), + interpreter.NewUnmeteredInt8Value(4), + interpreter.NewUnmeteredSomeValueNonCopying(storedAccountTypeValue), + ) + }, + expectedValue: func(inter *interpreter.Interpreter) interpreter.Value { + return interpreter.NewDictionaryValue( + inter, + interpreter.EmptyLocationRange, + interpreter.NewDictionaryStaticType( + nil, + interpreter.PrimitiveStaticTypeInt8, + interpreter.NewOptionalStaticType(nil, interpreter.PrimitiveStaticTypeMetaType), + ), + interpreter.NewUnmeteredInt8Value(4), + interpreter.NewUnmeteredSomeValueNonCopying(expectedAccountTypeValue), + ) + }, + validateValue: true, }, "dictionary_with_account_type_key": { - storedValue: interpreter.NewDictionaryValue( - inter, - locationRange, - interpreter.NewDictionaryStaticType( - nil, - interpreter.PrimitiveStaticTypeMetaType, - interpreter.PrimitiveStaticTypeInt8, - ), - interpreter.NewTypeValue( - nil, - dummyStaticType{ - PrimitiveStaticType: interpreter.PrimitiveStaticTypePublicAccount, //nolint:staticcheck - }, - ), - interpreter.NewUnmeteredInt8Value(4), - ), - expectedValue: interpreter.NewDictionaryValue( - inter, - locationRange, - interpreter.NewDictionaryStaticType( - nil, - interpreter.PrimitiveStaticTypeMetaType, - interpreter.PrimitiveStaticTypeInt8, - ), - expectedAccountTypeValue, - interpreter.NewUnmeteredInt8Value(4), - ), + storedValue: func(inter *interpreter.Interpreter) interpreter.Value { + return interpreter.NewDictionaryValue( + inter, + interpreter.EmptyLocationRange, + interpreter.NewDictionaryStaticType( + nil, + interpreter.PrimitiveStaticTypeMetaType, + interpreter.PrimitiveStaticTypeInt8, + ), + interpreter.NewTypeValue( + nil, + dummyStaticType{ + PrimitiveStaticType: interpreter.PrimitiveStaticTypePublicAccount, //nolint:staticcheck + }, + ), + interpreter.NewUnmeteredInt8Value(4), + ) + }, + expectedValue: func(inter *interpreter.Interpreter) interpreter.Value { + return interpreter.NewDictionaryValue( + inter, + interpreter.EmptyLocationRange, + interpreter.NewDictionaryStaticType( + nil, + interpreter.PrimitiveStaticTypeMetaType, + interpreter.PrimitiveStaticTypeInt8, + ), + expectedAccountTypeValue, + interpreter.NewUnmeteredInt8Value(4), + ) + }, + validateValue: false, }, "dictionary_with_account_type_key_and_value": { - storedValue: interpreter.NewDictionaryValue( - inter, - locationRange, - interpreter.NewDictionaryStaticType( - nil, - interpreter.PrimitiveStaticTypeMetaType, - interpreter.PrimitiveStaticTypeMetaType, - ), - interpreter.NewTypeValue( - nil, - dummyStaticType{ - PrimitiveStaticType: interpreter.PrimitiveStaticTypePublicAccount, //nolint:staticcheck - }, - ), - storedAccountTypeValue, - ), - expectedValue: interpreter.NewDictionaryValue( - inter, - locationRange, - interpreter.NewDictionaryStaticType( - nil, - interpreter.PrimitiveStaticTypeMetaType, - interpreter.PrimitiveStaticTypeMetaType, - ), - expectedAccountTypeValue, - expectedAccountTypeValue, - ), + storedValue: func(inter *interpreter.Interpreter) interpreter.Value { + return interpreter.NewDictionaryValue( + inter, + interpreter.EmptyLocationRange, + interpreter.NewDictionaryStaticType( + nil, + interpreter.PrimitiveStaticTypeMetaType, + interpreter.PrimitiveStaticTypeMetaType, + ), + interpreter.NewTypeValue( + nil, + dummyStaticType{ + PrimitiveStaticType: interpreter.PrimitiveStaticTypePublicAccount, //nolint:staticcheck + }, + ), + storedAccountTypeValue, + ) + }, + expectedValue: func(inter *interpreter.Interpreter) interpreter.Value { + return interpreter.NewDictionaryValue( + inter, + interpreter.EmptyLocationRange, + interpreter.NewDictionaryStaticType( + nil, + interpreter.PrimitiveStaticTypeMetaType, + interpreter.PrimitiveStaticTypeMetaType, + ), + expectedAccountTypeValue, + expectedAccountTypeValue, + ) + }, + validateValue: false, }, "composite_with_account_type": { - storedValue: interpreter.NewCompositeValue( - inter, - interpreter.EmptyLocationRange, - fooAddressLocation, - fooBarQualifiedIdentifier, - common.CompositeKindResource, - []interpreter.CompositeField{ - interpreter.NewUnmeteredCompositeField("field1", storedAccountTypeValue), - interpreter.NewUnmeteredCompositeField("field2", interpreter.NewUnmeteredStringValue("hello")), - }, - common.Address{}, - ), - expectedValue: interpreter.NewCompositeValue( - inter, - interpreter.EmptyLocationRange, - fooAddressLocation, - fooBarQualifiedIdentifier, - common.CompositeKindResource, - []interpreter.CompositeField{ - interpreter.NewUnmeteredCompositeField("field1", expectedAccountTypeValue), - interpreter.NewUnmeteredCompositeField("field2", interpreter.NewUnmeteredStringValue("hello")), - }, - common.Address{}, - ), + storedValue: func(inter *interpreter.Interpreter) interpreter.Value { + return interpreter.NewCompositeValue( + inter, + interpreter.EmptyLocationRange, + fooAddressLocation, + fooBarQualifiedIdentifier, + common.CompositeKindResource, + []interpreter.CompositeField{ + interpreter.NewUnmeteredCompositeField("field1", storedAccountTypeValue), + interpreter.NewUnmeteredCompositeField("field2", interpreter.NewUnmeteredStringValue("hello")), + }, + common.Address{}, + ) + }, + expectedValue: func(inter *interpreter.Interpreter) interpreter.Value { + return interpreter.NewCompositeValue( + inter, + interpreter.EmptyLocationRange, + fooAddressLocation, + fooBarQualifiedIdentifier, + common.CompositeKindResource, + []interpreter.CompositeField{ + interpreter.NewUnmeteredCompositeField("field1", expectedAccountTypeValue), + interpreter.NewUnmeteredCompositeField("field2", interpreter.NewUnmeteredStringValue("hello")), + }, + common.Address{}, + ) + }, + validateValue: true, }, } // Store values - for name, testCase := range testCases { - transferredValue := testCase.storedValue.Transfer( - inter, - locationRange, - atree.Address(account), - false, - nil, - nil, - ) - - inter.WriteStored( - account, - pathDomain.Identifier(), - interpreter.StringStorageMapKey(name), - transferredValue, - ) - } + test := func(name string, testCase testCase) { - err = storage.Commit(inter, true) - require.NoError(t, err) + t.Run(name, func(t *testing.T) { + t.Parallel() - // Migrate + ledger := NewTestLedger(nil, nil) + storage := runtime.NewStorage(ledger, nil) - migration := migrations.NewStorageMigration(inter, storage) + inter, err := interpreter.NewInterpreter( + nil, + utils.TestLocation, + &interpreter.Config{ + Storage: storage, + AtreeValueValidationEnabled: testCase.validateValue, + AtreeStorageValidationEnabled: true, + }, + ) + require.NoError(t, err) - reporter := newTestReporter() + transferredValue := testCase.storedValue(inter).Transfer( + inter, + interpreter.EmptyLocationRange, + atree.Address(account), + false, + nil, + nil, + ) - migration.Migrate( - &migrations.AddressSliceIterator{ - Addresses: []common.Address{ + inter.WriteStored( account, - }, - }, - migration.NewValueMigrationsPathMigrator( - reporter, - NewStaticTypeMigration(), - ), - ) + pathDomain.Identifier(), + interpreter.StringStorageMapKey(name), + transferredValue, + ) + + err = storage.Commit(inter, true) + require.NoError(t, err) + + // Migrate - err = migration.Commit() - require.NoError(t, err) + migration := migrations.NewStorageMigration(inter, storage) + + reporter := newTestReporter() - err = storage.CheckHealth() - require.NoError(t, err) + migration.Migrate( + &migrations.AddressSliceIterator{ + Addresses: []common.Address{ + account, + }, + }, + migration.NewValueMigrationsPathMigrator( + reporter, + NewStaticTypeMigration(), + ), + ) - require.Empty(t, reporter.errors) + err = migration.Commit() + require.NoError(t, err) - // Assert: Traverse through the storage and see if the values are updated now. + // Assert - storageMap := storage.GetStorageMap(account, pathDomain.Identifier(), false) - require.NotNil(t, storageMap) - require.Greater(t, storageMap.Count(), uint64(0)) + require.Empty(t, reporter.errors) - iterator := storageMap.Iterator(inter) + err = storage.CheckHealth() + require.NoError(t, err) - for key, value := iterator.Next(); key != nil; key, value = iterator.Next() { - identifier := string(key.(interpreter.StringAtreeValue)) + storageMap := storage.GetStorageMap(account, pathDomain.Identifier(), false) + require.NotNil(t, storageMap) + require.Equal(t, uint64(1), storageMap.Count()) - t.Run(identifier, func(t *testing.T) { - testCase, ok := testCases[identifier] - require.True(t, ok) + value := storageMap.ReadValue(nil, interpreter.StringStorageMapKey(name)) expectedStoredValue := testCase.expectedValue if expectedStoredValue == nil { expectedStoredValue = testCase.storedValue } - utils.AssertValuesEqual(t, inter, expectedStoredValue, value) + utils.AssertValuesEqual(t, inter, expectedStoredValue(inter), value) }) } + + for name, testCase := range testCases { + test(name, testCase) + } } func TestMigratingValuesWithAccountStaticType(t *testing.T) { @@ -870,255 +915,306 @@ func TestMigratingValuesWithAccountStaticType(t *testing.T) { pathDomain := common.PathDomainPublic type testCase struct { - storedValue interpreter.Value - expectedValue interpreter.Value + storedValue func(inter *interpreter.Interpreter) interpreter.Value + expectedValue func(inter *interpreter.Interpreter) interpreter.Value + validateStorage bool } - ledger := NewTestLedger(nil, nil) - storage := runtime.NewStorage(ledger, nil) - locationRange := interpreter.EmptyLocationRange - - inter, err := interpreter.NewInterpreter( - nil, - utils.TestLocation, - &interpreter.Config{ - Storage: storage, - AtreeValueValidationEnabled: false, - AtreeStorageValidationEnabled: false, - }, - ) - require.NoError(t, err) - testCases := map[string]testCase{ "dictionary_value": { - storedValue: interpreter.NewDictionaryValue( - inter, - locationRange, - interpreter.NewDictionaryStaticType( - nil, - interpreter.PrimitiveStaticTypeString, - interpreter.PrimitiveStaticTypePublicAccount, //nolint:staticcheck - ), - ), - expectedValue: interpreter.NewDictionaryValue( - inter, - locationRange, - interpreter.NewDictionaryStaticType( - nil, - interpreter.PrimitiveStaticTypeString, - unauthorizedAccountReferenceType, - ), - ), + storedValue: func(inter *interpreter.Interpreter) interpreter.Value { + return interpreter.NewDictionaryValue( + inter, + interpreter.EmptyLocationRange, + interpreter.NewDictionaryStaticType( + nil, + interpreter.PrimitiveStaticTypeString, + interpreter.PrimitiveStaticTypePublicAccount, //nolint:staticcheck + ), + ) + }, + expectedValue: func(inter *interpreter.Interpreter) interpreter.Value { + return interpreter.NewDictionaryValue( + inter, + interpreter.EmptyLocationRange, + interpreter.NewDictionaryStaticType( + nil, + interpreter.PrimitiveStaticTypeString, + unauthorizedAccountReferenceType, + ), + ) + }, + // NOTE: disabled, as storage is not expected to be always valid _during_ migration + validateStorage: false, }, "array_value": { - storedValue: interpreter.NewArrayValue( - inter, - locationRange, - interpreter.NewVariableSizedStaticType( - nil, - interpreter.PrimitiveStaticTypePublicAccount, //nolint:staticcheck - ), - common.Address{}, - ), - expectedValue: interpreter.NewArrayValue( - inter, - locationRange, - interpreter.NewVariableSizedStaticType( - nil, - unauthorizedAccountReferenceType, - ), - common.Address{}, - ), + storedValue: func(inter *interpreter.Interpreter) interpreter.Value { + return interpreter.NewArrayValue( + inter, + interpreter.EmptyLocationRange, + interpreter.NewVariableSizedStaticType( + nil, + interpreter.PrimitiveStaticTypePublicAccount, //nolint:staticcheck + ), + common.Address{}, + ) + }, + expectedValue: func(inter *interpreter.Interpreter) interpreter.Value { + return interpreter.NewArrayValue( + inter, + interpreter.EmptyLocationRange, + interpreter.NewVariableSizedStaticType( + nil, + unauthorizedAccountReferenceType, + ), + common.Address{}, + ) + }, + // NOTE: disabled, as storage is not expected to be always valid _during_ migration + validateStorage: false, }, "account_capability_value": { - storedValue: interpreter.NewUnmeteredCapabilityValue( - 123, - interpreter.NewAddressValue(nil, common.Address{0x42}), - interpreter.PrimitiveStaticTypePublicAccount, //nolint:staticcheck - ), - expectedValue: interpreter.NewUnmeteredCapabilityValue( - 123, - interpreter.NewAddressValue(nil, common.Address{0x42}), - unauthorizedAccountReferenceType, - ), + storedValue: func(_ *interpreter.Interpreter) interpreter.Value { + return interpreter.NewUnmeteredCapabilityValue( + 123, + interpreter.NewAddressValue(nil, common.Address{0x42}), + interpreter.PrimitiveStaticTypePublicAccount, //nolint:staticcheck + ) + }, + expectedValue: func(_ *interpreter.Interpreter) interpreter.Value { + return interpreter.NewUnmeteredCapabilityValue( + 123, + interpreter.NewAddressValue(nil, common.Address{0x42}), + unauthorizedAccountReferenceType, + ) + }, + validateStorage: true, }, "string_capability_value": { - storedValue: interpreter.NewUnmeteredCapabilityValue( - 123, - interpreter.NewAddressValue(nil, common.Address{0x42}), - interpreter.PrimitiveStaticTypeString, - ), + storedValue: func(_ *interpreter.Interpreter) interpreter.Value { + return interpreter.NewUnmeteredCapabilityValue( + 123, + interpreter.NewAddressValue(nil, common.Address{0x42}), + interpreter.PrimitiveStaticTypeString, + ) + }, + validateStorage: true, }, "account_capability_controller": { - storedValue: interpreter.NewUnmeteredAccountCapabilityControllerValue( - interpreter.NewReferenceStaticType( - nil, - interpreter.UnauthorizedAccess, - interpreter.PrimitiveStaticTypeAuthAccount, //nolint:staticcheck, - ), - 1234, - ), - expectedValue: interpreter.NewUnmeteredAccountCapabilityControllerValue( - authAccountReferenceType, - 1234, - ), + storedValue: func(_ *interpreter.Interpreter) interpreter.Value { + return interpreter.NewUnmeteredAccountCapabilityControllerValue( + interpreter.NewReferenceStaticType( + nil, + interpreter.UnauthorizedAccess, + interpreter.PrimitiveStaticTypeAuthAccount, //nolint:staticcheck, + ), + 1234, + ) + }, + expectedValue: func(_ *interpreter.Interpreter) interpreter.Value { + return interpreter.NewUnmeteredAccountCapabilityControllerValue( + authAccountReferenceType, + 1234, + ) + }, + validateStorage: true, }, "storage_capability_controller": { - storedValue: interpreter.NewUnmeteredStorageCapabilityControllerValue( - interpreter.NewReferenceStaticType( - nil, - interpreter.UnauthorizedAccess, - interpreter.PrimitiveStaticTypePublicAccount, //nolint:staticcheck, - ), - 1234, - interpreter.NewUnmeteredPathValue(common.PathDomainStorage, "v1"), - ), - expectedValue: interpreter.NewUnmeteredStorageCapabilityControllerValue( - unauthorizedAccountReferenceType, - 1234, - interpreter.NewUnmeteredPathValue(common.PathDomainStorage, "v1"), - ), + storedValue: func(_ *interpreter.Interpreter) interpreter.Value { + return interpreter.NewUnmeteredStorageCapabilityControllerValue( + interpreter.NewReferenceStaticType( + nil, + interpreter.UnauthorizedAccess, + interpreter.PrimitiveStaticTypePublicAccount, //nolint:staticcheck, + ), + 1234, + interpreter.NewUnmeteredPathValue(common.PathDomainStorage, "v1"), + ) + }, + expectedValue: func(_ *interpreter.Interpreter) interpreter.Value { + return interpreter.NewUnmeteredStorageCapabilityControllerValue( + unauthorizedAccountReferenceType, + 1234, + interpreter.NewUnmeteredPathValue(common.PathDomainStorage, "v1"), + ) + }, + validateStorage: true, }, "path_link_value": { - storedValue: interpreter.PathLinkValue{ //nolint:staticcheck - TargetPath: interpreter.NewUnmeteredPathValue(common.PathDomainStorage, "v1"), - Type: interpreter.PrimitiveStaticTypePublicAccount, //nolint:staticcheck + storedValue: func(_ *interpreter.Interpreter) interpreter.Value { + return interpreter.PathLinkValue{ //nolint:staticcheck + TargetPath: interpreter.NewUnmeteredPathValue(common.PathDomainStorage, "v1"), + Type: interpreter.PrimitiveStaticTypePublicAccount, //nolint:staticcheck + } }, - expectedValue: interpreter.PathLinkValue{ //nolint:staticcheck - TargetPath: interpreter.NewUnmeteredPathValue(common.PathDomainStorage, "v1"), - Type: unauthorizedAccountReferenceType, + expectedValue: func(_ *interpreter.Interpreter) interpreter.Value { + return interpreter.PathLinkValue{ //nolint:staticcheck + TargetPath: interpreter.NewUnmeteredPathValue(common.PathDomainStorage, "v1"), + Type: unauthorizedAccountReferenceType, + } }, + validateStorage: true, }, "account_link_value": { - storedValue: interpreter.AccountLinkValue{}, //nolint:staticcheck - expectedValue: interpreter.AccountLinkValue{}, //nolint:staticcheck + storedValue: func(_ *interpreter.Interpreter) interpreter.Value { + return interpreter.AccountLinkValue{} //nolint:staticcheck + }, + expectedValue: func(_ *interpreter.Interpreter) interpreter.Value { + return interpreter.AccountLinkValue{} //nolint:staticcheck + }, + validateStorage: true, }, "path_capability_value": { - storedValue: &interpreter.PathCapabilityValue{ //nolint:staticcheck - Address: interpreter.NewAddressValue(nil, common.Address{0x42}), - Path: interpreter.NewUnmeteredPathValue(common.PathDomainStorage, "v1"), - BorrowType: interpreter.PrimitiveStaticTypePublicAccount, //nolint:staticcheck + storedValue: func(_ *interpreter.Interpreter) interpreter.Value { + return &interpreter.PathCapabilityValue{ //nolint:staticcheck + Address: interpreter.NewAddressValue(nil, common.Address{0x42}), + Path: interpreter.NewUnmeteredPathValue(common.PathDomainStorage, "v1"), + BorrowType: interpreter.PrimitiveStaticTypePublicAccount, //nolint:staticcheck + } }, - expectedValue: &interpreter.PathCapabilityValue{ //nolint:staticcheck - Address: interpreter.NewAddressValue(nil, common.Address{0x42}), - Path: interpreter.NewUnmeteredPathValue(common.PathDomainStorage, "v1"), - BorrowType: unauthorizedAccountReferenceType, + expectedValue: func(_ *interpreter.Interpreter) interpreter.Value { + return &interpreter.PathCapabilityValue{ //nolint:staticcheck + Address: interpreter.NewAddressValue(nil, common.Address{0x42}), + Path: interpreter.NewUnmeteredPathValue(common.PathDomainStorage, "v1"), + BorrowType: unauthorizedAccountReferenceType, + } }, + validateStorage: true, }, "capability_dictionary": { - storedValue: interpreter.NewDictionaryValue( - inter, - locationRange, - interpreter.NewDictionaryStaticType( - nil, - interpreter.PrimitiveStaticTypeString, - interpreter.NewCapabilityStaticType( + storedValue: func(inter *interpreter.Interpreter) interpreter.Value { + return interpreter.NewDictionaryValue( + inter, + interpreter.EmptyLocationRange, + interpreter.NewDictionaryStaticType( + nil, + interpreter.PrimitiveStaticTypeString, + interpreter.NewCapabilityStaticType( + nil, + interpreter.PrimitiveStaticTypePublicAccount, //nolint:staticcheck + ), + ), + interpreter.NewUnmeteredStringValue("key"), + interpreter.NewCapabilityValue( nil, + interpreter.NewUnmeteredUInt64Value(1234), + interpreter.NewAddressValue(nil, common.Address{}), interpreter.PrimitiveStaticTypePublicAccount, //nolint:staticcheck ), - ), - interpreter.NewUnmeteredStringValue("key"), - interpreter.NewCapabilityValue( - nil, - interpreter.NewUnmeteredUInt64Value(1234), - interpreter.NewAddressValue(nil, common.Address{}), - interpreter.PrimitiveStaticTypePublicAccount, //nolint:staticcheck - ), - ), - expectedValue: interpreter.NewDictionaryValue( - inter, - locationRange, - interpreter.NewDictionaryStaticType( - nil, - interpreter.PrimitiveStaticTypeString, - interpreter.NewCapabilityStaticType( + ) + }, + expectedValue: func(inter *interpreter.Interpreter) interpreter.Value { + return interpreter.NewDictionaryValue( + inter, + interpreter.EmptyLocationRange, + interpreter.NewDictionaryStaticType( + nil, + interpreter.PrimitiveStaticTypeString, + interpreter.NewCapabilityStaticType( + nil, + unauthorizedAccountReferenceType, + ), + ), + interpreter.NewUnmeteredStringValue("key"), + interpreter.NewCapabilityValue( nil, + interpreter.NewUnmeteredUInt64Value(1234), + interpreter.NewAddressValue(nil, common.Address{}), unauthorizedAccountReferenceType, ), - ), - interpreter.NewUnmeteredStringValue("key"), - interpreter.NewCapabilityValue( - nil, - interpreter.NewUnmeteredUInt64Value(1234), - interpreter.NewAddressValue(nil, common.Address{}), - unauthorizedAccountReferenceType, - ), - ), + ) + }, + // NOTE: disabled, as storage is not expected to be always valid _during_ migration + validateStorage: false, }, } - // Store values - - for name, testCase := range testCases { - transferredValue := testCase.storedValue.Transfer( - inter, - locationRange, - atree.Address(account), - false, - nil, - nil, - ) + test := func(name string, testCase testCase) { - inter.WriteStored( - account, - pathDomain.Identifier(), - interpreter.StringStorageMapKey(name), - transferredValue, - ) - } + t.Run(name, func(t *testing.T) { + t.Parallel() - err = storage.Commit(inter, true) - require.NoError(t, err) + ledger := NewTestLedger(nil, nil) + storage := runtime.NewStorage(ledger, nil) - // Migrate + inter, err := interpreter.NewInterpreter( + nil, + utils.TestLocation, + &interpreter.Config{ + Storage: storage, + AtreeValueValidationEnabled: true, + AtreeStorageValidationEnabled: testCase.validateStorage, + }, + ) + require.NoError(t, err) - migration := migrations.NewStorageMigration(inter, storage) + // Store values - reporter := newTestReporter() + transferredValue := testCase.storedValue(inter).Transfer( + inter, + interpreter.EmptyLocationRange, + atree.Address(account), + false, + nil, + nil, + ) - migration.Migrate( - &migrations.AddressSliceIterator{ - Addresses: []common.Address{ + inter.WriteStored( account, - }, - }, - migration.NewValueMigrationsPathMigrator( - reporter, - NewStaticTypeMigration(), - ), - ) + pathDomain.Identifier(), + interpreter.StringStorageMapKey(name), + transferredValue, + ) + + err = storage.Commit(inter, true) + require.NoError(t, err) + + // Migrate - err = migration.Commit() - require.NoError(t, err) + migration := migrations.NewStorageMigration(inter, storage) + + reporter := newTestReporter() - err = storage.CheckHealth() - require.NoError(t, err) + migration.Migrate( + &migrations.AddressSliceIterator{ + Addresses: []common.Address{ + account, + }, + }, + migration.NewValueMigrationsPathMigrator( + reporter, + NewStaticTypeMigration(), + ), + ) - require.Empty(t, reporter.errors) + err = migration.Commit() + require.NoError(t, err) - // Assert: Traverse through the storage and see if the values are updated now. + // Assert - storageMap := storage.GetStorageMap(account, pathDomain.Identifier(), false) - require.NotNil(t, storageMap) - require.Greater(t, storageMap.Count(), uint64(0)) + require.Empty(t, reporter.errors) - iterator := storageMap.Iterator(inter) + err = storage.CheckHealth() + require.NoError(t, err) - for key, value := iterator.Next(); key != nil; key, value = iterator.Next() { - identifier := string(key.(interpreter.StringAtreeValue)) + storageMap := storage.GetStorageMap(account, pathDomain.Identifier(), false) + require.NotNil(t, storageMap) + require.Equal(t, uint64(1), storageMap.Count()) - t.Run(identifier, func(t *testing.T) { - testCase, ok := testCases[identifier] - require.True(t, ok) + value := storageMap.ReadValue(nil, interpreter.StringStorageMapKey(name)) expectedStoredValue := testCase.expectedValue if expectedStoredValue == nil { expectedStoredValue = testCase.storedValue } - utils.AssertValuesEqual(t, inter, expectedStoredValue, value) + utils.AssertValuesEqual(t, inter, expectedStoredValue(inter), value) }) } + + for name, testCase := range testCases { + test(name, testCase) + } } var testAddress = common.Address{0x42} @@ -1238,11 +1334,13 @@ func TestAccountTypeRehash(t *testing.T) { err := migration.Commit() require.NoError(t, err) - err = storage.CheckHealth() - require.NoError(t, err) + // Assert require.Empty(t, reporter.errors) + err = storage.CheckHealth() + require.NoError(t, err) + require.Equal(t, map[struct { interpreter.StorageKey diff --git a/migrations/statictypes/composite_type_migration_test.go b/migrations/statictypes/composite_type_migration_test.go index d1ebfe25c0..dfcfc452a1 100644 --- a/migrations/statictypes/composite_type_migration_test.go +++ b/migrations/statictypes/composite_type_migration_test.go @@ -185,11 +185,13 @@ func TestCompositeAndInterfaceTypeMigration(t *testing.T) { err = migration.Commit() require.NoError(t, err) - err = storage.CheckHealth() - require.NoError(t, err) + // Assert require.Empty(t, reporter.errors) + err = storage.CheckHealth() + require.NoError(t, err) + // Check reported migrated paths for identifier, test := range testCases { key := struct { diff --git a/migrations/statictypes/intersection_type_migration_test.go b/migrations/statictypes/intersection_type_migration_test.go index eff4e7483c..515edf0484 100644 --- a/migrations/statictypes/intersection_type_migration_test.go +++ b/migrations/statictypes/intersection_type_migration_test.go @@ -417,11 +417,13 @@ func TestIntersectionTypeMigration(t *testing.T) { err = migration.Commit() require.NoError(t, err) - err = storage.CheckHealth() - require.NoError(t, err) + // Assert require.Empty(t, reporter.errors) + err = storage.CheckHealth() + require.NoError(t, err) + // Assert the migrated values. // Traverse through the storage and see if the values are updated now. @@ -587,11 +589,13 @@ func TestIntersectionTypeRehash(t *testing.T) { err := migration.Commit() require.NoError(t, err) - err = storage.CheckHealth() - require.NoError(t, err) + // Assert require.Empty(t, reporter.errors) + err = storage.CheckHealth() + require.NoError(t, err) + require.Equal(t, map[struct { interpreter.StorageKey @@ -756,11 +760,13 @@ func TestRehashNestedIntersectionType(t *testing.T) { err := migration.Commit() require.NoError(t, err) - err = storage.CheckHealth() - require.NoError(t, err) + // Assert require.Empty(t, reporter.errors) + err = storage.CheckHealth() + require.NoError(t, err) + require.Equal(t, map[struct { interpreter.StorageKey @@ -900,11 +906,13 @@ func TestRehashNestedIntersectionType(t *testing.T) { err := migration.Commit() require.NoError(t, err) - err = storage.CheckHealth() - require.NoError(t, err) + // Assert require.Empty(t, reporter.errors) + err = storage.CheckHealth() + require.NoError(t, err) + require.Equal(t, map[struct { interpreter.StorageKey @@ -1077,11 +1085,13 @@ func TestIntersectionTypeMigrationWithInterfaceTypeConverter(t *testing.T) { err = migration.Commit() require.NoError(t, err) - err = storage.CheckHealth() - require.NoError(t, err) + // Assert require.Empty(t, reporter.errors) + err = storage.CheckHealth() + require.NoError(t, err) + // Assert the migrated value. storageMap := storage.GetStorageMap(testAddress, testPathDomain.Identifier(), false) @@ -1452,11 +1462,13 @@ func TestIntersectionTypeMigrationWithTypeConverters(t *testing.T) { err = migration.Commit() require.NoError(t, err) - err = storage.CheckHealth() - require.NoError(t, err) + // Assert require.Empty(t, reporter.errors) + err = storage.CheckHealth() + require.NoError(t, err) + key := struct { interpreter.StorageKey interpreter.StorageMapKey diff --git a/migrations/statictypes/statictype_migration_test.go b/migrations/statictypes/statictype_migration_test.go index 99b16a1aef..947aee24d0 100644 --- a/migrations/statictypes/statictype_migration_test.go +++ b/migrations/statictypes/statictype_migration_test.go @@ -41,6 +41,7 @@ func TestStaticTypeMigration(t *testing.T) { t *testing.T, staticTypeMigration *StaticTypeMigration, value interpreter.Value, + atreeValueValidationEnabled bool, ) interpreter.Value { // Store values @@ -53,7 +54,7 @@ func TestStaticTypeMigration(t *testing.T) { utils.TestLocation, &interpreter.Config{ Storage: storage, - AtreeValueValidationEnabled: false, + AtreeValueValidationEnabled: atreeValueValidationEnabled, AtreeStorageValidationEnabled: true, }, ) @@ -93,11 +94,13 @@ func TestStaticTypeMigration(t *testing.T) { err = migration.Commit() require.NoError(t, err) + // Assert + + require.Empty(t, reporter.errors) + err = storage.CheckHealth() require.NoError(t, err) - assert.Empty(t, reporter.errors) - storageMap := storage.GetStorageMap( testAddress, storageDomain, @@ -120,6 +123,10 @@ func TestStaticTypeMigration(t *testing.T) { actual := migrate(t, staticTypeMigration, interpreter.NewTypeValue(nil, nil), + // NOTE: atree value validation is disabled, + // because the type value has a nil type (which indicates an invalid or unknown type), + // and invalid unknown types are always unequal + false, ) assert.Equal(t, interpreter.NewTypeValue(nil, nil), @@ -144,6 +151,7 @@ func TestStaticTypeMigration(t *testing.T) { Path: path, Address: interpreter.AddressValue(testAddress), }, + true, ) assert.Equal(t, &interpreter.PathCapabilityValue{ //nolint:staticcheck @@ -181,6 +189,7 @@ func TestStaticTypeMigration(t *testing.T) { }, }, ), + true, ) assert.Equal(t, interpreter.NewUnmeteredTypeValue( @@ -220,6 +229,7 @@ func TestStaticTypeMigration(t *testing.T) { }, }, ), + true, ) assert.Equal(t, interpreter.NewUnmeteredTypeValue( @@ -268,6 +278,7 @@ func TestStaticTypeMigration(t *testing.T) { LegacyType: interpreter.PrimitiveStaticTypeAnyStruct, }, ), + true, ) assert.Equal(t, interpreter.NewUnmeteredTypeValue( @@ -307,6 +318,7 @@ func TestStaticTypeMigration(t *testing.T) { }, }, ), + true, ) assert.Equal(t, interpreter.NewUnmeteredTypeValue( @@ -347,6 +359,7 @@ func TestStaticTypeMigration(t *testing.T) { LegacyType: interpreter.PrimitiveStaticTypeAnyResource, }, ), + true, ) assert.Equal(t, interpreter.NewUnmeteredTypeValue( @@ -386,6 +399,7 @@ func TestStaticTypeMigration(t *testing.T) { }, }, ), + true, ) assert.Equal(t, interpreter.NewUnmeteredTypeValue( @@ -424,6 +438,7 @@ func TestStaticTypeMigration(t *testing.T) { LegacyType: interpreter.PrimitiveStaticTypeAnyStruct, }, ), + true, ) assert.Equal(t, interpreter.NewUnmeteredTypeValue( @@ -449,6 +464,7 @@ func TestStaticTypeMigration(t *testing.T) { }, }, ), + true, ) assert.Equal(t, interpreter.NewUnmeteredTypeValue( @@ -477,6 +493,7 @@ func TestStaticTypeMigration(t *testing.T) { LegacyType: interpreter.PrimitiveStaticTypeAnyResource, }, ), + true, ) assert.Equal(t, interpreter.NewUnmeteredTypeValue( @@ -502,6 +519,7 @@ func TestStaticTypeMigration(t *testing.T) { }, }, ), + true, ) assert.Equal(t, interpreter.NewUnmeteredTypeValue( @@ -533,6 +551,7 @@ func TestStaticTypeMigration(t *testing.T) { }, }, ), + true, ) assert.Equal(t, interpreter.NewUnmeteredTypeValue( @@ -565,6 +584,7 @@ func TestStaticTypeMigration(t *testing.T) { }, }, ), + true, ) assert.Equal(t, interpreter.NewUnmeteredTypeValue( @@ -642,11 +662,13 @@ func TestMigratingNestedContainers(t *testing.T) { err = migration.Commit() require.NoError(t, err) + // Assert + + require.Empty(t, reporter.errors) + err = storage.CheckHealth() require.NoError(t, err) - assert.Empty(t, reporter.errors) - storageMap := storage.GetStorageMap( testAddress, storageDomain, @@ -675,8 +697,9 @@ func TestMigratingNestedContainers(t *testing.T) { nil, utils.TestLocation, &interpreter.Config{ - Storage: storage, - AtreeValueValidationEnabled: false, + Storage: storage, + AtreeValueValidationEnabled: true, + // NOTE: disabled, as storage is not expected to be always valid _during_ migration AtreeStorageValidationEnabled: false, }, ) From 407233e05aaa4705f5a26cbb61a52d5326ab6c0d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Tue, 5 Mar 2024 09:57:12 -0800 Subject: [PATCH 06/17] add DictionaryValue InsertWithoutTransfer and RemoveWithoutTransfer --- runtime/interpreter/value.go | 86 +++++++++++++++++++++++++----------- 1 file changed, 61 insertions(+), 25 deletions(-) diff --git a/runtime/interpreter/value.go b/runtime/interpreter/value.go index db8970fe95..a6f91a47bb 100644 --- a/runtime/interpreter/value.go +++ b/runtime/interpreter/value.go @@ -18659,11 +18659,16 @@ type DictionaryIterator struct { mapIterator *atree.MapIterator } -func (i DictionaryIterator) NextKey(gauge common.MemoryGauge) Value { +func (i DictionaryIterator) NextKeyUnconverted() atree.Value { atreeValue, err := i.mapIterator.NextKey() if err != nil { panic(errors.NewExternalError(err)) } + return atreeValue +} + +func (i DictionaryIterator) NextKey(gauge common.MemoryGauge) Value { + atreeValue := i.NextKeyUnconverted() if atreeValue == nil { return nil } @@ -19149,11 +19154,14 @@ func (v *DictionaryValue) RemoveKey( return v.Remove(interpreter, locationRange, key) } -func (v *DictionaryValue) Remove( +func (v *DictionaryValue) RemoveWithoutTransfer( interpreter *Interpreter, locationRange LocationRange, - keyValue Value, -) OptionalValue { + keyValue atree.Value, +) ( + existingKeyStorable, + existingValueStorable atree.Storable, +) { interpreter.validateMutation(v.StorageID(), locationRange) @@ -19162,7 +19170,8 @@ func (v *DictionaryValue) Remove( // No need to clean up storable for passed-in key value, // as atree never calls Storable() - existingKeyStorable, existingValueStorable, err := v.dictionary.Remove( + var err error + existingKeyStorable, existingValueStorable, err = v.dictionary.Remove( valueComparator, hashInputProvider, keyValue, @@ -19170,12 +19179,27 @@ func (v *DictionaryValue) Remove( if err != nil { var keyNotFoundError *atree.KeyNotFoundError if goerrors.As(err, &keyNotFoundError) { - return NilOptionalValue + return nil, nil } panic(errors.NewExternalError(err)) } interpreter.maybeValidateAtreeValue(v.dictionary) + return existingKeyStorable, existingValueStorable +} + +func (v *DictionaryValue) Remove( + interpreter *Interpreter, + locationRange LocationRange, + keyValue Value, +) OptionalValue { + + existingKeyStorable, existingValueStorable := v.RemoveWithoutTransfer(interpreter, locationRange, keyValue) + + if existingKeyStorable == nil { + return NilOptionalValue + } + storage := interpreter.Storage() // Key @@ -19207,11 +19231,11 @@ func (v *DictionaryValue) InsertKey( v.SetKey(interpreter, locationRange, key, value) } -func (v *DictionaryValue) Insert( +func (v *DictionaryValue) InsertWithoutTransfer( interpreter *Interpreter, locationRange LocationRange, - keyValue, value Value, -) OptionalValue { + keyValue, value atree.Value, +) (existingValueStorable atree.Storable) { interpreter.validateMutation(v.StorageID(), locationRange) @@ -19221,8 +19245,31 @@ func (v *DictionaryValue) Insert( common.UseMemory(interpreter, dataSlabs) common.UseMemory(interpreter, metaDataSlabs) - interpreter.checkContainerMutation(v.Type.KeyType, keyValue, locationRange) - interpreter.checkContainerMutation(v.Type.ValueType, value, locationRange) + valueComparator := newValueComparator(interpreter, locationRange) + hashInputProvider := newHashInputProvider(interpreter, locationRange) + + // atree only calls Storable() on keyValue if needed, + // i.e., if the key is a new key + var err error + existingValueStorable, err = v.dictionary.Set( + valueComparator, + hashInputProvider, + keyValue, + value, + ) + if err != nil { + panic(errors.NewExternalError(err)) + } + interpreter.maybeValidateAtreeValue(v.dictionary) + + return existingValueStorable +} + +func (v *DictionaryValue) Insert( + interpreter *Interpreter, + locationRange LocationRange, + keyValue, value Value, +) OptionalValue { address := v.dictionary.Address() @@ -19248,21 +19295,10 @@ func (v *DictionaryValue) Insert( preventTransfer, ) - valueComparator := newValueComparator(interpreter, locationRange) - hashInputProvider := newHashInputProvider(interpreter, locationRange) + interpreter.checkContainerMutation(v.Type.KeyType, keyValue, locationRange) + interpreter.checkContainerMutation(v.Type.ValueType, value, locationRange) - // atree only calls Storable() on keyValue if needed, - // i.e., if the key is a new key - existingValueStorable, err := v.dictionary.Set( - valueComparator, - hashInputProvider, - keyValue, - value, - ) - if err != nil { - panic(errors.NewExternalError(err)) - } - interpreter.maybeValidateAtreeValue(v.dictionary) + existingValueStorable := v.InsertWithoutTransfer(interpreter, locationRange, keyValue, value) if existingValueStorable == nil { return NilOptionalValue From 15b7ac314b6c61398b76bd08e22309f7cf88bc28 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Tue, 5 Mar 2024 09:59:09 -0800 Subject: [PATCH 07/17] fix dictionary value case in storage migration: always remove old value before reinserting --- migrations/migration.go | 37 +++++++++++++++++++------------------ 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/migrations/migration.go b/migrations/migration.go index 90fa0d829e..f4c0f194c2 100644 --- a/migrations/migration.go +++ b/migrations/migration.go @@ -303,23 +303,6 @@ func (m *StorageMigration) MigrateNestedValue( if newKey == nil { keyToSet = existingKey } else { - // 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.NewUnexpectedError( - "failed to remove old value for migrated key: %s", - existingKey, - )) - } - keyToSet = newKey } @@ -330,7 +313,25 @@ func (m *StorageMigration) MigrateNestedValue( valueToSet = newValue } - dictionary.Insert( + existingKey = legacyKey(existingKey) + existingKeyStorable, existingValueStorable := dictionary.RemoveWithoutTransfer( + m.interpreter, + emptyLocationRange, + existingKey, + ) + + if existingKeyStorable == nil { + panic(errors.NewUnexpectedError( + "failed to remove old value for migrated key: %s", + existingKey, + )) + } + + interpreter.StoredValue(m.interpreter, existingValueStorable, m.storage). + DeepRemove(m.interpreter) + m.interpreter.RemoveReferencedSlab(existingValueStorable) + + dictionary.InsertWithoutTransfer( m.interpreter, emptyLocationRange, keyToSet, From 33b0c5286d23467687f00a12b73be671a3ff5c77 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Tue, 5 Mar 2024 10:02:17 -0800 Subject: [PATCH 08/17] fix dictionary value case in static type and entitlements migration remove keys/values from old dictionar and insert into new dictionary without transfer out/in --- migrations/entitlements/migration.go | 66 ++++++++++++++----- .../statictypes/statictype_migration.go | 56 ++++++++++++---- 2 files changed, 95 insertions(+), 27 deletions(-) diff --git a/migrations/entitlements/migration.go b/migrations/entitlements/migration.go index 959df44c3e..b11801ad42 100644 --- a/migrations/entitlements/migration.go +++ b/migrations/entitlements/migration.go @@ -21,8 +21,11 @@ package entitlements import ( "fmt" + "github.com/onflow/atree" + "github.com/onflow/cadence/migrations" "github.com/onflow/cadence/migrations/statictypes" + "github.com/onflow/cadence/runtime/errors" "github.com/onflow/cadence/runtime/interpreter" "github.com/onflow/cadence/runtime/sema" ) @@ -261,29 +264,62 @@ func ConvertValueToEntitlements( return nil, err } - if entitledElementType != nil { - var keysAndValues []interpreter.Value + if entitledElementType == nil { + return nil, nil + } - iterator := v.Iterator() - for { - keyValue, value := iterator.Next(inter) - if keyValue == nil { - break - } + newDictionary := interpreter.NewDictionaryValueWithAddress( + inter, + interpreter.EmptyLocationRange, + entitledElementType.(*interpreter.DictionaryStaticType), + v.GetOwner(), + ) + + var keys []atree.Value - keysAndValues = append(keysAndValues, keyValue) - keysAndValues = append(keysAndValues, value) + iterator := v.Iterator() + + for { + key := iterator.NextKeyUnconverted() + if key == nil { + break } - return interpreter.NewDictionaryValueWithAddress( + keys = append(keys, key) + } + + storage := inter.Storage() + + for _, key := range keys { + existingKeyStorable, existingValueStorable := v.RemoveWithoutTransfer( inter, interpreter.EmptyLocationRange, - entitledElementType.(*interpreter.DictionaryStaticType), - v.GetOwner(), - keysAndValues..., - ), nil + key, + ) + if existingKeyStorable == nil || existingValueStorable == nil { + panic(errors.NewUnreachableError()) + } + + newKey, err := existingKeyStorable.StoredValue(storage) + if err != nil { + panic(err) + } + + newValue, err := existingValueStorable.StoredValue(storage) + if err != nil { + panic(err) + } + + newDictionary.InsertWithoutTransfer( + inter, + interpreter.EmptyLocationRange, + newKey, + newValue, + ) } + return newDictionary, nil + case *interpreter.IDCapabilityValue: borrowType := v.BorrowType diff --git a/migrations/statictypes/statictype_migration.go b/migrations/statictypes/statictype_migration.go index 35f54e1cfd..c196b775a4 100644 --- a/migrations/statictypes/statictype_migration.go +++ b/migrations/statictypes/statictype_migration.go @@ -21,6 +21,8 @@ package statictypes import ( "fmt" + "github.com/onflow/atree" + "github.com/onflow/cadence/migrations" "github.com/onflow/cadence/runtime/common" "github.com/onflow/cadence/runtime/errors" @@ -154,27 +156,57 @@ func (m *StaticTypeMigration) Migrate( return } - var keysAndValues []interpreter.Value + newDictionary := interpreter.NewDictionaryValueWithAddress( + inter, + interpreter.EmptyLocationRange, + convertedElementType.(*interpreter.DictionaryStaticType), + value.GetOwner(), + ) + + var keys []atree.Value iterator := value.Iterator() for { - keyValue, value := iterator.Next(inter) - if keyValue == nil { + key := iterator.NextKeyUnconverted() + if key == nil { break } - keysAndValues = append(keysAndValues, keyValue) - keysAndValues = append(keysAndValues, value) + keys = append(keys, key) } - return interpreter.NewDictionaryValueWithAddress( - inter, - interpreter.EmptyLocationRange, - convertedElementType.(*interpreter.DictionaryStaticType), - value.GetOwner(), - keysAndValues..., - ), nil + storage := inter.Storage() + + for _, key := range keys { + existingKeyStorable, existingValueStorable := value.RemoveWithoutTransfer( + inter, + interpreter.EmptyLocationRange, + key, + ) + if existingKeyStorable == nil || existingValueStorable == nil { + panic(errors.NewUnreachableError()) + } + + newKey, err := existingKeyStorable.StoredValue(storage) + if err != nil { + panic(err) + } + + newValue, err := existingValueStorable.StoredValue(storage) + if err != nil { + panic(err) + } + + newDictionary.InsertWithoutTransfer( + inter, + interpreter.EmptyLocationRange, + newKey, + newValue, + ) + } + + return newDictionary, nil } return From 5db6a8cbf3051dd70aa4c5d9a63b8e77a9970638 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Tue, 5 Mar 2024 10:45:29 -0800 Subject: [PATCH 09/17] refactor dictionary creation into function --- migrations/entitlements/migration.go | 53 +---------------- .../statictypes/statictype_migration.go | 52 +---------------- runtime/interpreter/value.go | 58 +++++++++++++++++++ 3 files changed, 62 insertions(+), 101 deletions(-) diff --git a/migrations/entitlements/migration.go b/migrations/entitlements/migration.go index b11801ad42..a6b5d8e272 100644 --- a/migrations/entitlements/migration.go +++ b/migrations/entitlements/migration.go @@ -21,11 +21,8 @@ package entitlements import ( "fmt" - "github.com/onflow/atree" - "github.com/onflow/cadence/migrations" "github.com/onflow/cadence/migrations/statictypes" - "github.com/onflow/cadence/runtime/errors" "github.com/onflow/cadence/runtime/interpreter" "github.com/onflow/cadence/runtime/sema" ) @@ -268,57 +265,11 @@ func ConvertValueToEntitlements( return nil, nil } - newDictionary := interpreter.NewDictionaryValueWithAddress( + return v.NewWithType( inter, interpreter.EmptyLocationRange, entitledElementType.(*interpreter.DictionaryStaticType), - v.GetOwner(), - ) - - var keys []atree.Value - - iterator := v.Iterator() - - for { - key := iterator.NextKeyUnconverted() - if key == nil { - break - } - - keys = append(keys, key) - } - - storage := inter.Storage() - - for _, key := range keys { - existingKeyStorable, existingValueStorable := v.RemoveWithoutTransfer( - inter, - interpreter.EmptyLocationRange, - key, - ) - if existingKeyStorable == nil || existingValueStorable == nil { - panic(errors.NewUnreachableError()) - } - - newKey, err := existingKeyStorable.StoredValue(storage) - if err != nil { - panic(err) - } - - newValue, err := existingValueStorable.StoredValue(storage) - if err != nil { - panic(err) - } - - newDictionary.InsertWithoutTransfer( - inter, - interpreter.EmptyLocationRange, - newKey, - newValue, - ) - } - - return newDictionary, nil + ), nil case *interpreter.IDCapabilityValue: borrowType := v.BorrowType diff --git a/migrations/statictypes/statictype_migration.go b/migrations/statictypes/statictype_migration.go index c196b775a4..115007833b 100644 --- a/migrations/statictypes/statictype_migration.go +++ b/migrations/statictypes/statictype_migration.go @@ -21,8 +21,6 @@ package statictypes import ( "fmt" - "github.com/onflow/atree" - "github.com/onflow/cadence/migrations" "github.com/onflow/cadence/runtime/common" "github.com/onflow/cadence/runtime/errors" @@ -156,57 +154,11 @@ func (m *StaticTypeMigration) Migrate( return } - newDictionary := interpreter.NewDictionaryValueWithAddress( + return value.NewWithType( inter, interpreter.EmptyLocationRange, convertedElementType.(*interpreter.DictionaryStaticType), - value.GetOwner(), - ) - - var keys []atree.Value - - iterator := value.Iterator() - - for { - key := iterator.NextKeyUnconverted() - if key == nil { - break - } - - keys = append(keys, key) - } - - storage := inter.Storage() - - for _, key := range keys { - existingKeyStorable, existingValueStorable := value.RemoveWithoutTransfer( - inter, - interpreter.EmptyLocationRange, - key, - ) - if existingKeyStorable == nil || existingValueStorable == nil { - panic(errors.NewUnreachableError()) - } - - newKey, err := existingKeyStorable.StoredValue(storage) - if err != nil { - panic(err) - } - - newValue, err := existingValueStorable.StoredValue(storage) - if err != nil { - panic(err) - } - - newDictionary.InsertWithoutTransfer( - inter, - interpreter.EmptyLocationRange, - newKey, - newValue, - ) - } - - return newDictionary, nil + ), nil } return diff --git a/runtime/interpreter/value.go b/runtime/interpreter/value.go index bd783f6433..d235c61b9e 100644 --- a/runtime/interpreter/value.go +++ b/runtime/interpreter/value.go @@ -18574,6 +18574,64 @@ func newDictionaryValueFromAtreeMap( } } +func (v *DictionaryValue) NewWithType( + inter *Interpreter, + locationRange LocationRange, + newType *DictionaryStaticType, +) *DictionaryValue { + newDictionary := NewDictionaryValueWithAddress( + inter, + locationRange, + newType, + v.GetOwner(), + ) + + var keys []atree.Value + + iterator := v.Iterator() + + for { + key := iterator.NextKeyUnconverted() + if key == nil { + break + } + + keys = append(keys, key) + } + + storage := inter.Storage() + + for _, key := range keys { + existingKeyStorable, existingValueStorable := v.RemoveWithoutTransfer( + inter, + locationRange, + key, + ) + if existingKeyStorable == nil || existingValueStorable == nil { + panic(errors.NewUnreachableError()) + } + + newKey, err := existingKeyStorable.StoredValue(storage) + if err != nil { + panic(err) + } + + newValue, err := existingValueStorable.StoredValue(storage) + if err != nil { + panic(err) + } + + newDictionary.InsertWithoutTransfer( + inter, + locationRange, + newKey, + newValue, + ) + } + + return newDictionary +} + var _ Value = &DictionaryValue{} var _ atree.Value = &DictionaryValue{} var _ EquatableValue = &DictionaryValue{} From e841f2b35e095517f5b976e7af61acae7e4655be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Tue, 5 Mar 2024 11:17:41 -0800 Subject: [PATCH 10/17] add test for DictionaryValue.NewWithType --- runtime/interpreter/value_test.go | 44 +++++++++++++++++++++++++++++++ 1 file changed, 44 insertions(+) diff --git a/runtime/interpreter/value_test.go b/runtime/interpreter/value_test.go index 2fdb809b6d..785b991bd5 100644 --- a/runtime/interpreter/value_test.go +++ b/runtime/interpreter/value_test.go @@ -4187,3 +4187,47 @@ func TestValue_ConformsToStaticType(t *testing.T) { }) } + +func TestDictionaryValue_NewWithType(t *testing.T) { + + t.Parallel() + + inter := newTestInterpreter(t) + + address := common.Address{0x1} + + newDictionaryValue := func(dictionaryType *DictionaryStaticType) *DictionaryValue { + return NewDictionaryValueWithAddress( + inter, + EmptyLocationRange, + dictionaryType, + address, + NewUnmeteredStringValue("a"), + NewUnmeteredIntValueFromInt64(1), + NewUnmeteredStringValue("b"), + NewUnmeteredIntValueFromInt64(2), + NewUnmeteredStringValue("c"), + NewUnmeteredIntValueFromInt64(3), + ) + } + + oldType := &DictionaryStaticType{ + KeyType: PrimitiveStaticTypeString, + ValueType: PrimitiveStaticTypeAnyStruct, + } + + newType := &DictionaryStaticType{ + KeyType: PrimitiveStaticTypeString, + ValueType: PrimitiveStaticTypeInt, + } + + require.True(t, + newDictionaryValue(oldType). + NewWithType(inter, EmptyLocationRange, newType). + Equal( + inter, + EmptyLocationRange, + newDictionaryValue(newType), + ), + ) +} From 67e5130430f9e5dd4cbdaca4fc06a2a2bc33baae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Tue, 5 Mar 2024 11:28:23 -0800 Subject: [PATCH 11/17] add ArrayValue.InsertWithoutTransfer and RemoveWithoutTransfer --- runtime/interpreter/value.go | 56 +++++++++++++++++++++++++++--------- 1 file changed, 42 insertions(+), 14 deletions(-) diff --git a/runtime/interpreter/value.go b/runtime/interpreter/value.go index d235c61b9e..f66e318cd0 100644 --- a/runtime/interpreter/value.go +++ b/runtime/interpreter/value.go @@ -2260,8 +2260,12 @@ func (v *ArrayValue) InsertKey(interpreter *Interpreter, locationRange LocationR v.Insert(interpreter, locationRange, index, value) } -func (v *ArrayValue) Insert(interpreter *Interpreter, locationRange LocationRange, index int, element Value) { - +func (v *ArrayValue) InsertWithoutTransfer( + interpreter *Interpreter, + locationRange LocationRange, + index int, + element atree.Value, +) { interpreter.validateMutation(v.StorageID(), locationRange) // We only need to check the lower bound before converting from `int` (signed) to `uint64` (unsigned). @@ -2285,26 +2289,40 @@ func (v *ArrayValue) Insert(interpreter *Interpreter, locationRange LocationRang common.UseMemory(interpreter, metaDataSlabs) common.UseMemory(interpreter, common.AtreeArrayElementOverhead) - interpreter.checkContainerMutation(v.Type.ElementType(), element, locationRange) + err := v.array.Insert(uint64(index), element) + if err != nil { + v.handleIndexOutOfBoundsError(err, index, locationRange) + + panic(errors.NewExternalError(err)) + } + interpreter.maybeValidateAtreeValue(v.array) +} + +func (v *ArrayValue) Insert(interpreter *Interpreter, locationRange LocationRange, index int, element Value) { + + address := v.array.Address() + + preventTransfer := map[atree.StorageID]struct{}{ + v.StorageID(): {}, + } element = element.Transfer( interpreter, locationRange, - v.array.Address(), + address, true, nil, - map[atree.StorageID]struct{}{ - v.StorageID(): {}, - }, + preventTransfer, ) - err := v.array.Insert(uint64(index), element) - if err != nil { - v.handleIndexOutOfBoundsError(err, index, locationRange) + interpreter.checkContainerMutation(v.Type.ElementType(), element, locationRange) - panic(errors.NewExternalError(err)) - } - interpreter.maybeValidateAtreeValue(v.array) + v.InsertWithoutTransfer( + interpreter, + locationRange, + index, + element, + ) } func (v *ArrayValue) RemoveKey(interpreter *Interpreter, locationRange LocationRange, key Value) Value { @@ -2312,7 +2330,11 @@ func (v *ArrayValue) RemoveKey(interpreter *Interpreter, locationRange LocationR return v.Remove(interpreter, locationRange, index) } -func (v *ArrayValue) Remove(interpreter *Interpreter, locationRange LocationRange, index int) Value { +func (v *ArrayValue) RemoveWithoutTransfer( + interpreter *Interpreter, + locationRange LocationRange, + index int, +) atree.Storable { interpreter.validateMutation(v.StorageID(), locationRange) @@ -2335,6 +2357,12 @@ func (v *ArrayValue) Remove(interpreter *Interpreter, locationRange LocationRang } interpreter.maybeValidateAtreeValue(v.array) + return storable +} + +func (v *ArrayValue) Remove(interpreter *Interpreter, locationRange LocationRange, index int) Value { + storable := v.RemoveWithoutTransfer(interpreter, locationRange, index) + value := StoredValue(interpreter, storable, interpreter.Storage()) return value.Transfer( From 792bf1b07108d9e7cde6d7acb14a8ded3cf6b20a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Tue, 5 Mar 2024 11:28:38 -0800 Subject: [PATCH 12/17] add ArrayValue.NewWithType --- runtime/interpreter/value.go | 52 +++++++++++++++++++++++++++++++ runtime/interpreter/value_test.go | 39 +++++++++++++++++++++++ 2 files changed, 91 insertions(+) diff --git a/runtime/interpreter/value.go b/runtime/interpreter/value.go index f66e318cd0..ceb02940a5 100644 --- a/runtime/interpreter/value.go +++ b/runtime/interpreter/value.go @@ -3518,6 +3518,57 @@ func (v *ArrayValue) ToConstantSized( ) } +func (v *ArrayValue) NewWithType( + inter *Interpreter, + locationRange LocationRange, + newType ArrayStaticType, +) *ArrayValue { + + newArray := NewArrayValue( + inter, + locationRange, + newType, + v.GetOwner(), + ) + + storage := inter.Storage() + + count := v.Count() + + for index := 0; index < count; index++ { + + storable := v.RemoveWithoutTransfer( + inter, + locationRange, + // NOTE: always removing first element, + // until original array is empty + 0, + ) + + if storable == nil { + panic(errors.NewUnreachableError()) + } + + if v.Count() != count-index-1 { + panic(errors.NewUnreachableError()) + } + + newValue, err := storable.StoredValue(storage) + if err != nil { + panic(err) + } + + newArray.InsertWithoutTransfer( + inter, + locationRange, + index, + newValue, + ) + } + + return newArray +} + // NumberValue type NumberValue interface { ComparableValue @@ -18607,6 +18658,7 @@ func (v *DictionaryValue) NewWithType( locationRange LocationRange, newType *DictionaryStaticType, ) *DictionaryValue { + newDictionary := NewDictionaryValueWithAddress( inter, locationRange, diff --git a/runtime/interpreter/value_test.go b/runtime/interpreter/value_test.go index 785b991bd5..38fd3b5355 100644 --- a/runtime/interpreter/value_test.go +++ b/runtime/interpreter/value_test.go @@ -4231,3 +4231,42 @@ func TestDictionaryValue_NewWithType(t *testing.T) { ), ) } + +func TestArrayValue_NewWithType(t *testing.T) { + + t.Parallel() + + inter := newTestInterpreter(t) + + address := common.Address{0x1} + + newArrayValue := func(arrayType ArrayStaticType) *ArrayValue { + return NewArrayValue( + inter, + EmptyLocationRange, + arrayType, + address, + NewUnmeteredStringValue("a"), + NewUnmeteredStringValue("b"), + NewUnmeteredStringValue("c"), + ) + } + + oldType := &VariableSizedStaticType{ + Type: PrimitiveStaticTypeAnyStruct, + } + + newType := &VariableSizedStaticType{ + Type: PrimitiveStaticTypeString, + } + + require.True(t, + newArrayValue(oldType). + NewWithType(inter, EmptyLocationRange, newType). + Equal( + inter, + EmptyLocationRange, + newArrayValue(newType), + ), + ) +} From a301d13922d58e2cf4d2fa632e5f163696bdd311 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Tue, 5 Mar 2024 11:29:26 -0800 Subject: [PATCH 13/17] fix migration of arrays: migrate without transfer, remove old data if needed --- migrations/entitlements/migration.go | 10 +- migrations/migration.go | 29 +++-- .../statictypes/statictype_migration.go | 10 +- .../statictypes/statictype_migration_test.go | 101 ++++++++++++++++++ 4 files changed, 127 insertions(+), 23 deletions(-) diff --git a/migrations/entitlements/migration.go b/migrations/entitlements/migration.go index a6b5d8e272..72390a2a96 100644 --- a/migrations/entitlements/migration.go +++ b/migrations/entitlements/migration.go @@ -241,16 +241,10 @@ func ConvertValueToEntitlements( return nil, nil } - iterator := v.Iterator(inter, interpreter.EmptyLocationRange) - - return interpreter.NewArrayValueWithIterator( + return v.NewWithType( inter, + interpreter.EmptyLocationRange, entitledElementType.(interpreter.ArrayStaticType), - v.GetOwner(), - uint64(v.Count()), - func() interpreter.Value { - return iterator.Next(inter, interpreter.EmptyLocationRange) - }, ), nil case *interpreter.DictionaryValue: diff --git a/migrations/migration.go b/migrations/migration.go index f4c0f194c2..a076008119 100644 --- a/migrations/migration.go +++ b/migrations/migration.go @@ -189,7 +189,9 @@ func (m *StorageMigration) MigrateNestedValue( // Migrate array elements count := array.Count() for index := 0; index < count; index++ { + element := array.Get(m.interpreter, emptyLocationRange, index) + newElement := m.MigrateNestedValue( storageKey, storageMapKey, @@ -197,14 +199,27 @@ func (m *StorageMigration) MigrateNestedValue( valueMigrations, reporter, ) - if newElement != nil { - array.Set( - m.interpreter, - emptyLocationRange, - index, - newElement, - ) + + if newElement == nil { + continue } + + existingStorable := array.RemoveWithoutTransfer( + m.interpreter, + emptyLocationRange, + index, + ) + + interpreter.StoredValue(m.interpreter, existingStorable, m.storage). + DeepRemove(m.interpreter) + m.interpreter.RemoveReferencedSlab(existingStorable) + + array.InsertWithoutTransfer( + m.interpreter, + emptyLocationRange, + index, + newElement, + ) } case *interpreter.CompositeValue: diff --git a/migrations/statictypes/statictype_migration.go b/migrations/statictypes/statictype_migration.go index 115007833b..3bc3e7bd8f 100644 --- a/migrations/statictypes/statictype_migration.go +++ b/migrations/statictypes/statictype_migration.go @@ -136,16 +136,10 @@ func (m *StaticTypeMigration) Migrate( return } - iterator := value.Iterator(inter, interpreter.EmptyLocationRange) - - return interpreter.NewArrayValueWithIterator( + return value.NewWithType( inter, + interpreter.EmptyLocationRange, convertedElementType.(interpreter.ArrayStaticType), - value.GetOwner(), - uint64(value.Count()), - func() interpreter.Value { - return iterator.Next(inter, interpreter.EmptyLocationRange) - }, ), nil case *interpreter.DictionaryValue: diff --git a/migrations/statictypes/statictype_migration_test.go b/migrations/statictypes/statictype_migration_test.go index 947aee24d0..770bb3c2f8 100644 --- a/migrations/statictypes/statictype_migration_test.go +++ b/migrations/statictypes/statictype_migration_test.go @@ -788,4 +788,105 @@ func TestMigratingNestedContainers(t *testing.T) { utils.AssertValuesEqual(t, inter, expected, actual) }) + + t.Run("nested arrays", func(t *testing.T) { + t.Parallel() + + staticTypeMigration := NewStaticTypeMigration() + + locationRange := interpreter.EmptyLocationRange + + ledger := NewTestLedger(nil, nil) + storage := runtime.NewStorage(ledger, nil) + + inter, err := interpreter.NewInterpreter( + nil, + utils.TestLocation, + &interpreter.Config{ + Storage: storage, + AtreeValueValidationEnabled: true, + // NOTE: disabled, as storage is not expected to be always valid _during_ migration + AtreeStorageValidationEnabled: false, + }, + ) + require.NoError(t, err) + + storedValue := interpreter.NewArrayValue( + inter, + locationRange, + interpreter.NewVariableSizedStaticType( + nil, + interpreter.NewVariableSizedStaticType( + nil, + interpreter.NewCapabilityStaticType( + nil, + interpreter.PrimitiveStaticTypePublicAccount, //nolint:staticcheck + ), + ), + ), + common.ZeroAddress, + interpreter.NewArrayValue( + inter, + locationRange, + interpreter.NewVariableSizedStaticType( + nil, + interpreter.NewCapabilityStaticType( + nil, + interpreter.PrimitiveStaticTypePublicAccount, //nolint:staticcheck + ), + ), + common.ZeroAddress, + interpreter.NewCapabilityValue( + nil, + interpreter.NewUnmeteredUInt64Value(1234), + interpreter.NewAddressValue(nil, common.ZeroAddress), + interpreter.PrimitiveStaticTypePublicAccount, //nolint:staticcheck + ), + ), + ) + + actual := migrate(t, + staticTypeMigration, + storage, + inter, + storedValue, + ) + + expected := interpreter.NewArrayValue( + inter, + locationRange, + interpreter.NewVariableSizedStaticType( + nil, + interpreter.NewVariableSizedStaticType( + nil, + interpreter.NewCapabilityStaticType( + nil, + unauthorizedAccountReferenceType, + ), + ), + ), + common.ZeroAddress, + interpreter.NewArrayValue( + inter, + locationRange, + interpreter.NewVariableSizedStaticType( + nil, + interpreter.NewCapabilityStaticType( + nil, + unauthorizedAccountReferenceType, + ), + ), + common.ZeroAddress, + interpreter.NewCapabilityValue( + nil, + interpreter.NewUnmeteredUInt64Value(1234), + interpreter.NewAddressValue(nil, common.Address{}), + unauthorizedAccountReferenceType, + ), + ), + ) + + utils.AssertValuesEqual(t, inter, expected, actual) + }) + } From c6d423840cd254fd113fd6355cc59a75779ab6be Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Tue, 5 Mar 2024 11:59:41 -0800 Subject: [PATCH 14/17] add CompositeValue.SetMemberWithoutTransfer --- runtime/interpreter/value.go | 42 ++++++++++++++++++++++++------------ 1 file changed, 28 insertions(+), 14 deletions(-) diff --git a/runtime/interpreter/value.go b/runtime/interpreter/value.go index ceb02940a5..32ec72332f 100644 --- a/runtime/interpreter/value.go +++ b/runtime/interpreter/value.go @@ -17271,7 +17271,7 @@ func (v *CompositeValue) RemoveMember( ) } -func (v *CompositeValue) SetMember( +func (v *CompositeValue) SetMemberWithoutTransfer( interpreter *Interpreter, locationRange LocationRange, name string, @@ -17299,19 +17299,6 @@ func (v *CompositeValue) SetMember( }() } - address := v.StorageAddress() - - value = value.Transfer( - interpreter, - locationRange, - address, - true, - nil, - map[atree.StorageID]struct{}{ - v.StorageID(): {}, - }, - ) - existingStorable, err := v.dictionary.Set( StringAtreeValueComparator, StringAtreeValueHashInput, @@ -17335,6 +17322,33 @@ func (v *CompositeValue) SetMember( return false } +func (v *CompositeValue) SetMember( + interpreter *Interpreter, + locationRange LocationRange, + name string, + value Value, +) bool { + address := v.StorageAddress() + + value = value.Transfer( + interpreter, + locationRange, + address, + true, + nil, + map[atree.StorageID]struct{}{ + v.StorageID(): {}, + }, + ) + + return v.SetMemberWithoutTransfer( + interpreter, + locationRange, + name, + value, + ) +} + func (v *CompositeValue) String() string { return v.RecursiveString(SeenReferences{}) } From d10955b6c718151508f10a78ecf4ed51ceb6c444 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Tue, 5 Mar 2024 12:27:42 -0800 Subject: [PATCH 15/17] add test for migration of nested containers --- migrations/migration_test.go | 276 ++++++++++++++++++ .../statictypes/statictype_migration_test.go | 2 +- 2 files changed, 277 insertions(+), 1 deletion(-) diff --git a/migrations/migration_test.go b/migrations/migration_test.go index 61feb52d94..fdc768ee7d 100644 --- a/migrations/migration_test.go +++ b/migrations/migration_test.go @@ -1169,3 +1169,279 @@ func TestEmptyIntersectionTypeMigration(t *testing.T) { migratedCompositeValue.QualifiedIdentifier, ) } + +// testContainerMigration + +type testContainerMigration struct{} + +var _ ValueMigration = testContainerMigration{} + +func (testContainerMigration) Name() string { + return "testContainerMigration" +} + +func (testContainerMigration) Migrate( + _ interpreter.StorageKey, + _ interpreter.StorageMapKey, + value interpreter.Value, + inter *interpreter.Interpreter, +) (interpreter.Value, error) { + + switch value := value.(type) { + case *interpreter.DictionaryValue: + + newType := interpreter.NewDictionaryStaticType(nil, + interpreter.PrimitiveStaticTypeAnyStruct, + interpreter.PrimitiveStaticTypeAnyStruct, + ) + + return value.NewWithType(inter, emptyLocationRange, newType), nil + + case *interpreter.ArrayValue: + + newType := interpreter.NewVariableSizedStaticType(nil, + interpreter.PrimitiveStaticTypeAnyStruct, + ) + + return value.NewWithType(inter, emptyLocationRange, newType), nil + } + + return nil, nil +} + +func TestMigratingNestedContainers(t *testing.T) { + t.Parallel() + + var testAddress = common.Address{0x42} + + migrate := func( + t *testing.T, + storage *runtime.Storage, + inter *interpreter.Interpreter, + value interpreter.Value, + ) interpreter.Value { + + // Store values + + storageMapKey := interpreter.StringStorageMapKey("test_value") + storageDomain := common.PathDomainStorage.Identifier() + + value = value.Transfer( + inter, + interpreter.EmptyLocationRange, + atree.Address(testAddress), + false, + nil, + nil, + ) + + inter.WriteStored( + testAddress, + storageDomain, + storageMapKey, + value, + ) + + err := storage.Commit(inter, true) + require.NoError(t, err) + + // Migrate + + migration := NewStorageMigration(inter, storage) + + reporter := newTestReporter() + + migration.Migrate( + &AddressSliceIterator{ + Addresses: []common.Address{ + testAddress, + }, + }, + migration.NewValueMigrationsPathMigrator( + reporter, + testContainerMigration{}, + ), + ) + + err = migration.Commit() + require.NoError(t, err) + + // Assert + + require.Empty(t, reporter.errors) + + err = storage.CheckHealth() + require.NoError(t, err) + + storageMap := storage.GetStorageMap( + testAddress, + storageDomain, + false, + ) + require.NotNil(t, storageMap) + require.Equal(t, uint64(1), storageMap.Count()) + + result := storageMap.ReadValue(nil, storageMapKey) + require.NotNil(t, value) + + return result + } + + t.Run("nested dictionary", func(t *testing.T) { + t.Parallel() + + locationRange := interpreter.EmptyLocationRange + + ledger := NewTestLedger(nil, nil) + storage := runtime.NewStorage(ledger, nil) + + inter, err := interpreter.NewInterpreter( + nil, + utils.TestLocation, + &interpreter.Config{ + Storage: storage, + AtreeValueValidationEnabled: true, + // NOTE: disabled, as storage is not expected to be always valid _during_ migration + AtreeStorageValidationEnabled: false, + }, + ) + require.NoError(t, err) + + // {String: {String: Int}} + + storedValue := interpreter.NewDictionaryValue( + inter, + locationRange, + interpreter.NewDictionaryStaticType( + nil, + interpreter.PrimitiveStaticTypeString, + interpreter.NewDictionaryStaticType( + nil, + interpreter.PrimitiveStaticTypeString, + interpreter.PrimitiveStaticTypeInt, + ), + ), + interpreter.NewUnmeteredStringValue("key1"), + interpreter.NewDictionaryValue( + inter, + locationRange, + interpreter.NewDictionaryStaticType( + nil, + interpreter.PrimitiveStaticTypeString, + interpreter.PrimitiveStaticTypeInt, + ), + interpreter.NewUnmeteredStringValue("key2"), + interpreter.NewUnmeteredIntValueFromInt64(1234), + ), + ) + + actual := migrate(t, + storage, + inter, + storedValue, + ) + + // {AnyStruct: AnyStruct} with {AnyStruct: AnyStruct} + + expected := interpreter.NewDictionaryValue( + inter, + locationRange, + interpreter.NewDictionaryStaticType( + nil, + interpreter.PrimitiveStaticTypeAnyStruct, + interpreter.PrimitiveStaticTypeAnyStruct, + ), + interpreter.NewUnmeteredStringValue("key1"), + interpreter.NewDictionaryValue( + inter, + locationRange, + interpreter.NewDictionaryStaticType( + nil, + interpreter.PrimitiveStaticTypeAnyStruct, + interpreter.PrimitiveStaticTypeAnyStruct, + ), + interpreter.NewUnmeteredStringValue("key2"), + interpreter.NewUnmeteredIntValueFromInt64(1234), + ), + ) + + utils.AssertValuesEqual(t, inter, expected, actual) + }) + + t.Run("nested arrays", func(t *testing.T) { + t.Parallel() + + locationRange := interpreter.EmptyLocationRange + + ledger := NewTestLedger(nil, nil) + storage := runtime.NewStorage(ledger, nil) + + inter, err := interpreter.NewInterpreter( + nil, + utils.TestLocation, + &interpreter.Config{ + Storage: storage, + AtreeValueValidationEnabled: true, + // NOTE: disabled, as storage is not expected to be always valid _during_ migration + AtreeStorageValidationEnabled: false, + }, + ) + require.NoError(t, err) + + // [[String]] + + storedValue := interpreter.NewArrayValue( + inter, + locationRange, + interpreter.NewVariableSizedStaticType( + nil, + interpreter.NewVariableSizedStaticType( + nil, + interpreter.PrimitiveStaticTypeString, + ), + ), + common.ZeroAddress, + interpreter.NewArrayValue( + inter, + locationRange, + interpreter.NewVariableSizedStaticType( + nil, + interpreter.PrimitiveStaticTypeString, + ), + common.ZeroAddress, + interpreter.NewUnmeteredStringValue("abc"), + ), + ) + + actual := migrate(t, + storage, + inter, + storedValue, + ) + + // [AnyStruct] with [AnyStruct] + + expected := interpreter.NewArrayValue( + inter, + locationRange, + interpreter.NewVariableSizedStaticType( + nil, + interpreter.PrimitiveStaticTypeAnyStruct, + ), + common.ZeroAddress, + interpreter.NewArrayValue( + inter, + locationRange, + interpreter.NewVariableSizedStaticType( + nil, + interpreter.PrimitiveStaticTypeAnyStruct, + ), + common.ZeroAddress, + interpreter.NewUnmeteredStringValue("abc"), + ), + ) + + utils.AssertValuesEqual(t, inter, expected, actual) + }) + +} diff --git a/migrations/statictypes/statictype_migration_test.go b/migrations/statictypes/statictype_migration_test.go index 770bb3c2f8..061700295f 100644 --- a/migrations/statictypes/statictype_migration_test.go +++ b/migrations/statictypes/statictype_migration_test.go @@ -619,7 +619,7 @@ func TestMigratingNestedContainers(t *testing.T) { // Store values - storageMapKey := interpreter.StringStorageMapKey("test_type_value") + storageMapKey := interpreter.StringStorageMapKey("test_value") storageDomain := common.PathDomainStorage.Identifier() value = value.Transfer( From 40f8e687879ff567bdec22aabccf76d07d5cf9f4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Tue, 5 Mar 2024 13:34:15 -0800 Subject: [PATCH 16/17] fix composite field migration: set member without transfer --- migrations/migration.go | 2 +- migrations/migration_test.go | 92 ++++++++++++++++++++++++++++++++++++ 2 files changed, 93 insertions(+), 1 deletion(-) diff --git a/migrations/migration.go b/migrations/migration.go index a076008119..d7845dcd7b 100644 --- a/migrations/migration.go +++ b/migrations/migration.go @@ -252,7 +252,7 @@ func (m *StorageMigration) MigrateNestedValue( continue } - composite.SetMember( + composite.SetMemberWithoutTransfer( m.interpreter, emptyLocationRange, fieldName, diff --git a/migrations/migration_test.go b/migrations/migration_test.go index fdc768ee7d..2d7776fb9a 100644 --- a/migrations/migration_test.go +++ b/migrations/migration_test.go @@ -1204,6 +1204,19 @@ func (testContainerMigration) Migrate( ) return value.NewWithType(inter, emptyLocationRange, newType), nil + + case *interpreter.CompositeValue: + if value.QualifiedIdentifier == "Inner" { + return interpreter.NewCompositeValue( + inter, + emptyLocationRange, + utils.TestLocation, + "Inner2", + common.CompositeKindStructure, + nil, + value.GetOwner(), + ), nil + } } return nil, nil @@ -1444,4 +1457,83 @@ func TestMigratingNestedContainers(t *testing.T) { utils.AssertValuesEqual(t, inter, expected, actual) }) + t.Run("nested composite", func(t *testing.T) { + t.Parallel() + + locationRange := interpreter.EmptyLocationRange + + ledger := NewTestLedger(nil, nil) + storage := runtime.NewStorage(ledger, nil) + + inter, err := interpreter.NewInterpreter( + nil, + utils.TestLocation, + &interpreter.Config{ + Storage: storage, + AtreeValueValidationEnabled: true, + // NOTE: disabled, as storage is not expected to be always valid _during_ migration + AtreeStorageValidationEnabled: false, + }, + ) + require.NoError(t, err) + + // Outer(Inner()) + + storedValue := interpreter.NewCompositeValue( + inter, + locationRange, + utils.TestLocation, + "Outer", + common.CompositeKindStructure, + []interpreter.CompositeField{ + { + Name: "inner", + Value: interpreter.NewCompositeValue( + inter, + locationRange, + utils.TestLocation, + "Inner", + common.CompositeKindStructure, + nil, + common.ZeroAddress, + ), + }, + }, + common.ZeroAddress, + ) + + actual := migrate(t, + storage, + inter, + storedValue, + ) + + // Outer(Inner2()) + + expected := interpreter.NewCompositeValue( + inter, + locationRange, + utils.TestLocation, + "Outer", + common.CompositeKindStructure, + []interpreter.CompositeField{ + { + Name: "inner", + Value: interpreter.NewCompositeValue( + inter, + locationRange, + utils.TestLocation, + "Inner2", + common.CompositeKindStructure, + nil, + common.ZeroAddress, + ), + }, + }, + common.ZeroAddress, + ) + + utils.AssertValuesEqual(t, inter, expected, actual) + }) + } From d855aeedc38d6056af6bd2728d36114c3bcbbaaa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Tue, 5 Mar 2024 16:22:53 -0800 Subject: [PATCH 17/17] only remove existing value if there is a new value --- migrations/migration.go | 20 ++++---- migrations/migration_test.go | 98 ++++++++++++++++++++++++++++++++++-- 2 files changed, 104 insertions(+), 14 deletions(-) diff --git a/migrations/migration.go b/migrations/migration.go index d7845dcd7b..1403d83ae6 100644 --- a/migrations/migration.go +++ b/migrations/migration.go @@ -321,13 +321,6 @@ func (m *StorageMigration) MigrateNestedValue( keyToSet = newKey } - if newValue == nil { - valueToSet = existingValue - } else { - // Value was migrated - valueToSet = newValue - } - existingKey = legacyKey(existingKey) existingKeyStorable, existingValueStorable := dictionary.RemoveWithoutTransfer( m.interpreter, @@ -342,9 +335,16 @@ func (m *StorageMigration) MigrateNestedValue( )) } - interpreter.StoredValue(m.interpreter, existingValueStorable, m.storage). - DeepRemove(m.interpreter) - m.interpreter.RemoveReferencedSlab(existingValueStorable) + if newValue == nil { + valueToSet = existingValue + } else { + // Value was migrated + valueToSet = newValue + + interpreter.StoredValue(m.interpreter, existingValueStorable, m.storage). + DeepRemove(m.interpreter) + m.interpreter.RemoveReferencedSlab(existingValueStorable) + } dictionary.InsertWithoutTransfer( m.interpreter, diff --git a/migrations/migration_test.go b/migrations/migration_test.go index 2d7776fb9a..e7aac10c7f 100644 --- a/migrations/migration_test.go +++ b/migrations/migration_test.go @@ -1229,6 +1229,7 @@ func TestMigratingNestedContainers(t *testing.T) { migrate := func( t *testing.T, + valueMigration ValueMigration, storage *runtime.Storage, inter *interpreter.Interpreter, value interpreter.Value, @@ -1272,7 +1273,7 @@ func TestMigratingNestedContainers(t *testing.T) { }, migration.NewValueMigrationsPathMigrator( reporter, - testContainerMigration{}, + valueMigration, ), ) @@ -1300,7 +1301,7 @@ func TestMigratingNestedContainers(t *testing.T) { return result } - t.Run("nested dictionary", func(t *testing.T) { + t.Run("nested dictionary, value migrated", func(t *testing.T) { t.Parallel() locationRange := interpreter.EmptyLocationRange @@ -1320,7 +1321,7 @@ func TestMigratingNestedContainers(t *testing.T) { ) require.NoError(t, err) - // {String: {String: Int}} + // {"key1": {"key2": 1234}}: {String: {String: Int}} storedValue := interpreter.NewDictionaryValue( inter, @@ -1349,6 +1350,7 @@ func TestMigratingNestedContainers(t *testing.T) { ) actual := migrate(t, + testContainerMigration{}, storage, inter, storedValue, @@ -1381,6 +1383,92 @@ func TestMigratingNestedContainers(t *testing.T) { utils.AssertValuesEqual(t, inter, expected, actual) }) + t.Run("nested dictionary, key migrated", func(t *testing.T) { + t.Parallel() + + locationRange := interpreter.EmptyLocationRange + + ledger := NewTestLedger(nil, nil) + storage := runtime.NewStorage(ledger, nil) + + inter, err := interpreter.NewInterpreter( + nil, + utils.TestLocation, + &interpreter.Config{ + Storage: storage, + AtreeValueValidationEnabled: true, + // NOTE: disabled, as storage is not expected to be always valid _during_ migration + AtreeStorageValidationEnabled: false, + }, + ) + require.NoError(t, err) + + // {"key1": {"key2": 1234}}: {String: {String: Int}} + + storedValue := interpreter.NewDictionaryValue( + inter, + locationRange, + interpreter.NewDictionaryStaticType( + nil, + interpreter.PrimitiveStaticTypeString, + interpreter.NewDictionaryStaticType( + nil, + interpreter.PrimitiveStaticTypeString, + interpreter.PrimitiveStaticTypeInt, + ), + ), + interpreter.NewUnmeteredStringValue("key1"), + interpreter.NewDictionaryValue( + inter, + locationRange, + interpreter.NewDictionaryStaticType( + nil, + interpreter.PrimitiveStaticTypeString, + interpreter.PrimitiveStaticTypeInt, + ), + interpreter.NewUnmeteredStringValue("key2"), + interpreter.NewUnmeteredIntValueFromInt64(1234), + ), + ) + + actual := migrate(t, + testStringMigration{}, + storage, + inter, + storedValue, + ) + + // {"updated_key1": {"updated_key2": 1234}}: {String: {String: Int}} + + expected := interpreter.NewDictionaryValue( + inter, + locationRange, + interpreter.NewDictionaryStaticType( + nil, + interpreter.PrimitiveStaticTypeString, + interpreter.NewDictionaryStaticType( + nil, + interpreter.PrimitiveStaticTypeString, + interpreter.PrimitiveStaticTypeInt, + ), + ), + interpreter.NewUnmeteredStringValue("updated_key1"), + interpreter.NewDictionaryValue( + inter, + locationRange, + interpreter.NewDictionaryStaticType( + nil, + interpreter.PrimitiveStaticTypeString, + interpreter.PrimitiveStaticTypeInt, + ), + interpreter.NewUnmeteredStringValue("updated_key2"), + interpreter.NewUnmeteredIntValueFromInt64(1234), + ), + ) + + utils.AssertValuesEqual(t, inter, expected, actual) + }) + t.Run("nested arrays", func(t *testing.T) { t.Parallel() @@ -1401,7 +1489,7 @@ func TestMigratingNestedContainers(t *testing.T) { ) require.NoError(t, err) - // [[String]] + // [["abc"]]: [[String]] storedValue := interpreter.NewArrayValue( inter, @@ -1427,6 +1515,7 @@ func TestMigratingNestedContainers(t *testing.T) { ) actual := migrate(t, + testContainerMigration{}, storage, inter, storedValue, @@ -1503,6 +1592,7 @@ func TestMigratingNestedContainers(t *testing.T) { ) actual := migrate(t, + testContainerMigration{}, storage, inter, storedValue,