Skip to content

N
nebula
Commit 54a3e562 authored by Mark Tyneway's avatar Mark Tyneway
Browse files
Options

contracts-bedrock: optimize storage layout checks 

Optimize the storage layout checks by removing a dep
on hardhat. Instead of reading massive build-info files
from disk, simply read the forge artifacts directly and
then filter through the interesting ones.
This now only takes a few seconds to run when the compilation
is cached.

Also delete a dead hh task for doing deposits, this was built
as a PoC to trigger a deposit.
parent 9d534997
Hide whitespace changes
Inline Side-by-side
Showing with 182 additions and 125 deletions
packages/contracts-bedrock/package.json
View file @ 54a3e562
...@@ -32,7 +32,7 @@ ...@@ -32,7 +32,7 @@
"gas-snapshot": "yarn build:differential && yarn build:fuzz && forge snapshot --no-match-test 'testDiff|testFuzz|invariant|generateArtifact'", "gas-snapshot": "yarn build:differential && yarn build:fuzz && forge snapshot --no-match-test 'testDiff|testFuzz|invariant|generateArtifact'",
"storage-snapshot": "./scripts/storage-snapshot.sh", "storage-snapshot": "./scripts/storage-snapshot.sh",
"validate-deploy-configs": "hardhat compile && hardhat generate-deploy-config && ./scripts/validate-deploy-configs.sh", "validate-deploy-configs": "hardhat compile && hardhat generate-deploy-config && ./scripts/validate-deploy-configs.sh",
"validate-spacers": "hardhat compile && hardhat validate-spacers", "validate-spacers": "forge build && npx ts-node scripts/validate-spacers.ts",
"slither": "./scripts/slither.sh", "slither": "./scripts/slither.sh",
"slither:triage": "TRIAGE_MODE=1 ./scripts/slither.sh", "slither:triage": "TRIAGE_MODE=1 ./scripts/slither.sh",
"clean": "rm -rf ./dist ./artifacts ./forge-artifacts ./cache ./tsconfig.tsbuildinfo ./tsconfig.build.tsbuildinfo ./src/contract-artifacts.ts ./test-case-generator/fuzz", "clean": "rm -rf ./dist ./artifacts ./forge-artifacts ./cache ./tsconfig.tsbuildinfo ./tsconfig.build.tsbuildinfo ./src/contract-artifacts.ts ./test-case-generator/fuzz",
......
packages/contracts-bedrock/tasks/validate-spacers.ts → packages/contracts-bedrock/scripts/validate-spacers.ts
View file @ 54a3e562
import { task } from 'hardhat/config' import fs from 'fs'
import { parseFullyQualifiedName } from 'hardhat/utils/contract-names' import path from 'path'
import { HardhatRuntimeEnvironment } from 'hardhat/types'
import layoutLock from '../layout-lock.json' import layoutLock from '../layout-lock.json'
// Artifacts that should be skipped when inspecting their storage layout /**
const skipped = { * Directory path to the artifacts.
// Both of these are skipped because they are meant to be inherited * Can be configured as the first argument to the script or
// by the CrossDomainMessenger. It is the CrossDomainMessenger that * defaults to the forge-artifacts directory.
// should be inspected, not these contracts. */
CrossDomainMessengerLegacySpacer0: true, const directoryPath = process.argv[2] || path.join(__dirname, '..', 'forge-artifacts');
CrossDomainMessengerLegacySpacer1: true,
/**
* Returns true if the contract should be skipped when inspecting its storage layout.
* This is useful for abstract contracts that are meant to be inherited.
* The two particular targets are:
* - CrossDomainMessengerLegacySpacer0
* - CrossDomainMessengerLegacySpacer1
*/
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. * Parses out variable info from the variable structure in standard compiler json output.
* *
...@@ -68,121 +86,96 @@ const parseVariableInfo = ( ...@@ -68,121 +86,96 @@ const parseVariableInfo = (
} }
} }
task( /**
'validate-spacers', * Main logic of the script
'validates that spacer variables are in the correct positions' * - Ensures that all of the spacer variables are named correctly
).setAction(async ({}, hre: HardhatRuntimeEnvironment) => { * - Ensures that storage slots in the layout lock file do not change
const accounted: string[] = [] */
const main = async () => {
const names = await hre.artifacts.getAllFullyQualifiedNames() const paths = []
for (const fqn of names) {
// Script is remarkably slow because of getBuildInfo, so better to skip test files since they
// don't matter for this check.
if (fqn.includes('.t.sol')) {
continue
}
console.log(`Processing ${fqn}`) const readFilesRecursively = (dir: string) => {
const parsed = parseFullyQualifiedName(fqn) const files = fs.readdirSync(dir);
const contractName = parsed.contractName
if (skipped[contractName] === true) { for (const file of files) {
console.log(`Skipping ${contractName} because it is marked as skippable`) const filePath = path.join(dir, file)
continue const fileStat = fs.statSync(filePath)
if (fileStat.isDirectory()) {
readFilesRecursively(filePath)
} else {
paths.push(filePath)
}
} }
}
readFilesRecursively(directoryPath);
// Some files may not have buildInfo (certain libraries). We can safely skip these because we for (const filePath of paths) {
// make sure that everything is accounted for anyway. if (filePath.includes('t.sol')) {
const buildInfo = await hre.artifacts.getBuildInfo(fqn)
if (buildInfo === undefined) {
console.log(`Skipping ${fqn} because it has no buildInfo`)
continue continue
} }
const artifact = require(filePath)
const sources = buildInfo.output.contracts
for (const [sourceName, source] of Object.entries(sources)) { // Handle contracts without storage
// The source file may have our contract const storageLayout = artifact.storageLayout || {}
if (sourceName.includes(parsed.sourceName)) { if (storageLayout.storage) {
const contract = source[contractName] for (const variable of storageLayout.storage) {
if (!contract) { const fqn = variable.contract
console.log(`Skipping ${contractName} as its not found in the source`) // Skip some abstract contracts
if (skipped(fqn)) {
continue continue
} }
const storageLayout = (contract as any).storageLayout
if (!storageLayout) { const contractName = parseFqn(fqn)
continue
// 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`)
}
} }
if (layoutLock[contractName]) { // Check that the spacers are all named correctly
console.log(`Processing layout lock for ${contractName}`) if (variable.label.startsWith('spacer_')) {
const removed = Object.entries(layoutLock[contractName]).filter( const [, slot, offset, length] = variable.label.split('_')
([key, val]: any) => { const variableInfo = parseVariableInfo(variable)
const storage = storageLayout?.storage || []
return !storage.some((item: any) => { // Check that the slot is correct.
// Skip anything that doesn't clearly match the key because otherwise we'll get an if (parseInt(slot, 10) !== variableInfo.slot) {
// error while parsing the variable info for unsupported variable types.
if (!item.label.includes(key)) {
return false
}
// Make sure the variable matches **exactly**.
const variableInfo = parseVariableInfo(item)
return (
variableInfo.name === key &&
variableInfo.offset === val.offset &&
variableInfo.slot === val.slot &&
variableInfo.length === val.length
)
})
}
)
if (removed.length > 0) {
throw new Error( throw new Error(
`variable(s) removed from ${contractName}: ${removed.join(', ')}` `${fqn} ${variable.label} is in slot ${variable.slot} but should be in ${slot}`
) )
} }
console.log(`Valid layout lock for ${contractName}`)
accounted.push(contractName)
}
for (const variable of storageLayout?.storage || []) { // Check that the offset is correct.
if (variable.label.startsWith('spacer_')) { if (parseInt(offset, 10) !== variableInfo.offset) {
const [, slot, offset, length] = variable.label.split('_') throw new Error(
const variableInfo = parseVariableInfo(variable) `${fqn} ${variable.label} is at offset ${variable.offset} but should be at ${offset}`
)
// Check that the slot is correct. }
if (parseInt(slot, 10) !== variableInfo.slot) {
throw new Error( // Check that the length is correct.
`${contractName} ${variable.label} is in slot ${variable.slot} but should be in ${slot}` if (parseInt(length, 10) !== variableInfo.length) {
) throw new Error(
} `${fqn} ${variable.label} is ${variableInfo.length} bytes long but should be ${length}`
)
// Check that the offset is correct.
if (parseInt(offset, 10) !== variableInfo.offset) {
throw new Error(
`${contractName} ${variable.label} is at offset ${variable.offset} but should be at ${offset}`
)
}
// Check that the length is correct.
if (parseInt(length, 10) !== variableInfo.length) {
throw new Error(
`${contractName} ${variable.label} is ${variableInfo.length} bytes long but should be ${length}`
)
}
console.log(`${contractName}.${variable.label} is valid`)
} }
console.log(`${fqn}.${variable.label} is valid`)
} }
} }
} }
} }
}
for (const name of Object.keys(layoutLock)) { main()
if (!accounted.includes(name)) {
throw new Error(`contract ${name} is not accounted for`)
}
}
})
packages/contracts-bedrock/tasks/deposits.ts deleted 100644 → 0
View file @ 9d534997
/*
* Copyright (c) 2022, OP Labs PBC (MIT License)
* https://github.com/ethereum-optimism/optimism
*/
import { task, types } from 'hardhat/config'
import { providers, utils, Wallet, Event } from 'ethers'
import dotenv from 'dotenv'
import 'hardhat-deploy'
import '@nomiclabs/hardhat-ethers'
import { DepositTx } from '@eth-optimism/core-utils'
dotenv.config()
const sleep = async (ms: number) => {
return new Promise((resolve) => setTimeout(resolve, ms))
}
task('deposit', 'Deposits funds onto L2.')
.addParam(
'l1ProviderUrl',
'L1 provider URL.',
'http://localhost:8545',
types.string
)
.addParam(
'l2ProviderUrl',
'L2 provider URL.',
'http://localhost:9545',
types.string
)
.addParam('to', 'Recipient address.', null, types.string)
.addParam('amountEth', 'Amount in ETH to send.', null, types.string)
.addOptionalParam(
'privateKey',
'Private key to send transaction',
process.env.PRIVATE_KEY,
types.string
)
.setAction(async (args, hre) => {
const { l1ProviderUrl, l2ProviderUrl, to, amountEth, privateKey } = args
const proxy = await hre.deployments.get('OptimismPortalProxy')
const OptimismPortal = await hre.ethers.getContractAt(
'OptimismPortal',
proxy.address
)
const l1Provider = new providers.JsonRpcProvider(l1ProviderUrl)
const l2Provider = new providers.JsonRpcProvider(l2ProviderUrl)
let l1Wallet: Wallet | providers.JsonRpcSigner
if (privateKey) {
l1Wallet = new Wallet(privateKey, l1Provider)
} else {
l1Wallet = l1Provider.getSigner()
}
const from = await l1Wallet.getAddress()
console.log(`Sending from ${from}`)
const balance = await l1Wallet.getBalance()
if (balance.eq(0)) {
throw new Error(`${from} has no balance`)
}
const amountWei = utils.parseEther(amountEth)
const value = amountWei.add(utils.parseEther('0.01'))
console.log(`Depositing ${amountEth} ETH to ${to}`)
const preL2Balance = await l2Provider.getBalance(to)
console.log(`${to} has ${utils.formatEther(preL2Balance)} ETH on L2`)
// Below adds 0.01 ETH to account for gas.
const tx = await OptimismPortal.depositTransaction(
to,
amountWei,
'3000000',
false,
[],
{ value }
)
console.log(`Got TX hash ${tx.hash}. Waiting...`)
const receipt = await tx.wait()
console.log(`Included in block ${receipt.blockHash}`)
// find the transaction deposited event and derive
// the deposit transaction from it
const event = receipt.events.find(
(e: Event) => e.event === 'TransactionDeposited'
)
console.log(`Deposit has log index ${event.logIndex}`)
const l2tx = DepositTx.fromL1Event(event)
const hash = l2tx.hash()
console.log(`Waiting for L2 TX hash ${hash}`)
let i = 0
while (true) {
const expected = await l2Provider.send('eth_getTransactionByHash', [hash])
if (expected) {
console.log('Deposit success')
console.log(JSON.stringify(expected, null, 2))
console.log('Receipt:')
const l2Receipt = await l2Provider.getTransactionReceipt(hash)
console.log(JSON.stringify(l2Receipt, null, 2))
break
}
if (i % 100 === 0) {
const postL2Balance = await l2Provider.getBalance(to)
if (postL2Balance.gt(preL2Balance)) {
console.log(
`Unexpected balance increase without detecting deposit transaction`
)
}
const block = await l2Provider.getBlock('latest')
console.log(`latest block ${block.number}:${block.hash}`)
}
await sleep(500)
i++
}
})
packages/contracts-bedrock/tasks/index.ts
View file @ 54a3e562
import './deposits'
import './check-op-node' import './check-op-node'
import './validate-spacers'
import './solidity' import './solidity'
import './check-l2' import './check-l2'
import './generate-deploy-config' import './generate-deploy-config'
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment