From d9fa8a3536873499fd789b0067ed6ca9258555b4 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 15 Jan 2019 14:46:27 +1030 Subject: [PATCH] plugins/pay: retry on failure in a loop. We use the 'exclude' option to getroute for successive attempts. This is more robust than having gossipd disable for some limited time. Signed-off-by: Rusty Russell --- plugins/pay.c | 141 +++++++++++++++++++++++++++++++++++++++++---- tests/test_misc.py | 7 +-- 2 files changed, 134 insertions(+), 14 deletions(-) diff --git a/plugins/pay.c b/plugins/pay.c index 28691c9a8..da0cb335a 100644 --- a/plugins/pay.c +++ b/plugins/pay.c @@ -1,25 +1,114 @@ #include #include #include +#include #include #include #include +struct pay_attempt { + const char *route; + const char *failure; +}; + struct pay_command { + /* Destination, as text */ + const char *dest; + + /* How much we're paying, and what riskfactor for routing. */ + u64 msatoshi; + double riskfactor; + /* Payment hash, as text. */ const char *payment_hash; /* Description, if any. */ const char *desc; + + /* Chatty description of attempts. */ + struct pay_attempt *attempts; + + /* Time to stop retrying. */ + struct timeabs stoptime; + + /* Channels which have failed us. */ + const char **excludes; }; +static struct command_result *start_pay_attempt(struct command *cmd, + struct pay_command *pc); + +static struct command_result *waitsendpay_expired(struct command *cmd, + struct pay_command *pc) +{ + char *errmsg, *data; + + errmsg = tal_fmt(pc, "Gave up after %zu attempts", + tal_count(pc->attempts)); + data = tal_strdup(pc, "'attempts': [ "); + for (size_t i = 0; i < tal_count(pc->attempts); i++) + tal_append_fmt(&data, "%s { 'route': %s,\n 'failure': '%s'\n }", + i == 0 ? "" : ",", + pc->attempts[i].route, + pc->attempts[i].failure); + tal_append_fmt(&data, "]"); + return command_done_err(cmd, PAY_STOPPED_RETRYING, errmsg, data); +} + +static struct command_result *waitsendpay_error(struct command *cmd, + const char *buf, + const jsmntok_t *error, + struct pay_command *pc) +{ + struct pay_attempt *attempt; + const jsmntok_t *codetok, *scidtok, *dirtok; + int code; + + codetok = json_get_member(buf, error, "code"); + if (!json_to_int(buf, codetok, &code)) + plugin_err("waitsendpay error gave no 'code'? '%.*s'", + error->end - error->start, buf + error->start); + + /* FIXME: Handle PAY_UNPARSEABLE_ONION! */ + + /* Many error codes are final. */ + if (code != PAY_TRY_OTHER_ROUTE) { + return forward_error(cmd, buf, error, pc); + } + + scidtok = json_delve(buf, error, ".data.erring_channel"); + if (!scidtok) + plugin_err("waitsendpay error no erring_channel '%.*s'", + error->end - error->start, buf + error->start); + dirtok = json_delve(buf, error, ".data.erring_direction"); + if (!dirtok) + plugin_err("waitsendpay error no erring_direction '%.*s'", + error->end - error->start, buf + error->start); + + /* Add erring channel to exclusion list. */ + tal_arr_expand(&pc->excludes, tal_fmt(pc->excludes, "%.*s/%c", + scidtok->end - scidtok->start, + buf + scidtok->start, + buf[dirtok->start])); + + attempt = &pc->attempts[tal_count(pc->attempts)-1]; + attempt->failure = json_strdup(pc->attempts, buf, error); + + if (time_after(time_now(), pc->stoptime)) { + return waitsendpay_expired(cmd, pc); + } + + /* Try again. */ + return start_pay_attempt(cmd, pc); +} + static struct command_result *sendpay_done(struct command *cmd, const char *buf, const jsmntok_t *result, struct pay_command *pc) { return send_outreq(cmd, "waitsendpay", - forward_result, forward_error, pc, + forward_result, waitsendpay_error, pc, "'payment_hash': '%s', 'timeout': 60", pc->payment_hash); } @@ -29,12 +118,16 @@ static struct command_result *getroute_done(struct command *cmd, const jsmntok_t *result, struct pay_command *pc) { + struct pay_attempt attempt; const jsmntok_t *t = json_get_member(buf, result, "route"); char *json_desc; if (!t) plugin_err("getroute gave no 'route'? '%.*s'", result->end - result->start, buf); + attempt.route = json_strdup(pc->attempts, buf, result); + tal_arr_expand(&pc->attempts, attempt); + if (pc->desc) json_desc = tal_fmt(pc, ", 'description': '%s'", pc->desc); else @@ -47,6 +140,30 @@ static struct command_result *getroute_done(struct command *cmd, json_desc); } +static struct command_result *start_pay_attempt(struct command *cmd, + struct pay_command *pc) +{ + char *exclude; + + if (tal_count(pc->excludes) != 0) { + exclude = tal_strdup(tmpctx, ",'exclude': ["); + for (size_t i = 0; i < tal_count(pc->excludes); i++) + /* JSON.org grammar doesn't allow trailing , */ + tal_append_fmt(&exclude, "%s %s", + i == 0 ? "" : ",", + pc->excludes[i]); + tal_append_fmt(&exclude, "]"); + } else + exclude = ""; + + /* OK, ask for route to destination */ + return send_outreq(cmd, "getroute", getroute_done, forward_error, pc, + "'id': '%s'," + "'msatoshi': %"PRIu64"," + "'riskfactor': %f%s", + pc->dest, pc->msatoshi, pc->riskfactor, exclude); +} + static struct command_result *handle_pay(struct command *cmd, const char *buf, const jsmntok_t *params) @@ -56,11 +173,11 @@ static struct command_result *handle_pay(struct command *cmd, const char *b11str; char *fail; double *riskfactor; + unsigned int *retryfor; struct pay_command *pc = tal(cmd, struct pay_command); /* FIXME! */ double *maxfeepercent; - unsigned int *retryfor; unsigned int *maxdelay; unsigned int *exemptfee; @@ -86,29 +203,33 @@ static struct command_result *handle_pay(struct command *cmd, "Invalid bolt11: %s", fail); } + if (time_now().ts.tv_sec > b11->timestamp + b11->expiry) { + return command_fail(cmd, PAY_INVOICE_EXPIRED, "Invoice expired"); + } + if (b11->msatoshi) { if (msatoshi) { return command_fail(cmd, JSONRPC2_INVALID_PARAMS, "msatoshi parameter unnecessary"); } + pc->msatoshi = *b11->msatoshi; } else { if (!msatoshi) { return command_fail(cmd, JSONRPC2_INVALID_PARAMS, "msatoshi parameter required"); } - b11->msatoshi = tal_steal(b11, msatoshi); + pc->msatoshi = *msatoshi; } + pc->riskfactor = *riskfactor; + pc->dest = type_to_string(cmd, struct pubkey, &b11->receiver_id); pc->payment_hash = type_to_string(pc, struct sha256, &b11->payment_hash); + pc->stoptime = timeabs_add(time_now(), time_from_sec(*retryfor)); + pc->attempts = tal_arr(cmd, struct pay_attempt, 0); + pc->excludes = tal_arr(cmd, const char *, 0); - /* OK, ask for route to destination */ - return send_outreq(cmd, "getroute", getroute_done, forward_error, pc, - "'id': '%s'," - "'msatoshi': %"PRIu64"," - "'riskfactor': %f", - type_to_string(tmpctx, struct pubkey, &b11->receiver_id), - *b11->msatoshi, *riskfactor); + return start_pay_attempt(cmd, pc); } static const struct plugin_command commands[] = { { diff --git a/tests/test_misc.py b/tests/test_misc.py index 747b82b50..30536307f 100644 --- a/tests/test_misc.py +++ b/tests/test_misc.py @@ -905,12 +905,11 @@ def test_htlc_send_timeout(node_factory, bitcoind): with pytest.raises(RpcError) as excinfo: l1.rpc.pay(inv['bolt11']) - # FIXME: this error should be correct - # err = excinfo.value + err = excinfo.value # Complaints it couldn't find route. # FIXME: include in pylightning - # PAY_ROUTE_NOT_FOUND = 205 - # assert err.error['code'] == PAY_ROUTE_NOT_FOUND + PAY_ROUTE_NOT_FOUND = 205 + assert err.error['code'] == PAY_ROUTE_NOT_FOUND # FIXME: get payment status. # Temporary channel failure