feat: handle maya halted coins#1469
Conversation
📝 WalkthroughWalkthroughThe pull request enhances the Dash Wallet UI with radio group styling for action buttons (enabled/disabled states, custom colors) and significantly expands the Maya integration by adding Parcelable support across model classes, improving state management with StateFlow reactivity, and implementing a halted coins indicator in the currency picker interface. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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 |
|
@coderabbitai review |
1 similar comment
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
common/src/main/java/org/dash/wallet/common/ui/radio_group/RadioGroupAdapter.kt (1)
65-73:⚠️ Potential issue | 🟠 MajorDisabled items remain clickable.
The
isEnabledflag controls visual appearance (alpha at lines 225-229), but the click listener still fires for disabled items. Based on the usage inMayaCryptoCurrencyPickerFragment, halted coins setisEnabled = falsebut users can still tap and select them.🔧 Proposed fix to block clicks on disabled items
holder.binding.root.setOnClickListener { + val item = getItem(holder.adapterPosition) + if (!item.isEnabled) return@setOnClickListener + if (holder.adapterPosition != selectedIndex) { notifyItemChanged(selectedIndex) selectedIndex = holder.adapterPosition notifyItemChanged(holder.adapterPosition) } - clickListener.invoke(item, position) + clickListener.invoke(item, holder.adapterPosition) }Note: If some consumers need to handle disabled item clicks differently (e.g., show a toast explaining why it's disabled), consider adding a separate
disabledClickListenercallback instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@common/src/main/java/org/dash/wallet/common/ui/radio_group/RadioGroupAdapter.kt` around lines 65 - 73, The click handler on holder.binding.root currently always runs even when the item is disabled; update the listener in RadioGroupAdapter (the block that sets holder.binding.root.setOnClickListener) to first check the item's isEnabled flag and return early (or skip selection change and clickListener.invoke) when false. Ensure selectedIndex and notifyItemChanged calls only happen for enabled items, and keep clickListener.invoke(item, position) gated by the same check; if you need alternate behavior for disabled taps, add a separate disabledClickListener callback instead of invoking the normal click path.
🧹 Nitpick comments (1)
wallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt (1)
527-532: Consider usinglog.debugand simplifying the log message.This log statement fires for every monitored transaction on each new block, which can generate significant log output. Additionally, the comparison
{} == {}is redundant sincedepthInBlockswas just assigned that exact calculation on line 526—they will always be equal.♻️ Suggested improvement
- log.info( - "tx {}; {} == {}", + log.debug( + "tx {} depth: {}", transactionConfidence.transactionHash, - transactionConfidence.depthInBlocks, - wallet.lastBlockSeenHeight - transactionConfidence.appearedAtChainHeight + 1 + transactionConfidence.depthInBlocks )🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@wallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt` around lines 527 - 532, Change the verbose info-level log in BlockchainServiceImpl that prints transactionConfidence.transactionHash, transactionConfidence.depthInBlocks and the redundant computed value to a debug-level message and simplify it to avoid duplicate data; log only the tx hash and the depth (use transactionConfidence.depthInBlocks or the computed wallet.lastBlockSeenHeight - transactionConfidence.appearedAtChainHeight + 1, not both) and call log.debug instead of log.info so the statement no longer floods logs on every new block.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@integrations/maya/build.gradle`:
- Line 74: Replace the hardcoded dependency
"androidx.lifecycle:lifecycle-runtime-compose-android:2.10.0" with the project
variable and canonical artifact name: use
"androidx.lifecycle:lifecycle-runtime-compose:$lifecycleVersion" (i.e., remove
the "-android" suffix and reference $lifecycleVersion) so the integrations/maya
build.gradle aligns with other modules (e.g., wallet) and avoids version drift.
In `@integrations/maya/proguard-rules.pro`:
- Around line 1-2: Replace the ProGuard constructor keep rule so all
constructors are preserved for Gson deserialization: locate the rule targeting
org.dash.wallet.integrations.maya.model.** (the existing "-keep class
org.dash.wallet.integrations.maya.model.** { <init(); *; }") and change the
constructor spec to preserve all-arg constructors by using "<init>(...);"
instead of "<init();", i.e. keep the class pattern and members but use
"<init>(...);" so ProGuard retains all constructors required by Gson.
In
`@integrations/maya/src/main/java/org/dash/wallet/integrations/maya/ui/MayaConvertCryptoFragment.kt`:
- Around line 135-153: The current observer on swapTradeOrder in
MayaConvertCryptoFragment uses mayaViewModel.inboundAddresses.value directly and
may use a stale or halted DASH vault; change the flow to explicitly
refresh/fetch the latest inbound addresses before building the payment intent
(call whatever fetch method on mayaViewModel or add one if missing), then find
the DASH InboundAddress and ensure its halted flag is false; only call
getUpdatedPaymentIntent and safeNavigate when you have a fresh, non-halted
Address (use Address.fromBase58 with that address); if refresh fails, or no
valid DASH inbound address exists, do not navigate—surface an explicit error/
retry path (e.g., show an error UI or trigger
convertViewModel.retryFetchInboundAddresses) so the user can retry. Ensure you
update references in MayaConvertCryptoFragment: the swapTradeOrder observer, the
fetch call on mayaViewModel, the inboundAddress validation (!halted), and the
subsequent getUpdatedPaymentIntent/safeNavigate usage.
In
`@integrations/maya/src/main/java/org/dash/wallet/integrations/maya/ui/MayaCryptoCurrencyPickerFragment.kt`:
- Around line 132-171: The current mapping treats missing inbound address as not
halted (uses ?: false), causing items to appear enabled before addresses are
known; update the logic in the combine block (the mapping over
viewModel.poolList and viewModel.inboundAddresses) to distinguish "no address
yet" from "not halted": look up the inbound address for the pool into a nullable
(e.g., val inbound = addresses.find { it.chain == chain }), derive isHalted as
inbound?.halted (nullable) and compute isEnabled as inbound != null &&
inbound.halted == false; only compute price when inbound != null and
inbound.halted == false, and only show haltedLabel when inbound?.halted == true
so rows stay disabled/neutral until an inbound address is present.
In
`@integrations/maya/src/main/java/org/dash/wallet/integrations/maya/ui/MayaViewModel.kt`:
- Around line 81-84: The MutableStateFlow inboundAddresses should be made
private and exposed as a read-only StateFlow via asStateFlow(), and
updateInboundAddresses() must stop trying to call .clear()/.addAll() and instead
assign the new list to the flow's .value; specifically, replace the public
MutableStateFlow inboundAddresses with a private MutableStateFlow (e.g.,
_inboundAddresses) and a public val inboundAddresses =
_inboundAddresses.asStateFlow(), update updateInboundAddresses() to launch in
viewModelScope and set _inboundAddresses.value = mayaApi.getInboundAddresses(),
and ensure hasHaltedCoins still maps the exposed inboundAddresses StateFlow.
---
Outside diff comments:
In
`@common/src/main/java/org/dash/wallet/common/ui/radio_group/RadioGroupAdapter.kt`:
- Around line 65-73: The click handler on holder.binding.root currently always
runs even when the item is disabled; update the listener in RadioGroupAdapter
(the block that sets holder.binding.root.setOnClickListener) to first check the
item's isEnabled flag and return early (or skip selection change and
clickListener.invoke) when false. Ensure selectedIndex and notifyItemChanged
calls only happen for enabled items, and keep clickListener.invoke(item,
position) gated by the same check; if you need alternate behavior for disabled
taps, add a separate disabledClickListener callback instead of invoking the
normal click path.
---
Nitpick comments:
In `@wallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt`:
- Around line 527-532: Change the verbose info-level log in
BlockchainServiceImpl that prints transactionConfidence.transactionHash,
transactionConfidence.depthInBlocks and the redundant computed value to a
debug-level message and simplify it to avoid duplicate data; log only the tx
hash and the depth (use transactionConfidence.depthInBlocks or the computed
wallet.lastBlockSeenHeight - transactionConfidence.appearedAtChainHeight + 1,
not both) and call log.debug instead of log.info so the statement no longer
floods logs on every new block.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 98fcda10-a750-464b-85a4-4dbd276412c5
📒 Files selected for processing (20)
common/src/main/java/org/dash/wallet/common/ui/radio_group/IconifiedViewItem.ktcommon/src/main/java/org/dash/wallet/common/ui/radio_group/RadioGroupAdapter.ktfeatures/exploredash/src/main/java/org/dash/wallet/features/exploredash/network/service/stubs/FakeDashSpendService.ktintegrations/maya/MAYA_PROTOCOL.mdintegrations/maya/build.gradleintegrations/maya/proguard-rules.prointegrations/maya/src/main/java/org/dash/wallet/integrations/maya/model/CurrencyBeaconResponse.ktintegrations/maya/src/main/java/org/dash/wallet/integrations/maya/model/ExchangeRateResponse.ktintegrations/maya/src/main/java/org/dash/wallet/integrations/maya/model/FreeCurrencyResponse.ktintegrations/maya/src/main/java/org/dash/wallet/integrations/maya/model/InboundAddress.ktintegrations/maya/src/main/java/org/dash/wallet/integrations/maya/model/NetworkResponse.ktintegrations/maya/src/main/java/org/dash/wallet/integrations/maya/model/PoolInfo.ktintegrations/maya/src/main/java/org/dash/wallet/integrations/maya/model/SwapTransactionInfo.ktintegrations/maya/src/main/java/org/dash/wallet/integrations/maya/ui/MayaConvertCryptoFragment.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/layout/fragment_currency_picker.xmlintegrations/maya/src/main/res/values/strings-maya.xmlwallet/build.gradlewallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt
💤 Files with no reviewable changes (1)
- features/exploredash/src/main/java/org/dash/wallet/features/exploredash/network/service/stubs/FakeDashSpendService.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
Documentation
Bug Fixes