Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix various mistakes in the README API documentation #157

Merged
merged 3 commits into from
Jul 15, 2021

Conversation

Gudahtt
Copy link
Member

@Gudahtt Gudahtt commented Jul 15, 2021

The README included various mistakes in the API documentation.

  • The signTypedDataLegacy function was listed as signTypedData
  • The signTypedData function was listed as signTypedData_v3
  • The recoverTypedSignatureLegacy function was listed as recoverTypedSignature
  • The recoverTypedSignature function was missing
  • The recoverTypedSignature_v4 function was listed as recoverTypedSignature_V4

Lastly, the way that signTypedData (i.e. eth_signTypedData_v3) and signTypedData_v4 are characterized has been updated. Previously the V3 function was described as fully compliant with EIP-712, whereas V4 was "an extension" of EIP-712, because of added support for arrays and recursive data structures.

But in fact, V3 complies with no version of EIP-712. There was never a version that included the \x19\x01 prefix used by V3 but not support for arrays and recursive data structures. The spec supported arrays and recursive data structures since before that prefix was added. V4 has been the version that complies with the specification.

The documentation has been updated to instead describe V4 as spec-compliant, and V3 as spec-compliant except for the lack of support for arrays and recursive data structures.

Fixes #64

@Gudahtt Gudahtt requested a review from a team as a code owner July 15, 2021 13:45
The README included various mistakes in the API documentation.

* The `signTypedDataLegacy` function was listed as `signTypedData`
* The `signTypedData` function was listed as `signTypedData_v3`
* The `recoverTypedSignatureLegacy` function was listed as
  `recoverTypedSignature`
* The `recoverTypedSignature` function was missing
* The `recoverTypedSignature_v4` function was listed as
  `recoverTypedSignature_V4`

Lastly, the way that `signTypedData` (i.e. `eth_signTypedData_v3`) and
`signTypedData_v4` are characterized has been updated. Previously the
V3 function was described as fully compliant with EIP-712, whereas V4
was "an extension" of EIP-712, because of added support for arrays and
recursive data structures.

But in fact, V3 complies with no version of EIP-712. There was never a
version that included the `\x19\x01` prefix [1] used by V3 but not
support or arrays and recursive data structures. The spec supported
arrays and recursive data structures since before that prefix was
added. V4 has been the version that complies with the specification.

The documentation has been updated to instead describe V4 as spec-
compliant, and V3 as spec-compliant except for the lack of support for
arrays and recursive data structures.

Fixes #64
Copy link
Member

@rekmarks rekmarks left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@Gudahtt Gudahtt merged commit 5b44ece into main Jul 15, 2021
@Gudahtt Gudahtt deleted the fix-README-mistakes branch July 15, 2021 16:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

signTypedData_v3 in Readme but not in code
2 participants