← Watch feed
reverse swaps: in the CLI, replace 'server_mining_fee' with 'prepayment', which corresponds to the trusted part of the lightning payment.
Public commit record
73/100 · Adequate
What the developer wrote
Authored by ThomasV
reverse swaps: in the CLI, replace 'server_mining_fee' with
'prepayment', which corresponds to the trusted part of the
lightning payment.
We use 2*sm.mining_fee, where 'mining_fee' is the flat part of
the server fee. However, future protocol should probably allow
to set a value that does not depend on 'mining_fee'.
(note that LND uses a hardcoded amount).
✓ Specific, descriptive subject✓ Names a concrete action or component✓ Provides detailed explanatory context
What changed, and why it matters
This commit renames and doubles a fee parameter used when doing reverse submarine swaps (a way to move funds from Lightning back to on-chain Bitcoin) in the Electrum wallet. The old name suggested it was just the swap provider's mining fee; the new name, 'prepayment', reflects that the user must pay this amount upfront over Lightning and trust the provider to return it if the swap fails. The change is mostly a terminology and parameter refactor, but it also adjusts the safety check so the allowed prepayment equals twice the mining fee rather than twice the prepayment. There is no direct evidence in the commit that this fixes an active security vulnerability.
Recommended action
Treat this as a routine refactor with minor security-relevant clarification rather than a vulnerability fix. Reviewers should verify that `prepayment_sat` is always derived as `2 * mining_fee` and that the new validation `fee_lnaddr.get_amount_sat() > prepayment_sat` does not accidentally allow larger fee invoices when callers pass a custom prepayment. If the protocol later allows arbitrary prepayments, stricter server-side/user confirmation checks should be added.
Security signals we found
01
Parameter rename clarifies a trusted prepayment rather than a pure mining fee
02
Validation threshold changed from `server_mining_fee_sat * 2` to `prepayment_sat`
03
User-visible CLI argument renamed and documented as non-trustless
04
No bounds check or input sanitization changes beyond the rename
Evidence from the diff
The patch renames server_mining_fee_sat/provider_mining_fee to prepayment_sat/prepayment across the CLI, QML GUI, Qt GUI, swap manager, and tests. In commands.py, reverse_swap dryruns now return 2 * sm.mining_fee as the prepayment, and the user must supply that value explicitly. In submarine_swaps.py, the validation fee_lnaddr.get_amount_sat() > server_mining_fee_sat * 2 becomes fee_lnaddr.get_amount_sat() > prepayment_sat, which is mathematically equivalent because prepayment_sat is defined as 2 * mining_fee. The commit message frames this as aligning the CLI with the trusted portion of the Lightning payment and notes that future protocol versions should decouple the prepayment from mining_fee.
Changed components
electrum/commands.pyelectrum/gui/qml/qeswaphelper.pyelectrum/gui/qt/swap_dialog.pyelectrum/submarine_swaps.pytests/regtest/regtest.shtests/test_commands.pyInspect captured patch +25 / −25
diff --git a/electrum/commands.py b/electrum/commands.py
index 6bc172e..55078d0 100644
--- a/electrum/commands.py
+++ b/electrum/commands.py
@@ -1989,7 +1989,7 @@ class Commands(Logger):
"max_forward_sat": offer.pairs.max_forward,
"max_reverse_sat": offer.pairs.max_reverse,
"min_amount_sat": offer.pairs.min_amount,
- "provider_mining_fee": offer.pairs.mining_fee,
+ "prepayment": 2 * offer.pairs.mining_fee,
}
return result
@@ -2035,14 +2035,14 @@ class Commands(Logger):
@command('wnpl')
async def reverse_swap(
- self, lightning_amount, onchain_amount, provider_mining_fee='dryrun', password=None, wallet: Abstract_Wallet = None,
+ self, lightning_amount, onchain_amount, prepayment='dryrun', password=None, wallet: Abstract_Wallet = None,
):
"""
Reverse submarine swap: send on Lightning, receive on-chain
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
- arg:decimal_or_dryrun:provider_mining_fee:Mining fee required by the swap provider, in BTC. Set it to 'dryrun' to receive a value
+ arg:decimal_or_dryrun:prepayment:Lightning payment required by the swap provider in order to cover their mining fees. This is included in lightning_amount. However, this part of the operation is not trustless; the provider is trusted to fail this payment if the swap fails.
"""
sm = wallet.lnworker.swap_manager
assert self.config.SWAPSERVER_NPUB or self.config.SWAPSERVER_URL, \
@@ -2055,32 +2055,32 @@ class Commands(Logger):
if onchain_amount == 'dryrun':
lightning_amount_sat = satoshis(lightning_amount)
onchain_amount_sat = sm.get_recv_amount(lightning_amount_sat, is_reverse=True)
- assert provider_mining_fee == "dryrun", f"Cannot use {provider_mining_fee=} in dryrun. Set it to 'dryrun'."
- provider_mining_fee = sm.mining_fee
+ 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)
- assert provider_mining_fee == "dryrun", f"Cannot use {provider_mining_fee=} in dryrun. Set it to 'dryrun'."
- provider_mining_fee = sm.mining_fee
+ assert prepayment == "dryrun", f"Cannot use {prepayment=} in dryrun. Set it to 'dryrun'."
+ prepayment_sat = 2 * sm.mining_fee
funding_txid = None
else:
lightning_amount_sat = satoshis(lightning_amount)
claim_fee = sm.get_fee_for_txbatcher()
onchain_amount_sat = satoshis(onchain_amount) + claim_fee
- assert provider_mining_fee != "dryrun", "Provide the 'provider_mining_fee' obtained from the dryrun."
- provider_mining_fee = satoshis(provider_mining_fee)
+ assert prepayment != "dryrun", "Provide the 'prepayment' obtained from the dryrun."
+ 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,
- server_mining_fee_sat=provider_mining_fee,
+ prepayment_sat=prepayment_sat,
)
return {
'funding_txid': funding_txid,
'lightning_amount': format_satoshis(lightning_amount_sat),
'onchain_amount': format_satoshis(onchain_amount_sat),
- 'provider_mining_fee': format_satoshis(provider_mining_fee)
+ 'prepayment': format_satoshis(prepayment_sat)
}
@command('n')
diff --git a/electrum/gui/qml/qeswaphelper.py b/electrum/gui/qml/qeswaphelper.py
index 0084426..220003b 100644
--- a/electrum/gui/qml/qeswaphelper.py
+++ b/electrum/gui/qml/qeswaphelper.py
@@ -719,7 +719,7 @@ class QESwapHelper(AuthMixin, QObject, QtEventListener):
transport=self.swap_transport,
lightning_amount_sat=lightning_amount,
expected_onchain_amount_sat=onchain_amount + swap_manager.get_fee_for_txbatcher(),
- server_mining_fee_sat=self.serverMiningfee.satsInt,
+ prepayment_sat=2 * self.serverMiningfee.satsInt,
)
try: # swaphelper might be destroyed at this point
if txid:
diff --git a/electrum/gui/qt/swap_dialog.py b/electrum/gui/qt/swap_dialog.py
index 0120284..19de6be 100644
--- a/electrum/gui/qt/swap_dialog.py
+++ b/electrum/gui/qt/swap_dialog.py
@@ -338,7 +338,7 @@ class SwapDialog(WindowModalDialog, QtEventListener):
transport=transport,
lightning_amount_sat=lightning_amount,
expected_onchain_amount_sat=onchain_amount + self.swap_manager.get_fee_for_txbatcher(),
- server_mining_fee_sat=self.last_server_mining_fee_sat,
+ prepayment_sat=2 * self.last_server_mining_fee_sat,
)
try:
# we must not leave the context, so we use run_couroutine_dialog
diff --git a/electrum/submarine_swaps.py b/electrum/submarine_swaps.py
index 5ba8e04..9738c9f 100644
--- a/electrum/submarine_swaps.py
+++ b/electrum/submarine_swaps.py
@@ -926,7 +926,7 @@ class SwapManager(Logger):
transport: 'SwapServerTransport',
lightning_amount_sat: int,
expected_onchain_amount_sat: int,
- server_mining_fee_sat: int,
+ prepayment_sat: int,
channels: Optional[Sequence['Channel']] = None,
) -> Optional[str]:
"""send on Lightning, receive on-chain
@@ -943,9 +943,9 @@ class SwapManager(Logger):
- Server fulfills HTLC using preimage.
Note: expected_onchain_amount_sat is BEFORE deducting the on-chain claim tx fee.
- Note: server_mining_fee_sat is passed as argument instead of accessing self.mining_fee to ensure
+ Note: prepayment_sat is passed as argument instead of accessing self.mining_fee to ensure
the mining fees the user sees in the GUI are also the values used for the checks performed here.
- We commit to server_mining_fee_sat as it limits the max fee pre-payment amt, which the server is trusted with.
+ We commit to prepayment_sat as it limits the max fee pre-payment amt, which the server is trusted with.
"""
assert self.network
assert self.lnwatcher
@@ -1007,7 +1007,7 @@ class SwapManager(Logger):
raise Exception("rswap check failed: inconsistent RHASH and invoice")
if fee_invoice:
fee_lnaddr = self.lnworker._check_bolt11_invoice(fee_invoice)
- if fee_lnaddr.get_amount_sat() > server_mining_fee_sat * 2:
+ if fee_lnaddr.get_amount_sat() > prepayment_sat:
raise SwapServerError(_("Mining fee requested by swap-server larger "
"than what was announced in their offer."))
invoice_amount += fee_lnaddr.get_amount_sat()
diff --git a/tests/regtest/regtest.sh b/tests/regtest/regtest.sh
index 2d61e20..0473c65 100755
--- a/tests/regtest/regtest.sh
+++ b/tests/regtest/regtest.sh
@@ -248,8 +248,8 @@ if [[ $1 == "swapserver_success" ]]; then
echo "alice initiates swap"
dryrun=$($alice reverse_swap 0.02 dryrun)
onchain_amount=$(echo $dryrun| jq -r ".onchain_amount")
- swapserver_mining_fee=$(echo $dryrun| jq -r ".provider_mining_fee")
- swap=$($alice reverse_swap 0.02 $onchain_amount --provider_mining_fee $swapserver_mining_fee)
+ prepayment=$(echo $dryrun| jq -r ".prepayment")
+ swap=$($alice reverse_swap 0.02 $onchain_amount --prepayment $prepayment)
echo $swap | jq
funding_txid=$(echo $swap| jq -r ".funding_txid")
new_blocks 1
@@ -273,8 +273,8 @@ if [[ $1 == "swapserver_forceclose" ]]; then
echo "alice initiates swap"
dryrun=$($alice reverse_swap 0.02 dryrun)
onchain_amount=$(echo $dryrun| jq -r ".onchain_amount")
- swapserver_mining_fee=$(echo $dryrun| jq -r ".provider_mining_fee")
- swap=$($alice reverse_swap 0.02 $onchain_amount --provider_mining_fee $swapserver_mining_fee)
+ prepayment=$(echo $dryrun| jq -r ".prepayment")
+ swap=$($alice reverse_swap 0.02 $onchain_amount --prepayment $prepayment)
echo $swap | jq
funding_txid=$(echo $swap| jq -r ".funding_txid")
ctx_id=$($bob close_channel --force $channel)
@@ -309,8 +309,8 @@ if [[ $1 == "swapserver_refund" ]]; then
echo "alice initiates swap"
dryrun=$($alice reverse_swap 0.02 dryrun)
onchain_amount=$(echo $dryrun| jq -r ".onchain_amount")
- swapserver_mining_fee=$(echo $dryrun| jq -r ".provider_mining_fee")
- swap=$($alice reverse_swap 0.02 $onchain_amount --provider_mining_fee $swapserver_mining_fee)
+ prepayment=$(echo $dryrun| jq -r ".prepayment")
+ swap=$($alice reverse_swap 0.02 $onchain_amount --prepayment $prepayment)
echo $swap | jq
funding_txid=$(echo $swap| jq -r ".funding_txid")
new_blocks 140
diff --git a/tests/test_commands.py b/tests/test_commands.py
index 2a93d09..8b4c410 100644
--- a/tests/test_commands.py
+++ b/tests/test_commands.py
@@ -696,14 +696,14 @@ class TestCommandsTestnet(ElectrumTestCase):
"max_forward_sat": offer1.pairs.max_forward,
"max_reverse_sat": offer1.pairs.max_reverse,
"min_amount_sat": offer1.pairs.min_amount,
- "provider_mining_fee": offer1.pairs.mining_fee,
+ "prepayment": 2 * offer1.pairs.mining_fee,
},
offer2.server_npub: {
"percentage_fee": offer2.pairs.percentage,
"max_forward_sat": offer2.pairs.max_forward,
"max_reverse_sat": offer2.pairs.max_reverse,
"min_amount_sat": offer2.pairs.min_amount,
- "provider_mining_fee": offer2.pairs.mining_fee,
+ "prepayment": 2 * offer2.pairs.mining_fee,
}
}
self.assertEqual(result, expected_result)
Risk score
Our methodology →Why this scored 28/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.