Skip to content

cli/swaps: fast fail swap cli on changed fees #10840

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
f321x wants to merge 2 commits into
base: master
Choose a base branch
from
+29 −11
Open

cli/swaps: fast fail swap cli on changed fees #10840

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
89b10fe
cli/swaps: fail swap cli on changed fees
f321x Aug 10, 2026
4368018
cli: break translation import
f321x Aug 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 29 additions & 11 deletions electrum/commands.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@
from . import bitcoin
from .bitcoin import is_address, hash_160, COIN
from .bip32 import BIP32Node
from .i18n import _
from .transaction import (
Transaction, multisig_script, PartialTransaction, PartialTxOutput, tx_from_any, PartialTxInput, TxOutpoint,
convert_raw_tx_to_hex
Expand Down Expand Up @@ -87,6 +86,10 @@
from electrum.lnworker import PaymentInfo


def _(_): # break translation
raise ImportError("The CLI is intentionally always non-localized")

@SomberNight SomberNight Aug 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is it an ImportError instead of some other exception type (such as generic Exception)?



known_commands = {} # type: Dict[str, Command]


Expand Down Expand Up @@ -2132,7 +2135,8 @@ async def get_submarine_swap_providers(self, query_time=15, wallet: Abstract_Wal
@command('wnpl')
async def normal_swap(self, onchain_amount, lightning_amount, password=None, wallet: Abstract_Wallet = None):
"""
Normal submarine swap: send on-chain BTC, receive on Lightning
Normal submarine swap: send on-chain BTC, receive on Lightning.
Note: fees can change between the dryrun and the following swap request, causing the request to error and require a new dryrun.

arg:decimal_or_dryrun:lightning_amount:Amount to be received, in BTC. Set it to 'dryrun' to receive a value
arg:decimal_or_dryrun:onchain_amount:Amount to be sent, in BTC. Set it to 'dryrun' to receive a value
Expand All @@ -2156,6 +2160,13 @@ async def normal_swap(self, onchain_amount, lightning_amount, password=None, wal
else:
lightning_amount_sat = satoshis(lightning_amount)
onchain_amount_sat = satoshis(onchain_amount)
required_onchain_amount_sat = sm.get_send_amount(lightning_amount_sat, is_reverse=False)
# same 1 sat rounding tolerance as in `request_normal_swap()`
if not required_onchain_amount_sat \
or not onchain_amount_sat - 1 <= required_onchain_amount_sat <= onchain_amount_sat:

@SomberNight SomberNight Aug 14, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we use parens here? There is so much operator precedence stuff going on in this if, it's hard to imagine the AST in my head.

Suggested change
or not onchain_amount_sat - 1 <= required_onchain_amount_sat <= onchain_amount_sat:
or not (onchain_amount_sat - 1 <= required_onchain_amount_sat <= onchain_amount_sat):

it's a small nit, but I also mean in general in the codebase :)
Too many layers of parens makes it hard to read but too few makes it easy to make a mistake in operator precedence. Delicate tradeoff :P

@f321x f321x Aug 17, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed in 89b10fe. I see your point.

@SomberNight SomberNight Aug 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you forget to push that?
No relevant diff in https://github.com/spesmilo/electrum/compare/52a961f2f2c11dfa27d02bcdf5cc2276dabafdcf..43680185789f3212d9910e3b2e7575d8ae058a72

raise UserFacingException(
"Swap fees have changed since the dryrun was calculated. Do a new dryrun first."
+ f" ({required_onchain_amount_sat} != {onchain_amount_sat} sat)")
Comment thread
SomberNight marked this conversation as resolved.
txid = await wallet.lnworker.swap_manager.normal_swap(
transport=transport,
lightning_amount_sat=lightning_amount_sat,
Expand All @@ -2174,7 +2185,8 @@ async def reverse_swap(
self, lightning_amount, onchain_amount, prepayment='dryrun', password=None, wallet: Abstract_Wallet = None,
):
"""
Reverse submarine swap: send on Lightning, receive on-chain
Reverse submarine swap: send on Lightning, receive on-chain.
Note: fees can change between the dryrun and the following swap request, causing the request to error and require a new dryrun.

arg:decimal_or_dryrun:lightning_amount:Amount to be sent, in BTC. Set it to 'dryrun' to receive a value
arg:decimal_or_dryrun:onchain_amount:Amount to be received, in BTC. Set it to 'dryrun' to receive a value
Expand All @@ -2190,32 +2202,38 @@ async def reverse_swap(
raise TimeoutError("Could not find configured swap provider. Setup another one. See 'get_submarine_swap_providers'")
if onchain_amount == 'dryrun':
lightning_amount_sat = satoshis(lightning_amount)
onchain_amount_sat = sm.get_recv_amount(lightning_amount_sat, is_reverse=True)
onchain_recv_amount_sat = sm.get_recv_amount(lightning_amount_sat, is_reverse=True)
assert prepayment == "dryrun", f"Cannot use {prepayment=} in dryrun. Set it to 'dryrun'."
prepayment_sat = 2 * sm.mining_fee
funding_txid = None
elif lightning_amount == 'dryrun':
onchain_amount_sat = satoshis(onchain_amount)
lightning_amount_sat = sm.get_send_amount(onchain_amount_sat, is_reverse=True)
onchain_recv_amount_sat = satoshis(onchain_amount)
lightning_amount_sat = sm.get_send_amount(onchain_recv_amount_sat, is_reverse=True)
assert prepayment == "dryrun", f"Cannot use {prepayment=} in dryrun. Set it to 'dryrun'."
prepayment_sat = 2 * sm.mining_fee
funding_txid = None
else:
assert prepayment != "dryrun", "Provide the 'prepayment' obtained from the dryrun."
lightning_amount_sat = satoshis(lightning_amount)
requested_recv_amount_sat = satoshis(onchain_amount)
claim_fee = sm.get_fee_for_txbatcher()
onchain_amount_sat = satoshis(onchain_amount) + claim_fee
assert prepayment != "dryrun", "Provide the 'prepayment' obtained from the dryrun."
funding_utxo_value_sat = requested_recv_amount_sat + claim_fee
onchain_recv_amount_sat = sm.get_recv_amount(lightning_amount_sat, is_reverse=True)
if not onchain_recv_amount_sat or onchain_recv_amount_sat < requested_recv_amount_sat:
raise UserFacingException(
"Swap fees have changed since the dryrun was calculated. Do a new dryrun first."
+ f" ({onchain_recv_amount_sat} < {requested_recv_amount_sat} sat)")
Comment on lines +2220 to +2225

@SomberNight SomberNight Aug 14, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do I understand correctly that this is the "same" check as in reverse_swap()?

electrum/electrum/submarine_swaps.py

Lines 1296 to 1299 in a357e86

# check that the onchain amount is what we expected
if onchain_amount < expected_onchain_amount_sat:
raise Exception(f"rswap check failed: onchain_amount is less than what we expected: "
f"{onchain_amount} < {expected_onchain_amount_sat}")

Except there in sswaps.py, both amounts being compared are offset by +claim_fee ?
Uff took me like 10 minutes to untangle that >.<

@f321x f321x Aug 17, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it's a similar check, it just re-calculates the amount with the latest known fees client side to reduce the time window for disagreements and potentially prevent a useless request to the swap provider as the other check only happens after we got the servers response.

prepayment_sat = satoshis(prepayment)
funding_txid = await wallet.lnworker.swap_manager.reverse_swap(
transport=transport,
lightning_amount_sat=lightning_amount_sat,
expected_onchain_amount_sat=onchain_amount_sat,
expected_onchain_amount_sat=funding_utxo_value_sat,
prepayment_sat=prepayment_sat,
)
return {
'funding_txid': funding_txid,
'lightning_amount': format_satoshis(lightning_amount_sat),
'onchain_amount': format_satoshis(onchain_amount_sat),
'onchain_amount': format_satoshis(onchain_recv_amount_sat),
'prepayment': format_satoshis(prepayment_sat)
}

Expand Down Expand Up @@ -2436,7 +2454,7 @@ def subparser_call(self, parser, namespace, values, option_string=None):
parser = self._name_parser_map[parser_name]
except KeyError:
tup = parser_name, ', '.join(self._name_parser_map)
msg = _('unknown parser {!r} (choices: {})').format(*tup)
msg = 'unknown parser {!r} (choices: {})'.format(*tup)
raise ArgumentError(self, msg)
# parse all the remaining options into the namespace
# store any unrecognized options on the object, so that the top
Expand Down