diff --git a/docs/modules/ROOT/pages/api-core.adoc b/docs/modules/ROOT/pages/api-core.adoc index d6c2ea887..ae6413edb 100644 --- a/docs/modules/ROOT/pages/api-core.adoc +++ b/docs/modules/ROOT/pages/api-core.adoc @@ -119,7 +119,7 @@ If any errors are found, the command will exit with a non-zero exit code and pri * `--contract ` The name or fully qualified name of the contract to validate. If not specified, all upgradeable contracts in the build info directory will be validated. * `--reference ` Can only be used when the `--contract` option is also provided. The name or fully qualified name of the reference contract to use for storage layout comparisons. If not specified, uses the `@custom:oz-upgrades-from` annotation if it is defined in the contract that is being validated. * `--requireReference` Can only be used when the `--contract` option is also provided. Not compatible with `--unsafeSkipStorageCheck`. If specified, requires either the `--reference` option to be provided or the contract to have a `@custom:oz-upgrades-from` annotation. -* `--unsafeAllow ""` - Selectively disable one or more validation errors. Comma-separated list with one or more of the following: `state-variable-assignment, state-variable-immutable, external-library-linking, struct-definition, enum-definition, constructor, delegatecall, selfdestruct, missing-public-upgradeto` +* `--unsafeAllow ""` - Selectively disable one or more validation errors. Comma-separated list with one or more of the following: `state-variable-assignment, state-variable-immutable, external-library-linking, struct-definition, enum-definition, constructor, delegatecall, selfdestruct, missing-public-upgradeto, internal-function-storage` * `--unsafeAllowRenames` - Configure storage layout check to allow variable renaming. * `--unsafeSkipStorageCheck` - Skips checking for storage layout compatibility errors. This is a dangerous option meant to be used as a last resort. diff --git a/docs/modules/ROOT/pages/api-hardhat-upgrades.adoc b/docs/modules/ROOT/pages/api-hardhat-upgrades.adoc index c6f215e61..1b951d5cd 100644 --- a/docs/modules/ROOT/pages/api-hardhat-upgrades.adoc +++ b/docs/modules/ROOT/pages/api-hardhat-upgrades.adoc @@ -16,6 +16,7 @@ The following options are common to some functions. ** `"constructor"`: Allows defining a constructor. See `constructorArgs`. ** `"delegatecall"`, `"selfdestruct"`: Allow the use of these operations. Incorrect use of this option can put funds at risk of permanent loss. See xref:faq.adoc#delegatecall-selfdestruct[Can I safely use `delegatecall` and `selfdestruct`?] ** `"missing-public-upgradeto"`: Allow UUPS implementations that do not contain a public `upgradeTo` or `upgradeToAndCall` function. Enabling this option is likely to cause a revert due to the built-in UUPS safety mechanism. +** `"internal-function-storage"`: Allow internal functions in storage variables. Internal functions are code pointers which will no longer be valid after an upgrade, so they must be reassigned during upgrades. See xref:faq.adoc#internal-function-storage[How can I use internal functions in storage variables?] * `unsafeAllowRenames`: (`boolean`) Configure storage layout check to allow variable renaming. * `unsafeSkipStorageCheck`: (`boolean`) upgrades the proxy or beacon without first checking for storage layout compatibility errors. This is a dangerous option meant to be used as a last resort. * `constructorArgs`: (`unknown[]`) Provide arguments for the constructor of the implementation contract. Note that these are different from initializer arguments, and will be used in the deployment of the implementation contract itself. Can be used to initialize immutable variables. diff --git a/docs/modules/ROOT/pages/faq.adoc b/docs/modules/ROOT/pages/faq.adoc index 74e36a68a..027912919 100644 --- a/docs/modules/ROOT/pages/faq.adoc +++ b/docs/modules/ROOT/pages/faq.adoc @@ -22,9 +22,7 @@ As a replacement for the constructor, it is common to set up an `initialize` fun [source,solidity] ---- -import "@openzeppelin/contracts/proxy/utils/Initializable.sol"; -// Alternatively, if you are using @openzeppelin/contracts-upgradeable: -// import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; +import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; contract MyContract is Initializable { uint256 value; @@ -50,6 +48,7 @@ Deployment and upgrade related functions come with an optional `opts` object, wh * `delegatecall` * `selfdestruct` * `missing-public-upgradeto` + * `internal-function-storage` This function is a generalized version of the original `unsafeAllowCustomTypes` and `unsafeAllowLinkedLibraries` allowing any check to be manually disabled. @@ -193,13 +192,14 @@ When using UUPS proxies (through the `kind: 'uups'` option), the implementation The recommended way to include one or both of these functions is by inheriting the `UUPSUpgradeable` contract provided in OpenZeppelin Contracts, as shown below. This contract adds the required function(s), but also contains a built-in mechanism that will check on-chain, at the time of an upgrade, that the new implementation proposed also inherits `UUPSUpgradeable` or implements the same interface. In this way, when using the Upgrades Plugins there are two layers of mitigations to prevent accidentally disabling upgradeability: an off-chain check by the plugins, and an on-chain fallback in the contract itself. -```solidity +[source,solidity] +---- import "@openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol"; contract MyContract is Initializable, ..., UUPSUpgradeable { ... } -``` +---- Read more about the differences with the Transparent Proxy Pattern in xref:contracts:api:proxy.adoc#transparent-vs-uups[Transparent vs UUPS]. @@ -217,7 +217,8 @@ Renaming a variable is disallowed by default because there is a chance that a re It is possible to disable this check by passing the option `unsafeAllowRenames: true`. A more granular approach is to use a docstring comment `/// @custom:oz-renamed-from ` right above the variable that is being renamed, for example: -``` +[source,solidity] +---- contract V1 { uint x; } @@ -225,11 +226,12 @@ contract V2 { /// @custom:oz-renamed-from x uint y; } -``` +---- Changing the type of a variable is not allowed either, even in cases where the types have the same size and alignment, for the similar reason explained above. As long as we can guarantee that the rest of the layout is not affected by this type change, it is also possible to override this check by placing a docstring comment `/// @custom:oz-retyped-from `. -``` +[source,solidity] +---- contract V1 { bool x; } @@ -237,6 +239,68 @@ contract V2 { /// @custom:oz-retyped-from bool uint8 x; } -``` +---- Docstring comments don't yet work for struct members, due to a current Solidity limitation. + +[[internal-function-storage]] +== How can I use internal functions in storage variables? + +Internal functions in storage variables are code pointers which will no longer be valid after an upgrade, because the code will move around and the pointer would change. To avoid this issue, you can declare those functions as external, or avoid code pointers in storage altogether and define an `enum` that you will use with a dispatcher function to select from the list of available functions. If you must use internal functions, those internal functions need to be reassigned during each upgrade. + +For example, the `messageFunction` variable in the following contract is not upgrade safe. Attempting to call `showMessage()` after an upgrade would likely result in a revert. +[source,solidity] +---- +import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; + +contract V1 is Initializable { + function() internal pure returns (string memory) messageFunction; + + function initialize() initializer public { + messageFunction = hello; + } + + function hello() internal pure returns (string memory) { + return "Hello, World!"; + } + + function showMessage() public view returns (string memory) { + return messageFunction(); + } + ... +} +---- + +To allow the above contract to be deployed by the Upgrades Plugins, you can disable the `internal-function-storage` check according to <>, but ensure you follow the steps below to reassign the internal function during upgrades. + +In new versions of this contract, assign the internal function in the storage variable again, for example by using a https://docs.openzeppelin.com/contracts/api/proxy#Initializable-reinitializer-uint64-[reinitializer]: +[source,solidity] +---- +import "@openzeppelin/contracts-upgradeable/proxy/utils/Initializable.sol"; +import "./V1.sol"; + +contract V2 is V1 { + function initializeV2() reinitializer(2) public { + messageFunction = hello; + } + ... +} +---- + +Then when upgrading, call the reinitializer function as part of the upgrade process, for example in Hardhat: +[source,ts] +---- +await upgrades.upgradeProxy(PROXY_ADDRESS, ContractFactoryV2, { + call: 'initializeV2', + unsafeAllow: ['internal-function-storage'] +}); +---- +or in Foundry: +[source,solidity] +---- +Upgrades.upgradeProxy( + PROXY_ADDRESS, + "V2.sol", + abi.encodeCall(V2.initializeV2, ()) +); +---- diff --git a/packages/core/CHANGELOG.md b/packages/core/CHANGELOG.md index 374713036..50bd237a3 100644 --- a/packages/core/CHANGELOG.md +++ b/packages/core/CHANGELOG.md @@ -1,5 +1,9 @@ # Changelog +## 1.34.0 (2024-06-12) + +- Fix storage layout comparison for function types, disallow internal functions in storage. ([#1032](https://github.com/OpenZeppelin/openzeppelin-upgrades/pull/1032)) + ## 1.33.1 (2024-04-25) - Fix Hardhat compile error when variable has whitespace before semicolon. ([#1020](https://github.com/OpenZeppelin/openzeppelin-upgrades/pull/1020)) diff --git a/packages/core/contracts/test/Storage.sol b/packages/core/contracts/test/Storage.sol index 2cab6e46a..446c8913c 100644 --- a/packages/core/contracts/test/Storage.sol +++ b/packages/core/contracts/test/Storage.sol @@ -899,4 +899,31 @@ contract V2Storage { } contract StorageUpgrade_ConsumeAndAddGap_Storage_V2 is V2Storage, Parent1, Parent2 { +} + +contract StorageUpgrade_FunctionPointer_V1 { + struct S { + uint256 a; + function(bool) external b; + } + S private s; + function(bool) external c; +} + +contract StorageUpgrade_FunctionPointer_V2_Ok { + struct S { + uint256 a; + function(bool) external b; + } + S private s; + function(bool) external c; +} + +contract StorageUpgrade_FunctionPointer_V2_Bad { + struct S { + uint256 a; + function(bool) b; + } + S private s; + function(bool) c; } \ No newline at end of file diff --git a/packages/core/contracts/test/Validations.sol b/packages/core/contracts/test/Validations.sol index cbec23c36..5e9856497 100644 --- a/packages/core/contracts/test/Validations.sol +++ b/packages/core/contracts/test/Validations.sol @@ -206,3 +206,73 @@ contract TransitiveLibraryIsUnsafe { DirectLibrary.f2(); } } + +contract StructExternalFunctionPointer { + struct S { + function(bool) external foo; + } +} + +contract StructInternalFunctionPointer { + struct S { + function(bool) internal foo; + } +} + +contract StructImpliedInternalFunctionPointer { + struct S { + function(bool) foo; + } +} + +struct StandaloneStructInternalFn { + function(bool) internal foo; +} + +contract UsesStandaloneStructInternalFn { + StandaloneStructInternalFn bad; +} + +contract StructUsesStandaloneStructInternalFn { + struct Bad { + StandaloneStructInternalFn bad; + } +} + +contract RecursiveStructInternalFn { + StructUsesStandaloneStructInternalFn.Bad bad; +} + +contract MappingRecursiveStructInternalFn { + mapping(address => mapping(address => StructUsesStandaloneStructInternalFn.Bad)) bad; +} + +contract ArrayRecursiveStructInternalFn { + StructUsesStandaloneStructInternalFn.Bad[][] bad; +} + +contract SelfRecursiveMappingStructInternalFn { + struct SelfRecursive { + mapping(address => SelfRecursive) selfReference; + mapping(address => StructUsesStandaloneStructInternalFn.Bad) bad; + } +} + +contract SelfRecursiveArrayStructInternalFn { + struct SelfRecursiveArray { + SelfRecursiveArray[] selfReference; + StructUsesStandaloneStructInternalFn.Bad[] bad; + } +} + +contract ExternalFunctionPointer { + function(bool) external foo; +} + +contract InternalFunctionPointer { + function(bool) internal foo; +} + +contract ImpliedInternalFunctionPointer { + function(bool) foo; +} diff --git a/packages/core/package.json b/packages/core/package.json index 8df81305f..c12e5b53c 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -1,6 +1,6 @@ { "name": "@openzeppelin/upgrades-core", - "version": "1.33.1", + "version": "1.34.0", "description": "", "repository": "https://github.com/OpenZeppelin/openzeppelin-upgrades/tree/master/packages/core", "license": "MIT", diff --git a/packages/core/src/cli/cli.test.ts.md b/packages/core/src/cli/cli.test.ts.md index b6ab145f9..86aa1f1ab 100644 --- a/packages/core/src/cli/cli.test.ts.md +++ b/packages/core/src/cli/cli.test.ts.md @@ -19,7 +19,7 @@ Generated by [AVA](https://avajs.dev). --contract The name or fully qualified name of the contract to validate. If not specified, all upgradeable contracts in the build info directory will be validated.␊ --reference Can only be used when the --contract option is also provided. The name or fully qualified name of the reference contract to use for storage layout comparisons. If not specified, uses the @custom:oz-upgrades-from annotation if it is defined in the contract that is being validated.␊ --requireReference Can only be used when the --contract option is also provided. Not compatible with --unsafeSkipStorageCheck. If specified, requires either the --reference option to be provided or the contract to have a @custom:oz-upgrades-from annotation.␊ - --unsafeAllow "" Selectively disable one or more validation errors. Comma-separated list with one or more of the following: state-variable-assignment, state-variable-immutable, external-library-linking, struct-definition, enum-definition, constructor, delegatecall, selfdestruct, missing-public-upgradeto␊ + --unsafeAllow "" Selectively disable one or more validation errors. Comma-separated list with one or more of the following: state-variable-assignment, state-variable-immutable, external-library-linking, struct-definition, enum-definition, constructor, delegatecall, selfdestruct, missing-public-upgradeto, internal-function-storage␊ --unsafeAllowRenames Configure storage layout check to allow variable renaming.␊ --unsafeSkipStorageCheck Skips checking for storage layout compatibility errors. This is a dangerous option meant to be used as a last resort.␊ ` @@ -39,7 +39,7 @@ Generated by [AVA](https://avajs.dev). --contract The name or fully qualified name of the contract to validate. If not specified, all upgradeable contracts in the build info directory will be validated.␊ --reference Can only be used when the --contract option is also provided. The name or fully qualified name of the reference contract to use for storage layout comparisons. If not specified, uses the @custom:oz-upgrades-from annotation if it is defined in the contract that is being validated.␊ --requireReference Can only be used when the --contract option is also provided. Not compatible with --unsafeSkipStorageCheck. If specified, requires either the --reference option to be provided or the contract to have a @custom:oz-upgrades-from annotation.␊ - --unsafeAllow "" Selectively disable one or more validation errors. Comma-separated list with one or more of the following: state-variable-assignment, state-variable-immutable, external-library-linking, struct-definition, enum-definition, constructor, delegatecall, selfdestruct, missing-public-upgradeto␊ + --unsafeAllow "" Selectively disable one or more validation errors. Comma-separated list with one or more of the following: state-variable-assignment, state-variable-immutable, external-library-linking, struct-definition, enum-definition, constructor, delegatecall, selfdestruct, missing-public-upgradeto, internal-function-storage␊ --unsafeAllowRenames Configure storage layout check to allow variable renaming.␊ --unsafeSkipStorageCheck Skips checking for storage layout compatibility errors. This is a dangerous option meant to be used as a last resort.␊ ` diff --git a/packages/core/src/cli/cli.test.ts.snap b/packages/core/src/cli/cli.test.ts.snap index 693babc30..4cc73afe1 100644 Binary files a/packages/core/src/cli/cli.test.ts.snap and b/packages/core/src/cli/cli.test.ts.snap differ diff --git a/packages/core/src/storage.test.ts b/packages/core/src/storage.test.ts index 5ca4499ac..89113aaea 100644 --- a/packages/core/src/storage.test.ts +++ b/packages/core/src/storage.test.ts @@ -937,3 +937,40 @@ test('storage upgrade with struct gap', t => { }, }); }); + +test('storage upgrade with function pointers', t => { + const v1 = t.context.extractStorageLayout('StorageUpgrade_FunctionPointer_V1'); + const v2_Ok = t.context.extractStorageLayout('StorageUpgrade_FunctionPointer_V2_Ok'); + const v2_Bad = t.context.extractStorageLayout('StorageUpgrade_FunctionPointer_V2_Bad'); + + t.deepEqual(getStorageUpgradeErrors(v1, v2_Ok), []); + + t.like(getStorageUpgradeErrors(v1, v2_Bad), { + length: 2, + 0: { + kind: 'typechange', + change: { + kind: 'struct members', + ops: { + length: 1, + 0: { + kind: 'typechange', + change: { + kind: 'visibility change', + }, + }, + }, + }, + original: { label: 's' }, + updated: { label: 's' }, + }, + 1: { + kind: 'typechange', + change: { + kind: 'visibility change', + }, + original: { label: 'c' }, + updated: { label: 'c' }, + }, + }); +}); diff --git a/packages/core/src/storage/compare.ts b/packages/core/src/storage/compare.ts index 2e3f67b4a..5ce260458 100644 --- a/packages/core/src/storage/compare.ts +++ b/packages/core/src/storage/compare.ts @@ -283,7 +283,7 @@ export class StorageLayoutComparator { updated: ParsedTypeDetailed, { allowAppend }: { allowAppend: boolean }, ): TypeChange | undefined { - if (updated.head.startsWith('t_function')) { + if (original.head.startsWith('t_function') && updated.head.startsWith('t_function')) { return this.getVisibilityChange(original, updated); } diff --git a/packages/core/src/validate.test.ts b/packages/core/src/validate.test.ts index fb9f2793a..8be4e7bf4 100644 --- a/packages/core/src/validate.test.ts +++ b/packages/core/src/validate.test.ts @@ -140,6 +140,29 @@ testValid('TransitiveLibraryIsUnsafe', 'transparent', false); testValid('contracts/test/ValidationsSameNameSafe.sol:SameName', 'transparent', true); testValid('contracts/test/ValidationsSameNameUnsafe.sol:SameName', 'transparent', false); +testValid('StructExternalFunctionPointer', 'transparent', true); +testValid('StructInternalFunctionPointer', 'transparent', false); +testValid('StructImpliedInternalFunctionPointer', 'transparent', false); +testOverride( + 'StructImpliedInternalFunctionPointer', + 'transparent', + { unsafeAllow: ['internal-function-storage'] }, + true, +); + +testValid('UsesStandaloneStructInternalFn', 'transparent', false); +testValid('StructUsesStandaloneStructInternalFn', 'transparent', false); +testValid('RecursiveStructInternalFn', 'transparent', false); +testValid('MappingRecursiveStructInternalFn', 'transparent', false); +testValid('ArrayRecursiveStructInternalFn', 'transparent', false); +testValid('SelfRecursiveMappingStructInternalFn', 'transparent', false); +testValid('SelfRecursiveArrayStructInternalFn', 'transparent', false); + +testValid('ExternalFunctionPointer', 'transparent', true); +testValid('InternalFunctionPointer', 'transparent', false); +testValid('ImpliedInternalFunctionPointer', 'transparent', false); +testOverride('ImpliedInternalFunctionPointer', 'transparent', { unsafeAllow: ['internal-function-storage'] }, true); + test('ambiguous name', t => { const error = t.throws(() => getContractVersion(t.context.validation, 'SameName')); t.is( diff --git a/packages/core/src/validate/overrides.ts b/packages/core/src/validate/overrides.ts index 6e9648114..3c0d1a508 100644 --- a/packages/core/src/validate/overrides.ts +++ b/packages/core/src/validate/overrides.ts @@ -76,6 +76,11 @@ export const ValidationErrorUnsafeMessages: Record { diff --git a/packages/core/src/validate/report.ts b/packages/core/src/validate/report.ts index 2b72726ce..44b42cf9a 100644 --- a/packages/core/src/validate/report.ts +++ b/packages/core/src/validate/report.ts @@ -64,6 +64,14 @@ const errorInfo: ErrorDescriptions = { ` @openzeppelin/contracts-upgradeable/proxy/utils/UUPSUpgradeable.sol`, link: 'https://zpl.in/upgrades/error-008', }, + 'internal-function-storage': { + msg: e => `Variable \`${e.name}\` is an internal function`, + hint: () => + `Use external functions or avoid functions in storage.\n` + + ` If you must use internal functions, skip this check with the \`unsafeAllow.internal-function-storage\`\n` + + ` flag and ensure you always reassign internal functions in storage during upgrades`, + link: 'https://zpl.in/upgrades/error-009', + }, }; function describeError(e: ValidationError, color = true): string { diff --git a/packages/core/src/validate/run.ts b/packages/core/src/validate/run.ts index ea97b1816..2565314da 100644 --- a/packages/core/src/validate/run.ts +++ b/packages/core/src/validate/run.ts @@ -1,6 +1,12 @@ import { Node } from 'solidity-ast/node'; import { isNodeType, findAll, ASTDereferencer, astDereferencer } from 'solidity-ast/utils'; -import type { ContractDefinition, FunctionDefinition } from 'solidity-ast'; +import type { + ContractDefinition, + FunctionDefinition, + StructDefinition, + TypeName, + UserDefinedTypeName, +} from 'solidity-ast'; import debug from '../utils/debug'; import { SolcOutput, SolcBytecode, SolcInput } from '../solc-api'; @@ -15,6 +21,7 @@ import { getFullyQualifiedName } from '../utils/contract-name'; import { getAnnotationArgs as getSupportedAnnotationArgs, getDocumentation } from '../utils/annotations'; import { getStorageLocationAnnotation, isNamespaceSupported } from '../storage/namespace'; import { UpgradesError } from '../error'; +import { assertUnreachable } from '../utils/assert'; export type ValidationRunData = Record; @@ -40,6 +47,7 @@ export const errorKinds = [ 'delegatecall', 'selfdestruct', 'missing-public-upgradeto', + 'internal-function-storage', ] as const; export type ValidationError = @@ -60,7 +68,8 @@ interface ValidationErrorWithName extends ValidationErrorBase { | 'state-variable-immutable' | 'external-library-linking' | 'struct-definition' - | 'enum-definition'; + | 'enum-definition' + | 'internal-function-storage'; } interface ValidationErrorConstructor extends ValidationErrorBase { @@ -197,6 +206,7 @@ export function validate( // TODO: add linked libraries support // https://github.com/OpenZeppelin/openzeppelin-upgrades/issues/52 ...getLinkingErrors(contractDef, bytecode), + ...getInternalFunctionStorageErrors(contractDef, deref, decodeSrc), ]; validation[key].layout = extractStorageLayout( @@ -455,6 +465,16 @@ function tryDerefFunction(deref: ASTDereferencer, referencedDeclaration: number) } } +function tryDerefStruct(deref: ASTDereferencer, referencedDeclaration: number): StructDefinition | undefined { + try { + return deref(['StructDefinition'], referencedDeclaration); + } catch (e: any) { + if (!e.message.includes('No node with id')) { + throw e; + } + } +} + function* getInheritedContractOpcodeErrors( contractDef: ContractDefinition, deref: ASTDereferencer, @@ -557,3 +577,53 @@ function* getLinkingErrors( } } } + +function* getInternalFunctionStorageErrors( + contractOrStructDef: ContractDefinition | StructDefinition, + deref: ASTDereferencer, + decodeSrc: SrcDecoder, + visitedNodeIds = new Set(), +): Generator { + // Note: Solidity does not allow annotations for non-public state variables, nor recursive types for public variables, + // so annotations cannot be used to skip these checks. + for (const variableDec of findAll('VariableDeclaration', contractOrStructDef)) { + if (variableDec.typeName?.nodeType === 'FunctionTypeName' && variableDec.typeName.visibility === 'internal') { + // Find internal function types directly in this node's scope + yield { + kind: 'internal-function-storage', + name: variableDec.name, + src: decodeSrc(variableDec), + }; + } else if (variableDec.typeName) { + const userDefinedType = findUserDefinedType(variableDec.typeName); + // Recursively try to dereference struct since it may be declared elsewhere + if (userDefinedType !== undefined && !visitedNodeIds.has(userDefinedType.referencedDeclaration)) { + const structDef = tryDerefStruct(deref, userDefinedType.referencedDeclaration); + if (structDef !== undefined) { + visitedNodeIds.add(userDefinedType.referencedDeclaration); + yield* getInternalFunctionStorageErrors(structDef, deref, decodeSrc, visitedNodeIds); + } + } + } + } +} + +/** + * Recursively traverse array and mapping types to find user-defined types (which may be struct references). + */ +function findUserDefinedType(typeName: TypeName): UserDefinedTypeName | undefined { + switch (typeName.nodeType) { + case 'ArrayTypeName': + return findUserDefinedType(typeName.baseType); + case 'Mapping': + // only mapping values can possibly refer to other array, mapping, or user-defined types + return findUserDefinedType(typeName.valueType); + case 'UserDefinedTypeName': + return typeName; + case 'ElementaryTypeName': + case 'FunctionTypeName': + return undefined; + default: + return assertUnreachable(typeName); + } +}