-
Notifications
You must be signed in to change notification settings - Fork 557
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
[BREAKING] Add snap_resolveInterface
RPC method
#2509
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #2509 +/- ##
==========================================
+ Coverage 93.85% 93.89% +0.03%
==========================================
Files 453 454 +1
Lines 9412 9464 +52
Branches 1436 1445 +9
==========================================
+ Hits 8834 8886 +52
Misses 578 578 ☔ View full report in Codecov by Sentry. |
b780384
to
04dd758
Compare
b097787
to
5492553
Compare
snap_resolveInterface
RPC methodsnap_resolveInterface
RPC method
packages/snaps-controllers/src/interface/SnapInterfaceController.test.tsx
Outdated
Show resolved
Hide resolved
packages/snaps-rpc-methods/src/permitted/resolveInterface.test.ts
Outdated
Show resolved
Hide resolved
@@ -45,33 +46,33 @@ const MAX_FILE_SIZE = 10_000_000; // 10 MB | |||
*/ | |||
export function getInterfaceResponse( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Out of scope for this PR, but the new dialog type should be supported here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes I'm going to open a new PR once this one is merged with some support for this new Dialog type in snaps-jest
import type { PayloadAction } from '@reduxjs/toolkit'; | ||
import { createAction, createSelector, createSlice } from '@reduxjs/toolkit'; | ||
|
||
import type { ApplicationState } from './store'; | ||
|
||
export type Interface = { | ||
type: DialogType; | ||
type: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Mrtenz This type is only used internally, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think these are internal only, I convert them to the dialog type in getInterfaceResponse
}), | ||
).rejects.toThrow( | ||
'The "type" property must be one of: alert, confirmation, prompt.', | ||
'Invalid params: Expected the value to satisfy a union of `object | object`, but received: [object Object]', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should improve this, but out of scope
Please let @Mrtenz leave a review as well if he wants to request any changes for |
3536bde
to
c33b9d3
Compare
This PR adds a new non-restricted RPC method that allows a snap to resolve a given user interface bound to a
snap_dialog
with a custom value.resolveInterface
method in theSnapInterfaceController
snap_resolveInterface
RPC methodsnap_dialog
RPC method to set the approval ID to the interface IDsnap_dialog
now takes therequestUserApproval
hook which is meant to be bind to theaddAndShowRequest
method of theApprovalController