Estimate gas chooses wrong instruction set at genesis

[lightclient]

Bug

$ geth --dev dumpgenesis > genesis.json
$ geth --dev --datadir=data init genesis.json
$ geth --dev --datadir=data console
> eth.estimateGas({from: "0xd67dC7d443C902bd6ABfBc91C9E2E6906f55Ca46", data: "0x5959f3"})
Error: invalid opcode: PUSH0
        at web3.js:6382:9(39)
        at send (web3.js:5116:62(29))
        at <eval>:1:16(7)

Culprit

We determine random value based on if difficulty is set:

go-ethereum/core/evm.go

Lines 62 to 64 in a382917

	if header.Difficulty.Cmp(common.Big0) == 0 {
	random = &header.MixDigest
	}

But then here we use it as isMerge for creating the chain rules.

go-ethereum/core/vm/evm.go

Line 148 in a382917

chainRules: chainConfig.Rules(blockCtx.BlockNumber, blockCtx.Random != nil, blockCtx.Time),

The chain rules then dictate the instruction set, so we choose London instructions instead of Shanghai.

Resolution

I guess the main question is should we support this behavior. Is the genesis block technically considered post-merge? A simple fix would be to change the value that goes into Rules(..) to be blockCtx.Random != nil || chainConfig.TerminalTotalDifficultyPassed. But this doesn't feel too great either.

[lightclient]

h/t @fselmo for reporting

[fselmo]

