Skip to content

feat: load real transaction history in demo app - #102

Open
j-kon wants to merge 20 commits into
bitcoindevkit:mainfrom
j-kon:feat/bdk-demo-real-transaction-history
Open

feat: load real transaction history in demo app#102
j-kon wants to merge 20 commits into
bitcoindevkit:mainfrom
j-kon:feat/bdk-demo-real-transaction-history

Conversation

@j-kon

@j-kon j-kon commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Summary

  • replace transaction history sample rows with active-wallet-backed data
  • map BDK wallet transactions into demo-app history rows
  • bind transaction repositories to both the logical wallet ID and live FFI wallet
  • clear stale transaction state when the loaded wallet is missing or changes
  • refresh mounted transaction history immediately after a successful broadcast
  • run full transaction scans off the UI isolate while keeping direct lookups lightweight
  • distinguish missing transactions from invalid txids and FFI lookup failures
  • preserve existing rows and show a visible warning when a background refresh fails
  • remove the redundant manual transaction-history load and reload controls
  • show distinct no-wallet states for transaction list and detail screens
  • add provider-chain regression coverage for mounting, wallet switching, replacement, clearing, stale loads, and broadcast refresh
  • keep widget tests lightweight by using fake transaction repositories without constructing real BDK wallets

Context

Continues the transaction presentation scaffold from #62 by wiring the list/detail flow to active wallet transaction data instead of sample rows.

The production transaction repository uses the BDK Dart API to read wallet transactions. Transaction state remains auto-disposed and keyed by logical wallet ID, while an active wallet binding ensures each provider only reads from the matching FFI wallet.

Wallet replacement refreshes existing rows, wallet clearing removes stale rows, delayed results cannot cross wallet boundaries, and successful broadcasts invalidate the active wallet transaction controller. Full transaction scans run outside the UI isolate, and direct transaction lookup falls back to a scan only when the wallet reports that the transaction is missing.

Verification

  • dart format --output=none --set-exit-if-changed lib test example bdk_demo/lib bdk_demo/test
  • dart analyze --fatal-infos --fatal-warnings lib test example
  • dart test
  • flutter analyze inside bdk_demo
  • flutter test --no-pub test inside bdk_demo

Validation passed:

  • root Dart tests: 22 passed, 5 environment-gated integration tests skipped
  • bdk_demo Flutter tests: 207 passed
  • no analyzer or formatter issues

@j-kon
j-kon marked this pull request as ready for review June 30, 2026 14:35
@Shamsudeen12

Copy link
Copy Markdown

Hi @j-kon, nice work here

I noticed something. The transactionsControllerProvider isn't auto-disposed, so it outlives the TransactionsListPage and persists the data for the previous session. It gates on hasActiveWalletProvider, which is a boolean and switching from wallet A to wallet B through the Active Wallets screen (active_wallets_page.dart → activeWalletProvider.notifier.set(...)) leaves that provider at true, so the controller's build() never re-runs and the list keeps rendering wallet A's transactions under wallet B.

Steps to reproduce: load wallet A → open Transactions → Load Transaction History → switch to wallet B → reopen Transactions. It still shows wallet A's transactions; tapping one resolves against B's repository and 404s as "Transaction not found."

The sibling providers already handle this by keying off wallet identity, blockchain_providers.dart resets when activeWalletRecordProvider.id changes. Might be worth having the controller watch an activeWalletId (or ref.listen the record and reset) instead of a bool. Happy to be corrected if in-place switching isn't a supported flow.

@j-kon

j-kon commented Jul 14, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the detailed. You were right that the transaction state was scoped only to wallet availability rather than the logical active wallet.

I updated the transaction list and detail flow to use the active wallet record ID. Switching from wallet A to wallet B now clears the previous wallet’s transaction state, and stale asynchronous results are ignored if the active wallet changes before loading completes.

I also added coverage for wallet switching so transaction list and detail data cannot leak between active wallets.

@Shamsudeen12

Copy link
Copy Markdown

Thanks @j-kon
Everything looks good from my end 🙌🏾

Comment thread bdk_demo/lib/features/transactions/transactions_controller.dart Outdated

@Johnosezele Johnosezele left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Every time I leave the wallet/Home flow and navigate back to Transaction History, I land on "Transaction history not loaded yet" and have to tap Load Transaction History again, even right after a successful sync with txs already known to the wallet (Home shows the balance).

Likely related to transactionsControllerProvider being NotifierProvider.autoDispose.family keyed by wallet ID: leaving the page disposes the controller, so remount always returns TransactionsState.idle().

Please either:

  1. keep loaded history for the active wallet across navigations (drop autoDispose, or cache last success by wallet ID), or
  2. auto-load on page open when an active wallet is present,

and add widget coverage that pumps the list page, navigates away, returns and still shows (or auto-reloads) the previously loaded rows without an extra tap.

@Johnosezele Johnosezele left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

After an incoming tx confirms, Home correctly updates (Balance + Trusted spendable reflect the synced wallet), but Transaction History can still show the earlier pending row until I leave, come back, and tap Reload Transaction History.

So for a while Home and History disagree: spendable says confirmed funds are available, history still says "Awaiting confirmation."

Root cause looks like split data paths:

  • Home reads wallet.balance() via balanceSnapshotProvider on every successful sync
  • History only refreshes on explicit Load/Reload of transactionsControllerProvider (and with autoDispose it also drops to idle when leaving the route)

