diff --git a/interpreter/config.go b/interpreter/config.go index 1d921085f..d77bd667c 100644 --- a/interpreter/config.go +++ b/interpreter/config.go @@ -72,8 +72,6 @@ type Config struct { CapabilityBorrowHandler CapabilityBorrowHandlerFunc // LegacyContractUpgradeEnabled specifies whether to fall back to the old parser when attempting a contract upgrade LegacyContractUpgradeEnabled bool - // ContractUpdateTypeRemovalEnabled specifies if type removal is enabled in contract updates - ContractUpdateTypeRemovalEnabled bool // ValidateAccountCapabilitiesGetHandler is used to handle when a capability of an account is got. ValidateAccountCapabilitiesGetHandler ValidateAccountCapabilitiesGetHandlerFunc // ValidateAccountCapabilitiesPublishHandler is used to handle when a capability of an account is got. diff --git a/runtime/config.go b/runtime/config.go index c3be0083d..d6882cb35 100644 --- a/runtime/config.go +++ b/runtime/config.go @@ -37,6 +37,4 @@ type Config struct { CoverageReport *CoverageReport // LegacyContractUpgradeEnabled enabled specifies whether to use the old parser when parsing an old contract LegacyContractUpgradeEnabled bool - // ContractUpdateTypeRemovalEnabled specifies if type removal is enabled in contract updates - ContractUpdateTypeRemovalEnabled bool } diff --git a/runtime/contract_update_validation_test.go b/runtime/contract_update_validation_test.go index 74afc1c44..b4dd71def 100644 --- a/runtime/contract_update_validation_test.go +++ b/runtime/contract_update_validation_test.go @@ -177,31 +177,14 @@ func testWithValidatorsAndTypeRemovalEnabled( withC1Upgrade := withC1Upgrade name := name - for _, withTypeRemovalEnabled := range []bool{true, false} { - withTypeRemovalEnabled := withTypeRemovalEnabled - name := name - - switch { - case withC1Upgrade && withTypeRemovalEnabled: - name = fmt.Sprintf("%s (with C1 validator and type removal enabled)", name) - - case withC1Upgrade: - name = fmt.Sprintf("%s (with C1 validator)", name) - - case withTypeRemovalEnabled: - name = fmt.Sprintf("%s (with type removal enabled)", name) - } - - t.Run(name, func(t *testing.T) { - t.Parallel() + t.Run(name, func(t *testing.T) { + t.Parallel() - config := DefaultTestInterpreterConfig - config.LegacyContractUpgradeEnabled = withC1Upgrade - config.ContractUpdateTypeRemovalEnabled = withTypeRemovalEnabled + config := DefaultTestInterpreterConfig + config.LegacyContractUpgradeEnabled = withC1Upgrade - testFunc(t, config) - }) - } + testFunc(t, config) + }) } } @@ -3230,13 +3213,8 @@ func TestTypeRemovalPragmaUpdates(t *testing.T) { ` err := testDeployAndUpdate(t, "Test", oldCode, newCode, config) - - if config.ContractUpdateTypeRemovalEnabled { - var expectedErr *stdlib.TypeRemovalPragmaRemovalError - require.ErrorAs(t, err, &expectedErr) - } else { - require.NoError(t, err) - } + var expectedErr *stdlib.TypeRemovalPragmaRemovalError + require.ErrorAs(t, err, &expectedErr) }, ) @@ -3262,13 +3240,8 @@ func TestTypeRemovalPragmaUpdates(t *testing.T) { ` err := testDeployAndUpdate(t, "Test", oldCode, newCode, config) - - if config.ContractUpdateTypeRemovalEnabled { - var expectedErr *stdlib.TypeRemovalPragmaRemovalError - require.ErrorAs(t, err, &expectedErr) - } else { - require.NoError(t, err) - } + var expectedErr *stdlib.TypeRemovalPragmaRemovalError + require.ErrorAs(t, err, &expectedErr) }, ) @@ -3313,13 +3286,8 @@ func TestTypeRemovalPragmaUpdates(t *testing.T) { ` err := testDeployAndUpdate(t, "Test", oldCode, newCode, config) - - if config.ContractUpdateTypeRemovalEnabled { - var expectedErr *stdlib.InvalidTypeRemovalPragmaError - require.ErrorAs(t, err, &expectedErr) - } else { - require.NoError(t, err) - } + var expectedErr *stdlib.InvalidTypeRemovalPragmaError + require.ErrorAs(t, err, &expectedErr) }, ) @@ -3342,13 +3310,8 @@ func TestTypeRemovalPragmaUpdates(t *testing.T) { ` err := testDeployAndUpdate(t, "Test", oldCode, newCode, config) - - if config.ContractUpdateTypeRemovalEnabled { - var expectedErr *stdlib.InvalidTypeRemovalPragmaError - require.ErrorAs(t, err, &expectedErr) - } else { - require.NoError(t, err) - } + var expectedErr *stdlib.InvalidTypeRemovalPragmaError + require.ErrorAs(t, err, &expectedErr) }, ) @@ -3368,14 +3331,8 @@ func TestTypeRemovalPragmaUpdates(t *testing.T) { ` err := testDeployAndUpdate(t, "Test", oldCode, newCode, config) - - if config.ContractUpdateTypeRemovalEnabled { - - var expectedErr *stdlib.InvalidTypeRemovalPragmaError - require.ErrorAs(t, err, &expectedErr) - } else { - require.NoError(t, err) - } + var expectedErr *stdlib.InvalidTypeRemovalPragmaError + require.ErrorAs(t, err, &expectedErr) }, ) @@ -3395,13 +3352,8 @@ func TestTypeRemovalPragmaUpdates(t *testing.T) { ` err := testDeployAndUpdate(t, "Test", oldCode, newCode, config) - - if config.ContractUpdateTypeRemovalEnabled { - var expectedErr *stdlib.InvalidTypeRemovalPragmaError - require.ErrorAs(t, err, &expectedErr) - } else { - require.NoError(t, err) - } + var expectedErr *stdlib.InvalidTypeRemovalPragmaError + require.ErrorAs(t, err, &expectedErr) }, ) @@ -3422,13 +3374,7 @@ func TestTypeRemovalPragmaUpdates(t *testing.T) { ` err := testDeployAndUpdate(t, "Test", oldCode, newCode, config) - - if config.ContractUpdateTypeRemovalEnabled { - require.NoError(t, err) - } else { - var expectedErr *stdlib.MissingDeclarationError - require.ErrorAs(t, err, &expectedErr) - } + require.NoError(t, err) }, ) @@ -3451,13 +3397,7 @@ func TestTypeRemovalPragmaUpdates(t *testing.T) { ` err := testDeployAndUpdate(t, "Test", oldCode, newCode, config) - - if config.ContractUpdateTypeRemovalEnabled { - require.NoError(t, err) - } else { - var expectedErr *stdlib.MissingDeclarationError - require.ErrorAs(t, err, &expectedErr) - } + require.NoError(t, err) }, ) @@ -3526,13 +3466,7 @@ func TestTypeRemovalPragmaUpdates(t *testing.T) { ` err := testDeployAndUpdate(t, "Test", oldCode, newCode, config) - - if config.ContractUpdateTypeRemovalEnabled { - require.NoError(t, err) - } else { - var expectedErr *stdlib.MissingDeclarationError - require.ErrorAs(t, err, &expectedErr) - } + require.NoError(t, err) }, ) @@ -3574,13 +3508,8 @@ func TestTypeRemovalPragmaUpdates(t *testing.T) { ` err := testDeployAndUpdate(t, "Test", oldCode, newCode, config) - - if config.ContractUpdateTypeRemovalEnabled { - var expectedErr *stdlib.UseOfRemovedTypeError - require.ErrorAs(t, err, &expectedErr) - } else { - require.NoError(t, err) - } + var expectedErr *stdlib.UseOfRemovedTypeError + require.ErrorAs(t, err, &expectedErr) }, ) @@ -3602,13 +3531,8 @@ func TestTypeRemovalPragmaUpdates(t *testing.T) { ` err := testDeployAndUpdate(t, "Test", oldCode, newCode, config) - - if config.ContractUpdateTypeRemovalEnabled { - var expectedErr *stdlib.UseOfRemovedTypeError - require.ErrorAs(t, err, &expectedErr) - } else { - require.NoError(t, err) - } + var expectedErr *stdlib.UseOfRemovedTypeError + require.ErrorAs(t, err, &expectedErr) }, ) @@ -3630,13 +3554,8 @@ func TestTypeRemovalPragmaUpdates(t *testing.T) { ` err := testDeployAndUpdate(t, "Test", oldCode, newCode, config) - - if config.ContractUpdateTypeRemovalEnabled { - var expectedErr *stdlib.UseOfRemovedTypeError - require.ErrorAs(t, err, &expectedErr) - } else { - require.NoError(t, err) - } + var expectedErr *stdlib.UseOfRemovedTypeError + require.ErrorAs(t, err, &expectedErr) }, ) diff --git a/runtime/environment.go b/runtime/environment.go index ef3d0130d..d3624998a 100644 --- a/runtime/environment.go +++ b/runtime/environment.go @@ -196,7 +196,6 @@ func (e *interpreterEnvironment) newInterpreterConfig() *interpreter.Config { CapabilityBorrowHandler: e.newCapabilityBorrowHandler(), CapabilityCheckHandler: e.newCapabilityCheckHandler(), LegacyContractUpgradeEnabled: e.config.LegacyContractUpgradeEnabled, - ContractUpdateTypeRemovalEnabled: e.config.ContractUpdateTypeRemovalEnabled, ValidateAccountCapabilitiesGetHandler: e.newValidateAccountCapabilitiesGetHandler(), ValidateAccountCapabilitiesPublishHandler: e.newValidateAccountCapabilitiesPublishHandler(), } diff --git a/stdlib/account.go b/stdlib/account.go index 66907a6f9..97a8e4d0f 100644 --- a/stdlib/account.go +++ b/stdlib/account.go @@ -1678,7 +1678,6 @@ func changeAccountContracts( memoryGauge := invocation.Interpreter.SharedState.Config.MemoryGauge legacyUpgradeEnabled := invocation.Interpreter.SharedState.Config.LegacyContractUpgradeEnabled - contractUpdateTypeRemovalEnabled := invocation.Interpreter.SharedState.Config.ContractUpdateTypeRemovalEnabled var oldProgram *ast.Program @@ -1731,8 +1730,6 @@ func changeAccountContracts( ) } - validator = validator.WithTypeRemovalEnabled(contractUpdateTypeRemovalEnabled) - err = validator.Validate() handleContractUpdateError(err, newCode) } diff --git a/stdlib/cadence_v0.42_to_v1_contract_upgrade_validator.go b/stdlib/cadence_v0.42_to_v1_contract_upgrade_validator.go index 7b13c6491..921f0d103 100644 --- a/stdlib/cadence_v0.42_to_v1_contract_upgrade_validator.go +++ b/stdlib/cadence_v0.42_to_v1_contract_upgrade_validator.go @@ -75,10 +75,6 @@ func (validator *CadenceV042ToV1ContractUpdateValidator) Location() common.Locat return validator.underlyingUpdateValidator.location } -func (validator *CadenceV042ToV1ContractUpdateValidator) isTypeRemovalEnabled() bool { - return validator.underlyingUpdateValidator.isTypeRemovalEnabled() -} - func (validator *CadenceV042ToV1ContractUpdateValidator) WithUserDefinedTypeChangeChecker( typeChangeCheckFunc func(oldTypeID common.TypeID, newTypeID common.TypeID) (checked, valid bool), ) *CadenceV042ToV1ContractUpdateValidator { @@ -86,13 +82,6 @@ func (validator *CadenceV042ToV1ContractUpdateValidator) WithUserDefinedTypeChan return validator } -func (validator *CadenceV042ToV1ContractUpdateValidator) WithTypeRemovalEnabled( - enabled bool, -) UpdateValidator { - validator.underlyingUpdateValidator.WithTypeRemovalEnabled(enabled) - return validator -} - func (validator *CadenceV042ToV1ContractUpdateValidator) getCurrentDeclaration() ast.Declaration { return validator.underlyingUpdateValidator.getCurrentDeclaration() } diff --git a/stdlib/contract_update_validation.go b/stdlib/contract_update_validation.go index 71e958f84..82300d662 100644 --- a/stdlib/contract_update_validation.go +++ b/stdlib/contract_update_validation.go @@ -53,9 +53,6 @@ type UpdateValidator interface { oldDeclaration ast.Declaration, newDeclaration ast.Declaration, ) bool - - isTypeRemovalEnabled() bool - WithTypeRemovalEnabled(enabled bool) UpdateValidator } type checkConformanceFunc func( @@ -74,7 +71,6 @@ type ContractUpdateValidator struct { importLocations map[ast.Identifier]common.Location accountContractNamesProvider AccountContractNamesProvider errors []error - typeRemovalEnabled bool } // ContractUpdateValidator should implement ast.TypeEqualityChecker @@ -106,15 +102,6 @@ func (validator *ContractUpdateValidator) Location() common.Location { return validator.location } -func (validator *ContractUpdateValidator) isTypeRemovalEnabled() bool { - return validator.typeRemovalEnabled -} - -func (validator *ContractUpdateValidator) WithTypeRemovalEnabled(enabled bool) UpdateValidator { - validator.typeRemovalEnabled = enabled - return validator -} - func (validator *ContractUpdateValidator) getCurrentDeclaration() ast.Declaration { return validator.currentDecl } @@ -412,12 +399,10 @@ func (validator *ContractUpdateValidator) checkNestedDeclarationRemoval( return } - if validator.typeRemovalEnabled { - // OK to remove a type if it is included in a #removedType pragma, and it is not an interface - if removedTypes.Contains(nestedDeclaration.DeclarationIdentifier().Identifier) && - !declarationKind.IsInterfaceDeclaration() { - return - } + // OK to remove a type if it is included in a #removedType pragma, and it is not an interface + if removedTypes.Contains(nestedDeclaration.DeclarationIdentifier().Identifier) && + !declarationKind.IsInterfaceDeclaration() { + return } validator.report(&MissingDeclarationError{ @@ -443,10 +428,6 @@ func checkTypeNotRemoved( newDeclaration ast.Declaration, removedTypes *orderedmap.OrderedMap[string, struct{}], ) { - if !validator.isTypeRemovalEnabled() { - return - } - if removedTypes.Contains(newDeclaration.DeclarationIdentifier().Identifier) { validator.report(&UseOfRemovedTypeError{ Declaration: newDeclaration, @@ -463,33 +444,32 @@ func checkNestedDeclarations( ) { var removedTypes *orderedmap.OrderedMap[string, struct{}] - if validator.isTypeRemovalEnabled() { - // process pragmas first, as they determine whether types can later be removed - oldRemovedTypes := collectRemovedTypePragmas( - validator, - oldDeclaration.DeclarationMembers().Pragmas(), - // Do not report errors for pragmas in the old code. - // We are only interested in collecting the pragmas in old code. - // This also avoid reporting mixed errors from both old and new codes. - false, - ) - - removedTypes = collectRemovedTypePragmas( - validator, - newDeclaration.DeclarationMembers().Pragmas(), - true, - ) - - // #typeRemoval pragmas cannot be removed, so any that appear in the old program must appear in the new program - // they can however, be added, so use the new program's type removals for the purposes of checking the upgrade - oldRemovedTypes.Foreach(func(oldRemovedType string, _ struct{}) { - if !removedTypes.Contains(oldRemovedType) { - validator.report(&TypeRemovalPragmaRemovalError{ - RemovedType: oldRemovedType, - }) - } - }) - } + + // process pragmas first, as they determine whether types can later be removed + oldRemovedTypes := collectRemovedTypePragmas( + validator, + oldDeclaration.DeclarationMembers().Pragmas(), + // Do not report errors for pragmas in the old code. + // We are only interested in collecting the pragmas in old code. + // This also avoid reporting mixed errors from both old and new codes. + false, + ) + + removedTypes = collectRemovedTypePragmas( + validator, + newDeclaration.DeclarationMembers().Pragmas(), + true, + ) + + // #typeRemoval pragmas cannot be removed, so any that appear in the old program must appear in the new program + // they can however, be added, so use the new program's type removals for the purposes of checking the upgrade + oldRemovedTypes.Foreach(func(oldRemovedType string, _ struct{}) { + if !removedTypes.Contains(oldRemovedType) { + validator.report(&TypeRemovalPragmaRemovalError{ + RemovedType: oldRemovedType, + }) + } + }) oldNominalTypeDecls := getNestedNominalTypeDecls(oldDeclaration)