Skip to content

fix(dashspend): metadata & barcode #1368

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

Merged
merged 15 commits into from
Apr 1, 2025

Conversation

Syn-McJ
Copy link
Member

@Syn-McJ Syn-McJ commented Mar 30, 2025

Gift card metadata is missing after relaunch. Some improvements are needed in the Gift Card details screen.

Issue being fixed or feature implemented

  • Merge master branch
  • Tighten up threading around updating metadata
  • Always render barcode internally from barcode data
  • Force 100% brightness when barcode is showing

Related PR's and Dependencies

Screenshots / Videos

How Has This Been Tested?

  • QA (Mobile Team)

Checklist:

  • I have performed a self-review of my own code and added comments where necessary
  • I have added or updated relevant unit/integration/functional/e2e tests

Summary by CodeRabbit

  • New Features

    • Introduced a new transaction status label “CoinJoin Combine Dust.”
    • Enhanced the username creation flow with a refined retry mechanism and improved keyboard support.
  • Style

    • Upgraded UI elements for gift card details and staking dialogs with smoother interactions and dynamic brightness adjustments.
    • Revised layouts in “More” and “Request Username” screens for clearer typography and consistent design.
    • Improved transaction list refresh for more reliable display.

Syn-McJ and others added 13 commits March 13, 2025 21:01
* fix: proguard for firebase

* fix: last sync timestamp update

* chore: remove developer mode

* chore: ktlint
* fix: mxing limit & resource mapper

* chore: cleanup

* chore: coderabbit comments
* fix: prevent NPE if user goes to network monitor before blockchain service has started

* fix: many fixes for username creation and restoration

* fix: when creating username, properly mark requested name link when saving

* fix: remove NPE's with wallet balance observers
* fix: upgrade to kotlin 2.0
@Syn-McJ Syn-McJ self-assigned this Mar 30, 2025
Copy link
Contributor

coderabbitai bot commented Mar 30, 2025

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

This pull request updates several dependency and plugin versions across Gradle files, modernizes Kotlin code by replacing deprecated methods and adding suspend modifiers, and refactors UI components. It removes obsolete developer mode and sensor functionality while streamlining error handling and state management in multiple modules. Additionally, it introduces new ProGuard rules, enhances coroutine support, and reorganizes layout files and resources for improved readability and maintainability.

Changes

File(s) Change Summary
build.gradle, common/build.gradle, wallet/build.gradle, wallet/proguard.cfg Updated dependency and plugin versions including kotlin_version, dppVersion, hiltVersion, and updated the KSP plugin; added the Kotlin Compose plugin; upgraded Compose BOM and added Firebase ProGuard rules.
common/src/.../Configuration.java, ExchangeRatesConfig.kt, BlockchainStateProvider.kt, TransactionUtils.kt, UserAvatarPlaceholderDrawable.kt, TextViewExt.kt, res/values/styles.xml Removed legacy developer mode constants/methods; added a @Singleton annotation; modified methods to be suspend where needed; replaced deprecated toUpperCase()/toLowerCase() with their modern counterparts; introduced a new style for menu row subtitles.
features/exploredash/src/... Simplified database open callbacks and error handling; consolidated multiple LiveData observers into a single uiState for GiftCard details; added bottom sheet callbacks to adjust screen brightness dynamically.
integrations/crowdnode/src/... Changed APY-retrieval methods to suspend functions for coroutine support; migrated UI updates within dialogs to asynchronous coroutine calls; adjusted mocking to use coroutine-friendly approaches.
wallet/... (multiple files) Removed unused services (e.g., WorkManager foreground service); improved blockchain state and error handling with safe calls and suspend functions; migrated from LiveData to StateFlow; refactored layout files and resources (dimensions, strings, XML layouts); added dependency injection improvements and utility method adjustments for transaction mapping.
wallet/src/.../RequestUserNameViewModel.kt, RequestUsernameFragment.kt, MoreFragment.kt Enhanced username check state management by introducing a new boolean flag and reset method; refactored click/keyboard handling by encapsulating submission logic in an onContinue() method; added a retry function triggered by specific error messages.

Sequence Diagram(s)

sequenceDiagram
    participant U as User
    participant D as GiftCardDetailsDialog
    participant B as BottomSheetCallback
    participant S as setMaxBrightness

    U->>D: Opens Gift Card Details Dialog
    D->>D: subscribeToBottomSheetCallback()
    B->>D: onSlide(event) triggered
    D->>S: Invoke setMaxBrightness(enable)
    S-->>D: Brightness updated
    D->>U: Reflects updated screen brightness
Loading
sequenceDiagram
    participant U as User
    participant R as RequestUsernameFragment
    participant K as KeyboardUtil
    participant VM as CreateIdentityViewModel

    U->>R: Enters username & clicks submit
    R->>R: onContinue() invoked
    R->>K: Adjust keyboard visibility
    R->>VM: Validate and process username
    VM-->>R: Return updated UI state
Loading

Possibly related PRs

Suggested reviewers

  • HashEngineering

Poem

I’m a rabbit with a happy hop,
Updates and fixes, I just can’t stop.
Gradle leaps and Kotlin dreams,
UI refactors in radiant beams.
Bugs hop away in the code’s parade,
A whiskered cheer for changes made!
🐇✨


🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai plan to trigger planning for file edits and PR creation.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@Syn-McJ
Copy link
Member Author

Syn-McJ commented Mar 30, 2025

@CodeRabbit review

Copy link
Contributor

coderabbitai bot commented Mar 30, 2025

✅ Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🧹 Nitpick comments (20)
wallet/src/de/schildbach/wallet/ui/more/AboutActivity.kt (2)

21-21: Import statement looks redundant

The android.content.Intent import appears to be redundant since there's already an import for Intent.ACTION_VIEW on line 22, and there's no explicit usage of the full Intent class name in the code.

-import android.content.Intent

148-151: Consider implementing unified transition animations

The overridden finish() method applies a custom transition animation. Consider extracting these animation constants to a centralized location to ensure consistent transitions across the application. This would make maintenance easier if animation styles need to be changed in the future.

integrations/crowdnode/src/main/java/org/dash/wallet/integrations/crowdnode/ui/dialogs/StakingDialog.kt (1)

44-57: Improved UI responsiveness by moving operations to background thread.

Moved potentially blocking operations (like API calls or data processing) into a coroutine to prevent blocking the main thread during UI initialization, which enhances user experience by keeping the UI responsive.

Consider adding error handling within the coroutine:

lifecycleScope.launch {
+   try {
        binding.stakingMessageTwo.text = getString(
            R.string.crowdnode_staking_info_message_second,
            String.format(Locale.getDefault(), "%.1f", viewModel.getMasternodeAPY())
        )
        binding.stakingFirstMinDepositMessage.text = getString(
            R.string.crowdnode_staking_info_message,
            CrowdNodeConstants.DASH_FORMAT.format(CrowdNodeConstants.MINIMUM_DASH_DEPOSIT)
        )
        binding.stakingApyTitle.text = getString(
            R.string.crowdnode_staking_apy_title,
            String.format(Locale.getDefault(), "%.1f", viewModel.getCrowdNodeAPY())
        )
+   } catch (e: Exception) {
+       // Handle error - maybe set default values or show an error message
+       Log.e("StakingDialog", "Failed to load staking information", e)
+   }
}
wallet/src/de/schildbach/wallet/ui/more/MoreFragment.kt (1)

299-309: Good centralization of retry logic

The new retry method centralizes the logic for determining whether a new username is needed based on the error message, which improves maintainability.

Consider extracting the error message patterns into constants or a companion object for better maintainability. This would make it easier to update or expand the list of error patterns in the future.

 private fun retry(errorMessage: String) {
-    val needsNewName = errorMessage.contains("Document transitions with duplicate unique properties") ||
-            errorMessage.contains("Document Contest for vote_poll ContestedDocumentResourceVotePoll") ||
-            errorMessage.contains(Regex("does not have .* as a contender")) ||
-            errorMessage.contains("missing domain document for ")
+    val needsNewName = errorMessage.contains(ERROR_DUPLICATE_PROPERTIES) ||
+            errorMessage.contains(ERROR_DOCUMENT_CONTEST) ||
+            errorMessage.contains(Regex(ERROR_CONTENDER_REGEX)) ||
+            errorMessage.contains(ERROR_MISSING_DOMAIN)
     if (!needsNewName) {
         createIdentityViewModel.retryCreateIdentity()
     } else {
         startActivity(Intent(requireContext(), CreateUsernameActivity::class.java))
     }
 }

And add these constants to the companion object:

companion object {
    // ... existing constants
    private const val ERROR_DUPLICATE_PROPERTIES = "Document transitions with duplicate unique properties"
    private const val ERROR_DOCUMENT_CONTEST = "Document Contest for vote_poll ContestedDocumentResourceVotePoll"
    private const val ERROR_CONTENDER_REGEX = "does not have .* as a contender"
    private const val ERROR_MISSING_DOMAIN = "missing domain document for "
}
wallet/src/de/schildbach/wallet/service/platform/work/RestoreIdentityWorker.kt (1)

296-320: Improved error handling by adding conditional processing for username existence.

The new conditional block ensures that multiple identity state updates are only performed when a valid username exists. This change prevents potential NullPointerExceptions and improper state transitions when no username is available.

However, the sequence of state transitions (REQUESTED_NAME_CHECKED → REQUESTED_NAME_CHECKING → REQUESTED_NAME_CHECKED again → VOTING) seems potentially redundant. Consider documenting these transitions for better maintainability.

 if (blockchainIdentity.currentUsername != null) {
+    // State transitions for identity with username:
+    // 1. Set initial checked state
     platformRepo.updateIdentityCreationState(
         blockchainIdentityData,
         BlockchainIdentityData.CreationState.REQUESTED_NAME_CHECKED
     )
     platformRepo.updateBlockchainIdentityData(blockchainIdentityData, blockchainIdentity)
+    // 2. Set checking state to verify name
     platformRepo.updateIdentityCreationState(
         blockchainIdentityData,
         BlockchainIdentityData.CreationState.REQUESTED_NAME_CHECKING
     )

+    // 3. Set checked state after verification link recovery
     // recover the verification link
     platformRepo.updateIdentityCreationState(
         blockchainIdentityData,
         BlockchainIdentityData.CreationState.REQUESTED_NAME_CHECKED
     )
     platformRepo.updateBlockchainIdentityData(blockchainIdentityData, blockchainIdentity)
+    // 4. Set final voting state
     // set voting state
     platformRepo.updateIdentityCreationState(
         blockchainIdentityData,
         BlockchainIdentityData.CreationState.VOTING
     )
     platformRepo.updateBlockchainIdentityData(blockchainIdentityData, blockchainIdentity)
 }
wallet/res/layout/fragment_request_username.xml (5)

7-8: Ensure consistent parent-child styling.

You have set android:background="@color/background_secondary" on both the parent ConstraintLayout and this TextView. If the intention is to inherit the background from the parent, consider removing the redundant background on the TextView to avoid possible overdraw or color mismatch.


18-18: Confirm naming consistency.

top_stack is a bit generic. Since it holds introductory text and instructions, you might rename it for clarity (e.g., intro_stack or header_container).


84-84: Encapsulate dimensions in resources.

Consider referencing a dimension resource (e.g., @dimen/min_height_input) for android:minHeight="72dp" to maintain consistency across the app and simplify future global adjustments.


206-210: Set importantForAccessibility on ProgressBar if needed.

If this ProgressBar indicates loading for visually impaired users, consider adding android:importantForAccessibility="yes" and a content description or using android:contentDescription="@string/loading" to ensure screen readers announce it.


213-245: Consider content descriptions for accessibility.

The ImageView with @+id/check_voting_period likely conveys a warning or status. Include android:contentDescription for screen readers. Also, ensure the text elements inside are fully descriptive if they represent time-sensitive or status-based content.

Also applies to: 246-246

wallet/src/de/schildbach/wallet/ui/transactions/TxResourceMapper.kt (3)

37-41: Use or remove the dateTimeFormat property.

Currently, dateTimeFormat is a public property with a default override but not used within this file. You may want to either provide usage or remove it if it’s unnecessary.


121-125: Check for null or unknown errors.

This block delegates to getErrorName(error). Make sure that if TxError.fromTransaction(tx) returns null, the code is safe. Consider a fallback or explicit handling if there's a possibility of a null result from that method.


147-177: Enhance user feedback for unconfirmed states.

You handle multiple conditions (ChainLocked, InstantSendLocked, etc.). If a transaction hovers between statuses, consider whether more granular user feedback would improve clarity, especially for partial confirmations or long wait times.

wallet/src/de/schildbach/wallet/ui/main/MainViewModel.kt (5)

68-68: Avoid duplicate imports for StateFlow.

Line 67 already imports StateFlow. This second import might be a merge artifact. Consider removing to keep imports clean.


200-201: Track usage of stale-rate fields.

New properties currentStaleRateState and rateStaleDismissed appear to shape UI feedback on outdated rates. Ensure your UI binds to rateStaleDismissed if you intend to hide repeated stale-rate alerts.


556-561: Update UI only on state changes.

You’re setting _transactions.value and calling getContactsAndMetadataForTransactions after each refresh. Ensure that if there’s no actual data change, you don’t trigger repeated UI updates or expensive lookups.


667-669: Maintain consistent transaction manipulation.

Similar to lines 556-561, be mindful of extra updates. Batch operations can help minimize repeated calls to _transactions. Verify minimal duplication in the final logic.


742-743: Watch floating-point precision.

Calculating staking APY with (100.0 - crowdNodeApi.getFee()) / 100 could cause rounding or precision issues at scale. Consider a BigDecimal approach for improved accuracy.

features/exploredash/src/main/java/org/dash/wallet/features/exploredash/ui/ctxspend/dialogs/GiftCardDetailsDialog.kt (2)

84-92: Consider simplifying the bottom sheet callback implementation.

The onStateChanged method is currently empty and flagged by static analysis. Consider using a lambda-style implementation for just the onSlide method. Also, you might want to handle the case when the user slides the sheet back up after having slid it down past the -0.5 threshold.

