fix(cli): append v byte to EVM signMessage output - #188
Conversation
The CLI was returning 64-byte signatures (r + s only) instead of the expected 65-byte format (r + s + v). The recovery ID was available in the recoveryId field but not appended to the signature hex. Fixes open-wallet-standard#171
|
@teyrebaz33 is attempting to deploy a commit to the MoonPay Team on Vercel. A member of the Team first needs to authorize it. |
|
Thanks for digging into the message-signing path. I think this one introduces a regression though. EvmSigner::sign_message already sets the recovery byte before it returns: it writes the v value into signature[64] and stores the matching recovery_id as 27 or 28. This PR appends another v byte on top of that, so the result comes out as r || s || v || v at 66 bytes instead of the standard 65-byte r || s || v. That breaks EIP-191 signatures for anything that verifies them. Could you drop the extra append so the signature stays 65 bytes, and add a regression test against one of the existing EVM message vectors that asserts the length is 65 and v is 27 or 28? That'll lock the behavior in and make sure we don't slip back into a double byte. Happy to re-review once that's in. |
Bug
ows sign message --chain evmreturns 64-byte signatures (r + s) instead of the expected 65 bytes (r + s + v), causingecrecoverfailures in ERC-8128 and EIP-191 verification flows.Fixes #171
Root cause
In
sign_message.rs, the owner mode path encoded onlyoutput.signature(64 bytes) without appending the recovery ID as thevbyte. TherecoveryIdfield was correct but not included in the hex output.Fix
Append
v(normalized to 27/28) to the signature bytes before hex-encoding:Notes