From 66fbd3e44a3a461a1237c84cfb35189a84bb235b Mon Sep 17 00:00:00 2001 From: Daniel Sainati Date: Tue, 26 Mar 2024 12:44:18 -0400 Subject: [PATCH 1/2] allow removal of enum from contract interface --- ..._to_v1_contract_upgrade_validation_test.go | 55 +++++++++++++++++++ ..._v0.42_to_v1_contract_upgrade_validator.go | 21 +++++++ runtime/stdlib/contract_update_validation.go | 37 +++++++++---- 3 files changed, 101 insertions(+), 12 deletions(-) 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 4a4290fd9d..b64712bad9 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 @@ -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) @@ -2066,3 +2073,51 @@ 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) + + // This is not allowed because `@R{I}` is converted to `@R`, not `@{I}` + 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) + }) + +} 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 5ce8350a9a..459d4aeb8c 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 @@ -533,6 +533,27 @@ func (validator *CadenceV042ToV1ContractUpdateValidator) checkDeclarationKindCha return false } +func (validator *CadenceV042ToV1ContractUpdateValidator) checkNestedDeclarationRemoval( + 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, diff --git a/runtime/stdlib/contract_update_validation.go b/runtime/stdlib/contract_update_validation.go index 44577cd082..e24eb997fe 100644 --- a/runtime/stdlib/contract_update_validation.go +++ b/runtime/stdlib/contract_update_validation.go @@ -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( @@ -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, @@ -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. From 74f913096c416b1a43b5a2e0c71e47750b054bf6 Mon Sep 17 00:00:00 2001 From: Daniel Sainati Date: Tue, 26 Mar 2024 16:42:05 -0400 Subject: [PATCH 2/2] Update runtime/stdlib/cadence_v0.42_to_v1_contract_upgrade_validation_test.go Co-authored-by: Supun Setunga --- .../cadence_v0.42_to_v1_contract_upgrade_validation_test.go | 1 - 1 file changed, 1 deletion(-) 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 b64712bad9..754a283abb 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 @@ -2094,7 +2094,6 @@ func TestContractUpgradeRemoveEnum(t *testing.T) { err := testContractUpdate(t, oldCode, newCode) - // This is not allowed because `@R{I}` is converted to `@R`, not `@{I}` cause := getSingleContractUpdateErrorCause(t, err, "Test") assertMissingDeclarationError(t, cause, "E") })