Merged
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds BTC→L‑BTC receive/refund support: Breez SDK wrappers (logging, fees, truncation), swap-status classification, debounced refundables polling, in‑flight refund tracking/reconciliation, vault address generation, concurrent limit fetching, view updates, tests, and documentation. Changes
Sequence DiagramsequenceDiagram
actor User
participant UI as "UI (Receive/Transactions)"
participant App as "App"
participant BreezClient as "BreezClient"
participant BreezSDK as "Breez SDK"
participant Daemon as "Daemon"
rect rgba(100,200,150,0.5)
Note over User,UI: Limits & Address Display
User->>UI: Open "Bitcoin" receive tab
UI->>App: Request onchain limits
App->>BreezClient: fetch_onchain_limits()
BreezClient->>BreezSDK: get_sdk()?.onchain_limits()
BreezSDK-->>BreezClient: min/max sats
BreezClient-->>App: limits
App-->>UI: show warning or generate button
end
rect rgba(100,150,200,0.5)
Note over User,UI: Deposit & Event
User->>UI: Generate swap address / deposit BTC
UI->>App: receive_onchain request
App->>BreezClient: receive_onchain()
BreezClient->>BreezSDK: get_sdk()?.receive_onchain()
BreezSDK-->>BreezClient: swap address
end
rect rgba(200,150,100,0.5)
Note over BreezSDK,App: Failure detection & refundables refresh
BreezSDK-->>App: SdkEvent::PaymentFailed (or refundable events)
App->>App: refresh_refundables_task() (debounced)
App->>BreezClient: list_refundables()
BreezClient->>BreezSDK: get_sdk()?.list_refundables()
BreezSDK-->>BreezClient: [RefundableSwap...]
BreezClient-->>App: refundables
App-->>UI: display refundable swaps
end
rect rgba(200,100,150,0.5)
Note over User,App: Submit refund
User->>UI: SubmitRefund (fee, refund address)
UI->>App: SubmitRefund
App->>App: insert in_flight_refund (txid=None)
App->>Daemon: get_new_address() (if vault chosen)
Daemon-->>App: BTC address
App->>BreezClient: refund_onchain_tx(...)
BreezClient->>BreezSDK: get_sdk()?.refund(...)
BreezSDK-->>BreezClient: RefundResponse {refund_tx_id}
BreezClient-->>App: refund_tx_id
App->>App: update in_flight_refund with txid
App-->>UI: show "Refund broadcast · <txid>"
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
coincube-gui/src/app/state/liquid/receive.rs (1)
1089-1142:⚠️ Potential issue | 🟠 MajorSeparate swap-limit loading from swap-limit availability.
fetch_limits()still models the whole lifecycle with the cacheOptions alone. After a failed fetch, the limits stayNone; incoincube-gui/src/app/view/liquid/receive.rsthat now renders the perpetual “Fetching swap limits” state and keeps the OnChain generate button hidden even though no request is running. Add explicit loading/error state (or an automatic retry path) instead of treatingNoneas both “missing” and “loading”.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@coincube-gui/src/app/state/liquid/receive.rs` around lines 1089 - 1142, fetch_limits currently treats the Option fields lightning_receive_limits and onchain_receive_limits as both "missing" and "loading", causing the UI to show a perpetual fetching state after a failed fetch; replace those Option fields with an explicit state enum (e.g., LimitsState::NotRequested | Loading | Ready(Limits) | Error(String)) or add separate loading/error flags, update fetch_limits to set the appropriate Loading state before spawning each Task, and on task completion send LiquidReceiveMessage variants that set Ready(limits) or Error(...) instead of leaving them None; finally update the view logic to read the new state enums (or flags) so the "Fetching swap limits" UI and the OnChain generate button reflect loading, success, and error independently of "no request made".coincube-gui/src/app/view/liquid/transactions.rs (1)
121-157:⚠️ Potential issue | 🟠 MajorDon't show the wallet-empty state when refundables exist.
Line 121 keys the empty state off
payments.is_empty()only. UnderAll/L-BTC, a user with no payments but at least one refundable swap will still see “Your Liquid wallet is ready...” above a live refund section, which is misleading. Computeshow_refundablesbefore this branch and suppress the empty state when refundables are present.Suggested fix
+ let show_refundables = + !refundables.is_empty() && matches!(asset_filter, AssetFilter::All | AssetFilter::LbtcOnly); + - if payments.is_empty() { + if payments.is_empty() && !show_refundables { // Empty state with Kage quote content = content.push( Column::new() @@ - let show_refundables = - !refundables.is_empty() && matches!(asset_filter, AssetFilter::All | AssetFilter::LbtcOnly); if show_refundables {Also applies to: 178-183
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@coincube-gui/src/app/view/liquid/transactions.rs` around lines 121 - 157, Compute the show_refundables boolean (based on refundables collection) before the payments-empty branch and change the empty-state condition from payments.is_empty() to payments.is_empty() && !show_refundables so the "Your Liquid wallet is ready..." UI is suppressed whenever refundables exist; update the same conditional at the secondary location that currently checks payments.is_empty() (the block around the other empty-state rendering at the later snippet) to use the new combined condition as well.coincube-gui/src/app/state/liquid/transactions.rs (1)
428-472:⚠️ Potential issue | 🟡 MinorClear the pending fee-priority state on fetch failure.
The
Nonebranch only emitsShowError. That leavesself.pending_fee_priorityset, so the clicked button stays on…and cannot be retried from the same priority until the user edits the field or picks another button.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@coincube-gui/src/app/state/liquid/transactions.rs` around lines 428 - 472, The failure branch currently only emits ShowError and leaves self.pending_fee_priority set, so the button stays in the "…" state; update the Task::perform callback in the Message::View(view::Message::RefundFeeratePrioritySelected(priority)) handler so that when rate is None it also clears pending_fee_priority (set it to None) — e.g. emit a View message that the view handler uses to set self.pending_fee_priority = None (or add a small new View message variant like RefundFeeratePriorityCleared and dispatch it alongside ShowError) so the spinner is removed and the button can be retried.
🧹 Nitpick comments (2)
coincube-gui/test_assets/global_settings.json (1)
84-84: Add trailing newline for consistency.JSON files should end with a newline character for better POSIX compliance and git diff handling.
📝 Proposed fix
Add a newline after the closing brace on line 84.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@coincube-gui/test_assets/global_settings.json` at line 84, The file ends with a closing brace '}' but is missing a trailing newline; update the JSON file by adding a single newline character after the final closing brace so the file terminates with a newline (ensure the closing '}' remains intact and only append the newline).coincube-gui/src/app/view/liquid/transactions.rs (1)
945-962: This regression test never exercises the production predicate.The test recomputes
matches!(filter, AssetFilter::All | AssetFilter::LbtcOnly)locally, so it will keep passing even ifliquid_transactions_view()regresses. Extract the visibility rule into a helper and assert that, or drive the actual view/data path instead.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@coincube-gui/src/app/view/liquid/transactions.rs` around lines 945 - 962, The test refundables_gated_out_of_usdt_filter is recomputing the visibility logic locally instead of exercising the production predicate (so it won't catch regressions in liquid_transactions_view); change the test to call the real predicate used by the view (or extract that predicate into a shared helper function, e.g. is_refundable_visible(filter: &AssetFilter) and use that from both liquid_transactions_view and the test) and assert its result for AssetFilter::All, AssetFilter::LbtcOnly, and AssetFilter::UsdtOnly rather than using matches! locally; update references to AssetFilter and liquid_transactions_view/refundable visibility code accordingly so the test covers production logic.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@coincube-gui/src/app/mod.rs`:
- Around line 1269-1282: The debounce currently sets self.last_refundables_fetch
before calling list_refundables(), which suppresses refreshes even when the call
fails; change refresh_refundables_task to not update last_refundables_fetch
until a successful result is observed by recording the timestamp inside handling
of Message::RefundablesLoaded(Ok(...)); instead introduce an in-flight boolean
(e.g., refundables_fetch_in_flight) used in refresh_refundables_task to prevent
duplicate concurrent requests, and only set last_refundables_fetch when
processing a successful list_refundables() result in the
Message::RefundablesLoaded handler.
In `@coincube-gui/src/app/state/liquid/transactions.rs`:
- Around line 276-288: The current RefundablesLoaded handler prematurely drops
optimistic in-flight refunds by using self.in_flight_refunds.retain(|addr, _|
returned.contains(addr)); instead change the retain predicate so it keeps
entries that are still awaiting completion: preserve any in_flight_refunds where
the RefundEntry.refund_txid.is_none() (optimistic, awaiting broadcast) or where
RefundEntry.submitted_at is within a short grace window (e.g. now - submitted_at
< GRACE_DURATION), and only drop entries that both are missing from returned and
either have a refund_txid Some(_) or have expired the grace window; update the
Message::RefundablesLoaded arm in the same match and ensure
SubmitRefund/RefundCompleted flows still clear entries on RefundCompleted.
---
Outside diff comments:
In `@coincube-gui/src/app/state/liquid/receive.rs`:
- Around line 1089-1142: fetch_limits currently treats the Option fields
lightning_receive_limits and onchain_receive_limits as both "missing" and
"loading", causing the UI to show a perpetual fetching state after a failed
fetch; replace those Option fields with an explicit state enum (e.g.,
LimitsState::NotRequested | Loading | Ready(Limits) | Error(String)) or add
separate loading/error flags, update fetch_limits to set the appropriate Loading
state before spawning each Task, and on task completion send
LiquidReceiveMessage variants that set Ready(limits) or Error(...) instead of
leaving them None; finally update the view logic to read the new state enums (or
flags) so the "Fetching swap limits" UI and the OnChain generate button reflect
loading, success, and error independently of "no request made".
In `@coincube-gui/src/app/state/liquid/transactions.rs`:
- Around line 428-472: The failure branch currently only emits ShowError and
leaves self.pending_fee_priority set, so the button stays in the "…" state;
update the Task::perform callback in the
Message::View(view::Message::RefundFeeratePrioritySelected(priority)) handler so
that when rate is None it also clears pending_fee_priority (set it to None) —
e.g. emit a View message that the view handler uses to set
self.pending_fee_priority = None (or add a small new View message variant like
RefundFeeratePriorityCleared and dispatch it alongside ShowError) so the spinner
is removed and the button can be retried.
In `@coincube-gui/src/app/view/liquid/transactions.rs`:
- Around line 121-157: Compute the show_refundables boolean (based on
refundables collection) before the payments-empty branch and change the
empty-state condition from payments.is_empty() to payments.is_empty() &&
!show_refundables so the "Your Liquid wallet is ready..." UI is suppressed
whenever refundables exist; update the same conditional at the secondary
location that currently checks payments.is_empty() (the block around the other
empty-state rendering at the later snippet) to use the new combined condition as
well.
---
Nitpick comments:
In `@coincube-gui/src/app/view/liquid/transactions.rs`:
- Around line 945-962: The test refundables_gated_out_of_usdt_filter is
recomputing the visibility logic locally instead of exercising the production
predicate (so it won't catch regressions in liquid_transactions_view); change
the test to call the real predicate used by the view (or extract that predicate
into a shared helper function, e.g. is_refundable_visible(filter: &AssetFilter)
and use that from both liquid_transactions_view and the test) and assert its
result for AssetFilter::All, AssetFilter::LbtcOnly, and AssetFilter::UsdtOnly
rather than using matches! locally; update references to AssetFilter and
liquid_transactions_view/refundable visibility code accordingly so the test
covers production logic.
In `@coincube-gui/test_assets/global_settings.json`:
- Line 84: The file ends with a closing brace '}' but is missing a trailing
newline; update the JSON file by adding a single newline character after the
final closing brace so the file terminates with a newline (ensure the closing
'}' remains intact and only append the newline).
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2ee20e93-6eb0-4543-bfa3-eed11909f616
📒 Files selected for processing (11)
coincube-gui/src/app/breez/client.rscoincube-gui/src/app/breez/mod.rscoincube-gui/src/app/breez/swap_status.rscoincube-gui/src/app/mod.rscoincube-gui/src/app/state/liquid/receive.rscoincube-gui/src/app/state/liquid/transactions.rscoincube-gui/src/app/view/liquid/receive.rscoincube-gui/src/app/view/liquid/transactions.rscoincube-gui/src/app/view/message.rscoincube-gui/test_assets/global_settings.jsondocs/BREEZ_BTC_RECEIVE.md
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@coincube-gui/src/app/mod.rs`:
- Around line 1275-1296: The debounced background poll and manual panel reload
both emit Message::RefundablesLoaded, causing the poll path to be suppressed or
overwritten by reloads; modify Message::RefundablesLoaded to carry an origin tag
(e.g., Poll vs Reload) plus the payload, update refresh_refundables_task() to
emit RefundablesLoaded with origin=Poll and only set/clear
refundables_fetch_in_flight and update last_refundables_fetch when handling
messages whose origin==Poll, and update LiquidTransactions::reload() to emit
RefundablesLoaded with origin=Reload so the reload path does not affect the
debounce/in-flight fields.
In `@coincube-gui/src/app/state/liquid/transactions.rs`:
- Around line 566-588: The RefundCompleted branch in liquid_transactions.update
is enqueuing a global view::Message::Close (inside Task::batch) which can close
whatever UI panel is active; remove the
Task::done(Message::View(view::Message::Close)) entry from the Task::batch in
the Message::RefundCompleted(Ok(response)) branch so only the toast is emitted
(the code already clears self.selected_refundable, refund_address, and
refund_feerate locally), or replace it with a localized/transaction-scoped close
message if you need to signal only the refund detail UI to close; specifically
edit the Message::RefundCompleted handling in transactions.rs to stop emitting
view::Message::Close.
- Around line 456-500: The async fee lookup must not overwrite newer user input:
modify the Task::perform callback initiated in
Message::View(view::Message::RefundFeeratePrioritySelected) to return a message
that includes the originating priority (or a generated request id) instead of
directly emitting RefundFeerateEdited/RefundFeeratePriorityFailed, then in the
update handling for that new message check that the included priority/request id
still matches self.pending_fee_priority before applying RefundFeerateEdited or
showing RefundFeeratePriorityFailed; use the existing symbols
RefundFeeratePrioritySelected, RefundFeerateEdited, RefundFeeratePriorityFailed
and the pending_fee_priority field to locate and gate the update logic.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: e4c46943-48c4-4faa-85eb-fa5c73115050
📒 Files selected for processing (4)
coincube-gui/src/app/breez/mod.rscoincube-gui/src/app/mod.rscoincube-gui/src/app/state/liquid/transactions.rscoincube-gui/src/app/view/message.rs
🚧 Files skipped from review as they are similar to previous changes (2)
- coincube-gui/src/app/view/message.rs
- coincube-gui/src/app/breez/mod.rs
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
coincube-gui/src/app/view/liquid/transactions.rs (1)
161-243:⚠️ Potential issue | 🟡 MinorTreat refundables as content when deciding whether to show the empty state.
Right now
payments.is_empty()still renders the empty-wallet panel even whenshow_refundablesis true, so a filtered L-BTC view can tell the user the wallet is empty and then list actionable refund cards underneath.💡 Minimal fix
- if payments.is_empty() { + let show_refundables = + !refundables.is_empty() && matches!(asset_filter, AssetFilter::All | AssetFilter::LbtcOnly); + + if payments.is_empty() && !show_refundables { // Empty state with Kage quote content = content.push( Column::new() .spacing(20) .width(Length::Fill) @@ - let show_refundables = - !refundables.is_empty() && matches!(asset_filter, AssetFilter::All | AssetFilter::LbtcOnly); if show_refundables {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@coincube-gui/src/app/view/liquid/transactions.rs` around lines 161 - 243, The empty-wallet check currently uses payments.is_empty() alone and renders the empty state even when refundables exist; move the show_refundables calculation (the let show_refundables = ... that matches AssetFilter and checks refundables.is_empty()) above the payments.is_empty() branch and change the condition to check both collections (e.g., if payments.is_empty() && !show_refundables or if payments.is_empty() && refundables.is_empty()) so refundables are treated as content and prevent the empty-wallet panel from showing when actionable refund cards exist.
🧹 Nitpick comments (1)
coincube-gui/src/app/state/liquid/transactions.rs (1)
580-580: Consider a safer fallback for unparseable fee rate.The
unwrap_or(1)fallback could result in a stuck transaction if the value is somehow empty or malformed at submission time. While UI validation should prevent this, a defensive approach would be to either fail the submission or use a minimum safe rate (e.g., the SDK'seconomy_fee).🛡️ Option: fail fast on invalid fee rate
- let fee_rate = self.refund_feerate.value.parse::<u32>().unwrap_or(1); + let fee_rate = match self.refund_feerate.value.parse::<u32>() { + Ok(rate) if rate > 0 => rate, + _ => { + self.refunding = false; + self.in_flight_refunds.remove(&swap_address); + return Task::done(Message::View(view::Message::ShowError( + "Invalid fee rate".to_string(), + ))); + } + };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@coincube-gui/src/app/state/liquid/transactions.rs` at line 580, The line using self.refund_feerate.value.parse::<u32>().unwrap_or(1) can silently fall back to 1 and cause stuck transactions; update the refund submission logic in transactions.rs (the code that reads refund_feerate.value and parses it) to handle parse errors defensively: either return/propagate an error to abort submission when parsing fails (fail-fast) or replace the unwrap_or(1) with a bounded minimum from the SDK (e.g., use the SDK's economy_fee or a defined MIN_SAFE_FEE constant) and log/notify the user; ensure you change the parsing site so it validates the parsed u32 and does not silently default to 1.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@coincube-gui/src/app/view/liquid/transactions.rs`:
- Around line 716-724: The Vault-address shortcut button in
refundable_detail_view is shown even in walletless mode; thread a boolean flag
(e.g., has_vault: bool) into refundable_detail_view (and the analogous view at
857-876) from the caller and use it to conditionally render the
GenerateVaultRefundAddress UI element so it is hidden when has_vault is false;
update the callers (where the view is invoked) to pass cache.has_vault (or the
equivalent state flag) through to the view so the button is only displayed when
a Vault is configured.
---
Outside diff comments:
In `@coincube-gui/src/app/view/liquid/transactions.rs`:
- Around line 161-243: The empty-wallet check currently uses payments.is_empty()
alone and renders the empty state even when refundables exist; move the
show_refundables calculation (the let show_refundables = ... that matches
AssetFilter and checks refundables.is_empty()) above the payments.is_empty()
branch and change the condition to check both collections (e.g., if
payments.is_empty() && !show_refundables or if payments.is_empty() &&
refundables.is_empty()) so refundables are treated as content and prevent the
empty-wallet panel from showing when actionable refund cards exist.
---
Nitpick comments:
In `@coincube-gui/src/app/state/liquid/transactions.rs`:
- Line 580: The line using self.refund_feerate.value.parse::<u32>().unwrap_or(1)
can silently fall back to 1 and cause stuck transactions; update the refund
submission logic in transactions.rs (the code that reads refund_feerate.value
and parses it) to handle parse errors defensively: either return/propagate an
error to abort submission when parsing fails (fail-fast) or replace the
unwrap_or(1) with a bounded minimum from the SDK (e.g., use the SDK's
economy_fee or a defined MIN_SAFE_FEE constant) and log/notify the user; ensure
you change the parsing site so it validates the parsed u32 and does not silently
default to 1.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 96b86226-06cb-4f72-8374-1b6aa23ff792
📒 Files selected for processing (7)
coincube-gui/src/app/message.rscoincube-gui/src/app/mod.rscoincube-gui/src/app/state/liquid/overview.rscoincube-gui/src/app/state/liquid/send.rscoincube-gui/src/app/state/liquid/transactions.rscoincube-gui/src/app/view/liquid/transactions.rscoincube-gui/src/app/view/message.rs
🚧 Files skipped from review as they are similar to previous changes (1)
- coincube-gui/src/app/view/message.rs
…/coincube into feature/breez-liquid-refund
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
coincube-gui/src/app/view/liquid/transactions.rs (1)
74-81: Byte-based slicing is safe for addresses/txids but consider documenting the constraint.The
&s[..prefix_len]slicing is byte-indexed, which would panic on multi-byte UTF-8 boundaries. This is fine for Bitcoin addresses and txids (ASCII-only), but adding a brief doc comment noting this constraint would prevent misuse.📝 Optional: Add doc comment
/// Truncate a long on-chain address / txid like `bc1p7g…7ff6v` so it fits /// inside a card without overflowing. Used for display only. +/// +/// # Panics +/// Panics if `s` contains multi-byte UTF-8 characters and slicing lands on +/// a character boundary. Safe for hex strings, base58, and bech32 addresses. fn truncate_middle(s: &str, prefix_len: usize, suffix_len: usize) -> String {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@coincube-gui/src/app/view/liquid/transactions.rs` around lines 74 - 81, The truncate_middle function uses byte-indexed slicing (&s[..prefix_len], &s[s.len() - suffix_len..]) which will panic on multi-byte UTF-8 boundaries; update the function by adding a brief doc comment above truncate_middle stating it expects ASCII-only inputs (e.g., Bitcoin addresses/txids) or otherwise ensure callers provide byte-safe strings, and reference the byte slices in the comment so future maintainers know why byte slicing is used.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@coincube-gui/src/app/breez/swap_status.rs`:
- Around line 128-132: Fix the formatting of the match pattern in the
PaymentType::Send arm: rewrite the is_refund_leg binding to match the rustfmt
style for struct patterns (PaymentDetails::Bitcoin { refund_tx_id: Some(_), ..
}) on a single line or otherwise formatted per rustfmt so cargo fmt passes;
locate the is_refund_leg declaration in the PaymentType::Send branch and reflow
the PaymentDetails::Bitcoin pattern to conform to rustfmt (or run `cargo fmt --
coincube-gui/src/app/breez/swap_status.rs`) to apply the correct layout.
In `@coincube-gui/src/app/state/liquid/transactions.rs`:
- Around line 600-626: The current RefundCompleted handler
(Message::RefundCompleted) populates refund_txid by finding the first
in_flight_refunds entry with refund_txid == None, which is racy because
HashMap::values_mut() is unordered; change the flow so RefundCompleted carries
the originating swap_address (or other unique swap key) alongside the response,
then use that swap_address to look up the exact in_flight_refunds entry (e.g.,
self.in_flight_refunds.get_mut(&swap_address)) and set entry.refund_txid =
Some(txid), and update every place that constructs/sends
Message::RefundCompleted to include the swap_address so the mapping is
deterministic.
---
Nitpick comments:
In `@coincube-gui/src/app/view/liquid/transactions.rs`:
- Around line 74-81: The truncate_middle function uses byte-indexed slicing
(&s[..prefix_len], &s[s.len() - suffix_len..]) which will panic on multi-byte
UTF-8 boundaries; update the function by adding a brief doc comment above
truncate_middle stating it expects ASCII-only inputs (e.g., Bitcoin
addresses/txids) or otherwise ensure callers provide byte-safe strings, and
reference the byte slices in the comment so future maintainers know why byte
slicing is used.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2b56b1a6-d6bd-43db-b695-dfe1cb6dc82c
📒 Files selected for processing (3)
coincube-gui/src/app/breez/swap_status.rscoincube-gui/src/app/state/liquid/transactions.rscoincube-gui/src/app/view/liquid/transactions.rs
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@coincube-gui/src/app/breez/swap_status.rs`:
- Line 63: The enum variant BtcSwapReceiveStatus::Completed is currently
rendered as "Received", which mislabels outgoing sends; change the display
string for Self::Completed in the match producing the label (the arm currently
returning "Received") to a neutral term like "Completed" or "Settled" so it no
longer implies inbound receipt, and leave the existing mapping logic that maps
PaymentType::Send to BtcSwapReceiveStatus::Completed unchanged.
In `@coincube-gui/src/app/mod.rs`:
- Around line 1946-1957: The new SdkEvent logging emits full details.tx_id
(e.g., in the SdkEvent::PaymentRefundable, PaymentRefundPending and
PaymentRefunded branches) which reintroduces linking; update those log lines to
truncate the tx_id the same way you do for swap identifiers in
coincube-gui/src/app/breez/client.rs (the truncation utility used around lines
571-591). Locate the log::info! calls that reference details.tx_id in the
SdkEvent::PaymentRefundable / PaymentRefundPending / PaymentRefunded branches
and replace the direct details.tx_id with the existing truncation helper (or the
same truncation logic) so logs contain only the truncated identifier.
- Around line 2068-2086: The background poll handler currently forwards
RefundablesPolled(Err(_)) into panels.liquid_transactions.update which becomes a
ShowError toast; change the logic in the Message::RefundablesPolled arm so that
only successful results are forwarded as Message::RefundablesLoaded to
LiquidTransactions::update (keep clearing self.refundables_fetch_in_flight and
only set last_refundables_fetch on Ok), and for Err do not call
self.panels.liquid_transactions.update (optionally log locally or swallow the
error) to avoid surfacing background poll failures as global toasts.
In `@coincube-gui/src/app/state/liquid/transactions.rs`:
- Around line 536-563: The async refund-address path
(Message::View(view::Message::GenerateVaultRefundAddress)) must carry a request
id/token through the Task::perform so late completions don't overwrite newer
user edits; when dispatching the task generate a unique token, store it on the
transaction state (e.g. pending_refund_request_id), include that token in the
completion message instead of the bare RefundAddressEdited, and in handling of
Message::View(view::Message::RefundAddressEdited(token, addr)) only apply the
addr if token matches the current pending_refund_request_id (otherwise ignore);
on error, include the token in the ShowError completion similarly and
clear/ignore stale tokens as you did for the fee-priority flow
(daemon.get_new_address(), GenerateVaultRefundAddress, RefundAddressEdited,
ShowError).
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: a6cc8536-5a86-4f9d-bbc6-81af96b33f5a
📒 Files selected for processing (4)
coincube-gui/src/app/breez/swap_status.rscoincube-gui/src/app/message.rscoincube-gui/src/app/mod.rscoincube-gui/src/app/state/liquid/transactions.rs
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 7c1ba70. Configure here.
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Note
Medium Risk
Touches the BTC↔L-BTC swap refund path and event-driven state updates; bugs could hide refundable swaps, mislabel swap status, or cause incorrect refund UI state, but changes are scoped to the Liquid wallet GUI and add tests/guards.
Overview
Improves the Liquid wallet’s BTC onchain (BTC→L-BTC) swap refund flow by adding debounced, event-driven
list_refundables()polling and routing poll results directly to the Transactions panel so refundable swaps surface even when the user isn’t on that screen.Introduces a centralized swap lifecycle model (
BtcSwapReceiveStatus+classify_payment) and uses it in payment detail status rendering to avoid misclassifying non-refund Bitcoin sends as refunds and to better distinguish confirmation vs swap-progress states.Upgrades the refund UX: supports “Use Vault address” (async Vault address fetch with stale-response guards), adds fee-priority button spinners and a fallback to SDK
recommended_fees(), tracks in-flight refunds to keep cards visible with “broadcasting/broadcast” labels, gates refundables out of the USDt filter, and adds privacy-preserving address/txid truncation shared between UI and logging. Also fetches onchain receive limits eagerly and blocks address generation until limits are loaded, updates receive copy (“On-chain”→“Bitcoin”), and adds docs/tests covering truncation, status classification, and in-flight reconciliation.Reviewed by Cursor Bugbot for commit 8e4c72f. Bugbot is set up for automated code reviews on this repo. Configure here.
Summary by CodeRabbit
New Features
Bug Fixes
UI/UX Improvements
Documentation
Tests