Skip to content

fix(crowdnode): disable all crowdnode functions except withdraw for existing accounts#1490

Merged
HashEngineering merged 20 commits into
masterfrom
fix/disable-crowdnode
Jun 10, 2026
Merged

fix(crowdnode): disable all crowdnode functions except withdraw for existing accounts#1490
HashEngineering merged 20 commits into
masterfrom
fix/disable-crowdnode

Conversation

@HashEngineering

@HashEngineering HashEngineering commented Jun 9, 2026

Copy link
Copy Markdown
Collaborator

Issue being fixed or feature implemented

Disable most CrowdNode functions, except for withdraw on existing accounts

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

  • Refactor

    • Made address metadata operations idempotent and simplified transaction metadata handling.
  • Chores

    • Forced a specific build dependency version for consistent builds.
  • Bug Fixes

    • Simplified CrowdNode UI: hid existing-account and deposit actions; staking entry and shortcuts now appear only when the CrowdNode account is ready and state is refreshed.
  • Documentation

    • Added module documentation describing CrowdNode integration, architecture, and temporary UI restrictions.

@coderabbitai

coderabbitai Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 0afa10b2-71ca-4a07-941c-f8d494fb5e0c

📥 Commits

Reviewing files that changed from the base of the PR and between 1750f2e and 4ff48b8.

📒 Files selected for processing (1)
  • integrations/crowdnode/CLAUDE.md

📝 Walkthrough

Walkthrough

This PR makes address-marking idempotent, exposes CrowdNode sign-up state to viewmodels, gates staking/account UI and shortcuts by that state, pins the recaptcha Gradle dependency, and adds CrowdNode integration documentation.

Changes

CrowdNode Feature Gating and Metadata Idempotency

Layer / File(s) Summary
Address metadata idempotency
wallet/src/de/schildbach/wallet/database/dao/AddressMetadataDao.kt, wallet/src/de/schildbach/wallet/service/WalletTransactionMetadataProvider.kt
AddressMetadataDao.markAddress now uses INSERT OR IGNORE and returns row id (or -1L if already existed); WalletTransactionMetadataProvider uses that return value instead of pre-checking with exists().
CrowdNode reactive status in ExploreEntryViewModel
wallet/src/de/schildbach/wallet/ui/explore/ExploreEntryViewModel.kt
New hasCrowdNodeAccount: LiveData<Boolean> derived from crowdNodeApi.signUpStatus; crowdNodeApi.restoreStatus() called on init and when blockchain sync completes.
CrowdNode feature gating in UI layers
wallet/src/de/schildbach/wallet/ui/explore/ExploreFragment.kt, integrations/crowdnode/src/main/java/org/dash/wallet/integrations/crowdnode/ui/entry_point/EntryPointFragment.kt, integrations/crowdnode/src/main/java/org/dash/wallet/integrations/crowdnode/ui/portal/PortalFragment.kt
ExploreFragment hides/shows staking by CrowdNode account, EntryPointFragment hides existing-account button and removes its handler, PortalFragment hides deposit button and shows online-account only when status is Done.
ShortcutsViewModel CrowdNode integration
wallet/src/de/schildbach/wallet/ui/main/shortcuts/ShortcutsViewModel.kt
ShortcutsViewModel injects CrowdNodeApi, subscribes to signUpStatus, filters out unavailable shortcuts (gating STAKING), and refreshes custom/preset shortcuts; calls restoreStatus() at startup.
ReCAPTCHA dependency version pinning
build.gradle
Global Gradle resolution rule forces com.google.android.recaptcha:recaptcha to version 18.4.0.
CrowdNode module documentation
integrations/crowdnode/CLAUDE.md
Adds documentation covering module architecture, DI, concurrency/testing guidance, and the temporary withdrawals-only UI restriction with revert instructions.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested reviewers

  • abaranouski

🐰 The rabbit hops through code paths deep,
Where CrowdNode status flows to keep,
Insert-or-ignore makes data tight,
And shortcuts filter left and right,
Features gated, safe and sound! 🎯

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 5.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and specifically describes the main change: disabling CrowdNode functions except withdrawal for existing accounts, which aligns with the code changes across multiple files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/disable-crowdnode

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@HashEngineering HashEngineering changed the base branch from master to build-11.8 June 9, 2026 20:43
@HashEngineering HashEngineering self-assigned this Jun 9, 2026
@HashEngineering HashEngineering changed the base branch from build-11.8 to master June 9, 2026 21:33

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Nitpick comments (2)
integrations/crowdnode/src/main/java/org/dash/wallet/integrations/crowdnode/ui/portal/PortalFragment.kt (1)

224-237: 💤 Low value

Consider removing unused deposit-enablement logic.

setDepositsEnabled is still called (line 201, 226) but has no effect since depositBtn is now permanently hidden (line 138). This function can be removed in a future cleanup to reduce dead code.

🤖 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/crowdnode/src/main/java/org/dash/wallet/integrations/crowdnode/ui/portal/PortalFragment.kt`
around lines 224 - 237, The setDepositsEnabled(Coin) method is dead because
depositBtn is permanently hidden; remove the unused method and all calls to
setDepositsEnabled (search for setDepositsEnabled(...) usages) to eliminate dead
code, and run a build to ensure no remaining references; if backward
compatibility is required temporarily, replace callers with a no-op comment or
feature-flag check instead of leaving the method in place.
wallet/src/de/schildbach/wallet/ui/explore/ExploreEntryViewModel.kt (1)

27-27: ⚡ Quick win

Consider consolidating ViewModel state into a single UIState data class.

ExploreEntryViewModel exposes three separate LiveData properties (stakingAPY, isBlockchainSynced, hasCrowdNodeAccount) rather than a single reactive UIState. As per coding guidelines, ViewModels should use a single UIState data class with StateFlow. The new field follows the existing pattern, so this isn't a regression, but consolidating into a single state holder would improve maintainability and align with the project's architectural standards.

Also applies to: 38-50

🤖 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 `@wallet/src/de/schildbach/wallet/ui/explore/ExploreEntryViewModel.kt` at line
27, ExploreEntryViewModel currently exposes three separate observables
(stakingAPY, isBlockchainSynced, hasCrowdNodeAccount); consolidate them into a
single UIState data class and expose it as a MutableStateFlow/StateFlow from
ExploreEntryViewModel instead of multiple LiveData fields; create a data class
(e.g., ExploreUiState) containing those three properties, replace the individual
LiveData/MutableLiveData with a private MutableStateFlow<ExploreUiState> and a
public StateFlow<ExploreUiState>, update all places that set
stakingAPY/isBlockchainSynced/hasCrowdNodeAccount to atomically update the state
via .update { it.copy(...) } in ExploreEntryViewModel, and adjust consumers to
observe the single StateFlow (or convert to LiveData if needed).

Source: Coding guidelines

🤖 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 `@build.gradle`:
- Around line 84-88: The forced ReCAPTCHA version pin in the configurations.all
-> resolutionStrategy block currently forces
'com.google.android.recaptcha:recaptcha:18.4.0'; either update that forced
coordinate to the current non-deprecated version ('18.9.1') or keep 18.4.0 but
add an inline comment immediately above the force call explaining why 18.4.0 is
required (e.g., compatibility/regression test link or vendor guidance) and
reference the deprecation note; modify the force string in the
resolutionStrategy.force call or add the explanatory comment next to that force
call accordingly.

---

Nitpick comments:
In
`@integrations/crowdnode/src/main/java/org/dash/wallet/integrations/crowdnode/ui/portal/PortalFragment.kt`:
- Around line 224-237: The setDepositsEnabled(Coin) method is dead because
depositBtn is permanently hidden; remove the unused method and all calls to
setDepositsEnabled (search for setDepositsEnabled(...) usages) to eliminate dead
code, and run a build to ensure no remaining references; if backward
compatibility is required temporarily, replace callers with a no-op comment or
feature-flag check instead of leaving the method in place.

In `@wallet/src/de/schildbach/wallet/ui/explore/ExploreEntryViewModel.kt`:
- Line 27: ExploreEntryViewModel currently exposes three separate observables
(stakingAPY, isBlockchainSynced, hasCrowdNodeAccount); consolidate them into a
single UIState data class and expose it as a MutableStateFlow/StateFlow from
ExploreEntryViewModel instead of multiple LiveData fields; create a data class
(e.g., ExploreUiState) containing those three properties, replace the individual
LiveData/MutableLiveData with a private MutableStateFlow<ExploreUiState> and a
public StateFlow<ExploreUiState>, update all places that set
stakingAPY/isBlockchainSynced/hasCrowdNodeAccount to atomically update the state
via .update { it.copy(...) } in ExploreEntryViewModel, and adjust consumers to
observe the single StateFlow (or convert to LiveData if needed).
🪄 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: 2050712b-0ba3-4774-87c8-b1e8531f56de

📥 Commits

Reviewing files that changed from the base of the PR and between 07f49be and 1750f2e.

📒 Files selected for processing (8)
  • build.gradle
  • integrations/crowdnode/src/main/java/org/dash/wallet/integrations/crowdnode/ui/entry_point/EntryPointFragment.kt
  • integrations/crowdnode/src/main/java/org/dash/wallet/integrations/crowdnode/ui/portal/PortalFragment.kt
  • wallet/src/de/schildbach/wallet/database/dao/AddressMetadataDao.kt
  • wallet/src/de/schildbach/wallet/service/WalletTransactionMetadataProvider.kt
  • wallet/src/de/schildbach/wallet/ui/explore/ExploreEntryViewModel.kt
  • wallet/src/de/schildbach/wallet/ui/explore/ExploreFragment.kt
  • wallet/src/de/schildbach/wallet/ui/main/shortcuts/ShortcutsViewModel.kt

Comment thread build.gradle
Comment on lines +84 to +88
configurations.all {
resolutionStrategy {
force 'com.google.android.recaptcha:recaptcha:18.4.0'
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check for security advisories for com.google.android.recaptcha:recaptcha

gh api graphql -f query='
{
  securityVulnerabilities(first: 10, ecosystem: MAVEN, package: "com.google.android.recaptcha:recaptcha") {
    nodes {
      advisory {
        summary
        severity
        publishedAt
      }
      vulnerableVersionRange
      firstPatchedVersion {
        identifier
      }
    }
  }
}'

Repository: dashpay/dash-wallet

Length of output: 112


🌐 Web query:

What is the latest version of com.google.android.recaptcha:recaptcha library and are there any security advisories for version 18.4.0?

💡 Result:

As of June 9, 2026, the latest version of the com.google.android.recaptcha:recaptcha library is 18.9.1 [1][2]. Regarding version 18.4.0, there was a critical security vulnerability discovered that affected Android SDK versions 18.0.0 through 18.3.0 [3]. Version 18.4.0 was released specifically to address and patch this security issue [4][5][3]. Consequently, version 18.4.0 itself is not the subject of an advisory but rather the solution to a vulnerability present in older versions [4][3]. Users of affected older versions were strongly encouraged to update to version 18.4.0 or higher [4][6][5]. Note that version 18.4.0 is now classified as a deprecated version under the official mobile SDK deprecation policy [7].

Citations:


Update/document the forced ReCAPTCHA version pin (18.4.0) in build.gradle.

  • com.google.android.recaptcha:recaptcha is forced to 18.4.0 at build.gradle:84-88.
  • No public security advisories surfaced for this exact pinned dependency in the advisory lookup, but official ReCAPTCHA mobile docs classify 18.4.0 as deprecated and the latest version is 18.9.1.
  • Update the forced version (or keep it only with an inline comment explaining why 18.4.0 is still required).
🤖 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 `@build.gradle` around lines 84 - 88, The forced ReCAPTCHA version pin in the
configurations.all -> resolutionStrategy block currently forces
'com.google.android.recaptcha:recaptcha:18.4.0'; either update that forced
coordinate to the current non-deprecated version ('18.9.1') or keep 18.4.0 but
add an inline comment immediately above the force call explaining why 18.4.0 is
required (e.g., compatibility/regression test link or vendor guidance) and
reference the deprecation note; modify the force string in the
resolutionStrategy.force call or add the explanatory comment next to that force
call accordingly.

@HashEngineering HashEngineering merged commit ccfd04a into master Jun 10, 2026
3 checks passed
@coderabbitai coderabbitai Bot mentioned this pull request Jun 10, 2026
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