refactor(UI): more useful details on broadcast screen and more by ethicnology · Pull Request #2010 · SatoshiPortal/bullbitcoin-mobile

ethicnology · 2026-04-08T05:50:03Z

For externally-signed PSBTs we can't assume which output is change and input amounts aren't available locally. Fees only compute by fetching parent transactions from Electrum.

Transaction review on broadcast screen

Shared port + feature split. Primitives (Transaction abstract + concrete entities, TransactionPort, TransactionMapper) live in lib/core/transactions/ since electrum implements the port. Review-side code (BuildReviewableTransactionUsecase, ReviewableTransaction aggregate, TransactionReviewCubit, TransactionReviewView) lives in lib/features/broadcast_signed_tx/. Inputs resolved via ElectrumTransactionPortAdapter → fees + amounts shown for any signed PSBT regardless of origin.

TransactionMapper catches all scriptPubKey decode errors
isTestnet propagated so testnet outputs decode
MempoolUrlBuilder liquid txid falls back to tx/<txid> when no unblindedUrl
Errors split per layer: TransactionPortError (port) translated to discrete TransactionReviewError variants at the usecase boundary — view never sees the foreign type
isBroadcasting / isBroadcasted guards prevent double-tap and re-broadcast after success

Address / Transaction / Invoice viewers

Unifies 15+ inconsistent display patterns (manual truncateMiddle + copy icon, CopyInput, custom substring).

AddressViewer('bc1q...')                              // network auto-detected
TransactionViewer.bitcoin('abc...', isTestnet: false) // mempool.space link
TransactionViewer.liquid('abc...', unblindedUrl: ...) // unblinded explorer
TransactionViewer.ark('abc...')                       // no explorer link
InvoiceViewer('lnbc1...')                             // no explorer link

Adaptive truncation (min 5 head + 5 tail), dotted underline = tappable
Tap → BlurredDialog with full value in 4-char groups + copy + "View in explorer"
Long press → copy to clipboard with snackbar
clipboardText param copies a different value than displayed (e.g. full BIP21 URI when cell shows the address)
Address dialog has Address / Payment URI segmented toggle when clipboardText is bip21

BlurredDialog

Shared wrapper matching BlurredBottomSheet.show API. All showDialog call-sites migrated for consistent styling.

Receive screen

receive_enter_note split into receive_enter_amount; receive_qr_screen redesign
Payjoin URI no longer leaks across wallets (subscription reset on wallet change) or appears on empty wallets — new hasUtxos / canPayjoin getters gate pj=
Standalone "Copy address only" row + static "Payjoin activated" message → one bordered tappable switch; toggle ON without UTXOs shows snackbar
New label_entry_bottom_sheet + labels_facade.fetchDistinctLabels({type})
Receive note suggestions scoped to LabelType.address so private transaction labels never autocomplete into a counterparty-visible BIP21 message=

Architectural cleanup

Removed GetElectrumServersToUseUsecase — was being called from three secondary adapters (illegal layer direction). Orchestration absorbed into ElectrumServerRepository.fetchActiveServers; the affected adapters now depend on repositories directly. Request/response DTOs for the dead usecase deleted.
EnvironmentAdapter swapped onto SettingsRepository directly (was the same adapter→usecase inversion).
lib/core/transactions/ was originally added as a full feature inside core — split per the architecture rules (primitives stay, feature concerns moved to broadcast_signed_tx).
Folder migration interface_adapters/ → adapters/ for the touched modules (electrum, wallet, blockchain). Other modules' legacy folders untouched.
Dead code dropped: two unreachable execute methods on the build usecase, TransactionMapper.fromLiquidTx, TransactionError.parseFailed.
Naming aligned with AGENTS.md: TransactionEntity → ReviewableTransaction (no Entity suffix), TransactionCubit/State/Screen → TransactionReview*, single-file folders flattened.

Misc

Removed: TransactionDetailsWidget, BitcoinTransactionRepository, FetchBitcoinTransactionUsecase, TransactionLabelBottomsheet, unused price_input
New bordered_tappable_tile (used by payjoin switch row)
New strings added in English; non-EN locales fall back to English at runtime pending translation pass

Known follow-ups (out of scope for this PR)

