Skip to content

Convert add-chain.sh into a go package#204

Merged
bitwiseguy merged 33 commits intomainfrom
ss/addchain-go
May 3, 2024
Merged

Convert add-chain.sh into a go package#204
bitwiseguy merged 33 commits intomainfrom
ss/addchain-go

Conversation

@bitwiseguy
Copy link
Copy Markdown
Collaborator

@bitwiseguy bitwiseguy commented Apr 23, 2024

Description

This PR converts most of the existing add-chain.sh shell script code into go. As the add-chain.sh shell script grows more complex, it requires us to cover more edge cases and complex conditional statements/branching logic. Golang has the following advantages compared to shell scripting:

  • better suited to handle complex logic
  • easier to write unit/e2e tests
  • easier to modularize
  • provides type safety

Tests

An end-to-end test has been added at add-chain/e2e_test.go.

Additional context

This aims to be backwards compatible with the existing documentation in CONTRIBUTING.md that instructs the user to execute sh ./scripts/add-chain.sh standard. Therefore, no documentation changes need to be made as a result of this work.

The add-chain.sh script becomes a wrapper that invokes two go programs:

Follow-up work could include moving [2] from the optimism monorepo into a new directory within the superchain-registry repo: superchain-registry/registry-data. This would require removing some light dependencies that program currently has on op-service utility functions, but it would allow all of the scripts/programs related to adding a chain to the superchain-registry to live in the superchain-registry repo.

Metadata

Comment thread addchain/contract_addresses.go Outdated
Comment thread addchain/main.go Outdated
Comment thread addchain/genesis.go Outdated
Comment thread addchain/utils.go Outdated
@bitwiseguy bitwiseguy marked this pull request as ready for review April 25, 2024 13:20
@bitwiseguy bitwiseguy requested a review from a team April 25, 2024 13:20
@bitwiseguy bitwiseguy requested a review from a team as a code owner April 25, 2024 13:20
@bitwiseguy bitwiseguy requested a review from refcell April 25, 2024 13:20
Comment thread add-chain/go.mod Outdated
@geoknee geoknee self-requested a review April 25, 2024 20:28
geoknee

This comment was marked as outdated.

@bitwiseguy
Copy link
Copy Markdown
Collaborator Author

@geoknee regarding your test failure : I'm hoping this commit will fix it because I believe the smart contract function name is case sensitive. Let me know if it helps.

@geoknee
Copy link
Copy Markdown
Collaborator

geoknee commented Apr 29, 2024

@geoknee regarding your test failure : I'm hoping this commit will fix it because I believe the smart contract function name is case sensitive. Let me know if it helps.

This ended up being a problem with my local setup. I fixed that and the script worked!

@geoknee
Copy link
Copy Markdown
Collaborator

geoknee commented Apr 29, 2024

Looks like tests failed at least once because of rate limiting. To combat that we could follow this pattern:

params.SubmissionInterval, err = retry.Do(ctx, maxAttempts, retry.Exponential(),
func() (*big.Int, error) {
return l2OO.SubmissionInterval(callOpts)
})

Comment thread add-chain/contract_addresses.go
Comment thread add-chain/.gitignore
Copy link
Copy Markdown
Collaborator

@geoknee geoknee left a comment

Choose a reason for hiding this comment

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

This looks good to me but left quite a few comments.

Comment thread superchain/superchain.go Outdated
Comment thread superchain/superchain.go Outdated
Comment thread add-chain/main.go
Comment thread add-chain/contract_addresses.go
Comment thread add-chain/testdata/monorepo/op-node/rollup.json
@geoknee geoknee requested a review from mds1 May 2, 2024 15:31
Comment thread add-chain/testdata/monorepo/deployments/.deploy
Comment thread add-chain/testdata/superchain/extra/addresses/sepolia/expected.json
Comment thread add-chain/chain_config.go Outdated
Comment thread scripts/add-chain.sh
Comment thread go.work
@bitwiseguy bitwiseguy merged commit 10d69d7 into main May 3, 2024
@bitwiseguy bitwiseguy deleted the ss/addchain-go branch May 3, 2024 15:14
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.

bug: fix error handling in add-chain.sh Improve hardfork timestamp override handling in add-chain.sh

4 participants