Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Allow removal of enum declarations from contract interfaces for C1.0 upgrade #3197

Merged
merged 2 commits into from
Mar 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,13 @@ func getSingleContractUpdateErrorCause(t *testing.T, err error, contractName str
return updateErr.Errors[0]
}

func assertMissingDeclarationError(t *testing.T, err error, declName string) bool {
var missingDeclError *stdlib.MissingDeclarationError
require.ErrorAs(t, err, &missingDeclError)

return assert.Equal(t, declName, missingDeclError.Name)
}

func getContractUpdateError(t *testing.T, err error, contractName string) *stdlib.ContractUpdateError {
require.Error(t, err)

Expand Down Expand Up @@ -2066,3 +2073,50 @@ func TestInterfaceConformanceChange(t *testing.T) {
require.NoError(t, err)
})
}

func TestContractUpgradeRemoveEnum(t *testing.T) {

t.Parallel()

t.Run("remove from contract", func(t *testing.T) {

t.Parallel()

const oldCode = `
pub contract Test {
pub enum E: UInt {}
}
`

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

err := testContractUpdate(t, oldCode, newCode)

cause := getSingleContractUpdateErrorCause(t, err, "Test")
assertMissingDeclarationError(t, cause, "E")
})

t.Run("remove from contract interface", func(t *testing.T) {

t.Parallel()

const oldCode = `
pub contract interface Test {
pub enum E: UInt {}
}
`

const newCode = `
access(all) contract interface Test {

}
`

err := testContractUpdate(t, oldCode, newCode)

require.NoError(t, err)
})

}
Original file line number Diff line number Diff line change
Expand Up @@ -533,6 +533,27 @@ func (validator *CadenceV042ToV1ContractUpdateValidator) checkDeclarationKindCha
return false
}

func (validator *CadenceV042ToV1ContractUpdateValidator) checkNestedDeclarationRemoval(
dsainati1 marked this conversation as resolved.
Show resolved Hide resolved
nestedDeclaration ast.Declaration,
oldContainingDeclaration ast.Declaration,
newContainingDeclaration ast.Declaration,
) {

// enums can be removed from contract interfaces, as they have no interface equivalent and are not
// actually used in field type annotations in any contracts
if oldContainingDeclaration.DeclarationKind() == common.DeclarationKindContractInterface &&
newContainingDeclaration.DeclarationKind() == common.DeclarationKindContractInterface &&
nestedDeclaration.DeclarationKind() == common.DeclarationKindEnum {
return
}

validator.underlyingUpdateValidator.checkNestedDeclarationRemoval(
nestedDeclaration,
oldContainingDeclaration,
newContainingDeclaration,
)
}

func (validator *CadenceV042ToV1ContractUpdateValidator) checkConformanceV1(
oldDecl *ast.CompositeDeclaration,
newDecl *ast.CompositeDeclaration,
Expand Down
37 changes: 25 additions & 12 deletions runtime/stdlib/contract_update_validation.go
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,11 @@ type UpdateValidator interface {
setCurrentDeclaration(ast.Declaration)

checkField(oldField *ast.FieldDeclaration, newField *ast.FieldDeclaration)
checkNestedDeclarationRemoval(
nestedDeclaration ast.Declaration,
oldContainingDeclaration ast.Declaration,
newContainingDeclaration ast.Declaration,
)
getAccountContractNames(address common.Address) ([]string, error)

checkDeclarationKindChange(
Expand Down Expand Up @@ -300,6 +305,25 @@ func (validator *ContractUpdateValidator) checkDeclarationKindChange(
return true
}

func (validator *ContractUpdateValidator) checkNestedDeclarationRemoval(
nestedDeclaration ast.Declaration,
_ ast.Declaration,
newContainingDeclaration ast.Declaration,
) {
// OK to remove events - they are not stored
if nestedDeclaration.DeclarationKind() == common.DeclarationKindEvent {
return
}

validator.report(&MissingDeclarationError{
Name: nestedDeclaration.DeclarationIdentifier().Identifier,
Kind: nestedDeclaration.DeclarationKind(),
Range: ast.NewUnmeteredRangeFromPositioned(
newContainingDeclaration.DeclarationIdentifier(),
),
})
}

func checkNestedDeclarations(
validator UpdateValidator,
oldDeclaration ast.Declaration,
Expand Down Expand Up @@ -370,18 +394,7 @@ 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(),
Range: ast.NewUnmeteredRangeFromPositioned(
newDeclaration.DeclarationIdentifier(),
),
})
validator.checkNestedDeclarationRemoval(declaration, oldDeclaration, newDeclaration)
}

// Check enum-cases, if there are any.
Expand Down
Loading