Defi controller v2#9503
Open
bergarces wants to merge 21 commits into
Open
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
Contributor
Author
|
@metamaskbot publish-preview |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 2a8139a. Configure here.
| )) { | ||
| state.allDeFiPositionsV2[accountId] = positions; | ||
| } | ||
| }); |
There was a problem hiding this comment.
DeFi state not fully refreshed
Medium Severity
After a successful fetch, allDeFiPositionsV2 only updates account keys returned by groupDeFiPositionsV6, so queried accounts missing from the API response keep old positions. Early returns when the group has no DeFi-capable accounts leave prior group data in persisted state.
Reviewed by Cursor Bugbot for commit 2a8139a. Configure here.
| return { | ||
| networks: [...new Set(networks)] as CaipChainId[], | ||
| internalAccountIdByCaip, | ||
| }; |
There was a problem hiding this comment.
Networks list not always complete
Medium Severity
buildDeFiBalancesQuery builds the networks request parameter from EVM-only or Solana-only subsets based on which account types appear in the group, instead of always sending the full DEFI_SUPPORTED_NETWORKS list.
Reviewed by Cursor Bugbot for commit 2a8139a. Configure here.
| balance.metadata !== undefined && | ||
| (balance.metadata as Partial<V6BalanceMetadata>).protocolId !== undefined | ||
| ); | ||
| } |
There was a problem hiding this comment.
Position types not validated
Medium Severity
DeFi rows are accepted when only protocolId is present; positionType is not checked against exported DEFI_POSITION_TYPES, and liability handling hardcodes only lending instead of using that canonical list.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 2a8139a. Configure here.
Contributor
|
Preview builds have been published. Learn how to use preview builds in other projects. Expand for full list of packages and versions. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Explanation
References
Checklist
Note
Medium Risk
New portfolio-facing fetch path and valuation rules (e.g. lending liabilities), but additive alongside the legacy controller with broad unit tests and no auth changes.
Overview
Introduces
DeFiPositionsControllerV2, which loads DeFi positions for the selected account group viafetchV6MultiAccountBalances, maps per-chain CAIP-10 responses back to internal account IDs, and persistsallDeFiPositionsV2as protocol groups the DeFi tab can read without extra shaping.Supporting pieces include
buildDeFiBalancesQuery(EVM all-chains + Solana CAIP IDs, supported network list, EVM case normalization),groupDeFiPositionsV6(chain/protocol grouping, detail sections byprotocolName, icon stack,lendingsubtracted from aggregatedmarketValue), messengerfetchDeFiPositionswith per-account throttling and retry on failure, and a standalonedefi-visualizer.htmlfor manual API inspection.@metamask/core-backendnow exportsV6_DEFI_POSITION_TYPES/V6DeFiPositionTypeand typesV6BalanceMetadata.positionTypeaccordingly. Public exports fromassets-controllerscover the controller, state defaults, action types, and helpers. LegacyDeFiPositionsControlleris unchanged; the V2 state key avoids collisions when both run in one client.Reviewed by Cursor Bugbot for commit 2a8139a. Bugbot is set up for automated code reviews on this repo. Configure here.