Skip to content

Commit

Permalink
maint(core): Use OpenZeppelin logic for UUPS upgradeTo check
Browse files Browse the repository at this point in the history
  • Loading branch information
@RPate97
RPate97 committed Mar 31, 2023
1 parent 6645e4e commit 5334700
Show file tree
Hide file tree
Showing 8 changed files with 147 additions and 249 deletions.
228 changes: 119 additions & 109 deletions packages/core/src/config/parse.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ import {
getDefaultProxyAddress,
isExternalProxyType,
readContractArtifact,
assertValidContractReferences,
readBuildInfo,
getChugSplashManagerProxyAddress,
getEIP1967ProxyAdminAddress,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -595,19 +594,10 @@ export const parseInplaceInt: VariableHandler<UserConfigVariable, string> = (
*/
export const parseInplaceStruct: VariableHandler<
UserConfigVariable,
{
[name: string]: ParsedConfigVariable
}
ParsedConfigVariables
> = (
props: VariableHandlerProps<
UserConfigVariable,
{
[name: string]: ParsedConfigVariable
}
>
): {
[name: string]: ParsedConfigVariable
} => {
props: VariableHandlerProps<UserConfigVariable, ParsedConfigVariables>
): ParsedConfigVariables => {
const {
variable,
variableType,
Expand All @@ -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.
Expand Down Expand Up @@ -705,19 +693,10 @@ export const parseBytes: VariableHandler<UserConfigVariable, string> = (
*/
export const parseMapping: VariableHandler<
UserConfigVariable,
{
[name: string]: ParsedConfigVariable
}
ParsedConfigVariables
> = (
props: VariableHandlerProps<
UserConfigVariable,
{
[name: string]: ParsedConfigVariable
}
>
): {
[name: string]: ParsedConfigVariable
} => {
props: VariableHandlerProps<UserConfigVariable, ParsedConfigVariables>
): ParsedConfigVariables => {
const {
variable,
storageObj,
Expand All @@ -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,
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -1032,7 +1003,7 @@ const parseContractVariables = (
}
}

return { variables: parsedConfigVariables }
return parsedConfigVariables
}

/**
Expand All @@ -1050,7 +1021,7 @@ export const parseAndValidateConstructorArgs = (
referenceName: string,
abi: Array<Fragment>,
cre: ChugSplashRuntimeEnvironment
): { args: ParsedConfigVariables } => {
): ParsedConfigVariables => {
const parsedConstructorArgs: ParsedConfigVariables = {}

const constructorFragment = abi.find(
Expand All @@ -1064,7 +1035,7 @@ export const parseAndValidateConstructorArgs = (
`no constructor exists in the contract.`
)
} else {
return { args: parsedConstructorArgs }
return parsedConstructorArgs
}
}

Expand Down Expand Up @@ -1125,15 +1096,15 @@ export const parseAndValidateConstructorArgs = (
}
}

return { args: parsedConstructorArgs }
return parsedConstructorArgs
}

export const assertStorageCompatiblePreserveKeywords = (
contractConfig: ParsedContractConfig,
prevStorageLayout: StorageLayout,
newStorageLayout: StorageLayout,
cre: ChugSplashRuntimeEnvironment
): boolean => {
) => {
const prevDetailedLayout = getDetailedLayout(prevStorageLayout)
const newDetailedLayout = getDetailedLayout(newStorageLayout)

Expand Down Expand Up @@ -1172,22 +1143,95 @@ 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 (
provider: providers.Provider,
parsedConfig: ParsedChugSplashConfig,
userConfig: UserChugSplashConfig,
artifactPaths: ArtifactPaths,
integration: Integration,
cre: ChugSplashRuntimeEnvironment
): Promise<{
upgrade: boolean
}> => {
): Promise<boolean> => {
const { canonicalConfigPath, remoteExecution } = cre

// Determine if the deployment is an upgrade
Expand Down Expand Up @@ -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

Expand All @@ -1416,7 +1427,7 @@ permission to call the 'upgradeTo' function on each of them.
}
}

return { upgrade: isUpgrade }
return isUpgrade
}

export const assertValidContracts = (
Expand Down Expand Up @@ -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

Expand Down Expand Up @@ -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,
Expand All @@ -1709,7 +1724,7 @@ const parseAndValidateChugSplashConfig = async (
cre
)

const { args } = parseAndValidateConstructorArgs(
const args = parseAndValidateConstructorArgs(
constructorArgs ?? {},
referenceName,
compilerOutput.contracts[sourceName][contractName].abi,
Expand All @@ -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
)

Expand All @@ -1756,9 +1770,5 @@ const parseAndValidateChugSplashConfig = async (
process.exit(1)
}

return JSON.parse(
Handlebars.compile(JSON.stringify(parsedConfig))({
...contracts,
})
)
return parsedConfig
}
Loading

0 comments on commit 5334700

Please sign in to comment.