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.

Sign up for GitHub

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

Jump to bottom

Feat/add chain spec hardfork #56

Merged
merged 43 commits into from
Dec 9, 2024
Merged

Feat/add chain spec hardfork #56

merged 43 commits into from
Dec 9, 2024

Conversation

georgehao
Copy link
Member

No description provided.

Thegaram
Thegaram reviewed Dec 3, 2024
View reviewed changes
crates/net/peers/src/bootnodes/mod.rs Outdated Show resolved Hide resolved
crates/scroll/chainspec/res/genesis/dev.json Outdated Show resolved Hide resolved
crates/scroll/chainspec/res/genesis/scroll.json Outdated Show resolved Hide resolved
crates/scroll/chainspec/res/genesis/sepolia_scroll.json Outdated Show resolved Hide resolved
crates/scroll/chainspec/src/scroll.rs Outdated Show resolved Hide resolved
crates/scroll/chainspec/src/dev.rs Outdated Show resolved Hide resolved
crates/scroll/chainspec/src/lib.rs Outdated Show resolved Hide resolved
crates/scroll/hardforks/src/hardfork.rs Show resolved Hide resolved
georgehao and others added 2 commits December 4, 2024 00:49
@georgehao @Thegaram
Update crates/net/peers/src/bootnodes/mod.rs
8368780 
Co-authored-by: Péter Garamvölgyi <peter@scroll.io>
@georgehao
address comment
cd3f50e
greged93
greged93 reviewed Dec 4, 2024
View reviewed changes
Copy link
Collaborator

@greged93 greged93 left a comment

Choose a reason for hiding this comment

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

I think we should not have the EIP150, EIP155 and EIP158 in the Scroll hardforks, these should be Tangerine and SpuriousDragon

crates/scroll/chainspec/src/genesis.rs Outdated Show resolved Hide resolved
crates/scroll/chainspec/src/genesis.rs Outdated Show resolved Hide resolved
crates/scroll/hardforks/src/hardfork.rs Outdated Show resolved Hide resolved
crates/scroll/hardforks/src/hardfork.rs Outdated Show resolved Hide resolved
crates/scroll/hardforks/src/hardfork.rs Outdated Show resolved Hide resolved
crates/scroll/hardforks/src/hardfork.rs Outdated Show resolved Hide resolved
crates/scroll/hardforks/src/hardfork.rs Outdated Show resolved Hide resolved
crates/scroll/hardforks/src/hardfork.rs Outdated Show resolved Hide resolved
crates/scroll/hardforks/src/hardfork.rs Outdated Show resolved Hide resolved
@georgehao georgehao changed the title (WIP)Feat/add chain spec hardfork Feat/add chain spec hardfork Dec 4, 2024
frisitano
frisitano reviewed Dec 6, 2024
View reviewed changes
Copy link
Collaborator

@frisitano frisitano left a comment

Choose a reason for hiding this comment

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

Added a few comments inline, mostly around naming and documentation.

Cargo.toml Outdated Show resolved Hide resolved
crates/scroll/chainspec/src/genesis.rs Outdated Show resolved Hide resolved
crates/scroll/chainspec/src/genesis.rs Outdated Show resolved Hide resolved
crates/scroll/chainspec/src/scroll.rs Outdated Show resolved Hide resolved
crates/scroll/chainspec/src/scroll.rs Show resolved Hide resolved
crates/scroll/chainspec/src/lib.rs Outdated Show resolved Hide resolved
crates/scroll/chainspec/src/lib.rs Outdated Show resolved Hide resolved
crates/scroll/chainspec/src/lib.rs Outdated Show resolved Hide resolved
crates/scroll/chainspec/src/lib.rs Outdated Show resolved Hide resolved
crates/scroll/hardforks/src/dev.rs Outdated Show resolved Hide resolved
greged93
greged93 reviewed Dec 6, 2024
View reviewed changes
crates/chainspec/src/spec.rs Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
greged93
greged93 reviewed Dec 6, 2024
View reviewed changes
Copy link
Collaborator

@greged93 greged93 left a comment

Choose a reason for hiding this comment

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

just a couple of small comments on the ScrollChainSpecBuilder

crates/scroll/chainspec/src/lib.rs Outdated Show resolved Hide resolved
crates/scroll/chainspec/src/lib.rs Show resolved Hide resolved
crates/scroll/chainspec/src/lib.rs Outdated Show resolved Hide resolved
crates/scroll/chainspec/src/lib.rs Outdated Show resolved Hide resolved
greged93
greged93 previously approved these changes Dec 6, 2024
View reviewed changes
Copy link
Collaborator

@greged93 greged93 left a comment

Choose a reason for hiding this comment

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

lgtm

frisitano
frisitano previously approved these changes Dec 6, 2024
View reviewed changes
Copy link
Collaborator

@frisitano frisitano left a comment

Choose a reason for hiding this comment

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

LGTM. I think the additional changes required for this will become apparent when we integrate with the execution strategy in #63.

@georgehao georgehao dismissed stale reviews from frisitano and greged93 via 13f2f59 December 9, 2024 02:04
frisitano
frisitano approved these changes Dec 9, 2024
View reviewed changes
Copy link
Collaborator

@frisitano frisitano left a comment

Choose a reason for hiding this comment

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

Looks good! LGTM.

@frisitano frisitano merged commit c3c2dde into scroll Dec 9, 2024
40 checks passed
@frisitano frisitano deleted the feat/add_chain_spec_hardfork branch December 9, 2024 10:08
@greged93 greged93 mentioned this pull request Dec 10, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Thegaram Thegaram Thegaram left review comments

@frisitano frisitano frisitano approved these changes

@greged93 greged93 greged93 left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@georgehao @Thegaram @frisitano @greged93