mirrors/core-lightning
1
0
Fork 0
mirror of https://github.com/ElementsProject/lightning.git synced 2025-03-02 18:35:00 +01:00
Code Issues Projects Releases Wiki Activity

fix: rfc #740 requires 100% feespike margin

Browse source 
Changelog-Fixed: Use lightning-rfc #740 feespike margin factor of 2
This commit is contained in:
Michael Schmoock 2020-03-12 12:29:18 +01:00 • committed by Christian Decker
parent f2478e9160
commit af7e879308
4 changed files with 53 additions and 35 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

23
channeld/full_channel.c
View file

@ -390,18 +390,19 @@ static struct amount_sat fee_for_htlcs(const struct channel *channel,
return commit_tx_base_fee(feerate, untrimmed); return commit_tx_base_fee(feerate, untrimmed);
} }
/* There is a corner case where the funder can spend so much that the /*
* There is a corner case where the funder can spend so much that the
* non-funder can't add any non-dust HTLCs (since the funder would * non-funder can't add any non-dust HTLCs (since the funder would
* have to pay the additional fee, but it can't afford to). This * have to pay the additional fee, but it can't afford to). This
* leads to the channel starving at the feast! This was reported by * leads to the channel starving at the feast! This was reported by
* ACINQ's @t-bast * ACINQ's @t-bast
* (https://github.com/lightningnetwork/lightning-rfc/issues/728) and * (https://github.com/lightningnetwork/lightning-rfc/issues/728) and
* demonstrated with c-lightning by @m-schmook * demonstrated with c-lightning by @m-schmoock
* (https://github.com/ElementsProject/lightning/pull/3498). * (https://github.com/ElementsProject/lightning/pull/3498).
* *
* To mostly avoid this situation, at least from our side, we apply an * To mostly avoid this situation, at least from our side, we apply an
* additional constraint when we're funder trying to add an HTLC: make * additional constraint when we're funder trying to add an HTLC: make
* sure we can afford one more HTLC, even if fees increase 50%. * sure we can afford one more HTLC, even if fees increase by 100%.
* *
* We could do this for the peer, as well, by rejecting their HTLC * We could do this for the peer, as well, by rejecting their HTLC
* immediately in this case. But rejecting a remote HTLC here causes * immediately in this case. But rejecting a remote HTLC here causes
@ -409,7 +410,11 @@ static struct amount_sat fee_for_htlcs(const struct channel *channel,
* architected to reject HTLCs in channeld (it's usually lightningd's * architected to reject HTLCs in channeld (it's usually lightningd's
* job, but it doesn't have all the channel balance change calculation * job, but it doesn't have all the channel balance change calculation
* logic. So we look after ourselves for now, and hope other nodes start * logic. So we look after ourselves for now, and hope other nodes start
* self-regulating too. */ * self-regulating too.
*
* This mitigation will become BOLT #2 standard by:
* https://github.com/lightningnetwork/lightning-rfc/issues/740
*/
static bool local_funder_has_fee_headroom(const struct channel *channel, static bool local_funder_has_fee_headroom(const struct channel *channel,
struct amount_msat remainder, struct amount_msat remainder,
const struct htlc **committed, const struct htlc **committed,
@ -428,17 +433,17 @@ static bool local_funder_has_fee_headroom(const struct channel *channel,
feerate, feerate,
committed, adding, removing); committed, adding, removing);
/* Now, how much would it cost us if feerate increases 50% and we added /* Now, how much would it cost us if feerate increases 100% and we added
* another HTLC? */ * another HTLC? */
fee = commit_tx_base_fee(feerate + feerate/2, untrimmed + 1); fee = commit_tx_base_fee(2 * feerate, untrimmed + 1);
if (amount_msat_greater_eq_sat(remainder, fee)) if (amount_msat_greater_eq_sat(remainder, fee))
return true; return true;
status_debug("Adding HTLC would leave us only %s:" status_debug("Adding HTLC would leave us only %s: we need %s for"
" we need %s for another HTLC if fees increase 50%% to %uperkw", " another HTLC if fees increase by 100%% to %uperkw",
type_to_string(tmpctx, struct amount_msat, &remainder), type_to_string(tmpctx, struct amount_msat, &remainder),
type_to_string(tmpctx, struct amount_sat, &fee), type_to_string(tmpctx, struct amount_sat, &fee),
feerate + feerate/2); feerate + feerate);
return false; return false;
} }

