Dispose or destroy keyrings on reference drop

[mikesposito]

Description

This PR calls the optional methods destroy and dispose on keyrings that support them (Trezor and Ledger) whenever the reference to the keyring is dropped: when locked and when the last account from a keyring is removed.

To avoid introducing one of the two (Trezor or Ledger) keyrings just to be used on tests, I extended the HDKeyring prototype to also present a destroy and a dispose method, in order to be spied on.

Changes

• CHANGED: When defined, Keyring.dispose or Keyring.destroy methods are called on any keyring reference drop

References

• Fixes #1388

Checklist

• I've updated the test suite for new or updated code as appropriate
• I've updated documentation for new or updated code as appropriate (note: this will usually be JSDoc)
• I've highlighted breaking changes using the "BREAKING" category above as appropriate

[Gudahtt]

Looks great! Left a few comments/suggestions

[Gudahtt]

Between the two names, I'm slightly more inclined to use destroy because we already use this method name for some controllers and middleware.

Perhaps we could align on this name? Not a blocker, but renaming the method on Trezor might be fairly easy (that package had a recent release)

[mikesposito]

Are you suggesting removing the dispose call, then changing the method name on Trezor Keyring after this PR has been merged, or should we do it before this PR?

[Gudahtt]

Either before or after; as long as that change gets included in core and in the clients prior to this one.

But this was a non-blocking suggestion, so whatever you'd prefer to do here is fine

[mikesposito]

@Gudahtt I created this issue to track this work

[Gudahtt]

LGTM!