Skip to content

chore: clean up swaps - #2290

Merged
i5hi merged 8 commits into
developfrom
swap-review
Jun 15, 2026
Merged

chore: clean up swaps#2290
i5hi merged 8 commits into
developfrom
swap-review

Conversation

@i5hi

@i5hi i5hi commented Jun 13, 2026

Copy link
Copy Markdown
Collaborator

Swap rework: reliable status tracking, claims/refunds, and live fees

Reworks the swaps feature end-to-end to fix unreliable Boltz status updates and failing claims/refunds, and to make persisted fees reflect what was actually paid on-chain. Only includes logic improvements - no fundamental architectural changes.

All swap types need to be tested before merging.

  • Reverse swaps into LBTC and BTC from LN via Receive
  • Submarine swaps from LBTC and BTC to LN via Send
  • Chain swaps to and from LBTC - BTC via Transfer
  • Chain swaps to and from LBTC - BTC via Send

@claude

claude Bot commented Jun 13, 2026

Copy link
Copy Markdown

Claude finished @i5hi's task in 3m 58s —— View job


PR Review: chore: clean up swaps

I reviewed the diff against origin/develop for correctness bugs, regressions, and security issues. Overall the rework is a solid improvement — serialized per-swap processing, live fee estimation at execution time, REST reconciliation as a websocket backstop, and a dedicated refunded status are all good calls. But there are blocking issues that must be fixed before this can build or be tested.

Note: I can only post a single consolidated comment (I can't attach formal inline review comments), so findings are grouped below with file:line references.


🔴 Blocking — three new files are referenced but not committed

The PR adds imports/usages for three files that do not exist in the branch (git ls-files + on-disk check both confirm they're absent). As committed, the app will not compile and the migration will not link.

  1. lib/core/swaps/data/services/swap_status_mapper.dart — imported at boltz_datasource.dart:8 and used as _mapper.map(...) (boltz_datasource.dart:25,1098,1117). The whole new status pipeline (SwapStatusMapper, SwapUnchanged, etc.) depends on it.
  2. lib/core/swaps/domain/usecases/process_ongoing_swaps_usecase.dart — imported at swaps_locator.dart:21 and handler.dart:3; ProcessOngoingSwapsUsecase is registered (swaps_locator.dart:210) and resolved (handler.dart:45).
  3. lib/core/storage/migrations/schema_12_to_13.dart — exported from migrations.dart:4; Schema12To13.migrate is wired into the migrator at sqlite_database.dart:156. The schema version was bumped to 13 (sqlite_database.dart:101) and swap_model.dart relies on this migration to backfill refunded status, so without it the v12→v13 upgrade breaks.

This looks like a missed git add for the new files. Please commit them. Everything below assumes these files exist and is secondary.


🟡 Minor / worth a look

1. Fee floor can be undone by the "half the amount" capswap_watcher.dart _claimFees (~line 1378)

final withFloor = _absoluteWithFloor(live, txSize: txSize, isLiquid: isLiquid);
if (amountSat != null && amountSat > 0) {
  return min(withFloor, max(1, amountSat ~/ 2));
}

_absoluteWithFloor guarantees a relay-broadcastable minimum, but min(withFloor, amountSat ~/ 2) can then drop the fee back below that floor for very small swaps, producing a potentially unbroadcastable claim tx. Consider flooring the result again, e.g. max(floor, min(withFloor, amountSat ~/ 2)). Edge-case (tiny amounts), but it silently defeats the floor you just applied.

2. dart format violations will fail CItransfer_bloc.dart (lines ~415, 455, 508, 560, 985)
Several amountSat: arguments were reindented inconsistently, e.g.:

amountSat: isMaxSend
  ? null
  : swap.paymentAmount,

These don't match dart format output and will trip --set-exit-if-changed. Run dart format . before pushing.

3. Leftover SWAP_TESTER verbose loggingswap_watcher.dart, boltz_datasource.dart, transfer_bloc.dart
Multiple log.info('SWAP_TESTER ...') calls are tagged "temporary — remove after live testing." Fine for the testing phase noted in the PR description, but make sure these are stripped before merge (they log swap ids, addresses, amounts at info level).

4. _reconcileOngoing / _reconcileTimer lifecycleswap_watcher.dart:102
_reconcileTimer ??= Timer.periodic(...) is created in startWatching() but I don't see it cancelled anywhere (no dispose). If the service is ever torn down/recreated this leaks a periodic timer. Likely fine for a singleton, but worth confirming the service is never re-instantiated.


✅ Things that look correct

  • getOngoingSwaps().needsWatching now keeps watching expired/failed send/chain swaps that have a lockup but no refund — good, avoids stranding funds.
  • updateSwapFields re-fetches and merges per-field, avoiding stale-copy clobbering of concurrently-written fields (preimage, lockupFee). Good fix for the lost-update race.
  • SwapFees.totalFees now adds either refundFee or claimFee (never both) — correct, since the locked output is spent exactly once.
  • get_transactions_usecase dedup of the refund-back leg so a refunded chain swap shows as one row is reasonable, and the canonical-leg selection (s.txId == wtTxId) is sound.
  • send_cubit / transfer_bloc now also sync wallets on refunded so returned funds appear — good.
  • isTestnet: false hardcoding in the watcher matches the pre-existing repo-level isTestnet: false in swaps_locator.dart:52, so it's consistent (not a new regression) — though it does mean none of this path works on testnet.

I did not run flutter analyze/tests (dependencies aren't installed in this environment, and the missing files would fail the analyzer regardless). Once the three files are committed, a local flutter analyze + dart format . should be run before merging.

@i5hi

i5hi commented Jun 13, 2026

Copy link
Copy Markdown
Collaborator Author

Note: production flavor is commented out for local ios simulator development. It must be removed before merging.

@i5hi

i5hi commented Jun 13, 2026

Copy link
Copy Markdown
Collaborator Author

Note: debug logs must be cleaned before merging

i5hi added 6 commits June 13, 2026 15:52
…her teardown, and send-flow fee fixes (preserve lockup fee, tx-fee fallback, confirm-amount flash)
…N details use shimmer during load. Text update for Amount in receive for LN. Minor fixes in confirmation screen details - align invoice and add copy button for swap id
@i5hi
i5hi merged commit 4cd3755 into develop Jun 15, 2026
1 check passed
@claude claude Bot mentioned this pull request Jul 5, 2026
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