For more context, I am trying to set up a geth --dev with a config that turns on cancunTime. Setting shanghaiTime in genesis config works for me but setting cancunTime will use the difficulty that is set in the genesis file. So even if I set terminalTotalDifficulty=0 and difficulty=1 (since there's a validation that requires genesis difficulty > terminalTotalDifficulty), when I turn on cancunTime: 0 it will use the genesis difficulty and, as stated in the description, will not pick up the proper config rules.

For now, I'm actually able to hack a way around this to coax the config into no longer raising the PUSH0 related error, which I'm assuming means it at least clicks into Shanghai rules. I can do that by setting terminalTotalDifficulty = -1 and difficulty = 0 in genesis config. This seems to somehow work and it will pull difficulty = 0 into the block header and no longer raises with invalid opcode: PUSH0.

Obviously this isn't ideal but it gets me past that and into another situation where the first block won't seal due to invalid parameters. If I'm assuming that --dev mode never enters Cancun here, and I've only wiggled my way into Shanghai, then all of the errors I now see see should make sense. Sealing a Shanghai block would not recognize Cancun's invalid parameters and when I try to send any transaction it hangs on transaction indexing forever.

So it seems that even when I get around the PUSH0 issue, tricking the config to use difficulty = 0, I am still not in the expected EVM state, or at least I think that's the issue related to invalid parameters.

---

log when setting shanghaiTime (working):

INFO [03-29|12:53:45.588] Starting work on payload                 id=0x023860e15e737afe
TRACE[03-29|12:53:45.588] Engine API request received              method=GetPayload        id=0x023860e15e737afe
INFO [03-29|12:53:45.589] Updated payload                          id=0x023860e15e737afe number=1 hash=661826..9c65fd txs=0 withdrawals=0 gas=0 fees=0 root=149cc3..2373b3 elapsed="42.548µs"
TRACE[03-29|12:53:45.589] Engine API request received              method=NewPayload        number=1 hash=661826..9c65fd
INFO [03-29|12:53:45.589] Stopping work on payload                 id=0x023860e15e737afe reason=delivery
TRACE[03-29|12:53:45.589] Inserting block without sethead          hash=661826..9c65fd number=0x100ca0a80

log when setting cancunTime (not working):

INFO [03-29|12:47:50.598] Starting work on payload                 id=0x0274b8c685c83cd5
TRACE[03-29|12:47:50.598] Engine API request received              method=GetPayload        id=0x0274b8c685c83cd5
INFO [03-29|12:47:50.598] Updated payload                          id=0x0274b8c685c83cd5 number=1 hash=f42623..852eec txs=0 withdrawals=0 gas=0 fees=0 root=149cc3..2373b3 elapsed="43.128µs"
INFO [03-29|12:47:50.598] Stopping work on payload                 id=0x0274b8c685c83cd5 reason=delivery
WARN [03-29|12:47:50.598] Error performing sealing work            err="Invalid parameters"

[fselmo]

@lightclient looks like the latest master of geth resolves the EVM rules correctly. I didn't pinpoint exactly what commit or PR resolved it but I believe this can be closed unless there still exists a more specific issue with the difficulty.

---

edit: this commit probably helped 😅

[fselmo]

Correction, when I don't supply gas to transactions, and the library has to call eth_estimateGas, it still errs out with PUSH0 not supported (pre-shanghai EVM).

However, in current master, if I do provide gas for every transaction OR if I do the hack I posted above where I set terminalTotalDifficulty = -1 and difficulty = 0 for genesis, it does trick the current setup to find the correct EVM rules.

[jwasinger]

Resolved in #29469 .

[fselmo]

@jwasinger I'm still seeing this running against geth master branch

WARN [04-18|09:34:02.022] Served eth_estimateGas                   reqid=3 duration="419.912µs" err="invalid opcode: PUSH0"

I specify terminalTotalDifficulty in my genesis at 0 but since difficulty has to be greater than TTD, I have to put something other than 0 which might render that change (in #29469) obsolete. If a user specifies TTD = 0, I believe difficulty should also be allowed to be set to 0, no?

With the -1 "hack" I mentioned above, master branch works perfectly. So I think it's a matter of being able to set both values to 0 and also perhaps validating that TTD cannot be negative.

[fselmo]

I opened #29579 that resolves things for me. Perhaps just a better message when difficulty > TTD and we are post-merge at genesis would make for better UX.

[jwasinger]

@jwasinger I'm still seeing this running against geth master branch

WARN [04-18|09:34:02.022] Served eth_estimateGas                   reqid=3 duration="419.912µs" err="invalid opcode: PUSH0"

I specify terminalTotalDifficulty in my genesis at 0 but since difficulty has to be greater than TTD, I have to put something other than 0 which might render that change (in #29469) obsolete. If a user specifies TTD = 0, I believe difficulty should also be allowed to be set to 0, no?

With the -1 "hack" I mentioned above, master branch works perfectly. So I think it's a matter of being able to set both values to 0 and also perhaps validating that TTD cannot be negative.

That's weird that master is still broken for you. Trying the example that Matt posted in the issue description of #29404 (comment) works for me.

[fselmo]

hmmm does your genesis specify TTD and difficulty values when you tested?

[jwasinger]

Yes, I tested with the default geth dev mode config. Genesis TTD/difficulty are both 0, which I agree seems wrong as this would make the genesis block the last pre-merge block.

But post-merge features are activated at genesis, and the withdrawal shim functionality still works so... 🤷‍♂️ .

[fselmo]

If I use my own genesis I can't set them both to 0 is what I think the problem is. It requires difficulty > TTD and even at TTD = 0 (lowest it should be able to go) difficulty has to be above that and it breaks. What's a genesis config you would recommend if I'm trying to use my own? TTD can't be null and difficulty can't not be set. I'm not sure how I can get around it.

[jwasinger]

If I use my own genesis I can't set them both to 0 is what I think the problem is.

The default genesis config for dev mode (geth --dev dumpgenesis) sets these both to 0. So a custom configuration based on this should work.

[fselmo]

Correct but if a custom genesis is used, it enters that validation block where difficulty > TTD is validated. Which is why I submitted that PR to allow both of them to be 0 in a custom genesis. That validation is not triggered when the default is used afaict.

[jwasinger]

that validation block where difficulty > TTD is validated.

Can you link this in the code? And if setting TTD/difficulty to 0 is still breaking with your config, can you post a reproducable example (as this would indicate another bug at this point)?

If I geth --dev dumpgenesis and geth init against that configuration, it works fine for me locally.

[fselmo]

Can you link this in the code?

It's that code block both you and I ended up making the same exact change. I'm almost to my computer and can try to post a quick example. But it actually looks like that whole block is triggered if the DataDir flag is set?

Starts at flags.go line 1864. I'm not sure the default configuration enters that validation block because if it did it wouldn't allow difficulty to be the same as TTD (both 0).

Or you also need the datadir flag and even with the default configuration it would trigger.

[jwasinger]

Ah whoops. You are correct, we will fail to start up when a custom dev genesis block is used (https://github.com/ethereum/go-ethereum/blob/master/cmd/utils/flags.go#L1880) .

I forgot to switch my branch from #29566 ..

[fselmo]

Yep, that's the spot. At my computer now, should we continue the conversation in #29579?