state: Recover sender address from transaction signature - #1615
Open
chfast wants to merge 3 commits into
Open
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #1615 +/- ##
==========================================
+ Coverage 97.46% 97.48% +0.02%
==========================================
Files 170 170
Lines 15402 15451 +49
Branches 3604 3616 +12
==========================================
+ Hits 15012 15063 +51
Misses 282 282
+ Partials 108 106 -2
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.
Comment on lines
+151
to
+162
| const auto typed = tx.type != Transaction::Type::legacy; | ||
| auto payload = txbytes.substr(typed ? 1 : 0); // Skip the EIP-2718 type byte. | ||
|
|
||
| rlp::Header header; | ||
| [[maybe_unused]] const auto header_decoded = rlp::decode_header(payload, header); | ||
| assert(header_decoded); // tx has been decoded from txbytes, so its list header is valid. | ||
|
|
||
| // The signature fields are canonically encoded, so their sizes locate the slice boundary. | ||
| const auto signature_size = | ||
| rlp::encode(tx.v).size() + rlp::encode(tx.r).size() + rlp::encode(tx.s).size(); | ||
| assert(signature_size <= header.payload_length); | ||
| auto preimage = bytes{payload.substr(0, header.payload_length - signature_size)}; |
Comment on lines
+91
to
+101
| /// Recovers the sender (the signer) of the transaction @p tx decoded from @p txbytes, | ||
| /// std::nullopt if the signature is invalid: r or s outside [1, secp256k1n), or s in the upper | ||
| /// half (EIP-2). | ||
| /// | ||
| /// The serialization is needed as well because the signing preimage is a slice of it; @p tx must | ||
| /// be what decode_transaction(@p txbytes) returned. | ||
| /// TODO: The rules are applied at every revision, but EIP-2 (low s) starts at Homestead and | ||
| /// EIP-155 (v carrying the chain id) at Spurious Dragon, so a pre-Homestead transaction with a | ||
| /// high s is rejected here and a pre-EIP-155 one with v >= 35 is accepted. | ||
| [[nodiscard]] std::optional<address> recover_sender( | ||
| const Transaction& tx, bytes_view txbytes) noexcept; |
The state test runner still took the sender from the fixture's template, so a signature was only ever checked for shape. The 39 remaining frontier/validation/bad_v_r_s cases of execution-specs tests@v20.0.1 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.
chfast
force-pushed
the
state/tx-recover-sender
branch
from
July 27, 2026 22:28
21a09df to
0c7602b
Compare
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 two recovery tests share the decode-then-recover helper.
chfast
force-pushed
the
state/tx-recover-sender
branch
from
July 27, 2026 22:45
0c7602b to
52c648f
Compare
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 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);
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.
No description provided.