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

Get rid of the hardcoded list of chains in config_arbitrum.go (geth) instead use arbitrum_chain_info.json (nitro) #2658

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

ganeshvanahalli
Copy link
Contributor

@ganeshvanahalli ganeshvanahalli commented Sep 11, 2024

This PR changes nitro to only use arbitrum_chain_info.json to create default ChainConfigs and ArbitrumChainParams and removes these hardcoded defaults from the geth side.

Pulls geth PR- OffchainLabs/go-ethereum#357

Resolves NIT-2732

…instead use arbitrum_chain_info.json (nitro)
@cla-bot cla-bot bot added the s Automatically added by the CLA bot if the creator of a PR is registered as having signed the CLA. label Sep 11, 2024
@ganeshvanahalli ganeshvanahalli marked this pull request as ready for review September 11, 2024 11:06
@@ -0,0 +1,141 @@
// Copyright 2021-2022, Offchain Labs, Inc.
Copy link
Contributor

Choose a reason for hiding this comment

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

nitpick: Update date to 2024

cmd/chaininfo/chain_defaults.go Show resolved Hide resolved
cmd/chaininfo/chain_defaults.go Show resolved Hide resolved
Copy link
Contributor

@diegoximenes diegoximenes left a comment

Choose a reason for hiding this comment

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

LGTM, not approving to check if we can remove code that seems unused.

Comment on lines +103 to +117
func ArbitrumOneParams() params.ArbitrumChainParams {
return fetchArbitrumChainParams("arb1")
}
func ArbitrumNovaParams() params.ArbitrumChainParams {
return fetchArbitrumChainParams("nova")
}
func ArbitrumRollupGoerliTestnetParams() params.ArbitrumChainParams {
return fetchArbitrumChainParams("goerli-rollup")
}
func ArbitrumDevTestParams() params.ArbitrumChainParams {
return fetchArbitrumChainParams("arb-dev-test")
}
func ArbitrumDevTestDASParams() params.ArbitrumChainParams {
return fetchArbitrumChainParams("anytrust-dev-test")
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I didn't find any place in which those functions are used, why we need them?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure, the thing is- they were present on the geth side but not used anywhere either, so I thought its best to just to keep them here as well. Might be useful later on? Or maybe they are used by orbit?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

redirecting to @tsahee

cmd/chaininfo/chain_defaults.go Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design-approved s Automatically added by the CLA bot if the creator of a PR is registered as having signed the CLA.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants