Skip to content

Commit c6a97bd

Browse files
Lagrang3Lagrang3
committed
renepay: fix error handling
Fix error handling since we moved from sendpay to sendonion rpc. With sendonion once a route fails we don't get the scid and node_id that failed along the route, so we have to deduce those from our own internal data. Changelog-None Signed-off-by: Lagrang3 <[email protected]>
1 parent 448e330 commit c6a97bd

File tree

2 files changed

+174
-68
lines changed

2 files changed

+174
-68
lines changed

‎plugins/renepay/routefail.c‎

Lines changed: 173 additions & 67 deletions
Original file line numberDiff line numberDiff line change
@@ -126,18 +126,28 @@ static struct command_result *update_gossip_failure(struct command *cmd UNUSED,
126126
{
127127
assert(r);
128128
assert(r->payment);
129+
assert(r->route->result->erring_index);
130+
131+
const int index = *r->route->result->erring_index;
132+
struct short_channel_id_dir scidd;
133+
134+
if (r->route->result->erring_channel) {
135+
scidd.scid = *r->route->result->erring_channel;
136+
scidd.dir = *r->route->result->erring_direction;
137+
} else if (r->route->hops) {
138+
assert(index < tal_count(r->route->hops));
139+
const struct route_hop *hop = &r->route->hops[index];
140+
scidd.scid = hop->scid;
141+
scidd.dir = hop->direction;
142+
143+
} else /* don't have information to disable the erring channel */
144+
goto finish;
129145

130-
/* FIXME it might be too strong assumption that erring_channel should
131-
* always be present here, but at least the documentation for
132-
* waitsendpay says it is present in the case of error. */
133-
assert(r->route->result->erring_channel);
134-
struct short_channel_id_dir scidd = {
135-
.scid = *r->route->result->erring_channel,
136-
.dir = *r->route->result->erring_direction};
137146
payment_disable_chan(
138-
r->payment, scidd, LOG_INFORM,
139-
"addgossip failed (%.*s)", json_tok_full_len(result),
140-
json_tok_full(buf, result));
147+
r->payment, scidd, LOG_INFORM, "addgossip failed (%.*s)",
148+
json_tok_full_len(result), json_tok_full(buf, result));
149+
150+
finish:
141151
return update_gossip_done(cmd, method, buf, result, r);
142152
}
143153

@@ -188,6 +198,7 @@ static void route_final_error(struct route *route, enum jsonrpc_errcode error,
188198
route->final_msg = tal_strdup(route, what);
189199
}
190200

