-
-
Notifications
You must be signed in to change notification settings - Fork 267
feat: improve currency rate fallback logic #7606
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: improve currency rate fallback logic #7606
Conversation
- Refactor #fetchExchangeRatesWithFallback into smaller helper methods: - #fetchRatesFromPriceApi: handles primary Price API call - #fetchRatesFromTokenPricesService: handles fallback fetching - #createNullRatesForCurrencies: creates null entries for failed currencies - Rename private members to use hash syntax (fixing lint errors) - Add partial success handling: when some currencies succeed and others fail, only failed currencies trigger fallback - Add comprehensive tests for partial success scenarios - Update existing tests to match new expected behavior
|
Cursor Agent can help with this pull request. Just |
Removed unused suppressions after refactoring: - no-negated-condition: no longer needed - no-restricted-syntax: no longer needed - @typescript-eslint/explicit-function-return-type: reduced count from 3 to 1
cdd82a4 to
b7809cd
Compare
…nd failedCurrencies
- Updated #fetchRatesFromTokenPricesService to return { rates, failedCurrencies }
similar to #fetchRatesFromPriceApi for consistency
- Removed null currency creation from fallback method - failures now add to
failedCurrencies instead of creating null rates inline
- Step 4 now takes the final list of failed currencies from step 3 to build
null rates, simplifying the logic
- Simplified #createNullRatesForCurrencies to only take currencies array
- Renamed PriceApiResult type to FetchRatesResult for broader applicability
- Wrapped the entire method body in try-catch so it never throws - On any unexpected error, returns all currencies as failed - Updated JSDoc to document that the method is designed to never throw - This allows #fetchExchangeRatesWithFallback to use const instead of let and removes the external try-catch, simplifying the orchestration logic
Co-authored-by: prithpal.sooriya <prithpal.sooriya@consensys.net>
|
@cursor READ THESE MEMORY FILES: Can you resolve get latest from main and resolve merge conflicts. |
|
Done! I've merged the latest from main and resolved the merge conflict. SummaryConflict resolved in: The conflict was between:
Resolution: Combined both by keeping all entries from main and adding our CurrencyRateController entry at the end of the All 40 tests pass ✅ Learn more about Cursor Agents |
Resolved merge conflict in CHANGELOG.md by combining: - Dependency bumps from main (transaction-controller, network-controller, accounts-controller, polling-controller) - Plasma mapping update from main - CurrencyRateController refactor entry from this branch
|
https://github.com/cursor READ THESE MEMORY FILES: https://raw.githubusercontent.com/Prithpal-Sooriya/Cloud-Agent-Memory/refs/heads/main/Memory.md |
|
Done! Merged the latest from main and resolved the new conflict. SummaryNew commits from main:
Conflict resolved in: Combined the new All 40 tests pass ✅ |
Explanation
Modifies CurrencyRateController to better handle fallback currency rates
Also decoupled the complex currency fetching logic into separate methods for the main exchange rates fetch vs spot prices fetch.
Oh and also resolve ESLint issues (agent led this)
https://consensys.zoom.us/clips/share/GCgPlIVRRtejh3f0Wgyhmg
References
https://consensyssoftware.atlassian.net/browse/ASSETS-2269
MetaMask/metamask-extension#39194
Checklist
Note
Improves reliability and structure of exchange rate fetching in
CurrencyRateController.#createNullRatesForCurrencies#fetchRatesFromPriceApiand#fetchRatesFromTokenPricesService; addsFetchRatesResulttype; converts fields to private#membersWritten by Cursor Bugbot for commit bdec20c. This will update automatically on new commits. Configure here.