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: Add @metamask/wallet package [WIP] #4105

Closed
wants to merge 1 commit into from
Closed

Conversation

Gudahtt
Copy link
Member

@Gudahtt Gudahtt commented Mar 25, 2024

Explanation

The @metamask/wallet package is a library that is intended for building MetaMask wallets. It will provide an API for all functionality supported across all MetaMask wallts, minus the UI.

The first release of this package will not support all MetaMask wallet functionality. The initial version will support keyrings, accounts, approvals, preferences, network access, fiat currency rates, gas estimates, and transactions. We will add additional features one-by-one until this library includes all common functionality.

This MVP has not yet been implemented. I've written a type declaration and some non-functional unit tests to demonstrate roughly what the API would look like, to get early feedback. This will be written up as an architecture decision record before proceeding with the implementation.

References

Relates to https://github.com/MetaMask/MetaMask-planning/issues/1861

Changelog

@metamask/wallet

Added

  • Initial release

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

Copy link

socket-security bot commented Mar 25, 2024

New dependencies detected. Learn more about Socket for GitHub ↗︎

Package New capabilities Transitives Size Publisher
npm/@metamask/[email protected] None 0 27 kB metamaskbot
npm/@metamask/[email protected] Transitive: network +2 945 kB metamaskbot
npm/[email protected] None 0 68.8 MB typescript-bot

View full report↗︎

The `@metamask/wallet` package is a library that is intended for
building MetaMask wallets. It will provide an API for all functionality
supported across all MetaMask wallts, minus the UI.

The first release of this package will not support all MetaMask wallet
functionality. The initial version will support keyrings, accounts,
approvals, preferences, network access, fiat currency rates, gas
estimates, and transactions. We will add additional features one-by-one
until this library includes all common functionality.

This MVP has not yet been implemented. I've written a type declaration
and some non-functional unit tests to demonstrate roughly what the API
would look like, to get early feedback. This will be written up as an
architecture decision record before proceeding with the implementation.
@Gudahtt Gudahtt force-pushed the wallet-library-poc branch from 96f5f5e to 17a17e8 Compare March 25, 2024 20:05

// Additional jotnotes:
//
// Step0: Rename controller messenger, write ADR about services and selectors, refactor guts of ComposableController into compose function
Copy link
Member Author

@Gudahtt Gudahtt Mar 25, 2024

Choose a reason for hiding this comment

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

refactor guts of ComposableController into compose function

I haven't yet demonstrated in this PR how we'd combine the state of this wallet library with additional controllers. But my idea was to extract the internals of ComposableController into a function, which could be used both internally in the wallet library, and externally by the client to compose additional controller states.

* MetaMask wallet state.
*/
export type MetamaskState<
ControllerPermissionSpecification extends PermissionSpecificationConstraint &
Copy link
Member Author

Choose a reason for hiding this comment

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

Making the permissions extensible ends up being quite verbose. Surely there's a more concise way of doing this, but I haven't experimented with it much yet.

subjectType: SubjectType;
}): JsonRpcEngine;

// TODO: Add start/resume, pause, stop methods to control all polling/services
Copy link
Member Author

Choose a reason for hiding this comment

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

Establishing a standard approach for polling might be necessary for this.

The services part is easy, we can add an enableServices/disableServices method to handle that. But I am not sure on polling. Currently we don't even use actions for it, we use methods.

Probably we'll want to standardize some add/remove polling token actions, then add a method to pause/unpause all polling globally.

@Gudahtt
Copy link
Member Author

Gudahtt commented Mar 25, 2024

I haven't come up with a proper architecture diagram yet, but here is a rough sketch:

WalletLibrary

Note that I included snaps here to demonstrate how it would eventually be integrated. It's not included yet in this MVP.

> {
#controllerMessenger: ControllerMessenger<AllWalletActions, AllWalletEvents>;

#controllers: {
Copy link
Member Author

Choose a reason for hiding this comment

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

Here is the initial set of controllers that I'll be proposing for the MVP.

I wanted enough functionality to demonstrate that the JSON-RPC pipeline works and is useful, so the NetworkController was a must. I brought in the TransactionController as well because I wanted at least one intercepted method, and it seemed easier to bring that in than the SignatureController (because it's already on BaseControllerV2, but the message managers are not).

And the GasFeeController was brought in because it's a peer dependency of the TransactionController. Same for the ApprovalController. Though in both cases there are other reasons to include them as well: the GasFeeController is a nice example of a polling controller, and the ApprovalController establishes how we handle user confirmations.

CurrencyRateController was the one asset controller that seemed easy to bring on its own, and it lets us demonstrate more complex wallet API interactions down the line (e.g. when we go to write an integration test to approximate what the UI might want to render a confirmation).

AccountsController and PreferencesController were brought in for the identity state, which is still in both places at the moment. Even after it's removed from preferences, wallet preferences is nice to have on hand. Later I'll be suggesting that we audit the preferences to ensure they're all relevant to the wallet library, moving anything else to a MobilePreferencesController.

Lastly, KeyringController and PermissionController need to be included, they're fundamental to what the wallet is.

Copy link
Member Author

Choose a reason for hiding this comment

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

I was tempted to include snaps even in this early draft, but that seemed pretty complicated to integrate. And it's not completely landed on mobile yet.

Plus, snaps provides a nice demonstration of why we need extensible permissions.

Copy link
Contributor

@mcmire mcmire left a comment

Choose a reason for hiding this comment

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

I've looked this over and it matches more or less what I was picturing in my mind. Assuming we are able to keep this simple API, then this looks really great. Anxious to see the tests passing, but they are simple to understand at least as well. Looks good!

(I know this is a draft — the approval is just meant to cover the fact that the POC nature of this draft looks good. If it gets fleshed out, then I'll re-review with those new changes in mind.)

@Gudahtt
Copy link
Member Author

Gudahtt commented Jun 24, 2024

Superseded by #4451

@Gudahtt Gudahtt closed this Jun 24, 2024
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.

2 participants