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

all: define and enable the Berlin hard fork on all networks #22380

Merged
merged 1 commit into from
Feb 26, 2021

Conversation

karalabe
Copy link
Member

@karalabe karalabe commented Feb 25, 2021

@karalabe karalabe added this to the 1.10.0 milestone Feb 25, 2021
@karalabe karalabe requested review from fjl and holiman February 25, 2021 07:33
@karalabe karalabe changed the title all: defina and enable the Berlin hard fork on all networks all: define and enable the Berlin hard fork on all networks Feb 25, 2021
@karalabe karalabe force-pushed the berlin branch 2 times, most recently from 48d6b11 to 5eca033 Compare February 26, 2021 09:19
@holiman
Copy link
Contributor

holiman commented Feb 26, 2021

LGTM, maybe with one little diff suggestion - would be good to remove as much yolov3 stuff as possible

diff --git a/eth/tracers/api.go b/eth/tracers/api.go
index 013f5d52dd..cfb41a7ec3 100644
--- a/eth/tracers/api.go
+++ b/eth/tracers/api.go
@@ -585,8 +585,8 @@ func (api *API) standardTraceBlockToFile(ctx context.Context, block *types.Block
 			chainConfig.BerlinBlock = berlin
 			canon = false
 		}
-		if yolov3 := config.LogConfig.Overrides.YoloV3Block; yolov3 != nil {
-			chainConfig.YoloV3Block = yolov3
+		if berlin := config.LogConfig.Overrides.BerlinBlock; berlin != nil {
+			chainConfig.BerlinBlock = berlin
 			canon = false
 		}
 	}

@karalabe
Copy link
Member Author

I did add Berlin too there just the lines above :P Not sure if I can nuke it given that it accesses Yolo directly. I'm happy to remove support for Yolo. Should we also drop it for Berlin? Any particular reason to have this override?

@holiman
Copy link
Contributor

holiman commented Feb 26, 2021

Oops.
Anyway, the reason to have it is to enable folks to pre-check, before Berlin, how a particular block would behave with the upcoming rules, and perform analysis. So far, I estimate that I'm the only one who's ever done so (which is why I'm not concerned about dropping yolov3), but it might be good to leave it in there if others wants to analyze things, as berlin draws closer.

@karalabe
Copy link
Member Author

Removed Yolov3 from there. PTAL

@karalabe karalabe merged commit 744707a into ethereum:master Feb 26, 2021
Copy link

@Platinumwrist Platinumwrist left a comment

Choose a reason for hiding this comment

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

cmd/geth/config.go

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.

3 participants