tests: update for cancun #27721

holiman · 2023-07-13T16:44:45Z

This PR pulls the latest tests, and also the latest "eip tests" -- tests not for general consumption yet, but which are slated for a future fork. It also contains some necessary changes to statetests, state processing and errors, to handle fields related to 4844.

It passes the MCOPY and TSTORE/TLOAD tests, but fails a couple of BLOBHASH tests. These tests seem to be erroneously generated, as the geth stateroot matches besu.

--- FAIL: TestState (0.00s)
    --- FAIL: TestState/opcodeBlobhashOutOfRange.json (0.01s)
        --- FAIL: TestState/opcodeBlobhashOutOfRange.json/Cancun/0/trie (0.00s)
            state_test.go:110: post state root mismatch: got 579ea25b3ac839988eb9aab1f65c92d93031b4635767db7add0d94b6481952ca, want 9deeea534f5391503ba1116a4970f4a9264d5c2038426eb8a5092eea4146fc9d
            state_test.go:112: gas limit too high for EVM trace
        --- FAIL: TestState/opcodeBlobhashOutOfRange.json/Cancun/0/snap (0.00s)
            state_test.go:110: post state root mismatch: got 579ea25b3ac839988eb9aab1f65c92d93031b4635767db7add0d94b6481952ca, want 9deeea534f5391503ba1116a4970f4a9264d5c2038426eb8a5092eea4146fc9d
            state_test.go:112: gas limit too high for EVM trace
    --- FAIL: TestState/opcodeBlobhBounds.json (0.01s)
        --- FAIL: TestState/opcodeBlobhBounds.json/Cancun/0/trie (0.01s)
            state_test.go:110: post state root mismatch: got ad0547ef1a662305f20da1fb1c9c7567eb350c20f06fe001e4e381d442621b81, want 04a95f051977757ec2443a370fc7b46a18c321e8096cc7ca4a654c0d5486eaf3
            state_test.go:112: gas limit too high for EVM trace
        --- FAIL: TestState/opcodeBlobhBounds.json/Cancun/0/snap (0.00s)
            state_test.go:110: post state root mismatch: got ad0547ef1a662305f20da1fb1c9c7567eb350c20f06fe001e4e381d442621b81, want 04a95f051977757ec2443a370fc7b46a18c321e8096cc7ca4a654c0d5486eaf3
            state_test.go:112: gas limit too high for EVM trace

holiman · 2023-07-13T17:01:28Z

We add an instruction BLOBHASH (with opcode HASH_OPCODE_BYTE) which reads index from the top of the stack as big-endian uint256, and replaces it on the stack with tx.blob_versioned_hashes[index] if index < len(tx.blob_versioned_hashes), and otherwise with a zeroed bytes32 value. The opcode has a gas cost of HASH_OPCODE_GAS.

$ go run ./cmd/evm --json statetest ./tests/testdata/EIPTests/StateTests/stEIP4844-blobtransactions/opcodeBlobhashOutOfRange.json 
{"pc":0,"op":96,"gas":"0x3c9ebc","gasCost":"0x3","memSize":0,"stack":[],"depth":1,"refund":0,"opName":"PUSH1"}
{"pc":2,"op":73,"gas":"0x3c9eb9","gasCost":"0x3","memSize":0,"stack":["0x0"],"depth":1,"refund":0,"opName":"BLOBHASH"}
{"pc":3,"op":96,"gas":"0x3c9eb6","gasCost":"0x3","memSize":0,"stack":["0x1a915e4d060149eb4365960e6a7a45f334393093061116b197e3240065ff2d8"],"depth":1,"refund":0,"opName":"PUSH1"}
{"pc":5,"op":85,"gas":"0x3c9eb3","gasCost":"0xb54","memSize":0,"stack":["0x1a915e4d060149eb4365960e6a7a45f334393093061116b197e3240065ff2d8","0x0"],"depth":1,"refund":0,"opName":"SSTORE"}
{"pc":6,"op":96,"gas":"0x3c935f","gasCost":"0x3","memSize":0,"stack":[],"depth":1,"refund":0,"opName":"PUSH1"}
{"pc":8,"op":73,"gas":"0x3c935c","gasCost":"0x3","memSize":0,"stack":["0xa"],"depth":1,"refund":0,"opName":"BLOBHASH"}
{"pc":9,"op":96,"gas":"0x3c9359","gasCost":"0x3","memSize":0,"stack":["0x0"],"depth":1,"refund":0,"opName":"PUSH1"}
{"pc":11,"op":85,"gas":"0x3c9356","gasCost":"0xb54","memSize":0,"stack":["0x0","0x1"],"depth":1,"refund":4800,"opName":"SSTORE"}
{"pc":12,"op":0,"gas":"0x3c8802","gasCost":"0x0","memSize":0,"stack":[],"depth":1,"refund":4800,"opName":"STOP"}

