feat: update earncontroller initialisation for bip-44#19160
Conversation
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
d9d2d8b to
631584b
Compare
63c6220 to
fdaaa0f
Compare
nickewansmith
left a comment
There was a problem hiding this comment.
Pre-approving, will approve again after core code is merged
## Explanation To remove our dependency on AccountsController and use the new BIP-44 enabled AccountTreeController, this PR contains two main changes: - Swaps out the listener `AccountsController:selectedAccountChange` for `AccountTreeController:selectedAccountGroupChange` - Swaps out the action `AccountsController:getSelectedAccount` for `AccountTreeController:getAccountsFromSelectedAccountGroup` from which we derive the EVM-compatible account This relies on the latest 0.12.1 release of the account-tree-controller package. Small things of note: - The account listener no longer needs the account payload to update the account address as the race condition of concern [has been fixed](#5555) (this is relevant to mention still because despite using the new AccountTreeController event, it still uses AccountsController under the hood) - In the new `getSelectedEvmAccount()` function I initially handled the case of no account being found (which shouldn't be possible) by failing early with a 'No EVM-compatible account address found' error. But on seeing how this case is handled by consuming functions (sometimes returning early, sometimes returning empty objects, sometimes throwing error) I opted to keep it consistent with the current code ## References Fixes the core side of this issue: https://consensyssoftware.atlassian.net/browse/TAT-1315?atlOrigin=eyJpIjoiMmQ2MDY1ZjQ4MDU5NDdiYmJhMjRhYzNiMThhMjEwYzIiLCJwIjoiaiJ9 And should be tested alongside the accompanying mobile repo update: MetaMask/metamask-mobile#19160 ## Checklist - [x] I've updated the test suite for new or updated code as appropriate - [x] I've updated documentation (JSDoc, Markdown, etc.) for new or updated code as appropriate - [x] I've communicated my changes to consumers by [updating changelogs for packages I've changed](https://github.com/MetaMask/core/tree/main/docs/contributing.md#updating-changelogs), highlighting breaking changes as necessary - [x] I've prepared draft pull requests for clients and consumer packages to resolve any breaking changes
|
| "@ledgerhq/react-native-hw-transport-ble": "^6.34.1", | ||
| "@metamask/account-api": "^0.9.0", | ||
| "@metamask/account-tree-controller": "^0.8.0", | ||
| "@metamask/account-tree-controller": "^0.12.1", |
There was a problem hiding this comment.
Note for other reviewers: Bumping to this dep normally require some other peer dep updates (like for the accounts-controller). We (the accounts team) will update those peer deps in a later PR.
There was a problem hiding this comment.
Thanks for bringing this up @ccharly , we need to make sure we do not ship with peer deps misaligned to prevent bug prone scenarios
There was a problem hiding this comment.
@ameliejyc let's make sure this PR is not on the release with the peer dependencies misaligned please



Description
This PR integrates the EarnController core updates for BIP-44 in version 7.0.0.
It also bumps the account-tree-controller package to the latest patch we need 0.12.1 - the accounts team is fine for us to go ahead with this and will do a full alignment on their side later in the week.
Changelog
CHANGELOG entry: null
Related issues
Fixes: the mobile repo side of this
Accompanies this PR
Manual testing steps
Test the following scenarios that ensure that eligibility, pooled stakes, lending positions, and earn transactions work as expected.
Screenshots/Recordings
N/A
Pre-merge author checklist
Pre-merge reviewer checklist