fix: rfc ElementsProject#740 requires 100% feespike margin

channeld/full_channel.c

@@ -390,7 +390,8 @@ 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
+/*
+ * 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
@@ -399,17 +400,30 @@ static struct amount_sat fee_for_htlcs(const struct channel *channel,
  * demonstrated with c-lightning by @m-schmook
  * (https://github.com/ElementsProject/lightning/pull/3498).
  *
+ * This will be mitigated by applying a 2-times fee spike margin:
+ * https://github.com/lightningnetwork/lightning-rfc/issues/740
+ *
+ * BOLT #2: Adding an HTLC: `update_add_htlc`
+ *
+ * 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 2*feerate_per_kw
+ * while maintaining its channel reserve ("fee spike buffer").
+ *
+ *
  * 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%.
+ * 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
  * 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. */
+ * self-regulating too.
+ */
 static bool local_funder_has_fee_headroom(const struct channel *channel,
 					  struct amount_msat remainder,
 					  const struct htlc **committed,
@@ -428,14 +442,14 @@ static bool local_funder_has_fee_headroom(const struct channel *channel,
 					feerate,
 					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? */
-	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))
 		return true;
 
-	status_debug("Adding HTLC would leave us only %s:"
-		     " we need %s for another HTLC if fees increase 50%% to %uperkw",
+	status_debug("Adding HTLC would leave us only %s: we need %s for"
+		     " another HTLC if fees increase by 100%% to %uperkw",
 		     type_to_string(tmpctx, struct amount_msat, &remainder),
 		     type_to_string(tmpctx, struct amount_sat, &fee),
 		     feerate + feerate/2);

lightningd/peer_control.c

@@ -551,10 +551,17 @@ static struct amount_sat commit_txfee_spend(const struct channel *channel,
 			num_untrimmed_htlcs++;
 	}
 
-	/* Funder is conservative: makes sure it allows an extra HTLC
-	 * even if feerate increases 50% */
-	return commit_tx_base_fee(local_feerate + local_feerate / 2,
-				  num_untrimmed_htlcs + 1);
+	/*
+	 * BOLT #2:
+	 * Adding an HTLC - update_add_htlc
+	 *
+	 * 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 2 * feerate_per_kw
+	 * while maintaining its channel reserve ("fee spike buffer").
+	 */
+	return commit_tx_base_fee(2 * local_feerate, num_untrimmed_htlcs + 1);
 }
 
 /* Fee a commitment transaction would currently cost when receiving */
@@ -598,10 +605,17 @@ static struct amount_sat commit_txfee_recv(const struct channel *channel,
 			num_untrimmed_htlcs++;
 	}
 
-	/* Funder is conservative: makes sure it allows an extra HTLC
-	 * even if feerate increases 50% */
-	return commit_tx_base_fee(feerate + feerate / 2,
-				  num_untrimmed_htlcs + 1);
+	/*
+	 * BOLT #2:
+	 * Adding an HTLC - update_add_htlc
+	 *
+	 * 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 2 * feerate_per_kw
+	 * while maintaining its channel reserve ("fee spike buffer").
+	 */
+	return commit_tx_base_fee(2 * feerate, num_untrimmed_htlcs + 1);
 }
 
 

tests/test_pay.py

@@ -2354,7 +2354,7 @@ def test_lockup_drain(node_factory, bitcoind):
 
     # But if feerate increase just a little more, l2 should not be able to send
     # non-fust HTLC to l1
-    l1.force_feerates(22502)  # TODO: Why does 22501 fail? off by one in C code?
+    l1.force_feerates(30002)  # TODO: Why does 30001 fail? off by one in C code?
     with pytest.raises(RpcError, match=r".*Capacity exceeded.*"):
         l2.pay(l1, total // 2)