Looks to me like geth does it right, placing a 0 on the stack. I don't know what the test opcodeBlobhashOutOfRange.json expects.

winsvega · 2023-07-13T18:40:21Z

The provided test are calculated on nimbus lets investigate.

This errors should not happen.
Is t8n support cancun yet?

holiman · 2023-07-13T19:20:08Z

This is the state I get for opcodeBlobhashOutOfRange:

"state": {
      "root": "ef7ccce1de9cba62c324f81c6d3c29c2c33bd82c5e02ad2fe4aaa627a70f3033",
      "accounts": {
        "0x095e7baea6a6c7c4c2dfeb977efac326af552d87": {
          "balance": "1000000000000100000",
          "nonce": 0,
          "root": "0x4289ec81d647020d33f2cf78a124c5dcf163b473188a125d4df6335eaa6e9515",
          "codeHash": "0xb66aacbafdc7cb3e298496dd55427edfe775d6f703066bb071e2d2fb2cbfeed2",
          "code": "0x600049600055600a4960015500",
          "storage": {
            "0x0000000000000000000000000000000000000000000000000000000000000000": "01a915e4d060149eb4365960e6a7a45f334393093061116b197e3240065ff2d8"
          },
          "key": "0x0fbc62ba90dec43ec1d6016f9dd39dc324e967f2a3459a78281d1f4b2ba962a6"
        },
        "0x2adc25665018aa1fe0e6bc666dac8fc2697ff9ba": {
          "balance": "56444",
          "nonce": 0,
          "root": "0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421",
          "codeHash": "0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470",
          "key": "0x9d860e7bb7e6b09b87ab7406933ef2980c19d7d0192d8939cf6dc6908a03305f"
        },
        "0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b": {
          "balance": "999999999999646002",
          "nonce": 1,
          "root": "0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421",
          "codeHash": "0xc5d2460186f7233c927e7db2dcc703c0e500b653ca82273b7bfad8045d85a470",
          "key": "0x03601462093b5945d1676df093446790fd31b20e7b12a2e8e5e09d068109616b"
        }
      }

Is t8n support cancun yet?

I think it should, but I think it's lacking blobhash-parsing into transactions. (EDIT: I'm not sure. Maybe it works already)

holiman · 2023-07-13T19:46:24Z

Cc @jangko and @shemnon -- can either of you check my statedump for opcodeBlobhashOutOfRange against nimbus or besu?

holiman · 2023-07-13T19:49:40Z

Re the other failures, where an error is expected.

	for _, tx := range block.Transactions() {
		// Count the number of blobs to validate against the header's dataGasUsed
		blobs += len(tx.BlobHashes())

		// Validate the data blobs individually too
		if tx.Type() == types.BlobTxType {
			if len(tx.BlobHashes()) == 0 {
				return errors.New("no-blob blob transaction present in block body")
			}
			for _, hash := range tx.BlobHashes() {
				if hash[0] != params.BlobTxHashVersion {
					return fmt.Errorf("blob hash version mismatch (have %d, supported %d)", hash[0], params.BlobTxHashVersion)
				}
			}
		}
	}

This ^ is in the block validator, but seems to me that maybe we ought to do it somewhere else too, e.g wherever we to transaction validity checks in general @MariusVanDerWijden ?

winsvega · 2023-07-13T20:48:18Z

here is nimbus post state on that test
looks like gasprice disagreement, but nimbus does not have a vmtrace yet

Running tests for config 'Ethereum NIMBUS on StateTool' 2
Test Case "stEIP4844-blobtransactions": (1 of 1)
100%

Running test State Dump: (StateTests/stEIP4844-blobtransactions/opcodeBlobhashOutOfRange, fork: Cancun, TrInfo: d: 0, g: 0, v: 0, TrData: ` 0x00..`) 
{
    "0x095e7baea6a6c7c4c2dfeb977efac326af552d87" : {
        "code" : "0x600049600055600a4960015500",
        "nonce" : "0x00",
        "balance" : "0x0de0b6b3a76586a0",
        "storage" : {
            "0x00" : "0x01a915e4d060149eb4365960e6a7a45f334393093061116b197e3240065ff2d8"
        }
    },
    "0x2adc25665018aa1fe0e6bc666dac8fc2697ff9ba" : {
        "code" : "0x",
        "nonce" : "0x00",
        "balance" : "0x08dc7c",
        "storage" : {
        }
    },
    "0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b" : {
        "code" : "0x",
        "nonce" : "0x01",
        "balance" : "0x0de0b6b3a73a9932",
        "storage" : {
        }
    }
}
Reported root: 0x9deeea534f5391503ba1116a4970f4a9264d5c2038426eb8a5092eea4146fc9d

