feat(swap): auto-fallback to streaming swap when THORChain rapid slippage >3%#4297
Conversation
…page >3% When a rapid (interval=0) THORChain quote has fees >3% of gross output, fetch a streaming quote (interval=1, quantity=max_streaming_quantity) and return whichever gives the better expected_amount_out. Streaming fetch failures fall back silently to rapid so existing behaviour is unchanged for the vast majority of swaps. Threshold is a single named constant (STREAMING_SLIPPAGE_THRESHOLD_BPS=300) that can be set to Int.MAX_VALUE to disable without a code revert. Fixes #4145 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughFetches a rapid THORChain quote, computes rapid slippage, and when slippage exceeds a threshold, optionally requests a streaming quote then selects the better quote with silent fallback to rapid on streaming errors. Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client
participant Repo as SwapQuoteRepository
participant API as ThorChainApi
participant Log as Timber
Client->>Repo: getSwapQuote(...)
Repo->>API: fetchQuote(interval="0")
API-->>Repo: rapidQuote / error
Repo->>Repo: compute rapidSlippageBps
alt rapidSlippageBps ≤ threshold
Repo-->>Client: return rapidQuote
else rapidSlippageBps > threshold
Repo->>API: fetchQuote(interval="1", streaming_quantity?)
API-->>Repo: streamingQuote / error
alt streaming returns usable quote
Repo->>Repo: pickBestQuote(rapid, streaming)
Repo-->>Client: return chosenQuote
else streaming fails or error
Repo->>Log: log streaming failure
Repo-->>Client: return rapidQuote (or throw if rapid missing)
end
end
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
data/src/test/kotlin/com/vultisig/wallet/data/repositories/SwapQuoteRepositoryThorChainStreamingTest.kt (1)
87-223: Consider adding one exact-threshold test (300 bps).You already cover below and above threshold paths; adding equality (
== STREAMING_SLIPPAGE_THRESHOLD_BPS) would protect the<=behavior from regressions.Possible test addition
+ `@Test` + fun `rapid slippage equal threshold returns rapid without fetching streaming`() = runTest { + // fees=300, out=9700 -> 300 bps + coEvery { thorChainApi.getSwapQuotes(match { it.interval == "0" }) } returns + THORChainSwapQuoteDeserialized.Result( + thorQuote(expectedAmountOut = "9700", feesTotal = "300") + ) + + val result = + repository.getSwapQuote( + dstAddress = "TDst", + srcToken = btc, + dstToken = trx, + tokenValue = TokenValue(value = BigInteger("100000000"), token = btc), + ) + + coVerify(exactly = 0) { thorChainApi.getSwapQuotes(match { it.interval == "1" }) } + val thorResult = result as SwapQuote.ThorChain + assertEquals("9700", thorResult.data.expectedAmountOut) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@data/src/test/kotlin/com/vultisig/wallet/data/repositories/SwapQuoteRepositoryThorChainStreamingTest.kt` around lines 87 - 223, Add a test to assert equality behavior at the streaming slippage threshold: in SwapQuoteRepositoryThorChainStreamingTest create a new test (e.g., `rapid slippage equal to threshold returns rapid without fetching streaming`) that stubs thorChainApi.getSwapQuotes(interval="0") to return a rapid quote whose computed slippage equals STREAMING_SLIPPAGE_THRESHOLD_BPS and then calls repository.getSwapQuote; verify thorChainApi.getSwapQuotes(interval="1") is not invoked and the returned SwapQuote.ThorChain uses the rapid quote. Reference the repository.getSwapQuote call and the existing coEvery/coVerify patterns to mirror other tests.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@data/src/main/kotlin/com/vultisig/wallet/data/repositories/SwapQuoteRepository.kt`:
- Around line 301-304: The computed rapidSlippageBps (using
feesTotal.multiply(BigInteger.valueOf(10000)).divide(grossOut).toInt()) is doing
integer truncation which can hide values just above a threshold; change the
calculation in SwapQuoteRepository (the rapidSlippageBps computation and the
similar calculation at the other occurrence) to perform ceiling division so it
rounds up when there's any remainder (e.g. (feesTotal*10000 + grossOut - 1) /
grossOut semantics) before converting to Int, ensuring comparisons against the
>300 bps gate cannot be missed due to truncation.
---
Nitpick comments:
In
`@data/src/test/kotlin/com/vultisig/wallet/data/repositories/SwapQuoteRepositoryThorChainStreamingTest.kt`:
- Around line 87-223: Add a test to assert equality behavior at the streaming
slippage threshold: in SwapQuoteRepositoryThorChainStreamingTest create a new
test (e.g., `rapid slippage equal to threshold returns rapid without fetching
streaming`) that stubs thorChainApi.getSwapQuotes(interval="0") to return a
rapid quote whose computed slippage equals STREAMING_SLIPPAGE_THRESHOLD_BPS and
then calls repository.getSwapQuote; verify
thorChainApi.getSwapQuotes(interval="1") is not invoked and the returned
SwapQuote.ThorChain uses the rapid quote. Reference the repository.getSwapQuote
call and the existing coEvery/coVerify patterns to mirror other tests.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c7fca1b5-4014-44f6-b3e2-6b327a5abe88
📒 Files selected for processing (4)
data/src/main/kotlin/com/vultisig/wallet/data/api/ThorChainApi.ktdata/src/main/kotlin/com/vultisig/wallet/data/api/models/quotes/ThorChainSwapQuoteRequest.ktdata/src/main/kotlin/com/vultisig/wallet/data/repositories/SwapQuoteRepository.ktdata/src/test/kotlin/com/vultisig/wallet/data/repositories/SwapQuoteRepositoryThorChainStreamingTest.kt
When the rapid (interval=0) quote fails with an API error or network exception, retry with a streaming quote (interval=1) instead of surfacing the error to the user. The streaming quantity hint is omitted in this case so THORChain picks the optimal value. If streaming also fails, the original rapid error is rethrown. Adds three new unit tests covering: - rapid API error → streaming succeeds - rapid network exception → streaming succeeds - both rapid and streaming fail → throws the rapid error Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
data/src/main/kotlin/com/vultisig/wallet/data/repositories/SwapQuoteRepository.kt (1)
312-317:⚠️ Potential issue | 🟡 MinorThreshold gating still misses quotes just above 300 bps.
divide(grossOut)floors the result before the> 300comparison, so values like300.01 bpsare treated as300and skip the streaming fallback.Suggested fix
val rapidSlippageBps = if (grossOut > BigInteger.ZERO) - feesTotal.multiply(BigInteger.valueOf(10000)).divide(grossOut).toInt() + feesTotal + .multiply(BigInteger.valueOf(10_000)) + .add(grossOut.subtract(BigInteger.ONE)) + .divide(grossOut) + .toInt() else 0🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@data/src/main/kotlin/com/vultisig/wallet/data/repositories/SwapQuoteRepository.kt` around lines 312 - 317, The current computation in SwapQuoteRepository (variables rapidSlippageBps, grossOut, feesTotal) floors the bps by integer division so values just above the threshold (e.g., 300.01) are treated as 300; change the gating to avoid flooring by performing the comparison using exact arithmetic (e.g., compare feesTotal * 10000 to grossOut * STREAMING_SLIPPAGE_THRESHOLD_BPS) or compute the division with rounding up/using BigDecimal so that needsStreaming is true for any fractional bps above STREAMING_SLIPPING_THRESHOLD_BPS; update the logic that sets needsStreaming and streamingQuantityHint (and any use of rapidSlippageBps) accordingly, referencing rapidSlippageBps, grossOut, feesTotal, STREAMING_SLIPPING_THRESHOLD_BPS, needsStreaming, streamingQuantityHint and rd.maxStreamingQuantity.
🧹 Nitpick comments (1)
data/src/main/kotlin/com/vultisig/wallet/data/repositories/SwapQuoteRepository.kt (1)
289-292: Drop the force-unwrap onquote.data.error.This branch is already nullable-aware, so
!!adds an unnecessary crash edge and goes against the repo's Kotlin null-safety rule.Suggested cleanup
- is THORChainSwapQuoteDeserialized.Result -> - if (quote.data.error != null) - rapidError = SwapException.handleSwapException(quote.data.error!!) - else rapidData = quote.data + is THORChainSwapQuoteDeserialized.Result -> + quote.data.error?.let { error -> + rapidError = SwapException.handleSwapException(error) + } ?: run { + rapidData = quote.data + }As per coding guidelines: "Null safety: avoid !!, use ?.let, ?:"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@data/src/main/kotlin/com/vultisig/wallet/data/repositories/SwapQuoteRepository.kt` around lines 289 - 292, The code force-unwraps quote.data.error with !! inside the THORChainSwapQuoteDeserialized.Result branch; replace that unsafe unwrap with a null-safe handling (e.g., use quote.data.error?.let { rapidError = SwapException.handleSwapException(it) } ?: /*fallback*/ ) so rapidError is only set when error is non-null and no !! is used; locate the branch handling THORChainSwapQuoteDeserialized.Result and update the logic around quote.data.error, SwapException.handleSwapException, and rapidError to use ?.let or an Elvis operator instead of !!
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@data/src/main/kotlin/com/vultisig/wallet/data/repositories/SwapQuoteRepository.kt`:
- Around line 340-343: In the when expression in SwapQuoteRepository.kt that
handles THORChainSwapQuoteDeserialized (the branch checking is
THORChainSwapQuoteDeserialized.Error and the Result -> quote.data.takeIf {
it.error == null }), add a log call before returning null so non-exception
streaming failures are recorded: for THORChainSwapQuoteDeserialized.Error log
the error payload details (reason/message/stack) and for Result where
quote.data.error != null log the payload-level error and any relevant context
(asset IDs, request params) using the repository's existing logger/telemetry
instance (e.g., logger.warn or telemetry.capture) and then return null as
before. Ensure you include enough fields from quote/quote.data to make telemetry
useful.
---
Duplicate comments:
In
`@data/src/main/kotlin/com/vultisig/wallet/data/repositories/SwapQuoteRepository.kt`:
- Around line 312-317: The current computation in SwapQuoteRepository (variables
rapidSlippageBps, grossOut, feesTotal) floors the bps by integer division so
values just above the threshold (e.g., 300.01) are treated as 300; change the
gating to avoid flooring by performing the comparison using exact arithmetic
(e.g., compare feesTotal * 10000 to grossOut * STREAMING_SLIPPAGE_THRESHOLD_BPS)
or compute the division with rounding up/using BigDecimal so that needsStreaming
is true for any fractional bps above STREAMING_SLIPPING_THRESHOLD_BPS; update
the logic that sets needsStreaming and streamingQuantityHint (and any use of
rapidSlippageBps) accordingly, referencing rapidSlippageBps, grossOut,
feesTotal, STREAMING_SLIPPING_THRESHOLD_BPS, needsStreaming,
streamingQuantityHint and rd.maxStreamingQuantity.
---
Nitpick comments:
In
`@data/src/main/kotlin/com/vultisig/wallet/data/repositories/SwapQuoteRepository.kt`:
- Around line 289-292: The code force-unwraps quote.data.error with !! inside
the THORChainSwapQuoteDeserialized.Result branch; replace that unsafe unwrap
with a null-safe handling (e.g., use quote.data.error?.let { rapidError =
SwapException.handleSwapException(it) } ?: /*fallback*/ ) so rapidError is only
set when error is non-null and no !! is used; locate the branch handling
THORChainSwapQuoteDeserialized.Result and update the logic around
quote.data.error, SwapException.handleSwapException, and rapidError to use ?.let
or an Elvis operator instead of !!
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 670e8df9-b1d7-45d4-886f-4e3cb6a6947c
📒 Files selected for processing (2)
data/src/main/kotlin/com/vultisig/wallet/data/repositories/SwapQuoteRepository.ktdata/src/test/kotlin/com/vultisig/wallet/data/repositories/SwapQuoteRepositoryThorChainStreamingTest.kt
aminsato
left a comment
There was a problem hiding this comment.
A few non-blocking suggestions on the THORChain rapid/streaming fallback — the core logic and tests look solid.
premiumjibles
left a comment
There was a problem hiding this comment.
How do you do, fellow humans. I've had a look at your code.
Adds a slippage guard around THORChain rapid swaps (interval=0): when fees.total / gross_output exceeds 300 bps, fetch a parallel streaming quote (interval=1, streaming_quantity=max_streaming_quantity) and return whichever has the better expected_amount_out. Streaming-fetch failures fall back silently to rapid; a new pickBestQuote helper plus 9 unit tests cover all rapid/streaming combinations.
What's right
- Single tuning knob (STREAMING_SLIPPAGE_THRESHOLD_BPS) with documented disable-by-MAX_VALUE escape hatch — keeps the behaviour reversible without a code revert.
- Asymmetric fallback: rapid failure also triggers streaming, not just the slippage gate — covers the case where a pool is paused for rapid but streaming-acceptable.
- Tests exercise the four-quadrant matrix (rapid×streaming success/failure) plus the streaming-error-field branch the API distinguishes from HTTP errors.
Findings
[MAJOR] (confidence: med) — data/src/main/kotlin/com/vultisig/wallet/data/repositories/SwapQuoteRepository.kt:315
Slippage bps uses integer floor — values just above 3% gated out
data/src/main/kotlin/com/vultisig/wallet/data/repositories/SwapQuoteRepository.kt:315 — Also flagged by CodeRabbit. The bps formula feesTotal.multiply(10000).divide(grossOut).toInt() floors before the > STREAMING_SLIPPAGE_THRESHOLD_BPS comparison, so a swap whose true slippage is 300.99 bps reads as 300 and skips the streaming path. The PR contract is > 3% but the code implements floor(bps) > 300 (i.e. ≥301 bps). Consider comparing without flooring (e.g. feesTotal.multiply(10000) > grossOut.multiply(BigInteger.valueOf(STREAMING_SLIPPAGE_THRESHOLD_BPS.toLong()))) so the gate matches the PR description.
[MAJOR] (confidence: med) — data/src/main/kotlin/com/vultisig/wallet/data/repositories/SwapQuoteRepository.kt:360
Streaming quote expiry uses rapid's expiredAfter
data/src/main/kotlin/com/vultisig/wallet/data/repositories/SwapQuoteRepository.kt:360 — When the streaming quote wins, the returned SwapQuote.ThorChain is stamped with Clock.System.now() + expiredAfter (the rapid-path expiry constant), but a streaming swap with streaming_quantity=N, interval=1 runs for N THORChain blocks (~6s each) plus inbound/outbound — easily several minutes. If expiredAfter was tuned for rapid (single-block) lifetimes, users may see the quote expire mid-flow. Consider deriving expiry from streaming.totalSwapSeconds when the streaming branch is taken.
[MINOR] (confidence: med) — data/src/main/kotlin/com/vultisig/wallet/data/repositories/SwapQuoteRepository.kt:502
pickBestQuote ignores execution-time delta
data/src/main/kotlin/com/vultisig/wallet/data/repositories/SwapQuoteRepository.kt:502 — pickBestQuote picks streaming whenever streamingOut > rapidOut, even by a single base unit. A streaming swap that improves output by 0.001% but takes 5 minutes vs the rapid path's seconds is a UX regression. Consider requiring a meaningful delta (e.g. ≥1% improvement) before swapping, especially since the slippage gate already establishes that rapid is materially worse.
[MINOR] (confidence: med) — data/src/main/kotlin/com/vultisig/wallet/data/repositories/SwapQuoteRepository.kt:336
Streaming retry on rapid API error masks pool-suspended / pair-invalid signals
data/src/main/kotlin/com/vultisig/wallet/data/repositories/SwapQuoteRepository.kt:336 — When rapid returns Result(error='…') or an HTTP error, the new code unconditionally tries streaming. Some rapid-side errors (pool suspended, pair not supported) will reproduce on streaming; others (min amount in) won't. Consider only triggering streaming fallback for transient/network errors and propagating structured API errors.
[MINOR] (confidence: high) — data/src/main/kotlin/com/vultisig/wallet/data/repositories/SwapQuoteRepository.kt:328
streamingQuantity sent without lower-bound check
data/src/main/kotlin/com/vultisig/wallet/data/repositories/SwapQuoteRepository.kt:328 — streamingQuantityHint = rd.maxStreamingQuantity — if THORChain returns max_streaming_quantity = 0 for tiny inputs or paused pools, this PR sends streaming_quantity=0 which the API will likely reject. Consider rd.maxStreamingQuantity.takeIf { it > 0 }.
[QUESTION] (confidence: low) — data/src/test/kotlin/com/vultisig/wallet/data/repositories/SwapQuoteRepositoryThorChainStreamingTest.kt:117
No test asserts streaming memo flows to SwapQuote.ThorChain
data/src/test/kotlin/com/vultisig/wallet/data/repositories/SwapQuoteRepositoryThorChainStreamingTest.kt:117 — Tests assert expectedAmountOut matches but don't assert returned data.memo is the streaming memo (rather than rapid's). A SWAP:TRON.TRX:TDst:0/1/5 (streaming) vs SWAP:TRON.TRX:TDst (rapid) memo distinction is what gets signed downstream.
[NIT] (confidence: high) — data/src/main/kotlin/com/vultisig/wallet/data/repositories/SwapQuoteRepository.kt:675
STREAMING_SLIPPAGE_THRESHOLD_BPS exposed as public companion const
data/src/main/kotlin/com/vultisig/wallet/data/repositories/SwapQuoteRepository.kt:675 — const val STREAMING_SLIPPAGE_THRESHOLD_BPS = 300 in the companion is implicitly public. Consider private (or internal) so it doesn't accidentally become a load-bearing constant for other modules.
[NIT] (confidence: high) — data/src/main/kotlin/com/vultisig/wallet/data/repositories/SwapQuoteRepository.kt:305
Rapid-failure log drops the underlying error
data/src/main/kotlin/com/vultisig/wallet/data/repositories/SwapQuoteRepository.kt:305 — Timber.w("Rapid quote failed, trying streaming fallback") discards rapidError. Consider Timber.w(rapidError, "Rapid quote failed (%s); trying streaming fallback", rapidError?.message).
QA Report
| Check | Result |
|---|---|
| build/lint | not_run — sweep skipped checkout |
| tests | not_run — CI reports build-and-test passed |
| runtime probe | not applicable — mobile (Android) |
Dedup notes
- CodeRabbit: integer-floor on rapidSlippageBps comparison — file SwapQuoteRepository.kt around 312-317
- CodeRabbit: force-unwrap !! on quote.data.error inside null-checked branch — file SwapQuoteRepository.kt around 289-292
- CodeRabbit: missing exact-threshold test (300 bps boundary) — already on author's plate
- CodeRabbit: missing log/telemetry on non-exception streaming failures — already flagged
Verdict
COMMENT
🤖 review-pr@Claude-4.7 — single-agent run (Gemini second-pass deferred for sweep budget). See ~/.claude/skills/review-pr if this got something wrong.
premiumjibles
left a comment
There was a problem hiding this comment.
How do you do, fellow humans. I've had a look at your code.
Adds a slippage guard around THORChain rapid swaps (interval=0): when fees.total / gross_output exceeds 300 bps, fetch a parallel streaming quote (interval=1, streaming_quantity=max_streaming_quantity) and return whichever has the better expected_amount_out. Streaming-fetch failures fall back silently to rapid; a new pickBestQuote helper plus 9 unit tests cover all rapid/streaming combinations.
What's right
- Single tuning knob (STREAMING_SLIPPAGE_THRESHOLD_BPS) with documented disable-by-MAX_VALUE escape hatch — keeps the behaviour reversible without a code revert.
- Asymmetric fallback: rapid failure also triggers streaming, not just the slippage gate — covers the case where a pool is paused for rapid but streaming-acceptable.
- Tests exercise the four-quadrant matrix (rapid×streaming success/failure) plus the streaming-error-field branch the API distinguishes from HTTP errors.
Findings
[MAJOR] (confidence: med) — data/src/main/kotlin/com/vultisig/wallet/data/repositories/SwapQuoteRepository.kt:315
Slippage bps uses integer floor — values just above 3% gated out
data/src/main/kotlin/com/vultisig/wallet/data/repositories/SwapQuoteRepository.kt:315 — Also flagged by CodeRabbit. The bps formula feesTotal.multiply(10000).divide(grossOut).toInt() floors before the > STREAMING_SLIPPAGE_THRESHOLD_BPS comparison, so a swap whose true slippage is 300.99 bps reads as 300 and skips the streaming path. The PR contract is > 3% but the code implements floor(bps) > 300 (i.e. ≥301 bps). Consider comparing without flooring (e.g. feesTotal.multiply(10000) > grossOut.multiply(BigInteger.valueOf(STREAMING_SLIPPAGE_THRESHOLD_BPS.toLong()))) so the gate matches the PR description.
[MAJOR] (confidence: med) — data/src/main/kotlin/com/vultisig/wallet/data/repositories/SwapQuoteRepository.kt:360
Streaming quote expiry uses rapid's expiredAfter
data/src/main/kotlin/com/vultisig/wallet/data/repositories/SwapQuoteRepository.kt:360 — When the streaming quote wins, the returned SwapQuote.ThorChain is stamped with Clock.System.now() + expiredAfter (the rapid-path expiry constant), but a streaming swap with streaming_quantity=N, interval=1 runs for N THORChain blocks (~6s each) plus inbound/outbound — easily several minutes. If expiredAfter was tuned for rapid (single-block) lifetimes, users may see the quote expire mid-flow. Consider deriving expiry from streaming.totalSwapSeconds when the streaming branch is taken.
[MINOR] (confidence: med) — data/src/main/kotlin/com/vultisig/wallet/data/repositories/SwapQuoteRepository.kt:502
pickBestQuote ignores execution-time delta
data/src/main/kotlin/com/vultisig/wallet/data/repositories/SwapQuoteRepository.kt:502 — pickBestQuote picks streaming whenever streamingOut > rapidOut, even by a single base unit. A streaming swap that improves output by 0.001% but takes 5 minutes vs the rapid path's seconds is a UX regression. Consider requiring a meaningful delta (e.g. ≥1% improvement) before swapping, especially since the slippage gate already establishes that rapid is materially worse.
[MINOR] (confidence: med) — data/src/main/kotlin/com/vultisig/wallet/data/repositories/SwapQuoteRepository.kt:336
Streaming retry on rapid API error masks pool-suspended / pair-invalid signals
data/src/main/kotlin/com/vultisig/wallet/data/repositories/SwapQuoteRepository.kt:336 — When rapid returns Result(error='…') or an HTTP error, the new code unconditionally tries streaming. Some rapid-side errors (pool suspended, pair not supported) will reproduce on streaming; others (min amount in) won't. Consider only triggering streaming fallback for transient/network errors and propagating structured API errors.
[MINOR] (confidence: high) — data/src/main/kotlin/com/vultisig/wallet/data/repositories/SwapQuoteRepository.kt:328
streamingQuantity sent without lower-bound check
data/src/main/kotlin/com/vultisig/wallet/data/repositories/SwapQuoteRepository.kt:328 — streamingQuantityHint = rd.maxStreamingQuantity — if THORChain returns max_streaming_quantity = 0 for tiny inputs or paused pools, this PR sends streaming_quantity=0 which the API will likely reject. Consider rd.maxStreamingQuantity.takeIf { it > 0 }.
[QUESTION] (confidence: low) — data/src/test/kotlin/com/vultisig/wallet/data/repositories/SwapQuoteRepositoryThorChainStreamingTest.kt:117
No test asserts streaming memo flows to SwapQuote.ThorChain
data/src/test/kotlin/com/vultisig/wallet/data/repositories/SwapQuoteRepositoryThorChainStreamingTest.kt:117 — Tests assert expectedAmountOut matches but don't assert returned data.memo is the streaming memo (rather than rapid's). A SWAP:TRON.TRX:TDst:0/1/5 (streaming) vs SWAP:TRON.TRX:TDst (rapid) memo distinction is what gets signed downstream.
[NIT] (confidence: high) — data/src/main/kotlin/com/vultisig/wallet/data/repositories/SwapQuoteRepository.kt:675
STREAMING_SLIPPAGE_THRESHOLD_BPS exposed as public companion const
data/src/main/kotlin/com/vultisig/wallet/data/repositories/SwapQuoteRepository.kt:675 — const val STREAMING_SLIPPAGE_THRESHOLD_BPS = 300 in the companion is implicitly public. Consider private (or internal) so it doesn't accidentally become a load-bearing constant for other modules.
[NIT] (confidence: high) — data/src/main/kotlin/com/vultisig/wallet/data/repositories/SwapQuoteRepository.kt:305
Rapid-failure log drops the underlying error
data/src/main/kotlin/com/vultisig/wallet/data/repositories/SwapQuoteRepository.kt:305 — Timber.w("Rapid quote failed, trying streaming fallback") discards rapidError. Consider Timber.w(rapidError, "Rapid quote failed (%s); trying streaming fallback", rapidError?.message).
QA Report
| Check | Result |
|---|---|
| build/lint | not_run — sweep skipped checkout |
| tests | not_run — CI reports build-and-test passed |
| runtime probe | not applicable — mobile (Android) |
Dedup notes
- CodeRabbit: integer-floor on rapidSlippageBps comparison — file SwapQuoteRepository.kt around 312-317
- CodeRabbit: force-unwrap !! on quote.data.error inside null-checked branch — file SwapQuoteRepository.kt around 289-292
- CodeRabbit: missing exact-threshold test (300 bps boundary) — already on author's plate
- CodeRabbit: missing log/telemetry on non-exception streaming failures — already flagged
Verdict
COMMENT
🤖 review-pr@Claude-4.7 — single-agent run (Gemini second-pass deferred for sweep budget). See ~/.claude/skills/review-pr if this got something wrong.
Summary
interval=0)fees.total / gross_output > 3%(300 bps), fetches a streaming quote (interval=1,streaming_quantity=max_streaming_quantity) and returns whichever gives betterexpected_amount_outSTREAMING_SLIPPAGE_THRESHOLD_BPS = 300; set toInt.MAX_VALUEto disable without revertingChanges
ThorChainSwapQuoteRequest— adds optionalstreamingQuantityfield (defaultnull)ThorChainApi.getSwapQuotes— passesstreaming_quantityquery param when providedSwapQuoteRepository.getSwapQuote— implements the double-fetch + selection logicSwapQuoteRepositoryThorChainStreamingTest— 6 unit tests covering all four branches: below threshold, above + streaming better, above + streaming worse, streaming throws, streaming returns API error, streaming returnserrorfieldAcceptance criteria coverage
fees.total ≤ 3%→ streaming NOT fetchedfees.total > 3%→ streaming fetchedstreaming.expected_amount_out > rapid→ streaming memo surfaces to signingstreaming.expected_amount_out ≤ rapid→ rapid returnedCloses #4145
Test plan
./gradlew :data:testDebugUnitTest— all 6 new tests pass🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests