state: Recover sender address from transaction signature - #1615
Merged
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #1615 +/- ##
==========================================
- Coverage 97.46% 97.46% -0.01%
==========================================
Files 170 170
Lines 15402 15455 +53
Branches 3604 3618 +14
==========================================
+ Hits 15012 15063 +51
- Misses 282 284 +2
Partials 108 108
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
chfast
force-pushed
the
state/tx-recover-sender
branch
from
July 27, 2026 08:30
47981b6 to
d75a382
Compare
chfast
changed the title
chfast
force-pushed
the
state/tx-recover-sender
branch
2 times, most recently
from
July 27, 2026 14:10
a3e2e0c to
21a09df
Compare
There was a problem hiding this comment.
Pull request overview
Adds sender (signer) address recovery from transaction signatures in the state-test harness, aligning transaction execution with node-style behavior rather than trusting JSON-provided senders.
Changes:
- Introduces
state::recover_sender()to recover the signer address from(v, r, s)and the transaction’s serialized bytes. - Updates the state test runner to recover sender from
txbytesand reportINVALID_SIGNATUREwhen recovery fails. - Adds unit tests covering legacy protected/unprotected
vhandling and signatures-range rejection; adds a new error code/message for invalid signatures.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/unittests/state_rlp_decode_test.cpp | Adds unit tests for sender recovery and invalid-s rejection. |
| test/statetest/statetest_runner.cpp | Uses signature-based sender recovery when txbytes is present; maps failures to INVALID_SIGNATURE. |
| test/state/transaction.hpp | Declares recover_sender() and documents signature validity expectations/limitations. |
| test/state/transaction.cpp | Implements recover_sender() by slicing the signing preimage from the original RLP bytes and calling secp256k1 recovery. |
| test/state/errors.hpp | Adds INVALID_SIGNATURE error code and message. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
chfast
force-pushed
the
state/tx-recover-sender
branch
from
July 27, 2026 22:28
21a09df to
0c7602b
Compare
chfast
force-pushed
the
state/tx-recover-sender
branch
from
July 27, 2026 22:45
0c7602b to
52c648f
Compare
chfast
force-pushed
the
state/tx-recover-sender
branch
from
August 7, 2026 11:59
85d3762 to
70787b9
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
Suppressed comments (3)
test/state/transaction.cpp:158
recover_sender()relies onassert(is_list)andassert(signature_size <= payload.size()). In release builds these checks disappear; iftxbytesdoesn’t matchtx(accidentally or via future refactors),payload.size() - signature_sizecan underflow, producing an invalid slice and potentially huge allocation/UB inside anoexceptfunction. Consider making these hard runtime checks that returnstd::nulloptwhen violated.
auto envelope = txbytes.substr(typed ? 1 : 0); // Skip the EIP-2718 type byte.
bytes_view payload;
[[maybe_unused]] const auto is_list = rlp::take_list_payload(envelope, payload);
assert(is_list); // tx has been decoded from txbytes, so its list header is valid.
// The decoder accepts only canonical integers, so re-encoding (v, r, s) gives their wire sizes.
const auto signature_size =
rlp::encode(tx.v).size() + rlp::encode(tx.r).size() + rlp::encode(tx.s).size();
assert(signature_size <= payload.size());
auto preimage = bytes{payload.substr(0, payload.size() - signature_size)};
test/state/transaction.hpp:103
- The API/doc currently hard-codes EIP-2 (low-s) and EIP-155 handling as “applied at every revision”. That makes
recover_sender()unable to model historical consensus rules (e.g. pre-Homestead allowing high-s, pre-Spurious-Dragon disallowing EIP-155 v values) and can cause state tests for early revisions to diverge if fixtures ever include such cases. Consider takingevmc_revision(or an equivalent fork indicator) and applying the signature validity rules conditionally, so the runner can truly “recover as a node does” for the selectedrev.
/// Both rules are applied at every revision, although EIP-2 (low s) starts at Homestead and
/// EIP-155 (v carrying the chain id) at Spurious Dragon. The fixtures do not notice: they are
/// signed canonically, and no transaction predating Spurious Dragon carries an EIP-155 v.
/// Replaying real pre-Homestead history would, as about half of those signatures have a high s.
[[nodiscard]] std::optional<address> recover_sender(
const Transaction& tx, bytes_view txbytes) noexcept;
test/unittests/state_rlp_decode_test.cpp:46
recover()usesEXPECT_TRUE(tx.has_value())but then unconditionally callstx.value(). If decoding fails, this will throw/terminate (the function is non-void), turning a test failure into a crash. Prefer reporting the failure and returningstd::nullopt(or otherwise short-circuiting) before callingvalue().
const auto tx = state::decode_transaction(txbytes);
EXPECT_TRUE(tx.has_value());
return state::recover_sender(tx.value(), txbytes);
chfast
force-pushed
the
state/tx-recover-sender
branch
2 times, most recently
from
August 9, 2026 11:02
70787b9 to
46a44c9
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
Suppressed comments (1)
test/unittests/state_rlp_decode_test.cpp:46
recover()usesEXPECT_TRUE(tx.has_value())and then immediately callstx.value(). If decoding ever fails, this will throwstd::bad_optional_accessand can abort the test in a non-obvious way. Prefer guarding the optional and reporting a failure before returning.
const auto tx = state::decode_transaction(txbytes);
EXPECT_TRUE(tx.has_value());
return state::recover_sender(tx.value(), txbytes);
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.
recover_sender() read the transaction's list header by hand where rlp::take_list_payload() does it -- and checks that it is a list, which the open-coded version did not. It also picked the EIP-155 or the pre-EIP-155 base to subtract from v before taking the parity; both bases are odd, so the parity of v alone decides it either way. In the runner the transaction's emptiness and the error code held the same fact, kept in sync by resetting the optional; an engaged error code now carries it alone. The JSON template the transaction starts from is built only when the test has no encoding to decode it from, which for the EEST fixtures is never. The recovery tests share the decode-then-recover helper, and the EIP-2 bound is pinned at the boundary: the largest accepted s recovers, one above it does not.
A transaction that does not decode, or whose signature does not recover, is rejected by the runner rather than by the state library, so nothing reached those two branches in CI: the EEST fixtures the coverage job runs carry no such transaction, and the ones that do (frontier/validation/ bad_v_r_s) are in a release it does not download. Add the two integration fixtures next to the invalid-nonce one, which pins the same wiring for a transaction the state library rejects. Each asserts the message the runner prints, so the error codes and their strings are covered end to end.
chfast
force-pushed
the
state/tx-recover-sender
branch
from
August 9, 2026 12:07
46a44c9 to
3bb4e58
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add procedure to do full transaction sender address recovery from the transaction signature. Wire this to the
evmone-statetestand ignore JSON"sender"field.