Skip to content
New issue

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

Sign up for GitHub

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

Already on GitHub? Sign in to your account

Various improvements #2979

Merged
merged 5 commits into from
Dec 15, 2023
Merged

Various improvements #2979

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
674e15d
make supported types explicit
turbolent Dec 15, 2023
fc69d49
FunctionStaticType.ReturnType only needs a memory gauge, not a full i…
turbolent Dec 15, 2023
501d06c
report useful errors when a program logs or emits events, but handler…
turbolent Dec 15, 2023
e6fc3f5
make it explicit that migration is not expected
turbolent Dec 15, 2023
f5c41c9
fix qualified identifiers
turbolent Dec 15, 2023
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
20 changes: 14 additions & 6 deletions migrations/account_type/migration.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import (
"github.com/onflow/cadence/migrations"
"github.com/onflow/cadence/runtime/common"
"github.com/onflow/cadence/runtime/errors"
"github.com/onflow/cadence/runtime/interpreter"
"github.com/onflow/cadence/runtime/sema"
)
Expand Down Expand Up @@ -112,11 +113,15 @@

case *interpreter.IntersectionStaticType:
// No need to convert `staticType.Types` as they can only be interfaces.
convertedLegacyType := maybeConvertAccountType(staticType.LegacyType)
if convertedLegacyType != nil {
intersectionType := interpreter.NewIntersectionStaticType(nil, staticType.Types)
intersectionType.LegacyType = convertedLegacyType
return intersectionType

legacyType := staticType.LegacyType
if legacyType != nil {
convertedLegacyType := maybeConvertAccountType(legacyType)
if convertedLegacyType != nil {
intersectionType := interpreter.NewIntersectionStaticType(nil, staticType.Types)
intersectionType.LegacyType = convertedLegacyType
return intersectionType
}
}

case *interpreter.OptionalStaticType:
Expand Down Expand Up @@ -155,7 +160,7 @@
// Ignore the wrapper, and continue with the inner type.
return maybeConvertAccountType(staticType.PrimitiveStaticType)

default:
case interpreter.PrimitiveStaticType:
// Is it safe to do so?
switch staticType {
case interpreter.PrimitiveStaticTypePublicAccount: //nolint:staticcheck
Expand Down Expand Up @@ -187,6 +192,9 @@
case interpreter.PrimitiveStaticTypeAccountKey: //nolint:staticcheck
return interpreter.AccountKeyStaticType
}

default:
panic(errors.NewUnreachableError())

Check warning on line 197 in migrations/account_type/migration.go

View check run for this annotation

Codecov / codecov/patch

migrations/account_type/migration.go#L196-L197 

Added lines #L196 - L197 were not covered by tests
}

