Skip to content

Commit e732d49

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 1670a02 commit e732d49

File tree

1 file changed

+149
-67
lines changed

1 file changed

+149
-67
lines changed

‎plugins/renepay/routefail.c

+149-67
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) {
@@ -253,77 +269,105 @@ static struct command_result *handle_failure(struct routefail *r)
253269
else
254270
node_type = INTERMEDIATE_NODE;
255271
}
256-
257-
assert(result->erring_node);
272+
if (*result->erring_index == 0)
273+
node_type = ORIGIN_NODE;
258274

259275
switch (result->failcode) {
260276
// intermediate only
261277
case WIRE_INVALID_ONION_VERSION:
262278
case WIRE_INVALID_ONION_HMAC:
263279
case WIRE_INVALID_ONION_KEY:
264-
if (node_type == FINAL_NODE)
280+
switch (node_type) {
281+
case FINAL_NODE:
265282
payment_note(payment, LOG_UNUSUAL,
266-
"Final node %s reported strange "
283+
"Final node reported strange "
267284
"error code %04x (%s)",
268-
fmt_node_id(tmpctx, result->erring_node),
269285
result->failcode,
270286
onion_wire_name(result->failcode));
287+
break;
288+
case ORIGIN_NODE:
289+
case INTERMEDIATE_NODE:
290+
case UNKNOWN_NODE:
291+
break;
292+
}
271293

272294
case WIRE_INVALID_ONION_BLINDING:
273-
if (node_type == FINAL_NODE) {
295+
switch (node_type) {
296+
case FINAL_NODE:
274297
/* these errors from a final node mean a permanent
275298
* failure */
276299
route_final_error(
277300
route, PAY_DESTINATION_PERM_FAIL,
278301
"Received error code %04x (%s) at final node.",
279302
result->failcode,
280303
onion_wire_name(result->failcode));
281-
} else if (node_type == INTERMEDIATE_NODE ||
282-
node_type == ORIGIN_NODE) {
304+
305+
break;
306+
case INTERMEDIATE_NODE:
307+
case ORIGIN_NODE:
308+
if (!route->hops)
309+
break;
310+
283311
/* we disable the next node in the hop */
284312
assert(*result->erring_index < path_len);
285313
payment_disable_node(
286-
payment,
287-
route->hops[*result->erring_index].node_id, LOG_DBG,
288-
"received %s from previous hop",
314+
payment, route->hops[*result->erring_index].node_id,
315+
LOG_DBG, "received %s from previous hop",
289316
onion_wire_name(result->failcode));
317+
break;
318+
case UNKNOWN_NODE:
319+
break;
290320
}
291321
break;
292322

293323
// final only
294324
case WIRE_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS:
295325
case WIRE_FINAL_INCORRECT_HTLC_AMOUNT:
296326
case WIRE_FINAL_INCORRECT_CLTV_EXPIRY:
297-
if (node_type == INTERMEDIATE_NODE)
327+
switch (node_type) {
328+
case INTERMEDIATE_NODE:
298329
payment_note(payment, LOG_UNUSUAL,
299-
"Intermediate node %s reported strange "
330+
"Intermediate node reported strange "
300331
"error code %04x (%s)",
301-
fmt_node_id(tmpctx, result->erring_node),
302332
result->failcode,
303333
onion_wire_name(result->failcode));
334+
break;
335+
case ORIGIN_NODE:
336+
case FINAL_NODE:
337+
case UNKNOWN_NODE:
338+
break;
339+
}
304340

305341
case WIRE_PERMANENT_NODE_FAILURE:
306342
case WIRE_REQUIRED_NODE_FEATURE_MISSING:
307343
case WIRE_TEMPORARY_NODE_FAILURE:
308344
case WIRE_INVALID_ONION_PAYLOAD:
309-
310-
if (node_type == FINAL_NODE) {
345+
switch (node_type) {
346+
case FINAL_NODE:
311347
route_final_error(
312348
route, PAY_DESTINATION_PERM_FAIL,
313349
"Received error code %04x (%s) at final node.",
314350
result->failcode,
315351
onion_wire_name(result->failcode));
316-
} else if (node_type == ORIGIN_NODE) {
352+
break;
353+
case ORIGIN_NODE:
317354
route_final_error(
318355
route, PAY_UNSPECIFIED_ERROR,
319356
"Error code %04x (%s) reported at the origin.",
320357
result->failcode,
321358
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));
359+
break;
360+
case INTERMEDIATE_NODE:
361+
if (!route->hops)
362+
break;
363+
payment_disable_node(
364+
payment,
365+
route->hops[*result->erring_index - 1].node_id,
366+
LOG_INFORM, "received error %s",
367+
onion_wire_name(result->failcode));
368+
break;
369+
case UNKNOWN_NODE:
370+
break;
327371
}
328372
break;
329373

@@ -333,11 +377,11 @@ static struct command_result *handle_failure(struct routefail *r)
333377
case WIRE_UNKNOWN_NEXT_PEER:
334378
case WIRE_EXPIRY_TOO_FAR:
335379
case WIRE_CHANNEL_DISABLED:
336-
if (node_type == FINAL_NODE) {
380+
switch (node_type) {
381+
case FINAL_NODE:
337382
payment_note(payment, LOG_UNUSUAL,
338-
"Final node %s reported strange "
383+
"Final node reported strange "
339384
"error code %04x (%s)",
340-
fmt_node_id(tmpctx, result->erring_node),
341385
result->failcode,
342386
onion_wire_name(result->failcode));
343387

@@ -347,35 +391,56 @@ static struct command_result *handle_failure(struct routefail *r)
347391
result->failcode,
348392
onion_wire_name(result->failcode));
349393

350-
} else {
351-
assert(result->erring_channel);
394+
break;
395+
case ORIGIN_NODE:
396+
payment_note(payment, LOG_UNUSUAL,
397+
"First node reported strange "
398+
"error code %04x (%s)",
399+
result->failcode,
400+
onion_wire_name(result->failcode));
401+
402+
break;
403+
case INTERMEDIATE_NODE:
404+
if (!route->hops)
405+
break;
352406
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));
407+
.scid = route->hops[*result->erring_index].scid,
408+
.dir = route->hops[*result->erring_index].direction};
409+
payment_disable_chan(payment, scidd, LOG_INFORM, "%s",
410+
onion_wire_name(result->failcode));
411+
412+
break;
413+
case UNKNOWN_NODE:
414+
break;
358415
}
359416
break;
360417
// final only
361418
case WIRE_MPP_TIMEOUT:
362-
363-
if (node_type == INTERMEDIATE_NODE) {
419+
switch (node_type) {
420+
case INTERMEDIATE_NODE:
364421
/* Normally WIRE_MPP_TIMEOUT is raised by the final
365422
* node. If this is not the final node, then something
366423
* wrong is going on. We report it and disable that
367424
* node. */
368425
payment_note(payment, LOG_UNUSUAL,
369-
"Intermediate node %s reported strange "
426+
"Intermediate node reported strange "
370427
"error code %04x (%s)",
371-
fmt_node_id(tmpctx, result->erring_node),
372428
result->failcode,
373429
onion_wire_name(result->failcode));
374430

375-
payment_disable_node(payment,
376-
*result->erring_node, LOG_INFORM,
377-
"received error %s",
378-
onion_wire_name(result->failcode));
431+
if (!route->hops)
432+
break;
433+
payment_disable_node(
434+
payment,
435+
route->hops[*result->erring_index - 1].node_id,
436+
LOG_INFORM, "received error %s",
437+
onion_wire_name(result->failcode));
438+
439+
break;
440+
case ORIGIN_NODE:
441+
case FINAL_NODE:
442+
case UNKNOWN_NODE:
443+
break;
379444
}
380445
break;
381446

@@ -384,12 +449,11 @@ static struct command_result *handle_failure(struct routefail *r)
384449
case WIRE_INCORRECT_CLTV_EXPIRY:
385450
case WIRE_FEE_INSUFFICIENT:
386451
case WIRE_AMOUNT_BELOW_MINIMUM:
387-
388-
if (node_type == FINAL_NODE) {
452+
switch (node_type) {
453+
case FINAL_NODE:
389454
payment_note(payment, LOG_UNUSUAL,
390-
"Final node %s reported strange "
455+
"Final node reported strange "
391456
"error code %04x (%s)",
392-
fmt_node_id(tmpctx, result->erring_node),
393457
result->failcode,
394458
onion_wire_name(result->failcode));
395459

@@ -399,33 +463,44 @@ static struct command_result *handle_failure(struct routefail *r)
399463
result->failcode,
400464
onion_wire_name(result->failcode));
401465

402-
} else {
466+
break;
467+
case ORIGIN_NODE:
468+
payment_note(payment, LOG_UNUSUAL,
469+
"First node reported strange "
470+
"error code %04x (%s)",
471+
result->failcode,
472+
onion_wire_name(result->failcode));
473+
474+
break;
475+
case INTERMEDIATE_NODE:
476+
if (!route->hops)
477+
break;
403478
/* Usually this means we need to update the channel
404479
* information and try again. To avoid hitting this
405480
* error again with the same channel we flag it. */
406-
assert(result->erring_channel);
407481
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,
482+
.scid = route->hops[*result->erring_index].scid,
483+
.dir = route->hops[*result->erring_index].direction};
484+
payment_warn_chan(payment, scidd, LOG_INFORM,
412485
"received error %s",
413486
onion_wire_name(result->failcode));
414-
}
415487

