fix(maya): fix exchange rates using mayanode server#1484
fix(maya): fix exchange rates using mayanode server#1484HashEngineering wants to merge 11 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThis PR migrates the Maya integration from the deprecated Midgard v2 pools API to the mayanode endpoint, replaces server-side pool pricing with client-side USD/fiat conversion, removes legacy API code, and updates error handling with new error types and UI strings. ChangesMaya Midgard to Mayanode API Migration
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
Tip For best results, initiate chat on the files or code changes.
[review] |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
integrations/maya/src/main/java/org/dash/wallet/integrations/maya/model/PoolInfo.kt (1)
57-61: 💤 Low valueRemove dead code:
setAssetPricemethod andassetPriceInCacaopropertyNeither
setAssetPricenor theassetPriceInCacaoproperty it depends on have any call sites in the codebase. TheapplyPoolPrices()method inMayaViewModelsetspool.assetPriceFiatdirectly without invokingsetAssetPrice().🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@integrations/maya/src/main/java/org/dash/wallet/integrations/maya/model/PoolInfo.kt` around lines 57 - 61, Remove the dead API by deleting the setAssetPrice function and the assetPriceInCacao property from PoolInfo; locate the symbols setAssetPrice and assetPriceInCacao in PoolInfo.kt, verify there are no remaining references (applyPoolPrices in MayaViewModel already assigns pool.assetPriceFiat directly), and run a build to ensure no other code depends on those members before committing.integrations/maya/src/main/java/org/dash/wallet/integrations/maya/ui/MayaCryptoCurrencyPickerFragment.kt (1)
128-134: ⚡ Quick winThe new filter at line 129 makes the
elsebranch in.map {}(lines 158–170) unreachable dead code.Because
defaultItemMap.containsKey(pool.asset)is already required to betrueby the filter, every pool that reaches the.map {}block is guaranteed to satisfy theifat line 147. Theelsebranch (lines 158–170) can never execute and should be removed.♻️ Proposed cleanup
.map { pool -> val chain = pool.asset.substringBefore('.') val inbound = addresses.find { it.chain == chain } val isEnabled = inbound != null && !inbound.halted val price = if (isEnabled) { GenericUtils.formatFiatWithoutComma( viewModel.formatFiat(pool.assetPriceFiat) ) } else { null } val haltedLabel = if (inbound?.halted == true) getString(R.string.maya_halted_label) else null - if (defaultItemMap.containsKey(pool.asset)) { - defaultItemMap[pool.asset]!!.copy( - iconUrl = GenericUtils.getCoinIcon(pool.currencyCode), - iconSelectMode = IconSelectMode.None, - additionalInfo = price, - actionText = haltedLabel, - actionBackgroundColor = if (inbound?.halted == true) R.color.gray_100 else null, - actionTextColor = if (inbound?.halted == true) R.color.content_secondary else null, - isEnabled = isEnabled, - id = pool.asset - ) - } else { - IconifiedViewItem( - pool.currencyCode, - pool.asset, - iconUrl = GenericUtils.getCoinIcon(pool.currencyCode), - iconSelectMode = IconSelectMode.None, - additionalInfo = price, - actionText = haltedLabel, - actionBackgroundColor = if (inbound?.halted == true) R.color.gray_100 else null, - actionTextColor = if (inbound?.halted == true) R.color.content_secondary else null, - isEnabled = isEnabled, - id = pool.asset - ) - } + defaultItemMap[pool.asset]!!.copy( + iconUrl = GenericUtils.getCoinIcon(pool.currencyCode), + iconSelectMode = IconSelectMode.None, + additionalInfo = price, + actionText = haltedLabel, + actionBackgroundColor = if (inbound?.halted == true) R.color.gray_100 else null, + actionTextColor = if (inbound?.halted == true) R.color.content_secondary else null, + isEnabled = isEnabled, + id = pool.asset + ) }.sortedBy { it.title }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@integrations/maya/src/main/java/org/dash/wallet/integrations/maya/ui/MayaCryptoCurrencyPickerFragment.kt` around lines 128 - 134, The new upstream filter requires defaultItemMap.containsKey(pool.asset) so the conditional inside the .map { pool -> ... } (the if checking defaultItemMap.containsKey(pool.asset)) is now always true; remove the unreachable else branch (the code creating the fallback/else item) from the map block and simplify the map to only build the item for the if-case, updating any variable references in that block (pool, defaultItemMap) accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@integrations/maya/src/main/java/org/dash/wallet/integrations/maya/model/MayaErrorResponse.kt`:
- Around line 56-58: The current error-to-type mapping in MayaErrorResponse (the
branch that returns MayaErrorType.TRADING_IS_HALTED, AMOUNT_TOO_LOW or
QUOTE_ERROR) uses case-sensitive substring checks and can misclassify messages
with different casing; update the checks to be case-insensitive (e.g., normalize
the error string with lowercase()/uppercase() or use contains(..., ignoreCase =
true)) so the matches for "trading is halted" and "not enough asset to pay for
fees" reliably return MayaErrorType.TRADING_IS_HALTED and
MayaErrorType.AMOUNT_TOO_LOW respectively, otherwise fall back to
MayaErrorType.QUOTE_ERROR.
In
`@integrations/maya/src/main/java/org/dash/wallet/integrations/maya/model/PoolInfo.kt`:
- Around line 50-55: The assetPriceInCacao getter currently uses the throwing
BigDecimal(String) constructor for balanceCacao and also declares a local
variable named asset that shadows the class property; change the getter to
safely parse balanceCacao with toBigDecimalOrNull() (mirror balanceAsset usage),
return BigDecimal.ZERO if parsing fails or equals zero, and rename the local
parsed-asset variable (e.g., assetBd) to avoid shadowing the class property;
finally perform the divide with the same scale and RoundingMode.HALF_UP.
In
`@integrations/maya/src/main/java/org/dash/wallet/integrations/maya/ui/MayaViewModel.kt`:
- Around line 90-91: The backing MutableStateFlow _exchangeRates is currently
public and must be made private to prevent external emission; modify the
declaration of _exchangeRates (the MutableStateFlow<List<ExchangeRate>>) to be
private and keep the public immutable exchangeRates =
_exchangeRates.asStateFlow() so callers can only collect the state, not emit it
(this change should be applied inside the MayaViewModel where _exchangeRates and
exchangeRates are declared).
- Around line 115-126: Remove the dead diagnostic Flow block that observes
WalletUIConfig.SELECTED_CURRENCY and logs a computed rate (the chain starting
with walletUIConfig.observe(WalletUIConfig.SELECTED_CURRENCY) and including
flatMapLatest(exchangeRatesProvider::observeExchangeRate), onEach { ... } and
launchIn(viewModelScope)); this block never updates ViewModel state and uses
unsafe non-null assertions exchangeRate.rate!! and usdPrice.rate!! which can
throw NPEs and cancel the subscription. Delete the entire chain (or replace it
with a safe implementation that uses nullable checks on ExchangeRate.rate and
actually updates state via the ViewModel if needed) so no !! assertions remain
and the subscription is not silently killed.
---
Nitpick comments:
In
`@integrations/maya/src/main/java/org/dash/wallet/integrations/maya/model/PoolInfo.kt`:
- Around line 57-61: Remove the dead API by deleting the setAssetPrice function
and the assetPriceInCacao property from PoolInfo; locate the symbols
setAssetPrice and assetPriceInCacao in PoolInfo.kt, verify there are no
remaining references (applyPoolPrices in MayaViewModel already assigns
pool.assetPriceFiat directly), and run a build to ensure no other code depends
on those members before committing.
In
`@integrations/maya/src/main/java/org/dash/wallet/integrations/maya/ui/MayaCryptoCurrencyPickerFragment.kt`:
- Around line 128-134: The new upstream filter requires
defaultItemMap.containsKey(pool.asset) so the conditional inside the .map { pool
-> ... } (the if checking defaultItemMap.containsKey(pool.asset)) is now always
true; remove the unreachable else branch (the code creating the fallback/else
item) from the map block and simplify the map to only build the item for the
if-case, updating any variable references in that block (pool, defaultItemMap)
accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 31a93af7-5762-4f63-8022-d1bd4dd413e7
📒 Files selected for processing (15)
.claude/agents/update-maya-currencies.md.tx/configintegrations/maya/MAYA_PROTOCOL.mdintegrations/maya/src/main/java/org/dash/wallet/integrations/maya/api/FiatExchangeRateApi.ktintegrations/maya/src/main/java/org/dash/wallet/integrations/maya/api/MayaApi.ktintegrations/maya/src/main/java/org/dash/wallet/integrations/maya/api/MayaLegacyWebApi.ktintegrations/maya/src/main/java/org/dash/wallet/integrations/maya/api/MayaWebApi.ktintegrations/maya/src/main/java/org/dash/wallet/integrations/maya/di/MayaModule.ktintegrations/maya/src/main/java/org/dash/wallet/integrations/maya/model/MayaErrorResponse.ktintegrations/maya/src/main/java/org/dash/wallet/integrations/maya/model/PoolInfo.ktintegrations/maya/src/main/java/org/dash/wallet/integrations/maya/model/SwapTradeResponse.ktintegrations/maya/src/main/java/org/dash/wallet/integrations/maya/ui/MayaConversionPreviewFragment.ktintegrations/maya/src/main/java/org/dash/wallet/integrations/maya/ui/MayaCryptoCurrencyPickerFragment.ktintegrations/maya/src/main/java/org/dash/wallet/integrations/maya/ui/MayaViewModel.ktintegrations/maya/src/main/res/values/strings-maya.xml
💤 Files with no reviewable changes (2)
- integrations/maya/src/main/java/org/dash/wallet/integrations/maya/api/MayaLegacyWebApi.kt
- integrations/maya/src/main/java/org/dash/wallet/integrations/maya/di/MayaModule.kt
Issue being fixed or feature implemented
Related PR's and Dependencies
Screenshots / Videos
How Has This Been Tested?
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes
Documentation