[ben-kaufman]

Twin: synonymdev/bitkit-ios#1116

This PR shows transaction details already held by the local Lightning wallet when restored activity has no entry in the app's transaction-details cache.

Description

• Keeps the existing cache first, then reads the matching local node for valid transaction IDs in the default wallet.
• Checks wallet index and node identity before, during and after the queued lookup so switching wallets cannot return another wallet's details.
• Performs no network lookup and does not write fallback data into the cache.
• Keeps background incoming-payment and closed-channel synchronization cache-only, so repeated syncs do not perform node-detail reads or derive address batches on cache misses. Explicit transaction-details lookup retains its guarded fallback.

Out of Scope

This does not change transaction discovery, hardware-wallet lookup or USDT recovery. It does not establish why the original cache entry was absent.

Design

N/A — no UI changes.

Preview

N/A — existing transaction details presentation is unchanged.

QA Notes

Setup

Build the dev variant with the committed dependency coordinates using just compile. Automated tests inject an absent app-cache row and a matching local node transaction; no wallet reset or funds are needed. Device reproduction additionally requires an owned disposable restored wallet with a transaction present in LDK but absent from the app details cache. That controlled mismatch fixture is not available in standard staging.

Journeys

N/A — not drivable; see Manual Tests. Creating the required cache/node mismatch is not an exposed journey capability.

Manual Tests

• 1. With the controlled restored-wallet mismatch fixture, open the existing transaction details offline: inputs/outputs and fee come from the local wallet. Repeat in a different wallet scope: no fallback data appears. The fixture requires selective local-cache setup outside normal UI capabilities.

Automated Checks

• added cases in ActivityServiceTest.kt — cache-first lookup, default-wallet local fallback, invalid IDs and hardware-wallet exclusion.
• LightningServiceTest.kt covers wallet-index mismatch and node replacement. The queued replacement regression uses a controlled dispatcher to replace the node before queued work executes and verifies neither node is queried.

Evidence

Required just compile, just test and just lint passed on macOS ARM64. The final queued-node regression at 588da10f0 was followed by complete test/lint reruns: 4,589 tests passed, zero failures or skips; complete Detekt SARIF and Android Lint XML contain zero findings. Final source hashes match the validation reports. Independent review and bounded rechecks found no remaining actionable issues.

Validation caught two test-fixture mistakes, access to a private setter and a cache mock using a different default-wallet ID. Both were repaired without weakening production wallet guards.

Hosted CI at 588da10f0 also passed. E2E run 40079920212 completed all seven shards on the first test attempt: 39 passing cases, one skipped settings case, zero failing cases. Complete logs were inspected. These standard E2E cases do not provide the controlled cache-mismatch fixture below.

October 26 review repair at signed 96b5415393a9446999447ff23e54c7006c5bc26c: a valid-txid repeated-sync regression failed before the repair with two unwanted node lookups, then passed with the private synchronization helper reading only the Core cache. Existing explicit fallback and node/wallet guards remain unchanged. Focused 42 cases and final 4,590 tests / 252 suites passed with zero failures/errors/skips; full compile, format, Detekt and Android Lint passed with zero findings. Complete logs and structured reports were inspected; JVM, cursor and Detekt-plugin diagnostics match verified earlier evidence. Two consecutive independent final reviews were clean. All four live-master-range commits are locally verified and GitHub true/valid. Latest master changes only skip two unrelated mutating Blocktank instrumented tests; the synthetic merge has not been claimed as a locally tested tree. New-head hosted checks are pending. This establishes the repeated-sync component behavior, not a measured restored-history device performance result. The equivalent iOS private-helper repair is published in #1116 at 5d1a44a6 after 2,877 selected units, full formatting and two clean final reviews.

Documentation-only final head 89ee1fe891a034e4941522a9b7b898ea725d30e9 condenses the existing changelog into the repository-required single sentence. Native source/test inputs remain byte-identical to the final gate; final formatting and two independent scoped composition reviews pass. All five live-master-range commits are locally verified and GitHub true/valid. No history rewrite or additional unchanged native-suite rerun.

Current-head hosted evidence for 89ee1fe89: E2E40384167051 completed all seven shards on the first configured attempt, 39 passing and one settings skip. Complete per-test logs, structured job/step states and artifact SHA/CRC were inspected. The tested merge9bf33d80 includes mastereb902f52 and differs from the head tree. Existing dependency/WebDriver/emulator notices are retained; their causes are not classified from a successful result. This standard suite does not execute Manual case1's controlled cache/node mismatch.

Gaps

Manual case 1 has not run because the controlled restored-wallet cache-mismatch device fixture is unavailable. The local unit tests prove the service behavior, not the complete device journey. iOS has the equivalent before/inside/after node-identity guards; its injected-lookup tests do not directly exercise a queued node replacement in LightningService.

Models used

• Planning/scoping: gpt-6-astra (reasoning: xhigh)
• Implementation: gpt-6-astra (reasoning: xhigh)
• Review: gpt-6-astra (reasoning: xhigh)

[ovitrif]

Triage: 3.1.0, with its iOS twin synonymdev/bitkit-ios#1116. It fixes restored activity history, a shipped flow, and 3.1.0 is due 28 Oct.

[greptile-apps]

Confidence Score: 4/5

[Medium impact] The change reaches transaction-detail lookups in the activity workflow.

The PR appears safe to merge; the remaining feedback is a non-blocking test gap.

Findings

