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

Use toml instead of yaml for chain config files #386

Merged
merged 50 commits into from
Jul 17, 2024
Merged

Conversation

bitwiseguy
Copy link
Collaborator

@bitwiseguy bitwiseguy commented Jul 11, 2024

Description

Instead of using a combination of toml, yaml, and json config files, this PR combines the old config yaml, addresses json, and genesis-system-config json into a single toml file. This should make it easier to maintain thesuperchain package bindings going forward. There are already rust and go bindings for that package, but it will also be easier to add other languages in the future.

There are no breaking changes for downstream packages (i.e. op-geth and op-node) introduced by this PR. Instead, the superchain package init() function has just been updated so that it can parse the single toml config file and then populate the same data structures that were exported before.

Additional Context

Fixes #285

@bitwiseguy bitwiseguy requested a review from geoknee July 11, 2024 19:37
@bitwiseguy bitwiseguy force-pushed the ss/toml-over-yaml branch 2 times, most recently from b1c519c to c95da5a Compare July 11, 2024 19:49
Copy link
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 is looking pretty good to me so far, left a few comments.

superchain/superchain.go Outdated Show resolved Hide resolved
superchain/superchain.go Outdated Show resolved Hide resolved
superchain/superchain.go Outdated Show resolved Hide resolved
superchain/superchain.go Show resolved Hide resolved
superchain/superchain.go Show resolved Hide resolved
superchain/configs/sepolia/superchain.toml Outdated Show resolved Hide resolved
@geoknee
Copy link
Collaborator

geoknee commented Jul 16, 2024

The codegen functionality / check will break when we delete the extra addresses json files.

We should migrate codegen.js

/**
* Geneartes the addresses.json file that sits at the root of the
* superchain/extra/addresses folder. Useful to have a combined JSON file for
* all of the various addresses to avoid dynamic imports.
*/

to be performed by the existing Go codegen tool:

func main() {
allChains := make([]ChainEntry, 0)

I think we have all of the utility code we need to do that.

Finally, we would trim the node codegen.js line from the justfile.

superchain/superchain.go Outdated Show resolved Hide resolved
Copy link
Contributor

@refcell refcell left a comment

Choose a reason for hiding this comment

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

Rust code GTG. go LGTM.

@bitwiseguy bitwiseguy marked this pull request as ready for review July 17, 2024 01:52
@bitwiseguy bitwiseguy requested review from a team as code owners July 17, 2024 01:52
@geoknee geoknee merged commit c017220 into main Jul 17, 2024
18 checks passed
@geoknee geoknee deleted the ss/toml-over-yaml branch July 17, 2024 19:44
This pull request was closed.
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.

SuperchainRegistry: modify repo structure
3 participants