fix: tx tracking #1454
fix: tx tracking #1454
Conversation
📝 WalkthroughWalkthroughAdd monitoring for older (including coinbase) transactions and integrate it into wallet lifecycle; remove chainTip/chainHeight from transaction status/display APIs in favor of chainLockBlockHeight/confidence.depthInBlocks; branch OP_RETURN rendering by transaction version/type; minor layout, utility, resource, and build updates. Changes
Sequence Diagram(s)sequenceDiagram
participant Wallet as "Wallet"
participant BlockchainService as "BlockchainService"
participant WalletObserver as "WalletObserver"
participant TxConfidenceListener as "TxConfidenceListener"
Wallet->>BlockchainService: onCreate / wallet ready
BlockchainService->>WalletObserver: monitorOlderTransactions(wallet)
WalletObserver->>Wallet: getTransactions(true)
Wallet-->>WalletObserver: returns transactions
loop per candidate tx
WalletObserver->>TxConfidenceListener: register listener for tx
TxConfidenceListener-->>WalletObserver: confidence change events
alt reaches chainlock or depth threshold (coinbase uses spendableCoinbaseDepth)
WalletObserver->>BlockchainService: stop monitoring tx (detach listener)
BlockchainService-->>WalletObserver: cleanup tracking
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Fix all issues with AI agents
In `@wallet/res/layout/transaction_result_content.xml`:
- Around line 217-224: The LinearLayout with id
transaction_output_op_returns_container has an invalid constraint
app:layout_constraintStart_toEndOf="parent" causing it to be positioned
offscreen; change that constraint to
app:layout_constraintStart_toStartOf="parent" (or remove the Start constraint
and ensure Start_toStartOf and End_toEndOf both reference "parent") so the view
correctly spans the parent's width and is visible.
In `@wallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt`:
- Around line 519-524: In TransactionConfidence.Listener.ChangeReason.DEPTH
inside BlockchainServiceImpl, make the depth logic consistent with
WalletObserver by replacing the hardcoded 100 with
wallet.params.spendableCoinbaseDepth when computing requiredDepth, and change
the comparison to include the +1 offset on the left side (use
wallet.lastBlockSeenHeight + 1 >= transactionConfidence.appearedAtChainHeight +
requiredDepth) so lastBlockSeenHeight, appearedAtChainHeight, requiredDepth and
CONFIRMED_DEPTH behave identically to WalletObserver.
- Around line 544-561: The comment at
TransactionConfidence.ConfidenceType.PENDING in maybeAddMonitoring is
misleading: either remove the stray comment `//!confidence.isTransactionLocked`
or reinstate the intended logic by changing the pending branch to check
`!confidence.isTransactionLocked` before setting shouldMonitor; additionally,
when registering the listener call
`confidence.addEventListener(transactionConfidenceListener)` include the
threading parameter used elsewhere (Threading.USER_THREAD) so it becomes
`confidence.addEventListener(transactionConfidenceListener,
Threading.USER_THREAD)` to match other registrations and ensure correct thread
handling while retaining adding to oldTransactionsToMonitor and calling
wallet.addManualNotifyConfidenceChangeTransaction(tx).
In `@wallet/src/de/schildbach/wallet/transactions/WalletObserver.kt`:
- Line 191: The call to add the transaction confidence listener is missing the
Threading.USER_THREAD executor; update the registration at
confidence.addEventListener(transactionConfidenceListener) in WalletObserver (or
the surrounding method) to pass Threading.USER_THREAD as the executor, matching
the other registrations (lines where addEventListener is called with
Threading.USER_THREAD) so callbacks run on the user thread.
- Around line 176-194: The loop that iterates wallet.getTransactions(true)
registers transactionConfidenceListener unconditionally, but
transactionConfidenceListener is only set when observeTxConfidence is true,
causing a null registration; fix by guarding the whole listener registration
block (the if (shouldMonitor) branch that calls
confidence.addEventListener(transactionConfidenceListener) and
wallet.addManualNotifyConfidenceChangeTransaction(tx)) with the
observeTxConfidence check or skip calling confidence.addEventListener when
transactionConfidenceListener is null; update WalletObserver.kt to reference
observeTxConfidence and transactionConfidenceListener (the listener
initialization at line ~137) so listeners are only added when
observeTxConfidence is true.
In
`@wallet/src/de/schildbach/wallet/ui/transactions/TransactionDetailsDialogFragment.kt`:
- Line 148: The force-unwrap of viewModel.transaction.value in the call to
transactionResultViewBinder.setSentToReturn can NPE if topUpWork fires before
the transaction is available; change this to a null-safe pattern (e.g., check
viewModel.transaction.value for null or use the Kotlin safe-call/let pattern)
and only call transactionResultViewBinder.setSentToReturn when the transaction
is non-null, passing viewModel.topUpError and viewModel.topUpComplete unchanged;
if transaction is null, skip the call or handle the absent-transaction case
consistently with the existing pattern used around lines 162-168.
In
`@wallet/src/de/schildbach/wallet/ui/transactions/TransactionResultActivity.kt`:
- Line 215: The call to transactionResultViewBinder.setSentToReturn uses
viewModel.transaction.value directly and can NPE; guard against a null
transaction by using a safe-call or null-check before calling setSentToReturn
(e.g., val tx = viewModel.transaction.value ?: return or
viewModel.transaction.value?.let {
transactionResultViewBinder.setSentToReturn(it, viewModel.topUpError,
viewModel.topUpComplete) }) so only non-null transaction objects are passed to
setSentToReturn.
- Line 201: Replace the unsafe non-null assertion on viewModel.transaction.value
in the call to transactionResultViewBinder.setSentToReturn with a null-safe
guard: check viewModel.transaction.value for null (or use a let/if-null return)
and only call
transactionResultViewBinder.setSentToReturn(viewModel.transaction.value,
viewModel.topUpError, viewModel.topUpComplete) when the transaction is present;
otherwise return early or defer the observer until the transaction is loaded to
avoid the potential NPE.
In
`@wallet/src/de/schildbach/wallet/ui/transactions/TransactionResultViewBinder.kt`:
- Around line 403-422: In TransactionResultViewBinder inside the
outputOpReturns.forEach lambda (handling Transaction.SPECIAL_VERSION and
TRANSACTION_ASSET_LOCK), avoid adding an empty TextView: compute the text for
addressView first and only inflate/add the view to
binding.transactionOutputOpReturnsContainer when the text is non-empty (e.g.,
check text.isNotEmpty()). This ensures you only create/add the addressView when
it will display meaningful content.
🧹 Nitpick comments (4)
common/src/main/java/org/dash/wallet/common/data/entity/BlockchainState.kt (1)
23-24: Good documentation of known gaps.These TODOs clearly document technical debt related to blockchain state updates. Consider creating GitHub issues to track these items for better visibility and prioritization, especially since they may affect transaction tracking reliability.
Would you like me to help draft issue descriptions for these two items?
wallet/res/layout/transaction_result_content.xml (1)
200-225: Consider layout behavior withmatch_parentin ConstraintLayout.When using
android:layout_width="match_parent"inside a ConstraintLayout, you should typically use0dp(match_constraint) instead and rely on the start/end constraints. Usingmatch_parentcan lead to unexpected behavior as it ignores constraints.However, since the inner LinearLayouts have proper constraints defined, this may work as intended. Just verify the rendering is correct.
wallet/src/de/schildbach/wallet/ui/transactions/TransactionResultViewBinder.kt (1)
424-432: Consider string resource for "OP RETURN" and guard against duplicates.
- The hardcoded
"OP RETURN"string should ideally use a string resource for consistency and potential localization.- This block adds a view for each item in
outputOpReturns, but always displays"OP RETURN"regardless of the item's value. IfoutputOpReturnscontains multiple entries, multiple identical views will be added.Proposed fix
Transaction.TIMELOCK_VERSION, 1 -> { + if (it == "OP RETURN") { val addressView = inflater.inflate( R.layout.transaction_result_address_row, binding.transactionOutputOpReturnsContainer, false ) as TextView - addressView.text = "OP RETURN" + addressView.text = it // or use a string resource binding.transactionOutputOpReturnsContainer.addView(addressView) - + } }wallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt (1)
497-507: Consider null-safe wallet access in cleanup.The
application.wallet!!force unwrap at lines 502-503 could throw NPE if the wallet is null during shutdown. While unlikely, defensive coding would be safer here.💡 Suggested improvement
private fun stopMonitoringOlderTransactions() { + val wallet = application.wallet ?: return oldTransactionsToMonitor.forEach { (_, tx) -> - tx.getConfidence(application.wallet!!.context).removeEventListener(transactionConfidenceListener) - application.wallet!!.removeManualNotifyConfidenceChangeTransaction(tx) + tx.getConfidence(wallet.context).removeEventListener(transactionConfidenceListener) + wallet.removeManualNotifyConfidenceChangeTransaction(tx) } log.info("stop monitoring {} old transactions", oldTransactionsToMonitor.size) oldTransactionsToMonitor.clear() }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
common/src/main/java/org/dash/wallet/common/data/entity/BlockchainState.ktcommon/src/main/java/org/dash/wallet/common/transactions/TransactionUtils.ktwallet/res/layout/transaction_result_content.xmlwallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.ktwallet/src/de/schildbach/wallet/transactions/WalletObserver.ktwallet/src/de/schildbach/wallet/ui/dashpay/notification/TransactionViewHolder.ktwallet/src/de/schildbach/wallet/ui/main/MainViewModel.ktwallet/src/de/schildbach/wallet/ui/transactions/TransactionDetailsDialogFragment.ktwallet/src/de/schildbach/wallet/ui/transactions/TransactionGroupViewModel.ktwallet/src/de/schildbach/wallet/ui/transactions/TransactionResultActivity.ktwallet/src/de/schildbach/wallet/ui/transactions/TransactionResultViewBinder.ktwallet/src/de/schildbach/wallet/ui/transactions/TransactionRowView.ktwallet/src/de/schildbach/wallet/ui/transactions/TxResourceMapper.ktwallet/staging/res/values/values.xml
💤 Files with no reviewable changes (2)
- wallet/src/de/schildbach/wallet/ui/dashpay/notification/TransactionViewHolder.kt
- wallet/src/de/schildbach/wallet/ui/transactions/TransactionGroupViewModel.kt
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
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: 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: 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: 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-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/service/BlockchainServiceImpl.ktwallet/src/de/schildbach/wallet/transactions/WalletObserver.ktwallet/src/de/schildbach/wallet/ui/transactions/TransactionRowView.ktwallet/src/de/schildbach/wallet/ui/transactions/TxResourceMapper.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/service/BlockchainServiceImpl.ktwallet/src/de/schildbach/wallet/ui/transactions/TransactionResultActivity.ktwallet/src/de/schildbach/wallet/ui/transactions/TransactionRowView.ktwallet/src/de/schildbach/wallet/ui/transactions/TransactionDetailsDialogFragment.ktwallet/src/de/schildbach/wallet/ui/main/MainViewModel.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/service/BlockchainServiceImpl.kt
📚 Learning: 2025-08-25T15:02:39.634Z
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1422
File: features/exploredash/src/main/java/org/dash/wallet/features/exploredash/ui/ctxspend/dialogs/GiftCardDetailsViewModel.kt:154-158
Timestamp: 2025-08-25T15:02:39.634Z
Learning: The CTXSpend getGiftCardByTxid API in CTXSpendRepository guarantees a non-null GiftCardResponse payload when returning ResponseResource.Success, so using response.value!! is safe and doesn't risk NPE despite the nullable type signature.
Applied to files:
wallet/src/de/schildbach/wallet/ui/transactions/TransactionResultActivity.ktwallet/src/de/schildbach/wallet/ui/transactions/TransactionDetailsDialogFragment.kt
📚 Learning: 2025-05-06T15:46:59.440Z
Learnt from: Syn-McJ
Repo: dashpay/dash-wallet PR: 1386
File: wallet/src/de/schildbach/wallet/ui/send/SendCoinsViewModel.kt:108-0
Timestamp: 2025-05-06T15:46:59.440Z
Learning: In UI code where frequent updates to a value might trigger expensive operations (like the SendCoinsViewModel's `executeDryrun` method), it's preferable to use a Flow with debounce rather than launching a new coroutine for each update. This prevents race conditions, reduces resource usage, and provides a more reactive architecture.
Applied to files:
wallet/src/de/schildbach/wallet/ui/transactions/TransactionResultActivity.ktwallet/src/de/schildbach/wallet/ui/transactions/TransactionDetailsDialogFragment.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/staging/res/values/values.xml
⏰ 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 (21)
common/src/main/java/org/dash/wallet/common/transactions/TransactionUtils.kt (1)
100-119: LGTM! Good defensive fix for coinbase transactions.The coinbase guard correctly prevents processing coinbase transaction outputs, which is appropriate since coinbase transactions are not "sent" by the wallet user. Moving the try-catch inside the loop improves resilience—a
ScriptExceptionon one output no longer prevents processing subsequent outputs.wallet/staging/res/values/values.xml (1)
21-24: Remove the inaccurate change description. This file was newly created in this commit, not modified. The URL has never contained the:3002port, so there is no port removal to review. The final state shown is the actual and only state of this file.Likely an incorrect or invalid review comment.
wallet/src/de/schildbach/wallet/ui/transactions/TransactionRowView.kt (3)
105-106: LGTM!The update to pass only
chainLockBlockHeighttofromTransactionaligns with the broader refactor removingchainTipBlockHeightfrom transaction status calculations.
109-117: LGTM!The simplified signature removing
chainTipBlockHeightis consistent with the PR's goal of consolidating onchainLockBlockHeightfor status determination.
153-157: LGTM!The status string retrieval now correctly uses only
chainLockBlockHeight, matching the updatedTxResourceMapper.getReceivedStatusStringsignature.wallet/src/de/schildbach/wallet/ui/transactions/TxResourceMapper.kt (1)
156-178: LGTM!The refactor to use
confidence.depthInBlocksinstead of manually calculatingchainTipBlockHeight - appearedAtChainHeightis a cleaner approach. ThedepthInBlocksproperty is the standard bitcoinj mechanism for obtaining confirmation count, making this change both correct and more idiomatic.wallet/src/de/schildbach/wallet/ui/main/MainViewModel.kt (4)
344-346: LGTM!The added logging provides useful visibility into blockchain state during normal operation (non-replaying). This will help with debugging transaction tracking issues. The conditional
!state.replayingappropriately avoids log spam during blockchain rescan.
526-533: LGTM!The
TransactionRowView.fromTransactionWrappercall now correctly passes onlychainLockBlockHeight, aligning with the updated API signature.
600-619: LGTM!Both
TransactionRowView.fromTransactionandfromTransactionWrappercalls in the batch refresh are updated to use onlychainLockBlockHeight, maintaining consistency with the API changes throughout the PR.
720-728: LGTM!The metadata update path also correctly uses
chainLockBlockHeightfor constructing updated row views.wallet/src/de/schildbach/wallet/ui/transactions/TransactionDetailsDialogFragment.kt (1)
162-168: LGTM!The null-safe
?.let { }pattern correctly guards against NPE before callingsetSentToReturn.wallet/src/de/schildbach/wallet/ui/transactions/TransactionResultViewBinder.kt (3)
163-163: LGTM!The updated call correctly passes the transaction object with appropriate default values for the initial bind.
388-388: LGTM!Setting the address container visibility based on the presence of addresses is a sensible addition.
438-441: LGTM!The method correctly clears existing views before delegating to
setReturns. Note that it relies onoutputAddressesandoutputAssetLocksbeing populated by a priorbind()call, which appears to be the intended usage pattern based on the call sites.wallet/src/de/schildbach/wallet/transactions/WalletObserver.kt (2)
100-104: LGTM - Coinbase transaction monitoring logic is correct.The condition properly ensures coinbase transactions are always monitored regardless of the one-hour time window, which is necessary since coinbase transactions require many more confirmations before becoming spendable.
Also applies to: 121-126
148-158: LGTM - Confidence listener logic correctly handles coinbase transactions.The logic properly:
- Prevents coinbase transactions from stopping monitoring on chain locks or instant send locks
- Uses the network's
spendableCoinbaseDepthparameter for coinbase maturity requirements- Uses
CONFIRMED_DEPTHfor regular transactionswallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt (5)
698-707: LGTM - Transaction depth updates on download events.Calling
updateTransactionDepth()when headers or chain download starts ensures transaction confidence information is refreshed appropriately during sync operations.
762-765: LGTM - Transaction depth update on sync completion.Updating transaction depth when download completes ensures all transaction confidence information is current after sync.
1554-1556: LGTM - Initialization sequence for transaction monitoring.The initialization correctly:
- Propagates the bitcoinj context
- Updates transaction depth before starting monitoring
- Sets up monitoring for existing transactions after wallet listeners are registered
1852-1853: LGTM - Proper cleanup of transaction monitoring.The shutdown sequence correctly updates transaction depth one final time and removes all monitoring listeners to prevent memory leaks.
389-389: LGTM - Transaction monitoring integrated into wallet events.Adding
maybeAddMonitoringcalls in bothonCoinsReceivedandonCoinsSentensures new transactions are properly tracked for confidence changes.Also applies to: 411-411
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
wallet/src/de/schildbach/wallet/ui/transactions/TransactionDetailsDialogFragment.kt
Outdated
Show resolved
Hide resolved
wallet/src/de/schildbach/wallet/ui/transactions/TransactionResultActivity.kt
Outdated
Show resolved
Hide resolved
wallet/src/de/schildbach/wallet/ui/transactions/TransactionResultActivity.kt
Outdated
Show resolved
Hide resolved
wallet/src/de/schildbach/wallet/ui/transactions/TransactionResultViewBinder.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@wallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt`:
- Around line 500-507: stopMonitoringOlderTransactions() currently dereferences
application.wallet!! and can NPE during cleanup; guard against a null wallet by
capturing application.wallet into a local val (e.g., val wallet =
application.wallet) and if it's null either return early or skip the
wallet-specific calls per transaction, then use wallet.context and
wallet.removeManualNotifyConfidenceChangeTransaction(tx) instead of
application.wallet!!. Also ensure transactionConfidenceListener is still removed
only when a non-null Confidence object exists and keep the log message
consistent (use the original oldTransactionsToMonitor.size or the filtered count
if you skip entries).
- Around line 497-498: maybeAddMonitoring() can register a null
transactionConfidenceListener if onCoinsReceived/onCoinsSent fire before
monitorOlderTransactions() sets it; update maybeAddMonitoring() to guard uses of
transactionConfidenceListener (return early if null) or eagerly initialize
transactionConfidenceListener when BlockchainServiceImpl is constructed/started;
specifically, locate maybeAddMonitoring(), monitorOlderTransactions(), and the
transactionConfidenceListener field and either (a) add a null-check around any
registration/callback invocation that references transactionConfidenceListener
(and skip/queue adding to oldTransactionsToMonitor until listener exists) or (b)
move listener creation into the class init/start path so
transactionConfidenceListener is non-null before onCoinsReceived/onCoinsSent can
call maybeAddMonitoring().
In
`@wallet/src/de/schildbach/wallet/ui/transactions/TransactionResultViewBinder.kt`:
- Around line 408-417: The method setReturns declares an unused parameter
transoutputAddresses while the body uses the class member outputAddresses;
either use the parameter or remove it: if the intent was to evaluate the passed
addresses, replace references to outputAddresses inside setReturns (e.g., the
expression setting binding.outputsContainer.isVisible) with transoutputAddresses
(also fix the typo by renaming transoutputAddresses → outputAddressesParam or
outputAddresses), otherwise remove the transoutputAddresses parameter from
setReturns signature and any callers, keeping the method logic as-is; ensure
parameter name and usage are consistent across callers if you choose to keep it.
♻️ Duplicate comments (3)
wallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt (2)
519-528: Depth calculation still inconsistent with WalletObserver.The current code at line 527 uses:
wallet.lastBlockSeenHeight >= transactionConfidence.appearedAtChainHeight + requiredDepthThe past review noted that
WalletObserveruseslastBlockSeenHeight + 1in its comparison. While the hardcoded100has been fixed to usewallet.params.spendableCoinbaseDepth, the off-by-one difference in the comparison remains.🐛 Proposed fix for consistency
TransactionConfidence.Listener.ChangeReason.DEPTH -> { log.info("tx depth {} for {}", transactionConfidence.depthInBlocks, transactionConfidence.transactionHash) val requiredDepth = if (tx?.isCoinBase == true) { wallet.params.spendableCoinbaseDepth } else { CONFIRMED_DEPTH } - wallet.lastBlockSeenHeight >= transactionConfidence.appearedAtChainHeight + requiredDepth + wallet.lastBlockSeenHeight + 1 >= transactionConfidence.appearedAtChainHeight + requiredDepth }
548-565: Missing threading parameter and inconsistent depth constant.
Line 555: Hardcoded
6should useCONFIRMED_DEPTHfor consistency with the rest of the code.Line 562: Missing
Threading.USER_THREADparameter for consistency with other listener registrations in this codebase (e.g., lines 1553-1557 useThreading.SAME_THREAD).🔧 Proposed fix
private fun maybeAddMonitoring(tx: Transaction, wallet: Wallet) { val confidence = tx.getConfidence(wallet.context) val shouldMonitor = when (confidence.confidenceType) { TransactionConfidence.ConfidenceType.PENDING -> true TransactionConfidence.ConfidenceType.BUILDING -> when { tx.isCoinBase -> confidence.depthInBlocks < wallet.params.spendableCoinbaseDepth confidence.isChainLocked -> false - else -> confidence.depthInBlocks < 6 + else -> confidence.depthInBlocks < CONFIRMED_DEPTH } else -> false } if (shouldMonitor) { oldTransactionsToMonitor[tx.txId] = tx - confidence.addEventListener(transactionConfidenceListener) + confidence.addEventListener(Threading.USER_THREAD, transactionConfidenceListener) wallet.addManualNotifyConfidenceChangeTransaction(tx) } }wallet/src/de/schildbach/wallet/ui/transactions/TransactionResultViewBinder.kt (1)
442-451: Potential issue: Always adds "OP RETURN" regardless of item content.For
TIMELOCK_VERSIONand version1, the code adds a TextView with hardcoded "OP RETURN" text for every item inoutputOpReturns, regardless of the actual content ofit. IfoutputOpReturnscontains multiple items or items that aren't "OP RETURN", this will create misleading UI.Consider adding a check similar to the
SPECIAL_VERSIONbranch:🔧 Suggested fix
Transaction.TIMELOCK_VERSION, 1 -> { + if (it == "OP RETURN") { val addressView = inflater.inflate( R.layout.transaction_result_address_row, binding.transactionOutputOpReturnsContainer, false ) as TextView addressView.text = "OP RETURN" binding.transactionOutputOpReturnsContainer.addView(addressView) + } }
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
wallet/res/layout/transaction_result_content.xmlwallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.ktwallet/src/de/schildbach/wallet/transactions/WalletObserver.ktwallet/src/de/schildbach/wallet/ui/transactions/TransactionDetailsDialogFragment.ktwallet/src/de/schildbach/wallet/ui/transactions/TransactionResultActivity.ktwallet/src/de/schildbach/wallet/ui/transactions/TransactionResultViewBinder.kt
🚧 Files skipped from review as they are similar to previous changes (3)
- wallet/res/layout/transaction_result_content.xml
- wallet/src/de/schildbach/wallet/ui/transactions/TransactionDetailsDialogFragment.kt
- wallet/src/de/schildbach/wallet/transactions/WalletObserver.kt
🧰 Additional context used
🧠 Learnings (8)
📓 Common learnings
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: 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: 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: 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: 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-08-25T15:02:39.634Z
Learnt from: HashEngineering
Repo: dashpay/dash-wallet PR: 1422
File: features/exploredash/src/main/java/org/dash/wallet/features/exploredash/ui/ctxspend/dialogs/GiftCardDetailsViewModel.kt:154-158
Timestamp: 2025-08-25T15:02:39.634Z
Learning: The CTXSpend getGiftCardByTxid API in CTXSpendRepository guarantees a non-null GiftCardResponse payload when returning ResponseResource.Success, so using response.value!! is safe and doesn't risk NPE despite the nullable type signature.
Applied to files:
wallet/src/de/schildbach/wallet/ui/transactions/TransactionResultActivity.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/transactions/TransactionResultActivity.ktwallet/src/de/schildbach/wallet/ui/transactions/TransactionResultViewBinder.ktwallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt
📚 Learning: 2025-05-06T15:46:59.440Z
Learnt from: Syn-McJ
Repo: dashpay/dash-wallet PR: 1386
File: wallet/src/de/schildbach/wallet/ui/send/SendCoinsViewModel.kt:108-0
Timestamp: 2025-05-06T15:46:59.440Z
Learning: In UI code where frequent updates to a value might trigger expensive operations (like the SendCoinsViewModel's `executeDryrun` method), it's preferable to use a Flow with debounce rather than launching a new coroutine for each update. This prevents race conditions, reduces resource usage, and provides a more reactive architecture.
Applied to files:
wallet/src/de/schildbach/wallet/ui/transactions/TransactionResultActivity.kt
📚 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/transactions/TransactionResultActivity.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/transactions/TransactionResultActivity.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/service/BlockchainServiceImpl.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/service/BlockchainServiceImpl.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 (13)
wallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt (7)
61-61: LGTM!Good practice to import
CONFIRMED_DEPTHfromWalletObserverto ensure consistency between the two monitoring implementations.
702-711: LGTM!Good addition to trigger
updateTransactionDepth()when headers/blocks download starts, ensuring transaction confidence is refreshed during sync phases.
764-768: LGTM!Calling
updateTransactionDepth()indoneDownload()ensures transaction depths are finalized when sync completes.
389-389: LGTM!Appropriate placement to add monitoring for newly received transactions.
411-411: LGTM!Appropriate placement to add monitoring for sent transactions.
1558-1560: LGTM!Good initialization sequence: propagate context first, then update depths, then start monitoring existing transactions.
1856-1857: LGTM!Proper cleanup: update transaction depth one final time and stop monitoring before service destruction. The wallet null check at line 1852 protects these calls.
wallet/src/de/schildbach/wallet/ui/transactions/TransactionResultActivity.kt (2)
201-206: Fallback values warrant careful consideration.The safe navigation with fallbacks addresses the previous NPE concern. However, when
viewModel.transaction.valueis null, the fallback values (SPECIAL_VERSION+TRANSACTION_ASSET_LOCK) will cause the binder to render platform credits status. Verify this is the intended behavior for edge cases where the observer fires before the transaction is loaded.Also, minor formatting issue: Line 206 has extra trailing whitespace before the closing brace.
219-224: Consistent with error handling pattern above.The implementation mirrors the ERROR branch, maintaining consistency. The same fallback value considerations apply here.
wallet/src/de/schildbach/wallet/ui/transactions/TransactionResultViewBinder.kt (4)
163-171: LGTM - Updated call site aligns with new API.The call correctly passes
tx.versionShortandtx.typeto enable version/type-aware OP RETURN rendering.
396-396: Good visibility control addition.Setting
transactionOutputAddressesContainer.isVisiblebased on address presence is appropriate.
420-440: Good fix for empty view issue.The
if (addressView.text.isNotEmpty())check at lines 437-439 addresses the previous review feedback about adding empty TextViews.
456-464: LGTM - Clean method signature update.The updated
setSentToReturnproperly clears the container before re-rendering and delegates tosetReturnswith the appropriate parameters.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
wallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt
Outdated
Show resolved
Hide resolved
wallet/src/de/schildbach/wallet/ui/transactions/TransactionResultViewBinder.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@wallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt`:
- Around line 497-506: oldTransactionsToMonitor is a plain HashMap accessed from
multiple threads (wallet callbacks, confidence listeners, cleanup), which can
race or throw ConcurrentModificationException; replace it with a thread-safe map
(e.g., ConcurrentHashMap) or guard accesses with a dedicated lock and update
usages in stopMonitoringOlderTransactions to iterate over a snapshot (e.g., copy
values to a list) before removing listeners and clearing, and ensure all places
that read/write oldTransactionsToMonitor (initialization, any add/remove sites
and stopMonitoringOlderTransactions) use the same concurrent map or
synchronization and reference transactionConfidenceListener and
removeManualNotifyConfidenceChangeTransaction(tx) unchanged.
♻️ Duplicate comments (1)
wallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt (1)
518-527: Align depth stop condition with WalletObserver’s +1 offset.The depth comparison still uses
lastBlockSeenHeight >= appearedAtChainHeight + requiredDepth, which is off by one relative to WalletObserver’slastBlockSeenHeight + 1. This can end monitoring one block early/late.🛠️ Proposed fix
- wallet.lastBlockSeenHeight >= transactionConfidence.appearedAtChainHeight + requiredDepth + wallet.lastBlockSeenHeight + 1 >= transactionConfidence.appearedAtChainHeight + requiredDepth
| private val oldTransactionsToMonitor = hashMapOf<Sha256Hash, Transaction>() | ||
| private var transactionConfidenceListener: TransactionConfidence.Listener? = null | ||
|
|
||
| private fun stopMonitoringOlderTransactions(wallet: Wallet) { | ||
| oldTransactionsToMonitor.forEach { (_, tx) -> | ||
| tx.getConfidence(wallet.context).removeEventListener(transactionConfidenceListener) | ||
| wallet.removeManualNotifyConfidenceChangeTransaction(tx) | ||
| } | ||
| log.info("stop monitoring {} old transactions", oldTransactionsToMonitor.size) | ||
| oldTransactionsToMonitor.clear() |
There was a problem hiding this comment.
Synchronize oldTransactionsToMonitor access.
This map is mutated from wallet callbacks, confidence listeners, and cleanup paths that can run on different threads. HashMap is not thread-safe and can lead to races or ConcurrentModificationException. Consider a concurrent map or explicit synchronization.
🛠️ Suggested fix (ConcurrentHashMap + snapshot iteration)
+import java.util.concurrent.ConcurrentHashMap
...
- private val oldTransactionsToMonitor = hashMapOf<Sha256Hash, Transaction>()
+ private val oldTransactionsToMonitor = ConcurrentHashMap<Sha256Hash, Transaction>()
...
- oldTransactionsToMonitor.forEach { (_, tx) ->
+ oldTransactionsToMonitor.values.toList().forEach { tx ->
tx.getConfidence(wallet.context).removeEventListener(transactionConfidenceListener)
wallet.removeManualNotifyConfidenceChangeTransaction(tx)
}🤖 Prompt for AI Agents
In `@wallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt` around
lines 497 - 506, oldTransactionsToMonitor is a plain HashMap accessed from
multiple threads (wallet callbacks, confidence listeners, cleanup), which can
race or throw ConcurrentModificationException; replace it with a thread-safe map
(e.g., ConcurrentHashMap) or guard accesses with a dedicated lock and update
usages in stopMonitoringOlderTransactions to iterate over a snapshot (e.g., copy
values to a list) before removing listeners and clearing, and ensure all places
that read/write oldTransactionsToMonitor (initialization, any add/remove sites
and stopMonitoringOlderTransactions) use the same concurrent map or
synchronization and reference transactionConfidenceListener and
removeManualNotifyConfidenceChangeTransaction(tx) unchanged.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@wallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt`:
- Around line 560-563: The code is shadowing the method parameter wallet by
reassigning val wallet = application.wallet!! which can throw and bypass the
non-null parameter; instead, use the existing wallet parameter passed into the
function (remove the val wallet = application.wallet!! line) when calling
wallet.getTransactions(true) and maybeAddMonitoring(tx, wallet), or if you truly
need application.wallet, null-check it first (e.g., val appWallet =
application.wallet ?: return/handle) and reference that unique symbol
(application.wallet) explicitly rather than shadowing the parameter.
♻️ Duplicate comments (3)
wallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt (3)
497-506: MakeoldTransactionsToMonitorthread-safe.
HashMapis mutated from wallet callbacks, confidence listeners, and cleanup paths, which can race or throwConcurrentModificationException.🛠️ Proposed fix (concurrent map + snapshot iteration)
+import java.util.concurrent.ConcurrentHashMap ... - private val oldTransactionsToMonitor = hashMapOf<Sha256Hash, Transaction>() + private val oldTransactionsToMonitor = ConcurrentHashMap<Sha256Hash, Transaction>() ... - oldTransactionsToMonitor.forEach { (_, tx) -> + oldTransactionsToMonitor.values.toList().forEach { tx -> tx.getConfidence(wallet.context).removeEventListener(transactionConfidenceListener) wallet.removeManualNotifyConfidenceChangeTransaction(tx) }
522-531: Depth stop condition is still off-by-one vsWalletObserver.
This can stop monitoring at a different height than the UI/observer path.🐛 Proposed fix
- wallet.lastBlockSeenHeight >= transactionConfidence.appearedAtChainHeight + requiredDepth + wallet.lastBlockSeenHeight + 1 >= transactionConfidence.appearedAtChainHeight + requiredDepth
568-583: Guard the nullable listener and register on the user thread.
transactionConfidenceListenercan be null here, and the addEventListener overload should align with existing Threading usage.🛠️ Proposed fix
if (shouldMonitor) { - oldTransactionsToMonitor[tx.txId] = tx - confidence.addEventListener(transactionConfidenceListener) - wallet.addManualNotifyConfidenceChangeTransaction(tx) + val listener = transactionConfidenceListener ?: return + oldTransactionsToMonitor[tx.txId] = tx + confidence.addEventListener(Threading.USER_THREAD, listener) + wallet.addManualNotifyConfidenceChangeTransaction(tx) }
| val wallet = application.wallet!! | ||
| val transactions = wallet.getTransactions(true) | ||
| transactions.forEach { tx -> | ||
| maybeAddMonitoring(tx, wallet) |
There was a problem hiding this comment.
Avoid shadowing the wallet parameter with application.wallet!!.
This bypasses the non-null parameter and can still throw if application.wallet is null.
🔧 Proposed fix
- val wallet = application.wallet!!
- val transactions = wallet.getTransactions(true)
+ val transactions = wallet.getTransactions(true)🤖 Prompt for AI Agents
In `@wallet/src/de/schildbach/wallet/service/BlockchainServiceImpl.kt` around
lines 560 - 563, The code is shadowing the method parameter wallet by
reassigning val wallet = application.wallet!! which can throw and bypass the
non-null parameter; instead, use the existing wallet parameter passed into the
function (remove the val wallet = application.wallet!! line) when calling
wallet.getTransactions(true) and maybeAddMonitoring(tx, wallet), or if you truly
need application.wallet, null-check it first (e.g., val appWallet =
application.wallet ?: return/handle) and reference that unique symbol
(application.wallet) explicitly rather than shadowing the parameter.
Issue being fixed or feature implemented
Related PR's and Dependencies
Screenshots / Videos
How Has This Been Tested?
Checklist:
Summary by CodeRabbit
Bug Fixes
UI
Chores
✏️ Tip: You can customize this high-level summary in your review settings.