19
lightningd/peer_control.c
View file

@ -550,10 +550,21 @@ static struct amount_sat commit_txfee(const struct channel *channel,
num_untrimmed_htlcs++; num_untrimmed_htlcs++;
} }
/* Funder is conservative: makes sure it allows an extra HTLC /*
* even if feerate increases 50% */ * BOLT-95c74fef2fe590cb8adbd7b848743a229ffe825a #2:
return commit_tx_base_fee(local_feerate + local_feerate / 2, * Adding an HTLC: update_add_htlc
num_untrimmed_htlcs + 1); *
* A sending node:
* - if it is responsible for paying the Bitcoin fee:
* - SHOULD NOT offer `amount_msat` if, after adding that HTLC to
* its commitment transaction, its remaining balance doesn't allow
* it to pay the fee for a future additional non-dust HTLC at
* `N*feerate_per_kw` while maintaining its channel reserve
* ("fee spike buffer"), where `N` is a parameter chosen by the
* implementation (`N = 2` is recommended to ensure
* predictability).
*/
return commit_tx_base_fee(local_feerate * 2, num_untrimmed_htlcs + 1);
} }
static void subtract_offered_htlcs(const struct channel *channel, static void subtract_offered_htlcs(const struct channel *channel,

6
tests/test_connection.py
View file

@ -2207,7 +2207,7 @@ def test_change_chaining(node_factory, bitcoind):
def test_feerate_spam(node_factory, chainparams): def test_feerate_spam(node_factory, chainparams):
l1, l2 = node_factory.line_graph(2) l1, l2 = node_factory.line_graph(2)
slack = 35000000 slack = 45000000
# Pay almost everything to l2. # Pay almost everything to l2.
l1.pay(l2, 10**9 - slack) l1.pay(l2, 10**9 - slack)
@ -2218,8 +2218,8 @@ def test_feerate_spam(node_factory, chainparams):
# Now change feerates to something l1 can't afford. # Now change feerates to something l1 can't afford.
l1.set_feerates((100000, 100000, 100000)) l1.set_feerates((100000, 100000, 100000))
# It will raise as far as it can (34000) # It will raise as far as it can (48000)
l1.daemon.wait_for_log('Setting REMOTE feerate to 34000') l1.daemon.wait_for_log('Setting REMOTE feerate to 48000')
l1.daemon.wait_for_log('peer_out WIRE_UPDATE_FEE') l1.daemon.wait_for_log('peer_out WIRE_UPDATE_FEE')
# But it won't do it again once it's at max. # But it won't do it again once it's at max.

40
tests/test_pay.py
View file

@ -584,37 +584,29 @@ def test_sendpay_cant_afford(node_factory):
opts={'feerates': (15000, 15000, 15000)}) opts={'feerates': (15000, 15000, 15000)})
# Can't pay more than channel capacity. # Can't pay more than channel capacity.
def pay(lsrc, ldst, amt, label=None):
if not label:
label = ''.join(random.choice(string.ascii_letters + string.digits) for _ in range(20))
rhash = ldst.rpc.invoice(amt, label, label)['payment_hash']
routestep = {'msatoshi': amt, 'id': ldst.info['id'], 'delay': 5, 'channel': '1x1x1'}
lsrc.rpc.sendpay([routestep], rhash)
lsrc.rpc.waitsendpay(rhash)
with pytest.raises(RpcError): with pytest.raises(RpcError):
pay(l1, l2, 10**9 + 1) l1.pay(l2, 10**9 + 1)
# This is the fee, which needs to be taken into account for l1. # This is the fee, which needs to be taken into account for l1.
available = 10**9 - 24030000 available = 10**9 - 32040000
# Reserve is 1%. # Reserve is 1%.
reserve = 10**7 reserve = 10**7
# Can't pay past reserve. # Can't pay past reserve.
with pytest.raises(RpcError): with pytest.raises(RpcError):
pay(l1, l2, available) l1.pay(l2, available)
with pytest.raises(RpcError): with pytest.raises(RpcError):
pay(l1, l2, available - reserve + 1) l1.pay(l2, available - reserve + 1)
# Can pay up to reserve (1%) # Can pay up to reserve (1%)
pay(l1, l2, available - reserve) l1.pay(l2, available - reserve)
# And now it can't pay back, due to its own reserve. # And now it can't pay back, due to its own reserve.
with pytest.raises(RpcError): with pytest.raises(RpcError):
pay(l2, l1, available - reserve) l2.pay(l1, available - reserve)
# But this should work. # But this should work.
pay(l2, l1, available - reserve * 2) l2.pay(l1, available - reserve * 2)
def test_decodepay(node_factory): def test_decodepay(node_factory):
@ -1561,7 +1553,7 @@ def test_pay_retry(node_factory, bitcoind):
"""Make sure pay command retries properly. """ """Make sure pay command retries properly. """
def exhaust_channel(funder, fundee, scid, already_spent=0): def exhaust_channel(funder, fundee, scid, already_spent=0):
"""Spend all available capacity (10^6 - 1%) of channel""" """Spend all available capacity (10^6 - 1%) of channel"""
maxpay = (10**6 - 10**6 // 100 - 13440) * 1000 - already_spent maxpay = (10**6 - 10**6 // 100 - 16020) * 1000 - already_spent
inv = fundee.rpc.invoice(maxpay, inv = fundee.rpc.invoice(maxpay,
''.join(random.choice(string.ascii_letters + string.digits) for _ in range(20)), ''.join(random.choice(string.ascii_letters + string.digits) for _ in range(20)),
"exhaust_channel") "exhaust_channel")
@ -2293,17 +2285,27 @@ def test_lockup_drain(node_factory, bitcoind):
except RpcError: except RpcError:
msat //= 2 msat //= 2
# Even if feerate now increases 1.5x (22500), l2 should be able to send # Even if feerate now increases 2x (30000), l2 should be able to send
# non-dust HTLC to l1. # non-dust HTLC to l1.
l1.set_feerates([22500] * 3, False) l1.set_feerates([30000] * 3, False)
# Restart forces fast fee adjustment (otherwise it's smoothed and takes # Restart forces fast fee adjustment (otherwise it's smoothed and takes
# a very long time!). # a very long time!).
l1.restart() l1.restart()
wait_for(lambda: only_one(l1.rpc.listpeers()['peers'])['connected']) wait_for(lambda: only_one(l1.rpc.listpeers()['peers'])['connected'])
assert(l1.rpc.feerates('perkw')['perkw']['normal'] == 22500) assert(l1.rpc.feerates('perkw')['perkw']['normal'] == 30000)
l2.pay(l1, total // 2) l2.pay(l1, total // 2)
# But if feerate increase just a little more, l2 should not be able to send
# non-dust HTLC to l1
l1.set_feerates([30002] * 3, False) # TODO: why does 30001 fail, off by one in C code?
l1.restart()
wait_for(lambda: only_one(l1.rpc.listpeers()['peers'])['connected'])
assert(l1.rpc.feerates('perkw')['perkw']['normal'] == 30002)
with pytest.raises(RpcError, match=r".*Capacity exceeded.*"):
l2.pay(l1, total // 2)
def test_error_returns_blockheight(node_factory, bitcoind): def test_error_returns_blockheight(node_factory, bitcoind):
"""Test that incorrect_or_unknown_payment_details returns block height""" """Test that incorrect_or_unknown_payment_details returns block height"""