-
Notifications
You must be signed in to change notification settings - Fork 653
refactor(snaps-rpc-methods)!: Use messenger for restricted method implementations #3968
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
Open
FrederikBolding
wants to merge
19
commits into
main
Choose a base branch
from
fb/restricted-methods-messenger
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
19 commits
Select commit
Hold shift + click to select a range
32f9eb9
refactor(snaps-rpc-methods): Use messenger for method implementations
FrederikBolding 5d2a8e5
Fix snaps-simulation
FrederikBolding 3b477b9
Migrate more RPC methods
FrederikBolding 4cf3f79
Use hooks via messenger for simulation framework
FrederikBolding 46058a4
Replace some of mnemonic hook code
FrederikBolding 99a08ff
Move ignore after lint
FrederikBolding ce9f550
Add back shared hooks
FrederikBolding ebd0918
Revert JSDoc change
FrederikBolding fe4534b
Address PR comments and some tests
FrederikBolding b831c40
Fix lint
FrederikBolding 00f4206
Migrate tests
FrederikBolding 17526bb
Update coverage
FrederikBolding 516e75c
Update ignore
FrederikBolding 0c3f32b
Add a couple more test cases
FrederikBolding 5aa93a9
Add cast
FrederikBolding 2a748e9
Add missing type
FrederikBolding 605c8a4
Remove dead code
FrederikBolding c0bd4e3
Fixup HD keyring type
FrederikBolding 91050f5
Update coverage
FrederikBolding File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,8 @@ | ||
| import type { | ||
| PermissionConstraint, | ||
| PermissionSpecificationConstraint, | ||
| import { selectHooks } from '@metamask/json-rpc-engine/v2'; | ||
| import { | ||
| createRestrictedMethodMessenger, | ||
| type PermissionConstraint, | ||
| type PermissionSpecificationConstraint, | ||
| } from '@metamask/permission-controller'; | ||
| import type { SnapPermissions } from '@metamask/snaps-utils'; | ||
| import { hasProperty } from '@metamask/utils'; | ||
|
|
@@ -9,11 +11,11 @@ import { | |
| endowmentCaveatMappers, | ||
| endowmentPermissionBuilders, | ||
| } from './endowments'; | ||
| import type { RestrictedMethodMessenger } from './restricted'; | ||
| import { | ||
| caveatMappers, | ||
| restrictedMethodPermissionBuilders, | ||
| } from './restricted'; | ||
| import { selectHooks } from './utils'; | ||
|
|
||
| /** | ||
| * Map initial permissions as defined in a Snap manifest to something that can | ||
|
|
@@ -64,18 +66,29 @@ export const buildSnapEndowmentSpecifications = ( | |
| export const buildSnapRestrictedMethodSpecifications = ( | ||
| excludedPermissions: string[], | ||
| hooks: Record<string, unknown>, | ||
| messenger: RestrictedMethodMessenger, | ||
| ) => | ||
| Object.values(restrictedMethodPermissionBuilders).reduce< | ||
| Record<string, PermissionSpecificationConstraint> | ||
| >((specifications, { targetName, specificationBuilder, methodHooks }) => { | ||
| if (!excludedPermissions.includes(targetName)) { | ||
| specifications[targetName] = specificationBuilder({ | ||
| // @ts-expect-error The selectHooks type is wonky | ||
| methodHooks: selectHooks<typeof hooks, keyof typeof methodHooks>( | ||
| hooks, | ||
| methodHooks, | ||
| ) as Pick<typeof hooks, keyof typeof methodHooks>, | ||
| }); | ||
| } | ||
| return specifications; | ||
| }, {}); | ||
| >( | ||
| ( | ||
| specifications, | ||
| // @ts-expect-error TypeScript is not convinced methodHooks and actionNames exist | ||
| { targetName, specificationBuilder, methodHooks, actionNames }, | ||
| ) => { | ||
| if (!excludedPermissions.includes(targetName)) { | ||
| specifications[targetName] = specificationBuilder({ | ||
| // @ts-expect-error The selectHooks type is wonky | ||
| methodHooks: selectHooks(hooks, methodHooks), | ||
| // @ts-expect-error Messenger type cannot be narrowed correctly | ||
| messenger: createRestrictedMethodMessenger({ | ||
| namespace: targetName, | ||
| rootMessenger: messenger, | ||
| actionNames, | ||
| }), | ||
| }); | ||
| } | ||
| return specifications; | ||
| }, | ||
|
Comment on lines
+74
to
+92
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Suggestions welcome for how to satisfy TypeScript here 🫠 |
||
| {}, | ||
| ); | ||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
Hmm, this doesn't seem ideal 😅. I can take a look tomorrow.