shemnon · 2023-07-13T22:03:40Z

We don't have masFeePerDataGas or VersionedHashes wired into state tests at the moment. But when they are it looks like this:

{"pc":0,"op":96,"gas":"0x3c9ebc","gasCost":"0x3","memSize":0,"stack":[],"depth":1,"refund":0,"opName":"PUSH1"}
{"pc":2,"op":73,"gas":"0x3c9eb9","gasCost":"0x3","memSize":0,"stack":["0x0"],"depth":1,"refund":0,"opName":"DATAHASH"}
{"pc":3,"op":96,"gas":"0x3c9eb6","gasCost":"0x3","memSize":0,"stack":["0x1a915e4d060149eb4365960e6a7a45f334393093061116b197e3240065ff2d8"],"depth":1,"refund":0,"opName":"PUSH1"}
{"pc":5,"op":85,"gas":"0x3c9eb3","gasCost":"0xb54","memSize":0,"stack":["0x1a915e4d060149eb4365960e6a7a45f334393093061116b197e3240065ff2d8","0x0"],"depth":1,"refund":0,"opName":"SSTORE"}
{"pc":6,"op":96,"gas":"0x3c935f","gasCost":"0x3","memSize":0,"stack":[],"depth":1,"refund":0,"opName":"PUSH1"}
{"pc":8,"op":73,"gas":"0x3c935c","gasCost":"0x3","memSize":0,"stack":["0xa"],"depth":1,"refund":0,"opName":"DATAHASH"}
{"pc":9,"op":96,"gas":"0x3c9359","gasCost":"0x3","memSize":0,"stack":["0x0"],"depth":1,"refund":0,"opName":"PUSH1"}
{"pc":11,"op":85,"gas":"0x3c9356","gasCost":"0xb54","memSize":0,"stack":["0x0","0x1"],"depth":1,"refund":4800,"opName":"SSTORE"}
{"pc":12,"op":0,"gas":"0x3c8802","gasCost":"0x0","memSize":0,"stack":[],"depth":1,"refund":4800,"opName":"STOP"}
{
 "0x095e7baea6a6c7c4c2dfeb977efac326af552d87": {
  "code": "0x600049600055600a4960015500",
  "storage": {
   "0x0000000000000000000000000000000000000000000000000000000000000000": "0x01a915e4d060149eb4365960e6a7a45f334393093061116b197e3240065ff2d8"
  },
  "balance": "0xde0b6b3a76586a0"
 },
 "0x2adc25665018aa1fe0e6bc666dac8fc2697ff9ba": {
  "balance": "0xdc7c"
 },
 "0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b": {
  "balance": "0xde0b6b3a75a9932",
  "nonce": "0x1"
 },
}
{"output":"","gasUsed":"0x6e3e","test":"opcodeBlobhashOutOfRange","fork":"Cancun","d":0,"g":0,"v":0,"postHash":"0x579ea25b3ac839988eb9aab1f65c92d93031b4635767db7add0d94b6481952ca","postLogsHash":"0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347","pass":false}

It looks like nimbus is giving 0x2adc25665018aa1fe0e6bc666dac8fc2697ff9ba an extra 0x80000/524288 ether balance.

holiman · 2023-07-14T07:17:22Z

account	geth	besu	nimbus
`0x095e7baea6a6c7c4c2dfeb977efac326af552d87`	`1000000000000100000`	`1000000000000100000`	`1000000000000100000`
`0x2adc25665018aa1fe0e6bc666dac8fc2697ff9ba`	`56444`	`56444`	`580732`
`0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b`	`999999999999646002`	`999999999999383858`	`999999999997286706`
Total in state	`1999999999999802446`	`1999999999999540302`	`1999999999997967438`

So nimbus != geth,besu re 0x2adc, and also a threeway split re 0xa94.

holiman · 2023-07-14T07:55:23Z

@MariusVanDerWijden please check f2f7d20#diff-cbeb8282854297a2c8ef02293455cb0258ae75b1d2a1a398b2eb60e59a591ba5R92 and see if you agree

winsvega · 2023-07-14T08:00:52Z

