Skip to content

Commit

Permalink
Add type requirement removal
Browse files Browse the repository at this point in the history
  • Loading branch information
SupunS committed Feb 15, 2024
1 parent 6f44a0a commit 0204d96
Show file tree
Hide file tree
Showing 4 changed files with 249 additions and 11 deletions.
21 changes: 21 additions & 0 deletions runtime/contract_update_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2319,6 +2319,27 @@ func TestRuntimeContractUpdateValidation(t *testing.T) {
assertMissingDeclarationError(t, childErrors[1], "B")
}
})

t.Run("Remove event", func(t *testing.T) {

t.Parallel()

const oldCode = `
access(all) contract Test {
access(all) event Foo()
access(all) event Bar()
}
`

const newCode = `
access(all) contract Test {
access(all) event Bar()
}
`

err := testDeployAndUpdate(t, "Test", oldCode, newCode)
require.NoError(t, err)
})
}

func assertContractRemovalError(t *testing.T, err error, name string) {
Expand Down
137 changes: 137 additions & 0 deletions runtime/stdlib/cadence_v0.42_to_v1_contract_upgrade_validation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -817,6 +817,38 @@ func TestContractUpgradeIntersectionFieldType(t *testing.T) {
assertFieldTypeMismatchError(t, cause, "Test", "a", "R", "{I}")
})

t.Run("change field type AnyResource restricted type", func(t *testing.T) {

t.Parallel()

const oldCode = `
pub contract Test {
pub resource interface I {}
pub resource R:I {}
pub var a: @AnyResource{I}
init() {
self.a <- create R()
}
}
`

const newCode = `
access(all) contract Test {
access(all) resource interface I {}
access(all) resource R:I {}
access(all) var a: @{I}
init() {
self.a <- create R()
}
}
`

err := testContractUpdate(t, oldCode, newCode)
require.NoError(t, err)
})

t.Run("change field type restricted type variable sized", func(t *testing.T) {

t.Parallel()
Expand Down Expand Up @@ -1263,3 +1295,108 @@ func TestContractUpgradeIntersectionFieldType(t *testing.T) {
assertFieldAuthorizationMismatchError(t, cause, "Test", "a", "E", "E, F")
})
}

func TestTypeRequirementRemoval(t *testing.T) {

t.Parallel()

t.Run("resource valid", func(t *testing.T) {

t.Parallel()

const oldCode = `
access(all) contract interface Test {
access(all) resource R {}
access(all) fun foo(r: @R)
}
`

const newCode = `
access(all) contract interface Test {
access(all) resource interface R {}
access(all) fun foo(r: @{R})
}
`

err := testContractUpdate(t, oldCode, newCode)
require.NoError(t, err)
})

t.Run("resource invalid", func(t *testing.T) {

t.Parallel()

const oldCode = `
access(all) contract interface Test {
access(all) resource R {}
access(all) fun foo(r: @R)
}
`

const newCode = `
access(all) contract interface Test {
access(all) struct interface R {}
access(all) fun foo(r: {R})
}
`

err := testContractUpdate(t, oldCode, newCode)
cause := getSingleContractUpdateErrorCause(t, err, "Test")
declKindChangeError := &stdlib.InvalidDeclarationKindChangeError{}
require.ErrorAs(t, cause, &declKindChangeError)
})

t.Run("struct valid", func(t *testing.T) {

t.Parallel()

const oldCode = `
access(all) contract interface Test {
access(all) struct S {}
access(all) fun foo(r: S)
}
`

const newCode = `
access(all) contract interface Test {
access(all) struct interface S {}
access(all) fun foo(r: {S})
}
`

err := testContractUpdate(t, oldCode, newCode)
require.NoError(t, err)
})

t.Run("struct invalid", func(t *testing.T) {

t.Parallel()

const oldCode = `
access(all) contract interface Test {
access(all) struct S {}
access(all) fun foo(r: S)
}
`

const newCode = `
access(all) contract interface Test {
access(all) resource interface S {}
access(all) fun foo(r: @{S})
}
`

err := testContractUpdate(t, oldCode, newCode)
cause := getSingleContractUpdateErrorCause(t, err, "Test")
declKindChangeError := &stdlib.InvalidDeclarationKindChangeError{}
require.ErrorAs(t, cause, &declKindChangeError)
})
}
59 changes: 59 additions & 0 deletions runtime/stdlib/cadence_v0.42_to_v1_contract_upgrade_validator.go
Original file line number Diff line number Diff line change
Expand Up @@ -257,6 +257,7 @@ func (validator *CadenceV042ToV1ContractUpdateValidator) checkEntitlementsUpgrad

