go-ethereum icon indicating copy to clipboard operation
go-ethereum copied to clipboard

`debug_traceBlockBy{Number,Hash}` outputs new opcodes for pre-fork blocks

Open bearpebble opened this issue 10 months ago • 4 comments

System information

Geth version: geth version 1.13.14-stable-2bd6bd01 CL client & version: none OS & Version: Linux

Expected behaviour

The output of debug_traceBlockByNumber should contain invalid for cancun opcodes in pre-cancun blocks (or any other fork that introduces new opcodes).

Actual behaviour

The trace contains the new opcodes BLOBHASH, BLOBBASEFEE, TLOAD, TSTORE, MCOPY despite these not being active yet.

Steps to reproduce the behaviour

Make sure cancun is not active

geth --dev dumpgenesis | jq ".config"
{
  "chainId": 1337,
  "homesteadBlock": 0,
  "eip150Block": 0,
  "eip155Block": 0,
  "eip158Block": 0,
  "byzantiumBlock": 0,
  "constantinopleBlock": 0,
  "petersburgBlock": 0,
  "istanbulBlock": 0,
  "muirGlacierBlock": 0,
  "berlinBlock": 0,
  "londonBlock": 0,
  "arrowGlacierBlock": 0,
  "grayGlacierBlock": 0,
  "shanghaiTime": 0,
  "terminalTotalDifficulty": 0,
  "terminalTotalDifficultyPassed": true
}

Start geth and fund the test account

geth --http --http.port 8545 --dev --http.api eth,web3,net,debug --gcmode=archive &
sleep 10
geth attach --exec 'eth.sendTransaction({from: eth.coinbase, to: "0xf39Fd6e51aad88F6F4ce6aB8827279cffFb92266", value: web3.toWei(50, "ether")})' /tmp/geth.ipc

Send a transaction that uses BLOBHASH, specifically PUSH0, PUSH0, BLOBHASH, PUSH0, PUSH0.

cast send --rpc-url http://localhost:8545 --mnemonic "test test test test test test test test test test test junk" --gas-limit 500000 --create 0x5f5f495f5f
cast rpc debug_traceBlockByNumber $(cast block-number | cast to-hex) --rpc-url http://localhost:8545 | jq

Output of the trace will contain BLOBHASH as the last opcode despite the opcode being interpreted as invalid opcode.

[
  {
    "txHash": "0x5c0990c1793ac3bafc8c298766225ad544eb984c53535e004e9f06f01b1c8aae",
    "result": {
      "gas": 500000,
      "failed": true,
      "returnValue": "",
      "structLogs": [
        {
          "pc": 0,
          "op": "PUSH0",
          "gas": 446918,
          "gasCost": 2,
          "depth": 1,
          "stack": []
        },
        {
          "pc": 1,
          "op": "PUSH0",
          "gas": 446916,
          "gasCost": 2,
          "depth": 1,
          "stack": [
            "0x0"
          ]
        },
        {
          "pc": 2,
          "op": "BLOBHASH",
          "gas": 446914,
          "gasCost": 0,
          "depth": 1,
          "stack": [
            "0x0",
            "0x0"
          ]
        }
      ]
    }
  }
]

bearpebble avatar Apr 05 '24 12:04 bearpebble

I wonder is this the right way to resolve the issue you left.

I tried to handle it by adding the code below at here

	// for handling issue#29464
	if vmenv.ChainConfig().IsCancun(txctx.BlockNumber, vmenv.Context.Time) != true {
		for _, opcode := range message.Data {
			var opcode = vm.OpCode(opcode)

			// cancun, EIP-4844
			if opcode == vm.StringToOp("BLOBHASH") {
				return nil, fmt.Errorf("invalid cancun opcode: %s", opcode.String())
			}
			// cancun, EIP-7516
			if opcode == vm.StringToOp("BLOBBASEFEE") {
				return nil, fmt.Errorf("invalid cancun opcode: %s", opcode.String())
			}
			// cancun, EIP-1153
			if opcode == vm.StringToOp("TLOAD") || opcode == vm.StringToOp("TSTORE") {
				return nil, fmt.Errorf("invalid cancun opcode: %s", opcode.String())
			}
			// cancun, EIP-5656
			if opcode == vm.StringToOp("MCOPY") {
				return nil, fmt.Errorf("invalid cancun opcode: %s", opcode.String())
			}
			// cancun, EIP-6780 skip
		}
	}

Output is not the trace anymore but an error message.

(code: -32000, message: invalid cancun opcode: BLOBHASH, data: None)

you can also check the log from geth client

image

c0np4nn4 avatar May 14 '24 06:05 c0np4nn4

@c0np4nn4 I don't think that's the correct way of handling this. Tracing should still work and not return an error if there were invalid opcodes in the transaction.

bearpebble avatar May 15 '24 07:05 bearpebble

@s1na from looking at other issues it seems you are the most familiar with tracing. Do you believe this can be considered a bug that is worth fixing?

bearpebble avatar May 15 '24 08:05 bearpebble

The old trace format is bad this way (hence why we created a different output-format, standard-tracing, which has opCode as a byte, and opString as a string).

The actual underlying bytecode is non-ambiguous. Translating a byte into a string-representation is context-dependent, e.g DIFFICULTY and RANDAO are two names for the same op-byte, and depends on the fork you're at. In some places, we're not chain-rule-aware, and just output something.

For fuzzing, I never look at the string-name of the operation. That's for human consumption. The debug_traceBlock output-format is inherently flawed, IMO

holiman avatar May 15 '24 08:05 holiman