-private val bottomSheetCallback = object : BottomSheetBehavior.BottomSheetCallback() {
-    override fun onStateChanged(bottomSheet: View, newState: Int) { }
-
-    override fun onSlide(bottomSheet: View, slideOffset: Float) {
-        if (slideOffset < -0.5) {
-            setMaxBrightness(false)
-        }
-    }
-}
+private val bottomSheetCallback = BottomSheetBehavior.BottomSheetCallback { _, newState ->
+    // No operation needed here
+}.apply {
+    addOnSlideListener { _, slideOffset ->
+        if (slideOffset < -0.5) {
+            setMaxBrightness(false)
+        } else if (slideOffset >= -0.5 && originalBrightness >= 0) {
+            setMaxBrightness(true)
+        }
+    }
+}
🧰 Tools
🪛 detekt (1.23.7)

[warning] 85-85: This empty block of code can be removed.

(detekt.empty-blocks.EmptyFunctionBlock)


256-270: Ensure thread safety for window operations.

The setMaxBrightness method accesses dialog?.window which could potentially be null if the dialog is being dismissed. Consider adding a synchronized block or ensuring this method is only called from the main thread.

private fun setMaxBrightness(enable: Boolean) {
-    val window = dialog?.window ?: return
-    val params = window.attributes
+    val dialog = dialog ?: return
+    requireActivity().runOnUiThread {
+        val window = dialog.window ?: return@runOnUiThread
+        val params = window.attributes
    
-    if (enable) {
-        if (originalBrightness < 0) {
-            originalBrightness = params.screenBrightness
-        }
-        params.screenBrightness = 1.0f
-    } else {
-        params.screenBrightness = originalBrightness
-    }
+        if (enable) {
+            if (originalBrightness < 0) {
+                originalBrightness = params.screenBrightness
+            }
+            params.screenBrightness = 1.0f
+        } else {
+            params.screenBrightness = originalBrightness
+        }
    
-    window.attributes = params
+        window.attributes = params
+    }
}
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b9bc0ee and e340f1e.

📒 Files selected for processing (47)
  • build.gradle (2 hunks)
  • common/build.gradle (2 hunks)
  • common/src/main/java/org/dash/wallet/common/Configuration.java (0 hunks)
  • common/src/main/java/org/dash/wallet/common/data/ExchangeRatesConfig.kt (1 hunks)
  • common/src/main/java/org/dash/wallet/common/services/BlockchainStateProvider.kt (1 hunks)
  • common/src/main/java/org/dash/wallet/common/transactions/TransactionUtils.kt (0 hunks)
  • common/src/main/java/org/dash/wallet/common/ui/avatar/UserAvatarPlaceholderDrawable.kt (2 hunks)
  • common/src/main/java/org/dash/wallet/common/util/TextViewExt.kt (1 hunks)
  • common/src/main/res/values/styles.xml (1 hunks)
  • features/exploredash/src/main/java/org/dash/wallet/features/exploredash/ExploreDatabase.kt (1 hunks)
  • features/exploredash/src/main/java/org/dash/wallet/features/exploredash/ExploreSyncWorker.kt (1 hunks)
  • features/exploredash/src/main/java/org/dash/wallet/features/exploredash/ui/ctxspend/dialogs/GiftCardDetailsDialog.kt (8 hunks)
  • features/exploredash/src/main/java/org/dash/wallet/features/exploredash/ui/ctxspend/dialogs/GiftCardDetailsViewModel.kt (8 hunks)
  • features/exploredash/src/main/res/layout/dialog_gift_card_details.xml (3 hunks)
  • integrations/crowdnode/src/main/java/org/dash/wallet/integrations/crowdnode/ui/CrowdNodeViewModel.kt (2 hunks)
  • integrations/crowdnode/src/main/java/org/dash/wallet/integrations/crowdnode/ui/dialogs/StakingDialog.kt (2 hunks)
  • integrations/crowdnode/src/test/java/org/dash/wallet/integrations/crowdnode/CrowdNodeViewModelTest.kt (1 hunks)
  • wallet/AndroidManifest.xml (0 hunks)
  • wallet/build.gradle (2 hunks)
  • wallet/proguard.cfg (1 hunks)
  • wallet/res/layout/fragment_more.xml (1 hunks)
  • wallet/res/layout/fragment_request_username.xml (5 hunks)
  • wallet/res/values/dimens.xml (1 hunks)
  • wallet/res/values/strings.xml (1 hunks)
  • wallet/src/de/schildbach/wallet/WalletApplication.java (4 hunks)
  • wallet/src/de/schildbach/wallet/di/AppModule.kt (2 hunks)
  • wallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt (1 hunks)
  • wallet/src/de/schildbach/wallet/service/BlockchainStateDataProvider.kt (3 hunks)
  • wallet/src/de/schildbach/wallet/service/platform/PlatformSyncService.kt (2 hunks)
  • wallet/src/de/schildbach/wallet/service/platform/work/RestoreIdentityWorker.kt (2 hunks)
  • wallet/src/de/schildbach/wallet/transactions/coinjoin/CoinJoinTxResourceMapper.kt (2 hunks)
  • wallet/src/de/schildbach/wallet/transactions/coinjoin/CoinJoinTxWrapperFactory.kt (0 hunks)
  • wallet/src/de/schildbach/wallet/ui/CreateUsernameActivity.kt (1 hunks)
  • wallet/src/de/schildbach/wallet/ui/dashpay/CreateIdentityService.kt (2 hunks)
  • wallet/src/de/schildbach/wallet/ui/dashpay/GravatarProfilePictureDialog.kt (1 hunks)
  • wallet/src/de/schildbach/wallet/ui/main/MainViewModel.kt (7 hunks)
  • wallet/src/de/schildbach/wallet/ui/main/WalletTransactionsFragment.kt (3 hunks)
  • wallet/src/de/schildbach/wallet/ui/more/AboutActivity.kt (2 hunks)
  • wallet/src/de/schildbach/wallet/ui/more/MoreFragment.kt (3 hunks)
  • wallet/src/de/schildbach/wallet/ui/notifications/NotificationManagerWrapper.kt (2 hunks)
  • wallet/src/de/schildbach/wallet/ui/transactions/CrowdNodeTxResourceMapper.kt (1 hunks)
  • wallet/src/de/schildbach/wallet/ui/transactions/TransactionRowView.kt (0 hunks)
  • wallet/src/de/schildbach/wallet/ui/transactions/TxError.kt (2 hunks)
  • wallet/src/de/schildbach/wallet/ui/transactions/TxResourceMapper.java (0 hunks)
  • wallet/src/de/schildbach/wallet/ui/transactions/TxResourceMapper.kt (1 hunks)
  • wallet/src/de/schildbach/wallet/ui/username/voting/RequestUserNameViewModel.kt (4 hunks)
  • wallet/src/de/schildbach/wallet/ui/username/voting/RequestUsernameFragment.kt (6 hunks)
💤 Files with no reviewable changes (6)
  • common/src/main/java/org/dash/wallet/common/transactions/TransactionUtils.kt
  • wallet/src/de/schildbach/wallet/transactions/coinjoin/CoinJoinTxWrapperFactory.kt
  • wallet/src/de/schildbach/wallet/ui/transactions/TransactionRowView.kt
  • wallet/AndroidManifest.xml
  • common/src/main/java/org/dash/wallet/common/Configuration.java
  • wallet/src/de/schildbach/wallet/ui/transactions/TxResourceMapper.java
🧰 Additional context used
🧬 Code Definitions (4)
wallet/src/de/schildbach/wallet/ui/username/voting/RequestUsernameFragment.kt (2)
common/src/main/java/org/dash/wallet/common/util/NavigationExtensions.kt (1)
  • safeNavigate (30-40)