yes the nimbus is likely to be incorrect here, we need a vmtrace from nimbus.
also geth t8n should reject transaction with exception:

createBlobhashTx, that create transaction is not allowed. 
emptyBlobhashList, that tx with empty blobs list is not allowed.

also it is a question about option for t8n to allow generation of invalid cases like in here (if possible)
like a new flag --allowinvalid that t8n will print a warning that the fixture is invalid and like in here, tx are not rejected and accepted. (0 blobhashes, create blob tx and so on, so we can generate invalid tests) but this is a big question because there are many checks to consider.

ethereum/tests#1244

holiman · 2023-07-14T08:20:18Z

These tests are using a basefee of 0x07. Isn't that below the minimum? Maybe that causes some problems.

MariusVanDerWijden · 2023-07-14T08:26:04Z

@MariusVanDerWijden please check f2f7d20#diff-cbeb8282854297a2c8ef02293455cb0258ae75b1d2a1a398b2eb60e59a591ba5R92 and see if you agree

I think its okay. We need to duplicate that check in the txpool anyway to make sure we don't accept any transactions with the wrong version byte in the txpool.

holiman · 2023-07-14T11:44:19Z

I think its okay. We need to duplicate that check in the txpool anyway to make sure we don't accept any transactions with the wrong version byte in the txpool.

Well, what the txpool does nor does not do doesn't matter for the core, since the txpool has no bearing on consensus. For the consensus-checks, I think it's better to have the validation done in one place and one place only. I'm not sure what "I think its okay." meant, if you meant we could remove the ones from block validator or if you meant that it was ok to have it double.

holiman · 2023-07-14T12:40:40Z

Ah, geth is missing some gas-account that currently is implemented in e.g. #27511, hence geth doesn't calculate the transaction gas usage for blob txs correctly.

holiman · 2023-07-14T17:37:33Z

With the latest updates, geth agrees with besu :

--- FAIL: TestState (0.00s)
    --- FAIL: TestState/opcodeBlobhashOutOfRange.json (0.01s)
        --- FAIL: TestState/opcodeBlobhashOutOfRange.json/Cancun/0/trie (0.00s)
            state_test.go:110: post state root mismatch: got 579ea25b3ac839988eb9aab1f65c92d93031b4635767db7add0d94b6481952ca, want 9deeea534f5391503ba1116a4970f4a9264d5c2038426eb8a5092eea4146fc9d
            state_test.go:112: gas limit too high for EVM trace
        --- FAIL: TestState/opcodeBlobhashOutOfRange.json/Cancun/0/snap (0.00s)
            state_test.go:110: post state root mismatch: got 579ea25b3ac839988eb9aab1f65c92d93031b4635767db7add0d94b6481952ca, want 9deeea534f5391503ba1116a4970f4a9264d5c2038426eb8a5092eea4146fc9d
            state_test.go:112: gas limit too high for EVM trace
    --- FAIL: TestState/opcodeBlobhBounds.json (0.01s)
        --- FAIL: TestState/opcodeBlobhBounds.json/Cancun/0/trie (0.01s)
            state_test.go:110: post state root mismatch: got ad0547ef1a662305f20da1fb1c9c7567eb350c20f06fe001e4e381d442621b81, want 04a95f051977757ec2443a370fc7b46a18c321e8096cc7ca4a654c0d5486eaf3
            state_test.go:112: gas limit too high for EVM trace
        --- FAIL: TestState/opcodeBlobhBounds.json/Cancun/0/snap (0.00s)
            state_test.go:110: post state root mismatch: got ad0547ef1a662305f20da1fb1c9c7567eb350c20f06fe001e4e381d442621b81, want 04a95f051977757ec2443a370fc7b46a18c321e8096cc7ca4a654c0d5486eaf3
            state_test.go:112: gas limit too high for EVM trace

core/state_transition.go

fjl · 2023-07-14T18:59:32Z

I think we can merge this now, if we mark the failing tests as failing.

fjl · 2023-07-15T13:23:36Z

I've also added DataGasUsed, which is not used by tests right now, but will likely be needed for pyspec tests later.

winsvega · 2023-07-15T14:27:55Z

Tests do have data gas used

winsvega · 2023-07-15T14:28:29Z

These tests are using a basefee of 0x07. Isn't that below the minimum? Maybe that causes some problems.

That was a minimum allowed basefee as far as I remember

Yes ori did a test and it goes to 7 according to the formula. But there was no strict rule to reject baseffe below 7, just that it will never go lower than 7

