test: Take the state test transaction from its encoding - #1614
Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves state test execution fidelity by preferring the canonical transaction serialization (txbytes) when available, instead of trusting the decomposed JSON transaction template, enabling detection of additional decoding/encoding failures in state tests.
Changes:
- Added
txbytesstorage to per-case expectations and loaded it from JSON when present. - Updated the state test runner to decode transactions from
txbytes(falling back to the template transaction when not provided) and to reportINVALID_ENCODINGon decode failure. - Introduced
INVALID_ENCODINGas a new transaction error code and added export mapping for it.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| test/utils/statetest.hpp | Adds txbytes to per-case expectations to carry the canonical serialized transaction bytes. |
| test/utils/statetest_loader.cpp | Loads optional txbytes from JSON into expectations. |
| test/utils/statetest_export.cpp | Maps the new INVALID_ENCODING error to the standardized exported error string. |
| test/statetest/statetest_runner.cpp | Decodes transaction from txbytes when provided; returns INVALID_ENCODING if decoding fails. |
| test/state/errors.hpp | Adds INVALID_ENCODING to the error enum and category messages. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1614 +/- ##
==========================================
- Coverage 97.48% 97.46% -0.03%
==========================================
Files 170 170
Lines 15376 15388 +12
Branches 3591 3596 +5
==========================================
+ Hits 14990 14998 +8
- Misses 281 282 +1
- Partials 105 108 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
| tx = state::decode_transaction(*expected.txbytes); | ||
| if (tx.has_value()) | ||
| tx->sender = template_tx.sender; // No recovery yet, take sender from JSON. |
| const auto res = | ||
| tx.has_value() ? | ||
| transition(state, block, test.block_hashes, *tx, rev, vm, block.gas_limit, | ||
| static_cast<int64_t>(state::max_blob_gas_per_block(blob_params))) : | ||
| make_error_code(state::INVALID_ENCODING); |
1c514a6 to
7f72262
Compare
A state test's transaction is built from a shared multi-transaction template, which describes the transaction rather than being it: the template cannot express a malformed encoding, so a case whose defect lives in the bytes was executed as if it were well formed. In execution-specs tests@v20.0.1 that is the whole frontier/validation/bad_v_r_s family, 116 cases the runner reported as "unexpected valid transaction". Each post entry carries the serialization it was built from as "txbytes". Decode it and run that transaction instead; input that does not decode makes the transaction invalid (INVALID_ENCODING). The sender still comes from the template, so nothing about the signature is verified yet.
7f72262 to
57e3d53
Compare
The state test runner still took the sender from the fixture's template, so a signature was only ever checked for shape. The remaining frontier/validation/bad_v_r_s cases are legacy transactions that decode cleanly and carry an out-of-range r or s; nothing rejected them. Add state::recover_sender() and use it, as a node does; a signature that does not recover makes the transaction invalid (INVALID_SIGNATURE). Recovery is strict, so EIP-2 low-s and r, s in [1, secp256k1n) come from ecrecover itself. The signing preimage is a slice of the serialization -- the payload without the trailing (v, r, s), and for a protected legacy transaction (chain_id, 0, 0) in their place -- so recover_sender() takes the decoded transaction together with the bytes it came from, and finds the end of the signed prefix by subtracting the sizes of the canonically encoded signature fields. Reusing the slice avoids restating every transaction type's field order next to rlp_encode(), which already states it. The exported "expectException" name is added for the new code as well; EEST names encoding failures one by one, so INVALID_ENCODING, minted by the same runner path since #1614, gets its plain message instead.
Load test transaction from its encoding in
"txbytes"instead of trusting the decomposed JSON variant.This detected some additional expect decoding failures in tests.
We still lack sender address recovery from signatures, so take it as previously from JSON
"sender".