Skip to content

fix: block swap continue when external address is not on the counte network - #2539

Open
i5hi wants to merge 1 commit into
developfrom
fix/swap-receive-address-counter-network
Open

fix: block swap continue when external address is not on the counte network#2539
i5hi wants to merge 1 commit into
developfrom
fix/swap-receive-address-counter-network

Conversation

@i5hi

@i5hi i5hi commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

Currently - we show an error for a wrong address but we allow the user to continue and create a swap to a wrong recieve address network

@claude

claude Bot commented Jul 30, 2026

Copy link
Copy Markdown

Claude finished @i5hi's task in 4m 16s —— View job


Code review

  • Gather context (diff vs origin/develop)
  • Read changed files and surrounding code
  • Analyze for correctness bugs, security issues, regressions
  • Post findings + summary

The core change is sound: extracting _resolveCounterNetworkAddress as a single source of truth, re-validating in _onSwapCreated before continueClicked/isCreatingSwap are set, and gating the Continue button on isExternalAddressBlocking gives good defense in depth. The guard runs before any state mutation, so a rejected address returns early with no stuck spinner, and the swap can no longer be created to a wrong-network address. The handler refactor preserves the old behavior (sanitization, BIP21 amount handling, error messages, generic-exception fallback), and the error renders reactively via context.select in swap_external_address_input.dart.

One real issue found:

🟠 Stale externalAddressError can disable Continue after a wallet switch — lib/features/swap/presentation/transfer_state.dart:228

isExternalAddressBlocking turns externalAddressError into a hard gate on the Continue button, but that error is never actually re-validated when the from-wallet changes. _onFromWalletChanged re-dispatches the address for re-validation (transfer_bloc.dart:333), but it passes the unchanged state.externalAddress, which immediately hits the identity early-return in the handler:

// transfer_bloc.dart:754
if (state.externalAddress == event.address) return;

…so the wallet-switch re-validation is a no-op (pre-existing dead code, but this PR makes its consequences user-visible):

  • Regression: from-wallet is Bitcoin, user pastes a Bitcoin address → error "Please enter a valid Liquid address" is set. User switches the from-wallet to Liquid, making the address valid for the counter network — the stale error remains and Continue stays disabled until the user manually edits the address text. On develop the user could still continue in this case.
  • Mirror case: valid address → wallet switch → address is now on the wrong network but the error stays null, so Continue stays enabled. The new _onSwapCreated guard correctly blocks creation (this is exactly why the guard is valuable), but the inline error only appears after the click.

Suggested fix: make the wallet-switch re-validation real — bypass the identity guard when re-dispatching (e.g. a force flag on the event), or run _resolveCounterNetworkAddress directly in _onFromWalletChanged and emit the updated externalAddressError. Fix this →

Minor notes

  • lib/features/swap/presentation/transfer_bloc.dart:380 — the guard uses state.fromWallet!. It's unreachable from the UI today (a null wallet sets the "Please select a wallet first" error, which disables Continue), but since this guard's stated purpose is to not trust the UI state, a null-safe early return would be more consistent than a potential null-assertion crash.
  • lib/features/swap/presentation/transfer_bloc.dart:384 — on guard failure, a lingering swapCreationException from a previous attempt isn't cleared, so both errors could briefly show together. Cosmetic.
  • The new test only exercises the isExternalAddressBlocking getter with hand-set state; a bloc test for _onSwapCreated rejecting a wrong-network address (the actual fix) would protect the guard against future refactors.

No security issues found; the change strictly tightens validation. I could not run flutter test (no Flutter SDK in this runner) and couldn't post inline review comments (gh api is not in my allowed tools), so findings are consolidated here with file/line references.

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.

1 participant