Please invalidate or auto-reload transaction history for the active wallet when sync completes (same moment balance is applied), so pending, confirmed cannot lag behind Home.
Widget coverage: load history while pending, then, apply post-sync wallet, and assert list shows confirmed without a manual reload.

@j-kon

j-kon commented Jul 22, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @Johnosezele for the detailed review! I've updated PR #102 in commits e9b32f0 and 1958c9d:

  1. Auto-load on Navigation: Added microtask auto-loading in TransactionsController.build(). Navigating to TransactionsListPage now automatically loads transaction history without showing an un-loaded state or requiring a manual button tap.
  2. Auto-reload on Wallet Sync & Post-Broadcast: TransactionsController now listens to activeWalletProvider and syncStatusProvider (SyncStatus.synced), and send_page.dart triggers background transaction reloads upon broadcast success. As soon as a sync completes or a transaction confirms on-chain, transaction history auto-refreshes in the background without UI flicker or manual reloading.
  3. Widget Coverage: Added widget tests covering automatic initial load, navigation away and back, auto-reload on wallet sync (pending -> confirmed transition), and wallet switching.

Comment thread bdk_demo/lib/features/send/send_page.dart Outdated

@Johnosezele Johnosezele left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I've left some questions and nits here

Comment thread bdk_demo/test/presentation/transactions/transactions_list_page_test.dart Outdated
Comment thread bdk_demo/lib/providers/wallet_providers.dart Outdated
Comment thread bdk_demo/lib/features/transactions/transaction_detail_page.dart
Comment thread bdk_demo/lib/features/transactions/transactions_repository.dart Outdated
Comment thread bdk_demo/lib/features/transactions/transactions_list_page.dart Outdated
@j-kon

j-kon commented Aug 6, 2026

Copy link
Copy Markdown
Contributor Author

Thanks for the detailed review, John. I’ve gone through the six open threads and I understand the concerns.

The main issue is that the transaction history refresh path is still not fully consistent after broadcast, and there are also a few state and cleanup problems around wallet availability, empty states, and the tests.

I haven’t fixed these yet. I’m going to address them one by one, add focused regression coverage, and avoid pushing another broad update until I’ve verified the behavior locally.

I’ll reply to each thread with the exact change and test once it is done. Thanks for taking the time to review this carefully.

@j-kon

j-kon commented Aug 10, 2026

Copy link
Copy Markdown
Contributor Author

@Johnosezele All six open review points are addressed in 0986214, with direct thread replies and regression coverage. Formatting and analyzers pass; root tests are 22 passed with 5 integration skips, and the demo suite is 201 passed. Ready for re-review when convenient.

@Johnosezele Johnosezele left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nice work, thank you! I've left some comments, should be good to go after these fixes.

Comment thread bdk_demo/lib/features/transactions/transactions_repository.dart
Comment thread bdk_demo/lib/features/transactions/transactions_controller.dart
@Johnosezele

Copy link
Copy Markdown
Collaborator
Screenshot 2026-08-24 at 1 37 56 PM Screenshot 2026-08-24 at 1 38 19 PM

@j-kon remove the "Transaction History" banner with the "Reload Transaction History" button.

From your comment:

Auto-load on Navigation: Added microtask auto-loading in TransactionsController.build(). Navigating to TransactionsListPage now automatically loads transaction history without showing an un-loaded state or requiring a manual button tap.
Auto-reload on Wallet Sync & Post-Broadcast: TransactionsController now listens to activeWalletProvider and syncStatusProvider (SyncStatus.synced), and send_page.dart triggers background transaction reloads upon broadcast success. As soon as a sync completes or a transaction confirms on-chain, transaction history auto-refreshes in the background without UI flicker or manual reloading.

here

With auto-load / auto-reload in place, that banner is redundant, you can remove it.

while testing I noticed an incoming receive can show pending in history while Home already shows the updated total balance and trusted spendable still at 0, then after a Home refresh, trusted spendable catches up and the tx flips to confirmed, because sync refreshes the confirmation status.

This part is ux related and the fix is above the scope of this PR, not a merge blocker

@j-kon

j-kon commented Aug 27, 2026

Copy link
Copy Markdown
Contributor Author

@Johnosezele Updated in 17155c4: the redundant Transaction History banner and manual Load/Reload button are removed. The two inline follow-ups are also addressed, with focused regression coverage, and all review threads are resolved. Ready for re-review.

@Johnosezele Johnosezele left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nice work, we can remove these below...

Comment thread bdk_demo/lib/features/transactions/transactions_list_page.dart Outdated
Comment thread bdk_demo/lib/features/transactions/transaction_detail_page.dart Outdated
@j-kon

j-kon commented Aug 31, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the remaining review comments in 09e0d45 and cc809fc.

The redundant Transaction History heading and the Transaction Detail subtitle have both been removed.

Thanks again for the review, @Johnosezele. This should be ready for another look whenever you have a chance.

@Johnosezele Johnosezele left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

ACK cc809fc

@Johnosezele

Copy link
Copy Markdown
Collaborator

@j-kon You have some unverified commits in this PR, can you pls re-sign your commit history?

@j-kon
j-kon force-pushed the feat/bdk-demo-real-transaction-history branch from cc809fc to 0cb3375 Compare September 6, 2026 03:26
@j-kon

j-kon commented Sep 6, 2026

Copy link
Copy Markdown
Contributor Author

@Johnosezele Done! I've re-signed the entire commit history on the branch. All 20 commits are now verified on GitHub, with identical commit trees and diffs (tip commit is now 0cb3375).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants