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

feat: eip-1271 #320

Merged
merged 19 commits into from
May 9, 2023
Merged

feat: eip-1271 #320

merged 19 commits into from
May 9, 2023

Conversation

janek26
Copy link
Contributor

@janek26 janek26 commented Apr 6, 2023

Implements 2 new public actions:

  • verifyMessage
  • verifyTypedData

which have the same interface as their utils counterparts, but also support EIP-1271

Added some hints in the documentation (website and JSDoc) for developers which method they want to choose and why.

🤖 Generated by Copilot at 0f87d3d

This pull request adds two new public actions, verifyMessage and verifyTypedData, to the public client and the public actions module. It also adds the documentation for these actions in the site/docs folder and updates the sidebar accordingly. Additionally, it updates the tests and the constants for the public actions, and imports the smart account contract abi for testing ERC-1271 verification.


PR-Codex overview

This PR adds support for Contract Wallet signature verification (EIP-6492) via publicClient.verifyMessage & publicClient.verifyTypedData.

Detailed summary

  • Added support for Contract Wallet signature verification (EIP-6492) via publicClient.verifyMessage & publicClient.verifyTypedData
  • Added smartAccountAbi and universalSignatureValidatorAbi to abis.ts
  • Added verifyMessage and verifyTypedData functions to verifyMessage.ts and verifyTypedData.ts, respectively
  • Updated createPublicClient.test.ts and public.ts to include verifyMessage and verifyTypedData in PublicActions type
  • Added tests for verifyMessage and verifyTypedData in verifyMessage.test.ts and verifyTypedData.test.ts, respectively.

The following files were skipped due to too many changes: src/actions/public/verifyMessage.ts, src/clients/decorators/public.test.ts, src/actions/public/verifyTypedData.ts, src/actions/public/verifyHash.ts, contracts/src/DeploylessUniversalSigValidator.sol, site/docs/utilities/verifyTypedData.md, site/docs/actions/public/verifyMessage.md, site/docs/utilities/verifyMessage.md, src/constants/contracts.ts, src/actions/public/verifyHash.test.ts, site/docs/actions/public/verifyTypedData.md

✨ Ask PR-Codex anything about this PR by commenting with /codex {your question}

@changeset-bot
Copy link

changeset-bot bot commented Apr 6, 2023

🦋 Changeset detected

Latest commit: 1555156

The changes in this PR will be included in the next version bump.

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

@vercel
Copy link

vercel bot commented Apr 6, 2023

@janek26 is attempting to deploy a commit to the wagmi Team on Vercel.

A member of the Team first needs to authorize it.

param and return type are the same between the utils method and the public action
@janek26 janek26 force-pushed the feature/eip-1271 branch from 36db3f0 to 7fc8e0e Compare April 6, 2023 09:19
Copy link
Collaborator

@fubhy fubhy left a comment

Choose a reason for hiding this comment

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

This looks really solid! Nice work.

src/clients/decorators/public.ts Show resolved Hide resolved
Copy link
Member

@jxom jxom left a comment

Choose a reason for hiding this comment

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

Looks pretty good so far! Haven't looked at this too much in-depth yet, and will properly review this after the concerns I raised above have been addressed!

site/docs/actions/public/verifyMessage.md Outdated Show resolved Hide resolved
@tmm tmm changed the title Feature/eip-1271 feat: eip-1271 Apr 6, 2023
@pr-codex
Copy link

pr-codex bot commented Apr 7, 2023

Tldr

This PR adds two new methods, verifyMessage and verifyTypedData, which allow for verifying the validity of signed messages and typed data signatures respectively.

Detailed summary

  • A new file verifyMessage.md has been added to the actions/public directory.
  • A new file verifyTypedData.md has been added to the actions/public directory.
  • The sidebar.ts file has been updated to include links to the new methods.
  • The new methods allow for verifying the validity of signed messages and typed data signatures respectively.
  • The new methods support ERC-1271 and are therefore able to support more types of accounts.

Copy link
Contributor

@roninjin10 roninjin10 left a comment

Choose a reason for hiding this comment

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

I'm really excited about this pr thank you so much for doing this!

@janek26
Copy link
Contributor Author

janek26 commented Apr 7, 2023

Hey all! Just letting you know I’ll have a busy weekend and get back to you all after eastern! Enjoy everyone 🐰🥚

@jxom jxom force-pushed the main branch 3 times, most recently from c67a3b1 to 19c1e4e Compare April 12, 2023 01:25
@tmm tmm force-pushed the main branch 2 times, most recently from b54ae19 to 841c885 Compare April 13, 2023 16:54
@janek26 janek26 requested a review from jxom May 6, 2023 06:21
@janek26
Copy link
Contributor Author

janek26 commented May 6, 2023

updated the PR

It's now using a deployless universal verifier contract, so it's sending 1 eth_call request and gets back a result for any kind of account (ERC-6492 -> ERC-1271 -> EOA in this order).
Shoutout to @AmbireTech for the awesome ERC and the example implementation.

imo this is ready for reviews/merge 🙏

@codecov
Copy link

codecov bot commented May 6, 2023

Codecov Report

Merging #320 (660275e) into main (82414dc) will decrease coverage by 4.65%.
The diff coverage is 95.27%.

❗ Current head 660275e differs from pull request most recent head ad3ea08. Consider uploading reports for the commit ad3ea08 to get more accurate results

@@            Coverage Diff             @@
##             main     #320      +/-   ##
==========================================
- Coverage   99.92%   95.27%   -4.65%     
==========================================
  Files         254      254              
  Lines       23688    23635      -53     
  Branches     1967     1669     -298     
==========================================
- Hits        23670    22519    -1151     
- Misses         18     1111    +1093     
- Partials        0        5       +5     
Impacted Files Coverage Δ
src/actions/public/verifyMessage.ts 71.42% <71.42%> (ø)
src/actions/public/verifyHash.ts 100.00% <100.00%> (ø)
src/actions/public/verifyTypedData.ts 100.00% <100.00%> (ø)
src/clients/decorators/public.ts 99.92% <100.00%> (-0.08%) ⬇️
src/constants/abis.ts 99.20% <100.00%> (-0.80%) ⬇️
src/constants/contracts.ts 100.00% <100.00%> (ø)
src/utils/data/isBytesEqual.ts 100.00% <100.00%> (ø)
src/utils/signature/verifyMessage.ts 100.00% <100.00%> (ø)
src/utils/signature/verifyTypedData.ts 60.65% <100.00%> (-39.35%) ⬇️

... and 48 files with indirect coverage changes

@vercel
Copy link

vercel bot commented May 8, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
viem ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 8, 2023 2:19am

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.

5 participants