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

core/vm/runtime: set random to enable merge-opcodes #29799

Merged
merged 2 commits into from
May 28, 2024

Conversation

holiman
Copy link
Contributor

@holiman holiman commented May 17, 2024

This PR closes #29782 and #29722 . The IsMerge indicator used by core/vm/runtime checked if the prevrandao/random was set. This is actually a somewhat sane indicator, because if it is nil, it will trigger a nil deref later on if the RANDOM opcode is used.

This PR fixes it by setting it in case we're past Shanghai.

[user@work go-ethereum]$ go run ./cmd/evm --nomemory=false --json --code 5f44 run 
{"pc":0,"op":95,"gas":"0x2540be400","gasCost":"0x2","memSize":0,"stack":[],"depth":1,"refund":0,"opName":"PUSH0"}
{"pc":1,"op":68,"gas":"0x2540be3fe","gasCost":"0x2","memSize":0,"stack":["0x0"],"depth":1,"refund":0,"opName":"DIFFICULTY"}
{"pc":2,"op":0,"gas":"0x2540be3fc","gasCost":"0x0","memSize":0,"stack":["0x0","0x0"],"depth":1,"refund":0,"opName":"STOP"}
{"output":"","gasUsed":"0x4"}

@@ -101,6 +101,10 @@ func setDefaults(cfg *Config) {
if cfg.BlobBaseFee == nil {
cfg.BlobBaseFee = big.NewInt(params.BlobTxMinBlobGasprice)
}
// Merge indicators
if t := cfg.ChainConfig.ShanghaiTime; cfg.ChainConfig.TerminalTotalDifficultyPassed || (t != nil && *t == 0) {
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems like block number can be overriden. Is the *t == 0 check enough?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, it's strictly enough nor correct. We're checking shanghai but shanghai is after merge. This change just makes it work better than it does right now.

Copy link
Member

@rjl493456442 rjl493456442 May 24, 2024

Choose a reason for hiding this comment

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

Shouldn't we also enable these forks by default?

e.g.

diff --git a/core/vm/runtime/runtime.go b/core/vm/runtime/runtime.go
index b587d6d5a0..9c448a80c7 100644
--- a/core/vm/runtime/runtime.go
+++ b/core/vm/runtime/runtime.go
@@ -57,21 +57,32 @@ type Config struct {
 // sets defaults on the config
 func setDefaults(cfg *Config) {
 	if cfg.ChainConfig == nil {
+		var (
+			shanghaiTime = uint64(0)
+			cancunTime   = uint64(0)
+		)
 		cfg.ChainConfig = &params.ChainConfig{
-			ChainID:             big.NewInt(1),
-			HomesteadBlock:      new(big.Int),
-			DAOForkBlock:        new(big.Int),
-			DAOForkSupport:      false,
-			EIP150Block:         new(big.Int),
-			EIP155Block:         new(big.Int),
-			EIP158Block:         new(big.Int),
-			ByzantiumBlock:      new(big.Int),
-			ConstantinopleBlock: new(big.Int),
-			PetersburgBlock:     new(big.Int),
-			IstanbulBlock:       new(big.Int),
-			MuirGlacierBlock:    new(big.Int),
-			BerlinBlock:         new(big.Int),
-			LondonBlock:         new(big.Int),
+			ChainID:                       big.NewInt(1),
+			HomesteadBlock:                new(big.Int),
+			DAOForkBlock:                  new(big.Int),
+			DAOForkSupport:                false,
+			EIP150Block:                   new(big.Int),
+			EIP155Block:                   new(big.Int),
+			EIP158Block:                   new(big.Int),
+			ByzantiumBlock:                new(big.Int),
+			ConstantinopleBlock:           new(big.Int),
+			PetersburgBlock:               new(big.Int),
+			IstanbulBlock:                 new(big.Int),
+			MuirGlacierBlock:              new(big.Int),
+			BerlinBlock:                   new(big.Int),
+			LondonBlock:                   new(big.Int),
+			ArrowGlacierBlock:             nil,
+			GrayGlacierBlock:              nil,
+			TerminalTotalDifficulty:       big.NewInt(0),
+			TerminalTotalDifficultyPassed: true,
+			MergeNetsplitBlock:            nil,
+			ShanghaiTime:                  &shanghaiTime,
+			CancunTime:                    &cancunTime,
 		}
 	}

@fjl fjl merged commit 42471d7 into ethereum:master May 28, 2024
3 checks passed
@fjl fjl added this to the 1.14.4 milestone May 28, 2024
jorgemmsilva pushed a commit to iotaledger/go-ethereum that referenced this pull request Jun 17, 2024
stwiname pushed a commit to subquery/data-node-go-ethereum that referenced this pull request Sep 9, 2024
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.

cmd/evm can't invoke post-merge opcodes correctly
4 participants