return nil
Expand Down
47 changes: 31 additions & 16 deletions migrations/account_type/migration_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,9 @@ func TestTypeValueMigration(t *testing.T) {
const authAccountType = interpreter.PrimitiveStaticTypeAuthAccount //nolint:staticcheck
const stringType = interpreter.PrimitiveStaticTypeString

const fooBarQualifiedIdentifier = "Foo.Bar"
fooAddressLocation := common.NewAddressLocation(nil, account, "Foo")

type testCase struct {
storedType interpreter.StaticType
expectedType interpreter.StaticType
Expand Down Expand Up @@ -137,21 +140,24 @@ func TestTypeValueMigration(t *testing.T) {
expectedType: interpreter.NewOptionalStaticType(nil, unauthorizedAccountReferenceType),
},
"optional_string": {
storedType: interpreter.NewOptionalStaticType(nil, stringType),
storedType: interpreter.NewOptionalStaticType(nil, stringType),
expectedType: nil,
},
"constant_sized_account_array": {
storedType: interpreter.NewConstantSizedStaticType(nil, publicAccountType, 3),
expectedType: interpreter.NewConstantSizedStaticType(nil, unauthorizedAccountReferenceType, 3),
},
"constant_sized_string_array": {
storedType: interpreter.NewConstantSizedStaticType(nil, stringType, 3),
storedType: interpreter.NewConstantSizedStaticType(nil, stringType, 3),
expectedType: nil,
},
"variable_sized_account_array": {
storedType: interpreter.NewVariableSizedStaticType(nil, authAccountType),
expectedType: interpreter.NewVariableSizedStaticType(nil, authAccountReferenceType),
},
"variable_sized_string_array": {
storedType: interpreter.NewVariableSizedStaticType(nil, stringType),
storedType: interpreter.NewVariableSizedStaticType(nil, stringType),
expectedType: nil,
},
"dictionary_with_account_type_value": {
storedType: interpreter.NewDictionaryStaticType(
Expand Down Expand Up @@ -195,6 +201,7 @@ func TestTypeValueMigration(t *testing.T) {
stringType,
stringType,
),
expectedType: nil,
},
"capability": {
storedType: interpreter.NewCapabilityStaticType(
Expand All @@ -211,6 +218,7 @@ func TestTypeValueMigration(t *testing.T) {
nil,
stringType,
),
expectedType: nil,
},
"intersection": {
storedType: interpreter.NewIntersectionStaticType(
Expand All @@ -219,21 +227,23 @@ func TestTypeValueMigration(t *testing.T) {
interpreter.NewInterfaceStaticType(
nil,
nil,
"Bar",
fooBarQualifiedIdentifier,
common.NewTypeIDFromQualifiedName(
nil,
common.NewAddressLocation(nil, account, "Foo"),
"Bar",
fooAddressLocation,
fooBarQualifiedIdentifier,
),
),
},
),
expectedType: nil,
},
"empty intersection": {
storedType: interpreter.NewIntersectionStaticType(
nil,
[]*interpreter.InterfaceStaticType{},
),
expectedType: nil,
},
"intersection_with_legacy_type": {
storedType: &interpreter.IntersectionStaticType{
Expand Down Expand Up @@ -300,25 +310,27 @@ func TestTypeValueMigration(t *testing.T) {
storedType: interpreter.NewInterfaceStaticType(
nil,
nil,
"Bar",
fooBarQualifiedIdentifier,
common.NewTypeIDFromQualifiedName(
nil,
common.NewAddressLocation(nil, account, "Foo"),
"Bar",
fooAddressLocation,
fooBarQualifiedIdentifier,
),
),
expectedType: nil,
},
"composite": {
storedType: interpreter.NewCompositeStaticType(
nil,
nil,
"Bar",
fooBarQualifiedIdentifier,
common.NewTypeIDFromQualifiedName(
nil,
common.NewAddressLocation(nil, account, "Foo"),
"Bar",
fooAddressLocation,
fooBarQualifiedIdentifier,
),
),
expectedType: nil,
},
}

Expand Down Expand Up @@ -466,6 +478,9 @@ func TestNestedTypeValueMigration(t *testing.T) {
)
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),
Expand Down Expand Up @@ -623,8 +638,8 @@ func TestNestedTypeValueMigration(t *testing.T) {
storedValue: interpreter.NewCompositeValue(
inter,
interpreter.EmptyLocationRange,
common.NewAddressLocation(nil, common.Address{0x42}, "Foo"),
"Bar",
fooAddressLocation,
fooBarQualifiedIdentifier,
common.CompositeKindResource,
[]interpreter.CompositeField{
interpreter.NewUnmeteredCompositeField("field1", storedAccountTypeValue),
Expand All @@ -635,8 +650,8 @@ func TestNestedTypeValueMigration(t *testing.T) {
expectedValue: interpreter.NewCompositeValue(
inter,
interpreter.EmptyLocationRange,
common.NewAddressLocation(nil, common.Address{0x42}, "Foo"),
"Bar",
fooAddressLocation,
fooBarQualifiedIdentifier,
common.CompositeKindResource,
[]interpreter.CompositeField{
interpreter.NewUnmeteredCompositeField("field1", expectedAccountTypeValue),
Expand Down
4 changes: 2 additions & 2 deletions runtime/interpreter/statictype.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1218,10 +1218,10 @@ func NewFunctionStaticType(
}
}

func (t FunctionStaticType) ReturnType(interpreter *Interpreter) StaticType {
func (t FunctionStaticType) ReturnType(gauge common.MemoryGauge) StaticType {
var returnType StaticType
if t.Type.ReturnTypeAnnotation.Type != nil {
returnType = ConvertSemaToStaticType(interpreter, t.Type.ReturnTypeAnnotation.Type)
returnType = ConvertSemaToStaticType(gauge, t.Type.ReturnTypeAnnotation.Type)
}

return returnType
Expand Down
6 changes: 6 additions & 0 deletions runtime/tests/runtime_utils/testinterface.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -317,11 +317,17 @@ func (i *TestRuntimeInterface) GetSigningAccounts() ([]runtime.Address, error) {
}

func (i *TestRuntimeInterface) ProgramLog(message string) error {
if i.OnProgramLog == nil {
panic("must specify TestRuntimeInterface.OnProgramLog")
}
i.OnProgramLog(message)
return nil
}

func (i *TestRuntimeInterface) EmitEvent(event cadence.Event) error {
if i.OnEmitEvent == nil {
panic("must specify TestRuntimeInterface.OnEmitEvent")
}
return i.OnEmitEvent(event)
}

Expand Down
Loading