This updates the reference tests to the latest version and also adds logic to process EIP-4844 blob transactions into the state transition. We are now passing most Cancun fork tests. Co-authored-by: Marius van der Wijden <m.vanderwijden@live.de> Co-authored-by: Felix Lange <fjl@twurst.com>

…27721)" This reverts commit 1481df6.

holiman added 2 commits July 13, 2023 18:42

tests: update for cancun

fa2fb73

tests: parse blobhashes

274a8a6

cmd/evm: fix --dump option in staterunner

87a02cb

winsvega mentioned this pull request Jul 13, 2023

nimbus t8n to support vmtrace status-im/nimbus-eth1#1544

Closed

core: validate blob versions + nonempty

f2f7d20

holiman requested review from karalabe and rjl493456442 as code owners July 14, 2023 07:54

core: remove blob-checks from block validator

6a24eb9

holiman and others added 4 commits July 14, 2023 14:52

tests: skip blobtests

d6b1744

tests: fix sttransaction json

8d9a774

core: state processing for blobhash opcode

7e264d5

tests: updates

bd0b2f4

holiman commented Jul 14, 2023

View reviewed changes

core/state_transition.go Outdated Show resolved Hide resolved

tests: comment out erroneous tests

9b49d9f

fjl added 2 commits July 15, 2023 12:16

tests: mark broken 4844 tests as failing, rather than skipping them

3ecac07

core: remove blank line

19db4e0

fjl approved these changes Jul 15, 2023

View reviewed changes

fjl added 2 commits July 15, 2023 15:06

core: add DataGasUsed in Genesis

db101fd

core: apply DataGasUsed in genesis

92ce33c

core: test new state errors

fd7b28f

holiman added the cancun label Jul 15, 2023

fjl merged commit b058cf4 into ethereum:master Jul 15, 2023
1 of 2 checks passed

fjl added this to the 1.12.1 milestone Jul 15, 2023

holiman deleted the cancun_tests branch October 11, 2023 07:27

devopsbo3 added a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023

Revert "core, tests: EIP-4844 transaction processing logic (ethereum#…

c4e3285

…27721)" This reverts commit 1481df6.

devopsbo3 added a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023

Revert "core, tests: EIP-4844 transaction processing logic (ethereum#…

f324430

…27721)" This reverts commit 1481df6.

devopsbo3 added a commit to HorizenOfficial/go-ethereum that referenced this pull request Nov 10, 2023

Revert "core, tests: EIP-4844 transaction processing logic (ethereum#…

55f811f

…27721)" This reverts commit 1481df6.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

tests: update for cancun #27721

tests: update for cancun #27721

holiman commented Jul 13, 2023 •

edited

holiman commented Jul 13, 2023

winsvega commented Jul 13, 2023 •

edited

holiman commented Jul 13, 2023 •

edited

holiman commented Jul 13, 2023

holiman commented Jul 13, 2023

winsvega commented Jul 13, 2023 •

edited

shemnon commented Jul 13, 2023

holiman commented Jul 14, 2023

holiman commented Jul 14, 2023

winsvega commented Jul 14, 2023 •

edited

holiman commented Jul 14, 2023

MariusVanDerWijden commented Jul 14, 2023

holiman commented Jul 14, 2023

holiman commented Jul 14, 2023

holiman commented Jul 14, 2023

fjl commented Jul 14, 2023

fjl commented Jul 15, 2023

winsvega commented Jul 15, 2023

winsvega commented Jul 15, 2023 •

edited

tests: update for cancun #27721

tests: update for cancun #27721

Conversation

holiman commented Jul 13, 2023 • edited

holiman commented Jul 13, 2023

winsvega commented Jul 13, 2023 • edited

holiman commented Jul 13, 2023 • edited

holiman commented Jul 13, 2023

holiman commented Jul 13, 2023

winsvega commented Jul 13, 2023 • edited

shemnon commented Jul 13, 2023

holiman commented Jul 14, 2023

holiman commented Jul 14, 2023

winsvega commented Jul 14, 2023 • edited

holiman commented Jul 14, 2023

MariusVanDerWijden commented Jul 14, 2023

holiman commented Jul 14, 2023

holiman commented Jul 14, 2023

holiman commented Jul 14, 2023

fjl commented Jul 14, 2023

fjl commented Jul 15, 2023

winsvega commented Jul 15, 2023

winsvega commented Jul 15, 2023 • edited

holiman commented Jul 13, 2023 •

edited

winsvega commented Jul 13, 2023 •

edited

holiman commented Jul 13, 2023 •

edited

winsvega commented Jul 13, 2023 •

edited

winsvega commented Jul 14, 2023 •

edited

winsvega commented Jul 15, 2023 •

edited