Skip to content

Commit

Permalink
fix(core): validate usage of the preserve keyword
Browse files Browse the repository at this point in the history
  • Loading branch information
sam-goldman committed Mar 6, 2023
1 parent dfbccc7 commit a26ab46
Show file tree
Hide file tree
Showing 3 changed files with 90 additions and 2 deletions.
5 changes: 5 additions & 0 deletions .changeset/hungry-squids-arrive.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@chugsplash/core': patch
---

Validate usage of the preserve keyword
4 changes: 2 additions & 2 deletions docs/special-var-defs.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ myToken: "{{MyToken }}"

## Preserve Keyword

During an upgrade, you'll probably want to leave the values of some variables untouched. For example, if you have a `counter` variable that's incremented every time a user performs an action, you probably won't want to overwrite its existing value since it can change at any time. To prevent its value from being overwritten, use the preserve keyword:
During an upgrade, you may want to leave the values of some variables untouched. For example, if you have a `counter` variable that's incremented every time a user performs an action, you probably won't want to overwrite its existing value since it can change at any time. To prevent its value from being overwritten, use the preserve keyword:

```ts
counter: '{ preserve }'
Expand Down Expand Up @@ -63,7 +63,7 @@ The preserve keyword works for arbitrarily nested variables.

Using the preserve keyword will cause ChugSplash to omit the `SetStorage` action for the variable (or its member). This means ChugSplash will not modify its value when it upgrades the contract.

You can only use the preserve keyword for state variables that have the same exact type and storage slot position (i.e. storage slot key and offset) in the original contract and its upgraded version. Additionally, you can only use the keyword when upgrading a contract (not for a fresh deployment). Lastly, you cannot use the preserve keyword for immutable variables. If any of these conditions aren't met, ChugSplash will throw a helpful error when compiling your ChugSplash file.
You can only use the preserve keyword for state variables that have the same exact type and storage slot position (i.e. storage slot key and offset) in the original contract and its upgraded version. Additionally, you can only use the keyword when upgrading a contract (not for a fresh deployment). Lastly, you cannot use the preserve keyword for immutable variables. If any of these conditions aren't met, ChugSplash will throw an error when compiling your ChugSplash file.

The preserve keyword is not case sensitive, and allows whitespace. In other words, the following variations of the preserve keyword are valid:
```
Expand Down
83 changes: 83 additions & 0 deletions packages/core/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,16 @@ import {
UpgradeableContract,
ValidationOptions,
} from '@openzeppelin/upgrades-core'
import {
getDetailedLayout,
ParsedTypeDetailed,
StorageItem,
} from '@openzeppelin/upgrades-core/dist/storage/layout'
import {
StorageField,
StorageLayoutComparator,
stripContractSubstrings,
} from '@openzeppelin/upgrades-core/dist/storage/compare'
import { ContractDefinition } from 'solidity-ast'
import { CompilerInput, SolcBuild } from 'hardhat/types'
import { Compiler, NativeCompiler } from 'hardhat/internal/solidity/compiler'
Expand Down Expand Up @@ -793,6 +803,12 @@ permission to call the 'upgradeTo' function on each of them.
contractConfig.proxyType
)

assertStorageCompatiblePreserveKeywords(
contractConfig,
previousStorageLayout,
newStorageLayout
)

// We could check for the `skipStorageCheck` in the outer for-loop, but this makes it easy to
// support more granular storage layout config options in the future.
if (parsedConfig.options.skipStorageCheck !== true) {
Expand Down Expand Up @@ -1227,6 +1243,73 @@ export const parseFoundryArtifact = (artifact: any): ContractArtifact => {
return { abi, bytecode, sourceName, contractName }
}

export const isEqualType = (
prevStorageObj: StorageItem<ParsedTypeDetailed>,
newStorageObj: StorageItem<ParsedTypeDetailed>
): boolean => {
// Copied from OpenZeppelin's core upgrades package:
// https://github.com/OpenZeppelin/openzeppelin-upgrades/blob/13c072776e381d33cf285f8953127023b664de64/packages/core/src/storage/compare.ts#L197-L202
const isRetypedFromOriginal = (
original: StorageField,
updated: StorageField
): boolean => {
const originalLabel = stripContractSubstrings(original.type.item.label)
const updatedLabel = stripContractSubstrings(updated.retypedFrom?.trim())

return originalLabel === updatedLabel
}

const layoutComparator = new StorageLayoutComparator(false, false)

// Copied from OpenZeppelin's core upgrades package:
// https://github.com/OpenZeppelin/openzeppelin-upgrades/blob/13c072776e381d33cf285f8953127023b664de64/packages/core/src/storage/compare.ts#L171-L173
const isEqual =
!isRetypedFromOriginal(prevStorageObj, newStorageObj) &&
!layoutComparator.getTypeChange(prevStorageObj.type, newStorageObj.type, {
allowAppend: false,
})

return isEqual
}

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

const errorMessages: Array<string> = []
for (const newStorageObj of newDetailedLayout) {
if (
variableContainsPreserveKeyword(
contractConfig.variables[newStorageObj.label]
)
) {
const validPreserveKeyword = prevDetailedLayout.some(
(prevObj) =>
prevObj.label === newStorageObj.label &&
prevObj.slot === newStorageObj.slot &&
prevObj.offset === newStorageObj.offset &&
isEqualType(prevObj, newStorageObj)
)

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.`
)
}
}
}

if (errorMessages.length > 0) {
throw new Error(`${errorMessages.join('\n\n')}`)
}
}

/**
* Returns the Create2 address of an implementation contract deployed by ChugSplash, which is
* calculated as a function of the projectName and the corresponding contract's reference name. Note
Expand Down

0 comments on commit a26ab46

Please sign in to comment.