WIP

channeld/full_channel.c

@@ -525,7 +525,7 @@ static bool htlc_dust(const struct channel *channel,
  *
  * To mostly avoid this situation, at least from our side, we apply an
  * additional constraint when we're opener trying to add an HTLC: make
- * sure we can afford one more HTLC, even if fees increase by 100%.
+ * sure we can afford one more HTLC, even if fees increase.
  *
  * We could do this for the peer, as well, by rejecting their HTLC
  * immediately in this case.  But rejecting a remote HTLC here causes
@@ -560,39 +560,19 @@ static bool local_opener_has_fee_headroom(const struct channel *channel,
 					option_anchors_zero_fee_htlc_tx,
 					committed, adding, removing);
 
-	/* Scale the feerate up by a margin. This ensures that we have
-	 * some leeway even in rising fees. The calculation starts
-	 * with a 100% margin at very low fees, since they are likely
-	 * to rise, and can do so quickly, whereas on the higher fee
-	 * side, asking for a 100% margin is excessive, so ask for a
-	 * 10% margin. In-between these two regions we interpolate
-	 * linearly. Notice that minfeerate and maxfeerate are just
-	 * the markers of the linear interpolation, they don't have
-	 * to correspond to actual feerates seen in the network.
-	 *
-	 * See [CLN6974] for details and discussion.
-	 *
-	 * [CLN6974]: https://github.com/ElementsProject/lightning/issues/6974
-	 */
-	u64 minfeerate = 253, maxfeerate = 45000,
-	    min = feerate - minfeerate > maxfeerate ? maxfeerate
-						    : feerate - minfeerate;
-	double marginperc = 1 - min / (maxfeerate * 1.1);
-	u64 marginrate = 1 + marginperc;
-
 	/* Now, how much would it cost us if feerate increases 100% and we added
 	 * another HTLC? */
-	fee = commit_tx_base_fee(marginrate, untrimmed + 1,
+	fee = commit_tx_base_fee(marginal_feerate(feerate), untrimmed + 1,
 				 option_anchor_outputs,
 				 option_anchors_zero_fee_htlc_tx);
 	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 by 100%% to %uperkw",
+		     " another HTLC if fees increase from %uperkw to %uperkw",
 		     type_to_string(tmpctx, struct amount_msat, &remainder),
 		     type_to_string(tmpctx, struct amount_sat, &fee),
-		     feerate + feerate);
+		     feerate, marginal_feerate(feerate));
 	return false;
 }
 
@@ -770,6 +750,11 @@ static enum channel_add_err add_htlc(struct channel *channel,
 		if (htlc_fee)
 			*htlc_fee = fee;
 
+		status_debug("htlc_fee for %s %zu adding %zu removing == %s",
+			     recipient == LOCAL ? "us" : "them",
+			     tal_count(adding), tal_count(removing),
+			     type_to_string(tmpctx, struct amount_sat, &fee));
+
 		/* This is a little subtle:
 		 *
 		 * The change is being applied to the receiver but it will
@@ -834,6 +819,11 @@ static enum channel_add_err add_htlc(struct channel *channel,
 					    adding,
 					    removing,
 					    channel->opener);
+			status_debug("htlc_fee for %s %zu adding %zu removing == %s",
+				     recipient == LOCAL ? "us" : "them",
+				     tal_count(adding), tal_count(removing),
+				     type_to_string(tmpctx, struct amount_sat, &fee));
+
 			/* set fee output pointer if given */
 			if (htlc_fee && amount_sat_greater(fee, *htlc_fee))
 				*htlc_fee = fee;

common/fee_states.c

@@ -157,6 +157,27 @@ bool fee_states_valid(const struct fee_states *fee_states, enum side opener)
 	return fee_states->feerate[last_fee_state(opener)] != NULL;
 }
 
+/* The calculation starts with a 100% margin at very low fees, since
+ * they are likely to rise, and can do so quickly, whereas on the
+ * higher fee side, asking for a 100% margin is excessive, so ask for
+ * a 10% margin. In-between these two regions we interpolate
+ * linearly. Notice that minfeerate and maxfeerate are just the
+ * markers of the linear interpolation, they don't have to correspond
+ * to actual feerates seen in the network. */
+u32 marginal_feerate(u32 current_feerate)
+{
+	const u32 minfeerate = 253, maxfeerate = 45000;
+
+	assert(current_feerate >= minfeerate);
+	if (current_feerate > maxfeerate)
+		return current_feerate * 1.1;
+
+	/* min gives 1, max gives 0.1 */
+	double proportion = 1.0 - ((double)current_feerate - minfeerate) / (maxfeerate - minfeerate) * 0.9;
+
+	return current_feerate + proportion * current_feerate;
+}
+
 static const char *fmt_fee_states(const tal_t *ctx,
 				  const struct fee_states *fee_states)
 {

common/fee_states.h

@@ -90,4 +90,14 @@ bool fee_states_valid(const struct fee_states *fee_states, enum side opener);
 bool feerate_changes_done(const struct fee_states *fee_states,
 			  bool ignore_uncommitted);
 
+/**
+ * https://github.com/lightningnetwork/lightning-rfc/issues/740 indicates
+ * we should not allow an HTLC if a fee increase of 100% would cause us to
+ * get stuck.
+ *
+ * This proved overzealous: if fees are already high there is less chance
+ * they increase by another 100%, so we scale the factor.
+ */
+u32 marginal_feerate(u32 current_feerate);
+
 #endif /* LIGHTNING_COMMON_FEE_STATES_H */

lightningd/peer_control.c

@@ -673,9 +673,13 @@ static struct amount_sat commit_txfee(const struct channel *channel,
 	 *   reserve. It is recommended that this "fee spike buffer" can
 	 *   handle twice the current `feerate_per_kw` to ensure
 	 *   predictability between implementations.
-	*/
-	fee = commit_tx_base_fee(2 * feerate, num_untrimmed_htlcs + 1,
+	 */
+	fee = commit_tx_base_fee(marginal_feerate(feerate), num_untrimmed_htlcs + 1,
 				 option_anchor_outputs, option_anchors_zero_fee_htlc_tx);
+	log_debug(channel->log, "Before anchors: fee for %zu untrimmed at feerate %u (based on %u) will be %s",
+		  num_untrimmed_htlcs + 1,
+		  marginal_feerate(feerate), feerate,
+		  type_to_string(tmpctx, struct amount_sat, &fee));
 
 	if (option_anchor_outputs || option_anchors_zero_fee_htlc_tx) {
 		/* BOLT #3:
@@ -686,6 +690,8 @@ static struct amount_sat commit_txfee(const struct channel *channel,
 		 */
 		if (!amount_sat_add(&fee, fee, AMOUNT_SAT(660)))
 			; /* fee is somehow astronomical already.... */
+		log_debug(channel->log, "After anchors: fee is %s",
+			  type_to_string(tmpctx, struct amount_sat, &fee));
 	}
 
 	return fee;

tests/test_pay.py

@@ -754,7 +754,8 @@ def test_wait_sendpay(node_factory, executor):
 @pytest.mark.parametrize("anchors", [False, True])
 def test_sendpay_cant_afford(node_factory, anchors):
     # Set feerates the same so we don't have to wait for update.
-    opts = {'feerates': (15000, 15000, 15000, 15000)}
+    opts = {'feerates': (15000, 15000, 15000, 15000),
+            'commit-feerate-offset': 0}
     if anchors is False:
         opts['dev-force-features'] = "-23"
 
@@ -771,22 +772,28 @@ def test_sendpay_cant_afford(node_factory, anchors):
     # minimum = 1
     # maximum = 10**9
     # while maximum - minimum > 1:
-    #     l1, l2 = node_factory.line_graph(2, fundamount=10**6,
-    #                                      opts={'feerates': (15000, 15000, 15000, 15000)})
+    #     l1, l2 = node_factory.line_graph(2, fundamount=10**6, opts=opts)
     #     try:
     #         l1.pay(l2, (minimum + maximum) // 2)
+    #         print("XXX Pay {} WORKED!".format((minimum + maximum) // 2))
     #         minimum = (minimum + maximum) // 2
     #     except RpcError:
+    #         print("XXX Pay {} FAILED!".format((minimum + maximum) // 2))
     #         maximum = (minimum + maximum) // 2
     #     print("{} - {}".format(minimum, maximum))
-    # assert False
+
+    #     # Make sure not HTLCs left to interfere
+    #     wait_for(lambda: only_one(l1.rpc.listpeerchannels()['channels'])['htlcs'] == [])
+    #     wait_for(lambda: only_one(l2.rpc.listpeerchannels()['channels'])['htlcs'] == [])
+    # assert False, "Max we can pay == {}".format(minimum)
+    # # Currently this gives: 976560000 for non-anchors, 969900000 for anchors
+    # # Add reserve to this result to derive `available`
 
     # This is the fee, which needs to be taken into account for l1.
-    if anchors:
-        # option_anchor_outputs
-        available = 10**9 - 44700000
+    if anchors: 
+        available = 969900000 + reserve
     else:
-        available = 10**9 - 32040000
+        available = 976560000 + reserve
 
     # Can't pay past reserve.
     with pytest.raises(RpcError):
@@ -2507,11 +2514,15 @@ def test_setchannel_startup_opts(node_factory, bitcoind):
     assert result[1]['htlc_maximum_msat'] == Millisatoshi(5)
 
 
-def test_channel_spendable(node_factory, bitcoind):
+@pytest.mark.parametrize("anchors", [False, True])
+def test_channel_spendable(node_factory, bitcoind, anchors):
     """Test that spendable_msat is accurate"""
     sats = 10**6
+    opts={'plugin': os.path.join(os.getcwd(), 'tests/plugins/hold_invoice.py'), 'holdtime': '30'}
+    if anchors is False:
+        opts['dev-force-features'] = "-23"
     l1, l2 = node_factory.line_graph(2, fundamount=sats, wait_for_announce=True,
-                                     opts={'plugin': os.path.join(os.getcwd(), 'tests/plugins/hold_invoice.py'), 'holdtime': '30'})
+                                     opts=opts)
 
     inv = l2.rpc.invoice('any', 'inv', 'for testing')
     payment_hash = inv['payment_hash']