wallet/src/de/schildbach/wallet/ui/username/voting/VerifyIdentityFragment.kt (1)
  • checkViewConfirmDialog (109-120)
integrations/crowdnode/src/test/java/org/dash/wallet/integrations/crowdnode/CrowdNodeViewModelTest.kt (1)
integrations/crowdnode/src/main/java/org/dash/wallet/integrations/crowdnode/ui/CrowdNodeViewModel.kt (1)
  • getMasternodeAPY (412-419)
wallet/src/de/schildbach/wallet/service/BlockchainStateDataProvider.kt (2)
common/src/main/java/org/dash/wallet/common/services/BlockchainStateProvider.kt (1)
  • getMasternodeAPY (39-39)
integrations/crowdnode/src/main/java/org/dash/wallet/integrations/crowdnode/ui/CrowdNodeViewModel.kt (1)
  • getMasternodeAPY (412-419)
wallet/src/de/schildbach/wallet/WalletApplication.java (4)
common/src/main/java/org/dash/wallet/common/WalletDataProvider.kt (1)
  • wallet (33-81)
wallet/src/de/schildbach/wallet/transactions/WalletObserver.kt (1)
  • wallet (40-179)
wallet/src/de/schildbach/wallet/transactions/WalletMostRecentTransactionsObserver.kt (1)
  • wallet (34-98)
wallet/src/de/schildbach/wallet/transactions/WalletBalanceObserver.kt (1)
  • wallet (45-189)
🪛 detekt (1.23.7)
wallet/src/de/schildbach/wallet/service/BlockchainStateDataProvider.kt

[warning] 209-209: The caught exception is swallowed. The original exception could be lost.

(detekt.exceptions.SwallowedException)

features/exploredash/src/main/java/org/dash/wallet/features/exploredash/ui/ctxspend/dialogs/GiftCardDetailsDialog.kt

[warning] 85-85: This empty block of code can be removed.

(detekt.empty-blocks.EmptyFunctionBlock)

⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: build
🔇 Additional comments (86)
wallet/src/de/schildbach/wallet/ui/more/AboutActivity.kt (5)

43-44: Class signature simplified with removal of SensorEventListener interface

The removal of the SensorEventListener interface from the class signature aligns with the broader changes indicated in the PR summary - the application no longer implements shake detection functionality for developer mode toggling. This simplification is a positive change as it removes unused functionality.


53-95: onCreate method looks well-structured after removal of sensor setup

The onCreate method has been streamlined after removing the sensor setup code. It now focuses entirely on initializing the UI, setting up click listeners, and displaying application information. All functionality appears intact with appropriate error handling around the optional sync operation.


153-160: Implementation of support contact is clear and well-structured

The handleReportIssue() method appropriately checks if the activity is finishing before showing the dialog, which prevents potential window leaks. The dialog instantiation is also cleanly implemented.


97-107: Firebase ID handling methods are well-encapsulated

The showFirebaseIds() method properly follows MVVM architecture patterns by observing LiveData from the ViewModel and updating UI accordingly. The click listeners for copying IDs delegate to the ViewModel, maintaining a good separation of concerns.


109-146: Explore Dash sync status display is thorough and covers edge cases

The showExploreDashSyncStatus() method handles all potential states of sync including: in-progress, failed attempts, preloaded data, and successful sync. This comprehensive approach ensures the user always has clear visibility into the sync state.

wallet/src/de/schildbach/wallet/ui/transactions/TxError.kt (2)

20-21: Added necessary imports for context propagation.

These imports support the context propagation functionality that was added to the transaction error handling.


40-40: Thread safety improvement with context propagation.

Adding Context.propagate(Constants.CONTEXT) at the beginning of the method ensures that the BitcoinJ library operates with the correct context in the current thread. This is important for thread safety when working with BitcoinJ components and helps prevent potential runtime errors.

wallet/src/de/schildbach/wallet/ui/dashpay/GravatarProfilePictureDialog.kt (1)

53-53: Updated to modern Kotlin API.

Replaced the deprecated Java-style toLowerCase() with the Kotlin-idiomatic lowercase() function. This modernization maintains the same functionality while following current Kotlin best practices.

wallet/proguard.cfg (1)

254-259:

✅ Verification successful

Added Firebase-related ProGuard rules.

These rules prevent obfuscation of Firebase classes, which is essential for proper functionality of Firebase services in the application. This is particularly important for Firebase Authentication, as indicated by the specific rules for auth-related packages.

These rules should help prevent runtime errors related to obfuscated Firebase components. Consider verifying if all required Firebase components are covered by these rules.


🏁 Script executed:

#!/bin/bash
# Check if Firebase dependencies are explicitly declared in the project
grep -r "com.google.firebase" --include="*.gradle" .

Length of output: 1518


Firebase ProGuard rules verified & appear comprehensive

The updated rules in wallet/proguard.cfg (lines 254-259) correctly prevent obfuscation of all Firebase components declared across the project. In particular:

  • The general rule -keep class com.google.firebase.** { *; } ensures that all Firebase classes (including modules such as auth, database, analytics, in-app messaging, messaging, dynamic links, etc.) are preserved.
  • The inclusion of -keep class com.google.android.gms.** { *; } covers related Google services dependencies.
  • The specific rules for Firebase Authentication (-keepnames class com.google.firebase.auth.** and -keep interface com.google.firebase.auth.** { *; }) address potential issues related to auth component obfuscation.

The shell script output confirms that the declared Firebase dependencies (from modules like wallet, common, and features/exploredash) are adequately covered by these rules.

Approved – no further modifications required.

wallet/res/values/strings.xml (1)

181-181: Added string resource for CoinJoin Combine Dust transactions.

This new string resource enhances user understanding by providing a specific label for CoinJoin transactions that combine small amounts (dust) into more practical amounts.

The addition fits properly within the existing series of CoinJoin-related status strings in the file.

wallet/res/values/dimens.xml (1)

21-22: LGTM: Added new dimension resources

The new dimension resources provide useful values that can be referenced across the application layouts: a negative value for shifting UI elements and a zero constant for explicit zero-dimension declarations.

integrations/crowdnode/src/test/java/org/dash/wallet/integrations/crowdnode/CrowdNodeViewModelTest.kt (1)

91-91: Properly updated test mock for suspend function

The change from on to onBlocking correctly updates the test to handle the getMasternodeAPY() method that has been modified to be a suspend function in the actual implementation.

common/src/main/res/values/styles.xml (1)

168-175: LGTM: Added new style without line constraints

This new style appropriately inherits from Overline.Tertiary and provides the same styling as MenuRowSubTitle but without the line constraint, allowing for multi-line text display when needed.

wallet/src/de/schildbach/wallet/di/AppModule.kt (1)

126-130: LGTM: Added application-wide coroutine scope

Good implementation of a singleton application scope using SupervisorJob and Dispatchers.Default. This provides a structured way to manage coroutines across the application's lifecycle.

The SupervisorJob ensures that failures in child coroutines won't affect the entire scope, which is appropriate for an application-level scope.

wallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt (1)

1435-1435: Improved null safety with safe call operator.

Replacing the non-null assertion operator (!!) with a safe call operator (?.) makes this code more defensive against potential NullPointerException. This change allows the method to gracefully handle cases where blockChain is null, returning an empty list instead of crashing.