ElectrumTransactionPortAdapter should narrow per-server error swallowing and stop leaking raw lastError.toString() into user-visible messages
blockchain/adapters/electrum_server_adapter.dart and wallet/adapters/electrum_server_adapter.dart are byte-identical and could share an impl via a unified port (pre-existing duplication)
interface_adapters/ → adapters/ migration still pending in tor/, mempool/, swaps/, status/, recipients/, electrum_settings/, recoverbull/, withdraw/, pay/, onboarding/
`

ethicnology · 2026-04-08T05:53:17Z

rebased on develop

ethicnology · 2026-04-08T06:02:56Z

rename & squash some commits

Address follow-up review on PR #2010. Direction of dependency is now strictly electrum -> transactions, with all cross-module imports confined to the interface_adapters and DI layers. - Introduce ElectrumServersPort + adapter; fetch usecase no longer imports another usecase. - Move transaction_port from application/ to domain/ports/ to match the convention used by every other port in the codebase. - Relocate transaction_mapper from adapters/ to data/mappers/ to match the wallet module convention. - Drop cross-module domain dep: ElectrumTransactionRepository now returns the util BitcoinTx; mapping to the transactions domain entity happens in the port adapter. - Replace transactions/TransactionError thrown from the electrum usecase with electrum-local ElectrumFetchException subclasses; the port adapter catches and translates them at the module boundary.

ethicnology · 2026-05-20T11:59:31Z

@i5hi done

Address follow-up review on PR #2010. Direction of dependency is now strictly electrum -> transactions, with all cross-module imports confined to the interface_adapters and DI layers. - Introduce ElectrumServersPort + adapter; fetch usecase no longer imports another usecase. - Move transaction_port from application/ to domain/ports/ to match the convention used by every other port in the codebase. - Relocate transaction_mapper from adapters/ to data/mappers/ to match the wallet module convention. - Drop cross-module domain dep: ElectrumTransactionRepository now returns the util BitcoinTx; mapping to the transactions domain entity happens in the port adapter. - Replace transactions/TransactionError thrown from the electrum usecase with electrum-local ElectrumFetchException subclasses; the port adapter catches and translates them at the module boundary.

Broadcast screen - Resolve PSBT inputs via ElectrumTransactionPortAdapter (new lib/core/transactions/ module: entities, ports, cubit, mapper, build/fetch usecases, transaction_screen). - Robust mapper: catch-all scriptPubKey decode, testnet flag passing. - Liquid txid mempool URL falls back to tx/<txid> when no unblindedUrl. Address / Transaction / Invoice viewers - New widgets in lib/core/widgets/: AddressViewer, TransactionViewer, InvoiceViewer (replace transaction_details_widget + transaction_label_bottomsheet). - Tap to open detail dialog, press to copy. Dotted underline for tappability, alternating group styles. - Detail dialog: segmented Address / Payment URI toggle for bip21. Receive screen - Refactor: split receive_enter_note into receive_enter_amount; receive_qr_screen redesign. - Payjoin: single switch row replaces "address only" + "payjoin activated" UI. URI no longer leaks across wallets or appears on empty wallets (hasUtxos / canPayjoin gates). - New label_entry_bottom_sheet + labels_facade. Misc - BlurredDialog unifies dialog widgets. - bordered_tappable_tile. - Remove unused price_input.dart.

Introduce ElectrumTransactionRepository so the fetch usecase depends on a port instead of SqliteDatabase, and register ElectrumRemoteDatasource as a singleton instead of building it per call. Move BitcoinTx -> domain Transaction mapping into the repository impl and BuildTransactionUsecase so presentation no longer imports adapters.

Address follow-up review on PR #2010. Direction of dependency is now strictly electrum -> transactions, with all cross-module imports confined to the interface_adapters and DI layers. - Introduce ElectrumServersPort + adapter; fetch usecase no longer imports another usecase. - Move transaction_port from application/ to domain/ports/ to match the convention used by every other port in the codebase. - Relocate transaction_mapper from adapters/ to data/mappers/ to match the wallet module convention. - Drop cross-module domain dep: ElectrumTransactionRepository now returns the util BitcoinTx; mapping to the transactions domain entity happens in the port adapter. - Replace transactions/TransactionError thrown from the electrum usecase with electrum-local ElectrumFetchException subclasses; the port adapter catches and translates them at the module boundary.

The prior commit (e2f56de) edited transaction_mapper and transaction_port at their new paths but never recorded the rename; git still tracked them at the old paths.

FetchElectrumTransactionUsecase had exactly one caller — ElectrumTransactionPortAdapter — and contained pure infrastructure orchestration (server priority + fallback + typed errors), not any business workflow. A use case called only by a driven adapter inverts the layer rule (ARCHITECTURE.md, AGENTS.md rule 2). Inline the fallback loop into the adapter, depending directly on ElectrumServersPort + ElectrumTransactionRepository + EnvironmentPort. The adapter now throws TransactionError directly; the intermediate ElectrumFetchException family had only one producer + one consumer, both inside the deleted use case path, so it follows the use case out. The integration test switches from the use case to TransactionPort — the equivalent public entry point — and still exercises the sqlite-cache flow through the same code path.

Address / transaction / invoice detail dialogs duplicated a `GestureDetector(Row(Icon+Gap+Text))` action row across three files. The bare row gave a ~14dp tap target — well below the 48dp Material recommendation — and the duplication invited drift between the three viewers. Extract a shared `ViewerActionButton` in lib/core/widgets/: InkWell-based, 44dp minHeight, full-width tap area, theme tokens only. Swap the three viewers over. While here, pass `mode: LaunchMode.externalApplication` to launchUrl in AddressViewer + TransactionViewer to match how the rest of the codebase opens external links and avoid `platformDefault` misfires on some Android setups.

When `data` is a BIP21 URI (e.g. send flow's `paymentRequestAddress` returning the raw URI on MRH paths, or copied/scanned raw payment requests), Satoshifier returns a `Bip21` value — not `BitcoinAddress` / `LiquidAddress`. The old if-chain only matched the bare-address types and fell through to `return null`, so Copy Link / Open in Explorer silently did nothing. Switch to a sealed-pattern match and add a Bip21 arm that routes by `network.isLiquid` and uses `parsed.address` — the embedded bare address — for the mempool URL.

…pters to adapters ElectrumServersAdapter (electrum), ElectrumServerAdapter (blockchain, wallet) previously depended on GetElectrumServersToUseUsecase — an application-layer use case invoked from secondary adapters, reversing the layer-direction rule. The usecase served these adapters only. Absorb the orchestration (fetch + filter custom + sort by priority) into ElectrumServerRepository.fetchActiveServers as a query method. The two non-electrum adapters compose it with ElectrumSettingsRepository and SettingsRepository (for tor proxy) inline. Delete the usecase plus its request/response DTOs. Relocate all touched files from the legacy interface_adapters/ folder to the canonical adapters/ folder (electrum, wallet — matching the blockchain move already in flight). Remaining interface_adapters/ usage in tor/mempool/swaps/status is out of scope for this PR.

lib/core/transactions/ was structured as a full feature inside core (domain + application + presentation + ui), violating the rule that core is infrastructure only. The cubit, screen, usecase, and ReviewableTransaction aggregate were consumed solely by lib/features/broadcast_signed_tx/. Split: keep the shared primitives in core (Transaction entities, TransactionPort, TransactionMapper) since electrum implements the port and depends on the mapper; move review-specific concerns into the broadcast_signed_tx feature. Errors split per-layer: TransactionPortError in core (thrown by the port adapter), TransactionReviewError in the feature (thrown by the usecase, wrapping port errors at the boundary). The view renders TransactionReviewError and unwraps the inner port error for granular messaging — never sees a foreign error type. Drop dead code: BuildTransactionUsecase had three execute methods — executeFromTransaction was internal-only and executeFromWalletTransaction was never called. Collapse to a single execute(). Drop unused TransactionMapper.fromLiquidTx and the dead TransactionParseFailed variant. Rename per AGENTS.md naming: TransactionEntity -> ReviewableTransaction (no Entity suffix), TransactionCubit -> TransactionReviewCubit, TransactionScreen -> TransactionReviewView (it's an embedded widget, not a routed screen). Folder layout per rule 14 — no single-file subfolders.

…irectly EnvironmentAdapter still depended on GetSettingsUsecase — the same adapter→usecase inversion the earlier "drop usecase from adapter deps" commit claims to remove. Swap for a direct SettingsRepository dep so the cleanup is consistent.

LabelsFacade.fetchDistinctLabels() returned every label regardless of LabelType. The receive note bottom sheet surfaces these as suggestion chips, and the chosen string becomes BIP21 message= — counterparty visible. A user could autocomplete a private transaction label ("Boss salary") into a sender-visible field. Add an optional LabelType filter to the facade and pass LabelType.address from the receive bloc. Send/tx call sites stay unscoped (private context).

…error leak, guard re-broadcast Three review fixes against the broadcast/review code I shipped: - BuildReviewableTransactionUsecase took btc_utils.BitcoinTx and called TransactionMapper directly. That's application-importing-adapter and foreign-type-in-usecase-signature in one — the same rule-2 violation the previous commit claims to fix. Cubit now does the mapping at the primary-adapter boundary; usecase takes a domain Transaction. - TransactionReviewError.portFailure(TransactionPortError) was ceremonial — the view destructured it and pattern-matched on the inner port-layer variants, leaking the foreign error type. Replaced with discrete feature-layer variants (fetchFailed, noServersAvailable, inputResolutionFailed, unexpected). Usecase translates port errors at the boundary. - broadcastTransaction's early-return checked isBroadcasting and transaction != null but not isBroadcasted. After a successful broadcast the button re-enabled and the same tx could be broadcast again. Added the missing guard.

ethicnology self-assigned this Apr 8, 2026

ethicnology linked an issue Apr 8, 2026 that may be closed by this pull request

More useful details on broadcast screen #1247

Closed

ethicnology marked this pull request as draft April 8, 2026 05:50

ethicnology force-pushed the 1247-more-useful-details-on-broadcast-screen branch from e3450f8 to 8666604 Compare April 8, 2026 05:53

ethicnology force-pushed the 1247-more-useful-details-on-broadcast-screen branch from 8666604 to dbbbf88 Compare April 8, 2026 06:02

ethicnology force-pushed the 1247-more-useful-details-on-broadcast-screen branch 2 times, most recently from d23489c to 8eab6af Compare April 15, 2026 06:06

thibistaken added this to Wallet Releases May 13, 2026

thibistaken requested a review from i5hi May 13, 2026 09:27

ethicnology force-pushed the 1247-more-useful-details-on-broadcast-screen branch from 1722e25 to 75ebeec Compare May 18, 2026 05:59

ethicnology changed the title ~~refactor: more useful details on broadcast screen (and more)~~ refactor(UI): more useful details on broadcast screen and more May 18, 2026

ethicnology marked this pull request as ready for review May 18, 2026 06:59

thibistaken moved this to In Progress in Wallet Releases May 18, 2026

thibistaken added the enhancement New feature or request label May 18, 2026

thibistaken removed this from Wallet Releases May 18, 2026

thibistaken mentioned this pull request May 18, 2026

On-chain receive flow ignores requested amount #1832

Closed

1 task

ethicnology added this to Wallet Releases May 18, 2026

ethicnology moved this to PR Review in Wallet Releases May 18, 2026

i5hi reviewed May 19, 2026

View reviewed changes

ethicnology requested a review from i5hi May 19, 2026 06:40

ethicnology force-pushed the 1247-more-useful-details-on-broadcast-screen branch from 120bd76 to cb000f2 Compare May 19, 2026 07:57

ethicnology force-pushed the 1247-more-useful-details-on-broadcast-screen branch from 9c502ce to 713ecd8 Compare May 20, 2026 09:40

ethicnology force-pushed the 1247-more-useful-details-on-broadcast-screen branch from 025963a to a3bb57d Compare May 21, 2026 10:16

ethicnology added 8 commits May 22, 2026 10:36

chore: complete file moves missed in previous commit

1904d30

The prior commit (e2f56de) edited transaction_mapper and transaction_port at their new paths but never recorded the rename; git still tracked them at the old paths.

ethicnology force-pushed the 1247-more-useful-details-on-broadcast-screen branch from a3bb57d to 8c200c3 Compare May 22, 2026 07:40

i5hi and others added 4 commits May 22, 2026 17:32

Merge branch 'develop' into 1247-more-useful-details-on-broadcast-screen

8689069

i5hi approved these changes May 25, 2026

View reviewed changes

i5hi merged commit ef810e5 into develop May 25, 2026
1 check passed

github-project-automation Bot moved this from PR Review to User Testing in Wallet Releases May 25, 2026

thibistaken removed this from Wallet Releases May 25, 2026

thibistaken mentioned this pull request Jun 1, 2026

On-chain payment request amount not included in BIP21 URI #2187

Closed

Navigation Menu

Uh oh!

refactor(UI): more useful details on broadcast screen and more - #2010

refactor(UI): more useful details on broadcast screen and more#2010
i5hi merged 13 commits into
developfrom
1247-more-useful-details-on-broadcast-screen

ethicnology commented Apr 8, 2026 •

edited

Loading

Uh oh!

ethicnology commented Apr 8, 2026

Uh oh!

ethicnology commented Apr 8, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ethicnology commented May 20, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

ethicnology commented Apr 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Transaction review on broadcast screen

Address / Transaction / Invoice viewers

BlurredDialog

Receive screen

Architectural cleanup

Misc

Known follow-ups (out of scope for this PR)

Uh oh!

ethicnology commented Apr 8, 2026

Uh oh!

ethicnology commented Apr 8, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

ethicnology commented May 20, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ethicnology commented Apr 8, 2026 •

edited

Loading