Skip to content

Commit

Permalink
fix: handle immutable variable errors
Browse files Browse the repository at this point in the history
  • Loading branch information
sam-goldman committed Nov 2, 2022
1 parent 02c7a39 commit 233f960
Show file tree
Hide file tree
Showing 6 changed files with 99 additions and 37 deletions.
7 changes: 7 additions & 0 deletions .changeset/plenty-poems-admire.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@chugsplash/core': patch
'@chugsplash/demo': patch
'@chugsplash/plugins': patch
---

Handle errors with immutable variables
25 changes: 16 additions & 9 deletions packages/core/src/config/parse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,12 @@ import {
makeBundleFromActions,
} from '../actions'
import { getProxyAddress } from '../utils'
import { ChugSplashConfig, ConfigVariable, ContractReference } from './types'
import {
ChugSplashConfig,
ConfigVariable,
ContractConfig,
ContractReference,
} from './types'

export const loadChugSplashConfig = (
configFileName: string
Expand Down Expand Up @@ -167,16 +172,16 @@ export const makeActionBundleFromConfig = async (
})

// Replace any contract references with the contract's address.
const parsedVariables = parseContractReferences(
const parsedContractConfig = parseContractReferences(
config.options.projectName,
contractConfig.variables
contractConfig
)

// Compute our storage slots.
// TODO: One day we'll need to refactor this to support Vyper.
const slots = computeStorageSlots(
artifact.storageLayout,
parsedVariables,
parsedContractConfig.variables,
artifact.immutableVariables
)

Expand All @@ -196,18 +201,20 @@ export const makeActionBundleFromConfig = async (

export const parseContractReferences = (
projectName: string,
variables: ConfigVariable
): ConfigVariable => {
for (const [variableName, variable] of Object.entries(variables)) {
contractConfig: ContractConfig
): ContractConfig => {
for (const [variableName, variable] of Object.entries(
contractConfig.variables
)) {
if (isContractReference(variable)) {
const [referenceName] = Object.values(variable)
variables[variableName] = getProxyAddress(
contractConfig.variables[variableName] = getProxyAddress(
projectName,
referenceName.trim()
)
}
}
return variables
return contractConfig
}

export const isContractReference = (
Expand Down
16 changes: 9 additions & 7 deletions packages/core/src/config/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,15 @@ export interface ChugSplashConfig {
projectOwner: string
}
contracts: {
[referenceName: string]: {
contract: string
address?: string
variables?: {
[name: string]: ConfigVariable
}
}
[referenceName: string]: ContractConfig
}
}

export type ContractConfig = {
contract: string
address?: string
variables?: {
[name: string]: ConfigVariable
}
}

Expand Down
21 changes: 14 additions & 7 deletions packages/plugins/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ pragma solidity ^0.8.9;
contract SimpleStorage {
uint8 internal number;
bool internal stored;
string internal storageName;
address internal otherStorage;
string internal storageName;
function getNumber() external view returns (uint8) {
return number;
Expand All @@ -59,13 +59,13 @@ contract SimpleStorage {
return stored;
}
function getStorageName() external view returns (string memory) {
return storageName;
}
function getOtherStorage() external view returns (address) {
return otherStorage;
}
function getStorageName() external view returns (string memory) {
return storageName;
}
}
```

Expand Down Expand Up @@ -103,6 +103,7 @@ const config: ChugSplashConfig = {
number: 1,
stored: true,
storageName: 'First',
storageName: 'First',
otherStorage: { '!Ref': 'SecondSimpleStorage' }, // Reference to SecondSimpleStorage
},
},
Expand All @@ -128,6 +129,13 @@ Take a moment to familiarize yourself with the layout of the ChugSplash config f
npx hardhat chugsplash-deploy
```

### Immutable variables
ChugSplash supports all immutable variables except for [user defined value types](https://docs.soliditylang.org/en/latest/types.html#user-defined-value-types). You can define immutable variables in your ChugSplash config file the exact same way that you
define regular state variables. However, there is one caveat: you must instantiate the immutable
variables in your constructor or else the Solidity compiler will throw an error.



### Testing your deployments

1. In your existing test folder, create a new test file called `SimpleStorage.spec.ts`.
Expand Down Expand Up @@ -174,11 +182,10 @@ ChugSplash uses deterministic proxies to deploy contracts and set their state va
* Strings that are <= 31 bytes
* Bytes value types, i.e. bytes1, bytes2, …, bytes32. (Not dynamic bytes)
* Contract references (using `{ "!Ref: ..." }` syntax).
* Immutable variables are not supported.
* Quick, trustless deployments by remote ChugSplash bots are not supported.
* You cannot call contracts inside the constructor of any of your deployed contracts.
* References to contracts in other config files are not supported (i.e. `{"!Ref: MyOtherProject.OtherContract "}`)
* You cannot use ChugSplash to upgrade existing contracts.
* You cannot use ChugSplash to upgrade contracts.
* Source code is not automatically verified on Etherscan or Sourcify.
* Deployment artifacts are not generated.
* Contract ABIs, source code, and deployment configs are not published to NPM.
Expand Down
60 changes: 47 additions & 13 deletions packages/plugins/src/hardhat/artifacts.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import * as semver from 'semver'
import { SolidityStorageLayout } from '@chugsplash/core'
import { SolidityStorageLayout, ContractConfig } from '@chugsplash/core'
import { add0x, remove0x } from '@eth-optimism/core-utils'
import { ethers, utils } from 'ethers'

Expand Down Expand Up @@ -63,7 +63,7 @@ export const getStorageLayout = async (

export const getDeployedBytecode = async (
provider: ethers.providers.JsonRpcProvider,
contractConfig
contractConfig: ContractConfig
): Promise<string> => {
const { sourceName, contractName, bytecode, abi } = getContractArtifact(
contractConfig.contract
Expand All @@ -86,21 +86,22 @@ export const getDeployedBytecode = async (
const astIdToAbiEncodedValue = {}

// Maps a constructor argument name to the corresponding variable name in the ChugSplash config
const constructorArgNamesToVariableNames = {}
const constructorArgNamesToImmutableNames = {}

const contractNodes = buildInfo.output.sources[sourceName].ast.nodes
for (const contractNode of contractNodes) {
if (contractNode.nodeType === 'ContractDefinition') {
for (const node of contractNode.nodes) {
if (
node.nodeType === 'VariableDeclaration' &&
(node.mutability === 'immutable' || node.mutability === 'mutable')
node.mutability === 'immutable'
) {
const constructorArgName = getConstructorArgNameForConfigVariable(
const constructorArgName = getConstructorArgNameForImmutableVariable(
contractConfig.contract,
contractNode.nodes,
node.name
)
constructorArgNamesToVariableNames[constructorArgName] = node.name
constructorArgNamesToImmutableNames[constructorArgName] = node.name

let typeString: string
if (node.typeDescriptions.typeString.startsWith('contract')) {
Expand Down Expand Up @@ -142,11 +143,17 @@ export const getDeployedBytecode = async (
const constructorArgValues = []
constructorFragment.inputs.forEach((fragment) => {
constructorArgTypes.push(fragment.type)
constructorArgValues.push(
contractConfig.variables[
constructorArgNamesToVariableNames[fragment.name]
]
)
if (constructorArgNamesToImmutableNames.hasOwnProperty(fragment.name)) {
constructorArgValues.push(
contractConfig.variables[
constructorArgNamesToImmutableNames[fragment.name]
]
)
} else {
throw new Error(
`Detected a non-immutable constructor argument, "${fragment.name}", in ${contractConfig.contract}. Please remove it or make the corresponding variable immutable.`
)
}
})
const creationBytecodeWithConstructorArgs = bytecode.concat(
remove0x(
Expand All @@ -169,15 +176,42 @@ export const getDeployedBytecode = async (
return bytecodeDeployedWithConstructorArgs
}

export const getConstructorArgNameForConfigVariable = (
const getNestedConstructorArg = (variableName: string, args): string => {
let remainingArguments = args[0]
while (remainingArguments !== undefined) {
if (remainingArguments.name !== undefined) {
return remainingArguments.name
}
remainingArguments = remainingArguments.arguments[0]
}
throw new Error(
`Could not find nested constructor argument for the immutable variable ${variableName}. Please report this error.`
)
}

export const getConstructorArgNameForImmutableVariable = (
contractName: string,
nodes: any,
variableName: string
): string => {
for (const node of nodes) {
if (node.kind === 'constructor') {
for (const statement of node.body.statements) {
if (statement.expression.leftHandSide.name === variableName) {
return statement.expression.rightHandSide.name
if (typeof statement.expression.rightHandSide.name === 'string') {
return statement.expression.rightHandSide.name
} else if (
statement.expression.rightHandSide.kind === 'typeConversion'
) {
return getNestedConstructorArg(
variableName,
statement.expression.rightHandSide.arguments
)
} else {
throw new Error(
`The immutable variable "${variableName}" must be assigned directly to a constructor argument inside the body of the constructor in ${contractName}.`
)
}
}
}
}
Expand Down
7 changes: 6 additions & 1 deletion packages/plugins/src/hardhat/tasks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import {
registerChugSplashProject,
chugsplashContractsAreDeployedAndInitialized,
getChugSplashRegistry,
parseContractReferences,
} from '@chugsplash/core'
import { ChugSplashManagerABI } from '@chugsplash/contracts'
import ora from 'ora'
Expand Down Expand Up @@ -91,9 +92,13 @@ subtask(TASK_CHUGSPLASH_BUNDLE_LOCAL)
const artifacts = {}
for (const contractConfig of Object.values(config.contracts)) {
const storageLayout = await getStorageLayout(contractConfig.contract)
const parsedContractConfig = parseContractReferences(
config.options.projectName,
contractConfig
)
const deployedBytecode = await getDeployedBytecode(
hre.ethers.provider,
contractConfig
parsedContractConfig
)
const immutableVariables = await getImmutableVariables(contractConfig)
artifacts[contractConfig.contract] = {
Expand Down

0 comments on commit 233f960

Please sign in to comment.