wallet/src/de/schildbach/wallet/ui/CreateUsernameActivity.kt (1)

43-43: Removed unused view model dependency.

The removal of confirmTransactionSharedViewModel cleans up unnecessary dependencies. This is a good change that eliminates unused code, potentially reducing memory usage and preventing initialization overhead.

features/exploredash/src/main/java/org/dash/wallet/features/exploredash/ExploreSyncWorker.kt (1)

156-156: Improved data consistency when updating database preferences.

Refreshing the databasePrefs before saving the updated values ensures you're working with the most current data. This prevents potential race conditions and avoids overwriting any changes that might have occurred during sync operations.

wallet/res/layout/fragment_more.xml (1)

233-238: Updated text style and improved design-time preview.

Switching to MenuRowSubTitleWithoutLines style improves the visual presentation, while adding the tools:text attribute enhances the design-time preview for developers by showing what error messages might look like.

wallet/build.gradle (2)

10-10: Kotlin Compose plugin added to support Compose features.

The addition of this plugin is necessary to support Jetpack Compose for UI development, aligning with modern Android development practices.


98-98:

❓ Verification inconclusive

Compose BOM version updated to 2024.03.00.

The Compose BOM version has been updated from 2023.03.00 to 2024.03.00, which provides access to the latest Compose features and bug fixes.


🏁 Script executed:

#!/bin/bash
# Check if the updated Compose BOM version is compatible with the Kotlin version
cat gradle.properties | grep kotlin_version

Length of output: 45


Action Required: Confirm Kotlin Compatibility

  • The Compose BOM version update to 2024.03.00 appears to be correct and should enable access to the latest Compose features and bug fixes.
  • However, the initial script to check for a kotlin_version in gradle.properties returned no output. This suggests that the Kotlin version might be defined elsewhere or not at all in that file.
  • Please manually verify whether the project explicitly sets the Kotlin version (for example, in any root or submodule build.gradle files or settings.gradle) and confirm that it is compatible with Compose BOM version 2024.03.00.
common/src/main/java/org/dash/wallet/common/ui/avatar/UserAvatarPlaceholderDrawable.kt (2)

34-34: Updated to use modern Kotlin API for character case conversion.

Replaced deprecated toUpperCase() with the newer uppercaseChar() method, which is the recommended approach in Kotlin.


48-48: Updated string uppercase method to use modern Kotlin API.

Replaced deprecated toUpperCase() with the newer uppercase() method, which is the recommended approach in Kotlin.

wallet/src/de/schildbach/wallet/ui/notifications/NotificationManagerWrapper.kt (2)

25-25: Added Bitmap import to support explicit type casting.

The imported Bitmap class is necessary for the explicit type casting below.


99-99: Added explicit type casting for bigLargeIcon parameter.

The null parameter now has an explicit as Bitmap? cast to improve type safety and compatibility with the newer Kotlin compiler.

integrations/crowdnode/src/main/java/org/dash/wallet/integrations/crowdnode/ui/dialogs/StakingDialog.kt (1)

24-26: Added coroutine-related imports to support asynchronous operations.

These imports enable the use of lifecycleScope and launch for moving potentially blocking operations off the main thread.

common/src/main/java/org/dash/wallet/common/util/TextViewExt.kt (1)

47-48: Updated to use non-deprecated lowercase method.

The code now correctly uses the modern lowercase() method instead of the deprecated toLowerCase(). This change aligns with Kotlin's current best practices for string manipulation.

common/src/main/java/org/dash/wallet/common/data/ExchangeRatesConfig.kt (1)

8-10: LGTM: Added singleton annotation.

Adding the @Singleton annotation ensures that only one instance of ExchangeRatesConfig will be created and shared across the application. This is appropriate for configuration classes that should maintain consistent state.

wallet/src/de/schildbach/wallet/ui/transactions/CrowdNodeTxResourceMapper.kt (1)

29-31: Refactored method to property for better Kotlin idioms.

The change from override fun getDateTimeFormat(): Int to override val dateTimeFormat: Int with a getter follows Kotlin's idiomatic approach to properties. This improves code readability while maintaining the same functionality.

features/exploredash/src/main/res/layout/dialog_gift_card_details.xml (3)

144-144: Made barcode placeholder visible by default.

Changed the barcode placeholder visibility from gone to visible, ensuring it's always shown until the actual barcode is loaded. This provides better visual feedback to users during loading states.


181-181: Updated error visibility in design-time preview.

Changed the design-time visibility of the barcode loading error to gone, making it consistent with its runtime behavior and improving the accuracy of the layout preview in Android Studio.


325-325: Hide card error by default.

Added android:visibility="gone" to ensure the card error is hidden by default, which aligns with the PR objective of tightening the UI around metadata handling. This change supports the improved error state management described in the PR summary.

common/src/main/java/org/dash/wallet/common/services/BlockchainStateProvider.kt (1)

39-40:

❓ Verification inconclusive

API change: Methods are now suspend functions

The getMasternodeAPY() and getLastMasternodeAPY() methods have been converted to suspend functions. This is a significant API change that will require any calling code to invoke these methods from within a coroutine scope.

This change aligns with the PR objective of "tightening threading around the updating of metadata" and ensures these potentially long-running operations don't block the main thread.


🏁 Script executed:

#!/bin/bash
# Find all direct usages of these methods to ensure they're called from coroutine contexts

echo "Searching for usages of getMasternodeAPY() and getLastMasternodeAPY()"
rg "getMasternodeAPY\(\)" --type kotlin
rg "getLastMasternodeAPY\(\)" --type kotlin

Length of output: 2132


Attention: Suspension API update requires verifying coroutine usage

The API methods have now been converted to suspend functions, so all invocations must occur from within a coroutine context. Our search confirmed that the updated methods are used in various modules (e.g., in MainViewModel.kt, BlockchainStateDataProvider.kt, and CrowdNodeViewModel.kt). However, note that in the CrowdNode integration—specifically in integrations/crowdnode/src/main/java/org/dash/wallet/integrations/crowdnode/ui/dialogs/StakingDialog.kt—the call to getMasternodeAPY() is used inline via String.format(…). Please ensure that this (and any similar usage) is wrapped in a proper coroutine context to avoid runtime issues.

  • Verify that all direct calls (particularly within UI/dialog code) occur in coroutine scopes.
  • Ensure tests mocking these suspend functions handle suspending behavior appropriately.
wallet/src/de/schildbach/wallet/ui/username/voting/RequestUserNameViewModel.kt (3)

69-69: Improved UI state tracking for username verification

Added a new state flag to track when a username check is in progress. This is a good addition that will help prevent multiple concurrent checks and enable the UI to show loading indicators.


250-252: Added state reset functionality

The new reset() method provides a clean way to reset the UI state to its initial values. This is useful for form resets or when navigating away from and back to the username request screen.


298-298: Properly managing loading state during username checks

The checkingUsername flag is now properly set at the beginning and end of the username checking process, ensuring the UI can accurately reflect the loading state.

Also applies to: 329-329

common/build.gradle (1)

6-6:

✅ Verification successful

Updated Compose dependencies and configuration

Added the Kotlin Compose plugin and updated the Compose BOM to version 2024.03.00. This ensures the project uses the latest Jetpack Compose features and optimizations.

Also applies to: 81-81


🏁 Script executed:

#!/bin/bash
# Check if the compose plugin is also applied in other modules

echo "Checking for Kotlin Compose plugin in other Gradle files"
rg "org\.jetbrains\.kotlin\.plugin\.compose" --type gradle

echo "Checking for other Compose BOM versions that may need updating"
rg "androidx\.compose:compose-bom:" --type gradle

Length of output: 788


Compose Plugin and BOM Update Verified

The verification confirms that the Kotlin Compose plugin and the Compose BOM are updated consistently across the project. Both common/build.gradle and wallet/build.gradle now apply the Kotlin Compose plugin and use the Compose BOM version 2024.03.00, ensuring that the latest features and optimizations are available.

  • common/build.gradle: Confirmed apply plugin: 'org.jetbrains.kotlin.plugin.compose' at line 6.
  • wallet/build.gradle: Also applies the Compose plugin and sets the BOM to 2024.03.00.

No further changes are required.

wallet/src/de/schildbach/wallet/transactions/coinjoin/CoinJoinTxResourceMapper.kt (2)

31-32: Refactored method to property override

Replaced the getDateTimeFormat() method with a property override, which is a more idiomatic Kotlin approach. This improves code readability while maintaining the same functionality.


49-49:

✅ Verification successful

Added support for CombineDust transaction type

Added handling for a new CoinJoin transaction type: CombineDust, which will now display the appropriate label in the UI.


🏁 Script executed:

#!/bin/bash
# Verify that the string resource exists for the new transaction type

echo "Checking for the CoinJoin Combine Dust string resource"
rg "transaction_row_status_coinjoin_combine_dust" --type xml

Length of output: 304


Verified: CombineDust Transaction Type Support is Correct

The new CombineDust case in CoinJoinTxResourceMapper.kt correctly maps to the string resource, and the corresponding resource exists in wallet/res/values/strings.xml. No further changes are required.

build.gradle (2)

3-10: Library version updates look good

The updates to Kotlin (2.1.0), Dagger Hilt (2.53), and Hilt Work (1.2.0) bring the project up to more recent versions.

Since the dependency on dpp has been updated to a SNAPSHOT version (1.7.4-SNAPSHOT), verify that this version is stable enough for production use or that there's a plan to update to a stable release before deployment.


66-67: Good plugin version alignment with Kotlin

The KSP plugin version has been updated to match the new Kotlin version, and the Kotlin Compose plugin has been added, which aligns with modern Android development practices.

wallet/src/de/schildbach/wallet/ui/more/MoreFragment.kt (2)

188-193: Improved error handling flow

The changes improve the error handling by storing the error message in a local variable and using a more precise condition check before calling the retry method.


288-289: Better error handling with fallback

The retrieval of the error message with a fallback to an empty string prevents potential null pointer exceptions and ensures consistent behavior.

features/exploredash/src/main/java/org/dash/wallet/features/exploredash/ExploreDatabase.kt (1)

129-159: Improved code structure and error handling

The refactored onOpen callback has:

  1. Reduced nesting levels
  2. Improved error handling with a cleaner try-catch block
  3. Maintained the same functionality with a more readable structure

These changes make the code more maintainable while preserving the original behavior.

wallet/src/de/schildbach/wallet/service/platform/PlatformSyncService.kt (2)

219-219: Enhanced logging with more context

Adding the current lastPreBlockStage to the log message provides more context about the state of the process when this message is generated, which will be helpful for debugging.


234-235: Improved null safety with defensive programming

This change enhances robustness by safely handling potential null values for votingPeriodStart with a default value of 0L, preventing potential NullPointerException errors.

wallet/src/de/schildbach/wallet/WalletApplication.java (4)

1126-1127: Enhanced null-checking to prevent potential NullPointerExceptions.

The update adds a check for walletBalanceObserver == null alongside the existing wallet null check, improving robustness by ensuring both components are available before proceeding.


1136-1137: Added defensive null check for wallet balance observer.

Similar to other balance observer methods, this change prevents potential crashes by checking if the balance observer is properly initialized before attempting to use it.


1149-1150: Improved error handling in observeBalance method.

This change enhances the robustness of the balance observation functionality by ensuring both the wallet and its observer are available before attempting to observe balances.


1159-1160: Added null check for walletBalanceObserver in observeSpendableBalance.

Consistent with the pattern applied to other balance observation methods, this change prevents potential NullPointerExceptions when the wallet balance observer hasn't been initialized yet.

integrations/crowdnode/src/main/java/org/dash/wallet/integrations/crowdnode/ui/CrowdNodeViewModel.kt (2)

421-424: Made getCrowdNodeAPY a suspend function for coroutine compatibility.

This update ensures consistency with the getMasternodeAPY() method it depends on, allowing both methods to be called from coroutine contexts for asynchronous execution.


412-419:

✅ Verification successful

Updated method signature to support coroutine-based execution.

The getMasternodeAPY() method has been converted to a suspend function, which aligns with modern Kotlin asynchronous programming patterns and allows it to be called from coroutine contexts.

This change likely coordinates with updates in the BlockchainStateProvider interface where the methods being called (getMasternodeAPY() and getLastMasternodeAPY()) have also been updated to be suspend functions.


🏁 Script executed:

#!/bin/bash
# Check if the BlockchainStateProvider interface has been updated to use suspend functions
rg -A 5 "interface BlockchainStateProvider" | grep -A 10 "getMasternodeAPY" || echo "Not found"

Length of output: 406


Update Approved: Coroutine-based API changes verified

The updated getMasternodeAPY() method in CrowdNodeViewModel.kt now correctly supports coroutine-based execution. Verification confirms that both getMasternodeAPY() and getLastMasternodeAPY() in the BlockchainStateProvider interface are suspend functions, aligning well with this update.

  • The method conversion to a suspend function is appropriate.
  • The interface functions being invoked are correctly updated to suspend functions.
wallet/src/de/schildbach/wallet/ui/dashpay/CreateIdentityService.kt (2)

737-737: Improved state tracking for username verification link handling.

Changed the state transition from REQUESTED_NAME_CHECKED to REQUESTED_NAME_LINK_SAVING to provide clearer tracking of when the system is saving a verification link.

This state transition provides better granularity in monitoring the identity creation workflow, making debugging and error tracking more effective.


753-753: Added intermediate state to track verification link saving completion.

The state transition from REQUESTED_NAME_LINK_SAVING to REQUESTED_NAME_LINK_SAVED provides a clearer indication that the verification link has been successfully saved.

This change improves the state management flow by making it more explicit and traceable, which enhances maintainability and debugging capabilities.

wallet/src/de/schildbach/wallet/ui/main/WalletTransactionsFragment.kt (2)

57-57: Added important utility import for view observation.

The addition of this import enables the use of the observe extension function, which is now being used in the transaction list update logic.


159-179: Great improvement to fragment lifecycle handling!

This change adds a crucial lifecycle safety check when updating the transaction list. By capturing the current lifecycle state and verifying both that the fragment is still attached (isAdded) and in at least the STARTED state before showing the transaction list, you've prevented potential crashes that could occur if the fragment was detached during the async list update.

This is a common source of bugs in Android applications, and this defensive programming approach properly handles the race condition between async data updates and fragment lifecycle changes.

