From 6645e4e41ef43fd1a513ea641bf12386851ec6ae Mon Sep 17 00:00:00 2001 From: Ryan Pate Date: Tue, 28 Mar 2023 17:22:15 -0700 Subject: [PATCH 1/2] feat(core): Output all validation errors at once --- packages/core/src/config/parse.ts | 510 ++++++++++++------ packages/core/src/tasks/index.ts | 3 +- packages/core/src/types.ts | 13 +- packages/core/src/utils.ts | 73 ++- .../chugsplash/VariableValidation.config.ts | 4 +- packages/plugins/src/foundry/index.ts | 2 + packages/plugins/src/hardhat/artifacts.ts | 44 +- packages/plugins/src/hardhat/deployments.ts | 6 +- packages/plugins/src/hardhat/tasks.ts | 15 +- packages/plugins/src/utils.ts | 21 +- packages/plugins/test/Transfer.spec.ts | 72 ++- packages/plugins/test/Validation.spec.ts | 133 +++-- 12 files changed, 557 insertions(+), 339 deletions(-) diff --git a/packages/core/src/config/parse.ts b/packages/core/src/config/parse.ts index 74a3cac18..20170b650 100644 --- a/packages/core/src/config/parse.ts +++ b/packages/core/src/config/parse.ts @@ -10,7 +10,7 @@ import { Fragment } from 'ethers/lib/utils' import { assertStorageUpgradeSafe, StorageLayout, - ValidationErrors, + UpgradeableContractErrorReport, } from '@openzeppelin/upgrades-core' import { OZ_UUPS_UPDATER_ADDRESS, ProxyABI } from '@chugsplash/contracts' import { getDetailedLayout } from '@openzeppelin/upgrades-core/dist/storage/layout' @@ -66,12 +66,25 @@ import { import { ChugSplashRuntimeEnvironment } from '../types' class InputError extends Error { - constructor(message) { + constructor(message: string) { super(message) this.name = 'InputError' } } +let validationErrors = false + +const logValidationError = ( + logLevel: 'warning' | 'error' = 'warning', + title: string, + lines: string[], + silent: boolean, + stream: NodeJS.WritableStream +) => { + validationErrors = true + chugsplashLog(logLevel, title, lines, silent, stream) +} + /** * Reads a ChugSplash config file and completes full parsing and validation on it. * @@ -83,7 +96,8 @@ export const readValidatedChugSplashConfig = async ( configPath: string, artifactPaths: ArtifactPaths, integration: Integration, - cre: ChugSplashRuntimeEnvironment + cre: ChugSplashRuntimeEnvironment, + exitOnFailure: boolean = true ): Promise => { const userConfig = await readUnvalidatedChugSplashConfig(configPath) return parseAndValidateChugSplashConfig( @@ -91,7 +105,8 @@ export const readValidatedChugSplashConfig = async ( userConfig, artifactPaths, integration, - cre + cre, + exitOnFailure ) } @@ -128,11 +143,10 @@ export const isEmptyChugSplashConfig = (configFileName: string): boolean => { * * @param config Config file to validate. */ -export const assertValidUserConfigFields = (config: UserChugSplashConfig) => { - if (config.contracts === undefined) { - throw new Error('contracts field must be defined in ChugSplash config') - } - +export const assertValidUserConfigFields = ( + config: UserChugSplashConfig, + cre: ChugSplashRuntimeEnvironment +) => { const referenceNames: string[] = Object.keys(config.contracts) for (const [referenceName, contractConfig] of Object.entries( @@ -140,8 +154,12 @@ export const assertValidUserConfigFields = (config: UserChugSplashConfig) => { )) { // Block people from accidentally using templates in contract names. if (referenceName.includes('{') || referenceName.includes('}')) { - throw new Error( - `cannot use template strings in reference names: ${referenceName}` + logValidationError( + 'error', + `Cannot use template strings in reference names: ${referenceName}`, + [], + cre.silent, + cre.stream ) } @@ -150,8 +168,12 @@ export const assertValidUserConfigFields = (config: UserChugSplashConfig) => { contractConfig.contract.includes('{') || contractConfig.contract.includes('}') ) { - throw new Error( - `cannot use template strings in contract names: ${contractConfig.contract}` + logValidationError( + 'error', + `Cannot use template strings in contract name: ${contractConfig.contract}`, + [], + cre.silent, + cre.stream ) } @@ -160,8 +182,12 @@ export const assertValidUserConfigFields = (config: UserChugSplashConfig) => { contractConfig.externalProxy !== undefined && !ethers.utils.isAddress(contractConfig.externalProxy) ) { - throw new Error( - `external proxy address is not a valid address: ${contractConfig.externalProxy}` + logValidationError( + 'error', + `External proxy address is not a valid address: ${contractConfig.externalProxy}`, + [], + cre.silent, + cre.stream ) } @@ -170,8 +196,12 @@ export const assertValidUserConfigFields = (config: UserChugSplashConfig) => { contractConfig.externalProxyType !== undefined && isExternalProxyType(contractConfig.externalProxyType) === false ) { - throw new Error( - `External proxy type is not valid: ${contractConfig.externalProxyType}` + logValidationError( + 'error', + `External proxy type is not valid ${contractConfig.externalProxyType}`, + [], + cre.silent, + cre.stream ) } @@ -180,17 +210,23 @@ export const assertValidUserConfigFields = (config: UserChugSplashConfig) => { contractConfig.externalProxy !== undefined && contractConfig.externalProxyType === undefined ) { - throw new Error( - `User included an 'externalProxy' field for ${contractConfig.contract} in ${config.options.projectName},\n` + - `but did not include an 'externalProxyType' field. Please include both or neither.` + logValidationError( + 'error', + `User included an 'externalProxy' field, but did not include an 'externalProxyType'\nfield for ${contractConfig.contract}. Please include both or neither.`, + [], + cre.silent, + cre.stream ) } else if ( contractConfig.externalProxy === undefined && contractConfig.externalProxyType !== undefined ) { - throw new Error( - `User included an 'externalProxyType' field for ${contractConfig.contract} in ${config.options.projectName},\n` + - `but did not include an 'externalProxy' field. Please include both or neither.` + logValidationError( + 'error', + `User included an 'externalProxyType' field, but did not include an 'externalProxy'\nfield for ${contractConfig.contract}. Please include both or neither.`, + [], + cre.silent, + cre.stream ) } @@ -198,23 +234,33 @@ export const assertValidUserConfigFields = (config: UserChugSplashConfig) => { contractConfig.previousBuildInfo !== undefined && contractConfig.previousFullyQualifiedName === undefined ) { - throw new Error( - `User included a 'previousBuildInfo' field in the ChugSplash config file for ${contractConfig.contract}, but\n` + - `did not include a 'previousFullyQualifiedName' field. Please include both or neither.` + logValidationError( + 'error', + `User included a 'previousBuildInfo' field in the ChugSplash config file for ${contractConfig.contract}, but\ndid not include a 'previousFullyQualifiedName' field. Please include both or neither.`, + [], + cre.silent, + cre.stream ) } else if ( contractConfig.previousBuildInfo === undefined && contractConfig.previousFullyQualifiedName !== undefined ) { - throw new Error( - `User included a 'previousFullyQualifiedName' field in the ChugSplash config file for ${contractConfig.contract}, but\n` + - `did not include a 'previousBuildInfo' field. Please include both or neither.` + logValidationError( + 'error', + `User included a 'previousFullyQualifiedName' field in the ChugSplash config file for ${contractConfig.contract}, but\ndid not include a 'previousBuildInfo' field. Please include both or neither.`, + [], + cre.silent, + cre.stream ) } if (contractConfig.variables !== undefined) { // Check that all contract references are valid. - assertValidContractReferences(contractConfig.variables, referenceNames) + assertValidContractReferences( + contractConfig.variables, + referenceNames, + cre + ) } if (contractConfig.constructorArgs !== undefined) { @@ -225,9 +271,12 @@ export const assertValidUserConfigFields = (config: UserChugSplashConfig) => { keywords.preserve ) ) { - throw new Error( - `Detected the '{preserve}' keyword in the 'constructorArgs' field of your ChugSplash config file. This \n` + - `keyword can only be used in the 'variables' field. Please remove all instances of it in 'constructorArgs'.` + logValidationError( + 'error', + `Detected the '{preserve}' keyword in the 'constructorArgs' field of your ChugSplash config file. This \nkeyword can only be used in the 'variables' field. Please remove all instances of it in 'constructorArgs'.`, + [], + cre.silent, + cre.stream ) } } @@ -592,7 +641,7 @@ export const parseInplaceStruct: VariableHandler< return member.label === varName }) if (memberStorageObj === undefined) { - throw new Error( + throw new InputError( `User entered incorrect member in ${variableType.label}: ${varName}` ) } @@ -857,15 +906,18 @@ export const parseAndValidateVariable = ( const parseContractVariables = ( contractConfig: UserContractConfig, storageLayout: SolidityStorageLayout, - compilerOutput: CompilerOutput + compilerOutput: CompilerOutput, + cre: ChugSplashRuntimeEnvironment ): { - [name: string]: ParsedConfigVariable + variables: { + [name: string]: ParsedConfigVariable + } } => { const parsedConfigVariables: { [name: string]: ParsedConfigVariable } = {} if (!contractConfig.variables) { - return {} + return { variables: {} } } // Create an AST Dereferencer. We must convert the CompilerOutput type to `any` here because @@ -913,40 +965,74 @@ const parseContractVariables = ( unnecessarilyDefinedVariables.length > 0 || missingVariables.length > 0 ) { - let message = `We detected some issues in your ChugSplash config file, please resolve them and try again.\n\n` - if (inputErrors.length > 0) { - message += `The following variables were defined incorrectly:\n` + const lines: string[] = [] + for (const error of inputErrors) { - message += `${error} \n` + lines.push(error) } - message += '\n' + + logValidationError( + 'error', + 'Detected incorrectly defined variables:', + lines, + cre.silent, + cre.stream + ) } if (unnecessarilyDefinedVariables.length > 0) { - message += `The following variables were defined in the ChugSplash config file but do not exist in the contract ${contractConfig.contract}:\n` + const lines: string[] = [] + for (const variable of unnecessarilyDefinedVariables) { - message += `${variable} \n` + lines.push(`${variable}`) } - message += `- If any of these variables are immutable, please remove their definition in the 'variables' section of the ChugSplash config file and use the 'constructorArgs' field instead.\n` - message += `- If any of these variables are meant to be mutable, please remove their definition in the ChugSplash config file.\n` - message += `- If this problem persists, delete your cache folder then try again.\n\n` + lines.push( + `- If any of these variables are immutable, please remove their definition in the 'variables' section of the ChugSplash config file and use the 'constructorArgs' field instead.` + ) + lines.push( + `- If any of these variables are meant to be mutable, please remove their definition in the ChugSplash config file.` + ) + lines.push( + `- If this problem persists, delete your cache folder then try again.` + ) + + logValidationError( + 'error', + `Detected variables defined in the ChugSplash config file that do not exist in the contract ${contractConfig.contract}:`, + lines, + cre.silent, + cre.stream + ) } if (missingVariables.length > 0) { - message += `The following variables were defined in the contract ${contractConfig.contract} (or one of its parent contracts) but were not defined in the ChugSplash config file:\n` + const lines: string[] = [] + for (const variable of missingVariables) { - message += `${variable} \n` + lines.push(variable) } - message += `- Every variable defined in your contracts must be assigned a value in your ChugSplash config file.\n` - message += `- Please define the variable in your ChugSplash config file then run this command again.\n` - message += `- If this problem persists, delete your cache folder then try again.\n\n` - } + lines.push( + '- Every variable defined in your contracts must be assigned a value in your ChugSplash config file.' + ) + lines.push( + '- Please define the variable in your ChugSplash config file then run this command again.' + ) + lines.push( + '- If this problem persists, delete your cache folder then try again.' + ) - throw new InputError(message) + logValidationError( + 'error', + `The following variables were defined in the contract ${contractConfig.contract} (or one of its parent contracts) but were not defined in the ChugSplash config file:`, + lines, + cre.silent, + cre.stream + ) + } } - return parsedConfigVariables + return { variables: parsedConfigVariables } } /** @@ -962,8 +1048,9 @@ const parseContractVariables = ( export const parseAndValidateConstructorArgs = ( userConstructorArgs: UserConfigVariables, referenceName: string, - abi: Array -): ParsedConfigVariables => { + abi: Array, + cre: ChugSplashRuntimeEnvironment +): { args: ParsedConfigVariables } => { const parsedConstructorArgs: ParsedConfigVariables = {} const constructorFragment = abi.find( @@ -977,7 +1064,7 @@ export const parseAndValidateConstructorArgs = ( `no constructor exists in the contract.` ) } else { - return parsedConstructorArgs + return { args: parsedConstructorArgs } } } @@ -1008,34 +1095,45 @@ export const parseAndValidateConstructorArgs = ( incorrectConstructorArgNames.length > 0 || undefinedConstructorArgNames.length > 0 ) { - let message = `We detected some issues in your ChugSplash config file, please resolve them and try again.\n\n` if (incorrectConstructorArgNames.length > 0) { - message += `The following constructor arguments were found in your config for ${referenceName}, but are not present in the contract constructor:\n` - message += `${incorrectConstructorArgNames.map( - (argName) => `${argName}` - )}\n` - message += '\n' + const lines: string[] = [] + lines.push( + `${incorrectConstructorArgNames.map((argName) => `${argName}`)}` + ) + + logValidationError( + 'error', + `The following constructor arguments were found in your config for ${referenceName},\nbut are not present in the contract constructor:`, + lines, + cre.silent, + cre.stream + ) } if (undefinedConstructorArgNames.length > 0) { - message += `The following constructor arguments are required by the constructor for ${referenceName}, but were not found in your config:\n` - message += `${undefinedConstructorArgNames.map( - (argName) => `${argName}` - )}\n` - message += '\n' + const lines: string[] = [] + lines.push( + `${undefinedConstructorArgNames.map((argName) => `${argName}`)}` + ) + logValidationError( + 'error', + `The following constructor arguments are required by the constructor for ${referenceName},\nbut were not found in your config:`, + lines, + cre.silent, + cre.stream + ) } - - throw new InputError(message) } - return parsedConstructorArgs + return { args: parsedConstructorArgs } } export const assertStorageCompatiblePreserveKeywords = ( contractConfig: ParsedContractConfig, prevStorageLayout: StorageLayout, - newStorageLayout: StorageLayout -) => { + newStorageLayout: StorageLayout, + cre: ChugSplashRuntimeEnvironment +): boolean => { const prevDetailedLayout = getDetailedLayout(prevStorageLayout) const newDetailedLayout = getDetailedLayout(newStorageLayout) @@ -1056,18 +1154,28 @@ export const assertStorageCompatiblePreserveKeywords = ( ) if (!validPreserveKeyword) { - errorMessages.push( - `The variable "${newStorageObj.label}" contains the preserve keyword, but does not exist in the previous\n` + - `storage layout at the same slot position with the same variable type. Please fix this or remove\n` + - `the preserve keyword from this variable.` - ) + errorMessages.push(newStorageObj.label) } } } if (errorMessages.length > 0) { - throw new Error(`${errorMessages.join('\n\n')}`) + logValidationError( + 'error', + 'Invalid use of preserve keyword.', + [ + 'The following variables contain the preserve keyword, but do not exist in the previous', + 'storage layout at the same slot position with the same variable type. Please fix this', + 'or remove the preserve keyword from these variables:', + ...errorMessages, + ], + cre.silent, + cre.stream + ) + return false } + + return true } export const assertValidParsedChugSplashFile = async ( @@ -1077,21 +1185,15 @@ export const assertValidParsedChugSplashFile = async ( artifactPaths: ArtifactPaths, integration: Integration, cre: ChugSplashRuntimeEnvironment -) => { - const { - canonicalConfigPath, - remoteExecution, - autoConfirm, - openzeppelinStorageLayouts, - } = cre +): Promise<{ + upgrade: boolean +}> => { + const { canonicalConfigPath, remoteExecution } = cre // Determine if the deployment is an upgrade - const projectName = parsedConfig.options.projectName - const chugSplashManagerAddress = getChugSplashManagerProxyAddress( parsedConfig.options.projectName ) - const requiresOwnershipTransfer: { name: string proxyAddress: string @@ -1159,19 +1261,22 @@ export const assertValidParsedChugSplashFile = async ( } if (requiresOwnershipTransfer.length > 0) { - throw new Error( - `Detected proxy contracts which are not managed by ChugSplash:` + + logValidationError( + 'error', + `Detected proxy contracts which are not managed by ChugSplash:`, + [ `${requiresOwnershipTransfer.map( ({ name, proxyAddress, currentAdminAddress }) => `\n${name}: ${proxyAddress} | Current admin: ${currentAdminAddress}` )} - If you are using any Transparent proxies, you must transfer ownership of each to ChugSplash using the following command: npx hardhat chugsplash-transfer-ownership --network --config-path --proxy - If you are using any UUPS proxies, you must give your ChugSplashManager contract ${chugSplashManagerAddress} permission to call the 'upgradeTo' function on each of them. - ` + `, + ], + cre.silent, + cre.stream ) } @@ -1206,9 +1311,16 @@ permission to call the 'upgradeTo' function on each of them. // throw validation errors if detected if (upgradableContract.errors.length > 0) { - throw new ValidationErrors( - contractConfig.contract, - upgradableContract.errors + logValidationError( + 'error', + `Contract ${contractConfig.contract} is not upgrade safe`, + [ + new UpgradeableContractErrorReport( + upgradableContract.errors + ).explain(), + ], + false, + cre.stream ) } @@ -1226,13 +1338,14 @@ permission to call the 'upgradeTo' function on each of them. userContractConfig, remoteExecution, canonicalConfigPath, - openzeppelinStorageLayouts + cre ) assertStorageCompatiblePreserveKeywords( contractConfig, previousStorageLayout, - newStorageLayout + newStorageLayout, + cre ) if ( @@ -1271,10 +1384,14 @@ permission to call the 'upgradeTo' function on each of them. !userConfig.contracts[referenceName].unsafeAllow ?.missingPublicUpgradeTo ) { - throw new Error( - `Contract ${referenceName} proxy type is marked as UUPS, but the new implementation\n` + - `no longer has a public 'upgradeTo(address)' function. You must include this function \n` + - `or you will no longer be able to upgrade this contract.` + logValidationError( + 'error', + `Contract ${referenceName} proxy type is marked as UUPS, but the new implementation \n no longer has a public 'upgradeTo(address)' function.`, + [ + 'You must include this function or you will no longer be able to upgrade this contract.', + ], + cre.silent, + cre.stream ) } } @@ -1286,29 +1403,26 @@ permission to call the 'upgradeTo' function on each of them. if ( variableContainsKeyword(contractConfig.variables, keywords.preserve) ) { - throw new Error( - `Detected the '{preserve}' keyword in a fresh deployment. This keyword is reserved for\n` + - `upgrades only. Please remove all instances of it in your ChugSplash config file.` + logValidationError( + 'error', + 'Detected the "{preserve}" keyword in a fresh deployment.', + [ + 'This keyword is reserved for upgrades only. Please remove all instances of it in your ChugSplash config file.', + ], + cre.silent, + cre.stream ) } } } - // confirm - if (!autoConfirm && isUpgrade) { - // Confirm upgrade with user - const userConfirmed = await yesno({ - question: `Prior deployment(s) detected for project ${projectName}. Would you like to perform an upgrade? (y/n)`, - }) - if (!userConfirmed) { - throw new Error(`User denied upgrade.`) - } - } + return { upgrade: isUpgrade } } export const assertValidContracts = ( parsedConfig: ParsedChugSplashConfig, - artifactPaths: ArtifactPaths + artifactPaths: ArtifactPaths, + cre: ChugSplashRuntimeEnvironment ) => { for (const [referenceName, contractConfig] of Object.entries( parsedConfig.contracts @@ -1355,11 +1469,16 @@ export const assertValidContracts = ( constructorNodeContractNames[node.id] = contractNode.name } else if (node.nodeType === 'VariableDeclaration') { if (node.mutability === 'mutable' && node.value !== undefined) { - throw new Error( - `User attempted to assign a value to a non-immutable state variable '${node.name}' in\n` + - `the contract: ${contractNode.name}. This is not allowed because the value will not exist in\n` + - `the upgradeable contract. Please remove the value in the contract and define it in your ChugSplash\n` + - `file instead. Alternatively, can also set '${node.name} to be a constant or immutable variable.` + logValidationError( + 'error', + `Attempted to assign a value to a non-immutable state variable '${node.name}' in the contract: ${contractNode.name}`, + [ + 'This is not allowed because the value will not exist in the upgradeable contract.', + 'Please remove the value in the contract and define it in your ChugSplash file instead', + `Alternatively, you can also set '${node.name}' to be a constant or immutable variable.`, + ], + cre.silent, + cre.stream ) } @@ -1368,10 +1487,14 @@ export const assertValidContracts = ( node.value !== undefined && node.value.kind === 'functionCall' ) { - throw new Error( - `User attempted to assign the immutable variable '${node.name}' to the return value of a function call in\n` + - `the contract: ${contractNode.name}. This is not allowed to ensure that ChugSplash is\n` + - `deterministic. Please remove the function call.` + logValidationError( + 'error', + `Attempted to assign the immutable variable '${node.name}' to the return value of a function call in\nthe contract: ${contractNode.name}.`, + [ + 'This is not allowed to ensure that ChugSplash is deterministic. Please remove the function call.', + ], + cre.silent, + cre.stream ) } @@ -1385,24 +1508,35 @@ export const assertValidContracts = ( for (const node of constructorNodes) { for (const statement of node.body.statements) { if (statement.nodeType !== 'ExpressionStatement') { - throw new Error( - `Detected an unallowed expression, '${statement.nodeType}', in the constructor of the\n` + - `contract: ${ - constructorNodeContractNames[node.id] - }. Only immutable variable assignments are allowed in\n` + - `the constructor to ensure that ChugSplash can deterministically deploy your contracts.` + logValidationError( + 'error', + `Detected an unallowed expression '${ + statement.nodeType + }', in the constructor of the contract: ${ + constructorNodeContractNames[node.id] + }.`, + [ + 'Only immutable variable assignments are allowed in the constructor to ensure that ChugSplash', + 'can deterministically deploy your contracts.', + ], + cre.silent, + cre.stream ) } if (statement.expression.nodeType !== 'Assignment') { const unallowedOperation: string = statement.expression.expression.name ?? statement.expression.kind - throw new Error( - `Detected an unallowed operation, '${unallowedOperation}', in the constructor of the\n` + - `contract: ${ - constructorNodeContractNames[node.id] - }. Only immutable variable assignments are allowed in\n` + - `the constructor to ensure that ChugSplash can deterministically deploy your contracts.` + logValidationError( + 'error', + `Detected an unallowed operation, '${unallowedOperation}', in the constructor of the contract: ${ + constructorNodeContractNames[node.id] + }.`, + [ + 'Only immutable variable assignments are allowed in the constructor to ensure that ChugSplash is deterministic', + ], + cre.silent, + cre.stream ) } @@ -1411,22 +1545,32 @@ export const assertValidContracts = ( statement.expression.leftHandSide.referencedDeclaration ] !== true ) { - throw new Error( - `Detected an assignment to a non-immutable variable, '${statement.expression.leftHandSide.name}', in the\n` + - `constructor of the contract: ${ - constructorNodeContractNames[node.id] - }. Only immutable variable assignments are allowed in\n` + - `the constructor to ensure that ChugSplash can deterministically deploy your contracts.` + logValidationError( + 'error', + `Detected an assignment to a non-immutable variable, '${ + statement.expression.leftHandSide.name + }', in the constructor of the contract: ${ + constructorNodeContractNames[node.id] + }.`, + [], + cre.silent, + cre.stream ) } if (statement.expression.rightHandSide.kind === 'functionCall') { - throw new Error( - `User attempted to assign the immutable variable '${statement.expression.leftHandSide.name}' to the return \n` + - `value of a function call in the contract: ${ - constructorNodeContractNames[node.id] - }. This is not allowed to ensure that\n` + - `ChugSplash is deterministic. Please remove the function call.` + logValidationError( + 'error', + `User attempted to assign the immutable variable '${ + statement.expression.leftHandSide.name + }' to the return \n value of a function call in the contract: ${ + constructorNodeContractNames[node.id] + }.`, + [ + 'This is not allowed to ensure that ChugSplash is deterministic. Please remove the function call.', + ], + cre.silent, + cre.stream ) } } @@ -1462,7 +1606,7 @@ const logUnsafeOptions = ( } if (lines.length > 0) { - chugsplashLog( + logValidationError( 'warning', `Potentially unsafe deployment of ${referenceName}`, lines, @@ -1485,11 +1629,15 @@ const parseAndValidateChugSplashConfig = async ( userConfig: UserChugSplashConfig, artifactPaths: ArtifactPaths, integration: Integration, - cre: ChugSplashRuntimeEnvironment + cre: ChugSplashRuntimeEnvironment, + exitOnFailure: boolean = true ): Promise => { + // just in case, we reset the global validation errors flag before parsing + validationErrors = false + logUnsafeOptions(userConfig, cre.silent, cre.stream) - assertValidUserConfigFields(userConfig) + assertValidUserConfigFields(userConfig, cre) const parsedConfig: ParsedChugSplashConfig = { options: userConfig.options, @@ -1500,12 +1648,22 @@ const parseAndValidateChugSplashConfig = async ( for (const [referenceName, userContractConfig] of Object.entries( userConfig.contracts )) { - if ( - userContractConfig.externalProxy !== undefined && - (await provider.getCode(userContractConfig.externalProxy)) === '0x' - ) { + try { + if ( + userContractConfig.externalProxy !== undefined && + (await provider.getCode(userContractConfig.externalProxy)) === '0x' + ) { + logValidationError( + 'error', + `Entered a proxy address that does not exist: ${userContractConfig.externalProxy}`, + [], + cre.silent, + cre.stream + ) + } + } catch { throw new Error( - `User entered a proxy address that does not exist: ${userContractConfig.externalProxy}` + `Invalid proxy address: ${userContractConfig.externalProxy}` ) } @@ -1540,20 +1698,22 @@ const parseAndValidateChugSplashConfig = async ( const storageLayout = compilerOutput.contracts[sourceName][contractName].storageLayout - const parsedVariables = parseContractVariables( + const { variables: parsedVariables } = parseContractVariables( JSON.parse( Handlebars.compile(JSON.stringify(userContractConfig))({ ...contracts, }) ), storageLayout, - compilerOutput + compilerOutput, + cre ) - const args = parseAndValidateConstructorArgs( + const { args } = parseAndValidateConstructorArgs( constructorArgs ?? {}, referenceName, - compilerOutput.contracts[sourceName][contractName].abi + compilerOutput.contracts[sourceName][contractName].abi, + cre ) parsedConfig.contracts[referenceName] = { @@ -1567,8 +1727,9 @@ const parseAndValidateChugSplashConfig = async ( contracts[referenceName] = proxy } - assertValidContracts(parsedConfig, artifactPaths) - assertValidParsedChugSplashFile( + assertValidContracts(parsedConfig, artifactPaths, cre) + + const { upgrade } = await assertValidParsedChugSplashFile( provider, parsedConfig, userConfig, @@ -1576,6 +1737,25 @@ const parseAndValidateChugSplashConfig = async ( integration, cre ) + + // confirm + if (!cre.autoConfirm && upgrade) { + // Confirm upgrade with user + const userConfirmed = await yesno({ + question: `Prior deployment(s) detected for project ${userConfig.options.projectName}. Would you like to perform an upgrade? (y/n)`, + }) + if (!userConfirmed) { + throw new Error(`User denied upgrade.`) + } + } + + // Exit if validation errors are detected + // We also allow the user to disable this behavior by setting `exitOnFailure` to false. + // This is useful for testing. + if (validationErrors && exitOnFailure) { + process.exit(1) + } + return JSON.parse( Handlebars.compile(JSON.stringify(parsedConfig))({ ...contracts, diff --git a/packages/core/src/tasks/index.ts b/packages/core/src/tasks/index.ts index 5fc8de511..52046811d 100644 --- a/packages/core/src/tasks/index.ts +++ b/packages/core/src/tasks/index.ts @@ -12,6 +12,7 @@ import { ParsedChugSplashConfig, proxyTypeHashes, readUnvalidatedChugSplashConfig, + UserChugSplashConfig, verifyBundle, } from '../config' import { @@ -76,7 +77,7 @@ import { export const chugsplashRegisterAbstractTask = async ( provider: ethers.providers.JsonRpcProvider, signer: ethers.Signer, - parsedConfig: ParsedChugSplashConfig, + parsedConfig: UserChugSplashConfig | ParsedChugSplashConfig, allowManagedProposals: boolean, owner: string, integration: Integration, diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts index 0f6dd3d24..c536ea3b4 100644 --- a/packages/core/src/types.ts +++ b/packages/core/src/types.ts @@ -1,6 +1,9 @@ import { BaseServiceV2, LogLevel } from '@eth-optimism/common-ts' +import { StorageLayout } from '@openzeppelin/upgrades-core' import { ethers } from 'ethers' +import { HardhatRuntimeEnvironment } from 'hardhat/types' +import { ParsedContractConfig } from './config' import { Integration } from './constants' export type ChugSplashRuntimeEnvironment = { @@ -8,13 +11,13 @@ export type ChugSplashRuntimeEnvironment = { canonicalConfigPath: string | undefined remoteExecution: boolean autoConfirm: boolean - openzeppelinStorageLayouts: - | { - [referenceName: string]: any - } - | undefined stream: NodeJS.WritableStream silent: boolean + hre: HardhatRuntimeEnvironment | undefined + fetchOpenZeppelinStorageLayout: ( + hre: HardhatRuntimeEnvironment | undefined, + parsedContractConfig: ParsedContractConfig + ) => Promise } export type FoundryContractArtifact = { diff --git a/packages/core/src/utils.ts b/packages/core/src/utils.ts index 99068a184..b67fe43f2 100644 --- a/packages/core/src/utils.ts +++ b/packages/core/src/utils.ts @@ -63,7 +63,7 @@ import { import { ChugSplashActionBundle, ChugSplashActionType } from './actions/types' import { Integration } from './constants' import 'core-js/features/array/at' -import { FoundryContractArtifact } from './types' +import { ChugSplashRuntimeEnvironment, FoundryContractArtifact } from './types' import { ContractArtifact, ContractASTNode, @@ -673,52 +673,80 @@ export const isExternalProxyType = ( */ export const assertValidContractReferences = ( variable: UserConfigVariable, - referenceNames: string[] -) => { + referenceNames: string[], + cre: ChugSplashRuntimeEnvironment +): boolean => { + let isValid = true if ( typeof variable === 'string' && variable.includes('{{') && variable.includes('}}') ) { if (!variable.startsWith('{{')) { - throw new Error( - `Contract reference cannot contain leading spaces before '{{' : ${variable}` + isValid = false + chugsplashLog( + 'error', + `Contract reference cannot contain leading spaces before '{{' : ${variable}`, + [], + cre.silent, + cre.stream ) } if (!variable.endsWith('}}')) { - throw new Error( - `Contract reference cannot contain trailing spaces: ${variable}` + isValid = false + chugsplashLog( + 'error', + `Contract reference cannot contain trailing spaces: ${variable}`, + [], + cre.silent, + cre.stream ) } const contractReference = variable.substring(2, variable.length - 2).trim() if (!referenceNames.includes(contractReference)) { - throw new Error( - `Invalid contract reference: ${variable}.\n` + - `Did you misspell this contract reference, or forget to define a contract with this reference name?` + isValid = false + chugsplashLog( + 'error', + `Invalid contract reference: ${variable}.\nDid you misspell this contract reference, or forget to define a contract with this reference name?`, + [], + cre.silent, + cre.stream ) } } else if (Array.isArray(variable)) { for (const element of variable) { - assertValidContractReferences(element, referenceNames) + if (!assertValidContractReferences(element, referenceNames, cre)) { + isValid = false + } } } else if (typeof variable === 'object') { for (const [varName, varValue] of Object.entries(variable)) { - assertValidContractReferences(varName, referenceNames) - assertValidContractReferences(varValue, referenceNames) + if ( + !assertValidContractReferences(varName, referenceNames, cre) || + !assertValidContractReferences(varValue, referenceNames, cre) + ) { + isValid = false + } } } else if ( typeof variable === 'boolean' || typeof variable === 'number' || typeof variable === 'string' ) { - return + return isValid } else { - throw new Error( - `Detected unknown variable type, ${typeof variable}, for variable: ${variable}.` + chugsplashLog( + 'error', + `Detected unknown variable type, ${typeof variable}, for variable: ${variable}.`, + [], + cre.silent, + cre.stream ) } + + return isValid } export const getParentContractASTNodes = ( @@ -1068,10 +1096,13 @@ export const getPreviousStorageLayoutOZFormat = async ( userContractConfig: UserContractConfig, remoteExecution: boolean, canonicalConfigFolderPath: string, - openzeppelinStorageLayouts?: { - [referenceName: string]: StorageLayout - } + cre: ChugSplashRuntimeEnvironment ): Promise => { + const openzeppelinStorageLayout = await cre.fetchOpenZeppelinStorageLayout( + cre.hre, + parsedContractConfig + ) + if ((await provider.getCode(parsedContractConfig.proxy)) === '0x') { throw new Error( `Proxy has not been deployed for the contract: ${referenceName}.` @@ -1122,8 +1153,8 @@ export const getPreviousStorageLayoutOZFormat = async ( parsedContractConfig.proxyType, userContractConfig ).layout - } else if (openzeppelinStorageLayouts?.[referenceName] !== undefined) { - return openzeppelinStorageLayouts[referenceName] + } else if (openzeppelinStorageLayout !== undefined) { + return openzeppelinStorageLayout } else { throw new Error( `Could not find the previous storage layout for the contract: ${referenceName}. Please include\n` + diff --git a/packages/plugins/chugsplash/VariableValidation.config.ts b/packages/plugins/chugsplash/VariableValidation.config.ts index f64aa4f45..9f7641858 100644 --- a/packages/plugins/chugsplash/VariableValidation.config.ts +++ b/packages/plugins/chugsplash/VariableValidation.config.ts @@ -8,9 +8,7 @@ const config: UserChugSplashConfig = { contracts: { VariableValidation: { contract: 'VariableValidation', - constructorArgs: { - _immutableUint: 1, - }, + constructorArgs: {}, variables: { arrayInt8: [0, 1, 2], int8OutsideRange: 255, diff --git a/packages/plugins/src/foundry/index.ts b/packages/plugins/src/foundry/index.ts index 78b4bc4ef..04a928730 100644 --- a/packages/plugins/src/foundry/index.ts +++ b/packages/plugins/src/foundry/index.ts @@ -76,6 +76,7 @@ const command = args[0] 'foundry', cre ) + await provider.getNetwork() const address = await wallet.getAddress() owner = owner !== 'self' ? owner : address @@ -132,6 +133,7 @@ const command = args[0] 'foundry', cre ) + await provider.getNetwork() await wallet.getAddress() diff --git a/packages/plugins/src/hardhat/artifacts.ts b/packages/plugins/src/hardhat/artifacts.ts index d3356ddd0..3a505d85f 100644 --- a/packages/plugins/src/hardhat/artifacts.ts +++ b/packages/plugins/src/hardhat/artifacts.ts @@ -3,12 +3,11 @@ import path from 'path' import { HardhatRuntimeEnvironment } from 'hardhat/types' import { ArtifactPaths, - UserChugSplashConfig, UserContractConfigs, getEIP1967ProxyImplementationAddress, getOpenZeppelinValidationOpts, BuildInfo, - getDefaultProxyAddress, + ParsedContractConfig, } from '@chugsplash/core' import { Manifest, @@ -99,33 +98,26 @@ export const getArtifactPaths = async ( * Get storage layouts from OpenZeppelin's Network Files for any proxies that are being imported * into ChugSplash from the OpenZeppelin Hardhat Upgrades plugin. */ -export const importOpenZeppelinStorageLayouts = async ( +export const importOpenZeppelinStorageLayout = async ( hre: HardhatRuntimeEnvironment, - userConfig: UserChugSplashConfig -): Promise<{ [referenceName: string]: StorageLayout }> => { - const layouts: { [referenceName: string]: StorageLayout } = {} - - for (const [referenceName, userContractConfig] of Object.entries( - userConfig.contracts - )) { - const proxy = - userContractConfig.externalProxy || - getDefaultProxyAddress(userConfig.options.projectName, referenceName) + parsedContractConfig: ParsedContractConfig +): Promise => { + const { proxyType } = parsedContractConfig + if ( + proxyType === 'oz-transparent' || + proxyType === 'oz-ownable-uups' || + proxyType === 'oz-access-control-uups' + ) { + const proxy = parsedContractConfig.proxy const isProxyDeployed = await hre.ethers.provider.getCode(proxy) - const { externalProxyType } = userConfig.contracts[referenceName] - if ( - isProxyDeployed && - (externalProxyType === 'oz-transparent' || - externalProxyType === 'oz-ownable-uups' || - externalProxyType === 'oz-access-control-uups') - ) { + if (isProxyDeployed) { const manifest = await Manifest.forNetwork(hre.network.provider) const deployData = await getDeployData( hre, - await hre.ethers.getContractFactory(userContractConfig.contract), + await hre.ethers.getContractFactory(parsedContractConfig.contract), getOpenZeppelinValidationOpts( - userContractConfig.externalProxyType ?? 'internal-default', - userContractConfig + parsedContractConfig.proxyType, + parsedContractConfig ) ) const storageLayout = await getStorageLayoutForAddress( @@ -133,9 +125,11 @@ export const importOpenZeppelinStorageLayouts = async ( deployData.validations, await getEIP1967ProxyImplementationAddress(hre.ethers.provider, proxy) ) - layouts[referenceName] = storageLayout + return storageLayout } } - return layouts + throw new Error( + 'Should not attempt to import OpenZeppelin storage layout for non-OpenZeppelin proxy type. Please report this to the developers.' + ) } diff --git a/packages/plugins/src/hardhat/deployments.ts b/packages/plugins/src/hardhat/deployments.ts index eb0f17c64..c16dcf638 100644 --- a/packages/plugins/src/hardhat/deployments.ts +++ b/packages/plugins/src/hardhat/deployments.ts @@ -47,7 +47,7 @@ export const deployAllChugSplashConfigs = async ( silent: boolean, ipfsUrl: string, fileNames?: string[] -) => { +): Promise => { const remoteExecution = await isRemoteExecution(hre) fileNames = fileNames ?? (await fetchFilesRecursively(hre.config.paths.chugsplash)) @@ -71,7 +71,7 @@ export const deployAllChugSplashConfigs = async ( // Skip this config if it's empty. if (isEmptyChugSplashConfig(configPath)) { - return + return true } const userConfig = await readUnvalidatedChugSplashConfig(configPath) @@ -109,6 +109,8 @@ export const deployAllChugSplashConfigs = async ( executor ) } + + return true } export const getContract = async ( diff --git a/packages/plugins/src/hardhat/tasks.ts b/packages/plugins/src/hardhat/tasks.ts index 5deedd1e2..0ada03520 100644 --- a/packages/plugins/src/hardhat/tasks.ts +++ b/packages/plugins/src/hardhat/tasks.ts @@ -816,7 +816,10 @@ task(TASK_NODE) quiet: true, }) } - await deployAllChugSplashConfigs(hre, hide, '') + const succeeded = await deployAllChugSplashConfigs(hre, hide, '') + if (succeeded === false) { + return + } const networkName = await resolveNetworkName( hre.ethers.provider, 'hardhat' @@ -886,12 +889,15 @@ task(TASK_TEST) }) } if (!skipDeploy) { - await deployAllChugSplashConfigs( + const succeeded = await deployAllChugSplashConfigs( hre, !show, '', configPath ? [configPath] : undefined ) + if (succeeded === false) { + return + } } } await writeSnapshotId( @@ -936,7 +942,10 @@ task(TASK_RUN) quiet: true, }) } - await deployAllChugSplashConfigs(hre, true, '') + const succeeded = await deployAllChugSplashConfigs(hre, true, '') + if (succeeded === false) { + return + } } await runSuper(args) } diff --git a/packages/plugins/src/utils.ts b/packages/plugins/src/utils.ts index b8323f126..32b6d661f 100644 --- a/packages/plugins/src/utils.ts +++ b/packages/plugins/src/utils.ts @@ -1,10 +1,19 @@ import { ChugSplashRuntimeEnvironment, - readUnvalidatedChugSplashConfig, + ParsedContractConfig, } from '@chugsplash/core' import { HardhatRuntimeEnvironment } from 'hardhat/types' -import { importOpenZeppelinStorageLayouts } from './hardhat/artifacts' +import { importOpenZeppelinStorageLayout } from './hardhat/artifacts' + +const fetchOpenZeppelinStorageLayout = async ( + hre: HardhatRuntimeEnvironment | undefined = undefined, + parsedContractConfig: ParsedContractConfig +) => { + return hre + ? importOpenZeppelinStorageLayout(hre, parsedContractConfig) + : undefined +} export const createChugSplashRuntime = async ( configPath: string, @@ -14,18 +23,14 @@ export const createChugSplashRuntime = async ( silent: boolean, stream: NodeJS.WritableStream = process.stderr ): Promise => { - const userConfig = await readUnvalidatedChugSplashConfig(configPath) - const openzeppelinStorageLayouts = hre - ? await importOpenZeppelinStorageLayouts(hre, userConfig) - : undefined - return { configPath, canonicalConfigPath: hre ? hre.config.paths.canonicalConfigs : undefined, remoteExecution, autoConfirm, - openzeppelinStorageLayouts, stream, silent, + fetchOpenZeppelinStorageLayout, + hre, } } diff --git a/packages/plugins/test/Transfer.spec.ts b/packages/plugins/test/Transfer.spec.ts index 60325faf4..f5a4ebf5b 100644 --- a/packages/plugins/test/Transfer.spec.ts +++ b/packages/plugins/test/Transfer.spec.ts @@ -85,21 +85,14 @@ describe('Transfer', () => { false, true, hre, + // if the config parsing fails and exits with code 1, you should flip this to false to see verbose output true ) - const parsedConfig = await readValidatedChugSplashConfig( - provider, - transparentUpgradeConfigPath, - artifactPaths, - 'hardhat', - cre - ) - await chugsplashRegisterAbstractTask( provider, signer, - parsedConfig, + userConfig, false, signer.address, 'hardhat', @@ -107,7 +100,7 @@ describe('Transfer', () => { ) const managerProxyAddress = getChugSplashManagerProxyAddress( - parsedConfig.options.projectName + userConfig.options.projectName ) const ProxyAdmin = await hre.ethers.getContractAt( @@ -123,13 +116,18 @@ describe('Transfer', () => { managerProxyAddress ) - const configPath = - './chugsplash/hardhat/TransparentUpgradableUpgrade.config.ts' + const parsedConfig = await readValidatedChugSplashConfig( + provider, + transparentUpgradeConfigPath, + artifactPaths, + 'hardhat', + cre + ) await chugsplashDeployAbstractTask( provider, signer, - configPath, + transparentUpgradeConfigPath, false, '', true, @@ -205,21 +203,14 @@ describe('Transfer', () => { false, true, hre, + // if the config parsing fails and exits with code 1, you should flip this to false to see verbose output true ) - const parsedConfig = await readValidatedChugSplashConfig( - provider, - uupsOwnableUpgradeConfigPath, - artifactPaths, - 'hardhat', - cre - ) - await chugsplashRegisterAbstractTask( provider, signer, - parsedConfig, + userConfig, false, signer.address, 'hardhat', @@ -227,7 +218,7 @@ describe('Transfer', () => { ) const managerProxyAddress = getChugSplashManagerProxyAddress( - parsedConfig.options.projectName + userConfig.options.projectName ) await UUPSUpgradableTokenV1.transferOwnership(managerProxyAddress) @@ -238,13 +229,18 @@ describe('Transfer', () => { 'proxy owner is not chugsplash manager' ) - const configPath = - './chugsplash/hardhat/UUPSOwnableUpgradableUpgrade.config.ts' + const parsedConfig = await readValidatedChugSplashConfig( + provider, + uupsOwnableUpgradeConfigPath, + artifactPaths, + 'hardhat', + cre + ) await chugsplashDeployAbstractTask( provider, signer, - configPath, + uupsOwnableUpgradeConfigPath, false, '', true, @@ -343,21 +339,14 @@ describe('Transfer', () => { false, true, hre, + // if the config parsing fails and exits with code 1, you should flip this to false to see verbose output true ) - const parsedConfig = await readValidatedChugSplashConfig( - provider, - uupsAccessControlUpgradeConfigPath, - artifactPaths, - 'hardhat', - cre - ) - await chugsplashRegisterAbstractTask( provider, signer, - parsedConfig, + userConfig, false, signer.address, 'hardhat', @@ -365,7 +354,7 @@ describe('Transfer', () => { ) const managerProxyAddress = getChugSplashManagerProxyAddress( - parsedConfig.options.projectName + userConfig.options.projectName ) await UUPSAccessControlUpgradableTokenV1.grantRole( @@ -381,13 +370,18 @@ describe('Transfer', () => { ) ).to.equal(true, 'proxy owner is not chugsplash manager') - const configPath = - './chugsplash/hardhat/UUPSAccessControlUpgradableUpgrade.config.ts' + const parsedConfig = await readValidatedChugSplashConfig( + provider, + uupsAccessControlUpgradeConfigPath, + artifactPaths, + 'hardhat', + cre + ) await chugsplashDeployAbstractTask( provider, signer, - configPath, + uupsAccessControlUpgradeConfigPath, false, '', true, diff --git a/packages/plugins/test/Validation.spec.ts b/packages/plugins/test/Validation.spec.ts index 850a7e619..b68c99813 100644 --- a/packages/plugins/test/Validation.spec.ts +++ b/packages/plugins/test/Validation.spec.ts @@ -20,8 +20,8 @@ const constructorArgConfigPath = './chugsplash/ConstructorArgValidation.config.ts' describe('Validate', () => { - let variableErr: Error - let constructorArgErr: Error + let validationOutput = '' + before(async () => { const provider = hre.ethers.provider const varValidationUserConfig = await readUnvalidatedChugSplashConfig( @@ -42,218 +42,217 @@ describe('Validate', () => { path.join(hre.config.paths.artifacts, 'build-info') ) + process.stderr.write = (message: string) => { + validationOutput += message + return true + } + const creVariableValidate = await createChugSplashRuntime( variableValidateConfigPath, false, true, - hre + hre, + false, + process.stderr ) const creConstructorArg = await createChugSplashRuntime( variableValidateConfigPath, false, true, - hre - ) - - try { - await readValidatedChugSplashConfig( - provider, - variableValidateConfigPath, - varValidationArtifactPaths, - 'hardhat', - creVariableValidate - ) - } catch (error) { - expect(error).to.be.an('Error') - variableErr = error - } + hre, + false, + process.stderr + ) - try { - await readValidatedChugSplashConfig( - provider, - constructorArgConfigPath, - constructorArgsValidationArtifactPaths, - 'hardhat', - creConstructorArg - ) - } catch (error) { - expect(error).to.be.an('Error') - constructorArgErr = error - } + await readValidatedChugSplashConfig( + provider, + variableValidateConfigPath, + varValidationArtifactPaths, + 'hardhat', + creVariableValidate, + false + ) + + await readValidatedChugSplashConfig( + provider, + constructorArgConfigPath, + constructorArgsValidationArtifactPaths, + 'hardhat', + creConstructorArg, + false + ) }) it('did catch invalid arrayInt8', async () => { - expect(variableErr.toString()).to.have.string( + expect(validationOutput).to.have.string( 'invalid input type for variable arrayInt8 expected number, string, or BigNumber but got array' ) }) it('did catch invalid int8OutsideRange', async () => { - expect(variableErr.toString()).to.have.string( + expect(validationOutput).to.have.string( 'invalid value for int8OutsideRange: 255, outside valid range: [-128:127]' ) }) it('did catch invalid uint8OutsideRange', async () => { - expect(variableErr.toString()).to.have.string( + expect(validationOutput).to.have.string( 'invalid value for uint8OutsideRange: 256, outside valid range: [0:255]' ) }) it('did catch invalid intAddress', async () => { - expect(variableErr.toString()).to.have.string( + expect(validationOutput).to.have.string( 'invalid input type for intAddress: 1, expected address string but got number' ) }) it('did catch invalid arrayAddress', async () => { - expect(variableErr.toString()).to.have.string( + expect(validationOutput).to.have.string( 'invalid input type for arrayAddress: 0x00000000, expected address string but got array' ) }) it('did catch invalid shortAddress', async () => { - expect(variableErr.toString()).to.have.string( + expect(validationOutput).to.have.string( 'invalid address for shortAddress: 0x00000000' ) }) it('did catch invalid intBytes32', async () => { - expect(variableErr.toString()).to.have.string( + expect(validationOutput).to.have.string( 'invalid input type for intBytes32: 1, expected DataHexString but got number' ) }) it('did catch invalid arrayBytes32', async () => { - expect(variableErr.toString()).to.have.string( + expect(validationOutput).to.have.string( 'invalid input type for arrayBytes32: 1, expected DataHexString but got array' ) }) it('did catch invalid shortBytes32', async () => { - expect(variableErr.toString()).to.have.string( + expect(validationOutput).to.have.string( 'invalid length for bytes32 variable shortBytes32: 0x00000000' ) }) it('did catch invalid longBytes8', async () => { - expect(variableErr.toString()).to.have.string( + expect(validationOutput).to.have.string( 'invalid length for bytes8 variable longBytes8: 0x1111111111111111111111111111111111111111111111111111111111111111' ) }) it('did catch invalid malformedBytes16', async () => { - expect(variableErr.toString()).to.have.string( + expect(validationOutput).to.have.string( 'invalid input format for variable malformedBytes16, expected DataHexString but got 11111111111111111111111111111111' ) }) it('did catch invalid intBoolean', async () => { - expect(variableErr.toString()).to.have.string( + expect(validationOutput).to.have.string( 'invalid input type for variable intBoolean, expected boolean but got number' ) }) it('did catch invalid stringBoolean', async () => { - expect(variableErr.toString()).to.have.string( + expect(validationOutput).to.have.string( 'invalid input type for variable stringBoolean, expected boolean but got string' ) }) it('did catch invalid arrayBoolean', async () => { - expect(variableErr.toString()).to.have.string( + expect(validationOutput).to.have.string( 'invalid input type for variable arrayBoolean, expected boolean but got array' ) }) it('did catch invalid oversizedArray', async () => { - expect(variableErr.toString()).to.have.string( + expect(validationOutput).to.have.string( 'Expected array of size 2 for oversizedArray but got [1,2,3]' ) }) it('did catch invalid oversizedNestedArray', async () => { - expect(variableErr.toString()).to.have.string( + expect(validationOutput).to.have.string( 'Expected array of size 2 for oversizedNestedArray but got [[1,2],[1,2],[1,2]]' ) }) it('did catch invalid invalidBoolArray', async () => { - expect(variableErr.toString()).to.have.string( + expect(validationOutput).to.have.string( 'invalid input type for variable invalidBoolArray, expected boolean but got string' ) }) it('did catch invalid invalidBytes32Array', async () => { - expect(variableErr.toString()).to.have.string( + expect(validationOutput).to.have.string( 'invalid input type for invalidBytes32Array: 1, expected DataHexString but got number' ) }) it('did catch invalid invalidAddressArray', async () => { - expect(variableErr.toString()).to.have.string( + expect(validationOutput).to.have.string( 'invalid address for invalidAddressArray: 0x00000000' ) }) it('did catch invalid invalidStringStringMapping', async () => { - expect(variableErr.toString()).to.have.string( + expect(validationOutput).to.have.string( 'invalid input type for invalidStringStringMapping, expected DataHexString but got number' ) }) it('did catch invalid invalidStringIntMapping', async () => { - expect(variableErr.toString()).to.have.string( + expect(validationOutput).to.have.string( 'invalid input type for variable invalidStringIntMapping expected number, string, or BigNumber but got boolean' ) }) it('did catch invalid invalidNestedStringIntBoolMapping', async () => { - expect(variableErr.toString()).to.have.string( + expect(validationOutput).to.have.string( 'invalid input type for variable invalidNestedStringIntBoolMapping expected number, string, or BigNumber but got boolean' ) }) it('did catch missing variables', async () => { - expect(variableErr.toString()).to.have.string( + expect(validationOutput).to.have.string( 'were not defined in the ChugSplash config file' ) - expect(variableErr.toString()).to.have.string('notSetUint') - expect(variableErr.toString()).to.have.string('notSetString') + expect(validationOutput).to.have.string('notSetUint') + expect(validationOutput).to.have.string('notSetString') }) it('did catch extra variables', async () => { - expect(variableErr.toString()).to.have.string( - 'following variables were defined in the ChugSplash config file but do not exist in the contract' + expect(validationOutput).to.have.string( + 'defined in the ChugSplash config file that do not exist in the contract' ) - expect(variableErr.toString()).to.have.string('extraVar') - expect(variableErr.toString()).to.have.string('anotherExtraVar') + expect(validationOutput).to.have.string('extraVar') + expect(validationOutput).to.have.string('anotherExtraVar') }) it('did catch odd dynamic bytes', async () => { - expect(variableErr.toString()).to.have.string( + expect(validationOutput).to.have.string( 'invalid input type for variable oddDynamicBytes, expected DataHexString but got' ) }) it('did catch odd fixed bytes', async () => { - expect(variableErr.toString()).to.have.string( + expect(validationOutput).to.have.string( 'invalid input format for variable oddStaticBytes, expected DataHexString but got' ) }) it('did catch extra constructor argument', async () => { - expect(constructorArgErr.toString()).to.have.string( + expect(validationOutput).to.have.string( 'but are not present in the contract constructor' ) - expect(constructorArgErr.toString()).to.have.string('_immutableUint') + expect(validationOutput).to.have.string('_immutableUint') }) it('did catch missing constructor argument', async () => { - expect(constructorArgErr.toString()).to.have.string( - 'but were not found in your config' - ) - expect(constructorArgErr.toString()).to.have.string('_immutableBytes') + expect(validationOutput).to.have.string('but were not found in your config') + expect(validationOutput).to.have.string('_immutableBytes') }) }) From 5334700a042a96a3a4bf637e14962894c2025eaf Mon Sep 17 00:00:00 2001 From: Ryan Pate Date: Wed, 29 Mar 2023 14:34:07 -0700 Subject: [PATCH 2/2] maint(core): Use OpenZeppelin logic for UUPS upgradeTo check --- packages/core/src/config/parse.ts | 228 ++++++++++---------- packages/core/src/tasks/index.ts | 8 +- packages/core/src/types.ts | 4 +- packages/core/src/utils.ts | 113 ++-------- packages/plugins/src/hardhat/artifacts.ts | 6 +- packages/plugins/src/hardhat/deployments.ts | 6 +- packages/plugins/src/hardhat/tasks.ts | 15 +- packages/plugins/src/utils.ts | 16 +- 8 files changed, 147 insertions(+), 249 deletions(-) diff --git a/packages/core/src/config/parse.ts b/packages/core/src/config/parse.ts index 20170b650..d999de8ed 100644 --- a/packages/core/src/config/parse.ts +++ b/packages/core/src/config/parse.ts @@ -27,7 +27,6 @@ import { getDefaultProxyAddress, isExternalProxyType, readContractArtifact, - assertValidContractReferences, readBuildInfo, getChugSplashManagerProxyAddress, getEIP1967ProxyAdminAddress, @@ -75,7 +74,7 @@ class InputError extends Error { let validationErrors = false const logValidationError = ( - logLevel: 'warning' | 'error' = 'warning', + logLevel: 'warning' | 'error', title: string, lines: string[], silent: boolean, @@ -595,19 +594,10 @@ export const parseInplaceInt: VariableHandler = ( */ export const parseInplaceStruct: VariableHandler< UserConfigVariable, - { - [name: string]: ParsedConfigVariable - } + ParsedConfigVariables > = ( - props: VariableHandlerProps< - UserConfigVariable, - { - [name: string]: ParsedConfigVariable - } - > -): { - [name: string]: ParsedConfigVariable -} => { + props: VariableHandlerProps +): ParsedConfigVariables => { const { variable, variableType, @@ -626,9 +616,7 @@ export const parseInplaceStruct: VariableHandler< } // Structs are encoded recursively, as defined by their `members` field. - const parsedVariable: { - [name: string]: ParsedConfigVariable - } = {} + const parsedVariable: ParsedConfigVariables = {} if (variableType.members === undefined) { // The Solidity compiler prevents defining structs without any members, so this should // never occur. @@ -705,19 +693,10 @@ export const parseBytes: VariableHandler = ( */ export const parseMapping: VariableHandler< UserConfigVariable, - { - [name: string]: ParsedConfigVariable - } + ParsedConfigVariables > = ( - props: VariableHandlerProps< - UserConfigVariable, - { - [name: string]: ParsedConfigVariable - } - > -): { - [name: string]: ParsedConfigVariable -} => { + props: VariableHandlerProps +): ParsedConfigVariables => { const { variable, storageObj, @@ -728,9 +707,7 @@ export const parseMapping: VariableHandler< } = props // Iterate over every key/value in the mapping to get the storage slot pair for each one. - const mapping: { - [name: string]: ParsedConfigVariable - } = {} + const mapping: ParsedConfigVariables = {} for (const [mappingKey, mappingVal] of Object.entries(variable)) { const mappingValStorageObj = buildMappingStorageObj( storageTypes, @@ -908,16 +885,10 @@ const parseContractVariables = ( storageLayout: SolidityStorageLayout, compilerOutput: CompilerOutput, cre: ChugSplashRuntimeEnvironment -): { - variables: { - [name: string]: ParsedConfigVariable - } -} => { - const parsedConfigVariables: { - [name: string]: ParsedConfigVariable - } = {} +): ParsedConfigVariables => { + const parsedConfigVariables: ParsedConfigVariables = {} if (!contractConfig.variables) { - return { variables: {} } + return {} } // Create an AST Dereferencer. We must convert the CompilerOutput type to `any` here because @@ -1032,7 +1003,7 @@ const parseContractVariables = ( } } - return { variables: parsedConfigVariables } + return parsedConfigVariables } /** @@ -1050,7 +1021,7 @@ export const parseAndValidateConstructorArgs = ( referenceName: string, abi: Array, cre: ChugSplashRuntimeEnvironment -): { args: ParsedConfigVariables } => { +): ParsedConfigVariables => { const parsedConstructorArgs: ParsedConfigVariables = {} const constructorFragment = abi.find( @@ -1064,7 +1035,7 @@ export const parseAndValidateConstructorArgs = ( `no constructor exists in the contract.` ) } else { - return { args: parsedConstructorArgs } + return parsedConstructorArgs } } @@ -1125,7 +1096,7 @@ export const parseAndValidateConstructorArgs = ( } } - return { args: parsedConstructorArgs } + return parsedConstructorArgs } export const assertStorageCompatiblePreserveKeywords = ( @@ -1133,7 +1104,7 @@ export const assertStorageCompatiblePreserveKeywords = ( prevStorageLayout: StorageLayout, newStorageLayout: StorageLayout, cre: ChugSplashRuntimeEnvironment -): boolean => { +) => { const prevDetailedLayout = getDetailedLayout(prevStorageLayout) const newDetailedLayout = getDetailedLayout(newStorageLayout) @@ -1172,10 +1143,86 @@ export const assertStorageCompatiblePreserveKeywords = ( cre.silent, cre.stream ) - return false } +} + +/** + * Throws an error if the given variable contains any invalid contract references. Specifically, + * it'll throw an error if any of the following conditions occur: + * + * 1. There are any leading spaces before '{{', or any trailing spaces after '}}'. This ensures the + * template string converts into a valid address when it's parsed. If there are any leading or + * trailing spaces in an address, `ethers.utils.isAddress` will return false. + * + * 2. The contract reference is not included in the array of valid contract references. + * + * @param variable Config variable defined by the user. + * @param referenceNames Valid reference names for this ChugSplash config file. + */ +export const assertValidContractReferences = ( + variable: UserConfigVariable, + referenceNames: string[], + cre: ChugSplashRuntimeEnvironment +) => { + if ( + typeof variable === 'string' && + variable.includes('{{') && + variable.includes('}}') + ) { + if (!variable.startsWith('{{')) { + logValidationError( + 'error', + `Contract reference cannot contain leading spaces before '{{' : ${variable}`, + [], + cre.silent, + cre.stream + ) + } + if (!variable.endsWith('}}')) { + logValidationError( + 'error', + `Contract reference cannot contain trailing spaces: ${variable}`, + [], + cre.silent, + cre.stream + ) + } - return true + const contractReference = variable.substring(2, variable.length - 2).trim() + + if (!referenceNames.includes(contractReference)) { + logValidationError( + 'error', + `Invalid contract reference: ${variable}.\nDid you misspell this contract reference, or forget to define a contract with this reference name?`, + [], + cre.silent, + cre.stream + ) + } + } else if (Array.isArray(variable)) { + for (const element of variable) { + assertValidContractReferences(element, referenceNames, cre) + } + } else if (typeof variable === 'object') { + for (const [varName, varValue] of Object.entries(variable)) { + assertValidContractReferences(varName, referenceNames, cre) + assertValidContractReferences(varValue, referenceNames, cre) + } + } else if ( + typeof variable === 'boolean' || + typeof variable === 'number' || + typeof variable === 'string' + ) { + return + } else { + logValidationError( + 'error', + `Detected unknown variable type, ${typeof variable}, for variable: ${variable}.`, + [], + cre.silent, + cre.stream + ) + } } export const assertValidParsedChugSplashFile = async ( @@ -1183,11 +1230,8 @@ export const assertValidParsedChugSplashFile = async ( parsedConfig: ParsedChugSplashConfig, userConfig: UserChugSplashConfig, artifactPaths: ArtifactPaths, - integration: Integration, cre: ChugSplashRuntimeEnvironment -): Promise<{ - upgrade: boolean -}> => { +): Promise => { const { canonicalConfigPath, remoteExecution } = cre // Determine if the deployment is an upgrade @@ -1362,39 +1406,6 @@ permission to call the 'upgradeTo' function on each of them. ) } } - - // Check new UUPS implementations include a public `upgradeTo` function. This ensures that the - // user will be able to upgrade the proxy in the future. - if ( - contractConfig.proxyType === 'oz-ownable-uups' || - contractConfig.proxyType === 'oz-access-control-uups' - ) { - const artifact = readContractArtifact( - artifactPaths[referenceName].contractArtifactPath, - integration - ) - const containsPublicUpgradeTo = artifact.abi.some( - (fragment) => - fragment.name === 'upgradeTo' && - fragment.inputs.length === 1 && - fragment.inputs[0].type === 'address' - ) - if ( - !containsPublicUpgradeTo && - !userConfig.contracts[referenceName].unsafeAllow - ?.missingPublicUpgradeTo - ) { - logValidationError( - 'error', - `Contract ${referenceName} proxy type is marked as UUPS, but the new implementation \n no longer has a public 'upgradeTo(address)' function.`, - [ - 'You must include this function or you will no longer be able to upgrade this contract.', - ], - cre.silent, - cre.stream - ) - } - } } else { // Perform initial deployment specific validation @@ -1416,7 +1427,7 @@ permission to call the 'upgradeTo' function on each of them. } } - return { upgrade: isUpgrade } + return isUpgrade } export const assertValidContracts = ( @@ -1648,25 +1659,29 @@ const parseAndValidateChugSplashConfig = async ( for (const [referenceName, userContractConfig] of Object.entries( userConfig.contracts )) { - try { - if ( - userContractConfig.externalProxy !== undefined && - (await provider.getCode(userContractConfig.externalProxy)) === '0x' - ) { - logValidationError( - 'error', - `Entered a proxy address that does not exist: ${userContractConfig.externalProxy}`, - [], - cre.silent, - cre.stream - ) - } - } catch { + // throw an error if the external proxy is not a valid address + if ( + userContractConfig.externalProxy && + !ethers.utils.isAddress(userContractConfig.externalProxy) + ) { throw new Error( `Invalid proxy address: ${userContractConfig.externalProxy}` ) } + if ( + userContractConfig.externalProxy !== undefined && + (await provider.getCode(userContractConfig.externalProxy)) === '0x' + ) { + logValidationError( + 'error', + `Entered a proxy address that does not exist: ${userContractConfig.externalProxy}`, + [], + cre.silent, + cre.stream + ) + } + const { externalProxy, externalProxyType, constructorArgs } = userContractConfig @@ -1698,7 +1713,7 @@ const parseAndValidateChugSplashConfig = async ( const storageLayout = compilerOutput.contracts[sourceName][contractName].storageLayout - const { variables: parsedVariables } = parseContractVariables( + const parsedVariables = parseContractVariables( JSON.parse( Handlebars.compile(JSON.stringify(userContractConfig))({ ...contracts, @@ -1709,7 +1724,7 @@ const parseAndValidateChugSplashConfig = async ( cre ) - const { args } = parseAndValidateConstructorArgs( + const args = parseAndValidateConstructorArgs( constructorArgs ?? {}, referenceName, compilerOutput.contracts[sourceName][contractName].abi, @@ -1729,12 +1744,11 @@ const parseAndValidateChugSplashConfig = async ( assertValidContracts(parsedConfig, artifactPaths, cre) - const { upgrade } = await assertValidParsedChugSplashFile( + const upgrade = await assertValidParsedChugSplashFile( provider, parsedConfig, userConfig, artifactPaths, - integration, cre ) @@ -1756,9 +1770,5 @@ const parseAndValidateChugSplashConfig = async ( process.exit(1) } - return JSON.parse( - Handlebars.compile(JSON.stringify(parsedConfig))({ - ...contracts, - }) - ) + return parsedConfig } diff --git a/packages/core/src/tasks/index.ts b/packages/core/src/tasks/index.ts index 52046811d..5555fbd01 100644 --- a/packages/core/src/tasks/index.ts +++ b/packages/core/src/tasks/index.ts @@ -77,7 +77,7 @@ import { export const chugsplashRegisterAbstractTask = async ( provider: ethers.providers.JsonRpcProvider, signer: ethers.Signer, - parsedConfig: UserChugSplashConfig | ParsedChugSplashConfig, + config: UserChugSplashConfig, allowManagedProposals: boolean, owner: string, integration: Integration, @@ -85,20 +85,20 @@ export const chugsplashRegisterAbstractTask = async ( ) => { const spinner = ora({ isSilent: cre.silent, stream: cre.stream }) - spinner.start(`Registering ${parsedConfig.options.projectName}...`) + spinner.start(`Registering ${config.options.projectName}...`) const isFirstTimeRegistered = await registerChugSplashProject( provider, signer, await signer.getAddress(), - parsedConfig.options.projectName, + config.options.projectName, owner, allowManagedProposals ) const networkName = await resolveNetworkName(provider, integration) - const projectName = parsedConfig.options.projectName + const projectName = config.options.projectName await trackRegistered( await getProjectOwnerAddress(signer, projectName), projectName, diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts index c536ea3b4..aeb197594 100644 --- a/packages/core/src/types.ts +++ b/packages/core/src/types.ts @@ -14,8 +14,8 @@ export type ChugSplashRuntimeEnvironment = { stream: NodeJS.WritableStream silent: boolean hre: HardhatRuntimeEnvironment | undefined - fetchOpenZeppelinStorageLayout: ( - hre: HardhatRuntimeEnvironment | undefined, + importOpenZeppelinStorageLayout: ( + hre: HardhatRuntimeEnvironment, parsedContractConfig: ParsedContractConfig ) => Promise } diff --git a/packages/core/src/utils.ts b/packages/core/src/utils.ts index b67fe43f2..5feb95634 100644 --- a/packages/core/src/utils.ts +++ b/packages/core/src/utils.ts @@ -57,7 +57,6 @@ import { ParsedConfigVariables, ParsedContractConfig, ProxyType, - UserConfigVariable, UserContractConfig, } from './config/types' import { ChugSplashActionBundle, ChugSplashActionType } from './actions/types' @@ -658,97 +657,6 @@ export const isExternalProxyType = ( return externalProxyTypes.includes(proxyType) } -/** - * Throws an error if the given variable contains any invalid contract references. Specifically, - * it'll throw an error if any of the following conditions occur: - * - * 1. There are any leading spaces before '{{', or any trailing spaces after '}}'. This ensures the - * template string converts into a valid address when it's parsed. If there are any leading or - * trailing spaces in an address, `ethers.utils.isAddress` will return false. - * - * 2. The contract reference is not included in the array of valid contract references. - * - * @param variable Config variable defined by the user. - * @param referenceNames Valid reference names for this ChugSplash config file. - */ -export const assertValidContractReferences = ( - variable: UserConfigVariable, - referenceNames: string[], - cre: ChugSplashRuntimeEnvironment -): boolean => { - let isValid = true - if ( - typeof variable === 'string' && - variable.includes('{{') && - variable.includes('}}') - ) { - if (!variable.startsWith('{{')) { - isValid = false - chugsplashLog( - 'error', - `Contract reference cannot contain leading spaces before '{{' : ${variable}`, - [], - cre.silent, - cre.stream - ) - } - if (!variable.endsWith('}}')) { - isValid = false - chugsplashLog( - 'error', - `Contract reference cannot contain trailing spaces: ${variable}`, - [], - cre.silent, - cre.stream - ) - } - - const contractReference = variable.substring(2, variable.length - 2).trim() - - if (!referenceNames.includes(contractReference)) { - isValid = false - chugsplashLog( - 'error', - `Invalid contract reference: ${variable}.\nDid you misspell this contract reference, or forget to define a contract with this reference name?`, - [], - cre.silent, - cre.stream - ) - } - } else if (Array.isArray(variable)) { - for (const element of variable) { - if (!assertValidContractReferences(element, referenceNames, cre)) { - isValid = false - } - } - } else if (typeof variable === 'object') { - for (const [varName, varValue] of Object.entries(variable)) { - if ( - !assertValidContractReferences(varName, referenceNames, cre) || - !assertValidContractReferences(varValue, referenceNames, cre) - ) { - isValid = false - } - } - } else if ( - typeof variable === 'boolean' || - typeof variable === 'number' || - typeof variable === 'string' - ) { - return isValid - } else { - chugsplashLog( - 'error', - `Detected unknown variable type, ${typeof variable}, for variable: ${variable}.`, - [], - cre.silent, - cre.stream - ) - } - - return isValid -} - export const getParentContractASTNodes = ( compilerOutputSources: CompilerOutputSources, parentContractNodeAstIds: Array @@ -1032,7 +940,6 @@ export const getOpenZeppelinValidationOpts = ( 'state-variable-assignment', 'constructor', 'state-variable-immutable', - 'missing-public-upgradeto', ] if (contractConfig.unsafeAllow?.delegatecall) { unsafeAllow.push('delegatecall') @@ -1040,6 +947,9 @@ export const getOpenZeppelinValidationOpts = ( if (contractConfig.unsafeAllow?.selfdestruct) { unsafeAllow.push('selfdestruct') } + if (contractConfig.unsafeAllow?.missingPublicUpgradeTo) { + unsafeAllow.push('missing-public-upgradeto') + } const options = { kind: toOpenZeppelinProxyType(proxyType), @@ -1098,11 +1008,6 @@ export const getPreviousStorageLayoutOZFormat = async ( canonicalConfigFolderPath: string, cre: ChugSplashRuntimeEnvironment ): Promise => { - const openzeppelinStorageLayout = await cre.fetchOpenZeppelinStorageLayout( - cre.hre, - parsedContractConfig - ) - if ((await provider.getCode(parsedContractConfig.proxy)) === '0x') { throw new Error( `Proxy has not been deployed for the contract: ${referenceName}.` @@ -1153,7 +1058,17 @@ export const getPreviousStorageLayoutOZFormat = async ( parsedContractConfig.proxyType, userContractConfig ).layout - } else if (openzeppelinStorageLayout !== undefined) { + } else if (cre.hre !== undefined) { + const openzeppelinStorageLayout = await cre.importOpenZeppelinStorageLayout( + cre.hre, + parsedContractConfig + ) + if (!openzeppelinStorageLayout) { + throw new Error( + 'Should not attempt to import OpenZeppelin storage layout for non-OpenZeppelin proxy type. Please report this to the developers.' + ) + } + return openzeppelinStorageLayout } else { throw new Error( diff --git a/packages/plugins/src/hardhat/artifacts.ts b/packages/plugins/src/hardhat/artifacts.ts index 3a505d85f..d11eac485 100644 --- a/packages/plugins/src/hardhat/artifacts.ts +++ b/packages/plugins/src/hardhat/artifacts.ts @@ -101,7 +101,7 @@ export const getArtifactPaths = async ( export const importOpenZeppelinStorageLayout = async ( hre: HardhatRuntimeEnvironment, parsedContractConfig: ParsedContractConfig -): Promise => { +): Promise => { const { proxyType } = parsedContractConfig if ( proxyType === 'oz-transparent' || @@ -128,8 +128,4 @@ export const importOpenZeppelinStorageLayout = async ( return storageLayout } } - - throw new Error( - 'Should not attempt to import OpenZeppelin storage layout for non-OpenZeppelin proxy type. Please report this to the developers.' - ) } diff --git a/packages/plugins/src/hardhat/deployments.ts b/packages/plugins/src/hardhat/deployments.ts index c16dcf638..fadca0dcd 100644 --- a/packages/plugins/src/hardhat/deployments.ts +++ b/packages/plugins/src/hardhat/deployments.ts @@ -47,7 +47,7 @@ export const deployAllChugSplashConfigs = async ( silent: boolean, ipfsUrl: string, fileNames?: string[] -): Promise => { +) => { const remoteExecution = await isRemoteExecution(hre) fileNames = fileNames ?? (await fetchFilesRecursively(hre.config.paths.chugsplash)) @@ -71,7 +71,7 @@ export const deployAllChugSplashConfigs = async ( // Skip this config if it's empty. if (isEmptyChugSplashConfig(configPath)) { - return true + continue } const userConfig = await readUnvalidatedChugSplashConfig(configPath) @@ -109,8 +109,6 @@ export const deployAllChugSplashConfigs = async ( executor ) } - - return true } export const getContract = async ( diff --git a/packages/plugins/src/hardhat/tasks.ts b/packages/plugins/src/hardhat/tasks.ts index 0ada03520..5deedd1e2 100644 --- a/packages/plugins/src/hardhat/tasks.ts +++ b/packages/plugins/src/hardhat/tasks.ts @@ -816,10 +816,7 @@ task(TASK_NODE) quiet: true, }) } - const succeeded = await deployAllChugSplashConfigs(hre, hide, '') - if (succeeded === false) { - return - } + await deployAllChugSplashConfigs(hre, hide, '') const networkName = await resolveNetworkName( hre.ethers.provider, 'hardhat' @@ -889,15 +886,12 @@ task(TASK_TEST) }) } if (!skipDeploy) { - const succeeded = await deployAllChugSplashConfigs( + await deployAllChugSplashConfigs( hre, !show, '', configPath ? [configPath] : undefined ) - if (succeeded === false) { - return - } } } await writeSnapshotId( @@ -942,10 +936,7 @@ task(TASK_RUN) quiet: true, }) } - const succeeded = await deployAllChugSplashConfigs(hre, true, '') - if (succeeded === false) { - return - } + await deployAllChugSplashConfigs(hre, true, '') } await runSuper(args) } diff --git a/packages/plugins/src/utils.ts b/packages/plugins/src/utils.ts index 32b6d661f..616c3caee 100644 --- a/packages/plugins/src/utils.ts +++ b/packages/plugins/src/utils.ts @@ -1,20 +1,8 @@ -import { - ChugSplashRuntimeEnvironment, - ParsedContractConfig, -} from '@chugsplash/core' +import { ChugSplashRuntimeEnvironment } from '@chugsplash/core' import { HardhatRuntimeEnvironment } from 'hardhat/types' import { importOpenZeppelinStorageLayout } from './hardhat/artifacts' -const fetchOpenZeppelinStorageLayout = async ( - hre: HardhatRuntimeEnvironment | undefined = undefined, - parsedContractConfig: ParsedContractConfig -) => { - return hre - ? importOpenZeppelinStorageLayout(hre, parsedContractConfig) - : undefined -} - export const createChugSplashRuntime = async ( configPath: string, remoteExecution: boolean, @@ -30,7 +18,7 @@ export const createChugSplashRuntime = async ( autoConfirm, stream, silent, - fetchOpenZeppelinStorageLayout, + importOpenZeppelinStorageLayout, hre, } }