Migrate PermissionController and SubjectMetadataController #692
Conversation
FrederikBolding
changed the title
src/util.ts
Outdated
| req: JsonRpcRequest<U>, | ||
| res: PendingJsonRpcResponse<V>, | ||
| next: JsonRpcEngineNextCallback, | ||
| end: JsonRpcEngineEndCallback, | ||
| hooks: T, | ||
| ) => void | Promise<void>; | ||
|
|
||
| type BaseHandlerExport = { | ||
| methodNames: string[]; | ||
| }; | ||
|
|
||
| /** | ||
| * We use a mapped object type in order to create a type that requires the | ||
| * presence of the names of all hooks for the given handler. | ||
| * This can then be used to select only the necessary hooks whenever a method | ||
| * is called for purposes of POLA. | ||
| */ | ||
| export type HookNames<T> = { | ||
| [Property in keyof T]: true; | ||
| }; | ||
|
|
||
| export type PermittedHandlerExport<T, U, V> = { | ||
| implementation: HandlerMiddlewareFunction<T, U, V>; | ||
| hookNames: HookNames<T>; | ||
| } & BaseHandlerExport; |
There was a problem hiding this comment.
Let's import this from @metamask/rpc-methods instead.
There was a problem hiding this comment.
I think that might be problematic, as we might want to reference @metamask/controllers types in the RPC methods at some point. For example, when connecting the Snap RPC method to the new notification controller 😄
There was a problem hiding this comment.
It'll be a circular dependency then, right? 🤔
There was a problem hiding this comment.
We already have a circular dependency between @metamask/rpc-methods and the permission controller in @metamask/snap-controllers, so this wouldn't be a new problem at least. But it doesn't seem ideal. Maybe better to move this type from @metamask/rpc-methods to the @metamask/types package, and reference it in both places?
There was a problem hiding this comment.
There was a problem hiding this comment.
This PR should be good to go once we import those types @metamask/types.
FrederikBolding
force-pushed
the
fb/migrate-permission-controller
branch
from
41a8c2c
to
8d64247
Compare
* Initial copy-paste * Fix a bunch of imports etc * Fix some typing issues * Fix another type issue * Fix linting issues + add missing doc strings * Run Prettier on README * Fix some test typing issues + ignore others * Revert a recent change * Remove unused function * Remove remains of bold formatting * Fix usage of messengers to improve test typing * Fix some issues with JSDocs * Add util function tests * Use @metamask/types for a bunch of shared types * Fix a few import issues * Remove unnecessary ts-expect-error comments
* Initial copy-paste * Fix a bunch of imports etc * Fix some typing issues * Fix another type issue * Fix linting issues + add missing doc strings * Run Prettier on README * Fix some test typing issues + ignore others * Revert a recent change * Remove unused function * Remove remains of bold formatting * Fix usage of messengers to improve test typing * Fix some issues with JSDocs * Add util function tests * Use @metamask/types for a bunch of shared types * Fix a few import issues * Remove unnecessary ts-expect-error comments
Migrated
PermissionsControllerandSubjectMetadataControllerfromsnaps-skunkworks.Description
PermissionsControllerandSubjectMetadataControllerWould probably recommend looking through some of the history when reviewing this. The very first commit is a 1:1 copy paste of the files from
snaps-skunkworks. The following commits are the changes made to satisfy linting, making it build and have tests run successfully.Checklist
Issue
Resolves: MetaMask/snaps#197