From 5f94c04f0f80ace402c7055f803b6de889536e40 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 11 Feb 2020 13:33:42 +1030 Subject: [PATCH] channeld: channel drain mitigation. Add new check if we're funder trying to add HTLC, keeping us with enough extra funds to pay for another HTLC the peer might add. Signed-off-by: Rusty Russell --- channeld/full_channel.c | 61 +++++++++++++++++++++++++++++++++++++++++ tests/test_pay.py | 1 - 2 files changed, 61 insertions(+), 1 deletion(-) diff --git a/channeld/full_channel.c b/channeld/full_channel.c index f06d2ac3c7d0..1f319bbfb3a8 100644 --- a/channeld/full_channel.c +++ b/channeld/full_channel.c @@ -391,6 +391,58 @@ static struct amount_sat fee_for_htlcs(const struct channel *channel, return commit_tx_base_fee(feerate, untrimmed); } +/* 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 + * 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 + * ACINQ's @t-bast + * (https://github.com/lightningnetwork/lightning-rfc/issues/728) and + * demonstrated with c-lightning by @m-schmook + * (https://github.com/ElementsProject/lightning/pull/3498). + * + * 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 + * sure we can afford one more HTLC, even if fees increase 50%. + * + * We could do this for the peer, as well, by rejecting their HTLC + * immediately in this case. But rejecting a remote HTLC here causes + * us to get upset with them and close the channel: we're not well + * architected to reject HTLCs in channeld (it's usually lightningd's + * 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 + * self-regulating too. */ +static bool local_funder_has_fee_headroom(const struct channel *channel, + struct amount_msat remainder, + const struct htlc **committed, + const struct htlc **adding, + const struct htlc **removing) +{ + u32 feerate = channel_feerate(channel, LOCAL); + size_t untrimmed; + struct amount_sat fee; + + assert(channel->funder == LOCAL); + + /* How many untrimmed at current feerate? Increasing feerate can + * only *reduce* this number, so use current feerate here! */ + untrimmed = num_untrimmed_htlcs(LOCAL, channel->config[LOCAL].dust_limit, + feerate, + committed, adding, removing); + + /* Now, how much would it cost us if feerate increases 50% and we added + * another HTLC? */ + fee = commit_tx_base_fee(feerate + feerate/2, untrimmed + 1); + if (amount_msat_greater_eq_sat(remainder, fee)) + return true; + + status_debug("Adding HTLC would leave us only %s:" + " we need %s for another HTLC if fees increase 50%% to %u", + type_to_string(tmpctx, struct amount_msat, &remainder), + type_to_string(tmpctx, struct amount_sat, &fee), + feerate); + return false; +} + static enum channel_add_err add_htlc(struct channel *channel, enum htlc_state state, u64 id, @@ -572,6 +624,15 @@ static enum channel_add_err add_htlc(struct channel *channel, &remainder)); return CHANNEL_ERR_CHANNEL_CAPACITY_EXCEEDED; } + + if (sender == LOCAL + && !local_funder_has_fee_headroom(channel, + remainder, + committed, + adding, + removing)) { + return CHANNEL_ERR_CHANNEL_CAPACITY_EXCEEDED; + } } /* Try not to add a payment which will take funder into fees diff --git a/tests/test_pay.py b/tests/test_pay.py index 39d1d0c70ee0..564fb380c1d4 100644 --- a/tests/test_pay.py +++ b/tests/test_pay.py @@ -2333,7 +2333,6 @@ def test_channel_drainage(node_factory, bitcoind): l2.rpc.waitsendpay(payment_hash, TIMEOUT) -@pytest.mark.xfail(strict=True) def test_lockup_drain(node_factory, bitcoind): """Try to get channel into a state where funder can't afford fees on additional HTLC, so fundee can't add HTLC""" l1, l2 = node_factory.line_graph(2, opts={'may_reconnect': True})