Pay: handle when we're close to htlc limit

channeld/full_channel.c

@@ -178,18 +178,21 @@ void dump_htlcs(const struct channel *channel, const char *prefix)
  * committed: HTLCs currently committed.
  * pending_removal: HTLCs pending removal (subset of committed)
  * pending_addition: HTLCs pending addition (no overlap with committed)
+ *
+ * Also returns number of HTLCs for other side.
  */
-static void gather_htlcs(const tal_t *ctx,
-			 const struct channel *channel,
-			 enum side side,
-			 const struct htlc ***committed,
-			 const struct htlc ***pending_removal,
-			 const struct htlc ***pending_addition)
+static size_t gather_htlcs(const tal_t *ctx,
+			   const struct channel *channel,
+			   enum side side,
+			   const struct htlc ***committed,
+			   const struct htlc ***pending_removal,
+			   const struct htlc ***pending_addition)
 {
 	struct htlc_map_iter it;
 	const struct htlc *htlc;
 	const int committed_flag = HTLC_FLAG(side, HTLC_F_COMMITTED);
 	const int pending_flag = HTLC_FLAG(side, HTLC_F_PENDING);
+	size_t num_other_side = 0;
 
 	*committed = tal_arr(ctx, const struct htlc *, 0);
 	if (pending_removal)
@@ -198,18 +201,33 @@ static void gather_htlcs(const tal_t *ctx,
 		*pending_addition = tal_arr(ctx, const struct htlc *, 0);
 
 	if (!channel->htlcs)
-		return;
+		return num_other_side;
 
 	for (htlc = htlc_map_first(channel->htlcs, &it);
 	     htlc;
 	     htlc = htlc_map_next(channel->htlcs, &it)) {
 		if (htlc_has(htlc, committed_flag)) {
+#ifdef SUPERVERBOSE
+			dump_htlc(htlc, "COMMITTED");
+#endif
 			htlc_arr_append(committed, htlc);
-			if (htlc_has(htlc, pending_flag))
+			if (htlc_has(htlc, pending_flag)) {
+#ifdef SUPERVERBOSE
+				dump_htlc(htlc, "REMOVING");
+#endif
 				htlc_arr_append(pending_removal, htlc);
-		} else if (htlc_has(htlc, pending_flag))
+			} else if (htlc_owner(htlc) != side)
+				num_other_side++;
+		} else if (htlc_has(htlc, pending_flag)) {
 			htlc_arr_append(pending_addition, htlc);
+#ifdef SUPERVERBOSE
+			dump_htlc(htlc, "ADDING");
+#endif
+			if (htlc_owner(htlc) != side)
+				num_other_side++;
+		}
 	}
+	return num_other_side;
 }
 
 static bool sum_offered_msatoshis(struct amount_msat *total,
@@ -489,6 +507,7 @@ static enum channel_add_err add_htlc(struct channel *channel,
 	enum side sender = htlc_state_owner(state), recipient = !sender;
 	const struct htlc **committed, **adding, **removing;
 	const struct channel_view *view;
+	size_t htlc_count;
 
 	htlc = tal(tmpctx, struct htlc);
 
@@ -563,7 +582,7 @@ static enum channel_add_err add_htlc(struct channel *channel,
 	}
 
 	/* Figure out what receiver will already be committed to. */
-	gather_htlcs(tmpctx, channel, recipient, &committed, &removing, &adding);
+	htlc_count = gather_htlcs(tmpctx, channel, recipient, &committed, &removing, &adding);
 	htlc_arr_append(&adding, htlc);
 
 	/* BOLT #2:
@@ -572,8 +591,7 @@ static enum channel_add_err add_htlc(struct channel *channel,
 	 *     HTLCs to its local commitment transaction...
 	 *     - SHOULD fail the channel.
 	 */
-	if (tal_count(committed) - tal_count(removing) + tal_count(adding)
-	    > channel->config[recipient].max_accepted_htlcs) {
+	if (htlc_count + 1 > channel->config[recipient].max_accepted_htlcs) {
 		return CHANNEL_ERR_TOO_MANY_HTLCS;
 	}
 
@@ -583,8 +601,7 @@ static enum channel_add_err add_htlc(struct channel *channel,
 	 * spike with large commitment transactions.
 	 */
 	if (sender == LOCAL
-	    && tal_count(committed) - tal_count(removing) + tal_count(adding)
-	    > channel->config[LOCAL].max_accepted_htlcs) {
+	    && htlc_count + 1 > channel->config[LOCAL].max_accepted_htlcs) {
 		return CHANNEL_ERR_TOO_MANY_HTLCS;
 	}
 

plugins/libplugin-pay.c

@@ -3446,7 +3446,7 @@ static void presplit_cb(struct presplit_mod_data *d, struct payment *p)
 		/* The presplitter only acts on the root and only in the first
 		 * step. */
 		size_t count = 0;
-		u32 htlcs = payment_max_htlcs(p) / PRESPLIT_MAX_HTLC_SHARE;
+		u32 htlcs;
 		struct amount_msat target, amt = p->amount;
 		char *partids = tal_strdup(tmpctx, "");
 		u64 target_amount = MPP_TARGET_SIZE;
@@ -3471,14 +3471,25 @@ static void presplit_cb(struct presplit_mod_data *d, struct payment *p)
 		 * but makes debugging a bit easier. */
 		root->next_partid++;
 
+		htlcs = payment_max_htlcs(p);
+		/* Divide it up if we can, but it might be v low already */
+		if (htlcs >= PRESPLIT_MAX_HTLC_SHARE)
+			htlcs /= PRESPLIT_MAX_HTLC_SHARE;
+
+		int targethtlcs =
+		    p->amount.millisatoshis / target_amount; /* Raw: division */
 		if (htlcs == 0) {
 			p->abort = true;
 			return payment_fail(
 			    p, "Cannot attempt payment, we have no channel to "
 			       "which we can add an HTLC");
-		} else if (p->amount.millisatoshis / target_amount > htlcs) /* Raw: division */
-			target = amount_msat_div(p->amount, htlcs);
-		else
+		} else if (targethtlcs > htlcs) {
+			paymod_log(p, LOG_INFORM,
+				   "Number of pre-split HTLCs (%d) exceeds our "
+				   "HTLC budget (%d), skipping pre-splitter",
+				   targethtlcs, htlcs);
+			return payment_continue(p);
+		} else
 			target = amount_msat(target_amount);
 
 		/* If we are already below the target size don't split it
@@ -3591,18 +3602,17 @@ static void adaptive_splitter_cb(struct adaptive_split_mod_data *d, struct payme
 		 * update our htlc_budget that we own exclusively from now
 		 * on. We do this by subtracting the number of payment
 		 * attempts an eventual presplitter has already performed. */
-		struct payment_tree_result res;
-		res = payment_collect_result(p);
+		int children = tal_count(p->children);
 		d->htlc_budget = payment_max_htlcs(p);
-		if (res.attempts > d->htlc_budget) {
+		if (children > d->htlc_budget) {
 			p->abort = true;
 			return payment_fail(
 			    p,
 			    "Cannot add %d HTLCs to our channels, we "
 			    "only have %d HTLCs available.",
-			    res.attempts, d->htlc_budget);
+			    children, d->htlc_budget);
 		}
-		d->htlc_budget -= res.attempts;
+		d->htlc_budget -= children;
 	}
 
 	if (p->step == PAYMENT_STEP_ONION_PAYLOAD) {

plugins/pay.c

@@ -2026,7 +2026,7 @@ static struct command_result *json_paymod(struct command *cmd,
 
 	if (!bolt12_has_prefix(b11str)) {
 		b11 =
-		    bolt11_decode(cmd, b11str, plugin_feature_set(cmd->plugin),
+		    bolt11_decode(p, b11str, plugin_feature_set(cmd->plugin),
 				  NULL, chainparams, &b11_fail);
 		if (b11 == NULL)
 			return command_fail(cmd, JSONRPC2_INVALID_PARAMS,
@@ -2054,7 +2054,7 @@ static struct command_result *json_paymod(struct command *cmd,
 			    "Invalid bolt11:"
 			    " sets feature var_onion with no secret");
 	} else {
-		b12 = invoice_decode(cmd, b11str, strlen(b11str),
+		b12 = invoice_decode(p, b11str, strlen(b11str),
 				     plugin_feature_set(cmd->plugin),
 				     chainparams, &b12_fail);
 		if (b12 == NULL)

tests/test_pay.py

@@ -4,7 +4,7 @@
 from flaky import flaky  # noqa: F401
 from pyln.client import RpcError, Millisatoshi
 from pyln.proto.onion import TlvPayload
-from pyln.testing.utils import EXPERIMENTAL_DUAL_FUND
+from pyln.testing.utils import EXPERIMENTAL_DUAL_FUND, FUNDAMOUNT
 from utils import (
     DEVELOPER, wait_for, only_one, sync_blockheight, TIMEOUT,
     EXPERIMENTAL_FEATURES, env, VALGRIND
@@ -4292,3 +4292,15 @@ def test_routehint_tous(node_factory, bitcoind):
     l3.stop()
     with pytest.raises(RpcError, match=r'Destination .* is not reachable directly and all routehints were unusable'):
         l2.rpc.pay(inv)
+
+
+def test_pay_low_max_htlcs(node_factory):
+    """Test we can pay if *any* HTLC slots are available"""
+
+    l1, l2, l3 = node_factory.line_graph(3,
+                                         opts={'max-concurrent-htlcs': 1},
+                                         wait_for_announce=True)
+    l1.rpc.pay(l3.rpc.invoice(FUNDAMOUNT * 50, "test", "test")['bolt11'])
+    l1.daemon.wait_for_log(
+        r'Number of pre-split HTLCs \([0-9]+\) exceeds our HTLC budget \([0-9]+\), skipping pre-splitter'
+    )