Consolidate signTypedData and recoverTypedSignature functions
#156
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
Closed
Gudahtt
force-pushed
the
consolidate-sign-typed-data-methods
branch
from
2d85fbf
to
99816a7
Compare
The `signTypedData` and `recoverTypedSignature` functions each had three different implementations (one for each version of EIP-712). They have each been consolidated into a single function each. The functions `signTypedData` and `recoverTypedSignature` used to be specifically for `eth_signTypedData_v3`. Instead they now accept an additional `version` parameter that specifies which version should be used. This replaces `signTypedDataLegacy` and `recoverTypedSignatureLegacy`, which were for `eth_signTypedData_v1`, and it replaces `signTypedData_v4` and `recoverTypedSignature_v4`. The functions `signTypedMessage` and `recoverTypedMessage` used to provide a single interface for using any of EIP-712 types. But now that the base functions serve that purpose instead, they are now obsolete and have been removed. Additionally a few `TypedDataUtils` functions were updated to accept a `version` parameter instead of a `useV4` parameter, to bring them in-line with the other functions. The functions changed are `hashStruct`, `eip712Hash`, and `encodeData`. The README and inline documentation has been updated accordingly. New doc strings were added for `signTypedData` and `recoverTypedSignature`, which didn't have any yet before now. Credit to @aakilfernandes for this idea, who first implemented this in PR #66.
Gudahtt
force-pushed
the
consolidate-sign-typed-data-methods
branch
from
99816a7
to
4d8324d
Compare
shanejonas
reviewed
Jul 22, 2021
| ### signTypedData (privateKeyBuffer, msgParams) | ||
| | Version | Description | | ||
| | ------- | ------------------------------------------------------------ | | ||
| | `V1` | This is based on [an early version of EIP-712](https://github.com/ethereum/EIPs/pull/712/commits/21abe254fe0452d8583d5b132b1d7be87c0439ca) that lacked some later security improvements, and should generally be neglected in favor of `V3`. | |
There was a problem hiding this comment.
should we add a note about the lack of V2?
There was a problem hiding this comment.
Probably, yep! I just didn't know what to say - I have no idea why there isn't a V2
There was a problem hiding this comment.
Oh, actually, I just found more information on this in our docs:
The missing
v2represents an intermediary design that was implemented by the Cipher browser, so that we have room to implement it if there is ever enough developer demand for it.
I'll add this in a follow-up PR.
shanejonas
approved these changes
Jul 22, 2021
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.
The
signTypedDataandrecoverTypedSignaturefunctions each had three different implementations (one for each version of EIP-712). They have each been consolidated into a single function each.The functions
signTypedDataandrecoverTypedSignatureused to be specifically foreth_signTypedData_v3. Instead they now accept an additionalversionparameter that specifies which version should be used. This replacessignTypedDataLegacyandrecoverTypedSignatureLegacy, which were foreth_signTypedData_v1, and it replacessignTypedData_v4andrecoverTypedSignature_v4.The functions
signTypedMessageandrecoverTypedMessageused to provide a single interface for using any of EIP-712 types. But now that the base functions serve that purpose instead, they are now obsolete and have been removed.Additionally a few
TypedDataUtilsfunctions were updated to accept aversionparameter instead of auseV4parameter, to bring them in-line with the other functions. The functions changed arehashStruct,eip712Hash, andencodeData.The README and inline documentation has been updated accordingly. New doc strings were added for
signTypedDataandrecoverTypedSignature, which didn't have any yet before now.Credit to @aakilfernandes for this idea, who first implemented this in PR #66.