1.  Queued replacement stays untested ▶

Summary

Adds a local-node fallback when transaction details are missing from the app cache.

• Transaction details can come from the local wallet when the cache has no match.

_{Reviews (1) · Last reviewed commit: "fix: read missing transaction details fr..." · Reviewed by Greptile}

[talosmachina]

No findings. On a transaction-details cache miss for the default wallet, falls back to the local LDK node's getTransactionDetails, guarded by wallet index and node identity across the LDK queue hop. Reviewed 92f8b86, light tier, reasoned from the code: per the repo's review rules this branch is not built here.

What I checked, and 5 candidates I ruled out

Read in full: the diff; ActivityService.getTransactionDetails, mapToCoreTransactionDetails, fetchTransactionDetails, processOnchainPayment, findClosedChannelForTransaction in CoreService.kt; callOnchainSend, setup, listenForEvents and the sibling walletKnowsTransaction/observedOriginalSendFee guards in LightningService.kt; ServiceQueue.kt; ActivityDetailViewModel.fetchTransactionDetails.
Call sites traced: ActivityRepo.getTransactionDetails (details screen), fetchTransactionDetails from processOnchainPayment and findClosedChannelForTransaction.
CI: build, lint and build-local still pending at review time.

Ruled out

• Queue deadlock: the fallback runs on CORE's single thread when reached from processOnchainPayment, then hops to LDK via withContext. The LDK block only touches originalNode, never CORE, so no cycle. The same CORE-to-LDK hop already exists there (listPayments, listOnchainWalletAccounts).
• Cross-wallet leak: walletId != defaultWalletId returns before the node is read, and mapToCoreTransactionDetails stamps defaultWalletId, so the mapped row's scope matches the request. CoreService.walletIndex is only ever set by init() to 0, so the index check compares against the node's currentWalletIndex set in setup; the identity re-check after the queue covers a node swap mid-lookup, which the new LightningServiceTest case exercises.
• Sync path widened past the stated scope: fetchTransactionDetails in processOnchainPayment now gets LDK details for inbound payments whose cache row is missing, so address and contact resolution run for them where they used to be skipped. That puts those rows on the same path a cached row already takes on every sync. It is a behaviour change outside the details screen, but not an incorrect one, and nothing is written back to the details cache.
• Error surface on the details screen: a NodeException from the fallback goes through callOnchainSend's getOrThrow, becomes a Result failure in ActivityRepo, and the view model shows "unavailable". That is the same UI state as the old cache-miss null, plus an error log line.
• Txid validation: the 64-hex check rejects non-txid ids (for example, the "invalid" test case) before LDK parsing. Mixed case is accepted, and the cache lookup that runs first is unchanged.

Merge confidence: 4/5, no findings, but CI had not finished and the device journey (manual case 1) is unrun.

[github-actions]

Regtest APK

Built from 89ee1fe (run).

Download bitkit-dev-debug universal APK (expires in 30 days).

[jvsena42]

Code scan: no findings.

Checked and clean:

• Recovered details can't attach to the wrong transaction: the lookup is by exact txid, and storage is scoped by network and wallet index, with the wallet-id and identity guards. Nothing from a remote or backup source is trusted, and nothing overwrites a cached value.
• v3.0.0 backs up no transaction_details, so a seed restore is exactly the state this fixes, and no persisted format changes.
• Twin synonymdev/bitkit-ios#1116 makes the same change.

Performance-relevant. The node fallback sits in the shared getTransactionDetails (CoreService.kt:551-567), so the sync path (processOnchainPayment, findClosedChannelForTransaction) now runs the LDK lookup, the address search and the contact resolution for every restored inbound payment, on every syncActivities; on master those exited at details == null. The gate measures syncActivities duration and LDK-queue operations per sync on a restored wallet, head vs master. If it shows a real cost, confining the fallback to the details screen (ActivityRepo.getTransactionDetails) would avoid it.

Device gate (partial): 588da10 — not run yet; pending a seed restore of the regtest test vector (as in onchain-receive/restore-recent-receive-stays-silent.xml), the details screen of a restored receive, and that measurement. Device results will follow in a separate review.

[piotr-iohk]

QA review

Scope: Full review of the complete PR diff against merge base 78798d2aa2fc80e614b0dffbfb7dfb8ee504a583, at 588da10.

1 actionable finding — resolve or provide an evidence-backed rebuttal.

The default-wallet cache miss used by the transaction-details screen is guarded by wallet id, txid shape, and node identity across the LDK queue. The same getter is also on the activity-sync path, and a miss is not written back, so every later sync repeats the node read and address resolution for restored inbound payments. Review on 588da10 already flagged that cost; source inspection confirms the path. Compared the shared getter with bitkit-ios#1116 at ef2f5064bdcb20637526f01b63aa340d74f57b1e only; that PR was not reviewed.

Device testing: not performed in this review. Unit tests were inspected, not executed.

Suggested additional test cases

• Android, regtest: restore a wallet whose node contains several inbound on-chain payments and whose transaction-details cache has no row for them, including one receive address with no pre-activity metadata. Sync activities twice without opening transaction details. The second sync should not derive address batches again for those historical receives. Opening one payment should still show that payment's inputs and outputs, and another wallet scope should show none.

Findings

• [MEDIUM] Activity sync repeats the uncached node fallback for every restored receive — inline at app/src/main/java/to/bitkit/services/CoreService.kt:564.

Reviewer model: grok-4.7
Reasoning effort: high