Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat[integration-tests]: add verifier sync test #913

Merged
merged 17 commits into from
May 27, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/unlucky-shoes-bake.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@eth-optimism/integration-tests': patch
---

Add verifier sync test and extra docker-compose functions
1 change: 1 addition & 0 deletions integration-tests/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
"build:contracts": "hardhat compile",
"build:contracts:ovm": "hardhat compile --network optimism",
"test:integration": "hardhat --network optimism test",
"test:sync": "hardhat --network optimism test sync-tests/*.spec.ts --no-compile",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like we need sync tests that have a dummy db that is passed to geth (upstream geth in L1 mode because hardhat doesn't allow for this) and then we sync from that deterministically. Perhaps we can call these verifier tests? We need to do the exact same logic for replicas so maybe network tests? Since its a network of nodes?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

^can rename to network tests!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we necessarily need a pre-instantiated db as things are reasonably deterministic in our current envs? Think PR is fine rn otherwise

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was more of a longer term comment, sorry for the confusion. In the long term we need sync tests that basically sync thousands of blocks where we can observe things like performance and state

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

^opted to rename those later since sync is clearer for this context :)

"clean": "rimraf cache artifacts artifacts-ovm cache-ovm"
},
"devDependencies": {
Expand Down
101 changes: 101 additions & 0 deletions integration-tests/sync-tests/sync-verifier.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,101 @@
import chai, { expect } from 'chai'
import { Wallet, BigNumber, providers } from 'ethers'
import { injectL2Context } from '@eth-optimism/core-utils'

import { sleep, l2Provider, verifierProvider } from '../test/shared/utils'
import { OptimismEnv } from '../test/shared/env'
import { DockerComposeNetwork } from '../test/shared/docker-compose'

describe('Syncing a verifier', () => {
let env: OptimismEnv
let wallet: Wallet
let verifier: DockerComposeNetwork
let provider: providers.JsonRpcProvider

const sequencerProvider = injectL2Context(l2Provider)

/* Helper functions */

const waitForBatchSubmission = async (
totalElementsBefore: BigNumber
): Promise<BigNumber> => {
// Wait for batch submission to happen by watching the CTC
let totalElementsAfter = (await env.ctc.getTotalElements()) as BigNumber
while (totalElementsBefore.eq(totalElementsAfter)) {
await sleep(500)
totalElementsAfter = (await env.ctc.getTotalElements()) as BigNumber
}
return totalElementsAfter
}

const startVerifier = async () => {
// Bring up new verifier
verifier = new DockerComposeNetwork(['verifier'])
await verifier.up({ commandOptions: ['--scale', 'verifier=1'] })

// Wait for verifier to be looping
let logs = await verifier.logs()
while (!logs.out.includes('Starting Sequencer Loop')) {
await sleep(500)
logs = await verifier.logs()
}

provider = injectL2Context(verifierProvider)
}

const syncVerifier = async (sequencerBlockNumber: number) => {
// Wait until verifier has caught up to the sequencer
let latestVerifierBlock = (await provider.getBlock('latest')) as any
while (latestVerifierBlock.number < sequencerBlockNumber) {
await sleep(500)
latestVerifierBlock = (await provider.getBlock('latest')) as any
}

return provider.getBlock(sequencerBlockNumber)
}

before(async () => {
env = await OptimismEnv.new()
wallet = env.l2Wallet
})

describe('Basic transactions', () => {
afterEach(async () => {
await verifier.stop('verifier')
await verifier.rm()
})

Copy link
Contributor

@tynes tynes May 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why does afterEach remove the verifier but there is no beforeEach to bring it up? Shouldn't this be a single before to bring it up (decouple starting and syncing) and just start and then a single after to clean up?

Copy link
Contributor Author

@annieke annieke May 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i followed the structure described in #604 which specified spinning up the verifier after making transactions. i haven't looked into distinctions between the two approaches yet, just imagined this test as "starting with a fresh copy of what's syncing every test"

it('should sync dummy transaction', async () => {
const totalElementsBefore = (await env.ctc.getTotalElements()) as BigNumber

const tx = {
to: '0x' + '1234'.repeat(10),
gasLimit: 4000000,
gasPrice: 0,
data: '0x',
value: 0,
}
const result = await wallet.sendTransaction(tx)
await result.wait()

const totalElementsAfter = await waitForBatchSubmission(
totalElementsBefore
)
expect(totalElementsAfter.gt(totalElementsAfter))

const latestSequencerBlock = (await sequencerProvider.getBlock(
'latest'
)) as any

await startVerifier()

const matchingVerifierBlock = (await syncVerifier(
latestSequencerBlock.number
)) as any

expect(matchingVerifierBlock.stateRoot).to.eq(
latestSequencerBlock.stateRoot
)
})
})
})
28 changes: 25 additions & 3 deletions integration-tests/test/shared/docker-compose.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,12 @@ import * as compose from 'docker-compose'
import * as shell from 'shelljs'
import * as path from 'path'

type ServiceNames = 'batch_submitter' | 'dtl' | 'l2geth' | 'relayer'
type ServiceNames =
| 'batch_submitter'
| 'dtl'
| 'l2geth'
| 'relayer'
| 'verifier'

const OPS_DIRECTORY = path.join(process.cwd(), '../ops')
const DEFAULT_SERVICES: ServiceNames[] = [
Expand All @@ -15,8 +20,11 @@ const DEFAULT_SERVICES: ServiceNames[] = [
export class DockerComposeNetwork {
constructor(private readonly services: ServiceNames[] = DEFAULT_SERVICES) {}

async up() {
const out = await compose.upMany(this.services, { cwd: OPS_DIRECTORY })
async up(options?: compose.IDockerComposeOptions) {
const out = await compose.upMany(this.services, {
cwd: OPS_DIRECTORY,
...options,
})

const { err, exitCode } = out

Expand All @@ -35,5 +43,19 @@ export class DockerComposeNetwork {
cwd: OPS_DIRECTORY,
})
}

return out
}

async logs() {
return compose.logs(this.services, { cwd: OPS_DIRECTORY })
}

async stop(service: ServiceNames) {
return compose.stopOne(service, { cwd: OPS_DIRECTORY })
}

async rm() {
return compose.rm({ cwd: OPS_DIRECTORY })
}
}
5 changes: 5 additions & 0 deletions integration-tests/test/shared/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,10 @@ export const GWEI = BigNumber.from(1e9)
const env = cleanEnv(process.env, {
L1_URL: str({ default: 'http://localhost:9545' }),
L2_URL: str({ default: 'http://localhost:8545' }),
VERIFIER_URL: str({ default: 'http://localhost:8547' }),
L1_POLLING_INTERVAL: num({ default: 10 }),
L2_POLLING_INTERVAL: num({ default: 10 }),
VERIFIER_POLLING_INTERVAL: num({ default: 10 }),
PRIVATE_KEY: str({
default:
'0xac0974bec39a17e36ba4a6b4d238ff944bacb478cbed5efcae784d7bf4f2ff80',
Expand All @@ -39,6 +41,9 @@ l1Provider.pollingInterval = env.L1_POLLING_INTERVAL
export const l2Provider = new providers.JsonRpcProvider(env.L2_URL)
l2Provider.pollingInterval = env.L2_POLLING_INTERVAL

export const verifierProvider = new providers.JsonRpcProvider(env.VERIFIER_URL)
verifierProvider.pollingInterval = env.VERIFIER_POLLING_INTERVAL

// The sequencer private key which is funded on L1
export const l1Wallet = new Wallet(env.PRIVATE_KEY, l1Provider)

Expand Down
2 changes: 1 addition & 1 deletion integration-tests/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,6 @@
"compilerOptions": {
"resolveJsonModule": true
},
"include": ["./test"],
annieke marked this conversation as resolved.
Show resolved Hide resolved
"include": ["./test", "sync-tests/*.ts"],
"files": ["./hardhat.config.ts"]
}
6 changes: 6 additions & 0 deletions ops/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,12 @@ docker-compose \
up --build --detach
```

Optionally, run a verifier along the rest of the stack.
```
docker-compose up --scale verifier=1 \
--build --detach
```

A Makefile has been provided for convience. The following targets are available.
- make up
- make down
Expand Down
29 changes: 29 additions & 0 deletions ops/docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,35 @@ services:
URL: http://deployer:8081/addresses.json
SEQUENCER_PRIVATE_KEY: "0x59c6995e998f97a5a0044966f0945389dc9e86dae88c7a8412f4603b6b78690d"

verifier:
depends_on:
- l1_chain
- deployer
- dtl
image: ethereumoptimism/l2geth
deploy:
replicas: 0
build:
context: ..
dockerfile: ./ops/docker/Dockerfile.geth
# override with the geth script and the env vars required for it
entrypoint: sh ./geth.sh
env_file:
- ./envs/geth.env
environment:
ETH1_HTTP: http://l1_chain:8545
ROLLUP_STATE_DUMP_PATH: http://deployer:8081/state-dump.latest.json
# used for getting the addresses
URL: http://deployer:8081/addresses.json
# connecting to the DTL
ROLLUP_CLIENT_HTTP: http://dtl:7878
ETH1_CTC_DEPLOYMENT_HEIGHT: 8
RETRIES: 60
IS_VERIFIER: "true"
ports:
- ${VERIFIER_HTTP_PORT:-8547}:8545
- ${VERIFIER_WS_PORT:-8548}:8546

integration_tests:
image: ethereumoptimism/integration-tests
deploy:
Expand Down