Skip to content

test: Load transaction chainId as uint64#1570

Merged
chfast merged 1 commit into
masterfrom
tooling/t8n-chainid-uint64
Jun 17, 2026
Merged

test: Load transaction chainId as uint64#1570
chfast merged 1 commit into
masterfrom
tooling/t8n-chainid-uint64

Conversation

@chfast

@chfast chfast commented Jun 17, 2026

Copy link
Copy Markdown
Member

from_json_tx_common parsed the transaction "chainId" with from_json<uint8_t>, throwing from_json<uint8_t>: value > 0xFF for any chain ID above 255. This crashed evmone t8n on common networks such as Sepolia (11155111) and Arbitrum One (42161): the transaction is parsed before t8n overrides chain_id from its args, so the parse failed first and no output was produced.

Parse it as uint64_t, matching the Transaction::chain_id field type.

Fixes #1569.

from_json_tx_common parsed the transaction "chainId" with
from_json<uint8_t>, throwing `from_json<uint8_t>: value > 0xFF` for any
chain ID above 255. This crashed `evmone t8n` on common networks such as
Sepolia (11155111) and Arbitrum One (42161): the transaction is parsed
before t8n overrides chain_id from its args, so the parse failed first
and no output was produced.

Parse it as uint64_t, matching the Transaction::chain_id field type.

Fixes #1569.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Fixes a tooling crash in evmone t8n/statetest loaders by parsing per-transaction JSON "chainId" as uint64_t (matching state::Transaction::chain_id), instead of uint8_t which rejected real-world chain IDs > 255 (e.g. Sepolia, Arbitrum).

Changes:

  • Update from_json_tx_common() to load "chainId" with from_json<uint64_t>.
  • Add unit test coverage ensuring chainId == UINT64_MAX is accepted/loaded correctly in both the statetest loader and t8n tooling.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
test/utils/statetest_loader.cpp Fixes the core parsing bug by switching "chainId" parsing from uint8_t to uint64_t.
test/unittests/tooling_t8n_test.cpp Adds a regression test ensuring t8n() can parse/handle a transaction JSON with maximal chainId.
test/unittests/statetest_loader_tx_test.cpp Adds a regression test ensuring the statetest transaction loader preserves maximal chainId without truncation/overflow.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@codecov

codecov Bot commented Jun 17, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 97.37%. Comparing base (1ea7547) to head (0576587).
⚠️ Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1570   +/-   ##
=======================================
  Coverage   97.36%   97.37%           
=======================================
  Files         163      163           
  Lines       14488    14529   +41     
  Branches     3385     3388    +3     
=======================================
+ Hits        14106    14147   +41     
  Misses        280      280           
  Partials      102      102           
Flag Coverage Δ
eest-develop 89.66% <100.00%> (ø)
eest-develop-gmp 26.32% <0.00%> (-0.08%) ⬇️
eest-legacy 17.67% <2.38%> (-0.06%) ⬇️
eest-libsecp256k1 27.97% <0.00%> (-0.08%) ⬇️
eest-stable 89.75% <100.00%> (ø)
evmone-unittests 92.54% <100.00%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
core 95.95% <ø> (ø)
tooling 90.21% <100.00%> (ø)
tests 99.79% <100.00%> (+<0.01%) ⬆️
Files with missing lines Coverage Δ
test/unittests/statetest_loader_tx_test.cpp 100.00% <100.00%> (ø)
test/unittests/tooling_t8n_test.cpp 100.00% <100.00%> (ø)
test/utils/statetest_loader.cpp 93.11% <100.00%> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@chfast chfast merged commit f200c0d into master Jun 17, 2026
24 checks passed
@chfast chfast deleted the tooling/t8n-chainid-uint64 branch June 17, 2026 12:27
chfast added a commit that referenced this pull request Jun 17, 2026
A legacy EIP-155 transaction encodes `v` as chainId*2 + 35 + parity, so
`v` exceeds 0xff for any chainId >= 110 (e.g. chainId 300 gives v=0x27c).
The state-test loader narrowed `v` to `uint8_t` and `Transaction::v` was
`uint8_t`, so `from_json<uint8_t>` threw `value > 0xFF` and `evmone t8n`
rejected such transactions while geth `evm t8n` executes them.

Widen `Transaction::v` and its loader to `uint64_t`, matching the `chainId`
fix (#1570). `v` is only RLP-encoded for the transaction hash, so the wider
type is encoded correctly with no other behavior change.

Fixes #1487.
chfast added a commit that referenced this pull request Jun 17, 2026
A legacy EIP-155 transaction encodes `v` as chainId*2 + 35 + parity, so
`v` exceeds 0xff for any chainId >= 110 (e.g. chainId 300 gives v=0x27c).
The state-test loader narrowed `v` to `uint8_t` and `Transaction::v` was
`uint8_t`, so `from_json<uint8_t>` threw `value > 0xFF` and `evmone t8n`
rejected such transactions while geth `evm t8n` executes them.

Widen `Transaction::v` and its loader to `uint64_t`, matching the `chainId`
fix (#1570). `v` is only RLP-encoded for the transaction hash, so the wider
type is encoded correctly with no other behavior change.

Fixes #1487.
chfast added a commit that referenced this pull request Jun 17, 2026
A legacy EIP-155 transaction encodes `v` as chainId*2 + 35 + parity, so
`v` exceeds 0xff for any chainId >= 110 (e.g. chainId 300 gives v=0x27c).
The state-test loader narrowed `v` to `uint8_t` and `Transaction::v` was
`uint8_t`, so `from_json<uint8_t>` threw `value > 0xFF` and `evmone t8n`
rejected such transactions while geth `evm t8n` executes them.

Widen `Transaction::v` and its loader to `uint64_t`, matching the `chainId`
fix (#1570). `v` is only RLP-encoded for the transaction hash, so the wider
type is encoded correctly with no other behavior change.

Fixes #1487.
chfast added a commit that referenced this pull request Jun 17, 2026
A legacy EIP-155 transaction encodes `v` as chainId*2 + 35 + parity, so
`v` exceeds 0xff for any chainId >= 110 (e.g. chainId 300 gives
v=0x27c). The state-test loader narrowed `v` to `uint8_t` and
`Transaction::v` was `uint8_t`, so `from_json<uint8_t>` threw `value >
0xFF` and `evmone t8n` rejected such transactions while geth `evm t8n`
executes them.

Widen `Transaction::v` and its loader to `uint64_t`, matching the
`chainId` fix (#1570). `v` is only RLP-encoded for the transaction hash,
so the wider type is encoded correctly with no other behavior change.

Fixes #1487.
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.

evmone-t8n rejects transactions with chainId > 0xFF (parsed as uint8)

2 participants