diff --git a/channeld/full_channel.c b/channeld/full_channel.c index 55ae6214c0b3..7001712f4f3a 100644 --- a/channeld/full_channel.c +++ b/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; } diff --git a/plugins/libplugin-pay.c b/plugins/libplugin-pay.c index 2c51847f383d..6769b4a7447a 100644 --- a/plugins/libplugin-pay.c +++ b/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) { diff --git a/plugins/pay.c b/plugins/pay.c index b08c55c64213..95d09ba84bf3 100644 --- a/plugins/pay.c +++ b/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) diff --git a/tests/test_pay.py b/tests/test_pay.py index 5f75e0f0e02a..c701c38d4110 100644 --- a/tests/test_pay.py +++ b/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' + )