Skip to content

fix(autoswap): retry a rate-limited order creation - #2681

Merged
ethicnology merged 2 commits into
mainfrom
fix/autoswap-not-triggering
Aug 17, 2026
Merged

fix(autoswap): retry a rate-limited order creation#2681
ethicnology merged 2 commits into
mainfrom
fix/autoswap-not-triggering

Conversation

@wired-pasteque

Copy link
Copy Markdown
Collaborator

No description provided.

@wired-pasteque wired-pasteque self-assigned this Aug 14, 2026
@claude

This comment was marked as outdated.

@wired-pasteque

Copy link
Copy Markdown
Collaborator Author

@claude, check the fixes

@claude

claude Bot commented Aug 14, 2026

Copy link
Copy Markdown

Claude finished @wired-pasteque's task in 3m 4s —— View job


Review of the fixes

  • Gather context (PR diff vs origin/develop)
  • Review retry logic in exchange_autoswap_provider.dart
  • Review autoswap_watcher.dart changes
  • Review test coverage
  • Post review feedback

The fixes look correct to me. Both commits do what they say: the rate-limited createOrder gets exactly one bounded retry, and the watcher logging is now informative without being noisy. No blocking issues found.

What I verified

  • Rate-limit mapping is real: ExchangeRateLimitException (parsed from the Retry-After header in exchange_public_api_datasource.dart:115) is mapped to SwapRateLimitedFailure with a retryAfter duration in order_swap_repository_impl.dart:661-667, so the failure is SwapRateLimitedFailure check in exchange_autoswap_provider.dart:160 will actually fire.
  • Retry is bounded: exactly one retry, capped at 30s (_maxCreateRetryDelay), with a 5s fallback when the server omits Retry-After. No recursion, no unbounded sleep. The rationale in the doc comment (watcher's in-flight guard drops syncs while sleeping, dispose awaits the run) matches what autoswap_watcher.dart:19-26 actually does.
  • Constructor change is safe: dropping const to inject _delay is fine — the only production call site (autoswap_locator.dart:31) doesn't use const.
  • SwapRateLimitedFailure is reachable: it's exported through swap_facade.dart (line 27), which the provider already imports.
  • Tests cover the right paths: server-supplied delay honored, fallback delay, over-cap delay skipped (single call), and non-rate-limit failures not retried. The injected delay callback makes them deterministic with no real sleeps.

Minor observations (non-blocking)

  1. Doc comment attribution nitexchange_autoswap_provider.dart:23 says the 30s cap "matches the spacing RefreshOrderSwapsUsecase uses", but the 30s _requestSpacing constant actually lives in RefreshPendingOrderSwapsUsecase (refresh_pending_order_swaps_usecase.dart:28). Same subsystem, slightly wrong class name.
  2. "transfer under way" repeats while an order is pendingexecute() also returns Ok when _resume short-circuits on an already-broadcast pending order (exchange_autoswap_provider.dart:182-185), so autoswap_watcher.dart:33 logs [Autoswap] transfer under way on every liquid sync until the order completes. It's info-level so the noise is contained, but the wording suggests a new transfer each time.
  3. The retried request reuses the pre-wait quote — the second create() submits the same quotedCounterpartAmountSat captured before the wait. If the exchange re-validates quote freshness, the retry may fail with a quote-mismatch error instead — that's handled gracefully (falls through to AutoswapProviderFailure), just worth knowing the retry isn't guaranteed to succeed.
  4. getQuote isn't retried — if the rate limiter trips on the quote call rather than order creation, the run still gives up until the next sync. Out of scope for this PR (the comment at line 155-156 correctly explains why createOrder is the likely victim), but a possible follow-up if rate limiting shows up earlier in the flow.

Not run locally

Flutter isn't installed on this runner, so I couldn't execute flutter analyze/flutter test myself. The Analyze and Test workflow is currently in progress on this PR and will confirm.

@ethicnology
ethicnology changed the base branch from develop to main August 17, 2026 16:42
@ethicnology
ethicnology merged commit cc0bf65 into main Aug 17, 2026
2 checks passed
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.

2 participants