Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Improve migration #3110

Merged
merged 7 commits into from
Feb 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions migrations/capcons/capabilitymigration.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,11 @@
break
}

newBorrowType, ok := oldCapability.BorrowType.(*interpreter.ReferenceStaticType)
oldBorrowType := oldCapability.BorrowType

newBorrowType, ok := oldBorrowType.(*interpreter.ReferenceStaticType)
if !ok {
panic(errors.NewUnreachableError())
panic(errors.NewUnexpectedError("unexpected non-reference borrow type: %T", oldBorrowType))

Check warning on line 93 in migrations/capcons/capabilitymigration.go

View check run for this annotation

Codecov / codecov/patch

migrations/capcons/capabilitymigration.go#L93

Added line #L93 was not covered by tests
}

// Convert the old AuthAccount type to the new fully-entitled Account type
Expand Down
24 changes: 15 additions & 9 deletions migrations/capcons/linkmigration.go
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,10 @@

case interpreter.PathLinkValue: //nolint:staticcheck
var ok bool
borrowStaticType, ok = readValue.Type.(*interpreter.ReferenceStaticType)
borrowType := readValue.Type
borrowStaticType, ok = borrowType.(*interpreter.ReferenceStaticType)
if !ok {
panic(errors.NewUnreachableError())
panic(errors.NewUnexpectedError("unexpected non-reference borrow type: %T", borrowType))

Check warning on line 99 in migrations/capcons/linkmigration.go

View check run for this annotation

Codecov / codecov/patch

migrations/capcons/linkmigration.go#L99

Added line #L99 was not covered by tests
}

case interpreter.AccountLinkValue: //nolint:staticcheck
Expand All @@ -106,12 +107,13 @@
)

default:
panic(errors.NewUnreachableError())
panic(errors.NewUnexpectedError("unexpected value type: %T", value))

Check warning on line 110 in migrations/capcons/linkmigration.go

View check run for this annotation

Codecov / codecov/patch

migrations/capcons/linkmigration.go#L110

Added line #L110 was not covered by tests
}

borrowType, ok := inter.MustConvertStaticToSemaType(borrowStaticType).(*sema.ReferenceType)
convertedBorrowStaticType := inter.MustConvertStaticToSemaType(borrowStaticType)
borrowType, ok := convertedBorrowStaticType.(*sema.ReferenceType)
if !ok {
panic(errors.NewUnreachableError())
panic(errors.NewUnexpectedError("unexpected non-reference borrow type: %T", borrowType))

Check warning on line 116 in migrations/capcons/linkmigration.go

View check run for this annotation

Codecov / codecov/patch

migrations/capcons/linkmigration.go#L116

Added line #L116 was not covered by tests
}

// Get target
Expand Down Expand Up @@ -176,7 +178,7 @@
)

default:
panic(errors.NewUnreachableError())
panic(errors.NewUnexpectedError("unexpected target type: %T", target))

Check warning on line 181 in migrations/capcons/linkmigration.go

View check run for this annotation

Codecov / codecov/patch

migrations/capcons/linkmigration.go#L181

Added line #L181 was not covered by tests
}

// Record new capability ID in source path mapping.
Expand Down Expand Up @@ -310,9 +312,13 @@
// so it's possible that a capability value is encountered when determining the final target,
// when a part of the full link chain was already previously migrated.

capabilityBorrowType, ok := inter.MustConvertStaticToSemaType(value.BorrowType).(*sema.ReferenceType)
convertedBorrowType := inter.MustConvertStaticToSemaType(value.BorrowType)
capabilityBorrowType, ok := convertedBorrowType.(*sema.ReferenceType)
if !ok {
panic(errors.NewUnreachableError())
panic(errors.NewUnexpectedError(
"unexpected non-reference borrow type: %T",
convertedBorrowType,
))

Check warning on line 321 in migrations/capcons/linkmigration.go

View check run for this annotation

Codecov / codecov/patch

migrations/capcons/linkmigration.go#L318-L321

Added lines #L318 - L321 were not covered by tests
}

// Do not borrow final target (i.e. do not require target to exist),
Expand Down Expand Up @@ -349,7 +355,7 @@
}

default:
panic(errors.NewUnreachableError())
panic(errors.NewUnexpectedError("unexpected value type: %T", value))

Check warning on line 358 in migrations/capcons/linkmigration.go

View check run for this annotation

Codecov / codecov/patch

migrations/capcons/linkmigration.go#L358

Added line #L358 was not covered by tests
}
}
}
Expand Down
35 changes: 34 additions & 1 deletion migrations/entitlements/migration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -909,6 +909,11 @@ func TestConvertToEntitledValue(t *testing.T) {
),
Name: "[Type]",
},
{
Input: interpreter.NewTypeValue(inter, nil),
Output: interpreter.NewTypeValue(inter, nil),
Name: "Type(nil)",
},
{
Input: interpreter.NewDictionaryValue(
inter,
Expand Down Expand Up @@ -1041,6 +1046,19 @@ func TestConvertToEntitledValue(t *testing.T) {
},
Name: "PathCapability<&S>",
},
{
Input: &interpreter.PathCapabilityValue{ //nolint:staticcheck
Address: interpreter.NewAddressValue(inter, testAddress),
Path: testPathValue,
BorrowType: nil,
},
Output: &interpreter.PathCapabilityValue{ //nolint:staticcheck
Address: interpreter.NewAddressValue(inter, testAddress),
Path: testPathValue,
BorrowType: nil,
},
Name: "PathCapability<nil>",
},
{
Input: interpreter.NewCapabilityValue(
inter,
Expand Down Expand Up @@ -1276,6 +1294,21 @@ func TestConvertToEntitledValue(t *testing.T) {
}
return true

case interpreter.TypeValue:
// TypeValue considers missing type "unknown"/"invalid",
// and "unknown"/"invalid" type values unequal.
// However, we want to consider those equal here for testing/asserting purposes
other, ok := output.(interpreter.TypeValue)
if !ok {
return false
}

if other.Type == nil {
return v.Type == nil
} else {
return other.Type.Equal(v.Type)
}

case *interpreter.EphemeralReferenceValue:
otherReference, ok := output.(*interpreter.EphemeralReferenceValue)
if !ok || !v.Authorization.Equal(otherReference.Authorization) {
Expand Down Expand Up @@ -1304,7 +1337,7 @@ func TestConvertToEntitledValue(t *testing.T) {
convertedValue := convertEntireTestValue(inter, storage, testAddress, test.Input)
switch convertedValue := convertedValue.(type) {
case interpreter.EquatableValue:
require.True(t, referencePeekingEqual(convertedValue, test.Output))
require.True(t, referencePeekingEqual(convertedValue, test.Output), "expected: %s\nactual: %s", test.Output, convertedValue)
default:
require.Equal(t, convertedValue, test.Output)
}
Expand Down
7 changes: 4 additions & 3 deletions migrations/legacy_primitivestatic_type.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@
var _ interpreter.StaticType = LegacyPrimitiveStaticType{}

func (t LegacyPrimitiveStaticType) ID() common.TypeID {
switch t.PrimitiveStaticType {
primitiveStaticType := t.PrimitiveStaticType

switch primitiveStaticType {
case interpreter.PrimitiveStaticTypeAuthAccount: //nolint:staticcheck
return "AuthAccount"
case interpreter.PrimitiveStaticTypePublicAccount: //nolint:staticcheck
Expand All @@ -59,7 +61,6 @@
case interpreter.PrimitiveStaticTypeAccountKey: //nolint:staticcheck
return "AccountKey"
default:
panic(errors.NewUnreachableError())

panic(errors.NewUnexpectedError("unexpected non-legacy primitive static type: %s", primitiveStaticType))

Check warning on line 64 in migrations/legacy_primitivestatic_type.go

View check run for this annotation

Codecov / codecov/patch

migrations/legacy_primitivestatic_type.go#L64

Added line #L64 was not covered by tests
}
}
15 changes: 12 additions & 3 deletions migrations/migration.go
Original file line number Diff line number Diff line change
Expand Up @@ -306,7 +306,10 @@
)

if _, ok := oldValue.(*interpreter.SomeValue); !ok {
panic(errors.NewUnreachableError())
panic(errors.NewUnexpectedError(
"failed to remove old value for migrated key: %s",
existingKey,
))

Check warning on line 312 in migrations/migration.go

View check run for this annotation

Codecov / codecov/patch

migrations/migration.go#L309-L312

Added lines #L309 - L312 were not covered by tests
}

keyToSet = newKey
Expand Down Expand Up @@ -372,8 +375,14 @@
// is the same as the owner of the old value
if ownedValue, ok := value.(interpreter.OwnedValue); ok {
if ownedConvertedValue, ok := convertedValue.(interpreter.OwnedValue); ok {
if ownedConvertedValue.GetOwner() != ownedValue.GetOwner() {
panic(errors.NewUnreachableError())
convertedOwner := ownedConvertedValue.GetOwner()
originalOwner := ownedValue.GetOwner()
if convertedOwner != originalOwner {
panic(errors.NewUnexpectedError(
"migrated value has different owner: expected %s, got %s",
originalOwner,
convertedOwner,
))

Check warning on line 385 in migrations/migration.go

View check run for this annotation

Codecov / codecov/patch

migrations/migration.go#L381-L385

Added lines #L381 - L385 were not covered by tests
}
}
}
Expand Down
8 changes: 7 additions & 1 deletion migrations/statictypes/account_type_migration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ type testReporter struct {
interpreter.StorageKey
interpreter.StorageMapKey
}]struct{}
errors []error
}

func newTestReporter() *testReporter {
Expand Down Expand Up @@ -72,8 +73,9 @@ func (t *testReporter) Error(
_ interpreter.StorageKey,
_ interpreter.StorageMapKey,
_ string,
_ error,
err error,
) {
t.errors = append(t.errors, err)
}

func TestAccountTypeInTypeValueMigration(t *testing.T) {
Expand Down Expand Up @@ -424,6 +426,8 @@ func TestAccountTypeInTypeValueMigration(t *testing.T) {
err = migration.Commit()
require.NoError(t, err)

require.Empty(t, reporter.errors)

// Check reported migrated paths
for identifier, test := range testCases {
key := struct {
Expand Down Expand Up @@ -1080,6 +1084,8 @@ func TestAccountTypeRehash(t *testing.T) {
err := migration.Commit()
require.NoError(t, err)

require.Empty(t, reporter.errors)

require.Equal(t,
map[struct {
interpreter.StorageKey
Expand Down
2 changes: 2 additions & 0 deletions migrations/statictypes/composite_type_migration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,8 @@ func TestCompositeAndInterfaceTypeMigration(t *testing.T) {
err = migration.Commit()
require.NoError(t, err)

require.Empty(t, reporter.errors)

// Check reported migrated paths
for identifier, test := range testCases {
key := struct {
Expand Down
12 changes: 12 additions & 0 deletions migrations/statictypes/intersection_type_migration_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -443,6 +443,8 @@ func TestIntersectionTypeMigration(t *testing.T) {
err = migration.Commit()
require.NoError(t, err)

require.Empty(t, reporter.errors)

// Check reported migrated paths
for identifier, test := range testCases {
key := struct {
Expand Down Expand Up @@ -611,6 +613,8 @@ func TestIntersectionTypeRehash(t *testing.T) {
err := migration.Commit()
require.NoError(t, err)

require.Empty(t, reporter.errors)

require.Equal(t,
map[struct {
interpreter.StorageKey
Expand Down Expand Up @@ -775,6 +779,8 @@ func TestRehashNestedIntersectionType(t *testing.T) {
err := migration.Commit()
require.NoError(t, err)

require.Empty(t, reporter.errors)

require.Equal(t,
map[struct {
interpreter.StorageKey
Expand Down Expand Up @@ -914,6 +920,8 @@ func TestRehashNestedIntersectionType(t *testing.T) {
err := migration.Commit()
require.NoError(t, err)

require.Empty(t, reporter.errors)

require.Equal(t,
map[struct {
interpreter.StorageKey
Expand Down Expand Up @@ -1114,6 +1122,8 @@ func TestIntersectionTypeMigrationWithInterfaceTypeConverter(t *testing.T) {
err = migration.Commit()
require.NoError(t, err)

require.Empty(t, reporter.errors)

expectLegacyTypeConverted := convertCompositeType && legacyType != nil
expectInterfaceTypeConverted := convertInterfaceType && len(interfaceTypeQualifiedIdentifiers) > 0
expectMigration := len(interfaceTypeQualifiedIdentifiers) >= 2 ||
Expand Down Expand Up @@ -1282,6 +1292,8 @@ func TestIntersectionTypeMigrationWithTypeConverters(t *testing.T) {
err = migration.Commit()
require.NoError(t, err)

require.Empty(t, reporter.errors)

key := struct {
interpreter.StorageKey
interpreter.StorageMapKey
Expand Down
16 changes: 13 additions & 3 deletions migrations/statictypes/statictype_migration.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,12 @@
) (newValue interpreter.Value, err error) {
switch value := value.(type) {
case interpreter.TypeValue:
convertedType := m.maybeConvertStaticType(value.Type, nil)
// Type is optional. nil represents "unknown"/"invalid" type
ty := value.Type
if ty == nil {
return
}
convertedType := m.maybeConvertStaticType(ty, nil)
if convertedType == nil {
return
}
Expand All @@ -80,7 +85,12 @@
return interpreter.NewUnmeteredCapabilityValue(value.ID, value.Address, convertedBorrowType), nil

case *interpreter.PathCapabilityValue: //nolint:staticcheck
convertedBorrowType := m.maybeConvertStaticType(value.BorrowType, nil)
// Type is optional
borrowType := value.BorrowType
if borrowType == nil {
return
}
convertedBorrowType := m.maybeConvertStaticType(borrowType, nil)
if convertedBorrowType == nil {
return
}
Expand Down Expand Up @@ -345,7 +355,7 @@
}

default:
panic(errors.NewUnreachableError())
panic(errors.NewUnexpectedError("unexpected static type: %T", staticType))

Check warning on line 358 in migrations/statictypes/statictype_migration.go

View check run for this annotation

Codecov / codecov/patch

migrations/statictypes/statictype_migration.go#L358

Added line #L358 was not covered by tests
}

return nil
Expand Down
Loading
Loading