You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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 _onSwapCreatedbeforecontinueClicked/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 unchangedstate.externalAddress, which immediately hits the identity early-return in the handler:
…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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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