From 487e31ac299c2d1adcee282f8018db42dd42c1e5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Wed, 9 Oct 2024 14:56:27 -0700 Subject: [PATCH 01/14] prevent predeclared members to get confused using user-defined types --- runtime/ast/expression.go | 2 +- runtime/runtime_test.go | 67 +++++++++++++++ runtime/sema/check_composite_declaration.go | 94 ++++++++++++++++----- runtime/tests/checker/attachments_test.go | 23 ++++- runtime/tests/checker/composite_test.go | 52 ++++++++++++ runtime/tests/checker/contract_test.go | 30 +++++++ runtime/tests/checker/nesting_test.go | 3 +- 7 files changed, 246 insertions(+), 25 deletions(-) diff --git a/runtime/ast/expression.go b/runtime/ast/expression.go index 0142b92b20..deed882354 100644 --- a/runtime/ast/expression.go +++ b/runtime/ast/expression.go @@ -1416,7 +1416,7 @@ func FunctionDocument( } // NOTE: not all functions have a parameter list, - // e.g. the `destroy` special function + // e.g. the `init` (initializer, special function) if parameterList != nil { signatureDoc = append( diff --git a/runtime/runtime_test.go b/runtime/runtime_test.go index 7005274fed..2b57648779 100644 --- a/runtime/runtime_test.go +++ b/runtime/runtime_test.go @@ -11480,3 +11480,70 @@ func TestRuntimeStorageEnumAsDictionaryKey(t *testing.T) { loggedMessages, ) } + +func TestRuntimeBuiltInFunctionConfusion(t *testing.T) { + + t.Parallel() + + const contract = ` + access(all) contract Foo { + access(all) resource getType {} + + init() { + Foo.getType() + } + } + ` + + runtime := NewTestInterpreterRuntime() + + address := common.MustBytesToAddress([]byte{0x1}) + + accountCodes := map[common.AddressLocation][]byte{} + var events []cadence.Event + var loggedMessages []string + + runtimeInterface := &TestRuntimeInterface{ + Storage: NewTestLedger(nil, nil), + OnGetSigningAccounts: func() ([]common.Address, error) { + return []common.Address{address}, nil + }, + OnResolveLocation: NewSingleIdentifierLocationResolver(t), + OnUpdateAccountContractCode: func(location common.AddressLocation, code []byte) error { + accountCodes[location] = code + return nil + }, + OnGetAccountContractCode: func(location common.AddressLocation) (code []byte, err error) { + code = accountCodes[location] + return code, nil + }, + OnEmitEvent: func(event cadence.Event) error { + events = append(events, event) + return nil + }, + OnProgramLog: func(message string) { + loggedMessages = append(loggedMessages, message) + }, + } + + nextTransactionLocation := NewTransactionLocationGenerator() + + // Deploy contract + + err := runtime.ExecuteTransaction( + Script{ + Source: DeploymentTransaction( + "Foo", + []byte(contract), + ), + }, + Context{ + Interface: runtimeInterface, + Location: nextTransactionLocation(), + }, + ) + RequireError(t, err) + + var redeclarationError *sema.RedeclarationError + require.ErrorAs(t, err, &redeclarationError) +} diff --git a/runtime/sema/check_composite_declaration.go b/runtime/sema/check_composite_declaration.go index 5bc46ecd5e..84030e0224 100644 --- a/runtime/sema/check_composite_declaration.go +++ b/runtime/sema/check_composite_declaration.go @@ -1688,39 +1688,84 @@ func (checker *Checker) defaultMembersAndOrigins( } predeclaredMembers := checker.predeclaredMembers(containerType) - invalidIdentifiers := make(map[string]bool, len(predeclaredMembers)) - + predeclaredMemberNames := make(map[string]struct{}, len(predeclaredMembers)) for _, predeclaredMember := range predeclaredMembers { name := predeclaredMember.Identifier.Identifier - members.Set(name, predeclaredMember) - invalidIdentifiers[name] = true + predeclaredMemberNames[name] = struct{}{} + } - if predeclaredMember.DeclarationKind == common.DeclarationKindField { - fieldNames = append(fieldNames, name) + var nestedTypes *StringTypeOrderedMap + if containerType, ok := containerType.(ContainerType); ok { + nestedTypes = containerType.GetNestedTypes() + } + + checkRedeclaration := func( + identifier ast.Identifier, + declarationKind common.DeclarationKind, + isPredeclared bool, + ) bool { + name := identifier.Identifier + + if !isPredeclared { + if _, ok := predeclaredMemberNames[name]; ok { + checker.report( + &InvalidDeclarationError{ + Identifier: identifier.Identifier, + Kind: declarationKind, + Range: ast.NewRangeFromPositioned(checker.memoryGauge, identifier), + }, + ) + return false + } + } + + if nestedTypes != nil { + if _, ok := nestedTypes.Get(name); ok { + // TODO: provide previous position + checker.report( + &RedeclarationError{ + Name: name, + Kind: declarationKind, + Pos: identifier.Pos, + }, + ) + + return false + } } + + return true } - checkInvalidIdentifier := func(declaration ast.Declaration) bool { - identifier := declaration.DeclarationIdentifier() - if invalidIdentifiers == nil || !invalidIdentifiers[identifier.Identifier] { - return true + // declare all predeclared members (built-in functions and fields) + for _, predeclaredMember := range predeclaredMembers { + identifier := predeclaredMember.Identifier + name := identifier.Identifier + declarationKind := predeclaredMember.DeclarationKind + + if !checkRedeclaration( + identifier, + declarationKind, + true, + ) { + continue } - checker.report( - &InvalidDeclarationError{ - Identifier: identifier.Identifier, - Kind: declaration.DeclarationKind(), - Range: ast.NewRangeFromPositioned(checker.memoryGauge, identifier), - }, - ) + members.Set(name, predeclaredMember) - return false + if declarationKind == common.DeclarationKindField { + fieldNames = append(fieldNames, name) + } } // declare a member for each field for _, field := range fields { - if !checkInvalidIdentifier(field) { + if !checkRedeclaration( + field.Identifier, + field.DeclarationKind(), + false, + ) { continue } @@ -1792,7 +1837,12 @@ func (checker *Checker) defaultMembersAndOrigins( // declare a member for each function for _, function := range functions { - if !checkInvalidIdentifier(function) { + + if !checkRedeclaration( + function.Identifier, + function.DeclarationKind(), + false, + ) { continue } @@ -2406,7 +2456,7 @@ func (checker *Checker) declareBaseValue(fnAccess Access, baseType Type, attachm } // checkNestedIdentifiers checks that nested identifiers, i.e. fields, functions, -// and nested interfaces and composites, are unique and aren't named `init` or `destroy` +// and nested interfaces and composites, are unique and aren't named `init` func (checker *Checker) checkNestedIdentifiers(members *ast.Members) { positions := map[string]ast.Position{} @@ -2430,7 +2480,7 @@ func (checker *Checker) checkNestedIdentifiers(members *ast.Members) { } // checkNestedIdentifier checks that the nested identifier is unique -// and isn't named `init` or `destroy` +// and isn't named `init` func (checker *Checker) checkNestedIdentifier( identifier ast.Identifier, kind common.DeclarationKind, diff --git a/runtime/tests/checker/attachments_test.go b/runtime/tests/checker/attachments_test.go index 766e075e1d..ac8f324f01 100644 --- a/runtime/tests/checker/attachments_test.go +++ b/runtime/tests/checker/attachments_test.go @@ -4757,7 +4757,7 @@ func TestCheckAttachmentForEachAttachment(t *testing.T) { assert.IsType(t, &sema.NotDeclaredMemberError{}, errs[0]) }) - t.Run("cannot redeclare forEachAttachment", func(t *testing.T) { + t.Run("cannot redeclare forEachAttachment as function", func(t *testing.T) { t.Parallel() @@ -4773,6 +4773,27 @@ func TestCheckAttachmentForEachAttachment(t *testing.T) { assert.IsType(t, &sema.InvalidDeclarationError{}, errs[0]) }) + t.Run("cannot redeclare forEachAttachment as field", func(t *testing.T) { + + t.Parallel() + + _, err := ParseAndCheck(t, + ` + access(all) struct S { + let forEachAttachment: Int + + init() { + self.forEachAttachment = 1 + } + } + `, + ) + + errs := RequireCheckerErrors(t, err, 2) + assert.IsType(t, &sema.InvalidDeclarationError{}, errs[0]) + assert.IsType(t, &sema.TypeMismatchError{}, errs[1]) + }) + t.Run("downcasting reference with entitlements", func(t *testing.T) { t.Parallel() diff --git a/runtime/tests/checker/composite_test.go b/runtime/tests/checker/composite_test.go index fe4d0c5e5a..98f458db9d 100644 --- a/runtime/tests/checker/composite_test.go +++ b/runtime/tests/checker/composite_test.go @@ -2298,3 +2298,55 @@ func TestCheckKeywordsAsFieldNames(t *testing.T) { }) } } + +func TestCheckInvalidFunctionNestedTypeClash(t *testing.T) { + + t.Parallel() + interfacePossibilities := []bool{true, false} + + for _, kind := range common.CompositeKindsWithFieldsAndFunctions { + for _, isInterface := range interfacePossibilities { + + interfaceKeyword := "" + if isInterface { + interfaceKeyword = "interface" + } + + var baseType string + + switch kind { + case common.CompositeKindContract: + // Cannot nest contracts + continue + + case common.CompositeKindAttachment: + if isInterface { + continue + } + baseType = "for AnyStruct" + } + + testName := fmt.Sprintf("%s_%s", kind.Keyword(), interfaceKeyword) + + t.Run(testName, func(t *testing.T) { + + _, err := ParseAndCheck(t, + fmt.Sprintf( + ` + contract C { + %s %s getType %s {} + } + `, + kind.Keyword(), + interfaceKeyword, + baseType, + ), + ) + + errs := RequireCheckerErrors(t, err, 1) + + assert.IsType(t, &sema.RedeclarationError{}, errs[0]) + }) + } + } +} diff --git a/runtime/tests/checker/contract_test.go b/runtime/tests/checker/contract_test.go index 0a2edbcb0d..a3b055ca27 100644 --- a/runtime/tests/checker/contract_test.go +++ b/runtime/tests/checker/contract_test.go @@ -94,6 +94,36 @@ func TestCheckInvalidContractInterfaceAccountFunction(t *testing.T) { assert.IsType(t, &sema.InvalidDeclarationError{}, errs[0]) } +func TestCheckInvalidContractAccountType(t *testing.T) { + + t.Parallel() + + _, err := ParseAndCheck(t, ` + contract Test { + struct account {} + } + `) + + errs := RequireCheckerErrors(t, err, 1) + + assert.IsType(t, &sema.RedeclarationError{}, errs[0]) +} + +func TestCheckInvalidContractInterfaceAccountType(t *testing.T) { + + t.Parallel() + + _, err := ParseAndCheck(t, ` + contract interface Test { + struct interface account {} + } + `) + + errs := RequireCheckerErrors(t, err, 1) + + assert.IsType(t, &sema.RedeclarationError{}, errs[0]) +} + func TestCheckContractAccountFieldUse(t *testing.T) { t.Parallel() diff --git a/runtime/tests/checker/nesting_test.go b/runtime/tests/checker/nesting_test.go index fda0d70f50..6e09c18f57 100644 --- a/runtime/tests/checker/nesting_test.go +++ b/runtime/tests/checker/nesting_test.go @@ -258,9 +258,10 @@ func TestCheckInvalidCompositeDeclarationNestedDuplicateNames(t *testing.T) { } `) - errs := RequireCheckerErrors(t, err, 1) + errs := RequireCheckerErrors(t, err, 2) assert.IsType(t, &sema.RedeclarationError{}, errs[0]) + assert.IsType(t, &sema.RedeclarationError{}, errs[1]) } func TestCheckCompositeDeclarationNestedConstructorAndType(t *testing.T) { From 5f28ddaa6555cb0c7de0e053d6f71838903d0329 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Wed, 9 Oct 2024 22:30:08 -0700 Subject: [PATCH 02/14] forbid attachment conformances --- runtime/sema/check_composite_declaration.go | 26 +- runtime/sema/errors.go | 17 + runtime/tests/checker/attachments_test.go | 328 +------------------- runtime/tests/checker/entitlements_test.go | 110 ------- 4 files changed, 56 insertions(+), 425 deletions(-) diff --git a/runtime/sema/check_composite_declaration.go b/runtime/sema/check_composite_declaration.go index 84030e0224..cb2fac91bd 100644 --- a/runtime/sema/check_composite_declaration.go +++ b/runtime/sema/check_composite_declaration.go @@ -681,9 +681,31 @@ func (checker *Checker) declareCompositeType(declaration ast.CompositeLikeDeclar // Resolve conformances - if declaration.Kind() == common.CompositeKindEnum { + switch declaration.Kind() { + case common.CompositeKindEnum: compositeType.EnumRawType = checker.enumRawType(declaration.(*ast.CompositeDeclaration)) - } else { + + case common.CompositeKindAttachment: + // Attachments may not conform to interfaces + + conformanceList := declaration.ConformanceList() + conformanceCount := len(conformanceList) + if conformanceCount > 0 { + firstConformance := conformanceList[0] + lastConformance := conformanceList[conformanceCount-1] + + checker.report( + &InvalidAttachmentConformancesError{ + Range: ast.NewRange( + checker.memoryGauge, + firstConformance.StartPosition(), + lastConformance.EndPosition(checker.memoryGauge), + ), + }, + ) + } + + default: compositeType.ExplicitInterfaceConformances = checker.explicitInterfaceConformances(declaration, compositeType) } diff --git a/runtime/sema/errors.go b/runtime/sema/errors.go index 64c693802a..66efa2b57d 100644 --- a/runtime/sema/errors.go +++ b/runtime/sema/errors.go @@ -1440,6 +1440,23 @@ func (e *InvalidEnumConformancesError) Error() string { return "enums cannot conform to interfaces" } +// InvalidAttachmentConformancesError + +type InvalidAttachmentConformancesError struct { + ast.Range +} + +var _ SemanticError = &InvalidAttachmentConformancesError{} +var _ errors.UserError = &InvalidAttachmentConformancesError{} + +func (*InvalidAttachmentConformancesError) isSemanticError() {} + +func (*InvalidAttachmentConformancesError) IsUserError() {} + +func (e *InvalidAttachmentConformancesError) Error() string { + return "attachments cannot conform to interfaces" +} + // ConformanceError // TODO: report each missing member and mismatch as note diff --git a/runtime/tests/checker/attachments_test.go b/runtime/tests/checker/attachments_test.go index ac8f324f01..f6a3c75ae3 100644 --- a/runtime/tests/checker/attachments_test.go +++ b/runtime/tests/checker/attachments_test.go @@ -619,304 +619,42 @@ func TestCheckAttachmentWithMembers(t *testing.T) { }) } -func TestCheckAttachmentConformance(t *testing.T) { +func TestCheckInvalidAttachmentConformance(t *testing.T) { t.Parallel() - t.Run("basic", func(t *testing.T) { - - t.Parallel() - - _, err := ParseAndCheck(t, - ` - resource R {} - resource interface I { - } - attachment Test for R: I { - }`, - ) - - require.NoError(t, err) - }) - - t.Run("field", func(t *testing.T) { - - t.Parallel() - - _, err := ParseAndCheck(t, - ` - resource R {} - resource interface I { - let x: Int - } - attachment Test for R: I { - let x: Int - init(x: Int) { - self.x = x - } - }`, - ) - - require.NoError(t, err) - }) - - t.Run("method", func(t *testing.T) { - - t.Parallel() - - _, err := ParseAndCheck(t, - ` - resource R {} - resource interface I { - fun x(): Int - } - attachment Test for R: I { - fun x(): Int { return 0 } - }`, - ) - - require.NoError(t, err) - }) - - t.Run("field missing", func(t *testing.T) { - - t.Parallel() - - _, err := ParseAndCheck(t, - ` - resource R {} - resource interface I { - let x: Int - } - attachment Test for R: I { - }`, - ) - - errs := RequireCheckerErrors(t, err, 1) - - assert.IsType(t, &sema.ConformanceError{}, errs[0]) - }) - - t.Run("field type", func(t *testing.T) { - - t.Parallel() - - _, err := ParseAndCheck(t, - ` - resource R {} - resource interface I { - let x: Int - } - attachment Test for R: I { - let x: AnyStruct - init(x: AnyStruct) { - self.x = x - } - }`, - ) - - errs := RequireCheckerErrors(t, err, 1) - - assert.IsType(t, &sema.ConformanceError{}, errs[0]) - }) - - t.Run("method missing", func(t *testing.T) { - - t.Parallel() - - _, err := ParseAndCheck(t, - ` - resource R {} - resource interface I { - fun x(): Int - } - attachment Test for R: I { - }`, - ) - - errs := RequireCheckerErrors(t, err, 1) - - assert.IsType(t, &sema.ConformanceError{}, errs[0]) - }) - - t.Run("method type", func(t *testing.T) { + t.Run("struct", func(t *testing.T) { t.Parallel() - _, err := ParseAndCheck(t, - ` - resource R {} - resource interface I { - fun x(): Int - } - attachment Test for R: I { - fun x(): AnyStruct { return "" } - }`, - ) - - errs := RequireCheckerErrors(t, err, 1) - - assert.IsType(t, &sema.ConformanceError{}, errs[0]) - }) - - t.Run("method missing, exists in base type", func(t *testing.T) { + _, err := ParseAndCheck(t, ` + struct S {} - t.Parallel() + struct interface SI {} - _, err := ParseAndCheck(t, - ` - resource R { - fun x(): Int { return 3 } - } - resource interface I { - fun x(): Int - } - attachment Test for R: I { - }`, - ) + attachment Test for S: SI {} + `) errs := RequireCheckerErrors(t, err, 1) - assert.IsType(t, &sema.ConformanceError{}, errs[0]) + assert.IsType(t, &sema.InvalidAttachmentConformancesError{}, errs[0]) }) - t.Run("kind mismatch resource", func(t *testing.T) { + t.Run("resource", func(t *testing.T) { t.Parallel() - _, err := ParseAndCheck(t, - ` + _, err := ParseAndCheck(t, ` resource R {} - struct interface I {} - attachment Test for R: I {}`, - ) - - errs := RequireCheckerErrors(t, err, 1) - - assert.IsType(t, &sema.CompositeKindMismatchError{}, errs[0]) - }) - - t.Run("kind mismatch struct", func(t *testing.T) { - - t.Parallel() - - _, err := ParseAndCheck(t, - ` - struct R {} - resource interface I {} - attachment Test for R: I {}`, - ) - - errs := RequireCheckerErrors(t, err, 1) - - assert.IsType(t, &sema.CompositeKindMismatchError{}, errs[0]) - }) - - t.Run("conforms to base", func(t *testing.T) { - - t.Parallel() - - _, err := ParseAndCheck(t, - ` - resource interface I {} - attachment A for I: I {}`, - ) - - require.NoError(t, err) - }) - - t.Run("AnyResource base, resource conformance", func(t *testing.T) { - - t.Parallel() - _, err := ParseAndCheck(t, - ` - resource interface I {} - attachment A for AnyResource: I {}`, - ) + resource interface RI {} - require.NoError(t, err) - }) - - t.Run("AnyStruct base, struct conformance", func(t *testing.T) { - - t.Parallel() - - _, err := ParseAndCheck(t, - ` - struct interface I {} - attachment A for AnyStruct: I {}`, - ) - - require.NoError(t, err) - }) - - t.Run("AnyStruct base, resource conformance", func(t *testing.T) { - - t.Parallel() - - _, err := ParseAndCheck(t, - ` - resource interface I {} - attachment A for AnyStruct: I {}`, - ) - - errs := RequireCheckerErrors(t, err, 1) - - assert.IsType(t, &sema.CompositeKindMismatchError{}, errs[0]) - }) - - t.Run("AnyResource base, struct conformance", func(t *testing.T) { - - t.Parallel() - - _, err := ParseAndCheck(t, - ` - struct interface I {} - attachment A for AnyResource: I {}`, - ) + attachment Test for R: RI {} + `) errs := RequireCheckerErrors(t, err, 1) - assert.IsType(t, &sema.CompositeKindMismatchError{}, errs[0]) - }) - - t.Run("cross-contract concrete base", func(t *testing.T) { - - t.Parallel() - - _, err := ParseAndCheck(t, - ` - contract C0 { - resource interface R {} - } - contract C1 { - resource R {} - attachment A for R: C0.R {} - } - `, - ) - - require.NoError(t, err) - }) - - t.Run("cross-contract interface base", func(t *testing.T) { - - t.Parallel() - - _, err := ParseAndCheck(t, - ` - contract C0 { - resource R {} - } - contract C1 { - resource interface R {} - attachment A for C0.R: R {} - } - `, - ) - - require.NoError(t, err) + assert.IsType(t, &sema.InvalidAttachmentConformancesError{}, errs[0]) }) } @@ -1295,42 +1033,6 @@ func TestCheckAttachmentSelfTyping(t *testing.T) { require.NoError(t, err) }) - t.Run("return self struct interface", func(t *testing.T) { - - t.Parallel() - - _, err := ParseAndCheck(t, - ` - struct R {} - struct interface I {} - attachment Test for R: I { - fun foo(): &{I} { - return self - } - }`, - ) - - require.NoError(t, err) - }) - - t.Run("return self resource interface", func(t *testing.T) { - - t.Parallel() - - _, err := ParseAndCheck(t, - ` - resource R {} - resource interface I {} - attachment Test for R: I { - fun foo(): &{I} { - return self - } - }`, - ) - - require.NoError(t, err) - }) - t.Run("self access restricted", func(t *testing.T) { t.Parallel() @@ -3517,7 +3219,7 @@ func TestCheckAttachmentAccessAttachment(t *testing.T) { fmt.Sprintf(` resource R {} resource interface I {} - attachment A for AnyResource: I {} + attachment A for AnyResource {} access(all) fun foo(r: %sR) { r[I] %s diff --git a/runtime/tests/checker/entitlements_test.go b/runtime/tests/checker/entitlements_test.go index fb2a5f18dd..085d10370f 100644 --- a/runtime/tests/checker/entitlements_test.go +++ b/runtime/tests/checker/entitlements_test.go @@ -3031,116 +3031,6 @@ func TestCheckEntitlementInheritance(t *testing.T) { assert.NoError(t, err) }) - - t.Run("attachment inherited default entitled function entitlements on base", func(t *testing.T) { - t.Parallel() - - _, err := ParseAndCheck(t, ` - entitlement E - entitlement F - struct interface I { - access(E) fun foo(): auth(F) &Int { - return &1 as auth(F) &Int - } - } - struct interface I2: I {} - struct S { - access(E) fun foo() {} - } - access(all) attachment A for S: I2 {} - fun test() { - let s = attach A() to S() - let ref = &s as auth(E) &S - let attachmentRef: auth(E) &A = s[A]! - let i: auth(F) &Int = attachmentRef.foo() - } - `) - - assert.NoError(t, err) - }) - - t.Run("attachment inherited default mapped function entitlements on base", func(t *testing.T) { - t.Parallel() - - _, err := ParseAndCheck(t, ` - entitlement E - entitlement F - entitlement mapping M { - E -> F - } - struct interface I { - access(mapping M) fun foo(): auth(mapping M) &Int { - return &1 as auth(mapping M) &Int - } - } - struct interface I2: I {} - struct S { - access(E) fun foo() {} - } - access(all) attachment A for S: I2 {} - fun test() { - let s = attach A() to S() - let ref = &s as auth(E) &S - let attachmentRef: auth(E) &A = s[A]! - let i: auth(F) &Int = attachmentRef.foo() - } - `) - - assert.NoError(t, err) - }) - - t.Run("attachment inherited default mapped function entitlements not on base", func(t *testing.T) { - t.Parallel() - - _, err := ParseAndCheck(t, ` - entitlement E - entitlement F - entitlement mapping M { - E -> F - } - struct interface I { - access(mapping M) fun foo(): auth(mapping M) &Int { - return &1 as auth(mapping M) &Int - } - } - struct interface I2: I {} - struct S {} - access(all) attachment A for S: I2 {} - fun test() { - let s = attach A() to S() - let ref = &s as auth(E) &S - let i: auth(F) &Int = s[A]!.foo() - } - `) - - errs := RequireCheckerErrors(t, err, 1) - require.IsType(t, &sema.InvalidAttachmentEntitlementError{}, errs[0]) - }) - - t.Run("attachment inherited default entitled function entitlements not on base", func(t *testing.T) { - t.Parallel() - - _, err := ParseAndCheck(t, ` - entitlement E - entitlement F - struct interface I { - access(E) fun foo(): auth(F) &Int { - return &1 as auth(F) &Int - } - } - struct interface I2: I {} - struct S {} - access(all) attachment A for S: I2 {} - fun test() { - let s = attach A() to S() - let ref = &s as auth(E) &S - let i: auth(F) &Int = s[A]!.foo() - } - `) - - errs := RequireCheckerErrors(t, err, 1) - require.IsType(t, &sema.InvalidAttachmentEntitlementError{}, errs[0]) - }) } func TestCheckEntitlementTypeAnnotation(t *testing.T) { From 3f384d51309e8691e625a599c602e088e2d6dbfb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Thu, 10 Oct 2024 15:01:14 -0700 Subject: [PATCH 03/14] forbid change of attachment base type --- runtime/contract_update_validation_test.go | 49 +++++++++++++++++ ..._v0.42_to_v1_contract_upgrade_validator.go | 1 + runtime/stdlib/contract_update_validation.go | 55 +++++++++++++++++-- 3 files changed, 99 insertions(+), 6 deletions(-) diff --git a/runtime/contract_update_validation_test.go b/runtime/contract_update_validation_test.go index 451c4a2def..2ae0bade0e 100644 --- a/runtime/contract_update_validation_test.go +++ b/runtime/contract_update_validation_test.go @@ -3666,3 +3666,52 @@ func TestTypeRemovalPragmaUpdates(t *testing.T) { }, ) } + +func TestAttachmentsUpdates(t *testing.T) { + t.Parallel() + + testWithValidators(t, + "Keep base type", + func(t *testing.T, config Config) { + + const oldCode = ` + access(all) contract Test { + access(all) attachment A for AnyResource {} + } + ` + + const newCode = ` + access(all) contract Test { + access(all) attachment A for AnyResource {} + } + ` + + err := testDeployAndUpdate(t, "Test", oldCode, newCode, config) + require.NoError(t, err) + }, + ) + + testWithValidators(t, + "Change base type", + func(t *testing.T, config Config) { + + const oldCode = ` + access(all) contract Test { + access(all) attachment A for AnyResource {} + } + ` + + const newCode = ` + access(all) contract Test { + access(all) attachment A for AnyStruct {} + } + ` + + err := testDeployAndUpdate(t, "Test", oldCode, newCode, config) + + var expectedErr *stdlib.TypeMismatchError + require.ErrorAs(t, err, &expectedErr) + }, + ) + +} diff --git a/runtime/stdlib/cadence_v0.42_to_v1_contract_upgrade_validator.go b/runtime/stdlib/cadence_v0.42_to_v1_contract_upgrade_validator.go index ee5706967b..ccebf6c2d8 100644 --- a/runtime/stdlib/cadence_v0.42_to_v1_contract_upgrade_validator.go +++ b/runtime/stdlib/cadence_v0.42_to_v1_contract_upgrade_validator.go @@ -121,6 +121,7 @@ func (validator *CadenceV042ToV1ContractUpdateValidator) Validate() error { checkDeclarationUpdatability( validator, + validator.TypeComparator, oldRootDecl, newRootDecl, validator.checkConformanceV1, diff --git a/runtime/stdlib/contract_update_validation.go b/runtime/stdlib/contract_update_validation.go index 92ad1df81e..00cbad8076 100644 --- a/runtime/stdlib/contract_update_validation.go +++ b/runtime/stdlib/contract_update_validation.go @@ -142,6 +142,7 @@ func (validator *ContractUpdateValidator) Validate() error { checkDeclarationUpdatability( validator, + validator.TypeComparator, oldRootDecl, newRootDecl, validator.checkConformance, @@ -249,8 +250,8 @@ func collectRemovedTypePragmas(validator UpdateValidator, pragmas []*ast.PragmaD }) continue } - removedTypeName, isIdentifer := invocationExpression.Arguments[0].Expression.(*ast.IdentifierExpression) - if !isIdentifer { + removedTypeName, isIdentifier := invocationExpression.Arguments[0].Expression.(*ast.IdentifierExpression) + if !isIdentifier { validator.report(&InvalidTypeRemovalPragmaError{ Expression: pragma.Expression, Range: ast.NewUnmeteredRangeFromPositioned(pragma.Expression), @@ -265,6 +266,7 @@ func collectRemovedTypePragmas(validator UpdateValidator, pragmas []*ast.PragmaD func checkDeclarationUpdatability( validator UpdateValidator, + typeComparator *TypeComparator, oldDeclaration ast.Declaration, newDeclaration ast.Declaration, checkConformance checkConformanceFunc, @@ -293,13 +295,35 @@ func checkDeclarationUpdatability( checkFields(validator, oldDeclaration, newDeclaration) - checkNestedDeclarations(validator, oldDeclaration, newDeclaration, checkConformance) + checkNestedDeclarations( + validator, + typeComparator, + oldDeclaration, + newDeclaration, + checkConformance, + ) if newDecl, ok := newDeclaration.(*ast.CompositeDeclaration); ok { if oldDecl, ok := oldDeclaration.(*ast.CompositeDeclaration); ok { checkConformance(oldDecl, newDecl) } } + + // Check if the base type of the attachment has changed. + if oldAttachment, ok := oldDeclaration.(*ast.AttachmentDeclaration); ok && + oldAttachment.DeclarationKind() == common.DeclarationKindAttachment { + + // NOTE: no need to check declaration kinds match, already checked above + if newAttachment, ok := newDeclaration.(*ast.AttachmentDeclaration); ok { + err := typeComparator.CheckNominalTypeEquality( + oldAttachment.BaseType, + newAttachment.BaseType, + ) + if err != nil { + validator.report(err) + } + } + } } func checkFields( @@ -423,6 +447,7 @@ func checkTypeNotRemoved( func checkNestedDeclarations( validator UpdateValidator, + typeComparator *TypeComparator, oldDeclaration ast.Declaration, newDeclaration ast.Declaration, checkConformance checkConformanceFunc, @@ -457,7 +482,13 @@ func checkNestedDeclarations( continue } - checkDeclarationUpdatability(validator, oldNestedDecl, newNestedDecl, checkConformance) + checkDeclarationUpdatability( + validator, + typeComparator, + oldNestedDecl, + newNestedDecl, + checkConformance, + ) // If there's a matching new decl, then remove the old one from the map. delete(oldNominalTypeDecls, newNestedDecl.Identifier.Identifier) @@ -473,7 +504,13 @@ func checkNestedDeclarations( continue } - checkDeclarationUpdatability(validator, oldNestedDecl, newNestedDecl, checkConformance) + checkDeclarationUpdatability( + validator, + typeComparator, + oldNestedDecl, + newNestedDecl, + checkConformance, + ) // If there's a matching new decl, then remove the old one from the map. delete(oldNominalTypeDecls, newNestedDecl.Identifier.Identifier) @@ -489,7 +526,13 @@ func checkNestedDeclarations( continue } - checkDeclarationUpdatability(validator, oldNestedDecl, newNestedDecl, checkConformance) + checkDeclarationUpdatability( + validator, + typeComparator, + oldNestedDecl, + newNestedDecl, + checkConformance, + ) // If there's a matching new decl, then remove the old one from the map. delete(oldNominalTypeDecls, newNestedDecl.Identifier.Identifier) From 1e6ee46d35c0202ece2c8a6bbc338d578dd00446 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Thu, 10 Oct 2024 16:16:38 -0700 Subject: [PATCH 04/14] put attachment base type change validation behind a feature flag --- runtime/config.go | 2 + runtime/contract_update_validation_test.go | 50 +++++++++++++++-- runtime/environment.go | 25 +++++---- runtime/interpreter/config.go | 2 + runtime/stdlib/account.go | 5 +- ..._v0.42_to_v1_contract_upgrade_validator.go | 19 +++++-- runtime/stdlib/contract_update_validation.go | 56 ++++++++++++------- 7 files changed, 117 insertions(+), 42 deletions(-) diff --git a/runtime/config.go b/runtime/config.go index 3e3a273ff1..50b803a188 100644 --- a/runtime/config.go +++ b/runtime/config.go @@ -41,4 +41,6 @@ type Config struct { LegacyContractUpgradeEnabled bool // ContractUpdateTypeRemovalEnabled specifies if type removal is enabled in contract updates ContractUpdateTypeRemovalEnabled bool + // ContractUpdateAttachmentBaseTypeChangeEnabled specifies if attachment base type change is enabled in contract updates + ContractUpdateAttachmentBaseTypeChangeEnabled bool } diff --git a/runtime/contract_update_validation_test.go b/runtime/contract_update_validation_test.go index 2ae0bade0e..fad78918da 100644 --- a/runtime/contract_update_validation_test.go +++ b/runtime/contract_update_validation_test.go @@ -205,6 +205,43 @@ func testWithValidatorsAndTypeRemovalEnabled( } } +func testWithValidatorsAndAttachmentBaseTypeChangeEnabled( + t *testing.T, + name string, + testFunc func(t *testing.T, config Config), +) { + for _, withC1Upgrade := range []bool{true, false} { + withC1Upgrade := withC1Upgrade + name := name + + for _, withAttachmentBaseTypeChangeEnabled := range []bool{true, false} { + withAttachmentBaseTypeChangeEnabled := withAttachmentBaseTypeChangeEnabled + name := name + + switch { + case withC1Upgrade && withAttachmentBaseTypeChangeEnabled: + name = fmt.Sprintf("%s (with C1 validator and attachment base type change enabled)", name) + + case withC1Upgrade: + name = fmt.Sprintf("%s (with C1 validator)", name) + + case withAttachmentBaseTypeChangeEnabled: + name = fmt.Sprintf("%s (with attachment base type change enabled)", name) + } + + t.Run(name, func(t *testing.T) { + t.Parallel() + + config := DefaultTestInterpreterConfig + config.LegacyContractUpgradeEnabled = withC1Upgrade + config.ContractUpdateAttachmentBaseTypeChangeEnabled = withAttachmentBaseTypeChangeEnabled + + testFunc(t, config) + }) + } + } +} + func TestRuntimeContractUpdateValidation(t *testing.T) { t.Parallel() @@ -3670,7 +3707,7 @@ func TestTypeRemovalPragmaUpdates(t *testing.T) { func TestAttachmentsUpdates(t *testing.T) { t.Parallel() - testWithValidators(t, + testWithValidatorsAndAttachmentBaseTypeChangeEnabled(t, "Keep base type", func(t *testing.T, config Config) { @@ -3691,7 +3728,7 @@ func TestAttachmentsUpdates(t *testing.T) { }, ) - testWithValidators(t, + testWithValidatorsAndAttachmentBaseTypeChangeEnabled(t, "Change base type", func(t *testing.T, config Config) { @@ -3709,9 +3746,12 @@ func TestAttachmentsUpdates(t *testing.T) { err := testDeployAndUpdate(t, "Test", oldCode, newCode, config) - var expectedErr *stdlib.TypeMismatchError - require.ErrorAs(t, err, &expectedErr) + if config.ContractUpdateAttachmentBaseTypeChangeEnabled { + require.NoError(t, err) + } else { + var expectedErr *stdlib.TypeMismatchError + require.ErrorAs(t, err, &expectedErr) + } }, ) - } diff --git a/runtime/environment.go b/runtime/environment.go index 95a3ac847c..7c55dbf97d 100644 --- a/runtime/environment.go +++ b/runtime/environment.go @@ -185,18 +185,19 @@ func (e *interpreterEnvironment) newInterpreterConfig() *interpreter.Config { // and disable storage validation after each value modification. // Instead, storage is validated after commits (if validation is enabled), // see interpreterEnvironment.CommitStorage - AtreeStorageValidationEnabled: false, - Debugger: e.config.Debugger, - OnStatement: e.newOnStatementHandler(), - OnMeterComputation: e.newOnMeterComputation(), - OnFunctionInvocation: e.newOnFunctionInvocationHandler(), - OnInvokedFunctionReturn: e.newOnInvokedFunctionReturnHandler(), - CapabilityBorrowHandler: e.newCapabilityBorrowHandler(), - CapabilityCheckHandler: e.newCapabilityCheckHandler(), - LegacyContractUpgradeEnabled: e.config.LegacyContractUpgradeEnabled, - ContractUpdateTypeRemovalEnabled: e.config.ContractUpdateTypeRemovalEnabled, - ValidateAccountCapabilitiesGetHandler: e.newValidateAccountCapabilitiesGetHandler(), - ValidateAccountCapabilitiesPublishHandler: e.newValidateAccountCapabilitiesPublishHandler(), + AtreeStorageValidationEnabled: false, + Debugger: e.config.Debugger, + OnStatement: e.newOnStatementHandler(), + OnMeterComputation: e.newOnMeterComputation(), + OnFunctionInvocation: e.newOnFunctionInvocationHandler(), + OnInvokedFunctionReturn: e.newOnInvokedFunctionReturnHandler(), + CapabilityBorrowHandler: e.newCapabilityBorrowHandler(), + CapabilityCheckHandler: e.newCapabilityCheckHandler(), + LegacyContractUpgradeEnabled: e.config.LegacyContractUpgradeEnabled, + ContractUpdateTypeRemovalEnabled: e.config.ContractUpdateTypeRemovalEnabled, + ContractUpdateAttachmentBaseTypeChangeEnabled: e.config.ContractUpdateAttachmentBaseTypeChangeEnabled, + ValidateAccountCapabilitiesGetHandler: e.newValidateAccountCapabilitiesGetHandler(), + ValidateAccountCapabilitiesPublishHandler: e.newValidateAccountCapabilitiesPublishHandler(), } } diff --git a/runtime/interpreter/config.go b/runtime/interpreter/config.go index dc052342d1..f64413aef5 100644 --- a/runtime/interpreter/config.go +++ b/runtime/interpreter/config.go @@ -74,6 +74,8 @@ type Config struct { LegacyContractUpgradeEnabled bool // ContractUpdateTypeRemovalEnabled specifies if type removal is enabled in contract updates ContractUpdateTypeRemovalEnabled bool + // ContractUpdateAttachmentBaseTypeChangeEnabled specifies if attachment base type change is enabled in contract updates + ContractUpdateAttachmentBaseTypeChangeEnabled bool // ValidateAccountCapabilitiesGetHandler is used to handle when a capability of an account is got. ValidateAccountCapabilitiesGetHandler ValidateAccountCapabilitiesGetHandlerFunc // ValidateAccountCapabilitiesPublishHandler is used to handle when a capability of an account is got. diff --git a/runtime/stdlib/account.go b/runtime/stdlib/account.go index f7eb85b1b6..235dc29e6f 100644 --- a/runtime/stdlib/account.go +++ b/runtime/stdlib/account.go @@ -1679,6 +1679,7 @@ func changeAccountContracts( memoryGauge := invocation.Interpreter.SharedState.Config.MemoryGauge legacyUpgradeEnabled := invocation.Interpreter.SharedState.Config.LegacyContractUpgradeEnabled contractUpdateTypeRemovalEnabled := invocation.Interpreter.SharedState.Config.ContractUpdateTypeRemovalEnabled + contractUpdateAttachmentBaseTypeChangeEnabled := invocation.Interpreter.SharedState.Config.ContractUpdateAttachmentBaseTypeChangeEnabled var oldProgram *ast.Program @@ -1731,7 +1732,9 @@ func changeAccountContracts( ) } - validator = validator.WithTypeRemovalEnabled(contractUpdateTypeRemovalEnabled) + validator = validator. + WithTypeRemovalEnabled(contractUpdateTypeRemovalEnabled). + WithAttachmentBaseTypeChangeEnabled(contractUpdateAttachmentBaseTypeChangeEnabled) err = validator.Validate() handleContractUpdateError(err, newCode) diff --git a/runtime/stdlib/cadence_v0.42_to_v1_contract_upgrade_validator.go b/runtime/stdlib/cadence_v0.42_to_v1_contract_upgrade_validator.go index ccebf6c2d8..d3fc96209b 100644 --- a/runtime/stdlib/cadence_v0.42_to_v1_contract_upgrade_validator.go +++ b/runtime/stdlib/cadence_v0.42_to_v1_contract_upgrade_validator.go @@ -71,10 +71,6 @@ func NewCadenceV042ToV1ContractUpdateValidator( var _ UpdateValidator = &CadenceV042ToV1ContractUpdateValidator{} -func (validator *CadenceV042ToV1ContractUpdateValidator) isTypeRemovalEnabled() bool { - return validator.underlyingUpdateValidator.isTypeRemovalEnabled() -} - func (validator *CadenceV042ToV1ContractUpdateValidator) WithUserDefinedTypeChangeChecker( typeChangeCheckFunc func(oldTypeID common.TypeID, newTypeID common.TypeID) (checked, valid bool), ) *CadenceV042ToV1ContractUpdateValidator { @@ -82,6 +78,10 @@ func (validator *CadenceV042ToV1ContractUpdateValidator) WithUserDefinedTypeChan return validator } +func (validator *CadenceV042ToV1ContractUpdateValidator) isTypeRemovalEnabled() bool { + return validator.underlyingUpdateValidator.isTypeRemovalEnabled() +} + func (validator *CadenceV042ToV1ContractUpdateValidator) WithTypeRemovalEnabled( enabled bool, ) UpdateValidator { @@ -89,6 +89,17 @@ func (validator *CadenceV042ToV1ContractUpdateValidator) WithTypeRemovalEnabled( return validator } +func (validator *CadenceV042ToV1ContractUpdateValidator) isAttachmentBaseTypeChangeEnabled() bool { + return validator.underlyingUpdateValidator.isAttachmentBaseTypeChangeEnabled() +} + +func (validator *CadenceV042ToV1ContractUpdateValidator) WithAttachmentBaseTypeChangeEnabled( + enabled bool, +) UpdateValidator { + validator.underlyingUpdateValidator.WithAttachmentBaseTypeChangeEnabled(enabled) + return validator +} + func (validator *CadenceV042ToV1ContractUpdateValidator) getCurrentDeclaration() ast.Declaration { return validator.underlyingUpdateValidator.getCurrentDeclaration() } diff --git a/runtime/stdlib/contract_update_validation.go b/runtime/stdlib/contract_update_validation.go index 00cbad8076..0113ab439c 100644 --- a/runtime/stdlib/contract_update_validation.go +++ b/runtime/stdlib/contract_update_validation.go @@ -53,6 +53,9 @@ type UpdateValidator interface { isTypeRemovalEnabled() bool WithTypeRemovalEnabled(enabled bool) UpdateValidator + + isAttachmentBaseTypeChangeEnabled() bool + WithAttachmentBaseTypeChangeEnabled(enabled bool) UpdateValidator } type checkConformanceFunc func( @@ -63,15 +66,16 @@ type checkConformanceFunc func( type ContractUpdateValidator struct { *TypeComparator - location common.Location - contractName string - oldProgram *ast.Program - newProgram *ast.Program - currentDecl ast.Declaration - importLocations map[ast.Identifier]common.Location - accountContractNamesProvider AccountContractNamesProvider - errors []error - typeRemovalEnabled bool + location common.Location + contractName string + oldProgram *ast.Program + newProgram *ast.Program + currentDecl ast.Declaration + importLocations map[ast.Identifier]common.Location + accountContractNamesProvider AccountContractNamesProvider + errors []error + typeRemovalEnabled bool + attachmentBaseTypeChangeEnabled bool } // ContractUpdateValidator should implement ast.TypeEqualityChecker @@ -108,6 +112,15 @@ func (validator *ContractUpdateValidator) WithTypeRemovalEnabled(enabled bool) U return validator } +func (validator *ContractUpdateValidator) isAttachmentBaseTypeChangeEnabled() bool { + return validator.attachmentBaseTypeChangeEnabled +} + +func (validator *ContractUpdateValidator) WithAttachmentBaseTypeChangeEnabled(enabled bool) UpdateValidator { + validator.attachmentBaseTypeChangeEnabled = enabled + return validator +} + func (validator *ContractUpdateValidator) getCurrentDeclaration() ast.Declaration { return validator.currentDecl } @@ -309,18 +322,21 @@ func checkDeclarationUpdatability( } } - // Check if the base type of the attachment has changed. - if oldAttachment, ok := oldDeclaration.(*ast.AttachmentDeclaration); ok && - oldAttachment.DeclarationKind() == common.DeclarationKindAttachment { + if !validator.isAttachmentBaseTypeChangeEnabled() { - // NOTE: no need to check declaration kinds match, already checked above - if newAttachment, ok := newDeclaration.(*ast.AttachmentDeclaration); ok { - err := typeComparator.CheckNominalTypeEquality( - oldAttachment.BaseType, - newAttachment.BaseType, - ) - if err != nil { - validator.report(err) + // Check if the base type of the attachment has changed. + if oldAttachment, ok := oldDeclaration.(*ast.AttachmentDeclaration); ok && + oldAttachment.DeclarationKind() == common.DeclarationKindAttachment { + + // NOTE: no need to check declaration kinds match, already checked above + if newAttachment, ok := newDeclaration.(*ast.AttachmentDeclaration); ok { + err := typeComparator.CheckNominalTypeEquality( + oldAttachment.BaseType, + newAttachment.BaseType, + ) + if err != nil { + validator.report(err) + } } } } From 78495040812f57f720ad0f795c27d35aa8bb42ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Thu, 10 Oct 2024 16:36:41 -0700 Subject: [PATCH 05/14] put attachment conformances behind feature flag --- runtime/config.go | 2 + runtime/environment.go | 1 + runtime/sema/check_composite_declaration.go | 37 +++++---- runtime/sema/config.go | 2 + runtime/tests/checker/attachments_test.go | 86 ++++++++++++++++++--- 5 files changed, 101 insertions(+), 27 deletions(-) diff --git a/runtime/config.go b/runtime/config.go index 50b803a188..23b7e917df 100644 --- a/runtime/config.go +++ b/runtime/config.go @@ -37,6 +37,8 @@ type Config struct { CoverageReport *CoverageReport // AttachmentsEnabled specifies if attachments are enabled AttachmentsEnabled bool + // AttachmentConformancesEnabled specifies if attachment conformances are enabled + AttachmentConformancesEnabled bool // LegacyContractUpgradeEnabled enabled specifies whether to use the old parser when parsing an old contract LegacyContractUpgradeEnabled bool // ContractUpdateTypeRemovalEnabled specifies if type removal is enabled in contract updates diff --git a/runtime/environment.go b/runtime/environment.go index 7c55dbf97d..afbb9f763c 100644 --- a/runtime/environment.go +++ b/runtime/environment.go @@ -211,6 +211,7 @@ func (e *interpreterEnvironment) newCheckerConfig() *sema.Config { ImportHandler: e.resolveImport, CheckHandler: e.newCheckHandler(), AttachmentsEnabled: e.config.AttachmentsEnabled, + AttachmentConformancesEnabled: e.config.AttachmentConformancesEnabled, } } diff --git a/runtime/sema/check_composite_declaration.go b/runtime/sema/check_composite_declaration.go index cb2fac91bd..bcae42aec7 100644 --- a/runtime/sema/check_composite_declaration.go +++ b/runtime/sema/check_composite_declaration.go @@ -686,23 +686,30 @@ func (checker *Checker) declareCompositeType(declaration ast.CompositeLikeDeclar compositeType.EnumRawType = checker.enumRawType(declaration.(*ast.CompositeDeclaration)) case common.CompositeKindAttachment: - // Attachments may not conform to interfaces - conformanceList := declaration.ConformanceList() - conformanceCount := len(conformanceList) - if conformanceCount > 0 { - firstConformance := conformanceList[0] - lastConformance := conformanceList[conformanceCount-1] + if checker.Config.AttachmentConformancesEnabled { + compositeType.ExplicitInterfaceConformances = + checker.explicitInterfaceConformances(declaration, compositeType) - checker.report( - &InvalidAttachmentConformancesError{ - Range: ast.NewRange( - checker.memoryGauge, - firstConformance.StartPosition(), - lastConformance.EndPosition(checker.memoryGauge), - ), - }, - ) + } else { + // Attachments may not conform to interfaces + + conformanceList := declaration.ConformanceList() + conformanceCount := len(conformanceList) + if conformanceCount > 0 { + firstConformance := conformanceList[0] + lastConformance := conformanceList[conformanceCount-1] + + checker.report( + &InvalidAttachmentConformancesError{ + Range: ast.NewRange( + checker.memoryGauge, + firstConformance.StartPosition(), + lastConformance.EndPosition(checker.memoryGauge), + ), + }, + ) + } } default: diff --git a/runtime/sema/config.go b/runtime/sema/config.go index 06f6e9ce95..a48e620522 100644 --- a/runtime/sema/config.go +++ b/runtime/sema/config.go @@ -55,4 +55,6 @@ type Config struct { AllowStaticDeclarations bool // AttachmentsEnabled determines if attachments are enabled AttachmentsEnabled bool + // AttachmentConformancesEnabled determines if attachment conformances are enabled + AttachmentConformancesEnabled bool } diff --git a/runtime/tests/checker/attachments_test.go b/runtime/tests/checker/attachments_test.go index f6a3c75ae3..6a890e0124 100644 --- a/runtime/tests/checker/attachments_test.go +++ b/runtime/tests/checker/attachments_test.go @@ -623,34 +623,96 @@ func TestCheckInvalidAttachmentConformance(t *testing.T) { t.Parallel() - t.Run("struct", func(t *testing.T) { + t.Run("struct, enabled", func(t *testing.T) { t.Parallel() - _, err := ParseAndCheck(t, ` - struct S {} + _, err := ParseAndCheckWithOptions(t, + ` + struct S {} - struct interface SI {} + struct interface SI {} - attachment Test for S: SI {} - `) + attachment Test for S: SI {} + `, + ParseAndCheckOptions{ + Config: &sema.Config{ + AttachmentsEnabled: true, + AttachmentConformancesEnabled: true, + }, + }, + ) + + require.NoError(t, err) + }) + + t.Run("struct, disabled", func(t *testing.T) { + + t.Parallel() + + _, err := ParseAndCheckWithOptions(t, + ` + struct S {} + + struct interface SI {} + + attachment Test for S: SI {} + `, + ParseAndCheckOptions{ + Config: &sema.Config{ + AttachmentsEnabled: true, + AttachmentConformancesEnabled: false, + }, + }, + ) errs := RequireCheckerErrors(t, err, 1) assert.IsType(t, &sema.InvalidAttachmentConformancesError{}, errs[0]) }) - t.Run("resource", func(t *testing.T) { + t.Run("resource, enabled", func(t *testing.T) { t.Parallel() - _, err := ParseAndCheck(t, ` - resource R {} + _, err := ParseAndCheckWithOptions(t, + ` + resource R {} - resource interface RI {} + resource interface RI {} - attachment Test for R: RI {} - `) + attachment Test for R: RI {} + `, + ParseAndCheckOptions{ + Config: &sema.Config{ + AttachmentsEnabled: true, + AttachmentConformancesEnabled: true, + }, + }, + ) + + require.NoError(t, err) + }) + + t.Run("resource, disabled", func(t *testing.T) { + + t.Parallel() + + _, err := ParseAndCheckWithOptions(t, + ` + resource R {} + + resource interface RI {} + + attachment Test for R: RI {} + `, + ParseAndCheckOptions{ + Config: &sema.Config{ + AttachmentsEnabled: true, + AttachmentConformancesEnabled: false, + }, + }, + ) errs := RequireCheckerErrors(t, err, 1) From 605fc9563aca1b6810f581198b080c30b6e1f5f7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Fri, 11 Oct 2024 10:50:48 -0700 Subject: [PATCH 06/14] put sibling type check for members behind a feature flag --- runtime/config.go | 2 + runtime/environment.go | 1 + runtime/runtime_test.go | 81 +++++++++++++-------- runtime/sema/check_composite_declaration.go | 2 +- runtime/sema/config.go | 2 + 5 files changed, 58 insertions(+), 30 deletions(-) diff --git a/runtime/config.go b/runtime/config.go index 23b7e917df..c81d396b43 100644 --- a/runtime/config.go +++ b/runtime/config.go @@ -39,6 +39,8 @@ type Config struct { AttachmentsEnabled bool // AttachmentConformancesEnabled specifies if attachment conformances are enabled AttachmentConformancesEnabled bool + // MemberSiblingTypeCheckEnabled specifies if declaring members checks for sibling type declarations + MemberSiblingTypeCheckEnabled bool // LegacyContractUpgradeEnabled enabled specifies whether to use the old parser when parsing an old contract LegacyContractUpgradeEnabled bool // ContractUpdateTypeRemovalEnabled specifies if type removal is enabled in contract updates diff --git a/runtime/environment.go b/runtime/environment.go index afbb9f763c..ae9fe82cba 100644 --- a/runtime/environment.go +++ b/runtime/environment.go @@ -212,6 +212,7 @@ func (e *interpreterEnvironment) newCheckerConfig() *sema.Config { CheckHandler: e.newCheckHandler(), AttachmentsEnabled: e.config.AttachmentsEnabled, AttachmentConformancesEnabled: e.config.AttachmentConformancesEnabled, + MemberSiblingTypeCheckEnabled: e.config.MemberSiblingTypeCheckEnabled, } } diff --git a/runtime/runtime_test.go b/runtime/runtime_test.go index 2b57648779..393d060204 100644 --- a/runtime/runtime_test.go +++ b/runtime/runtime_test.go @@ -11495,42 +11495,46 @@ func TestRuntimeBuiltInFunctionConfusion(t *testing.T) { } ` - runtime := NewTestInterpreterRuntime() - address := common.MustBytesToAddress([]byte{0x1}) - accountCodes := map[common.AddressLocation][]byte{} - var events []cadence.Event - var loggedMessages []string + newRuntimeInterface := func() Interface { - runtimeInterface := &TestRuntimeInterface{ - Storage: NewTestLedger(nil, nil), - OnGetSigningAccounts: func() ([]common.Address, error) { - return []common.Address{address}, nil - }, - OnResolveLocation: NewSingleIdentifierLocationResolver(t), - OnUpdateAccountContractCode: func(location common.AddressLocation, code []byte) error { - accountCodes[location] = code - return nil - }, - OnGetAccountContractCode: func(location common.AddressLocation) (code []byte, err error) { - code = accountCodes[location] - return code, nil - }, - OnEmitEvent: func(event cadence.Event) error { - events = append(events, event) - return nil - }, - OnProgramLog: func(message string) { - loggedMessages = append(loggedMessages, message) - }, + accountCodes := map[common.AddressLocation][]byte{} + var events []cadence.Event + var loggedMessages []string + + return &TestRuntimeInterface{ + Storage: NewTestLedger(nil, nil), + OnGetSigningAccounts: func() ([]common.Address, error) { + return []common.Address{address}, nil + }, + OnResolveLocation: NewSingleIdentifierLocationResolver(t), + OnUpdateAccountContractCode: func(location common.AddressLocation, code []byte) error { + accountCodes[location] = code + return nil + }, + OnGetAccountContractCode: func(location common.AddressLocation) (code []byte, err error) { + code = accountCodes[location] + return code, nil + }, + OnEmitEvent: func(event cadence.Event) error { + events = append(events, event) + return nil + }, + OnProgramLog: func(message string) { + loggedMessages = append(loggedMessages, message) + }, + } } nextTransactionLocation := NewTransactionLocationGenerator() - // Deploy contract + // Deploy contract without check enabled - err := runtime.ExecuteTransaction( + err := NewTestInterpreterRuntimeWithConfig(Config{ + AtreeValidationEnabled: true, + MemberSiblingTypeCheckEnabled: false, + }).ExecuteTransaction( Script{ Source: DeploymentTransaction( "Foo", @@ -11538,7 +11542,26 @@ func TestRuntimeBuiltInFunctionConfusion(t *testing.T) { ), }, Context{ - Interface: runtimeInterface, + Interface: newRuntimeInterface(), + Location: nextTransactionLocation(), + }, + ) + require.NoError(t, err) + + // Deploy contract with check enabled + + err = NewTestInterpreterRuntimeWithConfig(Config{ + AtreeValidationEnabled: true, + MemberSiblingTypeCheckEnabled: true, + }).ExecuteTransaction( + Script{ + Source: DeploymentTransaction( + "Foo", + []byte(contract), + ), + }, + Context{ + Interface: newRuntimeInterface(), Location: nextTransactionLocation(), }, ) diff --git a/runtime/sema/check_composite_declaration.go b/runtime/sema/check_composite_declaration.go index bcae42aec7..f8280702ac 100644 --- a/runtime/sema/check_composite_declaration.go +++ b/runtime/sema/check_composite_declaration.go @@ -1748,7 +1748,7 @@ func (checker *Checker) defaultMembersAndOrigins( } } - if nestedTypes != nil { + if nestedTypes != nil && checker.Config.MemberSiblingTypeCheckEnabled { if _, ok := nestedTypes.Get(name); ok { // TODO: provide previous position checker.report( diff --git a/runtime/sema/config.go b/runtime/sema/config.go index a48e620522..2c760d701b 100644 --- a/runtime/sema/config.go +++ b/runtime/sema/config.go @@ -57,4 +57,6 @@ type Config struct { AttachmentsEnabled bool // AttachmentConformancesEnabled determines if attachment conformances are enabled AttachmentConformancesEnabled bool + // MemberSiblingTypeCheckEnabled determines if declaring members checks for sibling type declarations + MemberSiblingTypeCheckEnabled bool } From 74b013566bb2c3005960fa5d275d4f466a3f7a82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Fri, 11 Oct 2024 14:01:15 -0700 Subject: [PATCH 07/14] invert feature flag --- runtime/config.go | 4 ++-- runtime/environment.go | 2 +- runtime/runtime_test.go | 8 ++++---- runtime/sema/check_composite_declaration.go | 2 +- runtime/sema/config.go | 4 ++-- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/runtime/config.go b/runtime/config.go index c81d396b43..93f3d37844 100644 --- a/runtime/config.go +++ b/runtime/config.go @@ -39,8 +39,8 @@ type Config struct { AttachmentsEnabled bool // AttachmentConformancesEnabled specifies if attachment conformances are enabled AttachmentConformancesEnabled bool - // MemberSiblingTypeCheckEnabled specifies if declaring members checks for sibling type declarations - MemberSiblingTypeCheckEnabled bool + // MemberSiblingTypeOverrideEnabled specifies if declaring members checks for sibling type declarations + MemberSiblingTypeOverrideEnabled bool // LegacyContractUpgradeEnabled enabled specifies whether to use the old parser when parsing an old contract LegacyContractUpgradeEnabled bool // ContractUpdateTypeRemovalEnabled specifies if type removal is enabled in contract updates diff --git a/runtime/environment.go b/runtime/environment.go index ae9fe82cba..bef0cc5361 100644 --- a/runtime/environment.go +++ b/runtime/environment.go @@ -212,7 +212,7 @@ func (e *interpreterEnvironment) newCheckerConfig() *sema.Config { CheckHandler: e.newCheckHandler(), AttachmentsEnabled: e.config.AttachmentsEnabled, AttachmentConformancesEnabled: e.config.AttachmentConformancesEnabled, - MemberSiblingTypeCheckEnabled: e.config.MemberSiblingTypeCheckEnabled, + MemberSiblingTypeOverrideEnabled: e.config.MemberSiblingTypeOverrideEnabled, } } diff --git a/runtime/runtime_test.go b/runtime/runtime_test.go index 393d060204..b5375fb799 100644 --- a/runtime/runtime_test.go +++ b/runtime/runtime_test.go @@ -11532,8 +11532,8 @@ func TestRuntimeBuiltInFunctionConfusion(t *testing.T) { // Deploy contract without check enabled err := NewTestInterpreterRuntimeWithConfig(Config{ - AtreeValidationEnabled: true, - MemberSiblingTypeCheckEnabled: false, + AtreeValidationEnabled: true, + MemberSiblingTypeOverrideEnabled: true, }).ExecuteTransaction( Script{ Source: DeploymentTransaction( @@ -11551,8 +11551,8 @@ func TestRuntimeBuiltInFunctionConfusion(t *testing.T) { // Deploy contract with check enabled err = NewTestInterpreterRuntimeWithConfig(Config{ - AtreeValidationEnabled: true, - MemberSiblingTypeCheckEnabled: true, + AtreeValidationEnabled: true, + MemberSiblingTypeOverrideEnabled: false, }).ExecuteTransaction( Script{ Source: DeploymentTransaction( diff --git a/runtime/sema/check_composite_declaration.go b/runtime/sema/check_composite_declaration.go index f8280702ac..75b8c3a8d6 100644 --- a/runtime/sema/check_composite_declaration.go +++ b/runtime/sema/check_composite_declaration.go @@ -1748,7 +1748,7 @@ func (checker *Checker) defaultMembersAndOrigins( } } - if nestedTypes != nil && checker.Config.MemberSiblingTypeCheckEnabled { + if nestedTypes != nil && !checker.Config.MemberSiblingTypeOverrideEnabled { if _, ok := nestedTypes.Get(name); ok { // TODO: provide previous position checker.report( diff --git a/runtime/sema/config.go b/runtime/sema/config.go index 2c760d701b..c8e72c695e 100644 --- a/runtime/sema/config.go +++ b/runtime/sema/config.go @@ -57,6 +57,6 @@ type Config struct { AttachmentsEnabled bool // AttachmentConformancesEnabled determines if attachment conformances are enabled AttachmentConformancesEnabled bool - // MemberSiblingTypeCheckEnabled determines if declaring members checks for sibling type declarations - MemberSiblingTypeCheckEnabled bool + // MemberSiblingTypeOverrideEnabled determines if declaring members checks for sibling type declarations + MemberSiblingTypeOverrideEnabled bool } From 03826a54c49554ae5813f45e99a206fc4b95056e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Fri, 11 Oct 2024 14:41:17 -0700 Subject: [PATCH 08/14] enable v1.0.1 features based on current version --- runtime/config.go | 6 - runtime/contract_update_validation_test.go | 121 +++++++++++-------- runtime/empty.go | 4 + runtime/environment.go | 47 ++++--- runtime/interface.go | 2 + runtime/runtime_test.go | 19 +-- runtime/tests/runtime_utils/testinterface.go | 8 ++ 7 files changed, 126 insertions(+), 81 deletions(-) diff --git a/runtime/config.go b/runtime/config.go index 93f3d37844..3e3a273ff1 100644 --- a/runtime/config.go +++ b/runtime/config.go @@ -37,14 +37,8 @@ type Config struct { CoverageReport *CoverageReport // AttachmentsEnabled specifies if attachments are enabled AttachmentsEnabled bool - // AttachmentConformancesEnabled specifies if attachment conformances are enabled - AttachmentConformancesEnabled bool - // MemberSiblingTypeOverrideEnabled specifies if declaring members checks for sibling type declarations - MemberSiblingTypeOverrideEnabled bool // LegacyContractUpgradeEnabled enabled specifies whether to use the old parser when parsing an old contract LegacyContractUpgradeEnabled bool // ContractUpdateTypeRemovalEnabled specifies if type removal is enabled in contract updates ContractUpdateTypeRemovalEnabled bool - // ContractUpdateAttachmentBaseTypeChangeEnabled specifies if attachment base type change is enabled in contract updates - ContractUpdateAttachmentBaseTypeChangeEnabled bool } diff --git a/runtime/contract_update_validation_test.go b/runtime/contract_update_validation_test.go index fad78918da..1fc995699b 100644 --- a/runtime/contract_update_validation_test.go +++ b/runtime/contract_update_validation_test.go @@ -83,6 +83,10 @@ func newContractRemovalTransaction(contractName string) string { } func newContractDeploymentTransactor(t *testing.T, config Config) func(code string) error { + return newContractDeploymentTransactorWithVersion(t, config, "") +} + +func newContractDeploymentTransactorWithVersion(t *testing.T, config Config, version string) func(code string) error { rt := NewTestInterpreterRuntimeWithConfig(config) @@ -112,6 +116,9 @@ func newContractDeploymentTransactor(t *testing.T, config Config) func(code stri events = append(events, event) return nil }, + OnCurrentVersion: func() string { + return version + }, } nextTransactionLocation := NewTransactionLocationGenerator() @@ -132,7 +139,18 @@ func newContractDeploymentTransactor(t *testing.T, config Config) func(code stri // testDeployAndUpdate deploys a contract in one transaction, // then updates the contract in another transaction func testDeployAndUpdate(t *testing.T, name string, oldCode string, newCode string, config Config) error { - executeTransaction := newContractDeploymentTransactor(t, config) + return testDeployAndUpdateWithVersion(t, name, oldCode, newCode, config, "") +} + +func testDeployAndUpdateWithVersion( + t *testing.T, + name string, + oldCode string, + newCode string, + config Config, + version string, +) error { + executeTransaction := newContractDeploymentTransactorWithVersion(t, config, version) err := executeTransaction(newContractAddTransaction(name, oldCode)) require.NoError(t, err) @@ -205,43 +223,6 @@ func testWithValidatorsAndTypeRemovalEnabled( } } -func testWithValidatorsAndAttachmentBaseTypeChangeEnabled( - t *testing.T, - name string, - testFunc func(t *testing.T, config Config), -) { - for _, withC1Upgrade := range []bool{true, false} { - withC1Upgrade := withC1Upgrade - name := name - - for _, withAttachmentBaseTypeChangeEnabled := range []bool{true, false} { - withAttachmentBaseTypeChangeEnabled := withAttachmentBaseTypeChangeEnabled - name := name - - switch { - case withC1Upgrade && withAttachmentBaseTypeChangeEnabled: - name = fmt.Sprintf("%s (with C1 validator and attachment base type change enabled)", name) - - case withC1Upgrade: - name = fmt.Sprintf("%s (with C1 validator)", name) - - case withAttachmentBaseTypeChangeEnabled: - name = fmt.Sprintf("%s (with attachment base type change enabled)", name) - } - - t.Run(name, func(t *testing.T) { - t.Parallel() - - config := DefaultTestInterpreterConfig - config.LegacyContractUpgradeEnabled = withC1Upgrade - config.ContractUpdateAttachmentBaseTypeChangeEnabled = withAttachmentBaseTypeChangeEnabled - - testFunc(t, config) - }) - } - } -} - func TestRuntimeContractUpdateValidation(t *testing.T) { t.Parallel() @@ -3707,8 +3688,8 @@ func TestTypeRemovalPragmaUpdates(t *testing.T) { func TestAttachmentsUpdates(t *testing.T) { t.Parallel() - testWithValidatorsAndAttachmentBaseTypeChangeEnabled(t, - "Keep base type", + testWithValidators(t, + "Keep base type, v1.0.1", func(t *testing.T, config Config) { const oldCode = ` @@ -3723,13 +3704,13 @@ func TestAttachmentsUpdates(t *testing.T) { } ` - err := testDeployAndUpdate(t, "Test", oldCode, newCode, config) + err := testDeployAndUpdateWithVersion(t, "Test", oldCode, newCode, config, "v1.0.1") require.NoError(t, err) }, ) - testWithValidatorsAndAttachmentBaseTypeChangeEnabled(t, - "Change base type", + testWithValidators(t, + "Keep base type, v1.0.2", func(t *testing.T, config Config) { const oldCode = ` @@ -3740,18 +3721,54 @@ func TestAttachmentsUpdates(t *testing.T) { const newCode = ` access(all) contract Test { - access(all) attachment A for AnyStruct {} + access(all) attachment A for AnyResource {} } ` - err := testDeployAndUpdate(t, "Test", oldCode, newCode, config) - - if config.ContractUpdateAttachmentBaseTypeChangeEnabled { - require.NoError(t, err) - } else { - var expectedErr *stdlib.TypeMismatchError - require.ErrorAs(t, err, &expectedErr) - } + err := testDeployAndUpdateWithVersion(t, "Test", oldCode, newCode, config, "v1.0.2") + require.NoError(t, err) }, ) + + testWithValidators(t, + "Change base type, v1.0.1", + func(t *testing.T, config Config) { + + const oldCode = ` + access(all) contract Test { + access(all) attachment A for AnyResource {} + } + ` + + const newCode = ` + access(all) contract Test { + access(all) attachment A for AnyStruct {} + } + ` + + err := testDeployAndUpdateWithVersion(t, "Test", oldCode, newCode, config, "v1.0.1") + require.NoError(t, err) + }) + + testWithValidators(t, + "Change base type, v1.0.2", + func(t *testing.T, config Config) { + + const oldCode = ` + access(all) contract Test { + access(all) attachment A for AnyResource {} + } + ` + + const newCode = ` + access(all) contract Test { + access(all) attachment A for AnyStruct {} + } + ` + + err := testDeployAndUpdateWithVersion(t, "Test", oldCode, newCode, config, "v1.0.2") + + var expectedErr *stdlib.TypeMismatchError + require.ErrorAs(t, err, &expectedErr) + }) } diff --git a/runtime/empty.go b/runtime/empty.go index 86b5b0abce..103c318e90 100644 --- a/runtime/empty.go +++ b/runtime/empty.go @@ -260,3 +260,7 @@ func (EmptyRuntimeInterface) ValidateAccountCapabilitiesPublish( ) (bool, error) { panic("unexpected call to ValidateAccountCapabilitiesPublish") } + +func (EmptyRuntimeInterface) CurrentVersion() string { + panic("unexpected call to CurrentVersion") +} diff --git a/runtime/environment.go b/runtime/environment.go index bef0cc5361..882e3dca71 100644 --- a/runtime/environment.go +++ b/runtime/environment.go @@ -22,6 +22,7 @@ import ( "time" "go.opentelemetry.io/otel/attribute" + "golang.org/x/mod/semver" "github.com/onflow/cadence" "github.com/onflow/cadence/runtime/activations" @@ -185,19 +186,18 @@ func (e *interpreterEnvironment) newInterpreterConfig() *interpreter.Config { // and disable storage validation after each value modification. // Instead, storage is validated after commits (if validation is enabled), // see interpreterEnvironment.CommitStorage - AtreeStorageValidationEnabled: false, - Debugger: e.config.Debugger, - OnStatement: e.newOnStatementHandler(), - OnMeterComputation: e.newOnMeterComputation(), - OnFunctionInvocation: e.newOnFunctionInvocationHandler(), - OnInvokedFunctionReturn: e.newOnInvokedFunctionReturnHandler(), - CapabilityBorrowHandler: e.newCapabilityBorrowHandler(), - CapabilityCheckHandler: e.newCapabilityCheckHandler(), - LegacyContractUpgradeEnabled: e.config.LegacyContractUpgradeEnabled, - ContractUpdateTypeRemovalEnabled: e.config.ContractUpdateTypeRemovalEnabled, - ContractUpdateAttachmentBaseTypeChangeEnabled: e.config.ContractUpdateAttachmentBaseTypeChangeEnabled, - ValidateAccountCapabilitiesGetHandler: e.newValidateAccountCapabilitiesGetHandler(), - ValidateAccountCapabilitiesPublishHandler: e.newValidateAccountCapabilitiesPublishHandler(), + AtreeStorageValidationEnabled: false, + Debugger: e.config.Debugger, + OnStatement: e.newOnStatementHandler(), + OnMeterComputation: e.newOnMeterComputation(), + OnFunctionInvocation: e.newOnFunctionInvocationHandler(), + OnInvokedFunctionReturn: e.newOnInvokedFunctionReturnHandler(), + CapabilityBorrowHandler: e.newCapabilityBorrowHandler(), + CapabilityCheckHandler: e.newCapabilityCheckHandler(), + LegacyContractUpgradeEnabled: e.config.LegacyContractUpgradeEnabled, + ContractUpdateTypeRemovalEnabled: e.config.ContractUpdateTypeRemovalEnabled, + ValidateAccountCapabilitiesGetHandler: e.newValidateAccountCapabilitiesGetHandler(), + ValidateAccountCapabilitiesPublishHandler: e.newValidateAccountCapabilitiesPublishHandler(), } } @@ -211,8 +211,6 @@ func (e *interpreterEnvironment) newCheckerConfig() *sema.Config { ImportHandler: e.resolveImport, CheckHandler: e.newCheckHandler(), AttachmentsEnabled: e.config.AttachmentsEnabled, - AttachmentConformancesEnabled: e.config.AttachmentConformancesEnabled, - MemberSiblingTypeOverrideEnabled: e.config.MemberSiblingTypeOverrideEnabled, } } @@ -244,6 +242,8 @@ func (e *interpreterEnvironment) Configure( e.InterpreterConfig.Storage = storage e.coverageReport = coverageReport e.stackDepthLimiter.depth = 0 + + e.configureVersionedFeatures() } func (e *interpreterEnvironment) DeclareValue(valueDeclaration stdlib.StandardLibraryValue, location common.Location) { @@ -1461,3 +1461,20 @@ func (e *interpreterEnvironment) newValidateAccountCapabilitiesPublishHandler() return ok, err } } + +func (e *interpreterEnvironment) configureVersionedFeatures() { + var currentVersion string + errors.WrapPanic(func() { + currentVersion = e.runtimeInterface.CurrentVersion() + }) + enableOldBehaviour := useV101Behaviour(currentVersion) + + e.CheckerConfig.AttachmentConformancesEnabled = enableOldBehaviour + e.CheckerConfig.MemberSiblingTypeOverrideEnabled = enableOldBehaviour + e.InterpreterConfig.ContractUpdateAttachmentBaseTypeChangeEnabled = enableOldBehaviour +} + +func useV101Behaviour(currentVersion string) bool { + return currentVersion == "" || + semver.Compare(currentVersion, "v1.0.1") <= 0 +} diff --git a/runtime/interface.go b/runtime/interface.go index 9f20ba8f31..ea6ff29b4d 100644 --- a/runtime/interface.go +++ b/runtime/interface.go @@ -161,6 +161,8 @@ type Interface interface { path interpreter.PathValue, capabilityBorrowType *interpreter.ReferenceStaticType, ) (bool, error) + + CurrentVersion() string } type MeterInterface interface { diff --git a/runtime/runtime_test.go b/runtime/runtime_test.go index b5375fb799..f24973db89 100644 --- a/runtime/runtime_test.go +++ b/runtime/runtime_test.go @@ -11497,6 +11497,8 @@ func TestRuntimeBuiltInFunctionConfusion(t *testing.T) { address := common.MustBytesToAddress([]byte{0x1}) + var currentVersion string + newRuntimeInterface := func() Interface { accountCodes := map[common.AddressLocation][]byte{} @@ -11524,17 +11526,19 @@ func TestRuntimeBuiltInFunctionConfusion(t *testing.T) { OnProgramLog: func(message string) { loggedMessages = append(loggedMessages, message) }, + OnCurrentVersion: func() string { + return currentVersion + }, } } + runtime := NewTestInterpreterRuntime() + nextTransactionLocation := NewTransactionLocationGenerator() // Deploy contract without check enabled - err := NewTestInterpreterRuntimeWithConfig(Config{ - AtreeValidationEnabled: true, - MemberSiblingTypeOverrideEnabled: true, - }).ExecuteTransaction( + err := runtime.ExecuteTransaction( Script{ Source: DeploymentTransaction( "Foo", @@ -11550,10 +11554,9 @@ func TestRuntimeBuiltInFunctionConfusion(t *testing.T) { // Deploy contract with check enabled - err = NewTestInterpreterRuntimeWithConfig(Config{ - AtreeValidationEnabled: true, - MemberSiblingTypeOverrideEnabled: false, - }).ExecuteTransaction( + currentVersion = "v1.0.2" + + err = runtime.ExecuteTransaction( Script{ Source: DeploymentTransaction( "Foo", diff --git a/runtime/tests/runtime_utils/testinterface.go b/runtime/tests/runtime_utils/testinterface.go index e387075c54..b2fd4579dc 100644 --- a/runtime/tests/runtime_utils/testinterface.go +++ b/runtime/tests/runtime_utils/testinterface.go @@ -137,6 +137,7 @@ type TestRuntimeInterface struct { path interpreter.PathValue, capabilityBorrowType *interpreter.ReferenceStaticType, ) (bool, error) + OnCurrentVersion func() string lastUUID uint64 accountIDs map[common.Address]uint64 @@ -669,3 +670,10 @@ func (i *TestRuntimeInterface) ValidateAccountCapabilitiesPublish( capabilityBorrowType, ) } + +func (i *TestRuntimeInterface) CurrentVersion() string { + if i.OnCurrentVersion == nil { + return "" + } + return i.OnCurrentVersion() +} From 8ae61ac8bb3d739b9dcdbf6796576b7ecaf1c1ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Wed, 16 Oct 2024 14:14:45 -0700 Subject: [PATCH 09/14] rename new version function --- runtime/contract_update_validation_test.go | 2 +- runtime/empty.go | 4 ++-- runtime/environment.go | 6 +++--- runtime/interface.go | 2 +- runtime/runtime_test.go | 2 +- runtime/tests/runtime_utils/testinterface.go | 8 ++++---- 6 files changed, 12 insertions(+), 12 deletions(-) diff --git a/runtime/contract_update_validation_test.go b/runtime/contract_update_validation_test.go index 1fc995699b..ce0203ea11 100644 --- a/runtime/contract_update_validation_test.go +++ b/runtime/contract_update_validation_test.go @@ -116,7 +116,7 @@ func newContractDeploymentTransactorWithVersion(t *testing.T, config Config, ver events = append(events, event) return nil }, - OnCurrentVersion: func() string { + OnMinimumRequiredVersion: func() string { return version }, } diff --git a/runtime/empty.go b/runtime/empty.go index 103c318e90..89991e0539 100644 --- a/runtime/empty.go +++ b/runtime/empty.go @@ -261,6 +261,6 @@ func (EmptyRuntimeInterface) ValidateAccountCapabilitiesPublish( panic("unexpected call to ValidateAccountCapabilitiesPublish") } -func (EmptyRuntimeInterface) CurrentVersion() string { - panic("unexpected call to CurrentVersion") +func (EmptyRuntimeInterface) MinimumRequiredVersion() string { + panic("unexpected call to MinimumRequiredVersion") } diff --git a/runtime/environment.go b/runtime/environment.go index 882e3dca71..970bb4387f 100644 --- a/runtime/environment.go +++ b/runtime/environment.go @@ -1463,11 +1463,11 @@ func (e *interpreterEnvironment) newValidateAccountCapabilitiesPublishHandler() } func (e *interpreterEnvironment) configureVersionedFeatures() { - var currentVersion string + var minimumRequiredVersion string errors.WrapPanic(func() { - currentVersion = e.runtimeInterface.CurrentVersion() + minimumRequiredVersion = e.runtimeInterface.MinimumRequiredVersion() }) - enableOldBehaviour := useV101Behaviour(currentVersion) + enableOldBehaviour := useV101Behaviour(minimumRequiredVersion) e.CheckerConfig.AttachmentConformancesEnabled = enableOldBehaviour e.CheckerConfig.MemberSiblingTypeOverrideEnabled = enableOldBehaviour diff --git a/runtime/interface.go b/runtime/interface.go index ea6ff29b4d..f93ae5100a 100644 --- a/runtime/interface.go +++ b/runtime/interface.go @@ -162,7 +162,7 @@ type Interface interface { capabilityBorrowType *interpreter.ReferenceStaticType, ) (bool, error) - CurrentVersion() string + MinimumRequiredVersion() string } type MeterInterface interface { diff --git a/runtime/runtime_test.go b/runtime/runtime_test.go index f24973db89..5a91ab61a7 100644 --- a/runtime/runtime_test.go +++ b/runtime/runtime_test.go @@ -11526,7 +11526,7 @@ func TestRuntimeBuiltInFunctionConfusion(t *testing.T) { OnProgramLog: func(message string) { loggedMessages = append(loggedMessages, message) }, - OnCurrentVersion: func() string { + OnMinimumRequiredVersion: func() string { return currentVersion }, } diff --git a/runtime/tests/runtime_utils/testinterface.go b/runtime/tests/runtime_utils/testinterface.go index b2fd4579dc..6bf9fbc1d5 100644 --- a/runtime/tests/runtime_utils/testinterface.go +++ b/runtime/tests/runtime_utils/testinterface.go @@ -137,7 +137,7 @@ type TestRuntimeInterface struct { path interpreter.PathValue, capabilityBorrowType *interpreter.ReferenceStaticType, ) (bool, error) - OnCurrentVersion func() string + OnMinimumRequiredVersion func() string lastUUID uint64 accountIDs map[common.Address]uint64 @@ -671,9 +671,9 @@ func (i *TestRuntimeInterface) ValidateAccountCapabilitiesPublish( ) } -func (i *TestRuntimeInterface) CurrentVersion() string { - if i.OnCurrentVersion == nil { +func (i *TestRuntimeInterface) MinimumRequiredVersion() string { + if i.OnMinimumRequiredVersion == nil { return "" } - return i.OnCurrentVersion() + return i.OnMinimumRequiredVersion() } From 88dbd9d4cede299bfb567be291a2de7b14ca8e78 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Wed, 16 Oct 2024 15:59:08 -0700 Subject: [PATCH 10/14] update to latest version of golang.org/x/crypto --- go.mod | 13 +++++++------ go.sum | 31 ++++++++++++++++--------------- 2 files changed, 23 insertions(+), 21 deletions(-) diff --git a/go.mod b/go.mod index f650c26942..e3b766d7e3 100644 --- a/go.mod +++ b/go.mod @@ -22,10 +22,10 @@ require ( github.com/turbolent/prettier v0.0.0-20220320183459-661cc755135d go.opentelemetry.io/otel v1.8.0 go.uber.org/goleak v1.1.10 - golang.org/x/crypto v0.1.0 - golang.org/x/mod v0.14.0 - golang.org/x/text v0.4.0 - golang.org/x/tools v0.16.0 + golang.org/x/crypto v0.28.0 + golang.org/x/mod v0.17.0 + golang.org/x/text v0.19.0 + golang.org/x/tools v0.21.1-0.20240508182429-e35e4ccd0d2d golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 ) @@ -56,8 +56,9 @@ require ( github.com/zeebo/assert v1.3.0 // indirect github.com/zeebo/blake3 v0.2.3 // indirect golang.org/x/lint v0.0.0-20200302205851-738671d3881b // indirect - golang.org/x/sys v0.15.0 // indirect - golang.org/x/term v0.6.0 // indirect + golang.org/x/sync v0.8.0 // indirect + golang.org/x/sys v0.26.0 // indirect + golang.org/x/term v0.25.0 // indirect gonum.org/v1/gonum v0.6.1 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/go.sum b/go.sum index 2cb1deb2c9..40840eb0bb 100644 --- a/go.sum +++ b/go.sum @@ -23,8 +23,8 @@ github.com/fxamacker/cbor/v2 v2.4.1-0.20230228173756-c0c9f774e40c/go.mod h1:TA1x github.com/fxamacker/circlehash v0.3.0 h1:XKdvTtIJV9t7DDUtsf0RIpC1OcxZtPbmgIH7ekx28WA= github.com/fxamacker/circlehash v0.3.0/go.mod h1:3aq3OfVvsWtkWMb6A1owjOQFA+TLsD5FgJflnaQwtMM= github.com/golang/freetype v0.0.0-20170609003504-e2365dfdc4a0/go.mod h1:E/TSTwGwJL78qG/PmXZO1EjYhfJinVAhrmmHX6Z8B9k= -github.com/google/go-cmp v0.5.8 h1:e6P7q2lk1O+qJJb4BtCQXlK8vWEO8V1ZeuEdJNOqZyg= -github.com/google/go-cmp v0.5.8/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= +github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= +github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY= github.com/itchyny/gojq v0.12.14 h1:6k8vVtsrhQSYgSGg827AD+PVVaB1NLXEdX+dda2oZCc= github.com/itchyny/gojq v0.12.14/go.mod h1:y1G7oO7XkcR1LPZO59KyoCRy08T3j9vDYRV0GgYSS+s= github.com/itchyny/timefmt-go v0.1.5 h1:G0INE2la8S6ru/ZI5JecgyzbbJNs5lG1RcBqa7Jm6GE= @@ -121,8 +121,8 @@ go.uber.org/goleak v1.1.10 h1:z+mqJhf6ss6BSfSM671tgKyZBFPTTJM+HLxnhPC3wu0= go.uber.org/goleak v1.1.10/go.mod h1:8a7PlsEVH3e/a/GLqe5IIrQx6GzcnRmZEufDUTk4A7A= golang.org/x/crypto v0.0.0-20190308221718-c2843e01d9a2/go.mod h1:djNgcEr1/C05ACkg1iLfiJU5Ep61QUkGW8qpdssI0+w= golang.org/x/crypto v0.0.0-20191011191535-87dc89f01550/go.mod h1:yigFU9vqHzYiE8UmvKecakEJjdnWj3jj499lnFckfCI= -golang.org/x/crypto v0.1.0 h1:MDRAIl0xIo9Io2xV565hzXHw3zVseKrJKodhohM5CjU= -golang.org/x/crypto v0.1.0/go.mod h1:RecgLatLF4+eUMCP1PoPZQb+cVrJcOPbHkTkbkB9sbw= +golang.org/x/crypto v0.28.0 h1:GBDwsMXVQi34v5CCYUm2jkJvu4cbtru2U4TN2PSyQnw= +golang.org/x/crypto v0.28.0/go.mod h1:rmgy+3RHxRZMyY0jjAJShp2zgEdOqj2AO7U0pYmeQ7U= golang.org/x/exp v0.0.0-20180321215751-8460e604b9de/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/exp v0.0.0-20180807140117-3d87b88a115f/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= golang.org/x/exp v0.0.0-20190125153040-c74c464bbbf2/go.mod h1:CJ0aWSM057203Lf6IL+f9T1iT9GByDxfZKAQTCR3kQA= @@ -133,8 +133,8 @@ golang.org/x/lint v0.0.0-20190930215403-16217165b5de/go.mod h1:6SW0HCj/g11FgYtHl golang.org/x/lint v0.0.0-20200302205851-738671d3881b h1:Wh+f8QHJXR411sJR8/vRBTZ7YapZaRvUcLFFJhusH0k= golang.org/x/lint v0.0.0-20200302205851-738671d3881b/go.mod h1:3xt1FjdF8hUf6vQPIChWIBhFzV8gjjsPE/fR3IyQdNY= golang.org/x/mod v0.1.1-0.20191105210325-c90efee705ee/go.mod h1:QqPTAvyqsEbceGzBzNggFXnrqF1CaUcvgkdR5Ot7KZg= -golang.org/x/mod v0.14.0 h1:dGoOF9QVLYng8IHTm7BAyWqCqSheQ5pYWGhzW00YJr0= -golang.org/x/mod v0.14.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c= +golang.org/x/mod v0.17.0 h1:zY54UmvipHiNd+pm+m0x9KhZ9hl1/7QNMyxXbc6ICqA= +golang.org/x/mod v0.17.0/go.mod h1:hTbmBsO62+eylJbnUtE2MGJUyE7QWk4xUqPFrRgJ+7c= golang.org/x/net v0.0.0-20180218175443-cbe0f9307d01/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20181114220301-adae6a3d119a/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4= golang.org/x/net v0.0.0-20190311183353-d8887717615a/go.mod h1:t9HGtf8HONx5eT2rtn7q6eTqICYqUVnKs3thJo3Qplg= @@ -143,8 +143,8 @@ golang.org/x/net v0.0.0-20190620200207-3b0461eec859/go.mod h1:z5CRVTTTmAJ677TzLL golang.org/x/net v0.0.0-20191109021931-daa7c04131f5/go.mod h1:z5CRVTTTmAJ677TzLLGU+0bjPO0LkuOLi4/5GtJWs/s= golang.org/x/sync v0.0.0-20190423024810-112230192c58/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= golang.org/x/sync v0.0.0-20190911185100-cd5d95a43a6e/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM= -golang.org/x/sync v0.5.0 h1:60k92dhOjHxJkrqnwsfl8KuaHbn/5dl0lUPUklKo3qE= -golang.org/x/sync v0.5.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk= +golang.org/x/sync v0.8.0 h1:3NFvSEYkUoMifnESzZl15y791HH1qU2xm6eCJU5ZPXQ= +golang.org/x/sync v0.8.0/go.mod h1:Czt+wKu1gCyEFDUtn0jG5QVvpJ6rzVqr5aXyt9drQfk= golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190222072716-a9d3bda3a223/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY= golang.org/x/sys v0.0.0-20190412213103-97732733099d/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs= @@ -157,20 +157,21 @@ golang.org/x/sys v0.0.0-20200918174421-af09f7315aff/go.mod h1:h1NjWce9XRLGQEsW7w golang.org/x/sys v0.0.0-20220704084225-05e143d24a9e/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.0.0-20220811171246-fbc7d0a398ab/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.6.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= -golang.org/x/sys v0.15.0 h1:h48lPFYpsTvQJZF4EKyI4aLHaev3CxivZmv7yZig9pc= -golang.org/x/sys v0.15.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= -golang.org/x/term v0.6.0 h1:clScbb1cHjoCkyRbWwBEUZ5H/tIFu5TAXIqaZD0Gcjw= +golang.org/x/sys v0.26.0 h1:KHjCJyddX0LoSTb3J+vWpupP9p0oznkqVk/IfjymZbo= +golang.org/x/sys v0.26.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/term v0.6.0/go.mod h1:m6U89DPEgQRMq3DNkDClhWw02AUbt2daBVO4cn4Hv9U= +golang.org/x/term v0.25.0 h1:WtHI/ltw4NvSUig5KARz9h521QvRC8RmF/cuYqifU24= +golang.org/x/term v0.25.0/go.mod h1:RPyXicDX+6vLxogjjRxjgD2TKtmAO6NZBsBRfrOLu7M= golang.org/x/text v0.3.0/go.mod h1:NqM8EUOU14njkJ3fqMW+pc6Ldnwhi/IjpwHt7yyuwOQ= -golang.org/x/text v0.4.0 h1:BrVqGRd7+k1DiOgtnFvAkoQEWQvBc25ouMJM6429SFg= -golang.org/x/text v0.4.0/go.mod h1:mrYo+phRRbMaCq/xk9113O4dZlRixOauAjOtrjsXDZ8= +golang.org/x/text v0.19.0 h1:kTxAhCbGbxhK0IwgSKiMO5awPoDQ0RpfiVYBfK860YM= +golang.org/x/text v0.19.0/go.mod h1:BuEKDfySbSR4drPmRPG/7iBdf8hvFMuRexcpahXilzY= golang.org/x/tools v0.0.0-20180525024113-a5b4c53f6e8b/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20190206041539-40960b6deb8e/go.mod h1:n7NCudcB/nEzxVGmLbDWY5pfWTLqBcC2KZ6jyYvM4mQ= golang.org/x/tools v0.0.0-20190311212946-11955173bddd/go.mod h1:LCzVGOaR6xXOjkQ3onu1FJEFr0SW1gC7cKk1uF8kGRs= golang.org/x/tools v0.0.0-20191108193012-7d206e10da11/go.mod h1:b+2E5dAYhXwXZwtnZ6UAqBI28+e2cm9otk0dWdXHAEo= golang.org/x/tools v0.0.0-20200130002326-2f3ba24bd6e7/go.mod h1:TB2adYChydJhpapKDTa4BR/hXlZSLoq2Wpct/0txZ28= -golang.org/x/tools v0.16.0 h1:GO788SKMRunPIBCXiQyo2AaexLstOrVhuAL5YwsckQM= -golang.org/x/tools v0.16.0/go.mod h1:kYVVN6I1mBNoB1OX+noeBjbRk4IUEPa7JJ+TJMEooJ0= +golang.org/x/tools v0.21.1-0.20240508182429-e35e4ccd0d2d h1:vU5i/LfpvrRCpgM/VPfJLg5KjxD3E+hfT1SH+d9zLwg= +golang.org/x/tools v0.21.1-0.20240508182429-e35e4ccd0d2d/go.mod h1:aiJjzUbINMkxbQROHiO6hDPo2LHcIPhhQsa9DLh0yGk= golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0= golang.org/x/xerrors v0.0.0-20200804184101-5ec99f83aff1 h1:go1bK/D/BFZV2I8cIQd1NKEZ+0owSTG1fDTci4IqFcE= From 02cd8467ad86e777aea428f8ac4fc523ad23f6ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Wed, 23 Oct 2024 11:12:33 -0700 Subject: [PATCH 11/14] remove feature flags --- runtime/contract_update_validation_test.go | 49 +---------- runtime/environment.go | 12 +-- runtime/interpreter/config.go | 2 - runtime/runtime_test.go | 25 ------ runtime/sema/check_composite_declaration.go | 38 ++++---- runtime/sema/config.go | 4 - runtime/stdlib/account.go | 4 +- ..._v0.42_to_v1_contract_upgrade_validator.go | 11 --- runtime/stdlib/contract_update_validation.go | 56 +++++------- runtime/tests/checker/attachments_test.go | 86 +++---------------- 10 files changed, 55 insertions(+), 232 deletions(-) diff --git a/runtime/contract_update_validation_test.go b/runtime/contract_update_validation_test.go index ce0203ea11..8c6c11eb3c 100644 --- a/runtime/contract_update_validation_test.go +++ b/runtime/contract_update_validation_test.go @@ -3689,7 +3689,7 @@ func TestAttachmentsUpdates(t *testing.T) { t.Parallel() testWithValidators(t, - "Keep base type, v1.0.1", + "Keep base type", func(t *testing.T, config Config) { const oldCode = ` @@ -3704,34 +3704,13 @@ func TestAttachmentsUpdates(t *testing.T) { } ` - err := testDeployAndUpdateWithVersion(t, "Test", oldCode, newCode, config, "v1.0.1") - require.NoError(t, err) - }, - ) - - testWithValidators(t, - "Keep base type, v1.0.2", - func(t *testing.T, config Config) { - - const oldCode = ` - access(all) contract Test { - access(all) attachment A for AnyResource {} - } - ` - - const newCode = ` - access(all) contract Test { - access(all) attachment A for AnyResource {} - } - ` - - err := testDeployAndUpdateWithVersion(t, "Test", oldCode, newCode, config, "v1.0.2") + err := testDeployAndUpdate(t, "Test", oldCode, newCode, config) require.NoError(t, err) }, ) testWithValidators(t, - "Change base type, v1.0.1", + "Change base type", func(t *testing.T, config Config) { const oldCode = ` @@ -3746,27 +3725,7 @@ func TestAttachmentsUpdates(t *testing.T) { } ` - err := testDeployAndUpdateWithVersion(t, "Test", oldCode, newCode, config, "v1.0.1") - require.NoError(t, err) - }) - - testWithValidators(t, - "Change base type, v1.0.2", - func(t *testing.T, config Config) { - - const oldCode = ` - access(all) contract Test { - access(all) attachment A for AnyResource {} - } - ` - - const newCode = ` - access(all) contract Test { - access(all) attachment A for AnyStruct {} - } - ` - - err := testDeployAndUpdateWithVersion(t, "Test", oldCode, newCode, config, "v1.0.2") + err := testDeployAndUpdate(t, "Test", oldCode, newCode, config) var expectedErr *stdlib.TypeMismatchError require.ErrorAs(t, err, &expectedErr) diff --git a/runtime/environment.go b/runtime/environment.go index 970bb4387f..07c30cf23c 100644 --- a/runtime/environment.go +++ b/runtime/environment.go @@ -22,7 +22,6 @@ import ( "time" "go.opentelemetry.io/otel/attribute" - "golang.org/x/mod/semver" "github.com/onflow/cadence" "github.com/onflow/cadence/runtime/activations" @@ -1467,14 +1466,7 @@ func (e *interpreterEnvironment) configureVersionedFeatures() { errors.WrapPanic(func() { minimumRequiredVersion = e.runtimeInterface.MinimumRequiredVersion() }) - enableOldBehaviour := useV101Behaviour(minimumRequiredVersion) - e.CheckerConfig.AttachmentConformancesEnabled = enableOldBehaviour - e.CheckerConfig.MemberSiblingTypeOverrideEnabled = enableOldBehaviour - e.InterpreterConfig.ContractUpdateAttachmentBaseTypeChangeEnabled = enableOldBehaviour -} - -func useV101Behaviour(currentVersion string) bool { - return currentVersion == "" || - semver.Compare(currentVersion, "v1.0.1") <= 0 + // No feature flags yet + _ = minimumRequiredVersion } diff --git a/runtime/interpreter/config.go b/runtime/interpreter/config.go index f64413aef5..dc052342d1 100644 --- a/runtime/interpreter/config.go +++ b/runtime/interpreter/config.go @@ -74,8 +74,6 @@ type Config struct { LegacyContractUpgradeEnabled bool // ContractUpdateTypeRemovalEnabled specifies if type removal is enabled in contract updates ContractUpdateTypeRemovalEnabled bool - // ContractUpdateAttachmentBaseTypeChangeEnabled specifies if attachment base type change is enabled in contract updates - ContractUpdateAttachmentBaseTypeChangeEnabled bool // ValidateAccountCapabilitiesGetHandler is used to handle when a capability of an account is got. ValidateAccountCapabilitiesGetHandler ValidateAccountCapabilitiesGetHandlerFunc // ValidateAccountCapabilitiesPublishHandler is used to handle when a capability of an account is got. diff --git a/runtime/runtime_test.go b/runtime/runtime_test.go index 5a91ab61a7..c56c01d102 100644 --- a/runtime/runtime_test.go +++ b/runtime/runtime_test.go @@ -11497,8 +11497,6 @@ func TestRuntimeBuiltInFunctionConfusion(t *testing.T) { address := common.MustBytesToAddress([]byte{0x1}) - var currentVersion string - newRuntimeInterface := func() Interface { accountCodes := map[common.AddressLocation][]byte{} @@ -11526,9 +11524,6 @@ func TestRuntimeBuiltInFunctionConfusion(t *testing.T) { OnProgramLog: func(message string) { loggedMessages = append(loggedMessages, message) }, - OnMinimumRequiredVersion: func() string { - return currentVersion - }, } } @@ -11536,8 +11531,6 @@ func TestRuntimeBuiltInFunctionConfusion(t *testing.T) { nextTransactionLocation := NewTransactionLocationGenerator() - // Deploy contract without check enabled - err := runtime.ExecuteTransaction( Script{ Source: DeploymentTransaction( @@ -11550,24 +11543,6 @@ func TestRuntimeBuiltInFunctionConfusion(t *testing.T) { Location: nextTransactionLocation(), }, ) - require.NoError(t, err) - - // Deploy contract with check enabled - - currentVersion = "v1.0.2" - - err = runtime.ExecuteTransaction( - Script{ - Source: DeploymentTransaction( - "Foo", - []byte(contract), - ), - }, - Context{ - Interface: newRuntimeInterface(), - Location: nextTransactionLocation(), - }, - ) RequireError(t, err) var redeclarationError *sema.RedeclarationError diff --git a/runtime/sema/check_composite_declaration.go b/runtime/sema/check_composite_declaration.go index 75b8c3a8d6..0128333e90 100644 --- a/runtime/sema/check_composite_declaration.go +++ b/runtime/sema/check_composite_declaration.go @@ -687,29 +687,23 @@ func (checker *Checker) declareCompositeType(declaration ast.CompositeLikeDeclar case common.CompositeKindAttachment: - if checker.Config.AttachmentConformancesEnabled { - compositeType.ExplicitInterfaceConformances = - checker.explicitInterfaceConformances(declaration, compositeType) + // Attachments may not conform to interfaces - } else { - // Attachments may not conform to interfaces + conformanceList := declaration.ConformanceList() + conformanceCount := len(conformanceList) + if conformanceCount > 0 { + firstConformance := conformanceList[0] + lastConformance := conformanceList[conformanceCount-1] - conformanceList := declaration.ConformanceList() - conformanceCount := len(conformanceList) - if conformanceCount > 0 { - firstConformance := conformanceList[0] - lastConformance := conformanceList[conformanceCount-1] - - checker.report( - &InvalidAttachmentConformancesError{ - Range: ast.NewRange( - checker.memoryGauge, - firstConformance.StartPosition(), - lastConformance.EndPosition(checker.memoryGauge), - ), - }, - ) - } + checker.report( + &InvalidAttachmentConformancesError{ + Range: ast.NewRange( + checker.memoryGauge, + firstConformance.StartPosition(), + lastConformance.EndPosition(checker.memoryGauge), + ), + }, + ) } default: @@ -1748,7 +1742,7 @@ func (checker *Checker) defaultMembersAndOrigins( } } - if nestedTypes != nil && !checker.Config.MemberSiblingTypeOverrideEnabled { + if nestedTypes != nil { if _, ok := nestedTypes.Get(name); ok { // TODO: provide previous position checker.report( diff --git a/runtime/sema/config.go b/runtime/sema/config.go index c8e72c695e..06f6e9ce95 100644 --- a/runtime/sema/config.go +++ b/runtime/sema/config.go @@ -55,8 +55,4 @@ type Config struct { AllowStaticDeclarations bool // AttachmentsEnabled determines if attachments are enabled AttachmentsEnabled bool - // AttachmentConformancesEnabled determines if attachment conformances are enabled - AttachmentConformancesEnabled bool - // MemberSiblingTypeOverrideEnabled determines if declaring members checks for sibling type declarations - MemberSiblingTypeOverrideEnabled bool } diff --git a/runtime/stdlib/account.go b/runtime/stdlib/account.go index 235dc29e6f..14f20a18d3 100644 --- a/runtime/stdlib/account.go +++ b/runtime/stdlib/account.go @@ -1679,7 +1679,6 @@ func changeAccountContracts( memoryGauge := invocation.Interpreter.SharedState.Config.MemoryGauge legacyUpgradeEnabled := invocation.Interpreter.SharedState.Config.LegacyContractUpgradeEnabled contractUpdateTypeRemovalEnabled := invocation.Interpreter.SharedState.Config.ContractUpdateTypeRemovalEnabled - contractUpdateAttachmentBaseTypeChangeEnabled := invocation.Interpreter.SharedState.Config.ContractUpdateAttachmentBaseTypeChangeEnabled var oldProgram *ast.Program @@ -1733,8 +1732,7 @@ func changeAccountContracts( } validator = validator. - WithTypeRemovalEnabled(contractUpdateTypeRemovalEnabled). - WithAttachmentBaseTypeChangeEnabled(contractUpdateAttachmentBaseTypeChangeEnabled) + WithTypeRemovalEnabled(contractUpdateTypeRemovalEnabled) err = validator.Validate() handleContractUpdateError(err, newCode) diff --git a/runtime/stdlib/cadence_v0.42_to_v1_contract_upgrade_validator.go b/runtime/stdlib/cadence_v0.42_to_v1_contract_upgrade_validator.go index d3fc96209b..838c8ecdc3 100644 --- a/runtime/stdlib/cadence_v0.42_to_v1_contract_upgrade_validator.go +++ b/runtime/stdlib/cadence_v0.42_to_v1_contract_upgrade_validator.go @@ -89,17 +89,6 @@ func (validator *CadenceV042ToV1ContractUpdateValidator) WithTypeRemovalEnabled( return validator } -func (validator *CadenceV042ToV1ContractUpdateValidator) isAttachmentBaseTypeChangeEnabled() bool { - return validator.underlyingUpdateValidator.isAttachmentBaseTypeChangeEnabled() -} - -func (validator *CadenceV042ToV1ContractUpdateValidator) WithAttachmentBaseTypeChangeEnabled( - enabled bool, -) UpdateValidator { - validator.underlyingUpdateValidator.WithAttachmentBaseTypeChangeEnabled(enabled) - return validator -} - func (validator *CadenceV042ToV1ContractUpdateValidator) getCurrentDeclaration() ast.Declaration { return validator.underlyingUpdateValidator.getCurrentDeclaration() } diff --git a/runtime/stdlib/contract_update_validation.go b/runtime/stdlib/contract_update_validation.go index 0113ab439c..00cbad8076 100644 --- a/runtime/stdlib/contract_update_validation.go +++ b/runtime/stdlib/contract_update_validation.go @@ -53,9 +53,6 @@ type UpdateValidator interface { isTypeRemovalEnabled() bool WithTypeRemovalEnabled(enabled bool) UpdateValidator - - isAttachmentBaseTypeChangeEnabled() bool - WithAttachmentBaseTypeChangeEnabled(enabled bool) UpdateValidator } type checkConformanceFunc func( @@ -66,16 +63,15 @@ type checkConformanceFunc func( type ContractUpdateValidator struct { *TypeComparator - location common.Location - contractName string - oldProgram *ast.Program - newProgram *ast.Program - currentDecl ast.Declaration - importLocations map[ast.Identifier]common.Location - accountContractNamesProvider AccountContractNamesProvider - errors []error - typeRemovalEnabled bool - attachmentBaseTypeChangeEnabled bool + location common.Location + contractName string + oldProgram *ast.Program + newProgram *ast.Program + currentDecl ast.Declaration + importLocations map[ast.Identifier]common.Location + accountContractNamesProvider AccountContractNamesProvider + errors []error + typeRemovalEnabled bool } // ContractUpdateValidator should implement ast.TypeEqualityChecker @@ -112,15 +108,6 @@ func (validator *ContractUpdateValidator) WithTypeRemovalEnabled(enabled bool) U return validator } -func (validator *ContractUpdateValidator) isAttachmentBaseTypeChangeEnabled() bool { - return validator.attachmentBaseTypeChangeEnabled -} - -func (validator *ContractUpdateValidator) WithAttachmentBaseTypeChangeEnabled(enabled bool) UpdateValidator { - validator.attachmentBaseTypeChangeEnabled = enabled - return validator -} - func (validator *ContractUpdateValidator) getCurrentDeclaration() ast.Declaration { return validator.currentDecl } @@ -322,21 +309,18 @@ func checkDeclarationUpdatability( } } - if !validator.isAttachmentBaseTypeChangeEnabled() { - - // Check if the base type of the attachment has changed. - if oldAttachment, ok := oldDeclaration.(*ast.AttachmentDeclaration); ok && - oldAttachment.DeclarationKind() == common.DeclarationKindAttachment { + // Check if the base type of the attachment has changed. + if oldAttachment, ok := oldDeclaration.(*ast.AttachmentDeclaration); ok && + oldAttachment.DeclarationKind() == common.DeclarationKindAttachment { - // NOTE: no need to check declaration kinds match, already checked above - if newAttachment, ok := newDeclaration.(*ast.AttachmentDeclaration); ok { - err := typeComparator.CheckNominalTypeEquality( - oldAttachment.BaseType, - newAttachment.BaseType, - ) - if err != nil { - validator.report(err) - } + // NOTE: no need to check declaration kinds match, already checked above + if newAttachment, ok := newDeclaration.(*ast.AttachmentDeclaration); ok { + err := typeComparator.CheckNominalTypeEquality( + oldAttachment.BaseType, + newAttachment.BaseType, + ) + if err != nil { + validator.report(err) } } } diff --git a/runtime/tests/checker/attachments_test.go b/runtime/tests/checker/attachments_test.go index 6a890e0124..b1d6c7f646 100644 --- a/runtime/tests/checker/attachments_test.go +++ b/runtime/tests/checker/attachments_test.go @@ -623,96 +623,34 @@ func TestCheckInvalidAttachmentConformance(t *testing.T) { t.Parallel() - t.Run("struct, enabled", func(t *testing.T) { - - t.Parallel() - - _, err := ParseAndCheckWithOptions(t, - ` - struct S {} - - struct interface SI {} - - attachment Test for S: SI {} - `, - ParseAndCheckOptions{ - Config: &sema.Config{ - AttachmentsEnabled: true, - AttachmentConformancesEnabled: true, - }, - }, - ) - - require.NoError(t, err) - }) - - t.Run("struct, disabled", func(t *testing.T) { + t.Run("struct", func(t *testing.T) { t.Parallel() - _, err := ParseAndCheckWithOptions(t, - ` - struct S {} + _, err := ParseAndCheck(t, ` + struct S {} - struct interface SI {} + struct interface SI {} - attachment Test for S: SI {} - `, - ParseAndCheckOptions{ - Config: &sema.Config{ - AttachmentsEnabled: true, - AttachmentConformancesEnabled: false, - }, - }, - ) + attachment Test for S: SI {} + `) errs := RequireCheckerErrors(t, err, 1) assert.IsType(t, &sema.InvalidAttachmentConformancesError{}, errs[0]) }) - t.Run("resource, enabled", func(t *testing.T) { - - t.Parallel() - - _, err := ParseAndCheckWithOptions(t, - ` - resource R {} - - resource interface RI {} - - attachment Test for R: RI {} - `, - ParseAndCheckOptions{ - Config: &sema.Config{ - AttachmentsEnabled: true, - AttachmentConformancesEnabled: true, - }, - }, - ) - - require.NoError(t, err) - }) - - t.Run("resource, disabled", func(t *testing.T) { + t.Run("resource", func(t *testing.T) { t.Parallel() - _, err := ParseAndCheckWithOptions(t, - ` - resource R {} + _, err := ParseAndCheck(t, ` + resource R {} - resource interface RI {} + resource interface RI {} - attachment Test for R: RI {} - `, - ParseAndCheckOptions{ - Config: &sema.Config{ - AttachmentsEnabled: true, - AttachmentConformancesEnabled: false, - }, - }, - ) + attachment Test for R: RI {} + `) errs := RequireCheckerErrors(t, err, 1) From e996077a4d5d57a4526ea98b5a5521e0715d6810 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Fri, 25 Oct 2024 16:15:00 -0700 Subject: [PATCH 12/14] add error return value to MinimumRequiredVersion interface function --- runtime/contract_update_validation_test.go | 4 ++-- runtime/empty.go | 2 +- runtime/environment.go | 10 ++++++++-- runtime/interface.go | 2 +- runtime/tests/runtime_utils/testinterface.go | 6 +++--- 5 files changed, 15 insertions(+), 9 deletions(-) diff --git a/runtime/contract_update_validation_test.go b/runtime/contract_update_validation_test.go index 8c6c11eb3c..2a1cd279a8 100644 --- a/runtime/contract_update_validation_test.go +++ b/runtime/contract_update_validation_test.go @@ -116,8 +116,8 @@ func newContractDeploymentTransactorWithVersion(t *testing.T, config Config, ver events = append(events, event) return nil }, - OnMinimumRequiredVersion: func() string { - return version + OnMinimumRequiredVersion: func() (string, error) { + return version, nil }, } diff --git a/runtime/empty.go b/runtime/empty.go index 89991e0539..4e9c2b58b5 100644 --- a/runtime/empty.go +++ b/runtime/empty.go @@ -261,6 +261,6 @@ func (EmptyRuntimeInterface) ValidateAccountCapabilitiesPublish( panic("unexpected call to ValidateAccountCapabilitiesPublish") } -func (EmptyRuntimeInterface) MinimumRequiredVersion() string { +func (EmptyRuntimeInterface) MinimumRequiredVersion() (string, error) { panic("unexpected call to MinimumRequiredVersion") } diff --git a/runtime/environment.go b/runtime/environment.go index 07c30cf23c..f2607eb5bd 100644 --- a/runtime/environment.go +++ b/runtime/environment.go @@ -1462,10 +1462,16 @@ func (e *interpreterEnvironment) newValidateAccountCapabilitiesPublishHandler() } func (e *interpreterEnvironment) configureVersionedFeatures() { - var minimumRequiredVersion string + var ( + minimumRequiredVersion string + err error + ) errors.WrapPanic(func() { - minimumRequiredVersion = e.runtimeInterface.MinimumRequiredVersion() + minimumRequiredVersion, err = e.runtimeInterface.MinimumRequiredVersion() }) + if err != nil { + panic(err) + } // No feature flags yet _ = minimumRequiredVersion diff --git a/runtime/interface.go b/runtime/interface.go index f93ae5100a..d5bcad7b7c 100644 --- a/runtime/interface.go +++ b/runtime/interface.go @@ -162,7 +162,7 @@ type Interface interface { capabilityBorrowType *interpreter.ReferenceStaticType, ) (bool, error) - MinimumRequiredVersion() string + MinimumRequiredVersion() (string, error) } type MeterInterface interface { diff --git a/runtime/tests/runtime_utils/testinterface.go b/runtime/tests/runtime_utils/testinterface.go index 6bf9fbc1d5..b3ff850e03 100644 --- a/runtime/tests/runtime_utils/testinterface.go +++ b/runtime/tests/runtime_utils/testinterface.go @@ -137,7 +137,7 @@ type TestRuntimeInterface struct { path interpreter.PathValue, capabilityBorrowType *interpreter.ReferenceStaticType, ) (bool, error) - OnMinimumRequiredVersion func() string + OnMinimumRequiredVersion func() (string, error) lastUUID uint64 accountIDs map[common.Address]uint64 @@ -671,9 +671,9 @@ func (i *TestRuntimeInterface) ValidateAccountCapabilitiesPublish( ) } -func (i *TestRuntimeInterface) MinimumRequiredVersion() string { +func (i *TestRuntimeInterface) MinimumRequiredVersion() (string, error) { if i.OnMinimumRequiredVersion == nil { - return "" + return "", nil } return i.OnMinimumRequiredVersion() } From ee9735518ea0c0fbfd2b0fb01294c42bcfd815c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Mon, 28 Oct 2024 08:15:07 -0700 Subject: [PATCH 13/14] make EmptyRuntimeInterface.MinimumRequiredVersion a no-op --- runtime/empty.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runtime/empty.go b/runtime/empty.go index 4e9c2b58b5..19c6f089e4 100644 --- a/runtime/empty.go +++ b/runtime/empty.go @@ -262,5 +262,5 @@ func (EmptyRuntimeInterface) ValidateAccountCapabilitiesPublish( } func (EmptyRuntimeInterface) MinimumRequiredVersion() (string, error) { - panic("unexpected call to MinimumRequiredVersion") + return "", nil } From bc01ae190d6124c4903b9f3234cec70da18f9fd5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20M=C3=BCller?= Date: Mon, 28 Oct 2024 08:19:02 -0700 Subject: [PATCH 14/14] return a valid semver --- runtime/empty.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/runtime/empty.go b/runtime/empty.go index 19c6f089e4..8aedf0a7fe 100644 --- a/runtime/empty.go +++ b/runtime/empty.go @@ -262,5 +262,5 @@ func (EmptyRuntimeInterface) ValidateAccountCapabilitiesPublish( } func (EmptyRuntimeInterface) MinimumRequiredVersion() (string, error) { - return "", nil + return "0.0.0", nil }