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

t8n fails a unit tests when given input data as hex #1078

Open
winsvega opened this issue Jan 14, 2025 · 10 comments
Open

t8n fails a unit tests when given input data as hex #1078

winsvega opened this issue Jan 14, 2025 · 10 comments
Labels
A-tool Area: tooling C-bug Category: this is a bug, deviation, or other problem.

Comments

@winsvega
Copy link
Contributor

We have a problem in test generation.
Currently pyspecs uses decimal inputs provided to t8n in env.json
But some t8n's implemented only hex format without leading zeros

We try switching to hex format now, but one of the unit tests with exec specs t8n fails:

To reproduce run
uvx --with=tox-uv tox -e pytest

on this tests branch export_int_as_hex ethereum/execution-spec-tests#1027

Make sure that t8n accept field format of hex for int types

0x0
0x1
0x12

in env.json

@winsvega winsvega added A-tool Area: tooling C-bug Category: this is a bug, deviation, or other problem. labels Jan 14, 2025
@winsvega
Copy link
Contributor Author

@winsvega
Copy link
Contributor Author

winsvega commented Jan 20, 2025

Here is the env json dump from python unit test

{"fee_recipient":"0x2adc25665018aa1fe0e6bc666dac8fc2697ff9ba","gas_limit":"0x3b9aca00","number":"0x5","timestamp":"0x3e8","prev_randao":null,"difficulty":"0x20000","base_fee_per_gas":null,"excess_blob_gas":null,"target_blobs_per_block":null,"parent_difficulty":null,"parent_timestamp":null,"parent_base_fee_per_gas":null,"parent_gas_used":null,"parent_gas_limit":null,"blob_gas_used":null,"parent_ommers_hash":"0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347","parent_blob_gas_used":null,"parent_excess_blob_gas":null,"parent_beacon_block_root":null,"block_hashes":{"0x1":"0xdac58aa524e50956d0c0bae7f3f8bb9d35381365d07804dd5b48a5a297c06af4"},"ommers":[],"withdrawals":null,"parent_hash":"0xdac58aa524e50956d0c0bae7f3f8bb9d35381365d07804dd5b48a5a297c06af4"}

vs before the PR:

{"fee_recipient":"0x2adc25665018aa1fe0e6bc666dac8fc2697ff9ba","gas_limit":"1000000000","number":"5","timestamp":"1000","prev_randao":null,"difficulty":"131072","base_fee_per_gas":null,"excess_blob_gas":null,"target_blobs_per_block":null,"parent_difficulty":null,"parent_timestamp":null,"parent_base_fee_per_gas":null,"parent_gas_used":null,"parent_gas_limit":null,"blob_gas_used":null,"parent_ommers_hash":"0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347","parent_blob_gas_used":null,"parent_excess_blob_gas":null,"parent_beacon_block_root":null,"block_hashes":{"1":"0xdac58aa524e50956d0c0bae7f3f8bb9d35381365d07804dd5b48a5a297c06af4"},"ommers":[],"withdrawals":null,"parent_hash":"0xdac58aa524e50956d0c0bae7f3f8bb9d35381365d07804dd5b48a5a297c06af4"

and alloc

{"0x095e7baea6a6c7c4c2dfeb977efac326af552d87":{"nonce":"0x00","balance":"0x0de0b6b3a7640000","code":"0x600140","storage":{}},"0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b":{"nonce":"0x00","balance":"0x0de0b6b3a7640000","code":"0x","storage":{}}}

vs before the PR

{"0x095e7baea6a6c7c4c2dfeb977efac326af552d87":{"nonce":"0x00","balance":"0x0de0b6b3a7640000","code":"0x600140","storage":{}},"0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b":{"nonce":"0x00","balance":"0x0de0b6b3a7640000","code":"0x","storage":{}}}

@winsvega
Copy link
Contributor Author

maybe block_hashes dict is an issue

@danceratopz
Copy link
Member

Here's the command to run the failing test in isolation:

uv run pytest -c pytest-framework.ini src/ethereum_clis/tests/test_execution_specs.py::test_evm_t8n[3-ExecutionSpecsTransitionTool]

@danceratopz
Copy link
Member

danceratopz commented Jan 20, 2025

Here is the env json dump from python unit test

Thanks @winsvega, I was also looking at this locally, when you posted. Here's the output as a diff.

42,45c42,45
<       "currentGasLimit": "1000000000",
<       "currentNumber": "5",
<       "currentTimestamp": "1000",
<       "currentDifficulty": "131072",
---
>       "currentGasLimit": "0x3b9aca00",
>       "currentNumber": "0x5",
>       "currentTimestamp": "0x3e8",
>       "currentDifficulty": "0x20000",
48c48
<         "1": "0xdac58aa524e50956d0c0bae7f3f8bb9d35381365d07804dd5b48a5a297c06af4"
---
>         "0x1": "0xdac58aa524e50956d0c0bae7f3f8bb9d35381365d07804dd5b48a5a297c06af4"

@gurukamath perhaps this helps? As Dimitry said, perhaps its the unexpected hex key to the block_hashes dict that's the issue?

    "env": {
      "currentCoinbase": "0x2adc25665018aa1fe0e6bc666dac8fc2697ff9ba",
      "currentGasLimit": "0x3b9aca00",
      "currentNumber": "0x5",
      "currentTimestamp": "0x3e8",
      "currentDifficulty": "0x20000",
      "parentUncleHash": "0x1dcc4de8dec75d7aab85b567b6ccd41ad312451b948a7413f0a142fd40d49347",
      "blockHashes": {
        "0x1": "0xdac58aa524e50956d0c0bae7f3f8bb9d35381365d07804dd5b48a5a297c06af4"
      },
      "ommers": [],
      "parentHash": "0xdac58aa524e50956d0c0bae7f3f8bb9d35381365d07804dd5b48a5a297c06af4"
    }

@danceratopz
Copy link
Member

Ah, why is the blockHashes key 0x1 and not 0x0? This explains why we can still fill tests (all env blockHashes generated by the framework start with 0x0).

@gurukamath hang on a sec, this might be a problem with the unit test data on our side.

@winsvega
Copy link
Contributor Author

Hm why it then doesn't fail on main?

@gurukamath
Copy link
Collaborator

@gurukamath perhaps this helps? As Dimitry said, perhaps its the unexpected hex key to the block_hashes dict that's the issue?

Ah. Yes. This could be an issue. I think we expect the block_hashes keys to be a number in t8n. I can take a look into this

@danceratopz
Copy link
Member

Thanks @gurukamath! It does seem like an obvious culprit, but perhaps not!

We discussed the data sent to t8n in the unit test in question and it's generally correct, except one apparent inconsistency in the env:

  • we signal we're in block 5,
  • we add the blockHash for block 1
  • the parentHash for block 5 is the same as the blockHash for block 1, which must be invalid.

However, as the blockHash is not accessed during the test, it should have no effect. So I think there's still an issue on EELS side, but it's worth bearing in mind though.

Here's the relevant part of the trace from the fail if that helps:

  File "/home/dtopz/.cache/ethereum-spec-evm-resolver/Berlin/src/ethereum_spec_tools/evm_tools/daemon.py", line 66, in do_POST
    main(args=args, out_file=out_wrapper, in_file=input)
  File "/home/dtopz/.cache/ethereum-spec-evm-resolver/Berlin/src/ethereum_spec_tools/evm_tools/__init__.py", line 109, in main
    return t8n_tool.run()
  File "/home/dtopz/.cache/ethereum-spec-evm-resolver/Berlin/src/ethereum_spec_tools/evm_tools/t8n/__init__.py", line 461, in run
    self.apply_body()
  File "/home/dtopz/.cache/ethereum-spec-evm-resolver/Berlin/src/ethereum_spec_tools/evm_tools/t8n/__init__.py", line 377, in apply_body
    process_transaction_return = self.fork.process_transaction(
  File "/home/dtopz/.cache/ethereum-spec-evm-resolver/Berlin/src/ethereum/berlin/fork.py", line 712, in process_transaction
    output = process_message_call(message, env)
  File "/home/dtopz/.cache/ethereum-spec-evm-resolver/Berlin/src/ethereum/berlin/vm/interpreter.py", line 119, in process_message_call
    evm = process_message(message, env)
  File "/home/dtopz/.cache/ethereum-spec-evm-resolver/Berlin/src/ethereum/berlin/vm/interpreter.py", line 233, in process_message
    evm = execute_code(message, env)
  File "/home/dtopz/.cache/ethereum-spec-evm-resolver/Berlin/src/ethereum/berlin/vm/interpreter.py", line 296, in execute_code
    op_implementation[op](evm)
  File "/home/dtopz/.cache/ethereum-spec-evm-resolver/Berlin/src/ethereum/berlin/vm/instructions/block.py", line 49, in block_hash
    push(evm.stack, U256.from_be_bytes(hash))
  File "/home/dtopz/code/github/danceratopz/eest/.venv/lib/python3.10/site-packages/ethereum_types/numeric.py", line 561, in from_be_bytes
    if len(buffer) > byte_count:
TypeError: object of type 'NoneType' has no len()

@winsvega
Copy link
Contributor Author

also hashes being identical is related to question of "to what extent, t8n must verify validity of the inputs"
because having this hashes match allows as to create malformed or malicious blocks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-tool Area: tooling C-bug Category: this is a bug, deviation, or other problem.
Projects
None yet
Development

No branches or pull requests

3 participants