488+
break;
489+
case UNKNOWN_NODE:
490+
break;
491+
}
416492
break;
417493
// intermediate only
418494
case WIRE_TEMPORARY_CHANNEL_FAILURE:
419-
420-
if (node_type == FINAL_NODE) {
495+
switch (node_type) {
496+
case FINAL_NODE:
421497
/* WIRE_TEMPORARY_CHANNEL_FAILURE could mean that the
422498
* next channel has not enough outbound liquidity or
423499
* cannot add another HTLC. A final node cannot raise
424500
* this error. */
425501
payment_note(payment, LOG_UNUSUAL,
426-
"Final node %s reported strange "
502+
"Final node reported strange "
427503
"error code %04x (%s)",
428-
fmt_node_id(tmpctx, result->erring_node),
429504
result->failcode,
430505
onion_wire_name(result->failcode));
431506

@@ -434,9 +509,16 @@ static struct command_result *handle_failure(struct routefail *r)
434509
"Received error code %04x (%s) at final node.",
435510
result->failcode,
436511
onion_wire_name(result->failcode));
437-
}
438512

513+
break;
514+
case INTERMEDIATE_NODE:
515+
case ORIGIN_NODE:
516+
case UNKNOWN_NODE:
517+
break;
518+
}
439519
break;
440520
}
521+
522+
finish:
441523
return routefail_end(take(r));
442524
}

0 commit comments

Comments
 (0)