From f514ac80fe3d7b5346c2e9cd1c471e520ddd8d01 Mon Sep 17 00:00:00 2001 From: Supun Setunga Date: Wed, 15 May 2024 11:51:59 -0700 Subject: [PATCH] Fix restricted type checking in contract-update-validator --- ..._to_v1_contract_upgrade_validation_test.go | 192 ++++++++++++++++++ ..._v0.42_to_v1_contract_upgrade_validator.go | 45 ++-- 2 files changed, 220 insertions(+), 17 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 f9a965ee08..d858f1e826 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 @@ -1090,6 +1090,198 @@ func TestContractUpgradeFieldType(t *testing.T) { err := testContractUpdate(t, oldCode, newCode) require.NoError(t, err) }) + + t.Run("custom type change, explicit AnyResource restricted type", func(t *testing.T) { + + t.Parallel() + + const oldCode = ` + import MetadataViews from 0x02 + + access(all) contract Test { + access(all) resource interface Foo {} + + access(all) var a: Capability<&AnyResource{MetadataViews.Resolver}>? + init() { + self.a = nil + } + } + ` + + const newImport = ` + access(all) contract ViewResolver { + access(all) resource interface Resolver {} + } + ` + + const newCode = ` + import ViewResolver from 0x02 + + access(all) contract Test { + access(all) resource interface Foo {} + + access(all) var a: Capability<&{ViewResolver.Resolver}>? + init() { + self.a = nil + } + } + ` + + const contractName = "Test" + location := common.AddressLocation{ + Name: contractName, + Address: common.MustBytesToAddress([]byte{0x1}), + } + + metadataViewsLocation := common.AddressLocation{ + Name: "MetadataViews", + Address: common.MustBytesToAddress([]byte{0x2}), + } + + viewResolverLocation := common.AddressLocation{ + Name: "ViewResolver", + Address: common.MustBytesToAddress([]byte{0x2}), + } + + imports := map[common.Location]string{ + viewResolverLocation: newImport, + } + + oldProgram, newProgram, elaborations := parseAndCheckPrograms(t, location, oldCode, newCode, imports) + + metadataViewsResolverTypeID := common.NewTypeIDFromQualifiedName( + nil, + metadataViewsLocation, + "MetadataViews.Resolver", + ) + + viewResolverResolverTypeID := common.NewTypeIDFromQualifiedName( + nil, + viewResolverLocation, + "ViewResolver.Resolver", + ) + + upgradeValidator := stdlib.NewCadenceV042ToV1ContractUpdateValidator( + location, + contractName, + &runtime_utils.TestRuntimeInterface{ + OnGetAccountContractNames: func(address runtime.Address) ([]string, error) { + return []string{"TestImport"}, nil + }, + }, + oldProgram, + newProgram, + elaborations, + ).WithUserDefinedTypeChangeChecker( + func(oldTypeID common.TypeID, newTypeID common.TypeID) (checked, valid bool) { + switch oldTypeID { + case metadataViewsResolverTypeID: + return true, newTypeID == viewResolverResolverTypeID + } + + return false, false + }, + ) + + err := upgradeValidator.Validate() + require.NoError(t, err) + }) + + t.Run("custom type change, implicit AnyResource restricted type", func(t *testing.T) { + + t.Parallel() + + const oldCode = ` + import MetadataViews from 0x02 + + access(all) contract Test { + access(all) resource interface Foo {} + + access(all) var a: Capability<&{MetadataViews.Resolver}>? + init() { + self.a = nil + } + } + ` + + const newImport = ` + access(all) contract ViewResolver { + access(all) resource interface Resolver {} + } + ` + + const newCode = ` + import ViewResolver from 0x02 + + access(all) contract Test { + access(all) resource interface Foo {} + + access(all) var a: Capability<&{ViewResolver.Resolver}>? + init() { + self.a = nil + } + } + ` + + const contractName = "Test" + location := common.AddressLocation{ + Name: contractName, + Address: common.MustBytesToAddress([]byte{0x1}), + } + + metadataViewsLocation := common.AddressLocation{ + Name: "MetadataViews", + Address: common.MustBytesToAddress([]byte{0x2}), + } + + viewResolverLocation := common.AddressLocation{ + Name: "ViewResolver", + Address: common.MustBytesToAddress([]byte{0x2}), + } + + imports := map[common.Location]string{ + viewResolverLocation: newImport, + } + + oldProgram, newProgram, elaborations := parseAndCheckPrograms(t, location, oldCode, newCode, imports) + + metadataViewsResolverTypeID := common.NewTypeIDFromQualifiedName( + nil, + metadataViewsLocation, + "MetadataViews.Resolver", + ) + + viewResolverResolverTypeID := common.NewTypeIDFromQualifiedName( + nil, + viewResolverLocation, + "ViewResolver.Resolver", + ) + + upgradeValidator := stdlib.NewCadenceV042ToV1ContractUpdateValidator( + location, + contractName, + &runtime_utils.TestRuntimeInterface{ + OnGetAccountContractNames: func(address runtime.Address) ([]string, error) { + return []string{"TestImport"}, nil + }, + }, + oldProgram, + newProgram, + elaborations, + ).WithUserDefinedTypeChangeChecker( + func(oldTypeID common.TypeID, newTypeID common.TypeID) (checked, valid bool) { + switch oldTypeID { + case metadataViewsResolverTypeID: + return true, newTypeID == viewResolverResolverTypeID + } + + return false, false + }, + ) + + err := upgradeValidator.Validate() + require.NoError(t, err) + }) } func TestContractUpgradeIntersectionAuthorization(t *testing.T) { 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 1a9a0d1658..b05311f968 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 @@ -401,12 +401,12 @@ var astFullyEntitledAccountReferenceType = &ast.ReferenceType{ func (validator *CadenceV042ToV1ContractUpdateValidator) checkTypeUpgradability(oldType ast.Type, newType ast.Type, inCapability bool) error { -typeSwitch: switch oldType := oldType.(type) { case *ast.OptionalType: if newOptional, isOptional := newType.(*ast.OptionalType); isOptional { return validator.checkTypeUpgradability(oldType.Type, newOptional.Type, inCapability) } + case *ast.ReferenceType: if newReference, isReference := newType.(*ast.ReferenceType); isReference { @@ -432,10 +432,12 @@ typeSwitch: return nil } } + case *ast.IntersectionType: // If the intersection type have doesn't a legacy restricted type, - // the interface set must be compatible. - if oldType.LegacyRestrictedType == nil { + // or if the legacy restricted type is AnyStruct/AnyResource (i.e: {T} or AnyStruct{T} or AnyResource{T}) + // then the interface set must be compatible. + if dropRestrictedType(oldType) { newIntersectionType, ok := newType.(*ast.IntersectionType) if !ok { return newTypeMismatchError(oldType, newType) @@ -483,20 +485,6 @@ typeSwitch: // they must be upgraded according to the migration rules: i.e. R{I} -> R validator.currentRestrictedTypeUpgradeRestrictions = oldType.Types - // If the old restricted type is for AnyStruct/AnyResource, - // and if there are atleast one restriction, require them to drop the "restricted type". - // e.g-1: `AnyStruct{I} -> {I}` - // e.g-2: `AnyResource{I} -> {I}` - // See: https://github.com/onflow/cadence/issues/3112 - if restrictedNominalType, isNominal := oldType.LegacyRestrictedType.(*ast.NominalType); isNominal { - switch restrictedNominalType.Identifier.Identifier { - case "AnyStruct", "AnyResource": - if len(oldType.Types) > 0 { - break typeSwitch - } - } - } - // Otherwise require them to drop the "restrictions". // e.g-1: `T{I} -> T` // e.g-2: `AnyStruct{} -> AnyStruct` @@ -506,6 +494,7 @@ typeSwitch: if newVariableSizedType, isVariableSizedType := newType.(*ast.VariableSizedType); isVariableSizedType { return validator.checkTypeUpgradability(oldType.Type, newVariableSizedType.Type, inCapability) } + case *ast.ConstantSizedType: if newConstantSizedType, isConstantSizedType := newType.(*ast.ConstantSizedType); isConstantSizedType { if oldType.Size.Value.Cmp(newConstantSizedType.Size.Value) != 0 || @@ -514,6 +503,7 @@ typeSwitch: } return validator.checkTypeUpgradability(oldType.Type, newConstantSizedType.Type, inCapability) } + case *ast.DictionaryType: if newDictionaryType, isDictionaryType := newType.(*ast.DictionaryType); isDictionaryType { err := validator.checkTypeUpgradability(oldType.KeyType, newDictionaryType.KeyType, inCapability) @@ -522,6 +512,7 @@ typeSwitch: } return validator.checkTypeUpgradability(oldType.ValueType, newDictionaryType.ValueType, inCapability) } + case *ast.InstantiationType: // if the type is a Capability, allow the borrow type to change according to the normal upgrade rules if oldNominalType, isNominal := oldType.Type.(*ast.NominalType); isNominal && @@ -628,6 +619,26 @@ typeSwitch: } +func dropRestrictedType(intersectionType *ast.IntersectionType) bool { + if intersectionType.LegacyRestrictedType == nil { + return true + } + + // If the old restricted type is for AnyStruct/AnyResource, + // and if there are atleast one restriction, require them to drop the "restricted type". + // e.g-1: `AnyStruct{I} -> {I}` + // e.g-2: `AnyResource{I} -> {I}` + // See: https://github.com/onflow/cadence/issues/3112 + if restrictedNominalType, isNominal := intersectionType.LegacyRestrictedType.(*ast.NominalType); isNominal { + switch restrictedNominalType.Identifier.Identifier { + case "AnyStruct", "AnyResource": + return len(intersectionType.Types) > 0 + } + } + + return false +} + func (validator *CadenceV042ToV1ContractUpdateValidator) checkUserDefinedTypeCustomRules( oldType ast.Type, newType ast.Type,