state: Recover sender address from transaction signature - #1615
Open
chfast wants to merge 2 commits into
Open
Conversation
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #1615 +/- ##
==========================================
- Coverage 97.46% 97.41% -0.06%
==========================================
Files 170 170
Lines 15402 15446 +44
Branches 3604 3615 +11
==========================================
+ Hits 15012 15047 +35
- Misses 282 287 +5
- Partials 108 112 +4
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
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
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.