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

💡 [REQUEST] - Replace ethers by viem for EIP712 #1567

Open
freemanzMrojo opened this issue Dec 10, 2024 · 0 comments · May be fixed by #1616
Open

💡 [REQUEST] - Replace ethers by viem for EIP712 #1567

freemanzMrojo opened this issue Dec 10, 2024 · 0 comments · May be fixed by #1616

Comments

@freemanzMrojo
Copy link
Member

freemanzMrojo commented Dec 10, 2024

Summary

After the release of abitype 1.0.7 and viem 2.21.54, we are now ready to finally replace ethers by viem in the context of EIP712.

Ideally we want to keep compatibility with users (like veworld) in this sense. This means that we should implement a mechanism to find out the primaryType from the types so users do not need to provide that value. This is how ethers does it. The reason because viem does not do it is because primaryType is part of the spec, and their ethos is to be as close as possible to it.

The steps to complete this ticket should be:

  1. Revert this commit so a revert branch is created (we will modify it in the next steps, but the vast majority of the code should remain the same).
  2. Implement the code to infer the primaryType from the types (see link above as a reference).
  3. This means that method signature wise, our SDK should remain pretty much the same so there is no disruption for users when releasing the new version containing this.
  4. Create/update tests using as domain's chainId the VeChain values (see which ones are in this comment), both as string and bigint.
  5. (OPTIONAL but recommended) Build the packages locally and use them in veworld-mobile to validate that the changes are correct before creating the PR. By running yarn test in that repo should be enough for the validation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants