fix(core): add support for structs in constructor args

hujw77 · May 8, 2023 · ff58a7d · ff58a7d
1 parent 3f6f1f5
commit ff58a7d
Show file tree

Hide file tree

Showing 15 changed files with 271 additions and 58 deletions.
diff --git a/.changeset/warm-needles-double.md b/.changeset/warm-needles-double.md
@@ -0,0 +1,6 @@
+---
+'@chugsplash/plugins': patch
+'@chugsplash/core': patch
+---
+
+Add support for struct constructor args
diff --git a/packages/core/src/actions/artifacts.ts b/packages/core/src/actions/artifacts.ts
@@ -127,7 +127,7 @@ export const writeDeploymentArtifacts = async (
       const buildInfo = readBuildInfo(
         artifactPaths[referenceName].buildInfoPath
       )
-      const { constructorArgValues } = getConstructorArgs(
+      const constructorArgValues = getConstructorArgs(
         parsedConfig.contracts[referenceName].constructorArgs,
         abi
       )

diff --git a/packages/core/src/config/parse.ts b/packages/core/src/config/parse.ts
@@ -97,7 +97,9 @@ const logValidationError = (
   silent: boolean,
   stream: NodeJS.WritableStream
 ) => {
-  validationErrors = true
+  if (logLevel === 'error') {
+    validationErrors = true
+  }
   chugsplashLog(logLevel, title, lines, silent, stream)
 }
 
@@ -681,13 +683,23 @@ const parseUnsignedInteger = (
     .pow(8 * numberOfBytes)
     .sub(1)
 
-  if (
-    remove0x(BigNumber.from(variable).toHexString()).length / 2 >
-    numberOfBytes
-  ) {
-    throw new Error(
-      `invalid value for ${label}: ${variable}, outside valid range: [0:${maxValue}]`
-    )
+  try {
+    if (
+      remove0x(BigNumber.from(variable).toHexString()).length / 2 >
+      numberOfBytes
+    ) {
+      throw new Error(
+        `invalid value for ${label}: ${variable}, outside valid range: [0:${maxValue}]`
+      )
+    }
+  } catch (e) {
+    if (e.message.includes('invalid BigNumber string')) {
+      throw new Error(
+        `invalid value for ${label}, expected a valid number but got: ${variable}`
+      )
+    } else {
+      throw e
+    }
   }
 
   return BigNumber.from(variable).toString()
@@ -746,13 +758,23 @@ const parseInteger = (
     .pow(8 * numberOfBytes)
     .div(2)
     .sub(1)
-  if (
-    BigNumber.from(variable).lt(minValue) ||
-    BigNumber.from(variable).gt(maxValue)
-  ) {
-    throw new Error(
-      `invalid value for ${label}: ${variable}, outside valid range: [${minValue}:${maxValue}]`
-    )
+  try {
+    if (
+      BigNumber.from(variable).lt(minValue) ||
+      BigNumber.from(variable).gt(maxValue)
+    ) {
+      throw new Error(
+        `invalid value for ${label}: ${variable}, outside valid range: [${minValue}:${maxValue}]`
+      )
+    }
+  } catch (e) {
+    if (e.message.includes('invalid BigNumber string')) {
+      throw new Error(
+        `invalid value for ${label}, expected a valid number but got: ${variable}`
+      )
+    } else {
+      throw e
+    }
   }
 
   return BigNumber.from(variable).toString()
@@ -802,7 +824,7 @@ export const parseInplaceStruct: VariableHandler<
     })
     if (memberStorageObj === undefined) {
       throw new InputError(
-        `User entered incorrect member in ${variableType.label}: ${varName}`
+        `Extra member(s) detected in ${variableType.label}, ${storageObj.label}: ${varName}`
       )
     }
     parsedVariable[varName] = parseAndValidateVariable(
@@ -814,6 +836,21 @@ export const parseInplaceStruct: VariableHandler<
     )
   }
 
+  // Find any members missing from the struct
+  const missingMembers: string[] = []
+  for (const member of variableType.members) {
+    if (parsedVariable[member.label] === undefined) {
+      missingMembers.push(member.label)
+    }
+  }
+
+  if (missingMembers.length > 0) {
+    throw new InputError(
+      `Missing member(s) in struct ${variableType.label}, ${storageObj.label}: ` +
+        missingMembers.join(', ')
+    )
+  }
+
   return parsedVariable
 }
 
@@ -1224,7 +1261,9 @@ const parseContractVariables = (
 
 const parseArrayConstructorArg = (
   input: ParamType,
-  constructorArgValue: UserConfigVariable
+  name: string,
+  constructorArgValue: UserConfigVariable,
+  cre: ChugSplashRuntimeEnvironment
 ): ParsedConfigVariable[] => {
   if (!Array.isArray(constructorArgValue)) {
     throw new InputError(
@@ -1235,15 +1274,15 @@ const parseArrayConstructorArg = (
   if (input.arrayLength !== -1) {
     if (constructorArgValue.length !== input.arrayLength) {
       throw new InputError(
-        `Expected array of length ${input.arrayLength} for ${input.name} but got array of length ${constructorArgValue.length}`
+        `Expected array of length ${input.arrayLength} for ${name} but got array of length ${constructorArgValue.length}`
       )
     }
   }
 
   const parsedValues: ParsedConfigVariable = []
   for (const element of constructorArgValue) {
     parsedValues.push(
-      parseAndValidateConstructorArg(input.arrayChildren, element)
+      parseAndValidateConstructorArg(input.arrayChildren, name, element, cre)
     )
   }
 
@@ -1252,7 +1291,9 @@ const parseArrayConstructorArg = (
 
 export const parseStructConstructorArg = (
   paramType: ParamType,
-  constructorArgValue: UserConfigVariable
+  name: string,
+  constructorArgValue: UserConfigVariable,
+  cre: ChugSplashRuntimeEnvironment
 ) => {
   if (typeof constructorArgValue !== 'object') {
     throw new InputError(
@@ -1262,33 +1303,58 @@ export const parseStructConstructorArg = (
     )
   }
 
+  const memberErrors: string[] = []
   const parsedValues: ParsedConfigVariable = {}
   for (const [key, value] of Object.entries(constructorArgValue)) {
     const inputChild = paramType.components.find((component) => {
       return component.name === key
     })
     if (inputChild === undefined) {
-      throw new InputError(
-        `User entered incorrect member in ${paramType.name}: ${key}`
+      memberErrors.push(`Extra member(s) in struct ${paramType.name}: ${key}`)
+    } else {
+      parsedValues[key] = parseAndValidateConstructorArg(
+        inputChild,
+        `${name}.${key}`,
+        value,
+        cre
       )
     }
-    parsedValues[key] = parseAndValidateConstructorArg(inputChild, value)
+  }
+
+  // Find any members missing from the struct
+  const missingMembers: string[] = []
+  for (const member of paramType.components) {
+    if (parsedValues[member.name] === undefined) {
+      missingMembers.push(member.name)
+    }
+  }
+
+  if (missingMembers.length > 0) {
+    memberErrors.push(
+      `Missing member(s) in struct ${paramType.name}: ` +
+        missingMembers.join(', ')
+    )
+  }
+
+  if (memberErrors.length > 0) {
+    throw new InputError(memberErrors.join('\n'))
   }
 
   return parsedValues
 }
 
 const parseAndValidateConstructorArg = (
   input: ParamType,
-  constructorArgValue: UserConfigVariable
+  name: string,
+  constructorArgValue: UserConfigVariable,
+  cre: ChugSplashRuntimeEnvironment
 ): ParsedConfigVariable => {
   const constructorArgType = input.type
   // We fetch a new ParamType using the input type even though input is a ParamType object
   // This is b/c input is an incomplete object, so fetching the new ParamType yields
   // an object with more useful information on it
   const paramType =
     input.type === 'tuple' ? input : ethers.utils.ParamType.from(input.type)
-  const name = input.name
   if (
     paramType.baseType &&
     (paramType.baseType.startsWith('uint') ||
@@ -1334,7 +1400,9 @@ const parseAndValidateConstructorArg = (
   } else if (paramType.baseType === 'string') {
     return parseBytes(constructorArgValue, name, paramType.type, 0)
   } else if (paramType.baseType === 'array') {
-    return parseArrayConstructorArg(paramType, constructorArgValue)
+    return parseArrayConstructorArg(paramType, name, constructorArgValue, cre)
+  } else if (paramType.type === 'tuple') {
+    return parseStructConstructorArg(paramType, name, constructorArgValue, cre)
   } else {
     // throw or log error
     throw new InputError(
@@ -1414,7 +1482,9 @@ export const parseContractConstructorArgs = (
     try {
       parsedConstructorArgs[input.name] = parseAndValidateConstructorArg(
         input,
-        constructorArgValue
+        input.name,
+        constructorArgValue,
+        cre
       )
     } catch (e) {
       inputFormatErrors.push((e as Error).message)
@@ -1958,7 +2028,10 @@ export const assertValidContracts = (
         }
       }
 
-      if (!contractConfig.unsafeAllowEmptyPush) {
+      if (
+        !contractConfig.unsafeAllowEmptyPush &&
+        contractConfig.kind !== 'no-proxy'
+      ) {
         for (const memberAccessNode of findAll('MemberAccess', contractDef)) {
           const typeIdentifier =
             memberAccessNode.expression.typeDescriptions.typeIdentifier

diff --git a/packages/core/src/etherscan.ts b/packages/core/src/etherscan.ts
@@ -94,7 +94,7 @@ export const verifyChugSplashConfig = async (
   )) {
     const { artifact, buildInfo } = artifacts[referenceName]
     const { abi, contractName, sourceName } = artifact
-    const { constructorArgValues } = getConstructorArgs(
+    const constructorArgValues = getConstructorArgs(
       canonicalConfig.contracts[referenceName].constructorArgs,
       abi
     )

diff --git a/packages/core/src/utils.ts b/packages/core/src/utils.ts
@@ -831,43 +831,35 @@ export const getContractAddress = (
 export const getConstructorArgs = (
   constructorArgs: ParsedConfigVariables,
   abi: Array<Fragment>
-): {
-  constructorArgTypes: Array<string>
-  constructorArgValues: Array<ParsedConfigVariable>
-} => {
-  const constructorArgTypes: Array<string> = []
+): Array<ParsedConfigVariable> => {
   const constructorArgValues: Array<ParsedConfigVariable> = []
 
   const constructorFragment = abi.find(
     (fragment) => fragment.type === 'constructor'
   )
 
   if (constructorFragment === undefined) {
-    return { constructorArgTypes, constructorArgValues }
+    return constructorArgValues
   }
 
   constructorFragment.inputs.forEach((input) => {
-    constructorArgTypes.push(input.type)
     constructorArgValues.push(constructorArgs[input.name])
   })
 
-  return { constructorArgTypes, constructorArgValues }
+  return constructorArgValues
 }
 
 export const getCreationCodeWithConstructorArgs = (
   bytecode: string,
   constructorArgs: ParsedConfigVariables,
   abi: ContractArtifact['abi']
 ): string => {
-  const { constructorArgTypes, constructorArgValues } = getConstructorArgs(
-    constructorArgs,
-    abi
-  )
+  const constructorArgValues = getConstructorArgs(constructorArgs, abi)
+
+  const iface = new ethers.utils.Interface(abi)
 
   const creationCodeWithConstructorArgs = bytecode.concat(
-    remove0x(
-      utils.defaultAbiCoder.encode(constructorArgTypes, constructorArgValues)
-    )
+    remove0x(iface.encodeDeploy(constructorArgValues))
   )
 
   return creationCodeWithConstructorArgs

diff --git a/packages/executor/.env.example b/packages/executor/.env.example
@@ -1,13 +1,15 @@
+# Required for all environments
+CHUGSPLASH_EXECUTOR__NETWORK=<name of the target network, used to fetch the etherscan verification endpoint>
+CHUGSPLASH_EXECUTOR__PORT=7300
 CHUGSPLASH_EXECUTOR__PRIVATE_KEYS=<private key to deploy with>
 CHUGSPLASH_EXECUTOR__URL=<url of the network to monitor, http://host.docker.internal:8545 for local node>
-CHUGSPLASH_EXECUTOR__NETWORK=<name of the target network, used to fetch the etherscan verification endpoint>
+IPFS_PROJECT_ID=<ipfs project id to retrieve config file>
+IPFS_API_KEY_SECRET=<ipfs api key to retrieve config file>
+
+# Required in production
 CHUGSPLASH_EXECUTOR__AMPLITUDE_KEY=<amplitude api key for analytics (optional)>
 CHUGSPLASH_EXECUTOR__LOG_LEVEL=<filter logs based on this level, i.e 'error', 'warning', 'info'>
 CHUGSPLASH_EXECUTOR__LOOP_INTERVAL_MS=<amount of time to wait between loops>
-CHUGSPLASH_EXECUTOR__PORT=7300
 CHUGSPLASH_EXECUTOR__MANAGED_API_URL=<chugsplash managed api endpoint>
 MANAGED_PUBLIC_KEY=<public key for accessing the chugsplash managed api>
-IPFS_PROJECT_ID=<ipfs project id to retrieve config file>
-IPFS_API_KEY_SECRET=<ipfs api key to retrieve config file>
 ETHERSCAN_API_KEY=<Etherscan API key>
-HARDHAT_NETWORK=<Target hardhat network, used for verification>
diff --git a/packages/plugins/chugsplash/ConstructorArgValidation.config.ts b/packages/plugins/chugsplash/ConstructorArgValidation.config.ts
@@ -2,8 +2,8 @@ import { UserChugSplashConfig } from '@chugsplash/core'
 import { ethers } from 'ethers'
 
 import {
-  invalidValueTypesPartOne,
-  invalidValueTypesPartTwo,
+  invalidConstructorArgsPartOne,
+  invalidConstructorArgsPartTwo,
 } from '../test/constants'
 
 const projectName = 'Constructor Args Validation'
@@ -21,14 +21,14 @@ const config: UserChugSplashConfig = {
     ConstructorArgsValidationPartOne: {
       contract: 'ConstructorArgsValidationPartOne',
       constructorArgs: {
-        ...invalidValueTypesPartOne,
+        ...invalidConstructorArgsPartOne,
         _immutableUint: 1,
       },
     },
     ConstructorArgsValidationPartTwo: {
       contract: 'ConstructorArgsValidationPartTwo',
       constructorArgs: {
-        ...invalidValueTypesPartTwo,
+        ...invalidConstructorArgsPartTwo,
       },
     },
   },

diff --git a/packages/plugins/chugsplash/VariableValidation.config.ts b/packages/plugins/chugsplash/VariableValidation.config.ts
@@ -53,6 +53,14 @@ const config: UserChugSplashConfig = {
             testKey: true,
           },
         },
+        extraMemberStruct: {
+          a: 1,
+          b: 2,
+          c: 3,
+        },
+        missingMemberStruct: {
+          b: 2,
+        },
         // variables that are not in the contract
         extraVar: 214830928,
         anotherExtraVar: [],

diff --git a/packages/plugins/chugsplash/foundry/deploy.t.js b/packages/plugins/chugsplash/foundry/deploy.t.js
@@ -42,6 +42,16 @@ const complexConstructorArgs = {
       [16, 17, 18],
     ],
   ],
+  _complexStruct: {
+    b: 2,
+    a: '0x' + 'aa'.repeat(32),
+    c: 3,
+    d: [1, 2],
+    e: [
+      [1, 2, 3],
+      [4, 5, 6],
+    ],
+  },
 }
 
 const variables = {