bugfix: eip-191 hash calculation fix #2248
Merged
Conversation
This file contains 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
aramikm
commented
Jan 3, 2025
Comment on lines
-284
to
-287
| // frequency wrapped for Metamask compatibility | ||
| let frequency_wrapped = | ||
| eth_message_hash(&format!("<Frequency>0x{:?}</Frequency>", HexDisplay::from(&msg.get()))); | ||
| verify_signature(&signature.as_ref(), &frequency_wrapped, signer) |
There was a problem hiding this comment.
Removing this custom wrapping for now. We might not need it and we will cross that bridge in future in case it was necessary.
aramikm
commented
Jan 3, 2025
| let prefixed = format!("{}{}{}", "\x19Ethereum Signed Message:\n", message.len(), message); | ||
| log::debug!(target:"ETHEREUM", "prefixed {:?}",prefixed); | ||
| sp_io::hashing::keccak_256(prefixed.as_bytes()) | ||
| fn eth_message_hash(message: &[u8]) -> [u8; 32] { |
There was a problem hiding this comment.
aramikm
requested review from
shannonwells,
mattheworris,
enddynayn,
saraswatpuneet and
JoeCap08055
Codecov ReportAll modified and coverable lines are covered by tests ✅
|
github-actions
bot
added
the
metadata-changed
shannonwells
approved these changes
Jan 3, 2025
mattheworris
approved these changes
Jan 3, 2025
saraswatpuneet
approved these changes
Jan 3, 2025
JoeCap08055
approved these changes
Jan 3, 2025
aramikm
commented
Jan 3, 2025
| let payload = from_hex("0x0a0300e659a7a1628cdd93febc04a4e0646ea20e9f5f0ce097d9a05290d4a9e054df4e028c7d0a3500000000830000000100000026c1147602cf6557f4e0068a78cd4b22b6f6b03e106d05618cde8537e4ffe454c1f285c69f563934857a63463571d57723fbad6ac7de44611ed674f02c04c2ae00").expect("Should convert"); | ||
| let signature_raw = from_hex("0x056ca64d31251a1f20733ce2a741e2963c87a9674a35f8619b6b97210ae8c8b54c2853da1b943dd95ac3b893b37f69ca7dc38c13f8ec92a235b00ae03426505900").expect("Should convert"); | ||
| let payload = b"test eip-191 message payload"; | ||
| let signature_raw = from_hex("0x276dcc9c69da24dd8441ba3acc9b60d8aae0cb39f0bc5ad92c723a31bf11575031d860978280191a0a97a1f74336ca0c79a8b1b3aab013fb58a27f113b73b2081b").expect("Should convert"); |
There was a problem hiding this comment.
this signature is generated using metamask and the payload above
aramikm
commented
Jan 3, 2025
| let unified_signature = UnifiedSignature::from(ecdsa::Signature::from_raw( | ||
| signature_raw.try_into().expect("should convert"), | ||
| )); | ||
|
|
||
| let public_key = ecdsa::Public::from_raw( | ||
| from_hex("0x025b107c7f38d5ac7d618e626f9fa57eec683adf373b1352cd20e5e5c684747079") | ||
| from_hex("0x03be5b145e12c5fb95151374ed919eb445ade57637d729dd2d73bf161d4bc10329") |
There was a problem hiding this comment.
compressed public key associated with the account used in metamask for signing 0x6906405649a9c73f28BB3D9bC3B647280D822025
|
@shannonwells added some more comments in PR. The example test is also exactly what saas team was submitting to us and added that to ensure we are compatible with them |
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.
Goal
The goal of this PR is to make sure the hash calculation for eip-191 is compatible with JS libraries.
Closes #2247
Discussion
Checklist