Move ECDSA message hash methods to its own MessageHashUtils library
#4430
Conversation
🦋 Changeset detectedLatest commit: d976a6f The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Note that the Some other options could be:
|
There was a problem hiding this comment.
Files to this file doesn't feel right. Can you double check ?
There was a problem hiding this comment.
I merge master with:
git merge master -X theirsThen applied the patch with bash scripts/upgradeable/patch-apply.sh, solved conflicts (ECDSA vs MessageHashUtils import in EIP-712), and saved the patch with bash scripts/upgradeable/patch-save.sh.
Should be fine but I did the same conflict resolution before, let me know if I'm missing something.
Co-authored-by: Hadrien Croubois <hadrien.croubois@gmail.com>
Co-authored-by: Francisco <fg@frang.io>
Co-authored-by: Francisco <fg@frang.io>
There was a problem hiding this comment.
The patch looks good to me. The changes I see are to commit hashes and line numbers, as well as context that has changed.
| * hash signed when using the https://eth.wiki/json-rpc/API#eth_sign[`eth_sign`] JSON-RPC method. | ||
| * | ||
| * NOTE: The `hash` parameter is intended to be the result of hashing a raw message with | ||
| * keccak256, althoguh any bytes32 value can be safely used because the final digest will |
There was a problem hiding this comment.
"althoguh" -> typo; I think for such a fix no need for a PR overhead and one of you maintainers can quickly fix it in main ;-): cc: @ernestognw, @frangio, @Amxx
As part of #4318, we agreed that the
ECDSAlibrary can just focus onrecoverymethods, whereas a new library for ECDSA message hashes would be a suitable place for adding support to new message schemes.It includes some updates to the documentation.
PR Checklist
npx changeset add)