From 9bc54ff205d43423519015b2f43ac9065725bb35 Mon Sep 17 00:00:00 2001 From: Supun Setunga Date: Wed, 14 Feb 2024 20:00:54 +0530 Subject: [PATCH] Revert allowing to remove interface conformance --- runtime/contract_update_validation_test.go | 5 +- ..._to_v1_contract_upgrade_validation_test.go | 35 ++++++++++++++ runtime/stdlib/contract_update_validation.go | 47 +++++++++++++------ 3 files changed, 71 insertions(+), 16 deletions(-) diff --git a/runtime/contract_update_validation_test.go b/runtime/contract_update_validation_test.go index d66b07f89..5834a8d76 100644 --- a/runtime/contract_update_validation_test.go +++ b/runtime/contract_update_validation_test.go @@ -2558,7 +2558,10 @@ func TestRuntimeContractUpdateConformanceChanges(t *testing.T) { ` err := testDeployAndUpdate(t, "Test", oldCode, newCode) - require.NoError(t, err) + RequireError(t, err) + + cause := getSingleContractUpdateErrorCause(t, err, "Test") + assertConformanceMismatchError(t, cause, "Foo") }) t.Run("Change conformance order", func(t *testing.T) { diff --git a/runtime/stdlib/cadence_v0.42_to_v1_contract_upgrade_validation_test.go b/runtime/stdlib/cadence_v0.42_to_v1_contract_upgrade_validation_test.go index e4968d9b8..dc6935e47 100644 --- a/runtime/stdlib/cadence_v0.42_to_v1_contract_upgrade_validation_test.go +++ b/runtime/stdlib/cadence_v0.42_to_v1_contract_upgrade_validation_test.go @@ -528,6 +528,41 @@ func TestContractUpgradeFieldType(t *testing.T) { require.NoError(t, err) }) + + t.Run("change field types illegally", func(t *testing.T) { + + t.Parallel() + + const importCode = ` + access(all) contract interface CI {} + ` + + const oldCode = ` + import CI from 0x01 + + access(all) contract Test { + access(all) var a: Capability? + init() { + self.a = nil + } + } + ` + + const newCode = ` + import CI from 0x01 + + access(all) contract Test { + access(all) var a: Capability<{CI}>? + init() { + self.a = nil + } + } + ` + + err := testContractUpdateWithImports(t, oldCode, importCode, newCode, importCode) + + require.NoError(t, err) + }) } func TestContractUpgradeIntersectionAuthorization(t *testing.T) { diff --git a/runtime/stdlib/contract_update_validation.go b/runtime/stdlib/contract_update_validation.go index f90a27ba5..4eb0e0333 100644 --- a/runtime/stdlib/contract_update_validation.go +++ b/runtime/stdlib/contract_update_validation.go @@ -416,29 +416,46 @@ func checkConformance( newDecl *ast.CompositeDeclaration, ) { - // at this point the declaration kinds are known to be the same - if oldDecl.DeclarationKind() != common.DeclarationKindEnum { - return - } - // Here it is assumed enums will always have one and only one conformance. // This is enforced by the checker. // Therefore, below check for multiple conformances is only applicable // for non-enum type composite declarations. i.e: structs, resources, etc. - oldConformance := oldDecl.Conformances[0] - newConformance := newDecl.Conformances[0] + oldConformances := oldDecl.Conformances + newConformances := newDecl.Conformances + + // All the existing conformances must have a match. Order is not important. + // Having extra new conformance is OK. See: https://github.com/onflow/cadence/issues/1394 + + // Note: Removing a conformance is NOT OK. That could lead to type-safety issues. + // e.g: + // - Someone stores an array of type `[{I}]` with `T:I` objects inside. + // - Later T’s conformance to `I` is removed. + // - Now `[{I}]` contains objects if `T` that does not conform to `I`. + + for _, oldConformance := range oldConformances { + found := false + for index, newConformance := range newConformances { + err := oldConformance.CheckEqual(newConformance, validator) + if err == nil { + found = true + + // Remove the matched conformance, so we don't have to check it again. + // i.e: optimization + newConformances = append(newConformances[:index], newConformances[index+1:]...) + break + } + } - err := oldConformance.CheckEqual(newConformance, validator) + if !found { + validator.report(&ConformanceMismatchError{ + DeclName: newDecl.Identifier.Identifier, + Range: ast.NewUnmeteredRangeFromPositioned(newDecl.Identifier), + }) - if err == nil { - return + return + } } - - validator.report(&ConformanceMismatchError{ - DeclName: newDecl.Identifier.Identifier, - Range: ast.NewUnmeteredRangeFromPositioned(newDecl.Identifier), - }) } func (validator *ContractUpdateValidator) report(err error) {