fix: normalize signature v value for proper recovery #250
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.
Summary
Ledger devices may return signature
vas 0 or 1 (modern format), but Ethereum'secrecoverexpects 27 or 28 (Yellow Paper §F). This causes"The signature doesnt match the right address"errors insignPersonalMessageandsignTypedData.Fix
Why This Wasn't Fixed Before
Original bug (pre-2022): The code subtracted 27 from
v:This converted correct values (27/28) into incorrect ones (0/1).
PR #152 (August 2022): Changed to pass through the raw value:
This fixed the case where Ledger returns 27/28, but assumed Ledger always returns 27/28.
The gap: Ledger devices may return 0/1 depending on firmware, signing method, or device model. PR #152 didn't normalize these values.
Issue #10850 was closed prematurely in January 2024 with "I expect this was resolved in PR #152" — but PR #152 only fixed one direction of the problem.
References
This is a well-known issue with a standard fix across the ecosystem:
v = v === 0 || v === 1 ? v + 27 : vif (v <= 1) v += 27Note
Medium Risk
Touches signature construction/verification in
signPersonalMessageandsignTypedData, which can affect whether Ledger-signed messages are accepted or rejected. Change is small and well-covered by new tests, but it impacts crypto-critical signing paths.Overview
Fixes Ledger message signing compatibility by normalizing returned
vvalues: when the device returns0/1, the keyring now converts them to27/28before building the0x{r}{s}{v}signature insignPersonalMessageandsignTypedData.Updates tests to cover
vnormalization for0/1and to ensure27/28are preserved, adjusting expected signature suffixes and validating address recovery behavior.Written by Cursor Bugbot for commit c4b59ee. This will update automatically on new commits. Configure here.