Merge pull request sphinx-labs#573 from chugsplash/pate/chu-171-outpu…

…t-all-validation-errors-at-once

feat(core): Display all parsing validation errors

packages/core/src/tasks/index.ts

@@ -12,6 +12,7 @@ import {
   ParsedChugSplashConfig,
   proxyTypeHashes,
   readUnvalidatedChugSplashConfig,
+  UserChugSplashConfig,
   verifyBundle,
 } from '../config'
 import {
@@ -76,28 +77,28 @@ import {
 export const chugsplashRegisterAbstractTask = async (
   provider: ethers.providers.JsonRpcProvider,
   signer: ethers.Signer,
-  parsedConfig: ParsedChugSplashConfig,
+  config: UserChugSplashConfig,
   allowManagedProposals: boolean,
   owner: string,
   integration: Integration,
   cre: ChugSplashRuntimeEnvironment
 ) => {
   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,

packages/core/src/types.ts

@@ -1,20 +1,23 @@
 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 = {
   configPath: string
   canonicalConfigPath: string | undefined
   remoteExecution: boolean
   autoConfirm: boolean
-  openzeppelinStorageLayouts:
-    | {
-        [referenceName: string]: any
-      }
-    | undefined
   stream: NodeJS.WritableStream
   silent: boolean
+  hre: HardhatRuntimeEnvironment | undefined
+  importOpenZeppelinStorageLayout: (
+    hre: HardhatRuntimeEnvironment,
+    parsedContractConfig: ParsedContractConfig
+  ) => Promise<StorageLayout | undefined>
 }
 
 export type FoundryContractArtifact = {

packages/core/src/utils.ts

@@ -57,13 +57,12 @@ import {
   ParsedConfigVariables,
   ParsedContractConfig,
   ProxyType,
-  UserConfigVariable,
   UserContractConfig,
 } from './config/types'
 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,
@@ -658,69 +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[]
-) => {
-  if (
-    typeof variable === 'string' &&
-    variable.includes('{{') &&
-    variable.includes('}}')
-  ) {
-    if (!variable.startsWith('{{')) {
-      throw new Error(
-        `Contract reference cannot contain leading spaces before '{{' : ${variable}`
-      )
-    }
-    if (!variable.endsWith('}}')) {
-      throw new Error(
-        `Contract reference cannot contain trailing spaces: ${variable}`
-      )
-    }
-
-    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?`
-      )
-    }
-  } else if (Array.isArray(variable)) {
-    for (const element of variable) {
-      assertValidContractReferences(element, referenceNames)
-    }
-  } else if (typeof variable === 'object') {
-    for (const [varName, varValue] of Object.entries(variable)) {
-      assertValidContractReferences(varName, referenceNames)
-      assertValidContractReferences(varValue, referenceNames)
-    }
-  } else if (
-    typeof variable === 'boolean' ||
-    typeof variable === 'number' ||
-    typeof variable === 'string'
-  ) {
-    return
-  } else {
-    throw new Error(
-      `Detected unknown variable type, ${typeof variable}, for variable: ${variable}.`
-    )
-  }
-}
-
 export const getParentContractASTNodes = (
   compilerOutputSources: CompilerOutputSources,
   parentContractNodeAstIds: Array<number>
@@ -1004,14 +940,16 @@ export const getOpenZeppelinValidationOpts = (
     'state-variable-assignment',
     'constructor',
     'state-variable-immutable',
-    'missing-public-upgradeto',
   ]
   if (contractConfig.unsafeAllow?.delegatecall) {
     unsafeAllow.push('delegatecall')
   }
   if (contractConfig.unsafeAllow?.selfdestruct) {
     unsafeAllow.push('selfdestruct')
   }
+  if (contractConfig.unsafeAllow?.missingPublicUpgradeTo) {
+    unsafeAllow.push('missing-public-upgradeto')
+  }
 
   const options = {
     kind: toOpenZeppelinProxyType(proxyType),
@@ -1068,9 +1006,7 @@ export const getPreviousStorageLayoutOZFormat = async (
   userContractConfig: UserContractConfig,
   remoteExecution: boolean,
   canonicalConfigFolderPath: string,
-  openzeppelinStorageLayouts?: {
-    [referenceName: string]: StorageLayout
-  }
+  cre: ChugSplashRuntimeEnvironment
 ): Promise<StorageLayout> => {
   if ((await provider.getCode(parsedContractConfig.proxy)) === '0x') {
     throw new Error(
@@ -1122,8 +1058,18 @@ export const getPreviousStorageLayoutOZFormat = async (
       parsedContractConfig.proxyType,
       userContractConfig
     ).layout
-  } else if (openzeppelinStorageLayouts?.[referenceName] !== undefined) {
-    return openzeppelinStorageLayouts[referenceName]
+  } 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(
       `Could not find the previous storage layout for the contract: ${referenceName}. Please include\n` +

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,

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()
 

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,43 +98,34 @@ 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<StorageLayout | undefined> => {
+  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(
         manifest,
         deployData.validations,
         await getEIP1967ProxyImplementationAddress(hre.ethers.provider, proxy)
       )
-      layouts[referenceName] = storageLayout
+      return storageLayout
     }
   }
-
-  return layouts
 }

packages/plugins/src/hardhat/deployments.ts

@@ -71,7 +71,7 @@ export const deployAllChugSplashConfigs = async (
 
     // Skip this config if it's empty.
     if (isEmptyChugSplashConfig(configPath)) {
-      return
+      continue
     }
     const userConfig = await readUnvalidatedChugSplashConfig(configPath)
 

packages/plugins/src/utils.ts

@@ -1,10 +1,7 @@
-import {
-  ChugSplashRuntimeEnvironment,
-  readUnvalidatedChugSplashConfig,
-} from '@chugsplash/core'
+import { ChugSplashRuntimeEnvironment } from '@chugsplash/core'
 import { HardhatRuntimeEnvironment } from 'hardhat/types'
 
-import { importOpenZeppelinStorageLayouts } from './hardhat/artifacts'
+import { importOpenZeppelinStorageLayout } from './hardhat/artifacts'
 
 export const createChugSplashRuntime = async (
   configPath: string,
@@ -14,18 +11,14 @@ export const createChugSplashRuntime = async (
   silent: boolean,
   stream: NodeJS.WritableStream = process.stderr
 ): Promise<ChugSplashRuntimeEnvironment> => {
-  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,
+    importOpenZeppelinStorageLayout,
+    hre,
   }
 }