201+
/* FIXME: do proper error handling for BOLT12 */
191202
static struct command_result *handle_failure(struct routefail *r)
192203
{
193204
/* BOLT #4:
@@ -242,7 +253,12 @@ static struct command_result *handle_failure(struct routefail *r)
242253
if (route->hops)
243254
path_len = tal_count(route->hops);
244255

245-
assert(result->erring_index);
256+
if (!result->erring_index) {
257+
payment_note(
258+
payment, LOG_UNUSUAL,
259+
"The erring_index is unknown we skip error handling");
260+
goto finish;
261+
}
246262

247263
enum node_type node_type = UNKNOWN_NODE;
248264
if (route->hops) {
@@ -254,76 +270,128 @@ static struct command_result *handle_failure(struct routefail *r)
254270
node_type = INTERMEDIATE_NODE;
255271
}
256272

257-
assert(result->erring_node);
258-
259273
switch (result->failcode) {
260274
// intermediate only
261275
case WIRE_INVALID_ONION_VERSION:
262276
case WIRE_INVALID_ONION_HMAC:
263277
case WIRE_INVALID_ONION_KEY:
264-
if (node_type == FINAL_NODE)
278+
switch (node_type) {
279+
case FINAL_NODE:
265280
payment_note(payment, LOG_UNUSUAL,
266-
"Final node %s reported strange "
281+
"Final node reported strange "
267282
"error code %04x (%s)",
268-
fmt_node_id(tmpctx, result->erring_node),
269283
result->failcode,
270284
onion_wire_name(result->failcode));
285+
break;
286+
case ORIGIN_NODE:
287+
case INTERMEDIATE_NODE:
288+
case UNKNOWN_NODE:
289+
break;
290+
}
271291

272292
case WIRE_INVALID_ONION_BLINDING:
273-
if (node_type == FINAL_NODE) {
293+
switch (node_type) {
294+
case FINAL_NODE:
274295
/* these errors from a final node mean a permanent
275296
* failure */
276297
route_final_error(
277298
route, PAY_DESTINATION_PERM_FAIL,
278299
"Received error code %04x (%s) at final node.",
279300
result->failcode,
280301
onion_wire_name(result->failcode));
281-
} else if (node_type == INTERMEDIATE_NODE ||
282-
node_type == ORIGIN_NODE) {
302+
303+
break;
304+
case INTERMEDIATE_NODE:
305+
case ORIGIN_NODE:
306+
if (!route->hops)
307+
break;
308+
283309
/* we disable the next node in the hop */
284310
assert(*result->erring_index < path_len);
285311
payment_disable_node(
286-
payment,
287-
route->hops[*result->erring_index].node_id, LOG_DBG,
288-
"received %s from previous hop",
312+
payment, route->hops[*result->erring_index].node_id,
313+
LOG_DBG, "received %s from previous hop",
289314
onion_wire_name(result->failcode));
315+
break;
316+
case UNKNOWN_NODE:
317+
break;
290318
}
291319
break;
292320

293321
// final only
294322
case WIRE_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS:
323+
switch (node_type) {
324+
case FINAL_NODE:
325+
route_final_error(route, PAY_DESTINATION_PERM_FAIL,
326+
"Unknown invoice or wrong payment "
327+
"details at destination.");
328+
break;
329+
case ORIGIN_NODE:
330+
route_final_error(
331+
route, PAY_UNSPECIFIED_ERROR,
332+
"Error code %04x (%s) reported at the origin.",
333+
failcode,
334+
onion_wire_name(failcode));
335+
break;
336+
case INTERMEDIATE_NODE:
337+
if (!route->hops)
338+
break;
339+
payment_disable_node(
340+
payment,
341+
route->hops[*result->erring_index - 1].node_id,
342+
LOG_INFORM, "received error %s",
343+
onion_wire_name(failcode));
344+
break;
345+
case UNKNOWN_NODE:
346+
break;
347+
}
348+
break;
295349
case WIRE_FINAL_INCORRECT_HTLC_AMOUNT:
296350
case WIRE_FINAL_INCORRECT_CLTV_EXPIRY:
297-
if (node_type == INTERMEDIATE_NODE)
351+
switch (node_type) {
352+
case INTERMEDIATE_NODE:
298353
payment_note(payment, LOG_UNUSUAL,
299-
"Intermediate node %s reported strange "
354+
"Intermediate node reported strange "
300355
"error code %04x (%s)",
301-
fmt_node_id(tmpctx, result->erring_node),
302356
result->failcode,
303357
onion_wire_name(result->failcode));
358+
break;
359+
case ORIGIN_NODE:
360+
case FINAL_NODE:
361+
case UNKNOWN_NODE:
362+
break;
363+
}
304364

