Skip to content

Commit 96e3e8f

Browse files
committed
pay: Limit splitting when constraints exhausted
When a payment fails due to CLTV or fee budget constraints, the adaptive splitter creates excessive sub-payments before hitting its lower amount limit. Issue #8167 showed partid reaching 5787+. The fix adds a constraints_exhausted flag that limits split depth to 3 when CLTV or fee constraints cause failures. Depth counts only SPLIT operations, not retries. Fixes: #8167 Changelog-Fixed: pay: Prevent excessive payment attempts when CLTV or fee budget constraints are exceeded.
1 parent 9627bf9 commit 96e3e8f

File tree

3 files changed

+110
-0
lines changed

3 files changed

+110
-0
lines changed

plugins/libplugin-pay.c

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -101,6 +101,7 @@ struct payment *payment_new(tal_t *ctx, struct command *cmd,
101101
p->route = NULL;
102102
p->temp_exclusion = NULL;
103103
p->failroute_retry = false;
104+
p->constraints_exhausted = false;
104105
p->routetxt = NULL;
105106
p->max_htlcs = UINT32_MAX;
106107
p->aborterror = NULL;
@@ -904,6 +905,7 @@ static struct command_result *payment_getroute(struct payment *p)
904905
/* Ensure that our fee and CLTV budgets are respected. */
905906
if (amount_msat_greater(fee, p->constraints.fee_budget)) {
906907
p->route = tal_free(p->route);
908+
p->constraints_exhausted = true;
907909
return payment_fail(
908910
p, "Fee exceeds our fee budget: %s > %s, discarding route",
909911
fmt_amount_msat(tmpctx, fee),
@@ -913,6 +915,7 @@ static struct command_result *payment_getroute(struct payment *p)
913915
if (p->route[0].delay > p->constraints.cltv_budget) {
914916
u32 delay = p->route[0].delay;
915917
p->route = tal_free(p->route);
918+
p->constraints_exhausted = true;
916919
return payment_fail(p, "CLTV delay exceeds our CLTV budget: %d > %d",
917920
delay, p->constraints.cltv_budget);
918921
}
@@ -3695,6 +3698,17 @@ static struct command_result *adaptive_splitter_cb(struct adaptive_split_mod_dat
36953698
fields, root->payment_secret,
36963699
root->final_amount.millisatoshis); /* Raw: onion payload */
36973700
} else if (p->step == PAYMENT_STEP_FAILED && !p->abort) {
3701+
/* Limit split depth when constraints exhausted. */
3702+
if (p->constraints_exhausted) {
3703+
int split_depth = 0;
3704+
for (struct payment *pp = p->parent; pp != NULL; pp = pp->parent) {
3705+
if (pp->step == PAYMENT_STEP_SPLIT)
3706+
split_depth++;
3707+
}
3708+
if (split_depth >= 3)
3709+
return payment_continue(p);
3710+
}
3711+
36983712
if (amount_msat_greater(p->our_amount, MPP_ADAPTIVE_LOWER_LIMIT)) {
36993713
struct payment *a, *b;
37003714
/* Random number in the range [90%, 110%] */

plugins/libplugin-pay.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -279,6 +279,10 @@ struct payment {
279279
* amount. */
280280
bool failroute_retry;
281281

282+
/* Set when payment constraints (fee/CLTV) exceeded. Prevents splitting
283+
* since these don't depend on payment amount. */
284+
bool constraints_exhausted;
285+
282286
/* A unique id for the root of this payment. */
283287
u64 id;
284288

tests/test_pay.py

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,98 @@ def test_pay_limits(node_factory):
150150
assert status[0]['strategy'] == "Initial attempt"
151151

152152

153+
def test_pay_no_excessive_splitting_on_cltv(node_factory):
154+
"""Test that CLTV budget exceeded doesn't cause excessive splitting (#8167).
155+
"""
156+
l1, l2, l3 = node_factory.line_graph(3, wait_for_announce=True)
157+
158+
inv = l3.rpc.invoice(1000000, "test_cltv", 'description')['bolt11']
159+
160+
PAY_STOPPED_RETRYING = 210
161+
with pytest.raises(RpcError, match=r'CLTV delay exceeds our CLTV budget') as err:
162+
l1.rpc.call('pay', {'bolt11': inv, 'maxdelay': 5})
163+
164+
assert err.value.error['code'] == PAY_STOPPED_RETRYING
165+
166+
status = l1.rpc.call('paystatus', {'bolt11': inv})['pay'][0]['attempts']
167+
168+
# Without fix: ~62 attempts. With fix: ~30 (depth-3 split limit).
169+
assert len(status) < 35, \
170+
f"Too many attempts ({len(status)}), possible infinite loop bug"
171+
172+
173+
def test_pay_mpp_splitting_still_works(node_factory, bitcoind):
174+
"""Regression test: MPP splitting still works after #8167 fix.
175+
"""
176+
l1, l2, l3, l4 = node_factory.get_nodes(4)
177+
178+
# Diamond topology: l1->l2->l4 and l1->l3->l4, 200k capacity each
179+
l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
180+
l1.rpc.connect(l3.info['id'], 'localhost', l3.port)
181+
l2.rpc.connect(l4.info['id'], 'localhost', l4.port)
182+
l3.rpc.connect(l4.info['id'], 'localhost', l4.port)
183+
184+
l1.fundchannel(l2, 200000, wait_for_active=True)
185+
l1.fundchannel(l3, 200000, wait_for_active=True)
186+
l2.fundchannel(l4, 200000, wait_for_active=True)
187+
l3.fundchannel(l4, 200000, wait_for_active=True)
188+
189+
mine_funding_to_announce(bitcoind, [l1, l2, l3, l4])
190+
wait_for(lambda: len(l1.rpc.listchannels()['channels']) == 8)
191+
192+
# 300k needs splitting across both paths
193+
inv = l4.rpc.invoice(300000000, "diamond_split", "Needs splitting")['bolt11']
194+
195+
result = l1.rpc.pay(inv)
196+
assert result['status'] == 'complete'
197+
198+
payments = l1.rpc.listsendpays(bolt11=inv)['payments']
199+
successful = [p for p in payments if p['status'] == 'complete']
200+
assert len(successful) > 1, "Payment should have been split"
201+
202+
203+
def test_pay_splitting_bypass_cltv_multiple_paths(node_factory, bitcoind):
204+
"""Test that splitting can bypass high-CLTV path via multiple low-CLTV paths.
205+
206+
Topology:
207+
l1 ---(100k)---> l2 ---(100k)---> l5 [normal CLTV]
208+
l1 ---(100k)---> l3 ---(100k)---> l5 [normal CLTV]
209+
l1 ---(200k)---> l4 ---(200k)---> l5 [HIGH CLTV]
210+
211+
150k payment with tight CLTV budget must split via l2+l3, can't use l4.
212+
"""
213+
l1, l2, l3, l4, l5 = node_factory.get_nodes(5, opts=[
214+
{},
215+
{'cltv-delta': 6},
216+
{'cltv-delta': 6},
217+
{'cltv-delta': 100},
218+
{}
219+
])
220+
221+
l1.rpc.connect(l2.info['id'], 'localhost', l2.port)
222+
l1.rpc.connect(l3.info['id'], 'localhost', l3.port)
223+
l1.rpc.connect(l4.info['id'], 'localhost', l4.port)
224+
l2.rpc.connect(l5.info['id'], 'localhost', l5.port)
225+
l3.rpc.connect(l5.info['id'], 'localhost', l5.port)
226+
l4.rpc.connect(l5.info['id'], 'localhost', l5.port)
227+
228+
l1.fundchannel(l2, 100000, wait_for_active=True)
229+
l1.fundchannel(l3, 100000, wait_for_active=True)
230+
l1.fundchannel(l4, 200000, wait_for_active=True)
231+
l2.fundchannel(l5, 100000, wait_for_active=True)
232+
l3.fundchannel(l5, 100000, wait_for_active=True)
233+
l4.fundchannel(l5, 200000, wait_for_active=True)
234+
235+
mine_funding_to_announce(bitcoind, [l1, l2, l3, l4, l5])
236+
wait_for(lambda: len(l1.rpc.listchannels()['channels']) == 12)
237+
238+
inv = l5.rpc.invoice(150000000, "test_split_cltv", "Test splitting")['bolt11']
239+
240+
# maxdelay=30 allows l2/l3 paths (~12 blocks) but not l4 (~106 blocks)
241+
result = l1.rpc.call('pay', {'bolt11': inv, 'maxdelay': 30})
242+
assert result['status'] == 'complete'
243+
244+
153245
def test_pay_exclude_node(node_factory, bitcoind):
154246
"""Test excluding the node if there's the NODE-level error in the failure_code
155247
"""

0 commit comments

Comments
 (0)