Skip to content

Commit 3311383

Browse files
Lagrang3Lagrang3
committed
renepay: refactor routefail functions
Make extensive use of rpc batches so that we ensure all request have been processed before the notification is closed as handled. Changelog-None Signed-off-by: Lagrang3 <[email protected]>
1 parent fc75f85 commit 3311383

File tree

3 files changed

+115
-91
lines changed

3 files changed

+115
-91
lines changed

‎plugins/renepay/routefail.c‎

Lines changed: 91 additions & 87 deletions
Original file line numberDiff line numberDiff line change
@@ -15,29 +15,26 @@ enum node_type {
1515
};
1616

1717
struct routefail {
18+
struct rpcbatch *batch;
1819
struct payment *payment;
1920
struct route *route;
2021
};
2122

22-
static void update_gossip(struct command *cmd,
23-
struct request_batch *batch,
24-
struct routefail *r);
25-
26-
static void handle_failure(struct command *cmd,
27-
struct request_batch *batch,
28-
struct routefail *r);
29-
30-
static struct command_result *routefail_end(struct command *cmd,
31-
struct routefail *r)
23+
static bool get_erring_scidd(struct route *route,
24+
struct short_channel_id_dir *scidd)
3225
{
33-
/* Notify the tracker that route has failed and routefail have completed
34-
* handling all possible errors cases. */
35-
routetracker_add_to_final(r->payment, r->payment->routetracker,
36-
r->route);
37-
r = tal_free(r);
38-
return notification_handled(cmd);
26+
assert(scidd);
27+
if (!route->result->erring_direction || !route->result->erring_channel)
28+
return false;
29+
scidd->dir = *route->result->erring_direction;
30+
scidd->scid = *route->result->erring_channel;
31+
return true;
3932
}
4033

34+
static void update_gossip(struct routefail *r);
35+
36+
static void handle_failure(struct routefail *r);
37+
4138
static struct command_result *log_routefail_err(struct command *cmd,
4239
const char *method,
4340
const char *buf,
@@ -50,27 +47,59 @@ static struct command_result *log_routefail_err(struct command *cmd,
5047
return command_still_pending(cmd);
5148
}
5249

53-
struct command_result *routefail_start(const tal_t *ctx,
50+
static struct command_result *routefail_done(struct command *cmd,
51+
struct routefail *r)
52+
{
53+
/* Notify the tracker that route has failed and routefail have completed
54+
* handling all possible errors cases. */
55+
routetracker_add_to_final(r->payment, r->payment->routetracker, r->route);
56+
tal_free(r);
57+
return notification_handled(cmd);
58+
}
59+
60+
struct command_result *routefail_start(struct command *cmd,
5461
struct payment *payment,
55-
struct route *route, struct command *cmd)
62+
struct route *route)
5663
{
57-
assert(route);
58-
struct routefail *r = tal(ctx, struct routefail);
64+
struct routefail *r = tal(cmd, struct routefail);
65+
r->batch = rpcbatch_new(cmd, routefail_done, r);
66+
r->route = route;
67+
r->payment = payment;
68+
update_gossip(r);
69+
handle_failure(r);
70+
return rpcbatch_done(r->batch);
71+
}
5972

60-
if (payment == NULL)
61-
plugin_err(cmd->plugin,
62-
"%s: payment with hash %s not found.",
63-
__func__,
64-
fmt_sha256(tmpctx, &route->key.payment_hash));
73+
static void disable_node(struct routefail *r, struct node_id *node)
74+
{
75+
struct out_req *req = add_to_rpcbatch(r->batch, "askrene-disable-node",
76+
NULL, log_routefail_err, r);
77+
json_add_string(req->js, "layer", r->payment->payment_layer);
78+
json_add_node_id(req->js, "node", node);
79+
send_outreq(req);
80+
}
6581

66-
r->payment = payment;
67-
r->route = route;
82+
static void disable_channel(struct routefail *r,
83+
struct short_channel_id_dir scidd)
84+
{
85+
struct out_req *req = add_to_rpcbatch(
86+
r->batch, "askrene-udpate-channel", NULL, log_routefail_err, r);
87+
json_add_string(req->js, "layer", r->payment->payment_layer);
88+
json_add_short_channel_id_dir(req->js, "short_channel_id_dir", scidd);
89+
json_add_bool(req->js, "enabled", false);
90+
send_outreq(req);
91+
}
6892

69-
struct request_batch *batch =
70-
request_batch_new(cmd, NULL, log_routefail_err, routefail_end, r);
71-
update_gossip(cmd, batch, r);
72-
handle_failure(cmd, batch, r);
73-
return batch_done(cmd, batch);
93+
static void bias_channel(struct routefail *r, struct short_channel_id_dir scidd,
94+
int bias)
95+
{
96+
// FIXME: we want to increment the bias, not set it
97+
struct out_req *req = add_to_rpcbatch(r->batch, "askrene-bias-channel",
98+
NULL, log_routefail_err, r);
99+
json_add_string(req->js, "layer", r->payment->payment_layer);
100+
json_add_short_channel_id_dir(req->js, "short_channel_id_dir", scidd);
101+
json_add_num(req->js, "bias", bias);
102+
send_outreq(req);
74103
}
75104

76105
/*****************************************************************************
@@ -128,9 +157,24 @@ static u8 *channel_update_from_onion_error(const tal_t *ctx,
128157
return patch_channel_update(ctx, take(channel_update));
129158
}
130159

131-
static void update_gossip(struct command *cmd,
132-
struct request_batch *batch,
133-
struct routefail *r)
160+
static struct command_result *
161+
addgossip_fail(struct command *cmd, const char *method, const char *buf,
162+
const jsmntok_t *result, struct routefail *r)
163+
{
164+
struct short_channel_id_dir scidd;
165+
if (get_erring_scidd(r->route, &scidd)) {
166+
plugin_log(cmd->plugin, LOG_UNUSUAL,
167+
"failed to update gossip of erring channel %s",
168+
fmt_short_channel_id_dir(tmpctx, &scidd));
169+
disable_channel(r, scidd);
170+
} else {
171+
plugin_log(cmd->plugin, LOG_UNUSUAL,
172+
"failed to update gossip of UNKNOWN erring channel");
173+
}
174+
return command_still_pending(cmd);
175+
}
176+
177+
static void update_gossip(struct routefail *r)
134178
{
135179
/* if there is no raw_message we continue */
136180
if (!r->route->result->raw_message)
@@ -142,7 +186,8 @@ static void update_gossip(struct command *cmd,
142186
if (!update)
143187
goto skip_update_gossip;
144188

145-
struct out_req *req = add_to_batch(cmd, batch, "addgossip");
189+
struct out_req *req = add_to_rpcbatch(
190+
r->batch, "addgossip", NULL, addgossip_fail, r);
146191
json_add_hex_talarr(req->js, "message", update);
147192
send_outreq(req);
148193

@@ -175,42 +220,8 @@ static void route_final_error(struct route *route, enum jsonrpc_errcode error,
175220
route->final_msg = tal_strdup(route, what);
176221
}
177222

178-
static void disable_node(struct command *cmd, struct request_batch *batch,
179-
struct routefail *r, struct node_id *node)
180-
{
181-
struct out_req *req = add_to_batch(cmd, batch, "askrene-disable-node");
182-
json_add_string(req->js, "layer", r->payment->payment_layer);
183-
json_add_node_id(req->js, "node", node);
184-
send_outreq(req);
185-
}
186-
187-
static void disable_channel(struct command *cmd, struct request_batch *batch,
188-
struct routefail *r, struct short_channel_id_dir scidd)
189-
{
190-
struct out_req *req =
191-
add_to_batch(cmd, batch, "askrene-udpate-channel");
192-
json_add_string(req->js, "layer", r->payment->payment_layer);
193-
json_add_short_channel_id_dir(req->js, "short_channel_id_dir", scidd);
194-
json_add_bool(req->js, "enabled", false);
195-
send_outreq(req);
196-
}
197-
198-
static void bias_channel(struct command *cmd, struct request_batch *batch,
199-
struct routefail *r, struct short_channel_id_dir scidd,
200-
int bias)
201-
{
202-
// FIXME: we want to increment the bias, not set it
203-
struct out_req *req = add_to_batch(cmd, batch, "askrene-bias-channel");
204-
json_add_string(req->js, "layer", r->payment->payment_layer);
205-
json_add_short_channel_id_dir(req->js, "short_channel_id_dir", scidd);
206-
json_add_num(req->js, "bias", bias);
207-
send_outreq(req);
208-
}
209-
210223
/* FIXME: do proper error handling for BOLT12 */
211-
static void handle_failure(struct command *cmd,
212-
struct request_batch *batch,
213-
struct routefail *r)
224+
static void handle_failure(struct routefail *r)
214225
{
215226
/* BOLT #4:
216227
*
@@ -259,6 +270,7 @@ static void handle_failure(struct command *cmd,
259270
assert(result);
260271
struct payment *payment = r->payment;
261272
assert(payment);
273+
struct short_channel_id_dir scidd;
262274

263275
int path_len = 0;
264276
if (route->hops)
@@ -334,8 +346,7 @@ static void handle_failure(struct command *cmd,
334346
LOG_DBG, "received %s from previous hop",
335347
onion_wire_name(failcode));
336348
disable_node(
337-
cmd, batch, r,
338-
&route->hops[*result->erring_index].node_id);
349+
r, &route->hops[*result->erring_index].node_id);
339350
break;
340351
case UNKNOWN_NODE:
341352
break;
@@ -366,8 +377,7 @@ static void handle_failure(struct command *cmd,
366377
LOG_INFORM, "received error %s",
367378
onion_wire_name(failcode));
368379
disable_node(
369-
cmd, batch, r,
370-
&route->hops[*result->erring_index - 1].node_id);
380+
r, &route->hops[*result->erring_index - 1].node_id);
371381
break;
372382
case UNKNOWN_NODE:
373383
break;
@@ -417,8 +427,7 @@ static void handle_failure(struct command *cmd,
417427
LOG_INFORM, "received error %s",
418428
onion_wire_name(failcode));
419429
disable_node(
420-
cmd, batch, r,
421-
&route->hops[*result->erring_index - 1].node_id);
430+
r, &route->hops[*result->erring_index - 1].node_id);
422431
break;
423432
case UNKNOWN_NODE:
424433
break;
@@ -457,12 +466,10 @@ static void handle_failure(struct command *cmd,
457466
case INTERMEDIATE_NODE:
458467
if (!route->hops)
459468
break;
460-
struct short_channel_id_dir scidd = {
461-
.scid = route->hops[*result->erring_index].scid,
462-
.dir = route->hops[*result->erring_index].direction};
463469
payment_disable_chan(payment, scidd, LOG_INFORM, "%s",
464470
onion_wire_name(failcode));
465-
disable_channel(cmd, batch, r, scidd);
471+
if (get_erring_scidd(r->route, &scidd))
472+
disable_channel(r, scidd);
466473
break;
467474
case UNKNOWN_NODE:
468475
break;
@@ -490,8 +497,7 @@ static void handle_failure(struct command *cmd,
490497
LOG_INFORM, "received error %s",
491498
onion_wire_name(failcode));
492499
disable_node(
493-
cmd, batch, r,
494-
&route->hops[*result->erring_index - 1].node_id);
500+
r, &route->hops[*result->erring_index - 1].node_id);
495501
break;
496502
case ORIGIN_NODE:
497503
case FINAL_NODE:
@@ -534,13 +540,11 @@ static void handle_failure(struct command *cmd,
534540
/* Usually this means we need to update the channel
535541
* information and try again. To avoid hitting this
536542
* error again with the same channel we flag it. */
537-
struct short_channel_id_dir scidd = {
538-
.scid = route->hops[*result->erring_index].scid,
539-
.dir = route->hops[*result->erring_index].direction};
540543
payment_warn_chan(payment, scidd, LOG_INFORM,
541544
"received error %s",
542545
onion_wire_name(failcode));
543-
bias_channel(cmd, batch, r, scidd, -1);
546+
if (get_erring_scidd(r->route, &scidd))
547+
bias_channel(r, scidd, -1);
544548
break;
545549
case UNKNOWN_NODE:
546550
break;

‎plugins/renepay/routefail.h‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,8 @@
66
#include "config.h"
77
#include <plugins/renepay/route.h>
88

9-
struct command_result *routefail_start(const tal_t *ctx,
9+
struct command_result *routefail_start(struct command *cmd,
1010
struct payment *payment,
11-
struct route *route, struct command *cmd);
11+
struct route *route);
1212

1313
#endif /* LIGHTNING_PLUGINS_RENEPAY_ROUTEFAIL_H */

‎plugins/renepay/routetracker.c‎

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,26 @@ void payment_collect_results(struct payment *payment,
207207
tal_resize(&routetracker->finalized_routes, 0);
208208
}
209209

210+
static void get_erring_scidd_from_index(struct route *route)
211+
{
212+
struct payment_result *result = route->result;
213+
/* Both direction and short_channel_id are known, this step is not
214+
* necessary. */
215+
if (result->erring_direction && result->erring_channel)
216+
return;
217+
/* If the hops or the index are not known, this step is not possible. */
218+
if (!route->hops || !result->erring_index)
219+
return;
220+
/* Is this an error in the destination? */
221+
if (*result->erring_index >= tal_count(route->hops))
222+
return;
223+
result->erring_channel =
224+
tal_dup(result, struct short_channel_id,
225+
&route->hops[*result->erring_index].scid);
226+
result->erring_direction =
227+
tal_dup(result, int, &route->hops[*result->erring_index].direction);
228+
}
229+
210230
struct command_result *notification_sendpay_failure(struct command *cmd,
211231
const char *buf,
212232
const jsmntok_t *params)
@@ -270,10 +290,10 @@ struct command_result *notification_sendpay_failure(struct command *cmd,
270290
status_str);
271291
route->result->status = SENDPAY_FAILED;
272292
}
273-
293+
get_erring_scidd_from_index(route);
274294
/* we do some error processing steps before calling
275295
* route_failure_register. */
276-
return routefail_start(payment, payment, route, cmd);
296+
return routefail_start(cmd, payment, route);
277297
}
278298

279299
struct command_result *notification_sendpay_success(struct command *cmd,

0 commit comments

Comments
 (0)