305365
case WIRE_PERMANENT_NODE_FAILURE:
306366
case WIRE_REQUIRED_NODE_FEATURE_MISSING:
307367
case WIRE_TEMPORARY_NODE_FAILURE:
308368
case WIRE_INVALID_ONION_PAYLOAD:
309-
310-
if (node_type == FINAL_NODE) {
369+
switch (node_type) {
370+
case FINAL_NODE:
311371
route_final_error(
312372
route, PAY_DESTINATION_PERM_FAIL,
313373
"Received error code %04x (%s) at final node.",
314374
result->failcode,
315375
onion_wire_name(result->failcode));
316-
} else if (node_type == ORIGIN_NODE) {
376+
break;
377+
case ORIGIN_NODE:
317378
route_final_error(
318379
route, PAY_UNSPECIFIED_ERROR,
319380
"Error code %04x (%s) reported at the origin.",
320381
result->failcode,
321382
onion_wire_name(result->failcode));
322-
} else {
323-
payment_disable_node(payment,
324-
*result->erring_node, LOG_INFORM,
325-
"received error %s",
326-
onion_wire_name(result->failcode));
383+
break;
384+
case INTERMEDIATE_NODE:
385+
if (!route->hops)
386+
break;
387+
payment_disable_node(
388+
payment,
389+
route->hops[*result->erring_index - 1].node_id,
390+
LOG_INFORM, "received error %s",
391+
onion_wire_name(result->failcode));
392+
break;
393+
case UNKNOWN_NODE:
394+
break;
327395
}
328396
break;
329397

@@ -333,11 +401,11 @@ static struct command_result *handle_failure(struct routefail *r)
333401
case WIRE_UNKNOWN_NEXT_PEER:
334402
case WIRE_EXPIRY_TOO_FAR:
335403
case WIRE_CHANNEL_DISABLED:
336-
if (node_type == FINAL_NODE) {
404+
switch (node_type) {
405+
case FINAL_NODE:
337406
payment_note(payment, LOG_UNUSUAL,
338-
"Final node %s reported strange "
407+
"Final node reported strange "
339408
"error code %04x (%s)",
340-
fmt_node_id(tmpctx, result->erring_node),
341409
result->failcode,
342410
onion_wire_name(result->failcode));
343411

@@ -347,35 +415,56 @@ static struct command_result *handle_failure(struct routefail *r)
347415
result->failcode,
348416
onion_wire_name(result->failcode));
349417

350-
} else {
351-
assert(result->erring_channel);
418+
break;
419+
case ORIGIN_NODE:
420+
payment_note(payment, LOG_UNUSUAL,
421+
"First node reported strange "
422+
"error code %04x (%s)",
423+
result->failcode,
424+
onion_wire_name(result->failcode));
425+
426+
break;
427+
case INTERMEDIATE_NODE:
428+
if (!route->hops)
429+
break;
352430
struct short_channel_id_dir scidd = {
353-
.scid = *result->erring_channel,
354-
.dir = *result->erring_direction};
355-
payment_disable_chan(
356-
payment, scidd, LOG_INFORM,
357-
"%s", onion_wire_name(result->failcode));
431+
.scid = route->hops[*result->erring_index].scid,
432+
.dir = route->hops[*result->erring_index].direction};
433+
payment_disable_chan(payment, scidd, LOG_INFORM, "%s",
434+
onion_wire_name(result->failcode));
435+
436+
break;
437+
case UNKNOWN_NODE:
438+
break;
358439
}
359440
break;
360441
// final only
361442
case WIRE_MPP_TIMEOUT:
362-
363-
if (node_type == INTERMEDIATE_NODE) {
443+
switch (node_type) {
444+
case INTERMEDIATE_NODE:
364445
/* Normally WIRE_MPP_TIMEOUT is raised by the final
365446
* node. If this is not the final node, then something
366447
* wrong is going on. We report it and disable that
367448
* node. */
368449
payment_note(payment, LOG_UNUSUAL,
369-
"Intermediate node %s reported strange "
450+
"Intermediate node reported strange "
370451
"error code %04x (%s)",
371-
fmt_node_id(tmpctx, result->erring_node),
372452
result->failcode,
373453
onion_wire_name(result->failcode));
374454

375-
payment_disable_node(payment,
376-
*result->erring_node, LOG_INFORM,
377-
"received error %s",
378-
onion_wire_name(result->failcode));
455+
if (!route->hops)
456+
break;
457+
payment_disable_node(
458+
payment,
459+
route->hops[*result->erring_index - 1].node_id,
460+
LOG_INFORM, "received error %s",
461+
onion_wire_name(result->failcode));
462+
463+
break;
464+
case ORIGIN_NODE:
465+
case FINAL_NODE:
466+
case UNKNOWN_NODE:
467+
break;
379468
}
380469
break;
381470

@@ -384,12 +473,11 @@ static struct command_result *handle_failure(struct routefail *r)
384473
case WIRE_INCORRECT_CLTV_EXPIRY:
385474
case WIRE_FEE_INSUFFICIENT:
386475
case WIRE_AMOUNT_BELOW_MINIMUM:
387-
388-
if (node_type == FINAL_NODE) {
476+
switch (node_type) {
477+
case FINAL_NODE:
389478
payment_note(payment, LOG_UNUSUAL,
390-
"Final node %s reported strange "
479+
"Final node reported strange "
391480
"error code %04x (%s)",
392-
fmt_node_id(tmpctx, result->erring_node),
393481
result->failcode,
394482
onion_wire_name(result->failcode));
395483

@@ -399,33 +487,44 @@ static struct command_result *handle_failure(struct routefail *r)
399487
result->failcode,
400488
onion_wire_name(result->failcode));
401489

402-
} else {
490+
break;
491+
case ORIGIN_NODE:
492+
payment_note(payment, LOG_UNUSUAL,
493+
"First node reported strange "
494+
"error code %04x (%s)",
495+
result->failcode,
496+
onion_wire_name(result->failcode));
497+
498+
break;
499+
case INTERMEDIATE_NODE:
500+
if (!route->hops)
501+
break;
403502
/* Usually this means we need to update the channel
404503
* information and try again. To avoid hitting this
405504
* error again with the same channel we flag it. */
406-
assert(result->erring_channel);
407505
struct short_channel_id_dir scidd = {
408-
.scid = *result->erring_channel,
409-
.dir = *result->erring_direction};
410-
payment_warn_chan(payment,
411-
scidd, LOG_INFORM,
506+
.scid = route->hops[*result->erring_index].scid,
507+
.dir = route->hops[*result->erring_index].direction};
508+
payment_warn_chan(payment, scidd, LOG_INFORM,
412509
"received error %s",
413510
onion_wire_name(result->failcode));
414-
}
415511

512+
break;
513+
case UNKNOWN_NODE:
514+
break;
515+
}
416516
break;
417517
// intermediate only
418518
case WIRE_TEMPORARY_CHANNEL_FAILURE:
419-
420-
if (node_type == FINAL_NODE) {
519+
switch (node_type) {
520+
case FINAL_NODE:
421521
/* WIRE_TEMPORARY_CHANNEL_FAILURE could mean that the
422522
* next channel has not enough outbound liquidity or
423523
* cannot add another HTLC. A final node cannot raise
424524
* this error. */
425525
payment_note(payment, LOG_UNUSUAL,
426-
"Final node %s reported strange "
526+
"Final node reported strange "
427527
"error code %04x (%s)",
428-
fmt_node_id(tmpctx, result->erring_node),
429528
result->failcode,
430529
onion_wire_name(result->failcode));
431530

@@ -434,9 +533,16 @@ static struct command_result *handle_failure(struct routefail *r)
434533
"Received error code %04x (%s) at final node.",
435534
result->failcode,
436535
onion_wire_name(result->failcode));
437-
}
438536

537+
break;
538+
case INTERMEDIATE_NODE:
539+
case ORIGIN_NODE:
540+
case UNKNOWN_NODE:
541+
break;
542+
}
439543
break;
440544
}
545+
546+
finish:
441547
return routefail_end(take(r));
442548
}

‎tests/test_renepay.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ def test_errors(node_factory, bitcoind):
140140

141141
PAY_DESTINATION_PERM_FAIL = 203
142142
assert err.value.error["code"] == PAY_DESTINATION_PERM_FAIL
143-
assert "WIRE_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS" in err.value.error["message"]
143+
assert "Unknown invoice" in err.value.error["message"]
144144

145145

146146
@pytest.mark.openchannel("v1")

0 commit comments

Comments
 (0)