fix: reserve fee for unconfidential Liquid claims - #1586
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis change adds Liquid unconfidential claim surcharge handling, propagates actual claimed amounts through swap execution, updates fee and UI calculations, and adds unit, regtest, integration, and end-to-end coverage. ChangesLiquid unconfidential claim handling
Quote and display amount integration
Integration and end-to-end flows
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant CreateButton
participant Fees
participant ClaimFlow
participant Liquid
User->>CreateButton: select unconfidential Liquid destination
CreateButton->>Fees: quote claim surcharge
Fees-->>CreateButton: return surcharge-aware fees
CreateButton->>ClaimFlow: create swap with adjusted amounts
ClaimFlow->>Liquid: construct claim transaction
Liquid-->>ClaimFlow: return landed claimed amount
ClaimFlow-->>User: display landed receive amount
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed: dependency version conflict. Check your lock file or package.json. 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 |
21b0e2a to
137a976
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@src/status/TransactionLockupFailed.tsx`:
- Around line 200-202: Update the surcharge handling in TransactionLockupFailed,
including the corresponding logic at the additional referenced location, so
claimSurchargeForSwap is deducted only when the displayed amount represents the
Boltz claim output. Preserve the full post-position DEX output for
unconfidential Liquid swaps, matching TransactionClaimed’s behavior.
🪄 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 Plus
Run ID: 378a5f2d-d1a0-4a06-bdb1-cddbabd0085f
📒 Files selected for processing (32)
e2e/chainSwaps/chainSwaps.spec.tse2e/chainSwaps/quoteAcceptanceCrash.spec.tse2e/chainSwaps/zeroAmount.spec.tse2e/reverseSwap.spec.tse2e/utils.tspackages/boltz-swaps/integration/regtest.tspackages/boltz-swaps/integration/reverseSwap.regtest.spec.tspackages/boltz-swaps/src/chain.tspackages/boltz-swaps/src/reverse.tspackages/boltz-swaps/src/utxo/claim.tspackages/boltz-swaps/src/utxo/index.tspackages/boltz-swaps/tests/chainUtxo.spec.tspackages/boltz-swaps/tests/reverse.spec.tspackages/boltz-swaps/tests/utxo/claim.spec.tspackages/boltz-swaps/tests/utxo/claimReverse.spec.tspackages/boltz-swaps/tests/utxo/claimUnconfidential.spec.tspackages/boltz-swaps/tests/utxo/claimUnconfidentialEndToEnd.spec.tssrc/components/CreateButton.tsxsrc/components/Fees.tsxsrc/components/OptimizedRoute.tsxsrc/context/Pay.tsxsrc/pages/ClaimRescue.tsxsrc/status/TransactionClaimed.tsxsrc/status/TransactionLockupFailed.tsxsrc/utils/magicRoutingHint.tssrc/utils/unconfidential.tstests/components/Fees.spec.tsxtests/context/Pay.spec.tsxtests/status/TransactionClaimed.spec.tsxtests/status/TransactionLockupFailedAutoAccept.spec.tsxtests/utils/magicRoutingHint.spec.tstests/utils/unconfidential.spec.ts
137a976 to
e55829d
Compare
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/status/TransactionClaimed.tsx (1)
95-111: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winGuard post-bridge swaps when subtracting the claim surcharge.
TransactionLockupFailed.tsxskips the surcharge whenevergetFinalAssetReceive(...) !== assetReceive, because post-position DEX or bridge outputs are in a different asset.TransactionClaimed.tsxonly checks the post-position DEX case, so a Reverse/Chain swap withbridge.position === SwapPosition.Poststill subtractsclaimSurchargeForSwap()from the persistedreceiveAmount, whilereceiveDenomination()formats the bridge destination asset. Mirror the final-asset guard instead of only excluding post-DEX.🤖 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 `@src/status/TransactionClaimed.tsx` around lines 95 - 111, The receiveAmount function should skip claimSurchargeForSwap whenever the final receive asset differs from the original asset, including post-position bridge and DEX outputs. Mirror the final-asset guard used by TransactionLockupFailed.tsx while preserving the existing post-position DEX quoteAmount handling and receiveDenomination-compatible formatting.
🧹 Nitpick comments (3)
packages/boltz-swaps/src/utxo/claim.ts (1)
328-366: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider noting the divergence between
unconfidentialClaimSurchargeandneedsBlindedOpReturn.The exported helper decides purely from the destination address, while the claim path additionally requires blinded inputs. For an unblinded Liquid lockup, the quote reserves 6 sat that the claim never spends (user simply lands 6 sat more), so this is benign — but a short comment on the exported helper would prevent it from being read as an exact mirror of the claim-side condition.
🤖 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 `@packages/boltz-swaps/src/utxo/claim.ts` around lines 328 - 366, The exported unconfidentialClaimSurcharge helper should explicitly document that it is destination-based and may differ from the claim-side needsBlindedOpReturn condition, which also requires blinded inputs. Add a brief comment clarifying that unblinded Liquid lockups may still receive the reserved 6 sat without spending it, while preserving the existing behavior.packages/boltz-swaps/integration/reverseSwap.regtest.spec.ts (1)
189-219: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUnexplained
-1in fee assertion.
claimFee + liquidUnconfidentialClaimExtra - 1lacks a comment explaining the off-by-one. The sibling test at Line 221-222 documents its rationale; a similar one-liner here (likely relay-fee truncation, mirrored ine2e/reverseSwap.spec.ts's "truncating division" comment) would help future maintainers understand the exact expected value isn't accidental.🤖 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 `@packages/boltz-swaps/integration/reverseSwap.regtest.spec.ts` around lines 189 - 219, The fee assertion in the “is accepted by Elements and pays the surcharge out of the claim output” test needs an inline comment explaining the intentional -1 adjustment. Add a concise rationale referencing relay-fee truncation, matching the documented explanation in the sibling test and mirrored reverse-swap coverage, without changing the expected fee value.e2e/utils.ts (1)
497-508: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicates
waitForUTXOs's axios call; consider consolidating.
getAddressUtxosre-implements the sameGET .../address/${address}/utxocall already used bywaitForUTXOs(Lines 509-528), and re-declares an inline UTXO shape instead of reusing the existingUTXOtype. Consider havingwaitForUTXOscallgetAddressUtxosinternally to avoid drift between the two.♻️ Proposed consolidation
export const getAddressUtxos = async ( asset: AssetType, address: string, -): Promise<{ txid: string; vout: number; value?: number }[]> => +): Promise<UTXO[]> => ( - await axios.get<{ txid: string; vout: number; value?: number }[]>( + await axios.get<UTXO[]>( `${config.assets![asset].blockExplorerApis![0].normal}/address/${address}/utxo`, ) ).data; export const waitForUTXOs = async ( asset: AssetType, address: string, amount: number, ) => { await expect - .poll( - async () => { - const utxos = ( - await axios.get<UTXO[]>( - `${config.assets![asset].blockExplorerApis![0].normal}/address/${address}/utxo`, - ) - ).data; - - return utxos.length === amount; - }, - { timeout: 30_000 }, - ) + .poll(async () => (await getAddressUtxos(asset, address)).length === amount, { + timeout: 30_000, + }) .toBe(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 `@e2e/utils.ts` around lines 497 - 508, Consolidate the duplicated explorer request by updating waitForUTXOs to call getAddressUtxos instead of issuing its own axios GET to the address UTXO endpoint. Reuse the existing UTXO type for getAddressUtxos and waitForUTXOs so both paths share the same response shape and cannot drift.
🤖 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.
Outside diff comments:
In `@src/status/TransactionClaimed.tsx`:
- Around line 95-111: The receiveAmount function should skip
claimSurchargeForSwap whenever the final receive asset differs from the original
asset, including post-position bridge and DEX outputs. Mirror the final-asset
guard used by TransactionLockupFailed.tsx while preserving the existing
post-position DEX quoteAmount handling and receiveDenomination-compatible
formatting.
---
Nitpick comments:
In `@e2e/utils.ts`:
- Around line 497-508: Consolidate the duplicated explorer request by updating
waitForUTXOs to call getAddressUtxos instead of issuing its own axios GET to the
address UTXO endpoint. Reuse the existing UTXO type for getAddressUtxos and
waitForUTXOs so both paths share the same response shape and cannot drift.
In `@packages/boltz-swaps/integration/reverseSwap.regtest.spec.ts`:
- Around line 189-219: The fee assertion in the “is accepted by Elements and
pays the surcharge out of the claim output” test needs an inline comment
explaining the intentional -1 adjustment. Add a concise rationale referencing
relay-fee truncation, matching the documented explanation in the sibling test
and mirrored reverse-swap coverage, without changing the expected fee value.
In `@packages/boltz-swaps/src/utxo/claim.ts`:
- Around line 328-366: The exported unconfidentialClaimSurcharge helper should
explicitly document that it is destination-based and may differ from the
claim-side needsBlindedOpReturn condition, which also requires blinded inputs.
Add a brief comment clarifying that unblinded Liquid lockups may still receive
the reserved 6 sat without spending it, while preserving the existing behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: a257c8be-623e-4283-9761-a24b2d1494c9
📒 Files selected for processing (32)
e2e/chainSwaps/chainSwaps.spec.tse2e/chainSwaps/quoteAcceptanceCrash.spec.tse2e/chainSwaps/zeroAmount.spec.tse2e/reverseSwap.spec.tse2e/utils.tspackages/boltz-swaps/integration/regtest.tspackages/boltz-swaps/integration/reverseSwap.regtest.spec.tspackages/boltz-swaps/src/chain.tspackages/boltz-swaps/src/reverse.tspackages/boltz-swaps/src/utxo/claim.tspackages/boltz-swaps/src/utxo/index.tspackages/boltz-swaps/tests/chainUtxo.spec.tspackages/boltz-swaps/tests/reverse.spec.tspackages/boltz-swaps/tests/utxo/claim.spec.tspackages/boltz-swaps/tests/utxo/claimReverse.spec.tspackages/boltz-swaps/tests/utxo/claimUnconfidential.spec.tspackages/boltz-swaps/tests/utxo/claimUnconfidentialEndToEnd.spec.tssrc/components/CreateButton.tsxsrc/components/Fees.tsxsrc/components/OptimizedRoute.tsxsrc/context/Pay.tsxsrc/pages/ClaimRescue.tsxsrc/status/TransactionClaimed.tsxsrc/status/TransactionLockupFailed.tsxsrc/utils/magicRoutingHint.tssrc/utils/unconfidential.tstests/components/Fees.spec.tsxtests/context/Pay.spec.tsxtests/status/TransactionClaimed.spec.tsxtests/status/TransactionLockupFailedAutoAccept.spec.tsxtests/utils/magicRoutingHint.spec.tstests/utils/unconfidential.spec.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- tests/context/Pay.spec.tsx
- packages/boltz-swaps/src/utxo/index.ts
- packages/boltz-swaps/tests/utxo/claimReverse.spec.ts
- packages/boltz-swaps/tests/chainUtxo.spec.ts
1 participant
Summary by CodeRabbit