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.

Changelog-Fixed: Corner case where channel could become unusable (lightning/bolts#728)
Signed-off-by: Rusty Russell <[email protected]>

Author: rustyrussell

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

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})