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

Deploy create2deployer in the next hardfork #126

Merged
merged 5 commits into from
Oct 20, 2023

Conversation

mdehoog
Copy link
Contributor

@mdehoog mdehoog commented Aug 29, 2023

Description

The pcaversaccio/create2deployer helper is not able to be deployed to Base mainnet due to a nonce-increment from a deposit tx to the deployer address (see pcaversaccio/create2deployer#128).

In the discussion in pcaversaccio/create2deployer#129 it was suggested that we could deploy this in the next op-stack hard fork. This PR implements this deployment, using a stateDb.setCode call.

The code can be verified here.

Tests

  • TODO test E2E (will keep in draft for now)

core/state_processor.go Outdated Show resolved Hide resolved
consensus/misc/create2deployer.go Outdated Show resolved Hide resolved
miner/worker.go Outdated Show resolved Hide resolved
@trianglesphere
Copy link
Contributor

Do we want this to apply to base mainnet or all chains? I lean to just base because other deployed chains should already have this. For new chains, we're looking at options to reduce this footgun.

@trianglesphere
Copy link
Contributor

I'm just confirming that you want the bytecode from create2Deployer at 0xF49600926c7109BD66Ab97a2c036bf696e58Dbc2. I looked at some of the xdeployer byte code (0x13b0D85CcB8bf860b6b79AF3029fCA081AE9beF2), and the xdeployer bytecode does not match, but it also varies across chains.

@mdehoog
Copy link
Contributor Author

mdehoog commented Oct 13, 2023

@trianglesphere thanks for checking. The primary reason for the bytecode difference is the removal of the Ownable and Pausable interfaces removed, see this comment for more context: pcaversaccio/create2deployer#129 (comment)

An alternative would be to keep the same bytecode and leave the owner set to 0x0, which is functionally equivalent. Happy with either option.

@trianglesphere
Copy link
Contributor

@mdehoog I'm fine with it as long as you are & there's a good reason behind it.

@mdehoog mdehoog force-pushed the create2deployer branch 2 times, most recently from 40fb369 to a5e446e Compare October 18, 2023 01:25
@mdehoog mdehoog marked this pull request as ready for review October 18, 2023 01:25
@pcw109550
Copy link
Contributor

@mdehoog May I ask we do not have to initialize the storage of create2deployer predeploy? This is because we are using the version which Ownable, Pausable removed(ref: pcaversaccio/create2deployer#129 (comment)) so storage is not used, right?

@trianglesphere
Copy link
Contributor

@pcw109550 that is correct. The deployed contract does not touch any storage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants