feat: allow user to set the sync start date #1453
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
📝 WalkthroughWalkthroughAdds persistent wallet creation-date storage and surfaces it in restore/rescan flows; replaces legacy seed-restore dialog with Compose/activity date-pickers and dialogs; expands Compose typography/components; updates WalletFactory/service integration, resources, block-explorer endpoints, and related ViewModels/Activities. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Activity as RestoreWalletFromSeedActivity
participant VM as RestoreWalletFromSeedViewModel
participant Factory as DashWalletFactory
participant ServiceConfig as BlockchainServiceConfig
participant Prefs as DataStorePreferences
User->>Activity: Enter seed and optional date, tap Confirm
Activity->>VM: restoreWalletFromSeed(words) (suspend)
activate VM
VM->>VM: derive creationTime from selectedCreationDate
VM->>Factory: restoreFromSeed(words, creationTime)
activate Factory
Factory->>Factory: create Wallet with best-effort seed time
alt creationTime provided
Factory->>ServiceConfig: setWalletCreationDate(creationTime)
ServiceConfig->>Prefs: persist WALLET_CREATION_DATE
Prefs-->>ServiceConfig: ack
ServiceConfig-->>Factory: done
end
Factory-->>VM: return success/failure
deactivate Factory
VM-->>Activity: return Boolean result
deactivate VM
Activity->>User: show success or failure UI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
🚥 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
🧹 Recent nitpick comments
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (1)
🧰 Additional context used🧠 Learnings (12)📓 Common learnings📚 Learning: 2025-09-23T05:19:47.861ZApplied to files:
📚 Learning: 2025-08-08T13:29:37.702ZApplied to files:
📚 Learning: 2025-05-12T09:14:36.656ZApplied to files:
📚 Learning: 2025-08-25T14:48:39.247ZApplied to files:
📚 Learning: 2025-09-05T06:47:44.508ZApplied to files:
📚 Learning: 2025-08-08T16:48:49.964ZApplied to files:
📚 Learning: 2025-09-05T06:00:25.088ZApplied to files:
📚 Learning: 2025-08-25T15:00:20.777ZApplied to files:
📚 Learning: 2025-04-19T07:01:17.535ZApplied to files:
📚 Learning: 2025-04-19T07:01:17.535ZApplied to files:
📚 Learning: 2025-09-24T22:07:25.689ZApplied to files:
🧬 Code graph analysis (1)common/src/main/java/org/dash/wallet/common/data/BlockchainServiceConfig.kt (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (2)
✏️ Tip: You can disable this entire section by setting 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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In @common/src/main/java/org/dash/wallet/common/data/BlockchainServiceConfig.kt:
- Around line 59-66: The getWalletCreationDate() validation is inverted: it
currently returns creationDate only when creationDate <=
Constants.EARLIEST_HD_SEED_CREATION_TIME, which rejects valid dates; change the
comparison in getWalletCreationDate() to require creationDate >=
Constants.EARLIEST_HD_SEED_CREATION_TIME (keeping the null check and returned
value logic using WALLET_CREATION_DATE and
Constants.EARLIEST_HD_SEED_CREATION_TIME).
In @common/src/main/java/org/dash/wallet/common/ui/components/MyTheme.kt:
- Around line 201-206: Replace the non-standard FontWeight(650) with the
standard bold weight for consistency: update the TextStyle definitions (e.g.,
HeadlineLargeBold and the other bold headline styles that currently use
FontWeight(650)) to use FontWeight(700) or FontWeight.Bold instead of
FontWeight(650); ensure each bold headline TextStyle (the ones around the
current HeadlineLargeBold, and the similar bold variants at the other two
locations) is changed to the standard 700 weight.
- Line 135: Rename the mis-typed object Typeography to Typography in MyTheme.kt
and update all call sites to reference Typography (e.g., usages in
FeatureTopText.kt and DashModelDialog.kt); this is a public API rename so update
imports/usages, any references in XML or string templates, and run a
project-wide refactor/compile to catch and fix remaining references.
In @wallet/src/de/schildbach/wallet/ui/more/RescanBlockchainDialogFragment.kt:
- Around line 68-77: The fragment's lateinit callback onConfirm can crash after
recreation; change the declaration in RescanBlockchainDialogFragment from
"lateinit var onConfirm: (Boolean, Long?) -> Unit" to a nullable property like
"var onConfirm: ((Boolean, Long?) -> Unit)? = null" (or assign a no-op default)
and update any invocation sites to use a safe call (onConfirm?.invoke(...));
also persist/restore initialCreationDate via onSaveInstanceState and restore it
in onCreate/onViewStateRestored so the date survives configuration changes.
In @wallet/src/de/schildbach/wallet/ui/more/SettingsViewModel.kt:
- Around line 205-208: In setWalletCreationDate, avoid the force unwrap of
walletApplication.wallet; check walletApplication.wallet for null (or use a
safe-call) before mutating keyChainSeed.creationTimeSeconds and calling
blockchainServiceConfig.setWalletCreationDate(creationDate); if wallet is null,
return early (or no-op) so you don’t risk an NPE while preserving the existing
blockchainServiceConfig persistence call when a wallet exists.
In @wallet/src/de/schildbach/wallet/ui/RestoreWalletFromSeedActivity.kt:
- Line 207: The code concatenates a hardcoded " ✓" to
getString(R.string.restore_wallet_select_date) which breaks i18n; update the UI
assignment for binding.selectDateButton.text to use a localized string resource
(either a new string like restore_wallet_select_date_selected or a
placeholder-style resource) and fetch it via getString(...) so the checkmark is
part of the translatable text rather than concatenated at runtime.
In @wallet/src/de/schildbach/wallet/ui/SettingsFragment.kt:
- Line 194: The call to RescanBlockchainDialogFragment().show(...) is
force-unwrapping walletApplication.wallet
(walletApplication.wallet!!.keyChainSeed.creationTimeSeconds) which can throw an
NPE; update resetBlockchain (or the method containing that line in
SettingsFragment) to first null-check walletApplication.wallet and its
keyChainSeed/creationTimeSeconds and handle the null case (e.g., return early,
show an error/toast, or disable the dialog) before invoking
RescanBlockchainDialogFragment.show so the code never dereferences a null
wallet.
🧹 Nitpick comments (9)
common/src/main/java/org/dash/wallet/common/ui/components/FeatureTopText.kt (1)
44-54: Consider consolidating button visibility logic.The button visibility requires both
showButton=trueANDbuttonLabel!=null, which could be confusing. Consider either:
- Making
buttonLabelrequired whenshowButton=truevia documentation- Simplifying to just check
buttonLabel != nulland removing theshowButtonflagThis would prevent scenarios where
showButton=truebut nothing renders becausebuttonLabelis null.common/src/main/java/org/dash/wallet/common/ui/components/DashModelDialog.kt (2)
88-100: Remove commented-out code.Line 91 contains commented-out code referencing
iconBackgroundColor. Since this parameter appears to have been removed from the implementation, clean up this comment.
138-143: Remove commented-out padding code.Line 141 contains commented-out padding code. Either remove it entirely or document why it was disabled if this is intentional for the design update.
wallet/src/de/schildbach/wallet/ui/compose_views/WalletCreationDateInfoDialog.kt (2)
46-48: Unuseddialogparameter in content lambda.The
dialogparameter is passed but never used. Since this dialog has no buttons that need to dismiss it programmatically, consider using_to indicate it's intentionally unused.Suggested fix
- content = { dialog -> + content = { _ -> WalletCreationDateInfoContent() }
55-58: Remove commented-out code.Line 57 contains commented-out code. Either remove it or apply it if the background is needed.
Suggested fix
Column( modifier = Modifier .fillMaxWidth() - //.background(MyTheme.Colors.backgroundSecondary) .padding(top = 60.dp) // Space for drag indicator and close button ) {wallet/src/de/schildbach/wallet/ui/RestoreWalletFromSeedViewModel.kt (1)
53-53: Consider using StateFlow instead of LiveData for new code.Per the documented development patterns, asynchronously updated fields should use
StateFlowrather thanLiveData. However, given the existing codebase conventions, this can be addressed in a follow-up refactor.wallet/src/de/schildbach/wallet/service/WalletFactory.kt (1)
299-300: Remove emptyfinallyblock.The empty
finallyblock serves no purpose and can be removed for cleaner code.Proposed fix
checkWalletValid(wallet, params) blockchainServiceConfig.setWalletCreationDate(seedCreationTime) wallet - } finally { - } + } }.claude/agents/DEVELOPMENT-PATTERNS.md (1)
444-520: Consider adding blank lines around tables for markdown compatibility.Static analysis suggests adding blank lines around tables (MD058). This is a minor formatting issue that improves compatibility with some markdown renderers.
wallet/src/de/schildbach/wallet/ui/main/MainActivity.kt (1)
526-539: Minor inconsistency:wallet.isEncrypted()vswallet.isEncrypted.Line 531 uses
wallet.isEncrypted()(method call syntax) while line 505 useswallet.isEncrypted(property access syntax). Consider using consistent property access throughout for idiomatic Kotlin style.♻️ Suggested fix
- if (wallet.isEncrypted()) { + if (wallet.isEncrypted) {
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (28)
.claude/agents/DEVELOPMENT-PATTERNS.mdcommon/src/main/java/org/dash/wallet/common/data/BlockchainServiceConfig.ktcommon/src/main/java/org/dash/wallet/common/ui/components/DashModelDialog.ktcommon/src/main/java/org/dash/wallet/common/ui/components/FeatureTopText.ktcommon/src/main/java/org/dash/wallet/common/ui/components/MyTheme.ktcommon/src/main/java/org/dash/wallet/common/util/Constants.ktwallet/assets/checkpoints-testnet.txtwallet/devnet/res/values/values.xmlwallet/res/drawable/button_outlined_background.xmlwallet/res/drawable/rounded_background_light.xmlwallet/res/layout/activity_recover_wallet_from_seed.xmlwallet/res/values/strings.xmlwallet/src/de/schildbach/wallet/payments/RequestWalletBalanceTask.javawallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.ktwallet/src/de/schildbach/wallet/service/WalletFactory.ktwallet/src/de/schildbach/wallet/ui/OnboardingActivity.ktwallet/src/de/schildbach/wallet/ui/OnboardingViewModel.ktwallet/src/de/schildbach/wallet/ui/RestoreWalletFromFileViewModel.ktwallet/src/de/schildbach/wallet/ui/RestoreWalletFromSeedActivity.ktwallet/src/de/schildbach/wallet/ui/RestoreWalletFromSeedDialogFragment.javawallet/src/de/schildbach/wallet/ui/RestoreWalletFromSeedViewModel.ktwallet/src/de/schildbach/wallet/ui/SettingsFragment.ktwallet/src/de/schildbach/wallet/ui/backup/RestoreFromFileActivity.ktwallet/src/de/schildbach/wallet/ui/compose_views/WalletCreationDateInfoDialog.ktwallet/src/de/schildbach/wallet/ui/main/MainActivity.ktwallet/src/de/schildbach/wallet/ui/more/RescanBlockchainDialogFragment.ktwallet/src/de/schildbach/wallet/ui/more/SettingsViewModel.ktwallet/staging/res/values/values.xml
💤 Files with no reviewable changes (5)
- wallet/src/de/schildbach/wallet/ui/backup/RestoreFromFileActivity.kt
- wallet/src/de/schildbach/wallet/ui/RestoreWalletFromFileViewModel.kt
- wallet/src/de/schildbach/wallet/ui/RestoreWalletFromSeedDialogFragment.java
- wallet/src/de/schildbach/wallet/ui/OnboardingViewModel.kt
- wallet/src/de/schildbach/wallet/ui/OnboardingActivity.kt
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1423
File: wallet/src/de/schildbach/wallet/ui/EditProfileActivity.kt:418-446
Timestamp: 2025-08-25T14:48:39.247Z
Learning: HashEngineering prefers to refactor and reuse topup code for balance validation logic improvements in DashPay activities like EditProfileActivity, rather than implementing individual fixes in the current PR.
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1425
File: wallet/src/de/schildbach/wallet/ui/more/SettingsScreen.kt:122-151
Timestamp: 2025-09-05T06:47:44.508Z
Learning: HashEngineering prefers to defer UI edge case fixes like the CoinJoin finishing state display issue (showing "Finishing ...%" when mode is OFF but status is FINISHING) to later PRs when the main focus is on redesigning the settings screen.
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1417
File: features/exploredash/src/main/java/org/dash/wallet/features/exploredash/data/explore/MerchantDao.kt:0-0
Timestamp: 2025-08-08T16:48:49.964Z
Learning: In PR dashpay/dash-wallet#1417, HashEngineering chose to defer adding Room indexes for gift_card_providers (provider, denominationsType, merchantId) to a follow-up PR; do not block the current PR on this optimization. Files: features/exploredash/.../data/explore/MerchantDao.kt and features/exploredash/.../data/dashspend/GiftCardProvider.kt.
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1422
File: common/src/main/java/org/dash/wallet/common/util/Constants.kt:68-69
Timestamp: 2025-08-25T15:00:20.777Z
Learning: HashEngineering prefers to keep HTTP logging enabled in release mode (using log.info instead of gating with BuildConfig.DEBUG) to debug production errors, even though this may leak URLs in production logs. This is a deliberate trade-off for debugging purposes in the Dash wallet project.
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1410
File: wallet/src/de/schildbach/wallet/data/InvitationLinkData.kt:79-84
Timestamp: 2025-07-12T07:12:04.769Z
Learning: HashEngineering prefers to handle defensive validation improvements for URI parsing in follow-up PRs rather than including them in the current PR when the main focus is on replacing Firebase with AppsFlyer.
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1422
File: wallet/src/de/schildbach/wallet/data/PaymentIntent.java:476-481
Timestamp: 2025-08-25T15:04:18.461Z
Learning: HashEngineering prefers to keep payeeUsername and payeeUserId in PaymentIntent.toString() logs unredacted for debugging purposes, prioritizing troubleshooting capability over PII protection concerns in the Dash wallet project.
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1425
File: common/src/main/java/org/dash/wallet/common/ui/components/DashButton.kt:94-101
Timestamp: 2025-09-05T06:00:25.088Z
Learning: HashEngineering prefers to fix the DashButton alpha dimming issue (using explicit disabled colors instead of overall alpha) in a follow-up PR rather than including it in the current settings screen redesign PR.
📚 Learning: 2025-09-23T05:19:47.861Z
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1428
File: wallet/src/de/schildbach/wallet/ui/dashpay/PlatformRepo.kt:1065-1068
Timestamp: 2025-09-23T05:19:47.861Z
Learning: The Dash wallet app doesn't use non-encrypted wallets, so getWalletEncryptionKey() will never return null in practice. Defensive null-safe programming with ?.let is appropriate for methods like getIdentityFromPublicKeyId() in PlatformRepo.kt.
Applied to files:
wallet/src/de/schildbach/wallet/ui/more/SettingsViewModel.ktwallet/src/de/schildbach/wallet/ui/RestoreWalletFromSeedViewModel.ktcommon/src/main/java/org/dash/wallet/common/data/BlockchainServiceConfig.ktwallet/src/de/schildbach/wallet/service/WalletFactory.ktwallet/src/de/schildbach/wallet/ui/main/MainActivity.kt
📚 Learning: 2025-09-05T06:16:12.649Z
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1425
File: wallet/src/de/schildbach/wallet/ui/more/SettingsViewModel.kt:134-145
Timestamp: 2025-09-05T06:16:12.649Z
Learning: The dash-wallet project has a minimum SDK version of 24, so API availability checks for methods introduced in API 23 or earlier are not needed.
Applied to files:
wallet/src/de/schildbach/wallet/ui/more/SettingsViewModel.ktwallet/src/de/schildbach/wallet/service/WalletFactory.kt
📚 Learning: 2025-05-07T14:18:11.161Z
Learnt from: Syn-McJ
Repo: dashpay/dash-wallet PR: 1389
File: features/exploredash/src/main/java/org/dash/wallet/features/exploredash/ui/explore/SearchFragment.kt:45-46
Timestamp: 2025-05-07T14:18:11.161Z
Learning: In the ExploreViewModel of the dash-wallet application, `appliedFilters` is a StateFlow (not LiveData), so Flow operators like `distinctUntilChangedBy` can be used with it.
Applied to files:
wallet/src/de/schildbach/wallet/ui/RestoreWalletFromSeedViewModel.kt
📚 Learning: 2025-04-19T07:01:17.535Z
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1362
File: wallet/src/de/schildbach/wallet/ui/more/SettingsViewModel.kt:107-115
Timestamp: 2025-04-19T07:01:17.535Z
Learning: In the Dash Wallet app, DashPayConfig methods like isTransactionMetadataInfoShown() already use withContext(Dispatchers.IO) internally, so wrapping these calls with another withContext(Dispatchers.IO) in ViewModels is redundant.
Applied to files:
wallet/src/de/schildbach/wallet/ui/RestoreWalletFromSeedViewModel.ktcommon/src/main/java/org/dash/wallet/common/ui/components/DashModelDialog.kt
📚 Learning: 2025-04-19T07:01:17.535Z
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1362
File: wallet/src/de/schildbach/wallet/ui/more/SettingsViewModel.kt:107-115
Timestamp: 2025-04-19T07:01:17.535Z
Learning: In Dash Wallet, DataStore operations (like those in DashPayConfig through BaseConfig) already handle background threading internally. Adding another layer of withContext(Dispatchers.IO) around these calls in ViewModels is redundant because the Jetpack DataStore API automatically dispatches its operations to a background thread.
Applied to files:
common/src/main/java/org/dash/wallet/common/data/BlockchainServiceConfig.kt
📚 Learning: 2025-09-05T06:47:44.508Z
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1425
File: wallet/src/de/schildbach/wallet/ui/more/SettingsScreen.kt:122-151
Timestamp: 2025-09-05T06:47:44.508Z
Learning: HashEngineering prefers to defer UI edge case fixes like the CoinJoin finishing state display issue (showing "Finishing ...%" when mode is OFF but status is FINISHING) to later PRs when the main focus is on redesigning the settings screen.
Applied to files:
.claude/agents/DEVELOPMENT-PATTERNS.md
📚 Learning: 2025-04-16T17:07:27.359Z
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1377
File: wallet/src/de/schildbach/wallet/service/platform/PlatformService.kt:0-0
Timestamp: 2025-04-16T17:07:27.359Z
Learning: For PlatformService in the Dash Wallet, the implementation should avoid throwing exceptions when Constants.SUPPORTS_PLATFORM is false. The platform should only be initialized when SUPPORTS_PLATFORM is true, and initialization should be done lazily to defer Platform creation until needed.
Applied to files:
wallet/src/de/schildbach/wallet/service/WalletFactory.kt
📚 Learning: 2025-05-12T09:15:56.594Z
Learnt from: Syn-McJ
Repo: dashpay/dash-wallet PR: 1391
File: integrations/coinbase/src/main/java/org/dash/wallet/integrations/coinbase/ui/convert_currency/ConvertViewFragment.kt:134-155
Timestamp: 2025-05-12T09:15:56.594Z
Learning: The SegmentedPicker composable in Dash Wallet has a safety check for empty options, returning early if options.isEmpty() is true.
Applied to files:
wallet/src/de/schildbach/wallet/ui/RestoreWalletFromSeedActivity.kt
🧬 Code graph analysis (6)
wallet/src/de/schildbach/wallet/ui/compose_views/WalletCreationDateInfoDialog.kt (1)
common/src/main/java/org/dash/wallet/common/ui/components/FeatureTopText.kt (1)
FeatureTopText(43-119)
common/src/main/java/org/dash/wallet/common/data/BlockchainServiceConfig.kt (2)
wallet/src/de/schildbach/wallet/service/TestingSPVBlockStore.kt (1)
get(59-73)common/src/main/java/org/dash/wallet/common/data/BaseConfig.kt (1)
set(82-86)
wallet/src/de/schildbach/wallet/service/WalletFactory.kt (1)
wallet/src/de/schildbach/wallet/ui/RestoreWalletFromSeedViewModel.kt (1)
restoreWalletFromSeed(88-102)
wallet/src/de/schildbach/wallet/ui/more/RescanBlockchainDialogFragment.kt (3)
common/src/main/java/org/dash/wallet/common/ui/components/ComposeHostFrameLayout.kt (1)
setContent(18-24)wallet/src/de/schildbach/wallet/ui/compose_views/WalletCreationDateInfoDialog.kt (1)
createWalletCreationDateInfoDialog(42-50)common/src/main/java/org/dash/wallet/common/ui/components/DashModelDialog.kt (1)
ModalDialog(49-229)
wallet/src/de/schildbach/wallet/payments/RequestWalletBalanceTask.java (1)
wallet/src/de/schildbach/wallet/Constants.java (1)
Constants(48-340)
wallet/src/de/schildbach/wallet/ui/RestoreWalletFromSeedActivity.kt (2)
wallet/src/de/schildbach/wallet/ui/compose_views/WalletCreationDateInfoDialog.kt (1)
createWalletCreationDateInfoDialog(42-50)wallet/src/de/schildbach/wallet/database/entity/DashPayProfile.kt (1)
getString(60-62)
🪛 LanguageTool
.claude/agents/DEVELOPMENT-PATTERNS.md
[style] ~228-~228: This is not the usual sequence for adjectives that have no special emphasis.
Context: ...atureItemNumber (Internal Component) A blue circular badge with white number text, used for ...
(EN_ADJ_ORDER)
🪛 markdownlint-cli2 (0.18.1)
.claude/agents/DEVELOPMENT-PATTERNS.md
445-445: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
458-458: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
474-474: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
490-490: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
506-506: Tables should be surrounded by blank lines
(MD058, blanks-around-tables)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: check
🔇 Additional comments (24)
wallet/devnet/res/values/values.xml (1)
22-22: Verify the devnet block explorer endpoint is still functional.The removal of port
:3002from the block explorer URL appears intentional (part of a refactoring commit to remove obsolete code), but the change means the connection will use the default HTTPS port 443 instead. Confirm that the Insight service for devnet is accessible athttps://insight.ouzo.networks.dash.org/insightwithout the explicit port specification.wallet/staging/res/values/values.xml (1)
22-22: Remove this concern — the URL change is correct.The testnet block explorer service does not run on port 3002. Testing confirms the new URL without the port (defaulting to HTTPS port 443) is accessible and returns HTTP 200, while the old URL with
:3002fails to connect. This change fixes the block explorer URL, not breaks it.wallet/src/de/schildbach/wallet/payments/RequestWalletBalanceTask.java (1)
342-344: No issues found. The protocol difference (HTTPS for production/testnet, HTTP for devnet) is an intentional design choice appropriate for each environment. The devnet name extraction usingsubstring("devnet-".length())is consistent with the pattern used elsewhere in Constants.java. The endpoints are infrastructure dependencies that function as designed.wallet/assets/checkpoints-testnet.txt (1)
3-2395: Checkpoint header matches data rows — no action requiredDeclared checkpoint count (2392) equals the number of data rows; file format is consistent.
wallet/res/drawable/rounded_background_light.xml (1)
1-23: LGTM!The drawable resource is well-structured and follows Android XML conventions. The design parameters (8dp radius, 1dp stroke) align with the design system updates mentioned in this PR.
common/src/main/java/org/dash/wallet/common/ui/components/FeatureTopText.kt (1)
80-117: Button styling looks correct but verify small icon size is intentional.The button icons and text are sized at 13.dp/13.sp, which is quite small. Ensure this matches the design specifications, particularly for accessibility on smaller or lower-DPI screens.
common/src/main/java/org/dash/wallet/common/ui/components/DashModelDialog.kt (1)
50-61: Verify custom content doesn't conflict with other dialog elements.The new
contentparameter allows injecting arbitrary composables between the text blocks and buttons. Ensure there's guidance for users about:
- Proper spacing/padding expectations
- Interaction with
limitationItemslayout- Maximum height constraints to avoid dialog overflow
common/src/main/java/org/dash/wallet/common/ui/components/MyTheme.kt (1)
135-549: Comprehensive typography system added successfully.The typography system is well-structured with clear naming conventions and proper coverage of all size/weight combinations. The deprecated aliases provide good backward compatibility.
wallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt (1)
1403-1403: Change correctly prioritizesserviceConfig.getWalletCreationDate()with proper fallback.The implementation properly handles timestamp units (both are in seconds), includes synchronization between wallet and serviceConfig sources (verified in SettingsViewModel.kt:206-207), and the single usage site at this location correctly applies the new fallback pattern. No additional changes needed.
common/src/main/java/org/dash/wallet/common/util/Constants.kt (1)
30-30: LGTM!The constant is well-named and provides a clear earliest boundary for HD seed creation times (March 29, 2015). This aligns with the PR's goal of allowing users to set wallet sync start dates.
wallet/res/drawable/button_outlined_background.xml (1)
18-33: LGTM!Standard selector drawable with appropriate pressed and default states for an outlined button style. The 8dp corner radius and 1dp stroke provide consistent styling.
wallet/src/de/schildbach/wallet/ui/SettingsFragment.kt (1)
195-213: Verify the order of wallet creation date update and blockchain reset.The creation date is saved via
viewModel.setWalletCreationDate(it)(line 204) before callingwalletApplication.resetBlockchain()(line 206). Please confirm this order is correct, as the blockchain reset might need the updated creation date to be already persisted.wallet/res/layout/activity_recover_wallet_from_seed.xml (2)
43-97: LGTM!The date picker UI section is well-structured with:
- Clear label and info icon for user guidance
- Outlined button for date selection with calendar icon
- Hidden selected date display that can be revealed when a date is chosen
- Appropriate margins and padding for consistency
Accessibility is considered with the contentDescription on the info icon (line 62).
108-108: LGTM!The constraint update correctly references the new
selected_date_displayview, maintaining proper layout flow.wallet/src/de/schildbach/wallet/ui/RestoreWalletFromSeedViewModel.kt (1)
88-102: Wallet restoration flow looks correct.The suspend function properly passes the creation time to
walletFactory.restoreFromSeed()and returns a boolean success indicator. The flow handles both success and failure cases appropriately.wallet/src/de/schildbach/wallet/service/WalletFactory.kt (1)
275-298: Wallet restoration with creation time looks well-implemented.The implementation correctly:
- Falls back to
EARLIEST_HD_SEED_CREATION_TIMEwhen no creation time is provided- Constructs the
DeterministicSeedwith the appropriate timestamp- Persists the wallet creation date to
BlockchainServiceConfigwallet/src/de/schildbach/wallet/ui/RestoreWalletFromSeedActivity.kt (2)
167-198: Date picker implementation looks correct.The date picker properly:
- Constrains dates between
EARLIEST_HD_SEED_CREATION_TIMEand current time- Initializes to today's date
- Converts selected date to milliseconds and passes to ViewModel
225-235: Restoration flow correctly gates PIN setup on success.The updated flow only proceeds to
SetPinActivitywhenrestoreWalletFromSeedreturnstrue, preventing navigation on failure..claude/agents/DEVELOPMENT-PATTERNS.md (1)
1-50: Excellent development patterns documentation.This comprehensive guide covering MVVM architecture, Compose UI patterns, component mappings, and typography will significantly help maintain consistency across the codebase.
wallet/src/de/schildbach/wallet/ui/more/RescanBlockchainDialogFragment.kt (2)
125-129: Good use of DisposableEffect for date picker cleanup.Properly dismissing the
DatePickerDialogwhen the composable is disposed (e.g., when the screen locks) prevents window leaks.
112-262: Well-structured RescanBlockchainDialog composable.The dialog correctly:
- Manages selected date state with
remember- Provides date picker with proper constraints
- Shows clear button only when a date is selected
- Uses
ModalDialogwith appropriate content layoutwallet/res/values/strings.xml (2)
206-212: LGTM! Well-crafted user-facing strings.The wallet creation date strings are clear and informative. The explanation in
restore_wallet_creation_date_infoeffectively communicates the benefit of selecting a creation date, andwallet_creation_date_info_descriptionprovides a helpful warning about selecting a date that's too recent.
288-293: LGTM! Rescan blockchain dialog strings are consistent with the restore flow.The strings follow the established naming convention and provide clear guidance for the rescan feature.
wallet/src/de/schildbach/wallet/ui/main/MainActivity.kt (1)
501-524: LGTM! Good refactoring to reduce API surface and improve type safety.The visibility change from
opentoprivateis appropriate since these methods are internal implementation details. The non-nullImmutableList<ChildNumber>parameter type is safer and aligns with the call site at line 173 usingConstants.BIP44_PATH. The switch to property access syntax (wallet.isEncrypted,supportFragmentManager) is idiomatic Kotlin.
common/src/main/java/org/dash/wallet/common/data/BlockchainServiceConfig.kt
Show resolved
Hide resolved
common/src/main/java/org/dash/wallet/common/ui/components/MyTheme.kt
Outdated
Show resolved
Hide resolved
wallet/src/de/schildbach/wallet/ui/more/RescanBlockchainDialogFragment.kt
Show resolved
Hide resolved
wallet/src/de/schildbach/wallet/ui/RestoreWalletFromSeedActivity.kt
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In @common/src/main/java/org/dash/wallet/common/data/BlockchainServiceConfig.kt:
- Around line 68-70: The setter setWalletCreationDate currently persists any
Long but the getter (getWalletCreationDate) rejects dates before
EARLIEST_HD_SEED_CREATION_TIME; update setWalletCreationDate to validate the
input: ensure the provided date is >= EARLIEST_HD_SEED_CREATION_TIME (and
optionally <= System.currentTimeMillis()) before calling
set(WALLET_CREATION_DATE, date); if invalid, either throw an
IllegalArgumentException or return without persisting and log a clear error
message so invalid dates cannot be stored.
In @wallet/src/de/schildbach/wallet/ui/RestoreWalletFromSeedActivity.kt:
- Line 207: Replace the misuse of R.string.fiat_balance_with_currency and the
hardcoded "✓" by adding a dedicated localized string resource (e.g.,
restore_wallet_select_date_selected with placeholder like "%1$s ✓") and use it
when setting binding.selectDateButton.text; update the call to getString to use
R.string.restore_wallet_select_date_selected and pass
getString(R.string.restore_wallet_select_date) as the argument so the checkmark
comes from the localized format rather than a hardcoded symbol.
- Around line 167-198: The date value from showDatePickerDialog is currently
passed in milliseconds to viewModel.setWalletCreationDate but the rest of the
code (observeSelectedDate and BlockchainServiceConfig.getWalletCreationDate)
expects/validates seconds (Constants.EARLIEST_HD_SEED_CREATION_TIME is in
seconds); convert the selectedDateInMillis to seconds before calling
viewModel.setWalletCreationDate (e.g. divide by 1000L) so stored values are
seconds-consistent across observeSelectedDate and
BlockchainServiceConfig.getWalletCreationDate, and ensure any other
callers/readers of the wallet creation date treat it as seconds.
- Around line 225-235: The code currently does nothing when
viewModel.restoreWalletFromSeed(words) returns false; update the
RestoreWalletFromSeedActivity to show user-facing error feedback (e.g.,
Toast.makeText or a Snackbar) when restoreWalletFromSeed(words) is false, and
add the string resource key restore_wallet_from_seed_failed (with an appropriate
message) to resources; keep the existing startActivityForResult call to
SetPinActivity when the call returns true and only display the error UI and do
not proceed when it returns false.
🧹 Nitpick comments (2)
common/src/main/java/org/dash/wallet/common/ui/components/FeatureTopText.kt (1)
99-105: Prefer Typography style over inline text styling for consistency.The button label uses inline
fontSizeandlineHeightvalues instead of a predefined TextStyle fromMyTheme.Typography. For consistency with the heading (line 64) and body text (line 73), consider using a Typography style.The inline values
fontSize = 13.sp, lineHeight = 18.spmatchMyTheme.Captionor could useMyTheme.Typography.LabelLarge(14sp/20sp) for better alignment with the new Typography system.♻️ Proposed refactor
Text( text = buttonLabel, - fontSize = 13.sp, - lineHeight = 18.sp, + style = MyTheme.Caption, color = MyTheme.Colors.dashBlue, textAlign = TextAlign.Center )wallet/src/de/schildbach/wallet/ui/more/RescanBlockchainDialogFragment.kt (1)
105-110: Consider persisting initialCreationDate across configuration changes.The
show()method setsinitialCreationDatebut doesn't persist it insavedInstanceState. If the fragment is recreated (e.g., screen rotation), the initial date will be lost and the user will need to re-select it.While the
DisposableEffect(lines 125-129) properly dismisses any active date picker, persisting the date would provide a smoother user experience.♻️ Optional enhancement
Add state persistence:
override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) initialCreationDate = savedInstanceState?.getLong("initialCreationDate")?.takeIf { it != 0L } } override fun onSaveInstanceState(outState: Bundle) { super.onSaveInstanceState(outState) initialCreationDate?.let { outState.putLong("initialCreationDate", it) } }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
common/src/main/java/org/dash/wallet/common/data/BlockchainServiceConfig.ktcommon/src/main/java/org/dash/wallet/common/ui/components/FeatureTopText.ktcommon/src/main/java/org/dash/wallet/common/ui/components/MyTheme.ktwallet/src/de/schildbach/wallet/ui/RestoreWalletFromSeedActivity.ktwallet/src/de/schildbach/wallet/ui/more/RescanBlockchainDialogFragment.kt
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1425
File: wallet/src/de/schildbach/wallet/ui/more/SettingsScreen.kt:122-151
Timestamp: 2025-09-05T06:47:44.508Z
Learning: HashEngineering prefers to defer UI edge case fixes like the CoinJoin finishing state display issue (showing "Finishing ...%" when mode is OFF but status is FINISHING) to later PRs when the main focus is on redesigning the settings screen.
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1422
File: common/src/main/java/org/dash/wallet/common/util/Constants.kt:68-69
Timestamp: 2025-08-25T15:00:20.777Z
Learning: HashEngineering prefers to keep HTTP logging enabled in release mode (using log.info instead of gating with BuildConfig.DEBUG) to debug production errors, even though this may leak URLs in production logs. This is a deliberate trade-off for debugging purposes in the Dash wallet project.
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1410
File: wallet/src/de/schildbach/wallet/data/InvitationLinkData.kt:79-84
Timestamp: 2025-07-12T07:12:04.769Z
Learning: HashEngineering prefers to handle defensive validation improvements for URI parsing in follow-up PRs rather than including them in the current PR when the main focus is on replacing Firebase with AppsFlyer.
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1423
File: wallet/src/de/schildbach/wallet/ui/EditProfileActivity.kt:418-446
Timestamp: 2025-08-25T14:48:39.247Z
Learning: HashEngineering prefers to refactor and reuse topup code for balance validation logic improvements in DashPay activities like EditProfileActivity, rather than implementing individual fixes in the current PR.
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1422
File: wallet/src/de/schildbach/wallet/data/PaymentIntent.java:476-481
Timestamp: 2025-08-25T15:04:18.461Z
Learning: HashEngineering prefers to keep payeeUsername and payeeUserId in PaymentIntent.toString() logs unredacted for debugging purposes, prioritizing troubleshooting capability over PII protection concerns in the Dash wallet project.
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1425
File: common/src/main/java/org/dash/wallet/common/ui/components/DashButton.kt:94-101
Timestamp: 2025-09-05T06:00:25.088Z
Learning: HashEngineering prefers to fix the DashButton alpha dimming issue (using explicit disabled colors instead of overall alpha) in a follow-up PR rather than including it in the current settings screen redesign PR.
📚 Learning: 2025-09-23T05:19:47.861Z
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1428
File: wallet/src/de/schildbach/wallet/ui/dashpay/PlatformRepo.kt:1065-1068
Timestamp: 2025-09-23T05:19:47.861Z
Learning: The Dash wallet app doesn't use non-encrypted wallets, so getWalletEncryptionKey() will never return null in practice. Defensive null-safe programming with ?.let is appropriate for methods like getIdentityFromPublicKeyId() in PlatformRepo.kt.
Applied to files:
common/src/main/java/org/dash/wallet/common/data/BlockchainServiceConfig.kt
📚 Learning: 2025-04-19T07:01:17.535Z
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1362
File: wallet/src/de/schildbach/wallet/ui/more/SettingsViewModel.kt:107-115
Timestamp: 2025-04-19T07:01:17.535Z
Learning: In Dash Wallet, DataStore operations (like those in DashPayConfig through BaseConfig) already handle background threading internally. Adding another layer of withContext(Dispatchers.IO) around these calls in ViewModels is redundant because the Jetpack DataStore API automatically dispatches its operations to a background thread.
Applied to files:
common/src/main/java/org/dash/wallet/common/data/BlockchainServiceConfig.kt
📚 Learning: 2025-05-12T09:15:56.594Z
Learnt from: Syn-McJ
Repo: dashpay/dash-wallet PR: 1391
File: integrations/coinbase/src/main/java/org/dash/wallet/integrations/coinbase/ui/convert_currency/ConvertViewFragment.kt:134-155
Timestamp: 2025-05-12T09:15:56.594Z
Learning: The SegmentedPicker composable in Dash Wallet has a safety check for empty options, returning early if options.isEmpty() is true.
Applied to files:
wallet/src/de/schildbach/wallet/ui/RestoreWalletFromSeedActivity.ktwallet/src/de/schildbach/wallet/ui/more/RescanBlockchainDialogFragment.kt
🧬 Code graph analysis (3)
common/src/main/java/org/dash/wallet/common/data/BlockchainServiceConfig.kt (1)
common/src/main/java/org/dash/wallet/common/data/BaseConfig.kt (1)
set(82-86)
wallet/src/de/schildbach/wallet/ui/RestoreWalletFromSeedActivity.kt (2)
wallet/src/de/schildbach/wallet/ui/compose_views/WalletCreationDateInfoDialog.kt (1)
createWalletCreationDateInfoDialog(42-50)wallet/src/de/schildbach/wallet/database/entity/DashPayProfile.kt (1)
getString(60-62)
wallet/src/de/schildbach/wallet/ui/more/RescanBlockchainDialogFragment.kt (3)
common/src/main/java/org/dash/wallet/common/ui/components/ComposeHostFrameLayout.kt (1)
setContent(18-24)wallet/src/de/schildbach/wallet/ui/compose_views/WalletCreationDateInfoDialog.kt (1)
createWalletCreationDateInfoDialog(42-50)common/src/main/java/org/dash/wallet/common/ui/components/DashModelDialog.kt (1)
ModalDialog(49-229)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (6)
common/src/main/java/org/dash/wallet/common/ui/components/MyTheme.kt (1)
135-549: Typography system implementation looks solid.The comprehensive Typography object provides good coverage of text styles across Display, Headline, Title, Body, and Label categories. The deprecated aliases ensure backward compatibility during migration.
Note: Past review comments have already flagged the typo in the object name and the non-standard FontWeight(650) usage in the Headline Bold styles.
common/src/main/java/org/dash/wallet/common/ui/components/FeatureTopText.kt (1)
43-54: Well-structured component with good composability.The function signature provides sensible defaults and clear parameter names. The conditional rendering logic for text and button is clean, and the use of safe calls (
onButtonClick?.invoke()) properly handles nullable callbacks.Also applies to: 55-78, 80-98, 106-119
wallet/src/de/schildbach/wallet/ui/more/RescanBlockchainDialogFragment.kt (3)
68-70: Nullable callback addresses configuration change crash risk.The
onConfirmcallback is now nullable, which addresses the crash risk identified in past review comments when the fragment is recreated during configuration changes.
112-161: Solid date picker implementation with proper constraints.The composable correctly manages state with
rememberand properly constrains the date picker usingEARLIEST_HD_SEED_CREATION_TIMEas the minimum date. TheDisposableEffect(lines 125-129) ensures the date picker is dismissed when the dialog is disposed, preventing leaks.
163-261: Clean dialog UI with proper conditional rendering.The dialog layout is well-structured with:
- Clear labeling and info icon for the date field
- Conditional rendering of the clear button only when a date is selected (line 236)
- Proper visual hierarchy and spacing
- Standard cancel/confirm button pattern
wallet/src/de/schildbach/wallet/ui/RestoreWalletFromSeedActivity.kt (1)
168-168: No inconsistency issue exists—both Constants files define EARLIEST_HD_SEED_CREATION_TIME with identical values.
de.schildbach.wallet.Constants.EARLIEST_HD_SEED_CREATION_TIMEandorg.dash.wallet.common.util.Constants.EARLIEST_HD_SEED_CREATION_TIMEboth equal1427610960L. Date validation will remain consistent across the wallet restoration and configuration subsystems.Likely an incorrect or invalid review comment.
common/src/main/java/org/dash/wallet/common/data/BlockchainServiceConfig.kt
Outdated
Show resolved
Hide resolved
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @wallet/src/de/schildbach/wallet/ui/RestoreWalletFromSeedActivity.kt:
- Around line 200-213: In observeSelectedDate(), replace the misuse of
R.string.fiat_balance_with_currency and the hardcoded "✓" when setting
binding.selectDateButton.text: add a dedicated localized string resource (e.g.,
restore_wallet_select_date_selected with a placeholder for the label and
checkmark) and change the assignment in observeSelectedDate() to call
getString(R.string.restore_wallet_select_date_selected,
getString(R.string.restore_wallet_select_date)) so the checkmark is provided by
the string resource rather than hardcoded.
🧹 Nitpick comments (1)
wallet/src/de/schildbach/wallet/ui/RestoreWalletFromSeedActivity.kt (1)
156-165: Consider adding visual affordance for the clear action.The tap-to-clear behavior on
selectedDateDisplayisn't discoverable—users typically expect a clear/X button. Consider adding a small clear icon or tooltip, or alternatively show a confirmation before clearing.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
wallet/src/de/schildbach/wallet/ui/RestoreWalletFromSeedActivity.kt
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1425
File: wallet/src/de/schildbach/wallet/ui/more/SettingsScreen.kt:122-151
Timestamp: 2025-09-05T06:47:44.508Z
Learning: HashEngineering prefers to defer UI edge case fixes like the CoinJoin finishing state display issue (showing "Finishing ...%" when mode is OFF but status is FINISHING) to later PRs when the main focus is on redesigning the settings screen.
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1422
File: common/src/main/java/org/dash/wallet/common/util/Constants.kt:68-69
Timestamp: 2025-08-25T15:00:20.777Z
Learning: HashEngineering prefers to keep HTTP logging enabled in release mode (using log.info instead of gating with BuildConfig.DEBUG) to debug production errors, even though this may leak URLs in production logs. This is a deliberate trade-off for debugging purposes in the Dash wallet project.
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1410
File: wallet/src/de/schildbach/wallet/data/InvitationLinkData.kt:79-84
Timestamp: 2025-07-12T07:12:04.769Z
Learning: HashEngineering prefers to handle defensive validation improvements for URI parsing in follow-up PRs rather than including them in the current PR when the main focus is on replacing Firebase with AppsFlyer.
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1423
File: wallet/src/de/schildbach/wallet/ui/EditProfileActivity.kt:418-446
Timestamp: 2025-08-25T14:48:39.247Z
Learning: HashEngineering prefers to refactor and reuse topup code for balance validation logic improvements in DashPay activities like EditProfileActivity, rather than implementing individual fixes in the current PR.
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1422
File: wallet/src/de/schildbach/wallet/data/PaymentIntent.java:476-481
Timestamp: 2025-08-25T15:04:18.461Z
Learning: HashEngineering prefers to keep payeeUsername and payeeUserId in PaymentIntent.toString() logs unredacted for debugging purposes, prioritizing troubleshooting capability over PII protection concerns in the Dash wallet project.
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1425
File: common/src/main/java/org/dash/wallet/common/ui/components/DashButton.kt:94-101
Timestamp: 2025-09-05T06:00:25.088Z
Learning: HashEngineering prefers to fix the DashButton alpha dimming issue (using explicit disabled colors instead of overall alpha) in a follow-up PR rather than including it in the current settings screen redesign PR.
📚 Learning: 2025-05-12T09:15:56.594Z
Learnt from: Syn-McJ
Repo: dashpay/dash-wallet PR: 1391
File: integrations/coinbase/src/main/java/org/dash/wallet/integrations/coinbase/ui/convert_currency/ConvertViewFragment.kt:134-155
Timestamp: 2025-05-12T09:15:56.594Z
Learning: The SegmentedPicker composable in Dash Wallet has a safety check for empty options, returning early if options.isEmpty() is true.
Applied to files:
wallet/src/de/schildbach/wallet/ui/RestoreWalletFromSeedActivity.kt
🧬 Code graph analysis (1)
wallet/src/de/schildbach/wallet/ui/RestoreWalletFromSeedActivity.kt (1)
wallet/src/de/schildbach/wallet/ui/compose_views/WalletCreationDateInfoDialog.kt (1)
createWalletCreationDateInfoDialog(42-50)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (2)
wallet/src/de/schildbach/wallet/ui/RestoreWalletFromSeedActivity.kt (2)
145-154: LGTM!Clean separation of date picker setup into dedicated methods, and proper integration of the info dialog via
createWalletCreationDateInfoDialog().
225-237: Error feedback now provided—minor improvement opportunity.Good addition of error handling when
restoreWalletFromSeed()returns false. Consider using a more descriptive, dedicated error message instead of the genericR.string.errorplaceholder for better user experience.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
wallet/src/de/schildbach/wallet/ui/RestoreWalletFromSeedActivity.kt (1)
42-45: Redundant explicit imports.Lines 44-45 explicitly import
CalendarandDate, but these are already covered by the wildcard import on line 43 (java.util.*). Consider removing the redundant explicit imports.♻️ Suggested cleanup
import java.text.SimpleDateFormat import java.util.* -import java.util.Calendar -import java.util.Date import javax.inject.Inject
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
wallet/src/de/schildbach/wallet/ui/RestoreWalletFromSeedActivity.kt
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1425
File: wallet/src/de/schildbach/wallet/ui/more/SettingsScreen.kt:122-151
Timestamp: 2025-09-05T06:47:44.508Z
Learning: HashEngineering prefers to defer UI edge case fixes like the CoinJoin finishing state display issue (showing "Finishing ...%" when mode is OFF but status is FINISHING) to later PRs when the main focus is on redesigning the settings screen.
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1422
File: common/src/main/java/org/dash/wallet/common/util/Constants.kt:68-69
Timestamp: 2025-08-25T15:00:20.777Z
Learning: HashEngineering prefers to keep HTTP logging enabled in release mode (using log.info instead of gating with BuildConfig.DEBUG) to debug production errors, even though this may leak URLs in production logs. This is a deliberate trade-off for debugging purposes in the Dash wallet project.
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1410
File: wallet/src/de/schildbach/wallet/data/InvitationLinkData.kt:79-84
Timestamp: 2025-07-12T07:12:04.769Z
Learning: HashEngineering prefers to handle defensive validation improvements for URI parsing in follow-up PRs rather than including them in the current PR when the main focus is on replacing Firebase with AppsFlyer.
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1423
File: wallet/src/de/schildbach/wallet/ui/EditProfileActivity.kt:418-446
Timestamp: 2025-08-25T14:48:39.247Z
Learning: HashEngineering prefers to refactor and reuse topup code for balance validation logic improvements in DashPay activities like EditProfileActivity, rather than implementing individual fixes in the current PR.
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1422
File: wallet/src/de/schildbach/wallet/data/PaymentIntent.java:476-481
Timestamp: 2025-08-25T15:04:18.461Z
Learning: HashEngineering prefers to keep payeeUsername and payeeUserId in PaymentIntent.toString() logs unredacted for debugging purposes, prioritizing troubleshooting capability over PII protection concerns in the Dash wallet project.
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1425
File: common/src/main/java/org/dash/wallet/common/ui/components/DashButton.kt:94-101
Timestamp: 2025-09-05T06:00:25.088Z
Learning: HashEngineering prefers to fix the DashButton alpha dimming issue (using explicit disabled colors instead of overall alpha) in a follow-up PR rather than including it in the current settings screen redesign PR.
📚 Learning: 2025-05-12T09:15:56.594Z
Learnt from: Syn-McJ
Repo: dashpay/dash-wallet PR: 1391
File: integrations/coinbase/src/main/java/org/dash/wallet/integrations/coinbase/ui/convert_currency/ConvertViewFragment.kt:134-155
Timestamp: 2025-05-12T09:15:56.594Z
Learning: The SegmentedPicker composable in Dash Wallet has a safety check for empty options, returning early if options.isEmpty() is true.
Applied to files:
wallet/src/de/schildbach/wallet/ui/RestoreWalletFromSeedActivity.kt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (4)
wallet/src/de/schildbach/wallet/ui/RestoreWalletFromSeedActivity.kt (4)
145-154: LGTM!The date picker setup and info dialog integration are cleanly organized with appropriate separation of concerns.
156-165: LGTM!The date picker setup is clean. The click-to-clear behavior on the date display provides intuitive UX.
224-236: Error handling for failed restoration now in place.The addition of the error dialog on restoration failure (lines 234-236) addresses the previous concern about silent failures. Users now receive feedback when the operation fails.
177-189: Verify consistent time unit handling between Activity and ViewModel.Line 184 passes
selectedCalendar.time.time(milliseconds) tosetWalletCreationDate(), whileobserveSelectedDate()at line 200 treats the observed value as seconds (multiplying by 1000L). The commit message mentions fixing a ms/s bug—please confirm the ViewModel properly converts milliseconds to seconds on storage.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
build.gradle (1)
72-88: Scope "Central Portal Snapshots" to avoid unintended snapshot resolution and nondeterministic builds.The snapshot dependency
org.dashj:dashj-core:21.1.14-SNAPSHOTis actively used in wallet/build.gradle. With two unscoped snapshot repositories (s01.oss.sonatype.org and central.sonatype.com), Gradle will resolve the dependency from whichever repo appears first and contains the artifact. This creates non-deterministic builds if both repos host the same version and makes it impossible to control which source is authoritative.The commented-out
content { includeModule(...) }block indicates intent to scope this, but leaving it incomplete keeps the risk active. Uncomment and implement this filter to restrict Central Portal Snapshots to only the dependencies that require it.Proposed tightening (Groovy DSL)
allprojects { repositories { google() mavenLocal() mavenCentral() maven { url 'https://jitpack.io' } maven { url 'https://s01.oss.sonatype.org/content/repositories/snapshots/' } maven { name 'Central Portal Snapshots' url 'https://central.sonatype.com/repository/maven-snapshots/' - // Only search this repository for the specific dependency -// content { -// includeModule("<the snapshot's groupId>", "<the snapshot's artifactId>") -// } + // Only search this repository for the specific dependency + content { + includeModule('org.dashj', 'dashj-core') + } } } }Also evaluate whether both snapshot repositories are necessary, or if s01.oss.sonatype.org can be removed once Central Portal Snapshots is properly scoped.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
build.gradle
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1425
File: wallet/src/de/schildbach/wallet/ui/more/SettingsScreen.kt:122-151
Timestamp: 2025-09-05T06:47:44.508Z
Learning: HashEngineering prefers to defer UI edge case fixes like the CoinJoin finishing state display issue (showing "Finishing ...%" when mode is OFF but status is FINISHING) to later PRs when the main focus is on redesigning the settings screen.
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1422
File: common/src/main/java/org/dash/wallet/common/util/Constants.kt:68-69
Timestamp: 2025-08-25T15:00:20.777Z
Learning: HashEngineering prefers to keep HTTP logging enabled in release mode (using log.info instead of gating with BuildConfig.DEBUG) to debug production errors, even though this may leak URLs in production logs. This is a deliberate trade-off for debugging purposes in the Dash wallet project.
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1410
File: wallet/src/de/schildbach/wallet/data/InvitationLinkData.kt:79-84
Timestamp: 2025-07-12T07:12:04.769Z
Learning: HashEngineering prefers to handle defensive validation improvements for URI parsing in follow-up PRs rather than including them in the current PR when the main focus is on replacing Firebase with AppsFlyer.
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1423
File: wallet/src/de/schildbach/wallet/ui/EditProfileActivity.kt:418-446
Timestamp: 2025-08-25T14:48:39.247Z
Learning: HashEngineering prefers to refactor and reuse topup code for balance validation logic improvements in DashPay activities like EditProfileActivity, rather than implementing individual fixes in the current PR.
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1422
File: wallet/src/de/schildbach/wallet/data/PaymentIntent.java:476-481
Timestamp: 2025-08-25T15:04:18.461Z
Learning: HashEngineering prefers to keep payeeUsername and payeeUserId in PaymentIntent.toString() logs unredacted for debugging purposes, prioritizing troubleshooting capability over PII protection concerns in the Dash wallet project.
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1425
File: common/src/main/java/org/dash/wallet/common/ui/components/DashButton.kt:94-101
Timestamp: 2025-09-05T06:00:25.088Z
Learning: HashEngineering prefers to fix the DashButton alpha dimming issue (using explicit disabled colors instead of overall alpha) in a follow-up PR rather than including it in the current settings screen redesign PR.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build
- GitHub Check: check
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
common/src/main/java/org/dash/wallet/common/data/BlockchainServiceConfig.kt (1)
68-70: Consider validating input in the setter for consistency.The getter filters out dates
<= EARLIEST_HD_SEED_CREATION_TIME, but the setter accepts any value. This could lead to scenarios where a caller stores an invalid date that silently returnsnullon retrieval.🔧 Optional: Add validation to reject invalid dates early
suspend fun setWalletCreationDate(date: Long) { + require(date > Constants.EARLIEST_HD_SEED_CREATION_TIME) { + "Wallet creation date must be after EARLIEST_HD_SEED_CREATION_TIME" + } set(WALLET_CREATION_DATE, date) }wallet/src/de/schildbach/wallet/ui/SettingsFragment.kt (1)
193-194: Use the injectedwalletApplicationinstead of casting from Activity.The class already has
@Inject lateinit var walletApplication: WalletApplicationat line 77. Retrieving it again viarequireActivity().application as WalletApplicationis redundant.♻️ Use the injected instance
private fun resetBlockchain() { viewLifecycleOwner.lifecycleScope.launch { - val walletApplication = requireActivity().application as WalletApplication - val configuration = walletApplication.configuration + val configuration = walletApplication.configuration val creationDateFromSettings = viewModel.getWalletCreationDate()
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
common/src/main/java/org/dash/wallet/common/data/BlockchainServiceConfig.ktwallet/src/de/schildbach/wallet/ui/SettingsFragment.ktwallet/src/de/schildbach/wallet/ui/more/SettingsViewModel.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- wallet/src/de/schildbach/wallet/ui/more/SettingsViewModel.kt
🧰 Additional context used
🧠 Learnings (12)
📓 Common learnings
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1425
File: wallet/src/de/schildbach/wallet/ui/more/SettingsScreen.kt:122-151
Timestamp: 2025-09-05T06:47:44.508Z
Learning: HashEngineering prefers to defer UI edge case fixes like the CoinJoin finishing state display issue (showing "Finishing ...%" when mode is OFF but status is FINISHING) to later PRs when the main focus is on redesigning the settings screen.
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1422
File: common/src/main/java/org/dash/wallet/common/util/Constants.kt:68-69
Timestamp: 2025-08-25T15:00:20.777Z
Learning: HashEngineering prefers to keep HTTP logging enabled in release mode (using log.info instead of gating with BuildConfig.DEBUG) to debug production errors, even though this may leak URLs in production logs. This is a deliberate trade-off for debugging purposes in the Dash wallet project.
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1410
File: wallet/src/de/schildbach/wallet/data/InvitationLinkData.kt:79-84
Timestamp: 2025-07-12T07:12:04.769Z
Learning: HashEngineering prefers to handle defensive validation improvements for URI parsing in follow-up PRs rather than including them in the current PR when the main focus is on replacing Firebase with AppsFlyer.
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1450
File: docs/security/FALLBACK_TESTING_GUIDE.md:408-428
Timestamp: 2026-01-12T20:56:34.250Z
Learning: HashEngineering prefers to place testing utilities like FallbackTestingUtils in debug-only source sets rather than using ProGuard rules to strip them from release builds. This ensures test code is never compiled into production builds by the build system itself.
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1423
File: wallet/src/de/schildbach/wallet/ui/EditProfileActivity.kt:418-446
Timestamp: 2025-08-25T14:48:39.247Z
Learning: HashEngineering prefers to refactor and reuse topup code for balance validation logic improvements in DashPay activities like EditProfileActivity, rather than implementing individual fixes in the current PR.
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1422
File: wallet/src/de/schildbach/wallet/data/PaymentIntent.java:476-481
Timestamp: 2025-08-25T15:04:18.461Z
Learning: HashEngineering prefers to keep payeeUsername and payeeUserId in PaymentIntent.toString() logs unredacted for debugging purposes, prioritizing troubleshooting capability over PII protection concerns in the Dash wallet project.
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1425
File: common/src/main/java/org/dash/wallet/common/ui/components/DashButton.kt:94-101
Timestamp: 2025-09-05T06:00:25.088Z
Learning: HashEngineering prefers to fix the DashButton alpha dimming issue (using explicit disabled colors instead of overall alpha) in a follow-up PR rather than including it in the current settings screen redesign PR.
📚 Learning: 2025-09-23T05:19:47.861Z
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1428
File: wallet/src/de/schildbach/wallet/ui/dashpay/PlatformRepo.kt:1065-1068
Timestamp: 2025-09-23T05:19:47.861Z
Learning: The Dash wallet app doesn't use non-encrypted wallets, so getWalletEncryptionKey() will never return null in practice. Defensive null-safe programming with ?.let is appropriate for methods like getIdentityFromPublicKeyId() in PlatformRepo.kt.
Applied to files:
wallet/src/de/schildbach/wallet/ui/SettingsFragment.ktcommon/src/main/java/org/dash/wallet/common/data/BlockchainServiceConfig.kt
📚 Learning: 2025-08-08T13:29:37.702Z
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1419
File: wallet/src/de/schildbach/wallet/payments/SendCoinsTaskRunner.kt:312-331
Timestamp: 2025-08-08T13:29:37.702Z
Learning: Dash Wallet: For network presence checks, if a transaction is not in the wallet, we generally won’t have broadcast peer data (likely 0) and chain-locks only arrive after the block containing the transaction; thus fallback confidence checks on the local transaction often remain false. It’s safe but usually non-effective to include them; primary detection should rely on the wallet-known transaction.
Applied to files:
wallet/src/de/schildbach/wallet/ui/SettingsFragment.kt
📚 Learning: 2025-05-12T09:14:36.656Z
Learnt from: Syn-McJ
Repo: dashpay/dash-wallet PR: 1391
File: wallet/src/de/schildbach/wallet/ui/payments/PaymentsFragment.kt:111-119
Timestamp: 2025-05-12T09:14:36.656Z
Learning: In the dashpay/dash-wallet project, explicitly unregistering ViewPager2 callbacks is not considered critical since memory leaks haven't been observed in practice with the memory profiler.
Applied to files:
wallet/src/de/schildbach/wallet/ui/SettingsFragment.kt
📚 Learning: 2025-08-25T14:48:39.247Z
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1423
File: wallet/src/de/schildbach/wallet/ui/EditProfileActivity.kt:418-446
Timestamp: 2025-08-25T14:48:39.247Z
Learning: HashEngineering prefers to refactor and reuse topup code for balance validation logic improvements in DashPay activities like EditProfileActivity, rather than implementing individual fixes in the current PR.
Applied to files:
wallet/src/de/schildbach/wallet/ui/SettingsFragment.kt
📚 Learning: 2025-09-05T06:47:44.508Z
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1425
File: wallet/src/de/schildbach/wallet/ui/more/SettingsScreen.kt:122-151
Timestamp: 2025-09-05T06:47:44.508Z
Learning: HashEngineering prefers to defer UI edge case fixes like the CoinJoin finishing state display issue (showing "Finishing ...%" when mode is OFF but status is FINISHING) to later PRs when the main focus is on redesigning the settings screen.
Applied to files:
wallet/src/de/schildbach/wallet/ui/SettingsFragment.kt
📚 Learning: 2025-08-08T16:48:49.964Z
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1417
File: features/exploredash/src/main/java/org/dash/wallet/features/exploredash/data/explore/MerchantDao.kt:0-0
Timestamp: 2025-08-08T16:48:49.964Z
Learning: In PR dashpay/dash-wallet#1417, HashEngineering chose to defer adding Room indexes for gift_card_providers (provider, denominationsType, merchantId) to a follow-up PR; do not block the current PR on this optimization. Files: features/exploredash/.../data/explore/MerchantDao.kt and features/exploredash/.../data/dashspend/GiftCardProvider.kt.
Applied to files:
wallet/src/de/schildbach/wallet/ui/SettingsFragment.kt
📚 Learning: 2025-09-05T06:00:25.088Z
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1425
File: common/src/main/java/org/dash/wallet/common/ui/components/DashButton.kt:94-101
Timestamp: 2025-09-05T06:00:25.088Z
Learning: HashEngineering prefers to fix the DashButton alpha dimming issue (using explicit disabled colors instead of overall alpha) in a follow-up PR rather than including it in the current settings screen redesign PR.
Applied to files:
wallet/src/de/schildbach/wallet/ui/SettingsFragment.kt
📚 Learning: 2025-04-19T07:01:17.535Z
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1362
File: wallet/src/de/schildbach/wallet/ui/more/SettingsViewModel.kt:107-115
Timestamp: 2025-04-19T07:01:17.535Z
Learning: In the Dash Wallet app, DashPayConfig methods like isTransactionMetadataInfoShown() already use withContext(Dispatchers.IO) internally, so wrapping these calls with another withContext(Dispatchers.IO) in ViewModels is redundant.
Applied to files:
wallet/src/de/schildbach/wallet/ui/SettingsFragment.kt
📚 Learning: 2025-08-25T15:00:20.777Z
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1422
File: common/src/main/java/org/dash/wallet/common/util/Constants.kt:68-69
Timestamp: 2025-08-25T15:00:20.777Z
Learning: HashEngineering prefers to keep HTTP logging enabled in release mode (using log.info instead of gating with BuildConfig.DEBUG) to debug production errors, even though this may leak URLs in production logs. This is a deliberate trade-off for debugging purposes in the Dash wallet project.
Applied to files:
wallet/src/de/schildbach/wallet/ui/SettingsFragment.kt
📚 Learning: 2025-04-19T07:01:17.535Z
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1362
File: wallet/src/de/schildbach/wallet/ui/more/SettingsViewModel.kt:107-115
Timestamp: 2025-04-19T07:01:17.535Z
Learning: In Dash Wallet, DataStore operations (like those in DashPayConfig through BaseConfig) already handle background threading internally. Adding another layer of withContext(Dispatchers.IO) around these calls in ViewModels is redundant because the Jetpack DataStore API automatically dispatches its operations to a background thread.
Applied to files:
wallet/src/de/schildbach/wallet/ui/SettingsFragment.ktcommon/src/main/java/org/dash/wallet/common/data/BlockchainServiceConfig.kt
📚 Learning: 2025-09-24T22:07:25.689Z
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1429
File: features/exploredash/src/main/res/values/strings-explore-dash.xml:239-239
Timestamp: 2025-09-24T22:07:25.689Z
Learning: The Dash wallet app has custom link handling for the "contact:" URL scheme in GiftCardDetailsDialog.kt. When a link with "contact:" is clicked, a custom LinkMovementMethod intercepts it and calls contactSupport() instead of trying to open it as a standard web link. This allows for consistent support contact behavior across different services (CTX, PiggyCards, etc.) rather than using mailto: links.
Applied to files:
wallet/src/de/schildbach/wallet/ui/SettingsFragment.kt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (4)
common/src/main/java/org/dash/wallet/common/data/BlockchainServiceConfig.kt (2)
52-52: LGTM!The new preference key for wallet creation date is properly defined using
longPreferencesKey.
59-66: LGTM!The getter correctly validates the stored creation date against
EARLIEST_HD_SEED_CREATION_TIMEand returnsnullfor invalid or missing values. Based on learnings, DataStore operations handle background threading internally, so no additional dispatchers are needed.wallet/src/de/schildbach/wallet/ui/SettingsFragment.kt (2)
61-61: No action needed. The import ofjava.time.Instantis properly supported via core library desugaring enabled in the wallet module, which allows usage on minSdk 24+.Likely an incorrect or invalid review comment.
196-215: Code is correct —creationDateis consistently in epoch seconds.The
RescanBlockchainDialogFragmentconverts the DatePicker's millisecond value to seconds (selectedCalendar.timeInMillis / 1000L) before passing it to the callback. The subsequent use inSettingsFragmentwithInstant.ofEpochSecond(creationDate)and passage toviewModel.setWalletCreationDate(it)are all consistent with this expectation, as the ViewModel stores the value ascreationTimeSeconds. No changes needed.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @wallet/src/de/schildbach/wallet/service/WalletFactory.kt:
- Around line 298-301: The code checks creationTimeSeconds but mistakenly writes
seedCreationTime into the config; change the call to
blockchainServiceConfig.setWalletCreationDate(...) to pass creationTimeSeconds
(not seedCreationTime). Locate the block that uses creationTimeSeconds?.let {
... } and replace the stored value with the received creationTimeSeconds (ensure
any expected units match the config API before saving).
🧹 Nitpick comments (1)
wallet/src/de/schildbach/wallet/service/WalletFactory.kt (1)
275-284: Clarify the distinction between seed creation time and wallet creation date.The code uses
EARLIEST_HD_SEED_CREATION_TIMEfor theDeterministicSeed(line 282-283) while the user-providedcreationTimeSecondsis meant for the config. Consider adding a brief comment explaining this design choice — the wallet internally uses the earliest possible time for key derivation, while the config stores the user-selected date for blockchain sync optimization.This aligns with the commit message "don't save the UI date in the wallet."
📝 Suggested documentation
return try { - // The wallet creation time should always be the oldest possible time + // The wallet internally uses the earliest possible time for key derivation to ensure + // all historical transactions are found. The user-selected creationTimeSeconds is stored + // separately in BlockchainServiceConfig for sync optimization purposes only. val seedCreationTime = Constants.EARLIEST_HD_SEED_CREATION_TIME val seed = DeterministicSeed(words, null, "", seedCreationTime)
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
wallet/src/de/schildbach/wallet/service/WalletFactory.ktwallet/src/de/schildbach/wallet/ui/more/SettingsViewModel.kt
🧰 Additional context used
🧠 Learnings (13)
📓 Common learnings
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1425
File: wallet/src/de/schildbach/wallet/ui/more/SettingsScreen.kt:122-151
Timestamp: 2025-09-05T06:47:44.508Z
Learning: HashEngineering prefers to defer UI edge case fixes like the CoinJoin finishing state display issue (showing "Finishing ...%" when mode is OFF but status is FINISHING) to later PRs when the main focus is on redesigning the settings screen.
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1422
File: common/src/main/java/org/dash/wallet/common/util/Constants.kt:68-69
Timestamp: 2025-08-25T15:00:20.777Z
Learning: HashEngineering prefers to keep HTTP logging enabled in release mode (using log.info instead of gating with BuildConfig.DEBUG) to debug production errors, even though this may leak URLs in production logs. This is a deliberate trade-off for debugging purposes in the Dash wallet project.
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1410
File: wallet/src/de/schildbach/wallet/data/InvitationLinkData.kt:79-84
Timestamp: 2025-07-12T07:12:04.769Z
Learning: HashEngineering prefers to handle defensive validation improvements for URI parsing in follow-up PRs rather than including them in the current PR when the main focus is on replacing Firebase with AppsFlyer.
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1450
File: docs/security/FALLBACK_TESTING_GUIDE.md:408-428
Timestamp: 2026-01-12T20:56:41.213Z
Learning: HashEngineering prefers to place testing utilities like FallbackTestingUtils in debug-only source sets rather than using ProGuard rules to strip them from release builds. This ensures test code is never compiled into production builds by the build system itself.
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1423
File: wallet/src/de/schildbach/wallet/ui/EditProfileActivity.kt:418-446
Timestamp: 2025-08-25T14:48:39.247Z
Learning: HashEngineering prefers to refactor and reuse topup code for balance validation logic improvements in DashPay activities like EditProfileActivity, rather than implementing individual fixes in the current PR.
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1422
File: wallet/src/de/schildbach/wallet/data/PaymentIntent.java:476-481
Timestamp: 2025-08-25T15:04:18.461Z
Learning: HashEngineering prefers to keep payeeUsername and payeeUserId in PaymentIntent.toString() logs unredacted for debugging purposes, prioritizing troubleshooting capability over PII protection concerns in the Dash wallet project.
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1425
File: common/src/main/java/org/dash/wallet/common/ui/components/DashButton.kt:94-101
Timestamp: 2025-09-05T06:00:25.088Z
Learning: HashEngineering prefers to fix the DashButton alpha dimming issue (using explicit disabled colors instead of overall alpha) in a follow-up PR rather than including it in the current settings screen redesign PR.
📚 Learning: 2025-09-23T05:19:47.861Z
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1428
File: wallet/src/de/schildbach/wallet/ui/dashpay/PlatformRepo.kt:1065-1068
Timestamp: 2025-09-23T05:19:47.861Z
Learning: The Dash wallet app doesn't use non-encrypted wallets, so getWalletEncryptionKey() will never return null in practice. Defensive null-safe programming with ?.let is appropriate for methods like getIdentityFromPublicKeyId() in PlatformRepo.kt.
Applied to files:
wallet/src/de/schildbach/wallet/ui/more/SettingsViewModel.ktwallet/src/de/schildbach/wallet/service/WalletFactory.kt
📚 Learning: 2025-04-19T07:01:17.535Z
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1362
File: wallet/src/de/schildbach/wallet/ui/more/SettingsViewModel.kt:107-115
Timestamp: 2025-04-19T07:01:17.535Z
Learning: In the Dash Wallet app, DashPayConfig methods like isTransactionMetadataInfoShown() already use withContext(Dispatchers.IO) internally, so wrapping these calls with another withContext(Dispatchers.IO) in ViewModels is redundant.
Applied to files:
wallet/src/de/schildbach/wallet/ui/more/SettingsViewModel.kt
📚 Learning: 2025-04-19T07:01:17.535Z
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1362
File: wallet/src/de/schildbach/wallet/ui/more/SettingsViewModel.kt:107-115
Timestamp: 2025-04-19T07:01:17.535Z
Learning: In Dash Wallet, DataStore operations (like those in DashPayConfig through BaseConfig) already handle background threading internally. Adding another layer of withContext(Dispatchers.IO) around these calls in ViewModels is redundant because the Jetpack DataStore API automatically dispatches its operations to a background thread.
Applied to files:
wallet/src/de/schildbach/wallet/ui/more/SettingsViewModel.kt
📚 Learning: 2025-08-08T16:48:49.964Z
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1417
File: features/exploredash/src/main/java/org/dash/wallet/features/exploredash/data/explore/MerchantDao.kt:0-0
Timestamp: 2025-08-08T16:48:49.964Z
Learning: In PR dashpay/dash-wallet#1417, HashEngineering chose to defer adding Room indexes for gift_card_providers (provider, denominationsType, merchantId) to a follow-up PR; do not block the current PR on this optimization. Files: features/exploredash/.../data/explore/MerchantDao.kt and features/exploredash/.../data/dashspend/GiftCardProvider.kt.
Applied to files:
wallet/src/de/schildbach/wallet/ui/more/SettingsViewModel.ktwallet/src/de/schildbach/wallet/service/WalletFactory.kt
📚 Learning: 2025-08-25T14:48:39.247Z
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1423
File: wallet/src/de/schildbach/wallet/ui/EditProfileActivity.kt:418-446
Timestamp: 2025-08-25T14:48:39.247Z
Learning: HashEngineering prefers to refactor and reuse topup code for balance validation logic improvements in DashPay activities like EditProfileActivity, rather than implementing individual fixes in the current PR.
Applied to files:
wallet/src/de/schildbach/wallet/ui/more/SettingsViewModel.kt
📚 Learning: 2025-09-05T06:47:44.508Z
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1425
File: wallet/src/de/schildbach/wallet/ui/more/SettingsScreen.kt:122-151
Timestamp: 2025-09-05T06:47:44.508Z
Learning: HashEngineering prefers to defer UI edge case fixes like the CoinJoin finishing state display issue (showing "Finishing ...%" when mode is OFF but status is FINISHING) to later PRs when the main focus is on redesigning the settings screen.
Applied to files:
wallet/src/de/schildbach/wallet/ui/more/SettingsViewModel.kt
📚 Learning: 2025-09-05T06:00:25.088Z
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1425
File: common/src/main/java/org/dash/wallet/common/ui/components/DashButton.kt:94-101
Timestamp: 2025-09-05T06:00:25.088Z
Learning: HashEngineering prefers to fix the DashButton alpha dimming issue (using explicit disabled colors instead of overall alpha) in a follow-up PR rather than including it in the current settings screen redesign PR.
Applied to files:
wallet/src/de/schildbach/wallet/ui/more/SettingsViewModel.kt
📚 Learning: 2025-05-12T09:14:36.656Z
Learnt from: Syn-McJ
Repo: dashpay/dash-wallet PR: 1391
File: wallet/src/de/schildbach/wallet/ui/payments/PaymentsFragment.kt:111-119
Timestamp: 2025-05-12T09:14:36.656Z
Learning: In the dashpay/dash-wallet project, explicitly unregistering ViewPager2 callbacks is not considered critical since memory leaks haven't been observed in practice with the memory profiler.
Applied to files:
wallet/src/de/schildbach/wallet/ui/more/SettingsViewModel.kt
📚 Learning: 2025-08-25T15:00:20.777Z
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1422
File: common/src/main/java/org/dash/wallet/common/util/Constants.kt:68-69
Timestamp: 2025-08-25T15:00:20.777Z
Learning: HashEngineering prefers to keep HTTP logging enabled in release mode (using log.info instead of gating with BuildConfig.DEBUG) to debug production errors, even though this may leak URLs in production logs. This is a deliberate trade-off for debugging purposes in the Dash wallet project.
Applied to files:
wallet/src/de/schildbach/wallet/ui/more/SettingsViewModel.kt
📚 Learning: 2025-09-24T22:07:25.689Z
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1429
File: features/exploredash/src/main/res/values/strings-explore-dash.xml:239-239
Timestamp: 2025-09-24T22:07:25.689Z
Learning: The Dash wallet app has custom link handling for the "contact:" URL scheme in GiftCardDetailsDialog.kt. When a link with "contact:" is clicked, a custom LinkMovementMethod intercepts it and calls contactSupport() instead of trying to open it as a standard web link. This allows for consistent support contact behavior across different services (CTX, PiggyCards, etc.) rather than using mailto: links.
Applied to files:
wallet/src/de/schildbach/wallet/ui/more/SettingsViewModel.kt
📚 Learning: 2025-09-05T06:16:12.649Z
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1425
File: wallet/src/de/schildbach/wallet/ui/more/SettingsViewModel.kt:134-145
Timestamp: 2025-09-05T06:16:12.649Z
Learning: The dash-wallet project has a minimum SDK version of 24, so API availability checks for methods introduced in API 23 or earlier are not needed.
Applied to files:
wallet/src/de/schildbach/wallet/ui/more/SettingsViewModel.ktwallet/src/de/schildbach/wallet/service/WalletFactory.kt
📚 Learning: 2025-04-16T17:07:27.359Z
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1377
File: wallet/src/de/schildbach/wallet/service/platform/PlatformService.kt:0-0
Timestamp: 2025-04-16T17:07:27.359Z
Learning: For PlatformService in the Dash Wallet, the implementation should avoid throwing exceptions when Constants.SUPPORTS_PLATFORM is false. The platform should only be initialized when SUPPORTS_PLATFORM is true, and initialization should be done lazily to defer Platform creation until needed.
Applied to files:
wallet/src/de/schildbach/wallet/service/WalletFactory.kt
🧬 Code graph analysis (1)
wallet/src/de/schildbach/wallet/service/WalletFactory.kt (1)
wallet/src/de/schildbach/wallet/ui/RestoreWalletFromSeedViewModel.kt (1)
restoreWalletFromSeed(88-102)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (5)
wallet/src/de/schildbach/wallet/ui/more/SettingsViewModel.kt (2)
42-42: LGTM: BlockchainServiceConfig dependency injection.The new dependency is correctly injected via Hilt constructor injection, following the existing pattern for other config dependencies in this ViewModel.
Also applies to: 75-75
205-210: LGTM: Wallet creation date accessors.The new suspend functions correctly delegate to
blockchainServiceConfig. Based on learnings, DataStore operations in this codebase already handle background threading internally, so no additionalwithContext(Dispatchers.IO)wrapping is needed.wallet/src/de/schildbach/wallet/service/WalletFactory.kt (3)
59-62: LGTM: Interface updated to suspend function with optional creation time.The interface change correctly adds the
suspendmodifier and optionalcreationTimeSecondsparameter, aligning with the asynchronous restoration flow.
72-75: LGTM: BlockchainServiceConfig dependency added.Constructor correctly accepts the new dependency for persisting wallet creation date.
123-125: Verify:creationTimeSecondsparameter is correctly propagated.The override correctly delegates to
restoreWalletFromSeedwith all parameters. However, ensure the fix for storingcreationTimeSeconds(notseedCreationTime) is applied in the private implementation.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @wallet/src/de/schildbach/wallet/service/WalletFactory.kt:
- Around line 59-62: The test calling the suspend function restoreFromSeed is
missing a coroutine context; modify the failing test in WalletFactoryTest (the
test at the location that invokes restoreFromSeed) to run inside a coroutine
builder such as kotlinx.coroutines.test.runTest { ... } or
kotlinx.coroutines.runBlocking { ... }, moving the restoreFromSeed(...)
invocation into that lambda and importing the chosen builder so the suspend call
executes correctly.
🧹 Nitpick comments (1)
wallet/src/de/schildbach/wallet/service/WalletFactory.kt (1)
275-305: Remove emptyfinallyblock and consider error handling.The
try-finallywith an emptyfinallyclause serves no purpose and should be cleaned up.Additionally,
blockchainServiceConfig.setWalletCreationDate(it)is called after wallet creation but before returning. If the caller fails to persist the wallet, the creation date will already be stored, potentially causing inconsistent state. Consider whether this side effect should be moved to the caller or handled atomically.♻️ Suggested cleanup
private suspend fun restoreWalletFromSeed( words: List<String>, params: NetworkParameters, creationTimeSeconds: Long? = null ): Wallet { - return try { - // The wallet creation time should always be the oldest possible time - val seedCreationTime = Constants.EARLIEST_HD_SEED_CREATION_TIME - val seed = DeterministicSeed(words, null, "", seedCreationTime) - val group = KeyChainGroup.builder(params) - .fromSeed(seed, Script.ScriptType.P2PKH) - .addChain( - DeterministicKeyChain.builder() - .seed(seed) - .accountPath(Constants.BIP44_PATH) - .build() - ) - .build() - val wallet = WalletEx(params, group) - // add extensions - addMissingExtensions(wallet) - - checkWalletValid(wallet, params) - // set the creation date here - creationTimeSeconds?.let { - blockchainServiceConfig.setWalletCreationDate(it) - } - wallet - } finally { - } + // The wallet creation time should always be the oldest possible time + val seedCreationTime = Constants.EARLIEST_HD_SEED_CREATION_TIME + val seed = DeterministicSeed(words, null, "", seedCreationTime) + val group = KeyChainGroup.builder(params) + .fromSeed(seed, Script.ScriptType.P2PKH) + .addChain( + DeterministicKeyChain.builder() + .seed(seed) + .accountPath(Constants.BIP44_PATH) + .build() + ) + .build() + val wallet = WalletEx(params, group) + // add extensions + addMissingExtensions(wallet) + + checkWalletValid(wallet, params) + // set the creation date here + creationTimeSeconds?.let { + blockchainServiceConfig.setWalletCreationDate(it) + } + return wallet }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
wallet/src/de/schildbach/wallet/service/WalletFactory.kt
🧰 Additional context used
🧠 Learnings (4)
📓 Common learnings
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1425
File: wallet/src/de/schildbach/wallet/ui/more/SettingsScreen.kt:122-151
Timestamp: 2025-09-05T06:47:44.508Z
Learning: HashEngineering prefers to defer UI edge case fixes like the CoinJoin finishing state display issue (showing "Finishing ...%" when mode is OFF but status is FINISHING) to later PRs when the main focus is on redesigning the settings screen.
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1422
File: common/src/main/java/org/dash/wallet/common/util/Constants.kt:68-69
Timestamp: 2025-08-25T15:00:20.777Z
Learning: HashEngineering prefers to keep HTTP logging enabled in release mode (using log.info instead of gating with BuildConfig.DEBUG) to debug production errors, even though this may leak URLs in production logs. This is a deliberate trade-off for debugging purposes in the Dash wallet project.
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1410
File: wallet/src/de/schildbach/wallet/data/InvitationLinkData.kt:79-84
Timestamp: 2025-07-12T07:12:04.769Z
Learning: HashEngineering prefers to handle defensive validation improvements for URI parsing in follow-up PRs rather than including them in the current PR when the main focus is on replacing Firebase with AppsFlyer.
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1450
File: docs/security/FALLBACK_TESTING_GUIDE.md:408-428
Timestamp: 2026-01-12T20:56:41.213Z
Learning: HashEngineering prefers to place testing utilities like FallbackTestingUtils in debug-only source sets rather than using ProGuard rules to strip them from release builds. This ensures test code is never compiled into production builds by the build system itself.
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1423
File: wallet/src/de/schildbach/wallet/ui/EditProfileActivity.kt:418-446
Timestamp: 2025-08-25T14:48:39.247Z
Learning: HashEngineering prefers to refactor and reuse topup code for balance validation logic improvements in DashPay activities like EditProfileActivity, rather than implementing individual fixes in the current PR.
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1422
File: wallet/src/de/schildbach/wallet/data/PaymentIntent.java:476-481
Timestamp: 2025-08-25T15:04:18.461Z
Learning: HashEngineering prefers to keep payeeUsername and payeeUserId in PaymentIntent.toString() logs unredacted for debugging purposes, prioritizing troubleshooting capability over PII protection concerns in the Dash wallet project.
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1425
File: common/src/main/java/org/dash/wallet/common/ui/components/DashButton.kt:94-101
Timestamp: 2025-09-05T06:00:25.088Z
Learning: HashEngineering prefers to fix the DashButton alpha dimming issue (using explicit disabled colors instead of overall alpha) in a follow-up PR rather than including it in the current settings screen redesign PR.
📚 Learning: 2025-09-23T05:19:47.861Z
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1428
File: wallet/src/de/schildbach/wallet/ui/dashpay/PlatformRepo.kt:1065-1068
Timestamp: 2025-09-23T05:19:47.861Z
Learning: The Dash wallet app doesn't use non-encrypted wallets, so getWalletEncryptionKey() will never return null in practice. Defensive null-safe programming with ?.let is appropriate for methods like getIdentityFromPublicKeyId() in PlatformRepo.kt.
Applied to files:
wallet/src/de/schildbach/wallet/service/WalletFactory.kt
📚 Learning: 2025-04-16T17:07:27.359Z
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1377
File: wallet/src/de/schildbach/wallet/service/platform/PlatformService.kt:0-0
Timestamp: 2025-04-16T17:07:27.359Z
Learning: For PlatformService in the Dash Wallet, the implementation should avoid throwing exceptions when Constants.SUPPORTS_PLATFORM is false. The platform should only be initialized when SUPPORTS_PLATFORM is true, and initialization should be done lazily to defer Platform creation until needed.
Applied to files:
wallet/src/de/schildbach/wallet/service/WalletFactory.kt
📚 Learning: 2025-09-05T06:16:12.649Z
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1425
File: wallet/src/de/schildbach/wallet/ui/more/SettingsViewModel.kt:134-145
Timestamp: 2025-09-05T06:16:12.649Z
Learning: The dash-wallet project has a minimum SDK version of 24, so API availability checks for methods introduced in API 23 or earlier are not needed.
Applied to files:
wallet/src/de/schildbach/wallet/service/WalletFactory.kt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (3)
wallet/src/de/schildbach/wallet/service/WalletFactory.kt (3)
72-75: LGTM!Constructor injection of
BlockchainServiceConfigfollows the established DI pattern with Hilt.
123-125: LGTM!Clean delegation to the private implementation method.
281-301: Good design: separating seed time from user-provided creation date.The approach of using
EARLIEST_HD_SEED_CREATION_TIMEfor the seed while storing the user-providedcreationTimeSecondsinblockchainServiceConfigis correct. This ensures:
- Full blockchain scanning coverage from the earliest HD wallet time
- User's date preference is persisted separately for sync/UI purposes
This aligns with the PR objective to "not save the UI date in the wallet."
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue being fixed or feature implemented
Related PR's and Dependencies
Screenshots / Videos
How Has This Been Tested?
Checklist:
Summary by CodeRabbit
New Features
User-facing Changes
Bug Fixes
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.