Defi controller v2#9503
Open
bergarces wants to merge 24 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. |
Contributor
Author
|
@metamaskbot publish-preview |
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
High Risk
Breaking
V6BalanceMetadatarename in core-backend affects consumers; the controller’s hardcoded test account override would mis-attribute DeFi data in production if shipped.Overview
Introduces
DeFiPositionsControllerV2, which loads DeFi positions for the selected account group viafetchV6MultiAccountBalances, maps v6 CAIP account IDs back to internal account IDs, and persists a UI-ready shape underallDeFiPositionsV2.fetchDeFiPositionsis messenger-exposed, gated byisEnabled, throttled per account set, and clears throttle on failure.New helpers
buildDeFiBalancesQuery(EVMeip155:0+ Solana accounts, supported networks, CAIP normalization) andgroupDeFiPositionsV6(protocol groups per chain, detail sections byproductName,lendingsubtracted frommarketValue, icon groups and token images) implement the fetch and transform pipeline. Types and constants are exported fromassets-controllers.core-backendaligns v6 DeFi metadata with the API:protocolName→productName(breaking), typedpositionTypeviaV6_DEFI_POSITION_TYPES, andgroupIdonV6BalanceMetadata.Note:
fetchDeFiPositionsstill contains a temporary override that always queries a fixed test EVM address while attributing results to the selected account—likely not intended for release.Also adds a local
defi-visualizer.htmlfor manual API inspection and broad unit tests for query building, grouping, and the controller.Reviewed by Cursor Bugbot for commit 2a1338c. Bugbot is set up for automated code reviews on this repo. Configure here.