func (validator *CadenceV042ToV1ContractUpdateValidator) checkTypeUpgradability(oldType ast.Type, newType ast.Type) error {

typeSwitch:
switch oldType := oldType.(type) {
case *ast.OptionalType:
if newOptional, isOptional := newType.(*ast.OptionalType); isOptional {
Expand All @@ -282,6 +283,19 @@ func (validator *CadenceV042ToV1ContractUpdateValidator) checkTypeUpgradability(
break
}
validator.currentRestrictedTypeUpgradeRestrictions = oldType.Types

// If the old restricted type is for AnyStruct/AnyResource,
// require them to drop the "restricted type".
// e.g: `T{I} -> {I}`
if restrictedNominalType, isNominal := oldType.LegacyRestrictedType.(*ast.NominalType); isNominal {
switch restrictedNominalType.Identifier.Identifier {
case "AnyStruct", "AnyResource":
break typeSwitch
}
}

// Otherwise require them to drop the "restriction".
// e.g: `T{I} -> T`
return validator.checkTypeUpgradability(oldType.LegacyRestrictedType, newType)

case *ast.VariableSizedType:
Expand Down Expand Up @@ -349,6 +363,51 @@ func (validator *CadenceV042ToV1ContractUpdateValidator) checkField(oldField *as
})
}

func (validator *CadenceV042ToV1ContractUpdateValidator) checkDeclarationKindChange(
oldDeclaration ast.Declaration,
newDeclaration ast.Declaration,
) bool {
// Do not allow converting between different types of composite declarations:
// e.g: - 'contracts' and 'contract-interfaces',
// - 'structs' and 'enums'
//
// However, with the removal of type requirements, it is OK to convert a
// concrete type (Struct or Resource) to an interface type (StructInterface or ResourceInterface).
// However, resource should stay a resource interface, and cannot be a struct interface.

oldDeclKind := oldDeclaration.DeclarationKind()
newDeclKind := newDeclaration.DeclarationKind()
if oldDeclKind == newDeclKind {
return true
}

parent := validator.getCurrentDeclaration()

// If the parent is an interface, and the child is a concrete type,
// then it is a type requirement.
if parent.DeclarationKind() == common.DeclarationKindContractInterface {
// A struct is OK to be converted to a struct-interface
if oldDeclKind == common.DeclarationKindStructure &&
newDeclKind == common.DeclarationKindStructureInterface {
return true
}

// A resource is OK to be converted to a resource-interface
if oldDeclKind == common.DeclarationKindResource &&
newDeclKind == common.DeclarationKindResourceInterface {
return true
}
}

validator.report(&InvalidDeclarationKindChangeError{
Name: oldDeclaration.DeclarationIdentifier().Identifier,
OldKind: oldDeclaration.DeclarationKind(),
NewKind: newDeclaration.DeclarationKind(),
Range: ast.NewUnmeteredRangeFromPositioned(newDeclaration.DeclarationIdentifier()),
})
return false
}

// AuthorizationMismatchError is reported during a contract upgrade,
// when a field value is given authorization that is more powerful
// than that which the migration would grant it
Expand Down
43 changes: 32 additions & 11 deletions runtime/stdlib/contract_update_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,11 @@ type UpdateValidator interface {

checkField(oldField *ast.FieldDeclaration, newField *ast.FieldDeclaration)
getAccountContractNames(address common.Address) ([]string, error)

checkDeclarationKindChange(
oldDeclaration ast.Declaration,
newDeclaration ast.Declaration,
) bool
}

type ContractUpdateValidator struct {
Expand Down Expand Up @@ -187,17 +192,7 @@ func checkDeclarationUpdatability(
newDeclaration ast.Declaration,
) {

// Do not allow converting between different types of composite declarations:
// e.g: - 'contracts' and 'contract-interfaces',
// - 'structs' and 'enums'
if oldDeclaration.DeclarationKind() != newDeclaration.DeclarationKind() {
validator.report(&InvalidDeclarationKindChangeError{
Name: oldDeclaration.DeclarationIdentifier().Identifier,
OldKind: oldDeclaration.DeclarationKind(),
NewKind: newDeclaration.DeclarationKind(),
Range: ast.NewUnmeteredRangeFromPositioned(newDeclaration.DeclarationIdentifier()),
})

if !validator.checkDeclarationKindChange(oldDeclaration, newDeclaration) {
return
}

Expand Down Expand Up @@ -260,6 +255,27 @@ func (validator *ContractUpdateValidator) checkField(oldField *ast.FieldDeclarat
}
}

func (validator *ContractUpdateValidator) checkDeclarationKindChange(
oldDeclaration ast.Declaration,
newDeclaration ast.Declaration,
) bool {
// Do not allow converting between different types of composite declarations:
// e.g: - 'contracts' and 'contract-interfaces',
// - 'structs' and 'enums'
if oldDeclaration.DeclarationKind() != newDeclaration.DeclarationKind() {
validator.report(&InvalidDeclarationKindChangeError{
Name: oldDeclaration.DeclarationIdentifier().Identifier,
OldKind: oldDeclaration.DeclarationKind(),
NewKind: newDeclaration.DeclarationKind(),
Range: ast.NewUnmeteredRangeFromPositioned(newDeclaration.DeclarationIdentifier()),
})

return false
}

return true
}

func checkNestedDeclarations(
validator UpdateValidator,
oldDeclaration ast.Declaration,
Expand Down Expand Up @@ -329,6 +345,11 @@ func checkNestedDeclarations(
})

for _, declaration := range missingDeclarations {
// OK to remove events - they are not stored
if declaration.DeclarationKind() == common.DeclarationKindEvent {
continue
}

validator.report(&MissingDeclarationError{
Name: declaration.DeclarationIdentifier().Identifier,
Kind: declaration.DeclarationKind(),
Expand Down

0 comments on commit 0204d96

Please sign in to comment.