From 9fd6f9eae6c24faa906d384dc9fb64c0773890e3 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Sat, 2 Apr 2022 12:47:05 +1030 Subject: [PATCH 1/8] pytest: fix invalid invoice() call. Trips on our RPC checking introduced at the same time: msat should be an integer or an "xxxmsat"/sat/btc string. Signed-off-by: Rusty Russell --- tests/test_pay.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_pay.py b/tests/test_pay.py index 418c29a039db..985e8f1818d4 100644 --- a/tests/test_pay.py +++ b/tests/test_pay.py @@ -5255,7 +5255,7 @@ def test_pay_bolt11_metadata(node_factory, bitcoind): # After CI started failing, I *also* hacked it to set expiry to BIGNUM. inv = "lnbcrt1230n1p3yzgcxsp5q8g040f9rl9mu2unkjuj0vn262s6nyrhz5hythk3ueu2lfzahmzspp5ve584t0cv27hwmy0cx9ca8uwyqyfw9y9dm3r8vus9fv36r2l9yjsdq8v3jhxccmq6w35xjueqd9ejqmt9w3skgct5vyxqxra2q2qcqp99q2sqqqqqysgqfw6efxpzk5x5vfj8se46yg667x5cvhyttnmuqyk0q7rmhx3gs249qhtdggnek8c5adm2pztkjddlwyn2art2zg9xap2ckczzl3fzz4qqsej6mf" # Make l2 "know" about this invoice. - l2.rpc.invoice(msatoshi='123000', label='label1', description='desc', preimage='00' * 32) + l2.rpc.invoice(msatoshi=123000, label='label1', description='desc', preimage='00' * 32) with pytest.raises(RpcError, match=r'WIRE_INVALID_ONION_PAYLOAD'): l1.rpc.pay(inv) From 8cb5ddd66389d534745e107da2643e6799db2efd Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Sat, 2 Apr 2022 12:48:05 +1030 Subject: [PATCH 2/8] plugins/pay: remove legacypay. I think the new pay command has proven itself in the last 18 months! Also various pay tests took "compat" then didn't use it, so clean them up. Signed-off-by: Rusty Russell Changelog-Removed: JSON-RPC: `legacypay` (`pay` replaced it in 0.9.0). --- plugins/pay.c | 1425 +-------------------------------------------- tests/test_pay.py | 52 +- 2 files changed, 10 insertions(+), 1467 deletions(-) diff --git a/plugins/pay.c b/plugins/pay.c index 9dfedfc25e3f..23a9170c58c7 100644 --- a/plugins/pay.c +++ b/plugins/pay.c @@ -24,63 +24,8 @@ static unsigned int maxdelay_default; static bool exp_offers; static bool disablempp = false; -static LIST_HEAD(pay_status); - static LIST_HEAD(payments); -struct pay_attempt { - /* What we changed when starting this attempt. */ - const char *why; - /* Time we started & finished attempt */ - struct timeabs start, end; - /* Route hint we were using (if any) */ - struct route_info *routehint; - /* Channels we excluded when doing route lookup. */ - const char **excludes; - /* Route we got (NULL == route lookup fail). */ - const char *route; - /* Did we actually try to send a payment? */ - bool sendpay; - /* The failure result (NULL on success) */ - struct json_out *failure; - /* The non-failure result (NULL on failure) */ - const char *result; - /* The blockheight at which the payment attempt was - * started. */ - u32 start_block; -}; - -struct pay_status { - /* Destination, as text */ - const char *dest; - - /* We're in 'pay_status' global list. */ - struct list_node list; - - /* Description user provided (if any) */ - const char *label; - /* Amount they wanted to pay. */ - struct amount_msat msat; - /* CLTV delay required by destination. */ - u32 final_cltv; - /* Bolt11/bolt12 invoice. */ - const char *invstring; - - /* What we did about routehints (if anything) */ - const char *routehint_modifications; - - /* Details of shadow route we chose (if any) */ - char *shadow; - - /* Details of initial exclusions (if any) */ - const char *exclusions; - - /* Array of payment attempts. */ - struct pay_attempt *attempts; - - struct sha256 payment_hash; -}; - struct pay_command { /* Global state */ struct plugin *plugin; @@ -141,1257 +86,6 @@ struct pay_command { const char *shadow_dest; }; -static struct pay_attempt *current_attempt(struct pay_command *pc) -{ - return &pc->ps->attempts[tal_count(pc->ps->attempts)-1]; -} - -/* Helper to copy JSON object directly into a json_out */ -static void json_out_add_raw_len(struct json_out *jout, - const char *fieldname, - const char *jsonstr, size_t len) -{ - char *p; - - p = json_out_member_direct(jout, fieldname, len); - memcpy(p, jsonstr, len); -} - -static struct json_out *failed_start(struct pay_command *pc) -{ - struct pay_attempt *attempt = current_attempt(pc); - - attempt->end = time_now(); - attempt->failure = json_out_new(pc->ps->attempts); - json_out_start(attempt->failure, NULL, '{'); - return attempt->failure; -} - -static void failed_end(struct json_out *jout) -{ - json_out_end(jout, '}'); - json_out_finished(jout); -} - -/* Copy field and member to output, if it exists: return member */ -static const jsmntok_t *copy_member(struct json_out *ret, - const char *buf, const jsmntok_t *obj, - const char *membername) -{ - const jsmntok_t *m = json_get_member(buf, obj, membername); - if (!m) - return NULL; - - /* FIXME: The fact it is a string is probably the wrong thing - * to handle: if it *is* a string we should probably copy - * the quote marks, but json_tok_full/json_tok_full_len - * specifically remove those. - * It works *now* because it is only used in "code" and - * "data": "code" is always numeric, and "data" is usually - * a JSON object/key-value table, but pure stromgs will - * probably result in invalid JSON. - */ - /* Literal copy: it's already JSON escaped, and may be a string. */ - json_out_add_raw_len(ret, membername, - json_tok_full(buf, m), json_tok_full_len(m)); - return m; -} - -/* Copy (and modify) error object. */ -static void attempt_failed_tok(struct pay_command *pc, const char *method, - const char *buf, const jsmntok_t *errtok) -{ - const jsmntok_t *msg = json_get_member(buf, errtok, "message"); - struct json_out *failed = failed_start(pc); - - /* Every JSON error response has code and error. */ - copy_member(failed, buf, errtok, "code"); - json_out_add(failed, "message", true, - "Call to %s: %.*s", - method, msg->end - msg->start, - buf + msg->start); - copy_member(failed, buf, errtok, "data"); - failed_end(failed); -} - -static struct command_result *start_pay_attempt(struct command *cmd, - struct pay_command *pc, - const char *fmt, ...); - -/* Is this (erring) channel within the routehint itself? */ -static bool node_or_channel_in_routehint(struct plugin *plugin, - const struct route_info *routehint, - const char *idstr, size_t idlen) -{ - struct node_id nodeid; - struct short_channel_id scid; - bool node_err = true; - - if (!node_id_from_hexstr(idstr, idlen, &nodeid)) { - if (!short_channel_id_from_str(idstr, idlen, &scid)) - plugin_err(plugin, "bad erring_node or erring_channel '%.*s'", - (int)idlen, idstr); - else - node_err = false; - } - - for (size_t i = 0; i < tal_count(routehint); i++) { - if (node_err) { - if (node_id_eq(&nodeid, &routehint[i].pubkey)) - return true; - } else { - if (short_channel_id_eq(&scid, &routehint[i].short_channel_id)) - return true; - } - } - return false; -} - -/* Count times we actually tried to pay, not where route lookup failed or - * we disliked route for being too expensive, etc. */ -static size_t count_sendpays(const struct pay_attempt *attempts) -{ - size_t n = 0; - - for (size_t i = 0; i < tal_count(attempts); i++) - n += attempts[i].sendpay; - - return n; -} - -static struct command_result *waitsendpay_expired(struct command *cmd, - struct pay_command *pc) -{ - char *errmsg; - struct json_stream *data; - size_t num_attempts = count_sendpays(pc->ps->attempts); - - errmsg = tal_fmt(pc, "Gave up after %zu attempt%s: see paystatus", - num_attempts, num_attempts == 1 ? "" : "s"); - data = jsonrpc_stream_fail(cmd, PAY_STOPPED_RETRYING, errmsg); - json_object_start(data, "data"); - json_array_start(data, "attempts"); - for (size_t i = 0; i < tal_count(pc->ps->attempts); i++) { - json_object_start(data, NULL); - if (pc->ps->attempts[i].route) - json_add_jsonstr(data, "route", - pc->ps->attempts[i].route); - json_out_add_splice(data->jout, "failure", - pc->ps->attempts[i].failure); - json_object_end(data); - } - json_array_end(data); - json_object_end(data); - return command_finished(cmd, data); -} - -static bool routehint_excluded(struct plugin *plugin, - const struct route_info *routehint, - const char **excludes) -{ - /* Note that we ignore direction here: in theory, we could have - * found that one direction of a channel is unavailable, but they - * are suggesting we use it the other way. Very unlikely though! */ - for (size_t i = 0; i < tal_count(excludes); i++) - if (node_or_channel_in_routehint(plugin, - routehint, - excludes[i], - strlen(excludes[i]))) - return true; - return false; -} - -static struct command_result *next_routehint(struct command *cmd, - struct pay_command *pc) -{ - size_t num_attempts = count_sendpays(pc->ps->attempts); - - while (tal_count(pc->routehints) > 0) { - if (!routehint_excluded(pc->plugin, pc->routehints[0], - pc->excludes)) { - pc->current_routehint = pc->routehints[0]; - tal_arr_remove(&pc->routehints, 0); - return start_pay_attempt(cmd, pc, "Trying route hint"); - } - tal_free(pc->routehints[0]); - tal_arr_remove(&pc->routehints, 0); - } - - /* No (more) routehints; we're out of routes. */ - /* If we eliminated one because it was too pricy, return that. */ - if (pc->expensive_route) - return command_fail(cmd, PAY_ROUTE_TOO_EXPENSIVE, - "%s", pc->expensive_route); - - if (num_attempts > 0) - return command_fail(cmd, PAY_STOPPED_RETRYING, - "Ran out of routes to try after" - " %zu attempt%s: see paystatus", - num_attempts, num_attempts == 1 ? "" : "s"); - - return command_fail(cmd, PAY_ROUTE_NOT_FOUND, - "Could not find a route"); -} - -static struct command_result * -waitblockheight_done(struct command *cmd, - const char *buf UNUSED, - const jsmntok_t *result UNUSED, - struct pay_command *pc) -{ - return start_pay_attempt(cmd, pc, - "Retried due to blockheight " - "disagreement with payee"); -} -static struct command_result * -waitblockheight_error(struct command *cmd, - const char *buf UNUSED, - const jsmntok_t *error UNUSED, - struct pay_command *pc) -{ - if (time_after(time_now(), pc->stoptime)) - return waitsendpay_expired(cmd, pc); - else - /* Ehhh just retry it. */ - return waitblockheight_done(cmd, buf, error, pc); -} - -static struct command_result * -execute_waitblockheight(struct command *cmd, - u32 blockheight, - struct pay_command *pc) -{ - struct out_req *req; - struct timeabs now = time_now(); - struct timerel remaining; - - if (time_after(now, pc->stoptime)) - return waitsendpay_expired(cmd, pc); - - remaining = time_between(pc->stoptime, now); - - req = jsonrpc_request_start(cmd->plugin, cmd, "waitblockheight", - &waitblockheight_done, - &waitblockheight_error, - pc); - json_add_u32(req->js, "blockheight", blockheight); - json_add_u32(req->js, "timeout", time_to_sec(remaining)); - - return send_outreq(cmd->plugin, req); -} - -/* Gets the remote height from a - * WIRE_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS - * failure. - * Return 0 if unable to find such a height. - */ -static u32 -get_remote_block_height(const char *buf, const jsmntok_t *error) -{ - const u8 *raw_message; - size_t raw_message_len; - u16 type; - - /* Is there even a raw_message? */ - if (json_scan(tmpctx, buf, error, "{data:{raw_message:%}}", - JSON_SCAN_TAL(tmpctx, json_tok_bin_from_hex, - &raw_message)) != NULL) - return 0; - - /* BOLT #4: - * - * 1. type: PERM|15 (`incorrect_or_unknown_payment_details`) - * 2. data: - * * [`u64`:`htlc_msat`] - * * [`u32`:`height`] - * - */ - raw_message_len = tal_count(raw_message); - - type = fromwire_u16(&raw_message, &raw_message_len); /* type */ - if (type != WIRE_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS) - return 0; - - (void) fromwire_u64(&raw_message, &raw_message_len); /* htlc_msat */ - - return fromwire_u32(&raw_message, &raw_message_len); /* height */ -} - -static struct command_result *waitsendpay_error(struct command *cmd, - const char *buf, - const jsmntok_t *error, - struct pay_command *pc) -{ - struct pay_attempt *attempt = current_attempt(pc); - errcode_t code; - int failcode; - const char *err; - - attempt_failed_tok(pc, "waitsendpay", buf, error); - - err = json_scan(tmpctx, buf, error, "{code:%}", - JSON_SCAN(json_to_errcode, &code)); - if (err) - plugin_err(cmd->plugin, "waitsendpay error %s? '%.*s'", - err, - json_tok_full_len(error), json_tok_full(buf, error)); - - if (code != PAY_UNPARSEABLE_ONION) { - err = json_scan(tmpctx, buf, error, "{data:{failcode:%}}", - JSON_SCAN(json_to_int, &failcode)); - if (err) - plugin_err(cmd->plugin, "waitsendpay failcode error %s '%.*s'", - err, - json_tok_full_len(error), json_tok_full(buf, error)); - } - - /* Special case for WIRE_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS. - * - * One possible trigger for this failure is that the receiver - * thinks the final timeout it gets is too near the future. - * - * For the most part, we respect the indicated `final_cltv` - * in the invoice, and our shadow routing feature also tends - * to give more timing budget to the receiver than the - * `final_cltv`. - * - * However, there is an edge case possible on real networks: - * - * * We send out a payment respecting the `final_cltv` of - * the receiver. - * * Miners mine a new block while the payment is in transit. - * * By the time the payment reaches the receiver, the - * payment violates the `final_cltv` because the receiver - * is now using a different basis blockheight. - * - * This is a transient error. - * Unfortunately, WIRE_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS - * is marked with the PERM bit. - * This means that we would give up on this since `waitsendpay` - * would return PAY_DESTINATION_PERM_FAIL instead of - * PAY_TRY_OTHER_ROUTE. - * Thus the `pay` plugin would not retry this case. - * - * Thus, we need to add this special-case checking here, where - * the blockheight when we started the pay attempt was not - * the same as what the payee reports. - * - * In the past this particular failure had its own failure code, - * equivalent to 17. - * In case the receiver is a really old software, we also - * special-case it here. - */ - if ((code != PAY_UNPARSEABLE_ONION) && - ((failcode == 17) || - ((failcode == WIRE_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS) && - (attempt->start_block < get_remote_block_height(buf, error))))) { - u32 target_blockheight; - - if (failcode == 17) - target_blockheight = attempt->start_block + 1; - else - target_blockheight = get_remote_block_height(buf, error); - - return execute_waitblockheight(cmd, target_blockheight, - pc); - } - - /* FIXME: Handle PAY_UNPARSEABLE_ONION! */ - - /* Many error codes are final. */ - if (code != PAY_TRY_OTHER_ROUTE) { - return forward_error(cmd, buf, error, pc); - } - - if (time_after(time_now(), pc->stoptime)) { - return waitsendpay_expired(cmd, pc); - } - - if (failcode & NODE) { - struct node_id id; - const char *idstr, *err; - - err = json_scan(tmpctx, buf, error, "{data:{erring_node:%}}", - JSON_SCAN(json_to_node_id, &id)); - if (err) - plugin_err(cmd->plugin, "waitsendpay error %s '%.*s'", - err, - json_tok_full_len(error), - json_tok_full(buf, error)); - - /* FIXME: Keep as node_id, don't use strings. */ - idstr = node_id_to_hexstr(tmpctx, &id); - - /* If failure is in routehint part, try next one */ - if (node_or_channel_in_routehint(pc->plugin, pc->current_routehint, - idstr, strlen(idstr))) - return next_routehint(cmd, pc); - - /* Otherwise, add erring channel to exclusion list. */ - tal_arr_expand(&pc->excludes, tal_steal(pc->excludes, idstr)); - } else { - struct short_channel_id scid; - u32 dir; - const char *scidstr, *err; - - err = json_scan(tmpctx, buf, error, - "{data:{erring_channel:%,erring_direction:%}}", - JSON_SCAN(json_to_short_channel_id, &scid), - JSON_SCAN(json_to_number, &dir)); - if (err) - plugin_err(cmd->plugin, "waitsendpay error %s '%.*s'", - err, - json_tok_full_len(error), - json_tok_full(buf, error)); - - scidstr = short_channel_id_to_str(tmpctx, &scid); - /* If failure is in routehint part, try next one */ - if (node_or_channel_in_routehint(pc->plugin, pc->current_routehint, - scidstr, strlen(scidstr))) - return next_routehint(cmd, pc); - - /* Otherwise, add erring channel to exclusion list. */ - tal_arr_expand(&pc->excludes, - tal_fmt(pc->excludes, "%s/%u", scidstr, dir)); - } - - /* Try again. */ - return start_pay_attempt(cmd, pc, "Excluded %s %s", - failcode & NODE ? "node" : "channel", - pc->excludes[tal_count(pc->excludes)-1]); -} - -static struct command_result *waitsendpay_done(struct command *cmd, - const char *buf, - const jsmntok_t *result, - struct pay_command *pc) -{ - struct pay_attempt *attempt = current_attempt(pc); - - attempt->result = json_strdup(pc->ps->attempts, buf, result); - attempt->end = time_now(); - - return forward_result(cmd, buf, result, pc); -} - -static struct command_result *sendpay_done(struct command *cmd, - const char *buf, - const jsmntok_t *result, - struct pay_command *pc) -{ - struct out_req *req = jsonrpc_request_start(cmd->plugin, cmd, - "waitsendpay", - waitsendpay_done, - waitsendpay_error, pc); - json_add_string(req->js, "payment_hash", pc->payment_hash); - - return send_outreq(cmd->plugin, req); -} - -/* Calculate how many millisatoshi we need at the start of this route - * to get msatoshi to the end. */ -static bool route_msatoshi(struct amount_msat *total, - const struct amount_msat msat, - const struct route_info *route, size_t num_route) -{ - *total = msat; - for (ssize_t i = num_route - 1; i >= 0; i--) { - if (!amount_msat_add_fee(total, - route[i].fee_base_msat, - route[i].fee_proportional_millionths)) - return false; - } - return true; -} - -/* Calculate cltv we need at the start of this route to get cltv at the end. */ -static u32 route_cltv(u32 cltv, - const struct route_info *route, size_t num_route) -{ - for (size_t i = 0; i < num_route; i++) - cltv += route[i].cltv_expiry_delta; - return cltv; -} - -/* The pubkey to use is the destination of this routehint. */ -static const char *route_pubkey(const tal_t *ctx, - const struct pay_command *pc, - const struct route_info *routehint, - size_t n) -{ - if (n == tal_count(routehint)) - return pc->dest; - return type_to_string(ctx, struct node_id, &routehint[n].pubkey); -} - -static const char *join_routehint(const tal_t *ctx, - const char *buf, - const jsmntok_t *route, - const struct pay_command *pc, - const struct route_info *routehint) -{ - char *ret; - - /* Truncate closing ] from route */ - ret = tal_strndup(ctx, buf + route->start, route->end - route->start - 1); - for (size_t i = 0; i < tal_count(routehint); i++) { - /* amount to be received by *destination* */ - struct amount_msat dest_amount; - - if (!route_msatoshi(&dest_amount, pc->msat, - routehint + i + 1, - tal_count(routehint) - i - 1)) - return tal_free(ret); - - tal_append_fmt(&ret, ", {" - " \"id\": \"%s\"," - " \"channel\": \"%s\"," - " \"msatoshi\": \"%s\"," - " \"delay\": %u }", - /* pubkey of *destination* */ - route_pubkey(tmpctx, pc, routehint, i + 1), - type_to_string(tmpctx, struct short_channel_id, - &routehint[i].short_channel_id), - type_to_string(tmpctx, struct amount_msat, - &dest_amount), - /* cltv for *destination* */ - route_cltv(pc->final_cltv, routehint + i + 1, - tal_count(routehint) - i - 1)); - } - /* Put ] back */ - tal_append_fmt(&ret, "]"); - return ret; -} - -static struct command_result *sendpay_error(struct command *cmd, - const char *buf, - const jsmntok_t *error, - struct pay_command *pc) -{ - attempt_failed_tok(pc, "sendpay", buf, error); - - return forward_error(cmd, buf, error, pc); -} - -static const jsmntok_t *find_worst_channel(const char *buf, - const jsmntok_t *route, - const char *fieldname) -{ - u64 prev, worstval = 0; - const jsmntok_t *worst = NULL, *t, *t_prev = NULL; - size_t i; - - json_for_each_arr(i, t, route) { - u64 val; - - json_to_u64(buf, json_get_member(buf, t, fieldname), &val); - - /* For the first hop, now we can't know if it's the worst. - * Just store the info and continue. */ - if (!i) { - prev = val; - t_prev = t; - continue; - } - - if (worst == NULL || prev - val > worstval) { - worst = t_prev; - worstval = prev - val; - } - prev = val; - t_prev = t; - } - - return worst; -} - -/* Can't exclude if it's in routehint itself. */ -static bool maybe_exclude(struct pay_command *pc, - const char *buf, const jsmntok_t *route) -{ - const jsmntok_t *scid, *dir; - - if (!route) - return false; - - scid = json_get_member(buf, route, "channel"); - - if (node_or_channel_in_routehint(pc->plugin, - pc->current_routehint, - buf + scid->start, - scid->end - scid->start)) - return false; - - dir = json_get_member(buf, route, "direction"); - tal_arr_expand(&pc->excludes, - tal_fmt(pc->excludes, "%.*s/%c", - scid->end - scid->start, - buf + scid->start, - buf[dir->start])); - return true; -} - -static struct command_result *getroute_done(struct command *cmd, - const char *buf, - const jsmntok_t *result, - struct pay_command *pc) -{ - struct pay_attempt *attempt = current_attempt(pc); - const jsmntok_t *t = json_get_member(buf, result, "route"); - struct amount_msat fee; - struct amount_msat max_fee; - u32 delay; - struct out_req *req; - const char *err; - - if (!t) - plugin_err(cmd->plugin, "getroute gave no 'route'? '%.*s'", - result->end - result->start, buf); - - if (pc->current_routehint) { - attempt->route = join_routehint(pc->ps->attempts, buf, t, - pc, pc->current_routehint); - if (!attempt->route) { - struct json_out *failed = failed_start(pc); - json_out_add(failed, "message", true, - "Joining routehint gave absurd fee"); - failed_end(failed); - return next_routehint(cmd, pc); - } - } else - attempt->route = json_strdup(pc->ps->attempts, buf, t); - - err = json_scan(tmpctx, buf, t, "[0:{msatoshi:%,delay:%}]", - JSON_SCAN(json_to_msat, &fee), - JSON_SCAN(json_to_number, &delay)); - if (err) - plugin_err(cmd->plugin, "getroute %s? %.*s", - err, - json_tok_full_len(result), json_tok_full(buf, result)); - - if (pc->maxfee_pct_millionths / 100 > UINT32_MAX) - plugin_err(cmd->plugin, "max fee percent too large: %lf", - pc->maxfee_pct_millionths / 1000000.0); - - if (!amount_msat_fee(&max_fee, pc->msat, 0, - (u32)(pc->maxfee_pct_millionths / 100))) - plugin_err( - cmd->plugin, "max fee too large: %s * %lf%%", - type_to_string(tmpctx, struct amount_msat, &pc->msat), - pc->maxfee_pct_millionths / 1000000.0); - - if (amount_msat_greater(fee, pc->exemptfee) && - amount_msat_greater(fee, max_fee)) { - const jsmntok_t *charger; - struct json_out *failed; - char *feemsg; - - feemsg = tal_fmt(pc, "Route wanted fee of %s", - type_to_string(tmpctx, struct amount_msat, - &fee)); - failed = failed_start(pc); - json_out_addstr(failed, "message", feemsg); - failed_end(failed); - - /* Remember this if we eliminating this causes us to have no - * routes at all! */ - if (!pc->expensive_route) - pc->expensive_route = feemsg; - else - tal_free(feemsg); - - /* Try excluding most fee-charging channel (unless it's in - * routeboost). */ - charger = find_worst_channel(buf, t, "msatoshi"); - if (maybe_exclude(pc, buf, charger)) { - return start_pay_attempt(cmd, pc, - "Excluded expensive channel %s", - pc->excludes[tal_count(pc->excludes)-1]); - } - - return next_routehint(cmd, pc); - } - - if (delay > pc->maxdelay) { - const jsmntok_t *delayer; - struct json_out *failed; - char *feemsg; - - feemsg = tal_fmt(pc, "Route wanted delay of %u blocks", delay); - failed = failed_start(pc); - json_out_addstr(failed, "message", feemsg); - failed_end(failed); - - /* Remember this if we eliminating this causes us to have no - * routes at all! */ - if (!pc->expensive_route) - pc->expensive_route = feemsg; - else - tal_free(failed); - - delayer = find_worst_channel(buf, t, "delay"); - - /* Try excluding most delaying channel (unless it's in - * routeboost). */ - if (maybe_exclude(pc, buf, delayer)) { - return start_pay_attempt(cmd, pc, - "Excluded delaying channel %s", - pc->excludes[tal_count(pc->excludes)-1]); - } - - return next_routehint(cmd, pc); - } - - attempt->sendpay = true; - req = jsonrpc_request_start(cmd->plugin, cmd, "sendpay", - sendpay_done, sendpay_error, pc); - json_add_jsonstr(req->js, "route", attempt->route); - json_add_string(req->js, "payment_hash", pc->payment_hash); - json_add_string(req->js, "bolt11", pc->ps->invstring); - if (pc->label) - json_add_string(req->js, "label", pc->label); - if (pc->payment_secret) - json_add_string(req->js, "payment_secret", pc->payment_secret); - if (pc->payment_metadata) - json_add_string(req->js, "payment_metadata", pc->payment_metadata); - - return send_outreq(cmd->plugin, req); -} - -static struct command_result *getroute_error(struct command *cmd, - const char *buf, - const jsmntok_t *error, - struct pay_command *pc) -{ - errcode_t code; - const jsmntok_t *codetok; - - attempt_failed_tok(pc, "getroute", buf, error); - - codetok = json_get_member(buf, error, "code"); - if (!json_to_errcode(buf, codetok, &code)) - plugin_err(cmd->plugin, "getroute error gave no 'code'? '%.*s'", - error->end - error->start, buf + error->start); - - /* Strange errors from getroute should be forwarded. */ - if (code != PAY_ROUTE_NOT_FOUND) - return forward_error(cmd, buf, error, pc); - - return next_routehint(cmd, pc); -} - -/* Deep copy of excludes array. */ -static const char **dup_excludes(const tal_t *ctx, const char **excludes) -{ - const char **ret = tal_dup_talarr(ctx, const char *, excludes); - for (size_t i = 0; i < tal_count(ret); i++) - ret[i] = tal_strdup(ret, excludes[i]); - return ret; -} - -/* Get a route from the lightningd. */ -static struct command_result *execute_getroute(struct command *cmd, - struct pay_command *pc) -{ - struct pay_attempt *attempt = current_attempt(pc); - - u32 max_hops = ROUTING_MAX_HOPS; - struct amount_msat msat; - const char *dest; - u32 cltv; - struct out_req *req; - - /* routehint set below. */ - - /* If we have a routehint, try that first; we need to do extra - * checks that it meets our criteria though. */ - if (pc->current_routehint) { - attempt->routehint = tal_steal(pc->ps, pc->current_routehint); - if (!route_msatoshi(&msat, pc->msat, - attempt->routehint, - tal_count(attempt->routehint))) { - struct json_out *failed; - - failed = failed_start(pc); - json_out_addstr(failed, "message", - "Routehint absurd fee"); - failed_end(failed); - return next_routehint(cmd, pc); - } - dest = type_to_string(tmpctx, struct node_id, - &attempt->routehint[0].pubkey); - max_hops -= tal_count(attempt->routehint); - cltv = route_cltv(pc->final_cltv, - attempt->routehint, - tal_count(attempt->routehint)); - } else { - msat = pc->msat; - dest = pc->dest; - cltv = pc->final_cltv; - attempt->routehint = NULL; - } - - /* OK, ask for route to destination */ - req = jsonrpc_request_start(cmd->plugin, cmd, "getroute", - getroute_done, getroute_error, pc); - json_add_string(req->js, "id", dest); - json_add_string(req->js, "msatoshi", - type_to_string(tmpctx, struct amount_msat, &msat)); - json_add_u32(req->js, "cltv", cltv); - json_add_u32(req->js, "maxhops", max_hops); - json_add_member(req->js, "riskfactor", false, "%lf", - pc->riskfactor_millionths / 1000000.0); - if (tal_count(pc->excludes) != 0) { - json_array_start(req->js, "exclude"); - for (size_t i = 0; i < tal_count(pc->excludes); i++) - json_add_string(req->js, NULL, pc->excludes[i]); - json_array_end(req->js); - } - - return send_outreq(cmd->plugin, req); -} - -static struct command_result * -getstartblockheight_done(struct command *cmd, - const char *buf, - const jsmntok_t *result, - struct pay_command *pc) -{ - const jsmntok_t *blockheight_tok; - u32 blockheight; - - blockheight_tok = json_get_member(buf, result, "blockheight"); - if (!blockheight_tok) - plugin_err(cmd->plugin, "getstartblockheight: " - "getinfo gave no 'blockheight'? '%.*s'", - result->end - result->start, buf); - - if (!json_to_u32(buf, blockheight_tok, &blockheight)) - plugin_err(cmd->plugin, "getstartblockheight: " - "getinfo gave non-unsigned-32-bit 'blockheight'? '%.*s'", - result->end - result->start, buf); - - current_attempt(pc)->start_block = blockheight; - - return execute_getroute(cmd, pc); -} - -static struct command_result * -getstartblockheight_error(struct command *cmd, - const char *buf, - const jsmntok_t *error, - struct pay_command *pc) -{ - /* Should never happen. */ - plugin_err(cmd->plugin, "getstartblockheight: getinfo failed!? '%.*s'", - error->end - error->start, buf); -} - -static struct command_result * -execute_getstartblockheight(struct command *cmd, - struct pay_command *pc) -{ - struct out_req *req = jsonrpc_request_start(cmd->plugin, cmd, "getinfo", - &getstartblockheight_done, - &getstartblockheight_error, - pc); - return send_outreq(cmd->plugin, req); -} - -static struct command_result *start_pay_attempt(struct command *cmd, - struct pay_command *pc, - const char *fmt, ...) -{ - struct pay_attempt *attempt; - va_list ap; - size_t n; - - n = tal_count(pc->ps->attempts); - tal_resize(&pc->ps->attempts, n+1); - attempt = &pc->ps->attempts[n]; - - va_start(ap, fmt); - attempt->start = time_now(); - /* Mark it unfinished */ - attempt->end.ts.tv_sec = -1; - attempt->excludes = dup_excludes(pc->ps, pc->excludes); - attempt->route = NULL; - attempt->failure = NULL; - attempt->result = NULL; - attempt->sendpay = false; - attempt->why = tal_vfmt(pc->ps, fmt, ap); - va_end(ap); - - return execute_getstartblockheight(cmd, pc); -} - -/* BOLT #7: - * - * If a route is computed by simply routing to the intended recipient and - * summing the `cltv_expiry_delta`s, then it's possible for intermediate nodes - * to guess their position in the route. Knowing the CLTV of the HTLC, the - * surrounding network topology, and the `cltv_expiry_delta`s gives an - * attacker a way to guess the intended recipient. Therefore, it's highly - * desirable to add a random offset to the CLTV that the intended recipient - * will receive, which bumps all CLTVs along the route. - * - * In order to create a plausible offset, the origin node MAY start a limited - * random walk on the graph, starting from the intended recipient and summing - * the `cltv_expiry_delta`s, and use the resulting sum as the offset. This - * effectively creates a _shadow route extension_ to the actual route and - * provides better protection against this attack vector than simply picking a - * random offset would. - */ -static struct command_result *shadow_route(struct command *cmd, - struct pay_command *pc); - -static struct command_result *add_shadow_route(struct command *cmd, - const char *buf, - const jsmntok_t *result, - struct pay_command *pc) -{ - /* Use reservoir sampling across the capable channels. */ - const jsmntok_t *channels = json_get_member(buf, result, "channels"); - const jsmntok_t *chan, *best = NULL; - size_t i; - u64 sample = 0; - struct route_info *route = tal_arr(NULL, struct route_info, 1); - struct amount_msat fees, maxfees; - /* Don't go above this. Note how we use the initial amount to get the percentage - * of the fees, or it would increase with the addition of new shadow routes. */ - if (!amount_msat_fee(&maxfees, pc->initial_msat, 0, pc->maxfee_pct_millionths)) - plugin_err(cmd->plugin, "Overflow when computing maxfees for " - "shadow routes."); - - json_for_each_arr(i, chan, channels) { - u64 v = pseudorand(UINT64_MAX); - - if (!best || v > sample) { - struct amount_sat sat; - - json_to_sat(buf, json_get_member(buf, chan, "satoshis"), &sat); - if (amount_msat_greater_sat(pc->msat, sat)) - continue; - - /* Don't use if total would exceed 1/4 of our time allowance. */ - json_to_u16(buf, json_get_member(buf, chan, "delay"), - &route[0].cltv_expiry_delta); - if ((pc->final_cltv + route[0].cltv_expiry_delta) * 4 > pc->maxdelay) - continue; - - json_to_number(buf, json_get_member(buf, chan, "base_fee_millisatoshi"), - &route[0].fee_base_msat); - json_to_number(buf, json_get_member(buf, chan, "fee_per_millionth"), - &route[0].fee_proportional_millionths); - - if (!amount_msat_fee(&fees, pc->initial_msat, route[0].fee_base_msat, - route[0].fee_proportional_millionths) - || amount_msat_greater_eq(fees, maxfees)) - continue; - - best = chan; - sample = v; - } - } - - if (!best) { - tal_append_fmt(&pc->ps->shadow, - "No suitable channels found to %s. ", - pc->shadow_dest); - return start_pay_attempt(cmd, pc, "Initial attempt"); - } - - pc->final_cltv += route[0].cltv_expiry_delta; - pc->shadow_dest = json_strdup(pc, buf, - json_get_member(buf, best, "destination")); - route_msatoshi(&pc->msat, pc->msat, route, 1); - tal_append_fmt(&pc->ps->shadow, - "Added %u cltv delay, %u base fee, and %u ppm fee " - "for shadow to %s.", - route[0].cltv_expiry_delta, route[0].fee_base_msat, - route[0].fee_proportional_millionths, - pc->shadow_dest); - tal_free(route); - - return shadow_route(cmd, pc); -} - -static struct command_result *shadow_route(struct command *cmd, - struct pay_command *pc) -{ - struct out_req *req; - -#if DEVELOPER - if (!pc->use_shadow) - return start_pay_attempt(cmd, pc, "Initial attempt"); -#endif - if (pseudorand(2) == 0) - return start_pay_attempt(cmd, pc, "Initial attempt"); - - req = jsonrpc_request_start(cmd->plugin, cmd, "listchannels", - add_shadow_route, forward_error, pc); - json_add_string(req->js, "source", pc->shadow_dest); - return send_outreq(cmd->plugin, req); -} - -/* gossipd doesn't know much about the current state of channels; here we - * manually exclude peers which are disconnected and channels which lack - * current capacity (it will eliminate those without total capacity). */ -static struct command_result *listpeers_done(struct command *cmd, - const char *buf, - const jsmntok_t *result, - struct pay_command *pc) -{ - const jsmntok_t *peers, *peer; - size_t i; - char *mods = tal_strdup(tmpctx, ""); - - peers = json_get_member(buf, result, "peers"); - if (!peers) - plugin_err(cmd->plugin, "listpeers gave no 'peers'? '%.*s'", - result->end - result->start, buf); - - json_for_each_arr(i, peer, peers) { - const jsmntok_t *chans, *chan; - bool connected; - size_t j; - - json_to_bool(buf, json_get_member(buf, peer, "connected"), - &connected); - chans = json_get_member(buf, peer, "channels"); - json_for_each_arr(j, chan, chans) { - const jsmntok_t *state, *scid, *dir; - struct amount_msat spendable; - - /* gossipd will only consider things in state NORMAL - * anyway; we don't need to exclude others. */ - state = json_get_member(buf, chan, "state"); - if (!json_tok_streq(buf, state, "CHANNELD_NORMAL")) - continue; - - json_to_msat(buf, - json_get_member(buf, chan, - "spendable_msatoshi"), - &spendable); - - if (connected - && amount_msat_greater_eq(spendable, pc->msat)) - continue; - - /* Exclude this disconnected or low-capacity channel */ - scid = json_get_member(buf, chan, "short_channel_id"); - dir = json_get_member(buf, chan, "direction"); - tal_arr_expand(&pc->excludes, - tal_fmt(pc->excludes, "%.*s/%c", - scid->end - scid->start, - buf + scid->start, - buf[dir->start])); - - tal_append_fmt(&mods, - "Excluded channel %s (%s, %s). ", - pc->excludes[tal_count(pc->excludes)-1], - type_to_string(tmpctx, struct amount_msat, - &spendable), - connected ? "connected" : "disconnected"); - } - } - - if (!streq(mods, "")) - pc->ps->exclusions = tal_steal(pc->ps, mods); - - pc->ps->shadow = tal_strdup(pc->ps, ""); - return shadow_route(cmd, pc); -} - -/* Trim route to this length by taking from the *front* of route - * (end points to destination, so we need that bit!) */ -static void trim_route(struct route_info **route, size_t n) -{ - size_t remove = tal_count(*route) - n; - memmove(*route, *route + remove, sizeof(**route) * n); - tal_resize(route, n); -} - -/* Make sure routehints are reasonable length, and (since we assume we - * can append), not directly to us. Note: untrusted data! */ -static struct route_info **filter_routehints(struct pay_command *pc, - struct route_info **hints) -{ - char *mods = tal_strdup(tmpctx, ""); - - for (size_t i = 0; i < tal_count(hints); i++) { - /* Trim any routehint > 10 hops */ - size_t max_hops = ROUTING_MAX_HOPS / 2; - if (tal_count(hints[i]) > max_hops) { - tal_append_fmt(&mods, - "Trimmed routehint %zu (%zu hops) to %zu. ", - i, tal_count(hints[i]), max_hops); - trim_route(&hints[i], max_hops); - } - - /* If we are first hop, trim. */ - if (tal_count(hints[i]) > 0 - && node_id_eq(&hints[i][0].pubkey, &my_id)) { - tal_append_fmt(&mods, - "Removed ourselves from routehint %zu. ", - i); - trim_route(&hints[i], tal_count(hints[i])-1); - } - - /* If route is empty, remove altogether. */ - if (tal_count(hints[i]) == 0) { - tal_append_fmt(&mods, - "Removed empty routehint %zu. ", i); - tal_arr_remove(&hints, i); - i--; - } - } - - if (!streq(mods, "")) - pc->ps->routehint_modifications = tal_steal(pc->ps, mods); - - return tal_steal(pc, hints); -} - -static struct pay_status *add_pay_status(struct pay_command *pc, - const char *invstring STEALS) -{ - struct pay_status *ps = tal(NULL, struct pay_status); - - /* The pay_status outlives the pc, so it simply takes field ownership */ - ps->dest = tal_steal(ps, pc->dest); - ps->label = tal_steal(ps, pc->label); - ps->msat = pc->msat; - ps->final_cltv = pc->final_cltv; - ps->invstring = tal_steal(ps, invstring); - ps->routehint_modifications = NULL; - ps->shadow = NULL; - ps->exclusions = NULL; - ps->attempts = tal_arr(ps, struct pay_attempt, 0); - hex_decode(pc->payment_hash, strlen(pc->payment_hash), - &ps->payment_hash, sizeof(ps->payment_hash)); - - list_add_tail(&pay_status, &ps->list); - return ps; -} - -#ifndef COMPAT_V090 -UNUSED -#endif -static struct command_result *json_pay(struct command *cmd, - const char *buf, - const jsmntok_t *params) -{ - struct amount_msat *msat; - struct bolt11 *b11; - const char *b11str; - char *fail; - u64 *riskfactor_millionths; - unsigned int *retryfor; - struct pay_command *pc = tal(cmd, struct pay_command); - u64 *maxfee_pct_millionths; - unsigned int *maxdelay; - struct amount_msat *exemptfee; - struct out_req *req; -#if DEVELOPER - bool *use_shadow; -#endif - - if (!param(cmd, buf, params, p_req("bolt11", param_string, &b11str), - p_opt("msatoshi", param_msat, &msat), - p_opt("label", param_string, &pc->label), - p_opt_def("riskfactor", param_millionths, - &riskfactor_millionths, 10000000), - p_opt_def("maxfeepercent", param_millionths, - &maxfee_pct_millionths, 500000), - p_opt_def("retry_for", param_number, &retryfor, 60), - p_opt_def("maxdelay", param_number, &maxdelay, - maxdelay_default), - p_opt_def("exemptfee", param_msat, &exemptfee, - AMOUNT_MSAT(5000)), -#if DEVELOPER - p_opt_def("use_shadow", param_bool, &use_shadow, true), -#endif - NULL)) - return command_param_failed(); - - b11 = bolt11_decode(cmd, b11str, plugin_feature_set(cmd->plugin), - NULL, chainparams, &fail); - if (!b11) { - return command_fail(cmd, JSONRPC2_INVALID_PARAMS, - "Invalid bolt11: %s", fail); - } - - if (!b11->chain) { - return command_fail(cmd, PAY_ROUTE_NOT_FOUND, "Invoice is for an unknown network"); - } - - if (b11->chain != chainparams) { - return command_fail(cmd, PAY_ROUTE_NOT_FOUND, "Invoice is for another network %s", b11->chain->network_name); - } - - if (time_now().ts.tv_sec > b11->timestamp + b11->expiry) { - return command_fail(cmd, PAY_INVOICE_EXPIRED, "Invoice expired"); - } - - if (b11->msat) { - if (msat) { - return command_fail(cmd, JSONRPC2_INVALID_PARAMS, - "msatoshi parameter unnecessary"); - } - pc->msat = pc->initial_msat = *b11->msat; - } else { - if (!msat) { - return command_fail(cmd, JSONRPC2_INVALID_PARAMS, - "msatoshi parameter required"); - } - pc->msat = pc->initial_msat = *msat; - } - - /* Sanity check */ - if (feature_offered(b11->features, OPT_VAR_ONION) - && !b11->payment_secret) { - return command_fail(cmd, JSONRPC2_INVALID_PARAMS, - "Invalid bolt11:" - " sets feature var_onion with no secret"); - } - - pc->maxfee_pct_millionths = *maxfee_pct_millionths; - pc->maxdelay = *maxdelay; - pc->exemptfee = *exemptfee; - pc->riskfactor_millionths = *riskfactor_millionths; - pc->final_cltv = b11->min_final_cltv_expiry; - pc->dest = type_to_string(cmd, struct node_id, &b11->receiver_id); - pc->shadow_dest = tal_strdup(pc, pc->dest); - pc->payment_hash = type_to_string(pc, struct sha256, - &b11->payment_hash); - pc->stoptime = timeabs_add(time_now(), time_from_sec(*retryfor)); - pc->excludes = tal_arr(cmd, const char *, 0); - pc->ps = add_pay_status(pc, b11str); - if (b11->payment_secret) - pc->payment_secret = tal_hexstr(pc, b11->payment_secret, - sizeof(*b11->payment_secret)); - else - pc->payment_secret = NULL; - if (b11->metadata) - pc->payment_metadata = tal_hex(pc, b11->metadata); - else - pc->payment_metadata = NULL; - - /* We try first without using routehint */ - pc->current_routehint = NULL; - pc->routehints = filter_routehints(pc, b11->routes); - pc->expensive_route = NULL; -#if DEVELOPER - pc->use_shadow = *use_shadow; -#endif - - /* Get capacities of local channels (no parameters) */ - req = jsonrpc_request_start(cmd->plugin, cmd, "listpeers", - listpeers_done, forward_error, pc); - return send_outreq(cmd->plugin, req); -} - /* FIXME: Add this to ccan/time? */ #define UTC_TIMELEN (sizeof("YYYY-mm-ddTHH:MM:SS.nnnZ")) static void utc_timestring(const struct timeabs *time, char str[UTC_TIMELEN]) @@ -1410,66 +104,6 @@ static void utc_timestring(const struct timeabs *time, char str[UTC_TIMELEN]) (int) time->ts.tv_nsec / 1000000); } -static void add_attempt(struct json_stream *ret, - const struct pay_status *ps, - const struct pay_attempt *attempt) -{ - char timestr[UTC_TIMELEN]; - - utc_timestring(&attempt->start, timestr); - - json_object_start(ret, NULL); - json_add_string(ret, "strategy", attempt->why); - json_add_string(ret, "start_time", timestr); - json_add_u64(ret, "age_in_seconds", - time_to_sec(time_between(time_now(), attempt->start))); - if (attempt->result || attempt->failure) { - utc_timestring(&attempt->end, timestr); - json_add_string(ret, "end_time", timestr); - json_add_u64(ret, "duration_in_seconds", - time_to_sec(time_between(attempt->end, - attempt->start))); - } - if (tal_count(attempt->routehint)) { - json_array_start(ret, "routehint"); - for (size_t i = 0; i < tal_count(attempt->routehint); i++) { - json_object_start(ret, NULL); - json_add_string(ret, "id", - type_to_string(tmpctx, struct node_id, - &attempt->routehint[i].pubkey)); - json_add_string(ret, "channel", - type_to_string(tmpctx, - struct short_channel_id, - &attempt->routehint[i].short_channel_id)); - json_add_u64(ret, "fee_base_msat", - attempt->routehint[i].fee_base_msat); - json_add_u64(ret, "fee_proportional_millionths", - attempt->routehint[i].fee_proportional_millionths); - json_add_u64(ret, "cltv_expiry_delta", - attempt->routehint[i].cltv_expiry_delta); - json_object_end(ret); - } - json_array_end(ret); - } - if (tal_count(attempt->excludes)) { - json_array_start(ret, "excluded_nodes_or_channels"); - for (size_t i = 0; i < tal_count(attempt->excludes); i++) - json_add_string(ret, NULL, attempt->excludes[i]); - json_array_end(ret); - } - - if (attempt->route) - json_add_jsonstr(ret, "route", attempt->route); - - if (attempt->failure) - json_out_add_splice(ret->jout, "failure", attempt->failure); - - if (attempt->result) - json_add_member(ret, "success", true, "%s", attempt->result); - - json_object_end(ret); -} - static void json_add_sendpay_result(struct json_stream *s, const struct payment_result *r) { if (r->code != 0) { @@ -1558,7 +192,6 @@ static struct command_result *json_paystatus(struct command *cmd, const char *buf, const jsmntok_t *params) { - struct pay_status *ps; const char *invstring; struct json_stream *ret; struct payment *p; @@ -1572,38 +205,6 @@ static struct command_result *json_paystatus(struct command *cmd, ret = jsonrpc_stream_success(cmd); json_array_start(ret, "pay"); - /* FIXME: Index by bolt11 string! */ - /* TODO(cdecker) Remove once we migrated to `pay` with modifiers. */ - list_for_each(&pay_status, ps, list) { - if (invstring && !streq(invstring, ps->invstring)) - continue; - - json_object_start(ret, NULL); - json_add_invstring(ret, ps->invstring); - json_add_u64(ret, "msatoshi", - ps->msat.millisatoshis); /* Raw: JSON */ - json_add_string(ret, "amount_msat", - type_to_string(tmpctx, struct amount_msat, - &ps->msat)); - json_add_string(ret, "destination", ps->dest); - if (ps->label) - json_add_string(ret, "label", ps->label); - if (ps->routehint_modifications) - json_add_string(ret, "routehint_modifications", - ps->routehint_modifications); - if (ps->shadow && !streq(ps->shadow, "")) - json_add_string(ret, "shadow", ps->shadow); - if (ps->exclusions) - json_add_string(ret, "local_exclusions", ps->exclusions); - - /* If it's in listpeers right now, this can be 0 */ - json_array_start(ret, "attempts"); - for (size_t i = 0; i < tal_count(ps->attempts); i++) - add_attempt(ret, ps, &ps->attempts[i]); - json_array_end(ret); - json_object_end(ret); - } - list_for_each(&payments, p, list) { assert(p->parent == NULL); if (invstring && !streq(invstring, p->invstring)) @@ -1637,20 +238,11 @@ static struct command_result *json_paystatus(struct command *cmd, static bool attempt_ongoing(const struct sha256 *payment_hash) { - struct pay_status *ps; struct payment *root; - struct pay_attempt *attempt; struct payment_tree_result res; enum payment_step diff, final_states = PAYMENT_STEP_FAILED | PAYMENT_STEP_SUCCESS; - list_for_each(&pay_status, ps, list) { - if (!sha256_eq(payment_hash, &ps->payment_hash)) - continue; - attempt = &ps->attempts[tal_count(ps->attempts)-1]; - return attempt->result == NULL && attempt->failure == NULL; - } - list_for_each(&payments, root, list) { if (!sha256_eq(payment_hash, root->payment_hash)) continue; @@ -2323,9 +915,9 @@ static void destroy_payment(struct payment *p) list_del(&p->list); } -static struct command_result *json_paymod(struct command *cmd, - const char *buf, - const jsmntok_t *params) +static struct command_result *json_pay(struct command *cmd, + const char *buf, + const jsmntok_t *params) { struct payment *p; const char *b11str; @@ -2548,15 +1140,6 @@ static struct command_result *json_paymod(struct command *cmd, } static const struct plugin_command commands[] = { -#ifdef COMPAT_V090 - { - "legacypay", - "payment", - "Send payment specified by {bolt11} with {amount}", - "Try to send a payment, retrying {retry_for} seconds before giving up", - json_pay - }, -#endif { "paystatus", "payment", @@ -2575,7 +1158,7 @@ static const struct plugin_command commands[] = { "payment", "Send payment specified by {bolt11}", "Attempt to pay the {bolt11} invoice.", - json_paymod + json_pay }, }; diff --git a/tests/test_pay.py b/tests/test_pay.py index 985e8f1818d4..ac68c7b40ae2 100644 --- a/tests/test_pay.py +++ b/tests/test_pay.py @@ -7,7 +7,7 @@ from pyln.testing.utils import EXPERIMENTAL_DUAL_FUND, FUNDAMOUNT from utils import ( DEVELOPER, wait_for, only_one, sync_blockheight, TIMEOUT, - EXPERIMENTAL_FEATURES, env, VALGRIND, mine_funding_to_announce + EXPERIMENTAL_FEATURES, VALGRIND, mine_funding_to_announce ) import copy import os @@ -90,7 +90,7 @@ def test_pay_amounts(node_factory): @pytest.mark.developer("needs to deactivate shadow routing") -def test_pay_limits(node_factory, compat): +def test_pay_limits(node_factory): """Test that we enforce fee max percentage and max delay""" l1, l2, l3 = node_factory.line_graph(3, wait_for_announce=True) @@ -346,7 +346,7 @@ def test_pay_error_update_fees(node_factory): @pytest.mark.developer("needs to deactivate shadow routing") -def test_pay_optional_args(node_factory, compat): +def test_pay_optional_args(node_factory): l1, l2 = node_factory.line_graph(2) inv1 = l2.rpc.invoice(123000, 'test_pay', 'desc')['bolt11'] @@ -1779,7 +1779,7 @@ def listpays_nofail(b11): @pytest.mark.developer("needs DEVELOPER=1 otherwise gossip takes 5 minutes!") @pytest.mark.slow_test -def test_pay_routeboost(node_factory, bitcoind, compat): +def test_pay_routeboost(node_factory, bitcoind): """Make sure we can use routeboost information. """ # l1->l2->l3--private-->l4 l1, l2 = node_factory.line_graph(2, announce_channels=True, wait_for_announce=True) @@ -3448,7 +3448,7 @@ def test_sendpay_blinding(node_factory): l1.rpc.waitsendpay(inv['payment_hash']) -def test_excluded_adjacent_routehint(node_factory, bitcoind, compat): +def test_excluded_adjacent_routehint(node_factory, bitcoind): """Test case where we try have a routehint which leads to an adjacent node, but the result exceeds our maxfee; we crashed trying to find what part of the path was most expensive in that case @@ -3615,7 +3615,7 @@ def test_invalid_onion_channel_update(node_factory): @pytest.mark.developer("Requires use_shadow") -def test_pay_exemptfee(node_factory, compat): +def test_pay_exemptfee(node_factory): """Tiny payment, huge fee l1 -> l2 -> l3 @@ -3972,46 +3972,6 @@ def test_listpay_result_with_paymod(node_factory, bitcoind): assert 'destination' in l2.rpc.listpays()['pays'][0] -@unittest.skipIf(env('COMPAT') != 1, "legacypay requires COMPAT=1") -def test_listpays_ongoing_attempt(node_factory, bitcoind, executor): - """Test to reproduce issue #3915. - - The issue is that the bolt11 string is not initialized if the root payment - was split (no attempt with the bolt11 annotation ever hit `lightningd`, - hence we cannot filter by that. In addition keysends never have a bolt11 - string, so we need to switch to payment_hash comparisons anyway. - """ - plugin = os.path.join(os.path.dirname(__file__), 'plugins', 'hold_htlcs.py') - l1, l2, l3 = node_factory.line_graph(3, opts=[{}, {}, {'plugin': plugin}], - wait_for_announce=True) - - f = executor.submit(l1.rpc.keysend, l3.info['id'], 100) - l3.daemon.wait_for_log(r'Holding onto an incoming htlc') - l1.rpc.listpays() - f.result() - - inv = l2.rpc.invoice(10**6, 'legacy', 'desc')['bolt11'] - l1.rpc.legacypay(inv) - l1.rpc.listpays() - - # Produce loads of parts to increase probability of hitting the issue, - # should result in 100 splits at least - inv = l3.rpc.invoice(10**9, 'mpp invoice', 'desc')['bolt11'] - - # Start the payment, it'll get stuck for 10 seconds at l3 - executor.submit(l1.rpc.pay, inv) - l1.daemon.wait_for_log(r'Split into [0-9]+ sub-payments due to initial size') - l3.daemon.wait_for_log(r'Holding onto an incoming htlc') - - # While that is going on, check in with `listpays` to see if aggregation - # is working. - l1.rpc.listpays() - - # Now restart and see if we still can aggregate things correctly. - l1.restart() - l1.rpc.listpays() - - def test_listsendpays_and_listpays_order(node_factory): """listsendpays should be in increasing id order, listpays in created_at""" l1, l2 = node_factory.line_graph(2) From 335b0071a9867ef08eb50907dd2a4606345aad73 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Sat, 2 Apr 2022 12:49:05 +1030 Subject: [PATCH 3/8] doc: document `pay` localofferid field. Fixes: #4665 Reported-by: @shesek Signed-off-by: Rusty Russell --- contrib/pyln-client/pyln/client/lightning.py | 3 ++- doc/lightning-pay.7.md | 7 ++++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/contrib/pyln-client/pyln/client/lightning.py b/contrib/pyln-client/pyln/client/lightning.py index 3433eb10acfb..5b5d93298295 100644 --- a/contrib/pyln-client/pyln/client/lightning.py +++ b/contrib/pyln-client/pyln/client/lightning.py @@ -997,7 +997,7 @@ def newaddr(self, addresstype=None): def pay(self, bolt11, msatoshi=None, label=None, riskfactor=None, maxfeepercent=None, retry_for=None, - maxdelay=None, exemptfee=None, exclude=None): + maxdelay=None, exemptfee=None, localofferid=None, exclude=None): """ Send payment specified by {bolt11} with {msatoshi} (ignored if {bolt11} has an amount), optional {label} @@ -1012,6 +1012,7 @@ def pay(self, bolt11, msatoshi=None, label=None, riskfactor=None, "retry_for": retry_for, "maxdelay": maxdelay, "exemptfee": exemptfee, + "localofferid": localofferid, "exclude": exclude, } return self.call("pay", payload) diff --git a/doc/lightning-pay.7.md b/doc/lightning-pay.7.md index f563dddbedd9..2b52c6e54052 100644 --- a/doc/lightning-pay.7.md +++ b/doc/lightning-pay.7.md @@ -6,7 +6,7 @@ SYNOPSIS **pay** *bolt11* [*msatoshi*] [*label*] [*riskfactor*] [*maxfeepercent*] [*retry\_for*] [*maxdelay*] [*exemptfee*] -[*exclude*] +[*localofferid*] [*exclude*] DESCRIPTION ----------- @@ -32,6 +32,11 @@ leveraged by forwarding nodes. Setting `exemptfee` allows the `maxfeepercent` check to be skipped on fees that are smaller than `exemptfee` (default: 5000 millisatoshi). +`localofferid` is used by offers to link a payment attempt to a local +`send_invoice` offer created by lightningd-offerout(7). This ensures +that we only make a single payment for an offer, and that the offer is +marked `used` once paid. + The response will occur when the payment fails or succeeds. Once a payment has succeeded, calls to **pay** with the same *bolt11* will succeed immediately. From 90bce15e451f59df3404fd74e3eafac7554f3834 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Sat, 2 Apr 2022 12:55:04 +1030 Subject: [PATCH 4/8] pay: add absolute "maxfee" parameter. This is what LND does, and it's better for upper layers than trying to twist our maxfeepercent / exemptfee heuristics to suit. (I don't remember who complained about this, sorry!) I'm doing this now because I want to add YA parameter next! Signed-off-by: Rusty Russell Changelog-Added: JSON-RPC: `pay` has new parameter `maxfee` for setting absolute fee (instead of using `maxfeepercent` and/or `exemptfee`) --- contrib/pyln-client/pyln/client/lightning.py | 4 +- doc/lightning-pay.7.md | 10 ++++- doc/schemas/pay.request.json | 3 ++ plugins/pay.c | 45 ++++++++++++++------ tests/test_pay.py | 7 ++- tests/test_plugin.py | 3 +- 6 files changed, 54 insertions(+), 18 deletions(-) diff --git a/contrib/pyln-client/pyln/client/lightning.py b/contrib/pyln-client/pyln/client/lightning.py index 5b5d93298295..4996c81fb003 100644 --- a/contrib/pyln-client/pyln/client/lightning.py +++ b/contrib/pyln-client/pyln/client/lightning.py @@ -997,7 +997,8 @@ def newaddr(self, addresstype=None): def pay(self, bolt11, msatoshi=None, label=None, riskfactor=None, maxfeepercent=None, retry_for=None, - maxdelay=None, exemptfee=None, localofferid=None, exclude=None): + maxdelay=None, exemptfee=None, localofferid=None, exclude=None, + maxfee=None): """ Send payment specified by {bolt11} with {msatoshi} (ignored if {bolt11} has an amount), optional {label} @@ -1014,6 +1015,7 @@ def pay(self, bolt11, msatoshi=None, label=None, riskfactor=None, "exemptfee": exemptfee, "localofferid": localofferid, "exclude": exclude, + "maxfee": maxfee, } return self.call("pay", payload) diff --git a/doc/lightning-pay.7.md b/doc/lightning-pay.7.md index 2b52c6e54052..dd1ff3ad2fdf 100644 --- a/doc/lightning-pay.7.md +++ b/doc/lightning-pay.7.md @@ -5,8 +5,8 @@ SYNOPSIS -------- **pay** *bolt11* [*msatoshi*] [*label*] [*riskfactor*] -[*maxfeepercent*] [*retry\_for*] [*maxdelay*] [*exemptfee*] -[*localofferid*] [*exclude*] +[*maxfeepercent*] [*retry_for*] [*maxdelay*] [*exemptfee*] +[*localofferid*] [*exclude*] [*maxfee*] DESCRIPTION ----------- @@ -37,6 +37,12 @@ leveraged by forwarding nodes. Setting `exemptfee` allows the that we only make a single payment for an offer, and that the offer is marked `used` once paid. +*maxfee* overrides both *maxfeepercent* and *exemptfee* defaults (and +if you specify *maxfee* you cannot specify either of those), and +creates an absolute limit on what fee we will pay. This allows you to +implement your own heuristics rather than the primitive ones used +here. + The response will occur when the payment fails or succeeds. Once a payment has succeeded, calls to **pay** with the same *bolt11* will succeed immediately. diff --git a/doc/schemas/pay.request.json b/doc/schemas/pay.request.json index d7bcb5f85a15..b6dc27a1a1ee 100644 --- a/doc/schemas/pay.request.json +++ b/doc/schemas/pay.request.json @@ -45,6 +45,9 @@ } ] } + }, + "maxfee": { + "type": "msat" } } } diff --git a/plugins/pay.c b/plugins/pay.c index 23a9170c58c7..7bc0eb92cd30 100644 --- a/plugins/pay.c +++ b/plugins/pay.c @@ -925,7 +925,7 @@ static struct command_result *json_pay(struct command *cmd, char *b11_fail, *b12_fail; u64 *maxfee_pct_millionths; u32 *maxdelay; - struct amount_msat *exemptfee, *msat; + struct amount_msat *exemptfee, *msat, *maxfee; const char *label; unsigned int *retryfor; u64 *riskfactor_millionths; @@ -950,14 +950,15 @@ static struct command_result *json_pay(struct command *cmd, p_opt("label", param_string, &label), p_opt_def("riskfactor", param_millionths, &riskfactor_millionths, 10000000), - p_opt_def("maxfeepercent", param_millionths, - &maxfee_pct_millionths, 500000), + p_opt("maxfeepercent", param_millionths, + &maxfee_pct_millionths), p_opt_def("retry_for", param_number, &retryfor, 60), p_opt_def("maxdelay", param_number, &maxdelay, maxdelay_default), - p_opt_def("exemptfee", param_msat, &exemptfee, AMOUNT_MSAT(5000)), + p_opt("exemptfee", param_msat, &exemptfee), p_opt("localofferid", param_sha256, &local_offer_id), p_opt("exclude", param_route_exclusion_array, &exclusions), + p_opt("maxfee", param_msat, &maxfee), #if DEVELOPER p_opt_def("use_shadow", param_bool, &use_shadow, true), #endif @@ -1103,17 +1104,35 @@ static struct command_result *json_pay(struct command *cmd, p->getroute->riskfactorppm = *riskfactor_millionths; tal_free(riskfactor_millionths); - /* We free unneeded params as we use them, to keep memleak happy. */ - if (!amount_msat_fee(&p->constraints.fee_budget, p->amount, 0, - *maxfee_pct_millionths / 100)) { - return command_fail( - cmd, JSONRPC2_INVALID_PARAMS, - "Overflow when computing fee budget, fee rate too high."); + if (maxfee) { + if (maxfee_pct_millionths || exemptfee) { + return command_fail( + cmd, JSONRPC2_INVALID_PARAMS, + "If you specify maxfee, cannot specify maxfeepercent or exemptfee."); + } + p->constraints.fee_budget = *maxfee; + payment_mod_exemptfee_get_data(p)->amount = AMOUNT_MSAT(0); + } else { + u64 maxppm; + + if (maxfee_pct_millionths) + maxppm = *maxfee_pct_millionths / 100; + else + maxppm = 500000 / 100; + if (!amount_msat_fee(&p->constraints.fee_budget, p->amount, 0, + maxppm)) { + return command_fail( + cmd, JSONRPC2_INVALID_PARAMS, + "Overflow when computing fee budget, fee rate too high."); + } + payment_mod_exemptfee_get_data(p)->amount + = exemptfee ? *exemptfee : AMOUNT_MSAT(5000); + + /* We free unneeded params now to keep memleak happy. */ + tal_free(maxfee_pct_millionths); + tal_free(exemptfee); } - tal_free(maxfee_pct_millionths); - payment_mod_exemptfee_get_data(p)->amount = *exemptfee; - tal_free(exemptfee); shadow_route = payment_mod_shadowroute_get_data(p); payment_mod_presplit_get_data(p)->disable = disablempp; payment_mod_adaptive_splitter_get_data(p)->disable = disablempp; diff --git a/tests/test_pay.py b/tests/test_pay.py index ac68c7b40ae2..e0914887e362 100644 --- a/tests/test_pay.py +++ b/tests/test_pay.py @@ -128,10 +128,15 @@ def test_pay_limits(node_factory): assert(len(status) == 2) assert(status[0]['failure']['code'] == 205) + # This fails! + err = r'Fee exceeds our fee budget: 2msat > 1msat, discarding route' + with pytest.raises(RpcError, match=err) as err: + l1.rpc.pay(bolt11=inv['bolt11'], msatoshi=100000, maxfee=1) + # This works, because fee is less than exemptfee. l1.dev_pay(inv['bolt11'], msatoshi=100000, maxfeepercent=0.0001, exemptfee=2000, use_shadow=False) - status = l1.rpc.call('paystatus', {'bolt11': inv['bolt11']})['pay'][2]['attempts'] + status = l1.rpc.call('paystatus', {'bolt11': inv['bolt11']})['pay'][3]['attempts'] assert len(status) == 1 assert status[0]['strategy'] == "Initial attempt" diff --git a/tests/test_plugin.py b/tests/test_plugin.py index fecc7ac6e36d..01b90631d519 100644 --- a/tests/test_plugin.py +++ b/tests/test_plugin.py @@ -397,7 +397,8 @@ def test_pay_plugin(node_factory): # Make sure usage messages are present. msg = 'pay bolt11 [msatoshi] [label] [riskfactor] [maxfeepercent] '\ - '[retry_for] [maxdelay] [exemptfee] [localofferid] [exclude]' + '[retry_for] [maxdelay] [exemptfee] [localofferid] [exclude] '\ + '[maxfee]' if DEVELOPER: msg += ' [use_shadow]' assert only_one(l1.rpc.help('pay')['help'])['command'] == msg From f225b0710e6a11e27affef0fbe61388c5029a1cf Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Sat, 2 Apr 2022 13:03:05 +1030 Subject: [PATCH 5/8] pay: require description if bolt11 only has hash. Signed-off-by: Rusty Russell Changelog-Added: JSON-RPC: `pay` has `description` parameter, will be required if bolt11 only has a hash. Changelog-Deprecated: JSON-RPC: `pay` for a bolt11 which uses a `description_hash`, without setting `description`. --- contrib/pyln-client/pyln/client/lightning.py | 3 ++- doc/lightning-pay.7.md | 7 +++++- doc/schemas/pay.request.json | 3 +++ plugins/pay.c | 23 ++++++++++++++++++-- tests/test_invoices.py | 10 +++++++-- tests/test_plugin.py | 2 +- 6 files changed, 41 insertions(+), 7 deletions(-) diff --git a/contrib/pyln-client/pyln/client/lightning.py b/contrib/pyln-client/pyln/client/lightning.py index 4996c81fb003..c0c649db4b8e 100644 --- a/contrib/pyln-client/pyln/client/lightning.py +++ b/contrib/pyln-client/pyln/client/lightning.py @@ -998,7 +998,7 @@ def newaddr(self, addresstype=None): def pay(self, bolt11, msatoshi=None, label=None, riskfactor=None, maxfeepercent=None, retry_for=None, maxdelay=None, exemptfee=None, localofferid=None, exclude=None, - maxfee=None): + maxfee=None, description=None): """ Send payment specified by {bolt11} with {msatoshi} (ignored if {bolt11} has an amount), optional {label} @@ -1016,6 +1016,7 @@ def pay(self, bolt11, msatoshi=None, label=None, riskfactor=None, "localofferid": localofferid, "exclude": exclude, "maxfee": maxfee, + "description": description, } return self.call("pay", payload) diff --git a/doc/lightning-pay.7.md b/doc/lightning-pay.7.md index dd1ff3ad2fdf..223bfb086f75 100644 --- a/doc/lightning-pay.7.md +++ b/doc/lightning-pay.7.md @@ -6,7 +6,7 @@ SYNOPSIS **pay** *bolt11* [*msatoshi*] [*label*] [*riskfactor*] [*maxfeepercent*] [*retry_for*] [*maxdelay*] [*exemptfee*] -[*localofferid*] [*exclude*] [*maxfee*] +[*localofferid*] [*exclude*] [*maxfee*] [*description*] DESCRIPTION ----------- @@ -43,6 +43,11 @@ creates an absolute limit on what fee we will pay. This allows you to implement your own heuristics rather than the primitive ones used here. +*description* is only required for bolt11 invoices which do not +contain a description themselves, but contain a description hash. +*description* is then checked against the hash inside the invoice +before it will be paid. + The response will occur when the payment fails or succeeds. Once a payment has succeeded, calls to **pay** with the same *bolt11* will succeed immediately. diff --git a/doc/schemas/pay.request.json b/doc/schemas/pay.request.json index b6dc27a1a1ee..a02750911e46 100644 --- a/doc/schemas/pay.request.json +++ b/doc/schemas/pay.request.json @@ -48,6 +48,9 @@ }, "maxfee": { "type": "msat" + }, + "description": { + "type": "string" } } } diff --git a/plugins/pay.c b/plugins/pay.c index 7bc0eb92cd30..7cebac54c1ca 100644 --- a/plugins/pay.c +++ b/plugins/pay.c @@ -926,7 +926,7 @@ static struct command_result *json_pay(struct command *cmd, u64 *maxfee_pct_millionths; u32 *maxdelay; struct amount_msat *exemptfee, *msat, *maxfee; - const char *label; + const char *label, *description; unsigned int *retryfor; u64 *riskfactor_millionths; struct shadow_route_data *shadow_route; @@ -959,6 +959,7 @@ static struct command_result *json_pay(struct command *cmd, p_opt("localofferid", param_sha256, &local_offer_id), p_opt("exclude", param_route_exclusion_array, &exclusions), p_opt("maxfee", param_msat, &maxfee), + p_opt("description", param_string, &description), #if DEVELOPER p_opt_def("use_shadow", param_bool, &use_shadow, true), #endif @@ -971,7 +972,7 @@ static struct command_result *json_pay(struct command *cmd, if (!bolt12_has_prefix(b11str)) { b11 = bolt11_decode(tmpctx, b11str, plugin_feature_set(cmd->plugin), - NULL, chainparams, &b11_fail); + description, chainparams, &b11_fail); if (b11 == NULL) return command_fail(cmd, JSONRPC2_INVALID_PARAMS, "Invalid bolt11: %s", b11_fail); @@ -999,6 +1000,24 @@ static struct command_result *json_pay(struct command *cmd, cmd, JSONRPC2_INVALID_PARAMS, "Invalid bolt11:" " sets feature var_onion with no secret"); + + /* BOLT #11: + * A reader: + *... + * - MUST check that the SHA2 256-bit hash in the `h` field + * exactly matches the hashed description. + */ + if (!b11->description && !deprecated_apis) { + if (!b11->description_hash) { + return command_fail(cmd, + JSONRPC2_INVALID_PARAMS, + "Invalid bolt11: missing description"); + } + if (!description) + return command_fail(cmd, + JSONRPC2_INVALID_PARAMS, + "bolt11 uses description_hash, but you did not provide description parameter"); + } } else { b12 = invoice_decode(tmpctx, b11str, strlen(b11str), plugin_feature_set(cmd->plugin), diff --git a/tests/test_invoices.py b/tests/test_invoices.py index 1dec61081165..ba66a2efeecd 100644 --- a/tests/test_invoices.py +++ b/tests/test_invoices.py @@ -707,8 +707,14 @@ def test_invoice_deschash(node_factory, chainparams): listinv = only_one(l2.rpc.listinvoices()['invoices']) assert listinv['description'] == 'One piece of chocolate cake, one icecream cone, one pickle, one slice of swiss cheese, one slice of salami, one lollypop, one piece of cherry pie, one sausage, one cupcake, and one slice of watermelon' - # Make sure we can pay it! - l1.rpc.pay(inv['bolt11']) + # To pay it we need to provide the (correct!) description. + with pytest.raises(RpcError, match=r'you did not provide description parameter'): + l1.rpc.pay(inv['bolt11']) + + with pytest.raises(RpcError, match=r'does not match description'): + l1.rpc.pay(inv['bolt11'], description=listinv['description'][:-1]) + + l1.rpc.pay(inv['bolt11'], description=listinv['description']) # Try removing description. l2.rpc.delinvoice('label', "paid", desconly=True) diff --git a/tests/test_plugin.py b/tests/test_plugin.py index 01b90631d519..5cb9e8f81bcf 100644 --- a/tests/test_plugin.py +++ b/tests/test_plugin.py @@ -398,7 +398,7 @@ def test_pay_plugin(node_factory): # Make sure usage messages are present. msg = 'pay bolt11 [msatoshi] [label] [riskfactor] [maxfeepercent] '\ '[retry_for] [maxdelay] [exemptfee] [localofferid] [exclude] '\ - '[maxfee]' + '[maxfee] [description]' if DEVELOPER: msg += ' [use_shadow]' assert only_one(l1.rpc.help('pay')['help'])['command'] == msg From 2a4a412b5716d2d3097e0648175dfd1b38d19eae Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Sat, 2 Apr 2022 13:03:35 +1030 Subject: [PATCH 6/8] pay/sendpay: also store description in case bolt11 uses description_hash. Signed-off-by: Rusty Russell --- doc/lightning-listpays.7.md | 3 ++- doc/lightning-listsendpays.7.md | 3 ++- doc/lightning-sendpay.7.md | 3 ++- doc/schemas/listpays.schema.json | 7 +++++++ doc/schemas/listsendpays.schema.json | 7 +++++++ lightningd/pay.c | 21 ++++++++++++++++----- plugins/libplugin-pay.c | 6 ++++++ plugins/libplugin-pay.h | 3 +++ plugins/pay.c | 7 ++++++- tests/test_invoices.py | 10 ++++++++++ wallet/db.c | 2 ++ wallet/wallet.c | 18 ++++++++++++++++-- wallet/wallet.h | 3 +++ 13 files changed, 82 insertions(+), 11 deletions(-) diff --git a/doc/lightning-listpays.7.md b/doc/lightning-listpays.7.md index f4c2edf82bb9..8f098ec98273 100644 --- a/doc/lightning-listpays.7.md +++ b/doc/lightning-listpays.7.md @@ -24,6 +24,7 @@ On success, an object containing **pays** is returned. It is an array of object - **destination** (pubkey, optional): the final destination of the payment if known - **label** (string, optional): the label, if given to sendpay - **bolt11** (string, optional): the bolt11 string (if pay supplied one) +- **description** (string, optional): the description matching the bolt11 description hash (if pay supplied one) - **bolt12** (string, optional): the bolt12 string (if supplied for pay: **experimental-offers** only). If **status** is "pending" or "complete": @@ -56,4 +57,4 @@ RESOURCES Main web site: -[comment]: # ( SHA256STAMP:6ffbb1273de04f356cf79dab9a988ab030eee3317cb22e10d12d1c672249fc67) +[comment]: # ( SHA256STAMP:3c158259410ff8eb81669e26eca9ee53017002d739f89e7f0e2fd8e61edb8a14) diff --git a/doc/lightning-listsendpays.7.md b/doc/lightning-listsendpays.7.md index 13d82b8f45ae..c2511d09f583 100644 --- a/doc/lightning-listsendpays.7.md +++ b/doc/lightning-listsendpays.7.md @@ -34,6 +34,7 @@ On success, an object containing **payments** is returned. It is an array of ob - **destination** (pubkey, optional): the final destination of the payment if known - **label** (string, optional): the label, if given to sendpay - **bolt11** (string, optional): the bolt11 string (if pay supplied one) +- **description** (string, optional): the description matching the bolt11 description hash (if pay supplied one) - **bolt12** (string, optional): the bolt12 string (if supplied for pay: **experimental-offers** only). If **status** is "complete": @@ -60,4 +61,4 @@ RESOURCES Main web site: -[comment]: # ( SHA256STAMP:b03c2f306bafb1919f0933ebc695657bd691591484ddcb39b1e8706335593cd2) +[comment]: # ( SHA256STAMP:eaa0b4c6309d45bc2a72baf44288f1faa75d7f6ff2e8bf6d03be53747fe82c84) diff --git a/doc/lightning-sendpay.7.md b/doc/lightning-sendpay.7.md index 6ecb1806255e..f332e812f269 100644 --- a/doc/lightning-sendpay.7.md +++ b/doc/lightning-sendpay.7.md @@ -5,7 +5,8 @@ SYNOPSIS -------- **sendpay** *route* *payment\_hash* [*label*] [*msatoshi*] -[*bolt11*] [*payment_secret*] [*partid*] [*localofferid*] [*groupid*] [*payment_metadata*] +[*bolt11*] [*payment_secret*] [*partid*] [*localofferid*] [*groupid*] +[*payment_metadata*] [*description*] DESCRIPTION ----------- diff --git a/doc/schemas/listpays.schema.json b/doc/schemas/listpays.schema.json index f10062ea2180..ad74829abaa6 100644 --- a/doc/schemas/listpays.schema.json +++ b/doc/schemas/listpays.schema.json @@ -48,6 +48,10 @@ "type": "string", "description": "the bolt11 string (if pay supplied one)" }, + "description": { + "type": "string", + "description": "the description matching the bolt11 description hash (if pay supplied one)" + }, "bolt12": { "type": "string", "description": "the bolt12 string (if supplied for pay: **experimental-offers** only)." @@ -78,6 +82,7 @@ "created_at": {}, "label": {}, "bolt11": {}, + "description": {}, "bolt12": {}, "preimage": {}, "number_of_parts": {}, @@ -115,6 +120,7 @@ "created_at": {}, "label": {}, "bolt11": {}, + "description": {}, "bolt12": {}, "amount_msat": {}, "amount_sent_msat": {}, @@ -152,6 +158,7 @@ "created_at": {}, "label": {}, "bolt11": {}, + "description": {}, "bolt12": {}, "amount_sent_msat": {}, "erroronion": { diff --git a/doc/schemas/listsendpays.schema.json b/doc/schemas/listsendpays.schema.json index 8db7f553fe5b..3a45a3b7de81 100644 --- a/doc/schemas/listsendpays.schema.json +++ b/doc/schemas/listsendpays.schema.json @@ -72,6 +72,10 @@ "type": "string", "description": "the bolt11 string (if pay supplied one)" }, + "description": { + "type": "string", + "description": "the description matching the bolt11 description hash (if pay supplied one)" + }, "bolt12": { "type": "string", "description": "the bolt12 string (if supplied for pay: **experimental-offers** only)." @@ -108,6 +112,7 @@ "amount_sent_msat": {}, "label": {}, "bolt11": {}, + "description": {}, "bolt12": {}, "payment_preimage": { "type": "secret", @@ -146,6 +151,7 @@ "amount_sent_msat": {}, "label": {}, "bolt11": {}, + "description": {}, "bolt12": {}, "erroronion": { "type": "hex", @@ -182,6 +188,7 @@ "amount_sent_msat": {}, "label": {}, "bolt11": {}, + "description": {}, "bolt12": {} } } diff --git a/lightningd/pay.c b/lightningd/pay.c index 9e29f92d09ed..0f0745786eb0 100644 --- a/lightningd/pay.c +++ b/lightningd/pay.c @@ -154,6 +154,8 @@ void json_add_payment_fields(struct json_stream *response, else json_add_string(response, "bolt11", t->invstring); } + if (t->description) + json_add_string(response, "description", t->description); if (t->failonion) json_add_hex(response, "erroronion", t->failonion, @@ -857,6 +859,7 @@ send_payment_core(struct lightningd *ld, struct amount_msat total_msat, const char *label TAKES, const char *invstring TAKES, + const char *description TAKES, const struct onionpacket *packet, const struct node_id *destination, struct node_id *route_nodes TAKES, @@ -1100,6 +1103,10 @@ send_payment_core(struct lightningd *ld, payment->invstring = tal_strdup(payment, invstring); else payment->invstring = NULL; + if (description != NULL) + payment->description = tal_strdup(payment, description); + else + payment->description = NULL; payment->local_offer_id = tal_dup_or_null(payment, struct sha256, local_offer_id); @@ -1121,6 +1128,7 @@ send_payment(struct lightningd *ld, struct amount_msat total_msat, const char *label TAKES, const char *invstring TAKES, + const char *description TAKES, const struct sha256 *local_offer_id, const struct secret *payment_secret, const u8 *payment_metadata) @@ -1194,7 +1202,7 @@ send_payment(struct lightningd *ld, packet = create_onionpacket(tmpctx, path, ROUTING_INFO_SIZE, &path_secrets); return send_payment_core(ld, cmd, rhash, partid, group, &route[0], msat, total_msat, - label, invstring, + label, invstring, description, packet, &ids[n_hops - 1], ids, channels, path_secrets, local_offer_id); } @@ -1265,7 +1273,7 @@ static struct command_result *json_sendonion(struct command *cmd, struct route_hop *first_hop; struct sha256 *payment_hash; struct lightningd *ld = cmd->ld; - const char *label, *invstring; + const char *label, *invstring, *description; struct node_id *destination; struct secret *path_secrets; struct amount_msat *msat; @@ -1285,6 +1293,7 @@ static struct command_result *json_sendonion(struct command *cmd, p_opt("destination", param_node_id, &destination), p_opt("localofferid", param_sha256, &local_offer_id), p_opt("groupid", param_u64, &group), + p_opt("description", param_string, &description), NULL)) return command_param_failed(); @@ -1306,7 +1315,8 @@ static struct command_result *json_sendonion(struct command *cmd, return send_payment_core(ld, cmd, payment_hash, *partid, *group, first_hop, *msat, AMOUNT_MSAT(0), - label, invstring, packet, destination, NULL, NULL, + label, invstring, description, + packet, destination, NULL, NULL, path_secrets, local_offer_id); } @@ -1420,7 +1430,7 @@ static struct command_result *json_sendpay(struct command *cmd, struct sha256 *rhash; struct route_hop *route; struct amount_msat *msat; - const char *invstring, *label; + const char *invstring, *label, *description; u64 *partid, *group; struct secret *payment_secret; struct sha256 *local_offer_id; @@ -1439,6 +1449,7 @@ static struct command_result *json_sendpay(struct command *cmd, p_opt("localofferid", param_sha256, &local_offer_id), p_opt("groupid", param_u64, &group), p_opt("payment_metadata", param_bin_from_hex, &payment_metadata), + p_opt("description", param_string, &description), NULL)) return command_param_failed(); @@ -1488,7 +1499,7 @@ static struct command_result *json_sendpay(struct command *cmd, route, final_amount, msat ? *msat : final_amount, - label, invstring, local_offer_id, + label, invstring, description, local_offer_id, payment_secret, payment_metadata); } diff --git a/plugins/libplugin-pay.c b/plugins/libplugin-pay.c index 1c8779f411bf..b206ce759029 100644 --- a/plugins/libplugin-pay.c +++ b/plugins/libplugin-pay.c @@ -65,6 +65,7 @@ struct payment *payment_new(tal_t *ctx, struct command *cmd, p->temp_exclusion = NULL; p->failroute_retry = false; p->invstring = NULL; + p->description = NULL; p->routetxt = NULL; p->max_htlcs = UINT32_MAX; p->aborterror = NULL; @@ -1568,6 +1569,9 @@ static struct command_result *payment_createonion_success(struct command *cmd, /* FIXME: rename parameter to invstring */ json_add_string(req->js, "bolt11", p->invstring); + if (p->description) + json_add_string(req->js, "description", p->description); + if (p->destination) json_add_node_id(req->js, "destination", p->destination); @@ -3559,6 +3563,8 @@ static void presplit_cb(struct presplit_mod_data *d, struct payment *p) * they'll be used when aggregating the payments * again. */ c->invstring = tal_strdup(c, p->invstring); + if (p->description) + c->description = tal_strdup(c, p->description); /* Get ~ target, but don't exceed amt */ c->amount = fuzzed_near(target, amt); diff --git a/plugins/libplugin-pay.h b/plugins/libplugin-pay.h index 86aea238c464..220972547daa 100644 --- a/plugins/libplugin-pay.h +++ b/plugins/libplugin-pay.h @@ -259,6 +259,9 @@ struct payment { * by the invoice. */ const char *invstring; + /* Description, usually set if bolt11 has only description_hash */ + const char *description; + /* If this is paying a local offer, this is the one (sendpay ensures we * don't pay twice for single-use offers) */ struct sha256 *local_offer_id; diff --git a/plugins/pay.c b/plugins/pay.c index 7cebac54c1ca..610d369c150a 100644 --- a/plugins/pay.c +++ b/plugins/pay.c @@ -273,6 +273,8 @@ struct pay_mpp { /* Optional label (of first one!) */ const jsmntok_t *label; + /* Optional description (used for bolt11 with description_hash) */ + const jsmntok_t *description; /* Optional preimage (iff status is successful) */ const jsmntok_t *preimage; /* Only counts "complete" or "pending" payments. */ @@ -373,7 +375,8 @@ static void add_new_entry(struct json_stream *ret, json_object_start(ret, NULL); if (pm->invstring) json_add_invstring(ret, pm->invstring); - + if (pm->description) + json_add_tok(ret, "description", pm->description, buf); if (pm->destination) json_add_tok(ret, "destination", pm->destination, buf); @@ -465,6 +468,7 @@ static struct command_result *listsendpays_done(struct command *cmd, pm->invstring = tal_steal(pm, invstr); pm->destination = json_get_member(buf, t, "destination"); pm->label = json_get_member(buf, t, "label"); + pm->description = json_get_member(buf, t, "description"); pm->preimage = NULL; pm->amount_sent = AMOUNT_MSAT(0); pm->amount = talz(pm, struct amount_msat); @@ -968,6 +972,7 @@ static struct command_result *json_pay(struct command *cmd, p = payment_new(cmd, cmd, NULL /* No parent */, paymod_mods); p->invstring = tal_steal(p, b11str); + p->description = tal_steal(p, description); if (!bolt12_has_prefix(b11str)) { b11 = diff --git a/tests/test_invoices.py b/tests/test_invoices.py index ba66a2efeecd..8f4aa6b5c637 100644 --- a/tests/test_invoices.py +++ b/tests/test_invoices.py @@ -716,6 +716,16 @@ def test_invoice_deschash(node_factory, chainparams): l1.rpc.pay(inv['bolt11'], description=listinv['description']) + # Description will be in some. + found = False + for p in l1.rpc.listsendpays()['payments']: + if 'description' in p: + found = True + assert p['description'] == listinv['description'] + assert found + + assert only_one(l1.rpc.listpays(inv['bolt11'])['pays'])['description'] == listinv['description'] + # Try removing description. l2.rpc.delinvoice('label', "paid", desconly=True) assert 'description' not in only_one(l2.rpc.listinvoices()['invoices']) diff --git a/wallet/db.c b/wallet/db.c index 0d086fea0a53..04497400dcdb 100644 --- a/wallet/db.c +++ b/wallet/db.c @@ -873,6 +873,8 @@ static struct migration dbmigrations[] = { {SQL("ALTER TABLE channels ADD htlc_maximum_msat BIGINT DEFAULT 2100000000000000"), NULL}, {SQL("ALTER TABLE channels ADD htlc_minimum_msat BIGINT DEFAULT 0"), NULL}, {SQL("ALTER TABLE forwarded_payments ADD forward_style INTEGER DEFAULT NULL"), NULL}, + /* "description" is used for label, so we use "paydescription" here */ + {SQL("ALTER TABLE payments ADD paydescription TEXT;"), NULL}, }; /** diff --git a/wallet/wallet.c b/wallet/wallet.c index bf0710b0f352..da3fa96d0148 100644 --- a/wallet/wallet.c +++ b/wallet/wallet.c @@ -3033,8 +3033,9 @@ void wallet_payment_store(struct wallet *wallet, " total_msat," " partid," " local_offer_id," - " groupid" - ") VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?);")); + " groupid," + " paydescription" + ") VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?);")); db_bind_int(stmt, 0, payment->status); db_bind_sha256(stmt, 1, &payment->payment_hash); @@ -3083,6 +3084,11 @@ void wallet_payment_store(struct wallet *wallet, db_bind_u64(stmt, 14, payment->groupid); + if (payment->description != NULL) + db_bind_text(stmt, 15, payment->description); + else + db_bind_null(stmt, 15); + db_exec_prepared_v2(stmt); payment->id = db_last_insert_id_v2(stmt); assert(payment->id > 0); @@ -3177,6 +3183,11 @@ static struct wallet_payment *wallet_stmt2payment(const tal_t *ctx, else payment->label = NULL; + if (!db_col_is_null(stmt, "paydescription")) + payment->description = db_col_strdup(payment, stmt, "paydescription"); + else + payment->description = NULL; + if (!db_col_is_null(stmt, "bolt11")) payment->invstring = db_col_strdup(payment, stmt, "bolt11"); else @@ -3236,6 +3247,7 @@ wallet_payment_by_hash(const tal_t *ctx, struct wallet *wallet, ", msatoshi_sent" ", description" ", bolt11" + ", paydescription" ", failonionreply" ", total_msat" ", partid" @@ -3472,6 +3484,7 @@ wallet_payment_list(const tal_t *ctx, ", msatoshi_sent" ", description" ", bolt11" + ", paydescription" ", failonionreply" ", total_msat" ", partid" @@ -3538,6 +3551,7 @@ wallet_payments_by_offer(const tal_t *ctx, ", msatoshi_sent" ", description" ", bolt11" + ", paydescription" ", failonionreply" ", total_msat" ", partid" diff --git a/wallet/wallet.h b/wallet/wallet.h index 2c179b465539..4003adc749a5 100644 --- a/wallet/wallet.h +++ b/wallet/wallet.h @@ -358,6 +358,9 @@ struct wallet_payment { /* The label of the payment. Must support `tal_len` */ const char *label; + /* The description of the payment (used if invstring has hash). */ + const char *description; + /* If we could not decode the fail onion, just add it here. */ const u8 *failonion; From b2d5ef28c5f46636bcad99c827eadefa7e75fa91 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Mon, 4 Apr 2022 12:26:52 +0930 Subject: [PATCH 7/8] plugins/pay: always include `bolt11` (and `description`) in listpays. We were setting it on the root, but that doesn't get handed to sendpay. Our schema doesn't *require* bolt11, either, so this was missed (there could be a *bolt12* instead). Signed-off-by: Rusty Russell Changelog-Fixed: JSON-RPC: `listpays` always includes `bolt11` or `bolt12` field. --- plugins/keysend.c | 2 ++ plugins/libplugin-pay.c | 23 +++++++++++------------ plugins/libplugin-pay.h | 4 ++++ plugins/pay.c | 7 +++++++ tests/test_pay.py | 3 +++ 5 files changed, 27 insertions(+), 12 deletions(-) diff --git a/plugins/keysend.c b/plugins/keysend.c index 22ae85e3b77e..47f9440c16c9 100644 --- a/plugins/keysend.c +++ b/plugins/keysend.c @@ -186,6 +186,8 @@ static struct command_result *json_keysend(struct command *cmd, const char *buf, p->min_final_cltv_expiry = 22; p->features = NULL; p->invstring = NULL; + /* Don't try to use invstring to hand to sendonion! */ + p->invstring_used = true; p->why = "Initial attempt"; p->constraints.cltv_budget = *maxdelay; p->deadline = timeabs_add(time_now(), time_from_sec(*retryfor)); diff --git a/plugins/libplugin-pay.c b/plugins/libplugin-pay.c index b206ce759029..e415cf99d68f 100644 --- a/plugins/libplugin-pay.c +++ b/plugins/libplugin-pay.c @@ -61,11 +61,10 @@ struct payment *payment_new(tal_t *ctx, struct command *cmd, p->failreason = NULL; p->getroute->riskfactorppm = 10000000; p->abort = false; + p->invstring_used = false; p->route = NULL; p->temp_exclusion = NULL; p->failroute_retry = false; - p->invstring = NULL; - p->description = NULL; p->routetxt = NULL; p->max_htlcs = UINT32_MAX; p->aborterror = NULL; @@ -94,6 +93,8 @@ struct payment *payment_new(tal_t *ctx, struct command *cmd, p->local_id = parent->local_id; p->local_offer_id = parent->local_offer_id; p->groupid = parent->groupid; + p->invstring = parent->invstring; + p->description = parent->description; } else { assert(cmd != NULL); p->partid = 0; @@ -102,6 +103,7 @@ struct payment *payment_new(tal_t *ctx, struct command *cmd, p->channel_hints = tal_arr(p, struct channel_hint, 0); p->excluded_nodes = tal_arr(p, struct node_id, 0); p->id = next_id++; + p->description = NULL; /* Caller must set this. */ p->local_id = NULL; p->local_offer_id = NULL; @@ -1536,6 +1538,7 @@ static struct command_result *payment_createonion_success(struct command *cmd, struct out_req *req; struct route_hop *first = &p->route[0]; struct secret *secrets; + struct payment *root = payment_root(p); p->createonion_response = json_to_createonion_response(p, buffer, toks); @@ -1565,12 +1568,15 @@ static struct command_result *payment_createonion_success(struct command *cmd, if (p->label) json_add_string(req->js, "label", p->label); - if (p->invstring) + if (!root->invstring_used) { /* FIXME: rename parameter to invstring */ json_add_string(req->js, "bolt11", p->invstring); - if (p->description) - json_add_string(req->js, "description", p->description); + if (p->description) + json_add_string(req->js, "description", p->description); + + root->invstring_used = true; + } if (p->destination) json_add_node_id(req->js, "destination", p->destination); @@ -3559,13 +3565,6 @@ static void presplit_cb(struct presplit_mod_data *d, struct payment *p) struct payment *c = payment_new(p, NULL, p, p->modifiers); - /* Annotate the subpayments with the bolt11 string, - * they'll be used when aggregating the payments - * again. */ - c->invstring = tal_strdup(c, p->invstring); - if (p->description) - c->description = tal_strdup(c, p->description); - /* Get ~ target, but don't exceed amt */ c->amount = fuzzed_near(target, amt); diff --git a/plugins/libplugin-pay.h b/plugins/libplugin-pay.h index 220972547daa..eb125acef148 100644 --- a/plugins/libplugin-pay.h +++ b/plugins/libplugin-pay.h @@ -255,6 +255,10 @@ struct payment { * true. Set only on the root payment. */ bool abort; + /* We only set invstring/description on one of our sendpays per group, + * so we track when we've done that. */ + bool invstring_used; + /* Serialized bolt11/12 string, kept attachd to the root so we can filter * by the invoice. */ const char *invstring; diff --git a/plugins/pay.c b/plugins/pay.c index 610d369c150a..51b632a03660 100644 --- a/plugins/pay.c +++ b/plugins/pay.c @@ -480,6 +480,13 @@ static struct command_result *listsendpays_done(struct command *cmd, // First time we see the groupid we add it to the order // array, so we can retrieve them in the correct order. tal_arr_expand(&order, pm->sortkey); + } else { + /* Not all payments have bolt11/bolt12 or + * description, as an optimization */ + if (!pm->invstring) + pm->invstring = tal_steal(pm, invstr); + if (!pm->description) + pm->description = json_get_member(buf, t, "description"); } status = json_get_member(buf, t, "status"); diff --git a/tests/test_pay.py b/tests/test_pay.py index e0914887e362..67253820560a 100644 --- a/tests/test_pay.py +++ b/tests/test_pay.py @@ -3799,6 +3799,9 @@ def all_htlcs(n): pprint(p) pprint(l1.rpc.paystatus(inv)) + # listpays() shows bolt11 string + assert 'bolt11' in only_one(l1.rpc.listpays()['pays']) + def test_pay_fail_unconfirmed_channel(node_factory, bitcoind): ''' From 9d71db264e16cefcb0af6e81958789420be5df8b Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Mon, 4 Apr 2022 12:26:54 +0930 Subject: [PATCH 8/8] pytest: test that deduplication for `bolt11` works as expected. Signed-off-by: Rusty Russell --- tests/test_pay.py | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/tests/test_pay.py b/tests/test_pay.py index 67253820560a..6f41ad72e71e 100644 --- a/tests/test_pay.py +++ b/tests/test_pay.py @@ -3730,6 +3730,24 @@ def test_mpp_presplit(node_factory): assert(inv['msatoshi'] == inv['msatoshi_received']) + # Make sure that bolt11 isn't duplicated for every part + bolt11s = 0 + count = 0 + for p in l1.rpc.listsendpays()['payments']: + if 'bolt11' in p: + bolt11s += 1 + count += 1 + + # You were supposed to mpp! + assert count > 1 + # Not every one should have the bolt11 string + assert bolt11s < count + # In fact, only one should + assert bolt11s == 1 + + # But listpays() gathers it: + assert only_one(l1.rpc.listpays()['pays'])['bolt11'] == inv['bolt11'] + def test_mpp_adaptive(node_factory, bitcoind): """We have two paths, both too small on their own, let's combine them. @@ -3799,6 +3817,19 @@ def all_htlcs(n): pprint(p) pprint(l1.rpc.paystatus(inv)) + # Make sure that bolt11 isn't duplicated for every part + bolt11s = 0 + count = 0 + for p in l1.rpc.listsendpays()['payments']: + if 'bolt11' in p: + bolt11s += 1 + count += 1 + + # You were supposed to mpp! + assert count > 1 + # Not every one should have the bolt11 string + assert bolt11s < count + # listpays() shows bolt11 string assert 'bolt11' in only_one(l1.rpc.listpays()['pays'])