op-chain-ops: update config by tynes · Pull Request #3871 · ethereum-optimism/optimism

tynes · 2022-11-03T20:41:53Z

Description
We need config options for the recipients of the fee vaults. This adds it to the config parsing.

Also adds a helper function to read the correct deployment
artifacts from disk given a hardhat.Hardhat instance.
This code was being repeated throughout the codebase
so its better to consolidate it into a single location.
Now the DeployConfig has the L1 contract addresses
required to build a L2 genesis. These addresses do not
need to be present in the JSON file itself and instead
can be read from disk dynamically.

Also add a check function that will error on a config
that is not sane. This can be made more strict in the
future, currently do not want to break anything in the
process of introducing this.

changeset-bot · 2022-11-03T20:41:56Z

⚠️ No Changeset found

Latest commit: 7fa30a0

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

mergify · 2022-11-04T02:48:02Z

Hey @tynes! This PR has merge conflicts. Please fix them before continuing review.

protolambda

LGTM, updating the hardhat deploy configs with the new l1 recipient addresses would be nice to add though.

mergify · 2022-11-04T12:25:35Z

This PR has been added to the merge queue, and will be merged soon.

mergify · 2022-11-04T12:25:42Z

Hey @tynes, this pull request failed to merge and has been dequeued from the merge train. If you believe your PR failed in the merge train because of a flaky test, requeue it by commenting with @mergifyio requeue.
More details can be found on the Queue: Embarked in merge train check-run.

tynes · 2022-11-04T16:55:25Z

I've updated the deploy config files to contain the correct values

mergify · 2022-11-08T16:34:44Z

Hey @tynes! This PR has merge conflicts. Please fix them before continuing review.

We need config options for the recipients of the fee vaults. This adds it to the config parsing.

Adds a helper function to read the correct deployment artifacts from disk given a `hardhat.Hardhat` instance. This code was being repeated throughout the codebase so its better to consolidate it into a single location. Now the `DeployConfig` has the L1 contract addresses required to build a L2 genesis. These addresses do not need to be present in the JSON file itself and instead can be read from disk dynamically. Also add a check function that will error on a config that is not sane. This can be made more strict in the future, currently do not want to break anything in the process of introducing this.

mslipper · 2022-11-08T17:58:07Z

op-node/cmd/genesis/cmd.go

 				return err
 			}

+			if err := config.Check(); err != nil {


Is this check duplicated on line 145 below?

That is a different command, both need the check

mslipper · 2022-11-08T17:59:12Z

Looks like the devnet has an expected failure given these changes:

CRIT [11-08|17:46:05.867] Application failed                       message="invalid deploy config: ProxyAdminOwner cannot be address(0)"

tynes · 2022-11-08T18:01:54Z

Looks like the devnet has an expected failure given these changes:
CRIT [11-08|17:46:05.867] Application failed                       message="invalid deploy config: ProxyAdminOwner cannot be address(0)"

Will fix each issue

tynes · 2022-11-08T18:08:11Z

Getting a segfault in op-geth in TestL2Verifier_SequenceWindow

panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0xbd10bf]

goroutine 5087 [running]:
github.com/ethereum/go-ethereum/core/types.(*Block).NumberU64(...)
	/go/pkg/mod/github.com/ethereum-optimism/op-geth@v0.0.0-20221104231810-30db39cae2be/core/types/block.go:299
github.com/ethereum/go-ethereum/core/txpool.(*TxPool).reset(0xc006406600, 0xc01165ed80, 0xc01165fd40)
	/go/pkg/mod/github.com/ethereum-optimism/op-geth@v0.0.0-20221104231810-30db39cae2be/core/txpool/txpool.go:1268 +0xf3f
github.com/ethereum/go-ethereum/core/txpool.(*TxPool).runReorg(0xc006406600, 0xc0171e10e0, 0xc0026213e0, 0x0, 0xc00c74cf90)
	/go/pkg/mod/github.com/ethereum-optimism/op-geth@v0.0.0-20221104231810-30db39cae2be/core/txpool/txpool.go:1172 +0x26b
created by github.com/ethereum/go-ethereum/core/txpool.(*TxPool).scheduleReorgLoop
	/go/pkg/mod/github.com/ethereum-optimism/op-geth@v0.0.0-20221104231810-30db39cae2be/core/txpool/txpool.go:1101 +0x1b3

trianglesphere · 2022-11-08T18:09:10Z

Getting a segfault in op-geth in TestL2Verifier_SequenceWindow

@tynes this is a known flake in the action tests

tynes · 2022-11-08T18:26:48Z

Flake with the devnet + deploy:

Starting withdrawal
An unexpected error occurred:

