From 266d1c5d1a7b2d9d9d782aac81578aac1a098728 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 23 Jul 2020 14:20:10 +0930 Subject: [PATCH 1/2] plugins/pay: revert 899a2e64b02d47fc3aa68d70b9f483c2e8d7d685, use routehints in order. This uses @cdecker's idea of excluding the routehinted channel from the route, and also consumes the route hints as it goes so that it makes progress. I don't know if this is correct, but it reliably passes tests/test_pay.py::test_tlv_or_legacy now. Signed-off-by: Rusty Russell --- plugins/libplugin-pay.c | 41 ++++++++++++++++++++++++++--------------- plugins/libplugin-pay.h | 3 +++ 2 files changed, 29 insertions(+), 15 deletions(-) diff --git a/plugins/libplugin-pay.c b/plugins/libplugin-pay.c index b4f612cd1109..e01fde513fe4 100644 --- a/plugins/libplugin-pay.c +++ b/plugins/libplugin-pay.c @@ -26,6 +26,7 @@ struct payment *payment_new(tal_t *ctx, struct command *cmd, p->getroute->riskfactorppm = 10000000; p->abort = false; p->route = NULL; + p->temp_exclusion = NULL; /* Copy over the relevant pieces of information. */ if (parent != NULL) { @@ -482,6 +483,16 @@ static void payment_getroute_add_excludes(struct payment *p, for (size_t i=0; itemp_exclusion) { + struct short_channel_id_dir scidd; + scidd.scid = *p->temp_exclusion; + for (size_t dir = 0; dir < 2; dir++) { + scidd.dir = dir; + json_add_short_channel_id_dir(js, NULL, &scidd); + } + } + json_array_end(js); } @@ -1768,21 +1779,19 @@ static bool routehint_excluded(struct payment *p, } static struct route_info *next_routehint(struct routehints_data *d, - struct payment *p) + struct payment *p) { - /* Implements a random selection of a routehint, or none in 1/numhints - * cases, by starting the iteration of the routehints in a random - * order, and adding a virtual NULL result at the end. */ - size_t numhints = tal_count(d->routehints); - size_t offset = pseudorand(numhints + 1); - - for (size_t i=0; iroutehints); i++) { - size_t curr = (offset + i) % (numhints + 1); - if (curr == numhints) - return NULL; - - if (!routehint_excluded(p, d->routehints[curr])) - return d->routehints[i]; + /* BOLT #11: + * + * - if a writer offers more than one of any field type, it: + * - MUST specify the most-preferred field first, followed + * by less-preferred fields, in order. + */ + while (tal_count(d->routehints) != 0) { + struct route_info *ret = d->routehints[0]; + tal_arr_remove(&d->routehints, 0); + if (!routehint_excluded(p, ret)) + return ret; } return NULL; } @@ -1859,12 +1868,14 @@ static void routehint_step_cb(struct routehints_data *d, struct payment *p) p->getroute->cltv = route_cltv(p->getroute->cltv, d->current_routehint, tal_count(d->current_routehint)); + p->temp_exclusion = &d->current_routehint[0].short_channel_id; plugin_log(p->plugin, LOG_DBG, "Using routehint %s (%s) cltv_delta=%d", type_to_string(tmpctx, struct node_id, &d->current_routehint->pubkey), type_to_string(tmpctx, struct short_channel_id, &d->current_routehint->short_channel_id), d->current_routehint->cltv_expiry_delta ); - } + } else + p->temp_exclusion = NULL; } else if (p->step == PAYMENT_STEP_GOT_ROUTE) { /* Now it's time to stitch the two partial routes together. */ struct amount_msat dest_amount; diff --git a/plugins/libplugin-pay.h b/plugins/libplugin-pay.h index 209c2edce650..ecbcfeac1312 100644 --- a/plugins/libplugin-pay.h +++ b/plugins/libplugin-pay.h @@ -232,6 +232,9 @@ struct payment { struct channel_hint *channel_hints; struct node_id *excluded_nodes; + /* Optional temporarily excluded channel (i.e. this routehint) */ + struct short_channel_id *temp_exclusion; + struct payment_result *result; /* Did something happen that will cause all future attempts to fail? From 8d00b01d54d04912c229422bf6372d66df98c1e8 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 23 Jul 2020 16:30:37 +0930 Subject: [PATCH 2/2] paymod: fix typo which can cause memory overrun. Signed-off-by: Rusty Russell --- plugins/libplugin-pay.c | 43 +++++++++++++++-------------------------- 1 file changed, 16 insertions(+), 27 deletions(-) diff --git a/plugins/libplugin-pay.c b/plugins/libplugin-pay.c index e01fde513fe4..9a0d101e3760 100644 --- a/plugins/libplugin-pay.c +++ b/plugins/libplugin-pay.c @@ -26,7 +26,6 @@ struct payment *payment_new(tal_t *ctx, struct command *cmd, p->getroute->riskfactorppm = 10000000; p->abort = false; p->route = NULL; - p->temp_exclusion = NULL; /* Copy over the relevant pieces of information. */ if (parent != NULL) { @@ -483,16 +482,6 @@ static void payment_getroute_add_excludes(struct payment *p, for (size_t i=0; itemp_exclusion) { - struct short_channel_id_dir scidd; - scidd.scid = *p->temp_exclusion; - for (size_t dir = 0; dir < 2; dir++) { - scidd.dir = dir; - json_add_short_channel_id_dir(js, NULL, &scidd); - } - } - json_array_end(js); } @@ -1767,7 +1756,7 @@ static bool routehint_excluded(struct payment *p, * are suggesting we use it the other way. Very unlikely though! */ for (size_t i = 0; i < tal_count(routehint); i++) { const struct route_info *r = &routehint[i]; - for (size_t j=0; tal_count(nodes); j++) + for (size_t j = 0; j < tal_count(nodes); j++) if (node_id_eq(&r->pubkey, &nodes[j])) return true; @@ -1779,19 +1768,21 @@ static bool routehint_excluded(struct payment *p, } static struct route_info *next_routehint(struct routehints_data *d, - struct payment *p) + struct payment *p) { - /* BOLT #11: - * - * - if a writer offers more than one of any field type, it: - * - MUST specify the most-preferred field first, followed - * by less-preferred fields, in order. - */ - while (tal_count(d->routehints) != 0) { - struct route_info *ret = d->routehints[0]; - tal_arr_remove(&d->routehints, 0); - if (!routehint_excluded(p, ret)) - return ret; + /* Implements a random selection of a routehint, or none in 1/numhints + * cases, by starting the iteration of the routehints in a random + * order, and adding a virtual NULL result at the end. */ + size_t numhints = tal_count(d->routehints); + size_t offset = pseudorand(numhints + 1); + + for (size_t i=0; iroutehints); i++) { + size_t curr = (offset + i) % (numhints + 1); + if (curr == numhints) + return NULL; + + if (!routehint_excluded(p, d->routehints[curr])) + return d->routehints[i]; } return NULL; } @@ -1868,14 +1859,12 @@ static void routehint_step_cb(struct routehints_data *d, struct payment *p) p->getroute->cltv = route_cltv(p->getroute->cltv, d->current_routehint, tal_count(d->current_routehint)); - p->temp_exclusion = &d->current_routehint[0].short_channel_id; plugin_log(p->plugin, LOG_DBG, "Using routehint %s (%s) cltv_delta=%d", type_to_string(tmpctx, struct node_id, &d->current_routehint->pubkey), type_to_string(tmpctx, struct short_channel_id, &d->current_routehint->short_channel_id), d->current_routehint->cltv_expiry_delta ); - } else - p->temp_exclusion = NULL; + } } else if (p->step == PAYMENT_STEP_GOT_ROUTE) { /* Now it's time to stitch the two partial routes together. */ struct amount_msat dest_amount;