wallet/src/de/schildbach/wallet/ui/username/voting/RequestUsernameFragment.kt (7)

7-8: Added necessary imports for new UI functionality.

These imports support the keyboard handling and layout adjustments implemented in this update.


41-41: Added keyboard utility for improved user experience.

This new property will be used to manage keyboard visibility and automatically adjust the UI layout accordingly.


72-78: Enhanced input handling with keyboard action support.

This improvement allows users to submit the form using the keyboard's action button when the continue button is enabled, providing a more intuitive experience for keyboard users.


86-86: Code cleanup through method extraction.

Replacing the inline logic with a call to the extracted onContinue() method improves code organization and maintainability.


107-108: Enhanced UI feedback during username checking.

These visibility changes provide better visual feedback to users during the username checking process:

  • The progress indicator is shown while checking
  • The voting period container is hidden during checking and shown when appropriate

This creates a more responsive and informative user experience.


185-195: Improved keyboard interaction with automatic layout adjustments.

This implementation adjusts the top margin of the UI when the keyboard appears, creating a better visual experience by ensuring content remains properly visible even with the keyboard open.


271-305: Good refactoring: Extracted complex logic into a dedicated method.

Moving this logic from the button click handler to a separate method improves code organization and enables reuse from multiple entry points (button click and keyboard action). The method handles the different flows for contestable usernames versus regular usernames in a clear, structured way.

features/exploredash/src/main/java/org/dash/wallet/features/exploredash/ui/ctxspend/dialogs/GiftCardDetailsViewModel.kt (6)

51-57: Excellent architectural improvement: Consolidated UI state into a data class.

This data class creates a single source of truth for the UI state, which will improve state management and testability. Using a data class with immutable properties ensures that state changes are explicit and traceable.


61-61: Good separation of concerns: Using applicationScope for long-running operations.

Injecting the application-scoped coroutine allows long-running operations to continue even if the ViewModel is cleared, which is appropriate for operations like network requests that should complete regardless of UI state.


79-80: Architectural improvement: Implemented StateFlow for UI state management.

Converting from individual LiveData properties to a StateFlow with a single state object:

  1. Creates a more predictable state update pattern
  2. Avoids potential race conditions between multiple state properties
  3. Makes state transitions more explicit and easier to debug

92-101: Improved state management using immutable updates.

Using _uiState.update { ... } with currentState.copy() ensures state updates are atomic and preserve all other state properties. This pattern prevents state inconsistencies that could occur with multiple separate property updates.


108-120: Good optimization: Reusing barcode when value hasn't changed.

The conditional update for barcode that only creates a new Barcode object when the value has changed helps optimize performance by avoiding unnecessary object creation and potential rendering.


234-234: Appropriate usage of applicationScope for network operations.

Moving the barcode fetching to the application scope ensures the operation can complete even if the view is destroyed, which is appropriate for this kind of background task that should complete regardless of UI state.

wallet/src/de/schildbach/wallet/service/BlockchainStateDataProvider.kt (1)

157-166: Good performance improvement: Added suspend modifier for coroutine support.

Converting getLastMasternodeAPY() to a suspend function with withContext improves application performance by moving the potentially heavy calculation to a background thread. This aligns with the interface changes observed in related files and follows modern Kotlin coroutine practices.

wallet/res/layout/fragment_request_username.xml (2)

28-28: Double-check duplication.

You already set @color/background_secondary on the parent container at line 7. Confirm overlapping backgrounds are intentional or remove the redundant background here.


200-201: FrameLayout usage appears valid.

You’ve introduced a FrameLayout to group and layer the voting progress indicator and the voting container. This is a clear pattern for overlaying views. No issues spotted.

wallet/src/de/schildbach/wallet/ui/transactions/TxResourceMapper.kt (3)

47-115: Validate coinjoin logic and transaction states.

This function branches heavily based on transaction types and coinjoin checks. Ensure all new Transaction.Type cases are thoroughly tested. Also confirm that any newly introduced masternode or identity-related transaction types are handled properly to avoid silent failures.


127-141: Ensure full coverage on error enumerations.

If new TxError enums are added later, remember to update this function. Otherwise, unknown errors might be silently mapped to TxError.Unknown.


188-195: Cross-check “Sending” condition logic.

This condition might cause false positives if broadcast peers are temporarily zero or if transaction confidence updates are delayed. Consider verifying states in real usage to prevent incorrectly labeling a transaction as "sending."

wallet/src/de/schildbach/wallet/ui/main/MainViewModel.kt (4)

156-156: Examine limited concurrency.

You used Dispatchers.IO.limitedParallelism(1). This ensures only one concurrent IO operation in your viewModelWorkerScope. Confirm that artificially limiting concurrency won’t cause performance issues (e.g., when fetching multiple resources in parallel).


163-164: Migration to StateFlow is consistent.

Your shift from LiveData to StateFlow is correct for reactive stream handling. Please confirm UI observers are updated to handle flows properly if you removed any asLiveData() usage in other parts of the app.


671-671: Confirm correct scoping for additional calls.

This function calls viewModelWorkerScope.launch { ... }. If external calls or canceled jobs rely on main-thread interactions, ensure you handle them on the correct dispatcher or coordinate to avoid race conditions.


676-677: Avoid blocking main flow.

Using viewModelWorkerScope.launch for contacting platformRepo.blockchainIdentity.getContactForTransaction() is good. Confirm that all UI states remain consistent if the user navigates away and the coroutine is still running.

features/exploredash/src/main/java/org/dash/wallet/features/exploredash/ui/ctxspend/dialogs/GiftCardDetailsDialog.kt (8)

82-83: Good addition for screen brightness management.

Adding the originalBrightness variable to store the initial screen brightness allows you to restore it properly when the barcode is no longer displayed, preventing the screen from staying at maximum brightness.


113-166: Excellent refactoring to use a unified UI state.

The transition from observing multiple individual properties to a single uiState object is a significant improvement. This consolidated approach simplifies the code, makes state management more predictable, and ensures that UI updates are synchronized.


176-176: Good addition of bottom sheet callback.

Adding the bottom sheet callback subscription helps control screen brightness when the user interacts with the bottom sheet.


194-194: Good use of extension function.

Using the toUri() extension function on the URL string is cleaner than creating a URI directly.


200-236: Excellent barcode generation implementation.

The barcode generation logic has been well-implemented with:

  1. Proper background threading using coroutines
  2. Appropriate error handling with logging
  3. Good UI feedback for success/failure states
  4. Responsive sizing based on the device's screen dimensions

This directly addresses one of the PR objectives to ensure barcodes are always rendered from barcode data.


238-254: Good implementation of barcode display with brightness control.

The implementation properly:

  1. Displays the barcode using Coil's image loading library
  2. Sets maximum brightness on successful loading
  3. Provides visual feedback in case of loading errors

This addresses the PR objective of ensuring 100% brightness when displaying barcodes.


272-279: Good implementation of callback subscription.

The method properly finds the bottom sheet view and sets up the callback. This is essential for tracking bottom sheet interactions to manage screen brightness.


281-284: Essential cleanup implementation.

The overridden dismiss and onDestroyView methods now properly:

  1. Reset screen brightness to its original value
  2. Remove the bottom sheet callback to prevent memory leaks
  3. Call their respective super methods

This ensures that resources are properly cleaned up when the dialog is dismissed or destroyed.

