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: integrating signature decoding api #4855

Merged
merged 56 commits into from
Nov 7, 2024
Merged

feat: integrating signature decoding api #4855

merged 56 commits into from
Nov 7, 2024

Conversation

jpuri
Copy link
Contributor

@jpuri jpuri commented Oct 28, 2024

Explanation

Integrate signature decoding api in signature-controller. When a permit request is received this api is called and the result is asynchronously updated into signature request.

References

Changelog

@metamask/signature-controller

  • Added: For Permit signatures invoke signature decoding api and update the signature with the response.

Checklist

  • I've updated the test suite for new or updated code as appropriate
  • I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate
  • I've highlighted breaking changes using the "BREAKING" category above as appropriate
  • I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes

@jpuri jpuri marked this pull request as ready for review October 29, 2024 11:42
@jpuri jpuri requested a review from a team as a code owner October 29, 2024 11:42
packages/signature-controller/src/SignatureController.ts Outdated Show resolved Hide resolved
packages/signature-controller/src/SignatureController.ts Outdated Show resolved Hide resolved
packages/signature-controller/src/SignatureController.ts Outdated Show resolved Hide resolved
packages/signature-controller/src/SignatureController.ts Outdated Show resolved Hide resolved
packages/signature-controller/src/SignatureController.ts Outdated Show resolved Hide resolved
packages/signature-controller/src/types.ts Outdated Show resolved Hide resolved
packages/signature-controller/src/types.ts Outdated Show resolved Hide resolved
packages/signature-controller/src/types.ts Outdated Show resolved Hide resolved
@jpuri jpuri requested a review from a team as a code owner October 30, 2024 11:07
@jpuri jpuri requested a review from a team as a code owner October 30, 2024 12:59
@jpuri jpuri requested a review from matthewwalsh0 November 4, 2024 10:10
packages/signature-controller/src/utils/decoding-api.ts Outdated Show resolved Hide resolved
packages/signature-controller/src/types.ts Outdated Show resolved Hide resolved
packages/signature-controller/src/types.ts Outdated Show resolved Hide resolved
@jpuri jpuri requested a review from matthewwalsh0 November 6, 2024 04:07
Copy link
Member

@matthewwalsh0 matthewwalsh0 left a comment

Choose a reason for hiding this comment

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

Happy to approve once the keyring-api dependency is removed.

@@ -0,0 +1,8 @@
export const EthMethod = {
Copy link
Member

Choose a reason for hiding this comment

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

Minor, this feels more like an enum as it's a fixed set of values to identify a type of something.

Which would also mean it could go in types.ts.

But we can do this later since it's not exported.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried that but it is giving build issues somehow.

packages/signature-controller/package.json Outdated Show resolved Hide resolved
@jpuri jpuri requested a review from matthewwalsh0 November 7, 2024 09:21
@jpuri jpuri merged commit 596a96a into main Nov 7, 2024
120 checks passed
@jpuri jpuri deleted the sign_api branch November 7, 2024 10:23
@jpuri jpuri mentioned this pull request Nov 8, 2024
4 tasks
jpuri added a commit that referenced this pull request Nov 8, 2024
## Explanation

Signature controller update that add decoding api integration.

## References

* Related to
[#67890](MetaMask/MetaMask-planning#3553)

## Changelog

### `@metamask/signature-controller`

- **ADDED**: Add isDecodeSignatureRequestEnabled to SignatureController
constructor to be able to check if decoding api should be invoked for
the signature ([#4903](#4903))
- **ADDED**: Integrating signature decoding api
([#4855](#4855))


## Checklist

- [X] I've updated the test suite for new or updated code as appropriate
- [X] I've updated documentation (JSDoc, Markdown, etc.) for new or
updated code as appropriate
- [X] I've highlighted breaking changes using the "BREAKING" category
above as appropriate
- [X] I've prepared draft pull requests for clients and consumer
packages to resolve any breaking changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants