Repository navigation
Add submarine swap refund (lightning_refund) - #134
Conversation
There was a problem hiding this comment.
@coelhogonzalo I'd like your opinion on this. Claude decided to use the reference implementation of Musig2 since there aren't libs that includes it for Python (I checked with Wallycore and LWK).
it's tested and works, but I'd like to find a more ironed option.
There was a problem hiding this comment.
Hey, sorry for the delay reviewing this. Why not libsecp256k1? (coincurve 21.0.0, already in pyproject, exposes the full secp256k1_musig_* API via its cffi handle) I mean, I think the reference implementation is OK but it seems like libsecp256k1 is more mature and feels like the safe bet. Your call, might be too much effort for little gain.
Options, ranked:
- Bind libsecp256k1 MuSig through coincurve (recommended, . Zero new dependencies. Audited, constant-time. Keeps the vendored file only for test vectors, or drops it. Cost: roughly 150 to 250 lines of cffi glue and careful memory handling.
- Keep the reference implementation. Acceptable today because the refund key is ephemeral and per-swap, so a timing leak has limited value. This is a real argument, but "reference code in production" is a hard sell in review.
Needed for cooperative submarine swap refunds: neither wallycore nor coincurve expose MuSig2. Trimmed to the algorithm code (dropped the file-based test-vector scaffolding) and validated here against the official BIP-327 vectors instead.
Reconstructs a Boltz v2 lockup's taproot swap tree from its public parameters and spends it two ways: cooperatively via a MuSig2 key-path signature (immediate, needs the provider's cosign), or unilaterally via the refund leaf's script path once the timeout block passes. libwally computes the Elements key-path sighash but hardcodes Bitcoin's tapleaf version inside its own script-path variant, so the script path gets a hand-rolled sighash instead. Both are pinned against real, recovered mainnet swaps: the reconstructed scriptPubKey matches what the lockup output actually carries on chain, and the manual sighash matches wally's own key-path result byte for byte (and, fed the same leaf version wally hardcodes, its script-path result too). decode_bolt11_payment_hash is the other missing primitive: the claim leaf commits to the invoice's payment hash, which nothing in the repo extracted before.
lightning_refund (CLI: aqua lightning refund --swap-id <id>) recovers the L-BTC a failed send swap locked up, cooperatively where the provider allows it and unilaterally otherwise. Works with both providers (Indra/Boltz). pay_invoice now persists claimPublicKey, blindingKey, the lockup address and swapTree from the provider's create response — previously discarded, which meant no swap could ever be refunded. --claim-public- key/--blinding-key let a swap created before this land still be refunded, once the provider hands those back. LightningManager.refund_send_swap validates the swap (send, not already refunded, has a lockup to spend), refreshes its remote status without swallowing network failures, and only refunds swaps the provider actually reports as failed. A completed refund is recorded as a new "refunded" status with its own txid; refund_info in lightning_transaction_status now reports refundable/lockup_address/ refund_txid so an agent can tell a swap needs one without guessing. A broadcast refund is final, so a stale provider status (many keep reporting the original failure long after) can no longer overwrite a locally recorded refund. WalletManager gains the three chain primitives the refund needs and nothing else touched: get_block_height, get_transaction_hex and broadcast_raw_tx.
docs/REFUND.md covers the cooperative/unilateral paths, what a swap record needs to be refundable, the legacy-swap override, and the implementation notes worth knowing before touching boltz_refund.py. Cross-referenced from README, docs/CONFIG.md and both AGENTS.md files; the root AGENTS.md gets a new invariant about the refund key never being seed-derived.
Multi-line comments and docstrings that exceeded the project's length conventions are compressed to 1-3 line pointers; content that carried durable reasoning (nonce-reuse risk, the dry-run address quirk) moved into docs/REFUND.md first. No logic changes.
Matches the module it documents. Updated every pointer to it across README, AGENTS.md, docs/CONFIG.md, and the code comments/docstrings in boltz_refund.py, lightning.py, tools.py and _bip327.py.
"Lightning refund" reads as if any Lightning payment could be reversed; this only ever refunds a failed submarine swap's on-chain lockup. Same scope change in the doc's title. Updated every pointer across README, AGENTS.md, docs/CONFIG.md, and the code comments/docstrings.
778e8ad to
09e9986
Compare
The fee draft carried a single 64-byte dummy witness, but the unilateral refund spends the script path with [sig, refund_leaf, control_block], so its fee underpaid the requested rate. Also reword the cooperative failure message to not blame the provider for local errors.
TomasCast
left a comment
TomasCast
left a comment
There was a problem hiding this comment.
Looks good, but check the comments before merging
| _TAG_PAYMENT_HASH = 1 # 'p' | ||
| _TAG_DESCRIPTION = 13 # 'd' |
There was a problem hiding this comment.
Just for curiosity, what does the comments like "# 'p'" mean? is this intentional?
There was a problem hiding this comment.
p (payment hash), d (description) , x (expiry)
| claim_pubkey = claim_public_key or swap.claim_public_key | ||
| blinding = blinding_key or swap.blinding_key | ||
| if not claim_pubkey or not blinding: | ||
| missing = [] | ||
| if not claim_pubkey: | ||
| missing.append("claim_public_key") | ||
| if not blinding: | ||
| missing.append("blinding_key") | ||
| raise ValueError( | ||
| f"Swap {swap_id} predates local storage of {' and '.join(missing)}. " | ||
| "Ask the provider for the swap's keys and pass them explicitly: " | ||
| f"aqua lightning refund --swap-id {swap_id} " | ||
| "--claim-public-key <hex> --blinding-key <hex>" |
There was a problem hiding this comment.
blindingKey is optional, but at the same time it's needed for bulding the refund. If the provider returns an incomplete answer, the code sends the lockup anyways and leaves the funds impsossible to refund automatically. Right?
There was a problem hiding this comment.
Good catch, you're right. Fixed
| if not swap.lockup_txid: | ||
| raise ValueError( | ||
| f"Swap {swap_id} was never funded (no lockup transaction); there is " | ||
| "nothing to refund." | ||
| ) |
There was a problem hiding this comment.
The swap record is saved before the lockup is sent, but lockup_txid is written only after broadcast. If the process dies between those two steps, the provider may already hold the lockup and return its transaction hex, yet refund still aborts before consulting that response and incorrectly claims the swap was never funded.
The provider status lookup should happen before requiring lockup_txid; that field is only needed as a network fallback.
There was a problem hiding this comment.
Agreed, fixed. refund_send_swap no longer requires lockup_txid up front
| ) != elements_taproot_sighash(*self._args(), GENESIS_BLOCK_HASH["testnet"]) | ||
|
|
||
|
|
||
| class TestFindAndUnblindLockup: |
There was a problem hiding this comment.
This was found by cursor agent, I guess it makes sense:
Missing positive coverage for the full confidential refund path. find_and_unblind_lockup only has a negative test. There are also no direct tests of refund_submarine_swap; the manager tests mock it out entirely.
CI would not catch regressions in a successful unblind / asset byte order, cooperative → unilateral fallback, spent-lockup vs wait-for-timeout, or fee-rate fallback and broadcast.
coelhogonzalo
left a comment
coelhogonzalo
left a comment
There was a problem hiding this comment.
Approved. I couldn't find anything worth raising. I verified my doubts talking with Fable.
There was a problem hiding this comment.
Maybe worth renaming boltz into something generic at some point... Maybe not. In my humble opinion, I think we can keep the "boltz" reference in some places since they did stablish the API contract we are using... Just something to think about. I wouldn't change anything for now.
…ateral, fees, broadcast)
…'url' argument (#138) * 🐛 fix: lightning_refund chain helpers after the Airavata backend switch get_block_height, get_transaction_hex and broadcast_raw_tx (added in #134) still called _get_client(network), whose signature #131 changed to (network, url), so every refund raised TypeError. Route them through the backend fallback, reuse the idempotent _broadcast, and read tx hex over HTTP for Esplora backends: lwk's EsploraClient has no get_tx, and every default Liquid backend is Esplora. Closes #137 * 🐛 fix: get_transaction_hex Electrum misses and non-hex 200 bodies Electrum: lwk's ElectrumClient.get_tx never returns None; a missing tx raises LwkError ("missing transaction", probed live on Blockstream's Liquid Electrum). The old None check was dead, so a miss escaped as a raw LwkError instead of the "Transaction ... not found" ValueError. Recognize the miss and treat it like an Esplora 404. The test now raises what lwk really raises instead of returning None. Esplora: a 200 whose body is not tx hex (a proxy HTML page, a JSON error, an empty or non-UTF-8 body) was returned as the lockup hex and failed later inside wally.tx_from_hex, without trying the next backend. Validate the body, record the backend as failed and fall back.
4 participants
Purpose
Failed Lightning sends leave L-BTC locked in a taproot swap output with no way back — this PR adds
lightning_refundto reclaim it, cooperatively with the provider (Indra/Boltz) where possible and unilaterally otherwise. Verified against two real, previously-stuck mainnet swaps: both recovered their funds on chain during development.Description
pay_invoicediscardedclaimPublicKey,blindingKey, the lockup address andswapTreefrom the provider's create response, so no swap could ever be refunded. This PR persists that data going forward and adds--claim-public-key/--blinding-keyoverrides so a swap created before this lands can still be refunded once the provider hands those values back.The crypto (
src/aqua/boltz_refund.py) reconstructs the lockup's taproot swap tree from its public parameters and spends it two ways: a MuSig2 key-path signature cosigned by the provider (immediate), or the refund leaf's script path once the timeout block passes (works with no provider involvement). libwally computes the Elements key-path sighash but hardcodes Bitcoin's tapleaf version inside its own script-path variant, so the script path uses a hand-rolled sighash instead — pinned against wally byte-for-byte in tests (fed the same leaf version wally hardcodes) and against the real on-chain scriptPubKey of both recovered swaps. MuSig2 comes from a vendored BIP-327 reference implementation (src/aqua/_bip327.py, BSD-3-Clause), validated against the official BIP-327 test vectors, since neither wallycore nor coincurve expose it.Full write-up in
docs/REFUND.md.Main Changes
lightning_refundMCP tool /aqua lightning refund --swap-id <id>— recovers a failed send swap's locked L-BTC, cooperative-first with an automatic unilateral fallback after the timeoutsrc/aqua/boltz_refund.py— swap tree reconstruction, Elements taproot sighash (manual, cross-checked against wally), refund transaction construction/blinding, cooperative + unilateral signing flowssrc/aqua/_bip327.py), validated against the official test vectorspay_invoicenow persistsclaim_public_key,blinding_key,lockup_address,swap_treefrom the provider response instead of discarding them; newrefund_txidfield records a completed refundrefunded(mapped fromtransaction.refunded);lightning_transaction_statusnow reportsrefund_info.refundable/lockup_address/refund_txiddecode_bolt11_payment_hash— the claim leaf commits to the invoice's payment hash, previously never extractedBoltzClient.post_refund_signature/get_chain_fees/broadcast_transaction;WalletManager.get_block_height/get_transaction_hex/broadcast_raw_tx— inherited by Indra automaticallydocs/REFUND.md; cross-referenced fromREADME.md,docs/CONFIG.md, and bothAGENTS.mdfiles (new invariant: the refund key is random, not seed-derived, so losing the swap record loses the funds)Checklist