Also applies to: 286-293

Comment on lines +200 to 241
override suspend fun getMasternodeAPY(): Double {
return withContext(Dispatchers.Default) {
val masternodeListManager = dashSystemService.system.masternodeListManager
val blockChain = dashSystemService.system.blockChain
if (masternodeListManager != null && blockChain != null) {
val mnlist = masternodeListManager.listAtChainTip
if (mnlist.height != 0L) {
var prevBlock = try {
blockChain.blockStore.get(mnlist.height.toInt() - 1)
} catch (e: BlockStoreException) {
null
}
// if we cannot retrieve the previous block, use the chain tip
if (prevBlock == null) {
prevBlock = blockChain.chainHead
}

val validMNsCount = if (mnlist.size() != 0) {
var virtualMNCount = 0
mnlist.forEachMN(true) { entry ->
virtualMNCount += if (entry.isHPMN) 4 else 1
val validMNsCount = if (mnlist.size() != 0) {
var virtualMNCount = 0
mnlist.forEachMN(true) { entry ->
virtualMNCount += if (entry.isHPMN) 4 else 1
}
virtualMNCount
} else {
MASTERNODE_COUNT
}
virtualMNCount
} else {
MASTERNODE_COUNT
}

if (prevBlock != null) {
val apy = getMasternodeAPY(
walletDataProvider.wallet!!.params,
mnlist.height.toInt(),
prevBlock.header.difficultyTarget,
validMNsCount
)
configuration.prefsKeyCrowdNodeStakingApy = apy.toFloat()
return apy
if (prevBlock != null) {
val apy = getMasternodeAPY(
walletDataProvider.wallet!!.params,
mnlist.height.toInt(),
prevBlock.header.difficultyTarget,
validMNsCount
)
configuration.prefsKeyCrowdNodeStakingApy = apy.toFloat()
return@withContext apy
}
}
}
return@withContext configuration.prefsKeyCrowdNodeStakingApy.toDouble()
}
return configuration.prefsKeyCrowdNodeStakingApy.toDouble()
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Performance improvement with potential exception handling issue.

Converting getMasternodeAPY() to a suspend function and moving the execution to Dispatchers.Default is a good performance optimization. However, the caught BlockStoreException on line 209 is silently swallowed without logging or proper handling.

Improve the exception handling by logging the exception before falling back to the default path:

try {
    blockChain.blockStore.get(mnlist.height.toInt() - 1)
} catch (e: BlockStoreException) {
+    log.error("Failed to get previous block from blockstore", e)
    null
}
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
override suspend fun getMasternodeAPY(): Double {
return withContext(Dispatchers.Default) {
val masternodeListManager = dashSystemService.system.masternodeListManager
val blockChain = dashSystemService.system.blockChain
if (masternodeListManager != null && blockChain != null) {
val mnlist = masternodeListManager.listAtChainTip
if (mnlist.height != 0L) {
var prevBlock = try {
blockChain.blockStore.get(mnlist.height.toInt() - 1)
} catch (e: BlockStoreException) {
null
}
// if we cannot retrieve the previous block, use the chain tip
if (prevBlock == null) {
prevBlock = blockChain.chainHead
}
val validMNsCount = if (mnlist.size() != 0) {
var virtualMNCount = 0
mnlist.forEachMN(true) { entry ->
virtualMNCount += if (entry.isHPMN) 4 else 1
val validMNsCount = if (mnlist.size() != 0) {
var virtualMNCount = 0
mnlist.forEachMN(true) { entry ->
virtualMNCount += if (entry.isHPMN) 4 else 1
}
virtualMNCount
} else {
MASTERNODE_COUNT
}
virtualMNCount
} else {
MASTERNODE_COUNT
}
if (prevBlock != null) {
val apy = getMasternodeAPY(
walletDataProvider.wallet!!.params,
mnlist.height.toInt(),
prevBlock.header.difficultyTarget,
validMNsCount
)
configuration.prefsKeyCrowdNodeStakingApy = apy.toFloat()
return apy
if (prevBlock != null) {
val apy = getMasternodeAPY(
walletDataProvider.wallet!!.params,
mnlist.height.toInt(),
prevBlock.header.difficultyTarget,
validMNsCount
)
configuration.prefsKeyCrowdNodeStakingApy = apy.toFloat()
return@withContext apy
}
}
}
return@withContext configuration.prefsKeyCrowdNodeStakingApy.toDouble()
}
return configuration.prefsKeyCrowdNodeStakingApy.toDouble()
}
override suspend fun getMasternodeAPY(): Double {
return withContext(Dispatchers.Default) {
val masternodeListManager = dashSystemService.system.masternodeListManager
val blockChain = dashSystemService.system.blockChain
if (masternodeListManager != null && blockChain != null) {
val mnlist = masternodeListManager.listAtChainTip
if (mnlist.height != 0L) {
var prevBlock = try {
blockChain.blockStore.get(mnlist.height.toInt() - 1)
} catch (e: BlockStoreException) {
log.error("Failed to get previous block from blockstore", e)
null
}
// if we cannot retrieve the previous block, use the chain tip
if (prevBlock == null) {
prevBlock = blockChain.chainHead
}
val validMNsCount = if (mnlist.size() != 0) {
var virtualMNCount = 0
mnlist.forEachMN(true) { entry ->
virtualMNCount += if (entry.isHPMN) 4 else 1
}
virtualMNCount
} else {
MASTERNODE_COUNT
}
if (prevBlock != null) {
val apy = getMasternodeAPY(
walletDataProvider.wallet!!.params,
mnlist.height.toInt(),
prevBlock.header.difficultyTarget,
validMNsCount
)
configuration.prefsKeyCrowdNodeStakingApy = apy.toFloat()
return@withContext apy
}
}
}
return@withContext configuration.prefsKeyCrowdNodeStakingApy.toDouble()
}
}
🧰 Tools
🪛 detekt (1.23.7)

[warning] 209-209: The caught exception is swallowed. The original exception could be lost.

(detekt.exceptions.SwallowedException)

}

private fun updateSyncStatus(state: BlockchainState) {
private suspend fun updateSyncStatus(state: BlockchainState) {
if (_isBlockchainSynced.value != state.isSynced()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Change in function signature.

private suspend fun updateSyncStatus(state: BlockchainState) might influence call sites. Make sure all invocation points can handle suspension if this was previously a non-suspending method.

@Syn-McJ Syn-McJ requested a review from HashEngineering March 31, 2025 02:56
fun getMasternodeAPY(): Double
fun getLastMasternodeAPY(): Double
suspend fun getMasternodeAPY(): Double
suspend fun getLastMasternodeAPY(): Double
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

withContext call moved lower the stack

Copy link
Collaborator

@HashEngineering HashEngineering left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. also tested this to ensure that it works on my devices.

@HashEngineering HashEngineering merged commit 5af1492 into feature-dashspend Apr 1, 2025
3 checks passed
@Syn-McJ Syn-McJ deleted the fix/dashspend-txs branch April 2, 2025 05:08
@coderabbitai coderabbitai bot mentioned this pull request Apr 27, 2025
3 tasks
@coderabbitai coderabbitai bot mentioned this pull request Jul 30, 2025
3 tasks
@coderabbitai coderabbitai bot mentioned this pull request Aug 8, 2025
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants