From 54c57e749520b0b4eacdbd81fe4b0e5ef0862df2 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Thu, 3 Dec 2020 20:03:55 +1030 Subject: [PATCH] libplugin-pay: don't expose bolt11 details. When we support bolt12, this won't exist. We only need min_final_cltv_expiry, routes and features, so put them into struct payment explicitly. We move the default final ctlv out to the caller, too, which is clearer. e.g. keysend was using this value, but it was hard to tell. Signed-off-by: Rusty Russell --- common/bolt11.h | 7 +++++++ plugins/keysend.c | 4 +++- plugins/libplugin-pay.c | 45 ++++++++++++++++------------------------- plugins/libplugin-pay.h | 5 ++++- plugins/pay.c | 6 ++++-- 5 files changed, 35 insertions(+), 32 deletions(-) diff --git a/common/bolt11.h b/common/bolt11.h index f58190eab..77b066462 100644 --- a/common/bolt11.h +++ b/common/bolt11.h @@ -14,6 +14,13 @@ /* We only have 10 bits for the field length, meaning < 640 bytes */ #define BOLT11_FIELD_BYTE_LIMIT ((1 << 10) * 5 / 8) +/* BOLT #11: + * * `c` (24): `data_length` variable. + * `min_final_cltv_expiry` to use for the last HTLC in the route. + * Default is 18 if not specified. + */ +#define DEFAULT_FINAL_CLTV_DELTA 18 + struct feature_set; struct bolt11_field { diff --git a/plugins/keysend.c b/plugins/keysend.c index 03d0c89d6..1dac75752 100644 --- a/plugins/keysend.c +++ b/plugins/keysend.c @@ -137,7 +137,9 @@ static struct command_result *json_keysend(struct command *cmd, const char *buf, p->destination_has_tlv = true; p->payment_secret = NULL; p->amount = *msat; - p->invoice = NULL; + p->routes = NULL; + p->min_final_cltv_expiry = DEFAULT_FINAL_CLTV_DELTA; + p->features = NULL; p->bolt11 = NULL; p->why = "Initial attempt"; p->constraints.cltv_budget = *maxdelay; diff --git a/plugins/libplugin-pay.c b/plugins/libplugin-pay.c index 0d2eb5d44..5df4d8cac 100644 --- a/plugins/libplugin-pay.c +++ b/plugins/libplugin-pay.c @@ -15,13 +15,6 @@ static struct gossmap *gossmap; -/* BOLT #11: - * * `c` (24): `data_length` variable. - * `min_final_cltv_expiry` to use for the last HTLC in the route. - * Default is 18 if not specified. - */ -#define DEFAULT_FINAL_CLTV_DELTA 18 - struct payment *payment_new(tal_t *ctx, struct command *cmd, struct payment *parent, struct payment_modifier **mods) @@ -74,7 +67,9 @@ struct payment *payment_new(tal_t *ctx, struct command *cmd, p->constraints = *parent->start_constraints; p->deadline = parent->deadline; - p->invoice = parent->invoice; + p->min_final_cltv_expiry = parent->min_final_cltv_expiry; + p->routes = parent->routes; + p->features = parent->features; p->id = parent->id; p->local_id = parent->local_id; } else { @@ -264,10 +259,7 @@ void payment_start_at_blockheight(struct payment *p, u32 blockheight) * before we actually call `getroute` */ p->getroute->destination = p->destination; p->getroute->max_hops = ROUTING_MAX_HOPS; - if (root->invoice != NULL && root->invoice->min_final_cltv_expiry != 0) - p->getroute->cltv = root->invoice->min_final_cltv_expiry; - else - p->getroute->cltv = DEFAULT_FINAL_CLTV_DELTA; + p->getroute->cltv = root->min_final_cltv_expiry; p->getroute->amount = p->amount; p->start_constraints = tal_dup(p, struct payment_constraints, &p->constraints); @@ -2419,13 +2411,13 @@ static struct command_result *routehint_getroute_result(struct command *cmd, if (d->destination_reachable) { tal_arr_expand(&d->routehints, NULL); /* The above could trigger a realloc. - * However, p->invoice->routes and d->routehints are + * However, p->routes and d->routehints are * actually the same array, so we need to update the - * p->invoice->routes pointer, since the realloc + * p->routes pointer, since the realloc * might have changed pointer addresses, in order to * ensure that the pointers are not stale. */ - p->invoice->routes = d->routehints; + p->routes = d->routehints; /* FIXME: ***DO*** we need to add this extra routehint? * Once we run out of routehints the default system will @@ -2474,7 +2466,7 @@ static void routehint_step_cb(struct routehints_data *d, struct payment *p) const struct payment *root = payment_root(p); if (p->step == PAYMENT_STEP_INITIALIZED) { - if (root->invoice == NULL || root->invoice->routes == NULL) + if (root->routes == NULL) return payment_continue(p); /* We filter out non-functional routehints once at the @@ -2482,18 +2474,18 @@ static void routehint_step_cb(struct routehints_data *d, struct payment *p) * exluded ones on the fly. */ if (p->parent == NULL) { d->routehints = filter_routehints(d, p->local_id, - p->invoice->routes); + p->routes); /* filter_routehints modifies the array, but * this could trigger a resize and the resize * could trigger a realloc. * Keep the invoice pointer up-to-date. * FIXME: We should really consider that if we are - * mutating p->invoices->routes, maybe we should - * drop d->routehints and just use p->invoice->routes + * mutating p->routes, maybe we should + * drop d->routehints and just use p->routes * directly. * It is probably not a good idea to *copy* the * routehints: other paymods are interested in - * p->invoice->routes, and if the routehints system + * p->routes, and if the routehints system * itself adds or removes routehints from its * copy, the *actual* number of routehints that we * end up using is the one that the routehints paymod @@ -2501,9 +2493,9 @@ static void routehint_step_cb(struct routehints_data *d, struct payment *p) * set of routehints that is the important one. * So rather than copying the array of routehints * in paymod, paymod should use (and mutate) the - * p->invoice->routes array, and + * p->routes array, and */ - p->invoice->routes = d->routehints; + p->routes = d->routehints; paymod_log(p, LOG_DBG, "After filtering routehints we're left with " @@ -3167,10 +3159,7 @@ static void payment_lower_max_htlcs(struct payment *p, u32 limit, static bool payment_supports_mpp(struct payment *p) { - if (p->invoice == NULL || p->invoice->features == NULL) - return false; - - return feature_offered(p->invoice->features, OPT_BASIC_MPP); + return feature_offered(p->features, OPT_BASIC_MPP); } /* Return fuzzed amount ~= target, but never exceeding max */ @@ -3524,8 +3513,8 @@ payee_incoming_limit_count(struct command *cmd, num_channels = channelstok->size; /* If num_channels is 0, check if there is an invoice. */ - if (num_channels == 0 && p->invoice) - num_channels = tal_count(p->invoice->routes); + if (num_channels == 0) + num_channels = tal_count(p->routes); /* If we got a decent number of channels, limit! */ if (num_channels != 0) { diff --git a/plugins/libplugin-pay.h b/plugins/libplugin-pay.h index 6cc0b35dd..cdf945c53 100644 --- a/plugins/libplugin-pay.h +++ b/plugins/libplugin-pay.h @@ -232,7 +232,10 @@ struct payment { void **modifier_data; int current_modifier; - struct bolt11 *invoice; + /* Information from the invoice. */ + u32 min_final_cltv_expiry; + struct route_info **routes; + const u8 *features; /* tal_arr of channel_hints we incrementally learn while performing * payment attempts. */ diff --git a/plugins/pay.c b/plugins/pay.c index ad20439bc..f86cce093 100644 --- a/plugins/pay.c +++ b/plugins/pay.c @@ -2022,13 +2022,15 @@ static struct command_result *json_paymod(struct command *cmd, p->local_id = &my_id; p->json_buffer = tal_steal(p, buf); p->json_toks = params; - p->destination = &b11->receiver_id; + p->destination = tal_dup(p, struct node_id, &b11->receiver_id); p->destination_has_tlv = feature_offered(b11->features, OPT_VAR_ONION); p->payment_hash = tal_dup(p, struct sha256, &b11->payment_hash); p->payment_secret = b11->payment_secret ? tal_dup(p, struct secret, b11->payment_secret) : NULL; - p->invoice = tal_steal(p, b11); + p->routes = tal_steal(p, b11->routes); + p->min_final_cltv_expiry = b11->min_final_cltv_expiry; + p->features = tal_steal(p, b11->features); p->bolt11 = tal_steal(p, b11str); p->why = "Initial attempt"; p->constraints.cltv_budget = *maxdelay;