diff --git a/packages/contracts-bedrock/CONTRIBUTING.md b/packages/contracts-bedrock/CONTRIBUTING.md index cf55668c5e23..57c2b3375fd6 100644 --- a/packages/contracts-bedrock/CONTRIBUTING.md +++ b/packages/contracts-bedrock/CONTRIBUTING.md @@ -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 diff --git a/packages/contracts-bedrock/layout-lock.json b/packages/contracts-bedrock/layout-lock.json deleted file mode 100644 index d181e170a241..000000000000 --- a/packages/contracts-bedrock/layout-lock.json +++ /dev/null @@ -1,108 +0,0 @@ -{ - "L1CrossDomainMessenger": { - "spacer_0_0_20": { - "slot": 0, - "offset": 0, - "length": 20 - }, - "spacer_1_0_1600": { - "slot": 1, - "offset": 0, - "length": 1600 - }, - "spacer_51_0_20": { - "slot": 51, - "offset": 0, - "length": 20 - }, - "spacer_52_0_1568": { - "slot": 52, - "offset": 0, - "length": 1568 - }, - "spacer_101_0_1": { - "slot": 101, - "offset": 0, - "length": 1 - }, - "spacer_102_0_1568": { - "slot": 102, - "offset": 0, - "length": 1568 - }, - "spacer_151_0_32": { - "slot": 151, - "offset": 0, - "length": 32 - }, - "spacer_201_0_32": { - "slot": 201, - "offset": 0, - "length": 32 - }, - "spacer_202_0_32": { - "slot": 202, - "offset": 0, - "length": 32 - } - }, - "L2CrossDomainMessenger": { - "spacer_0_0_20": { - "slot": 0, - "offset": 0, - "length": 20 - }, - "spacer_1_0_1600": { - "slot": 1, - "offset": 0, - "length": 1600 - }, - "spacer_51_0_20": { - "slot": 51, - "offset": 0, - "length": 20 - }, - "spacer_52_0_1568": { - "slot": 52, - "offset": 0, - "length": 1568 - }, - "spacer_101_0_1": { - "slot": 101, - "offset": 0, - "length": 1 - }, - "spacer_102_0_1568": { - "slot": 102, - "offset": 0, - "length": 1568 - }, - "spacer_151_0_32": { - "slot": 151, - "offset": 0, - "length": 32 - }, - "spacer_201_0_32": { - "slot": 201, - "offset": 0, - "length": 32 - }, - "spacer_202_0_32": { - "slot": 202, - "offset": 0, - "length": 32 - } - }, - "L1StandardBridge": { - "spacer_0_2_20": { - "slot": 0, - "offset": 2, - "length": 20 - }, - "spacer_1_0_20": { - "slot": 1, - "offset": 0, - "length": 20 - } - } -} diff --git a/packages/contracts-bedrock/scripts/validate-spacers.ts b/packages/contracts-bedrock/scripts/validate-spacers.ts index 493bc77da0ea..4ea652f6dad7 100644 --- a/packages/contracts-bedrock/scripts/validate-spacers.ts +++ b/packages/contracts-bedrock/scripts/validate-spacers.ts @@ -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 @@ -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. * @@ -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 = [] @@ -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('_')