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 : Cleanup L1 Genesis Generation and removed unused crypto dependencies #12557

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

mahmudsudo
Copy link

Description
Cleanup L1 Genesis Generation by removing clique support

Tests

There was no need for additional tests as this was a refactor
all former tests passes

-closes #12518

@tynes
Copy link
Contributor

tynes commented Oct 22, 2024

Can you please also remove L1UseClique from the DeployConfig and all additional references to it?

$ git grep -rin l1useclique

op-chain-ops/genesis/config.go:786:	// L1UseClique represents whether or not to use the clique consensus engine.
op-chain-ops/genesis/config.go:787:	L1UseClique bool `json:"l1UseClique"`
op-chain-ops/genesis/genesis.go:149:	if config.L1UseClique {
op-chain-ops/genesis/genesis.go:181:	if !config.L1UseClique && config.L1CancunTimeOffset != nil {
op-chain-ops/genesis/testdata/test-deploy-config-full.json:9:  "l1UseClique": false,
op-e2e/config/init.go:206:	dc.L1UseClique = false
packages/contracts-bedrock/deploy-config/sepolia-devnet-0.json:19:  "l1UseClique": false,

Also the CliqueSignerAddress

op-chain-ops/genesis/config.go:783:	// CliqueSignerAddress represents the signer address for the clique consensus engine.
op-chain-ops/genesis/config.go:785:	CliqueSignerAddress common.Address `json:"cliqueSignerAddress"`
op-chain-ops/genesis/genesis.go:156:		extraData = append(append(make([]byte, 32), config.CliqueSignerAddress[:]...), make([]byte, crypto.SignatureLength)...)
op-chain-ops/genesis/testdata/test-deploy-config-full.json:10:  "cliqueSignerAddress": "0x0000000000000000000000000000000000000000",
packages/contracts-bedrock/deploy-config/sepolia-devnet-0.json:18:  "cliqueSignerAddress": "0x0000000000000000000000000000000000000000",

@mahmudsudo
Copy link
Author

Can you please also remove L1UseClique from the DeployConfig and all additional references to it?

$ git grep -rin l1useclique

op-chain-ops/genesis/config.go:786:	// L1UseClique represents whether or not to use the clique consensus engine.
op-chain-ops/genesis/config.go:787:	L1UseClique bool `json:"l1UseClique"`
op-chain-ops/genesis/genesis.go:149:	if config.L1UseClique {
op-chain-ops/genesis/genesis.go:181:	if !config.L1UseClique && config.L1CancunTimeOffset != nil {
op-chain-ops/genesis/testdata/test-deploy-config-full.json:9:  "l1UseClique": false,
op-e2e/config/init.go:206:	dc.L1UseClique = false
packages/contracts-bedrock/deploy-config/sepolia-devnet-0.json:19:  "l1UseClique": false,

Also the CliqueSignerAddress

op-chain-ops/genesis/config.go:783:	// CliqueSignerAddress represents the signer address for the clique consensus engine.
op-chain-ops/genesis/config.go:785:	CliqueSignerAddress common.Address `json:"cliqueSignerAddress"`
op-chain-ops/genesis/genesis.go:156:		extraData = append(append(make([]byte, 32), config.CliqueSignerAddress[:]...), make([]byte, crypto.SignatureLength)...)
op-chain-ops/genesis/testdata/test-deploy-config-full.json:10:  "cliqueSignerAddress": "0x0000000000000000000000000000000000000000",
packages/contracts-bedrock/deploy-config/sepolia-devnet-0.json:18:  "cliqueSignerAddress": "0x0000000000000000000000000000000000000000",

made the necessary adjustment

@mahmudsudo
Copy link
Author

@tynes

@tynes
Copy link
Contributor

tynes commented Oct 22, 2024

CI is currently failing

golangci-lint run -E goimports,sqlclosecheck,bodyclose,asciicheck,misspell,errorlint --timeout 5m -e "errors.As" -e "errors.Is" ./...
op-chain-ops/genesis/genesis.go:149: File is not `goimports`-ed (goimports)

op-e2e/config/init.go:204: File is not `goimports`-ed (goimports)

@mahmudsudo
Copy link
Author

@tynes

@tynes
Copy link
Contributor

tynes commented Oct 24, 2024

/ci authorize fc71aa6

@tynes
Copy link
Contributor

tynes commented Oct 24, 2024

Given the failure in CI, looks like we need to make a small modification to make it pass

This is the error:

                Error Trace:    /var/opt/circleci/data/workdir/op-e2e/actions/helpers/l1_replica.go:87
                                                        /var/opt/circleci/data/workdir/op-e2e/actions/helpers/l1_miner.go:49
                                                        /var/opt/circleci/data/workdir/op-e2e/actions/helpers/setups.go:24
                                                        /var/opt/circleci/data/workdir/op-e2e/actions/helpers/setups.go:67
                                                        /var/opt/circleci/data/workdir/op-e2e/actions/upgrades/ecotone_fork_test.go:60
                Error:          Received unexpected error:
                                only PoS networks are supported, please transition old ones with Geth v1.13.x
                Test:           TestEcotoneNetworkUpgradeTransactions

You can see how the error is created here: https://github.com/ethereum/go-ethereum/blob/3e567b8b2901611f004b5a6070a9b6d286be128d/eth/ethconfig/config.go#L165

This means we should make a modification around here in the callstack:

ethCfg := &ethconfig.Config{

by adding the TerminalTotalDifficulty to the chain config, it should be fine if it is big.NewInt(0), it just cannot be nil. There may be a few other places where this will be needed to be added

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.

op-chain-ops: Cleanup L1 Genesis Generation
2 participants