Skip to content

Commit

Permalink
Revert allowing to remove interface conformance
Browse files Browse the repository at this point in the history
  • Loading branch information
SupunS committed Feb 14, 2024
1 parent ecb5e3c commit 64f2274
Show file tree
Hide file tree
Showing 3 changed files with 71 additions and 21 deletions.
5 changes: 4 additions & 1 deletion runtime/contract_update_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<CI>?
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) {
Expand Down
52 changes: 32 additions & 20 deletions runtime/stdlib/contract_update_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -416,29 +416,41 @@ 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) {
Expand Down

0 comments on commit 64f2274

Please sign in to comment.