Error: transaction failed [ See: https://links.ethers.org/v5-errors-CALL_EXCEPTION ] (transactionHash="0x3b7453282ec8cb1f26f0cb8b98f5a2e441589d3716fd0850f4c52519371e6849", transaction={"type":2,"chainId":901,"nonce":1,"maxPriorityFeePerGas":{"type":"BigNumber","hex":"0x59682f00"},"maxFeePerGas":{"type":"BigNumber","hex":"0x59685f30"},"gasPrice":null,"gasLimit":{"type":"BigNumber","hex":"0x028c73"},"to":"0x4200000000000000000000000000000000000010","value":{"type":"BigNumber","hex":"0x00"},"data":"0x32b7006d0000000000000000000000007c6b91d9be155a6db01f749217d76ff02a7227f20000000000000000000000000000000000000000000000000de0b6b3a7640000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000800000000000000000000000000000000000000000000000000000000000000000","accessList":[],"hash":"0x3b7453282ec8cb1f26f0cb8b98f5a2e441589d3716fd0850f4c52519371e6849","v":0,"r":"0xd368ebcf1b0947e99b046acbddd5cd507ce3c1986fdc163f36bb171ca58290fb","s":"0x00d94dbc489a905ed61385087a4acb56267b36d1d2762a82688643789e7c9341","from":"0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266","confirmations":0}, receipt={"to":"0x4200000000000000000000000000000000000010","from":"0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266","contractAddress":null,"transactionIndex":2,"gasUsed":{"type":"BigNumber","hex":"0xa42d"},"logsBloom":"0x00000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000","blockHash":"0x368b7c99a293adc3319f48ee0306329b2d9cd45fa3454581b811a256838f3ed9","transactionHash":"0x3b7453282ec8cb1f26f0cb8b98f5a2e441589d3716fd0850f4c52519371e6849","logs":[],"blockNumber":76,"confirmations":22,"cumulativeGasUsed":{"type":"BigNumber","hex":"0x11c43d"},"effectiveGasPrice":{"type":"BigNumber","hex":"0x59685f30"},"status":0,"type":2,"byzantium":true}, code=CALL_EXCEPTION, version=providers/5.7.1)
    at Logger.makeError (/home/circleci/project/node_modules/@ethersproject/providers/node_modules/@ethersproject/logger/src.ts/index.ts:269:28)

This has become really flakey lately, not sure why

tynes requested review from mslipper, protolambda and trianglesphere and removed request for protolambda November 3, 2022 20:41

trianglesphere approved these changes Nov 3, 2022

View reviewed changes

mergify bot added the conflict label Nov 4, 2022

protolambda approved these changes Nov 4, 2022

View reviewed changes

mergify bot added the on-merge-train label Nov 4, 2022

mergify bot removed the on-merge-train label Nov 4, 2022

tynes force-pushed the fix/config-update branch from fa41a48 to 5e8f639 Compare November 4, 2022 16:55

github-actions bot added 2-reviewers A-pkg-contracts-bedrock Area: packages/contracts-bedrock labels Nov 4, 2022

mergify bot removed the conflict label Nov 4, 2022

mergify bot added the conflict label Nov 8, 2022

tynes added 7 commits November 8, 2022 08:36

op-chain-ops: parse the new config

d6d7cd4

We need config options for the recipients of the fee vaults. This adds it to the config parsing.

op-chain-ops: update config

1023cd7

op-bindings: modularize errors

1aa7e74

op-e2e: clean up config

697f16c

op-node: clean up config usage

a421d75

op-chain-ops: clean up migrate command

9c7e99d

tynes force-pushed the fix/config-update branch from 5e8f639 to 9c7e99d Compare November 8, 2022 17:41

tynes changed the title ~~op-chain-ops: parse the new config~~ op-chain-ops: update config Nov 8, 2022

mergify bot removed the conflict label Nov 8, 2022

tynes mentioned this pull request Nov 8, 2022

hive: fix config for L2 genesis creation ethereum-optimism/hive#60

Merged

mslipper approved these changes Nov 8, 2022

View reviewed changes

contracts-bedrock: update deploy config

2d4cdbb

contracts-bedrock: configure devnetL1

7fa30a0

tynes merged commit 0da966c into develop Nov 8, 2022

tynes deleted the fix/config-update branch November 8, 2022 18:36

Comments

Conversation

tynes commented Nov 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

changeset-bot bot commented Nov 3, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ No Changeset found

Uh oh!

mergify bot commented Nov 4, 2022

Uh oh!

protolambda left a comment

Choose a reason for hiding this comment

Uh oh!

mergify bot commented Nov 4, 2022

Uh oh!

mergify bot commented Nov 4, 2022

Uh oh!

tynes commented Nov 4, 2022

Uh oh!

mergify bot commented Nov 8, 2022

Uh oh!

mslipper Nov 8, 2022

Choose a reason for hiding this comment

Uh oh!

tynes Nov 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mslipper commented Nov 8, 2022

Uh oh!

tynes commented Nov 8, 2022

Uh oh!

tynes commented Nov 8, 2022

Uh oh!

trianglesphere commented Nov 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tynes commented Nov 8, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

tynes commented Nov 3, 2022 •

edited

Loading

changeset-bot bot commented Nov 3, 2022 •

edited

Loading

tynes Nov 8, 2022 •

edited

Loading

trianglesphere commented Nov 8, 2022 •

edited

Loading