feat: add FeeRateField component to set fee rates for swaps - #2432
Open
im-adithya wants to merge 10 commits into
Open
feat: add FeeRateField component to set fee rates for swaps#2432im-adithya wants to merge 10 commits into
im-adithya wants to merge 10 commits into
Conversation
Contributor
|
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 a shared fee-rate field, threads internal swap fee-rate data through the frontend and backend, and updates several on-chain labels and messages. ChangesFee Rate UI and Internal Swap Flow
Sequence Diagram(s)sequenceDiagram
participant SwapInForm
participant InitiateSwapIn API
participant SwapsService
participant LnClient
participant SwapInStatus
SwapInForm->>InitiateSwapIn API: submit internalPayment + feeRate
InitiateSwapIn API->>SwapsService: SwapIn(amountSat, autoSwap, internalPayment, feeRate)
SwapsService->>LnClient: RedeemOnchainFunds(..., feeRate)
LnClient-->>SwapsService: lockupTxId
SwapsService-->>InitiateSwapIn API: SwapResponse
SwapInForm->>SwapInStatus: navigate with ?internal=true
SwapInStatus->>SwapInStatus: derive isInternalSwap from search params
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly Related PRs
Suggested Reviewers
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 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (2)
frontend/src/screens/wallet/swap/index.tsx (1)
320-331: 💤 Low valueConsider adding
aria-labelfor better accessibility.Similar to the send flow, this button triggers fee rate editing but lacks an explicit accessible label. Adding
aria-label="Edit fee rate"would improve screen reader experience.♿ Suggested accessibility improvement
<button type="button" className="flex items-center gap-2 cursor-pointer" + aria-label="Edit fee rate" onClick={() => setEditFee(true)} >🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/screens/wallet/swap/index.tsx` around lines 320 - 331, The fee-edit button in the Swap screen (the button using setEditFee, feeRate and rendering PencilIcon) lacks an accessible name; add an aria-label (e.g., aria-label="Edit fee rate") to the button element so screen readers announce its purpose, and ensure the label remains accurate when feeRate is loading (the Loading state) or present.frontend/src/screens/wallet/send/Onchain.tsx (1)
214-225: 💤 Low valueConsider adding
aria-labelfor better accessibility.The button element triggers edit mode but lacks an explicit accessible label. While the button contains visual content (fee rate text and pencil icon), an
aria-label="Edit fee rate"would improve screen reader experience by clearly announcing the button's purpose.♿ Suggested accessibility improvement
<button type="button" className="flex items-center gap-2 cursor-pointer" + aria-label="Edit fee rate" onClick={() => setEditFee(true)} >🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/screens/wallet/send/Onchain.tsx` around lines 214 - 225, The button that toggles edit mode (onClick={() => setEditFee(true)}) lacks an accessible label; update the <button> element that renders feeRate/Loading and the PencilIcon to include aria-label="Edit fee rate" (or similar) so screen readers announce its purpose, ensuring the interactive element remains unchanged otherwise and still uses setEditFee, feeRate, Loading and PencilIcon as before.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@frontend/src/screens/wallet/send/Onchain.tsx`:
- Around line 214-225: The button that toggles edit mode (onClick={() =>
setEditFee(true)}) lacks an accessible label; update the <button> element that
renders feeRate/Loading and the PencilIcon to include aria-label="Edit fee rate"
(or similar) so screen readers announce its purpose, ensuring the interactive
element remains unchanged otherwise and still uses setEditFee, feeRate, Loading
and PencilIcon as before.
In `@frontend/src/screens/wallet/swap/index.tsx`:
- Around line 320-331: The fee-edit button in the Swap screen (the button using
setEditFee, feeRate and rendering PencilIcon) lacks an accessible name; add an
aria-label (e.g., aria-label="Edit fee rate") to the button element so screen
readers announce its purpose, and ensure the label remains accurate when feeRate
is loading (the Loading state) or present.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d92108aa-1cd1-43e6-82f8-4dc3bab82dbd
📒 Files selected for processing (5)
frontend/src/components/FeeRateField.tsxfrontend/src/screens/wallet/WithdrawOnchainFunds.tsxfrontend/src/screens/wallet/send/Onchain.tsxfrontend/src/screens/wallet/swap/SwapInStatus.tsxfrontend/src/screens/wallet/swap/index.tsx
rolznz
reviewed
Jun 10, 2026
rolznz
reviewed
Jun 10, 2026
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
frontend/src/components/FeeRateField.tsx (1)
30-39: 💤 Low valueConsider streamlining the dependency array.
The effect includes
onFeeRateChangein the dependency array. SinceonFeeRateChangeissetFeeRatefromuseState(which is stable across renders), including it may cause unnecessary effect re-evaluation. While React 19 tolerates this, you could exclude it for cleaner deps:- }, [feeRate, onFeeRateChange, recommendedFees]); + }, [feeRate, recommendedFees]);Alternatively, if you want to keep the dep for explicitness, add an ESLint disable comment to document the intentional inclusion.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/FeeRateField.tsx` around lines 30 - 39, The useEffect that sets the default fee currently lists onFeeRateChange in its dependency array even though it is the stable state setter (setFeeRate), causing an unnecessary dep; update the dependency array for the effect that references useEffect, hasInitializedDefaultFee, recommendedFees, feeRate, and onFeeRateChange by removing onFeeRateChange (leaving [feeRate, recommendedFees]) to avoid spurious re-runs, or if you prefer to keep it for explicitness, add an inline ESLint disable comment (e.g., // eslint-disable-next-line react-hooks/exhaustive-deps) immediately above the useEffect to document the intentional inclusion.frontend/src/screens/wallet/send/Onchain.tsx (1)
111-159: 💤 Low valueConsider adding defensive validation for feeRate.
OnchainFormsubmits whenfeeRateis converted to a number (line 129:feeRate: +feeRate). IffeeRateis an empty string (e.g., if the user submits beforeFeeRateFieldauto-initializes),+""evaluates to0, which may be rejected by the API or cause unexpected behavior.While the UX mitigates this (FeeRateField shows a loading state and auto-initializes quickly, plus HTML5
requiredvalidation in edit mode), adding explicit validation would be more defensive:🛡️ Suggested defensive check
const onSubmit = async (event: React.FormEvent<HTMLFormElement>) => { event.preventDefault(); try { if (!balances) { return; } + if (!feeRate || +feeRate <= 0) { + throw new Error("Please wait for fee rate to load or enter a valid fee rate."); + } if (balances.onchain.spendableSat <= ONCHAIN_DUST_SATS) { throw new Error( "You currently don't have enough sats to pay for an on-chain transaction. Consider swapping from Lightning Balance." ); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/screens/wallet/send/Onchain.tsx` around lines 111 - 159, The onSubmit handler currently converts feeRate with +feeRate which turns an empty string into 0; update onSubmit to defensively validate feeRate before creating the RedeemOnchainFundsRequest: ensure feeRate is present, parsable to a positive number (e.g., Number(feeRate) > 0), show a user-facing error (toast.error) and return early (ensuring setLoading(false) is called) if validation fails; reference the feeRate React state, the onSubmit function, and the RedeemOnchainFundsRequest payload when making this change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@frontend/src/screens/wallet/send/Onchain.tsx`:
- Around line 189-191: OnchainForm currently renders FeeRateField
unconditionally; mirror the conditional used in WithdrawOnchainFunds so fee rate
input is only shown for backends that support it (check backendType === "LDK" ||
backendType === "LND"). Update the JSX in OnchainForm to wrap FeeRateField (and
use of setFeeRate/feeRate state) in the same backendType conditional, ensuring
RedeemOnchainFundsRequest.feeRate remains optional for unsupported backends and
no feeRate is sent when backendType is not LDK or LND.
---
Nitpick comments:
In `@frontend/src/components/FeeRateField.tsx`:
- Around line 30-39: The useEffect that sets the default fee currently lists
onFeeRateChange in its dependency array even though it is the stable state
setter (setFeeRate), causing an unnecessary dep; update the dependency array for
the effect that references useEffect, hasInitializedDefaultFee, recommendedFees,
feeRate, and onFeeRateChange by removing onFeeRateChange (leaving [feeRate,
recommendedFees]) to avoid spurious re-runs, or if you prefer to keep it for
explicitness, add an inline ESLint disable comment (e.g., //
eslint-disable-next-line react-hooks/exhaustive-deps) immediately above the
useEffect to document the intentional inclusion.
In `@frontend/src/screens/wallet/send/Onchain.tsx`:
- Around line 111-159: The onSubmit handler currently converts feeRate with
+feeRate which turns an empty string into 0; update onSubmit to defensively
validate feeRate before creating the RedeemOnchainFundsRequest: ensure feeRate
is present, parsable to a positive number (e.g., Number(feeRate) > 0), show a
user-facing error (toast.error) and return early (ensuring setLoading(false) is
called) if validation fails; reference the feeRate React state, the onSubmit
function, and the RedeemOnchainFundsRequest payload when making this change.
🪄 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: 8b6c2f5f-88fd-41ee-8064-bdc906dcdc02
📒 Files selected for processing (8)
frontend/src/components/CloseChannelDialogContent.tsxfrontend/src/components/FeeRateField.tsxfrontend/src/screens/channels/CurrentChannelOrder.tsxfrontend/src/screens/wallet/WithdrawOnchainFunds.tsxfrontend/src/screens/wallet/send/Onchain.tsxfrontend/src/screens/wallet/swap/SwapInStatus.tsxfrontend/src/screens/wallet/swap/SwapOutStatus.tsxfrontend/src/screens/wallet/swap/index.tsx
✅ Files skipped from review due to trivial changes (3)
- frontend/src/components/CloseChannelDialogContent.tsx
- frontend/src/screens/wallet/swap/SwapOutStatus.tsx
- frontend/src/screens/channels/CurrentChannelOrder.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- frontend/src/screens/wallet/swap/SwapInStatus.tsx
- frontend/src/screens/wallet/swap/index.tsx
rolznz
reviewed
Jun 11, 2026
| `/wallet/swap/in/status/${swapInResponse.swapId}${swapFrom === "internal" ? "?internal=true" : ""}` | ||
| `/wallet/swap/in/status/${swapInResponse.swapId}${ | ||
| swapFrom === "internal" | ||
| ? `?internal=true&feeRate=${encodeURIComponent(feeRate)}` |
Member
There was a problem hiding this comment.
Isn't it weird actually we pass the fee rate here rather than to the swap endpoint itself when creating the swap?
Member
There was a problem hiding this comment.
Then we can remove this effect that runs on page load (which is potentially a bit dodgy)
Member
There was a problem hiding this comment.
For this we should add a new parameter (e.g. ?internal=true to the api call) so that by default it works the same as now (for backward compatibility / hub CLI)
Member
Author
There was a problem hiding this comment.
I added it to InitiateSwapRequest as InternalPayment
Member
|
Nice change! 💪 |
Closed
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@api/api.go`:
- Around line 1053-1058: The SwapIn flow in api.go forwards user-provided
FeeRate directly into wallet spending without validation. Add boundary
validation in the swap initiation path before calling GetSwapsService().SwapIn,
rejecting FeeRate values of 0 and capping excessive values to an acceptable
maximum, while keeping the service call itself trusted. Use the existing
initiateSwapInRequest handling and SwapIn call site to place the check, and
return a clear error before any funding attempt if the FeeRate is invalid.
In `@frontend/src/screens/wallet/swap/index.tsx`:
- Around line 135-140: Avoid sending a coerced fee rate in the swap payload: the
feeRate handling in the internalPayment branch currently uses parseInt(feeRate),
which can truncate values like decimals and turn invalid input into NaN/null.
Update the payload-building logic in the swap screen to validate feeRate as a
positive integer before including it, and only add the feeRate field when the
value is explicitly valid.
In `@swaps/swaps_service.go`:
- Around line 501-524: The internal payment path in `swapIn` should not return a
failure after `svc.lnClient.RedeemOnchainFunds` has already broadcast the lockup
transaction. Keep the successfully returned `lockupTxId` and, if the
`svc.db.Model(&dbSwap).Updates(...)` call fails, log the DB error but continue
so the swap is not marked failed and `startSwapInListener` can still run. Update
the `internalPayment` block to preserve the funded swap state using
`dbSwap.LockupTxId`/`lockupTxId` even when persistence fails.
🪄 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: af616334-3e4c-4c59-986b-497d38b853c9
📒 Files selected for processing (6)
api/api.goapi/models.gofrontend/src/screens/wallet/swap/SwapInStatus.tsxfrontend/src/screens/wallet/swap/index.tsxfrontend/src/types.tsswaps/swaps_service.go
rolznz
reviewed
Jun 25, 2026
rolznz
reviewed
Jun 25, 2026
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
2 participants
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.
Fixes #2429
Summary by CodeRabbit