MetaMask / MetaMask/core

Align keyrings `destroy` / `dispose` API

Open
#1,428 0 comments 0 reactions 0 assignees View on GitHub
Epic team-wallet-framework wf-keyring
Dominant language
TypeScript
Stars
413
Forks
308
Avg merge
1d 4h
Merged PRs (30d)
253

Description

Currently, the [type](https://github.com/MetaMask/utils/blob/main/src/keyring.ts#L47) we use for Keyrings does not include a "destroy" function. On the other hand, Ledger and Trezor keyring have a function for this and is named `dispose` for Trezor and `destroy` for Ledger.

At the moment Ledger and Trezor keyrings are not implementing the Keyring type, but would be beneficial to:

- [x] [Add the `destroy` method on Keyring type](https://github.com/MetaMask/utils/issues/107)
- [x] [Rename Trezor's `dispose` method to `destroy`](https://github.com/MetaMask/eth-trezor-keyring/issues/178)
- [ ] [Trezor keyring should implement Keyring type](https://github.com/MetaMask/eth-trezor-keyring/issues/171)
- [ ] [Ledger keyring should implement Keyring type](https://github.com/MetaMask/eth-ledger-bridge-keyring/issues/190)

See also original discussion: https://github.com/MetaMask/KeyringController/pull/233#discussion_r1228228870

Contributor guide

No contributing guide indexed for this repository

Research direction

Start with the Keyring type in src/keyring.ts at the linked definition, then review the Trezor and Ledger keyring issues referenced in the checklist. Done means both keyrings implement the TypeScript Keyring type with the aligned destroy API; confirm the linked cross-repository work before starting.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
api
Issue type
Feature
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.