Skip to content

Commit

Permalink
Merge pull request #8226 from ethereum-optimism/refcell/remove-layout…
Browse files Browse the repository at this point in the history
…-lock

chore(ctb): Remove Layout Lock
  • Loading branch information
refcell authored Nov 21, 2023
2 parents 8cd36c2 + 0f27b7f commit 2e35a8f
Show file tree
Hide file tree
Showing 3 changed files with 5 additions and 158 deletions.
25 changes: 5 additions & 20 deletions packages/contracts-bedrock/CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -86,26 +86,11 @@ To deploy the smart contracts on a local devnet, run `make devnet-up` in the mon

### Tools

#### Layout Locking

We use a system called "layout locking" as a safety mechanism to prevent certain contract variables from being moved to different storage slots accidentally.
To lock a contract variable, add it to the `layout-lock.json` file which has the following format:

```json
{
"MyContractName": {
"myVariableName": {
"slot": 1,
"offset": 0,
"length": 32
}
}
}
```

With the above config, the `validate-spacers` script will check that we have a contract called `MyContractName`, that the contract has a variable named `myVariableName`, and that the variable is in the correct position as defined in the lock file.
You should add things to the `layout-lock.json` file when you want those variables to **never** change.
Layout locking should be used in combination with diffing the `.storage-layout` file in CI.
#### Validate Spacing

In order to make sure that we don't accidentally overwrite storage slots, contract storage layouts are checked to make sure spacing is correct.

This uses the `.storage-layout` file to check contract spacing. Run `pnpm validate-spacers` to check the spacing of all contracts.

#### Gas Snapshots

Expand Down
108 changes: 0 additions & 108 deletions packages/contracts-bedrock/layout-lock.json

This file was deleted.

30 changes: 0 additions & 30 deletions packages/contracts-bedrock/scripts/validate-spacers.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
import fs from 'fs'
import path from 'path'

import layoutLock from '../layout-lock.json'

/**
* Directory path to the artifacts.
* Can be configured as the first argument to the script or
Expand All @@ -22,15 +20,6 @@ const skipped = (contractName: string): boolean => {
return contractName.includes('CrossDomainMessengerLegacySpacer')
}

/**
* Parses the fully qualified name of a contract into the name of the contract.
* For example `contracts/Foo.sol:Foo` becomes `Foo`.
*/
const parseFqn = (name: string): string => {
const parts = name.split(':')
return parts[parts.length - 1]
}

/**
* Parses out variable info from the variable structure in standard compiler json output.
*
Expand Down Expand Up @@ -89,7 +78,6 @@ const parseVariableInfo = (
/**
* Main logic of the script
* - Ensures that all of the spacer variables are named correctly
* - Ensures that storage slots in the layout lock file do not change
*/
const main = async () => {
const paths = []
Expand Down Expand Up @@ -128,24 +116,6 @@ const main = async () => {
continue
}

const contractName = parseFqn(fqn)

// Check that the layout lock has not changed
const lock = layoutLock[contractName] || {}
if (lock[variable.label]) {
const variableInfo = parseVariableInfo(variable)
const expectedInfo = lock[variable.label]
if (variableInfo.slot !== expectedInfo.slot) {
throw new Error(`${fqn}.${variable.label} slot has changed`)
}
if (variableInfo.offset !== expectedInfo.offset) {
throw new Error(`${fqn}.${variable.label} offset has changed`)
}
if (variableInfo.length !== expectedInfo.length) {
throw new Error(`${fqn}.${variable.label} length has changed`)
}
}

// Check that the spacers are all named correctly
if (variable.label.startsWith('spacer_')) {
const [, slot, offset, length] = variable.label.split('_')
Expand Down

0 comments on commit 2e35a8f

Please sign in to comment.