From be64dd84cad7ce8475ca2fa98528792e63dce40b Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Tue, 15 Jan 2019 14:32:27 +1030 Subject: [PATCH] waitsendpay: indicate which channel direction the error was. You can figure this yourself by knowing the route, but it's better to report it directly here. Signed-off-by: Rusty Russell --- CHANGELOG.md | 1 + bitcoin/pubkey.h | 6 ++++++ gossipd/routing.h | 6 ------ lightningd/pay.c | 34 +++++++++++++++++++++++++++++----- lightningd/pay.h | 1 + wallet/db.c | 2 ++ wallet/wallet.c | 19 ++++++++++++++----- wallet/wallet.h | 9 ++++++--- 8 files changed, 59 insertions(+), 19 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 41027b66d..d1f600084 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0. - plugins: fully enabled, and ready for you to write some! - lightning-cli: `help ` finds man pages even if `make install` not run. +- JSON API: `waitsendpay` now has an `erring_channel_direction` field. ### Changed diff --git a/bitcoin/pubkey.h b/bitcoin/pubkey.h index c433a6559..18a951e77 100644 --- a/bitcoin/pubkey.h +++ b/bitcoin/pubkey.h @@ -45,6 +45,12 @@ void pubkey_to_der(u8 der[PUBKEY_DER_LEN], const struct pubkey *key); /* Compare the keys `a` and `b`. Return <0 if `a`<`b`, 0 if equal and >0 otherwise */ int pubkey_cmp(const struct pubkey *a, const struct pubkey *b); +/* If the two nodes[] are id1 and id2, which index would id1 be? */ +static inline int pubkey_idx(const struct pubkey *id1, const struct pubkey *id2) +{ + return pubkey_cmp(id1, id2) > 0; +} + /** * pubkey_to_hash160 - Get the hash for p2pkh payments for a given pubkey */ diff --git a/gossipd/routing.h b/gossipd/routing.h index 3c2c16c92..71bc565e2 100644 --- a/gossipd/routing.h +++ b/gossipd/routing.h @@ -136,12 +136,6 @@ HTABLE_DEFINE_TYPE(struct node, node_map_keyof_node, node_map_hash_key, node_map struct pending_node_map; struct pending_cannouncement; -/* If the two nodes[] are id1 and id2, which index would id1 be? */ -static inline int pubkey_idx(const struct pubkey *id1, const struct pubkey *id2) -{ - return pubkey_cmp(id1, id2) > 0; -} - /* Fast versions: if you know n is one end of the channel */ static inline struct node *other_node(const struct node *n, const struct chan *chan) diff --git a/lightningd/pay.c b/lightningd/pay.c index 7fd7f850a..ab1c50b20 100644 --- a/lightningd/pay.c +++ b/lightningd/pay.c @@ -252,7 +252,8 @@ static struct routing_failure* immediate_routing_failure(const tal_t *ctx, const struct lightningd *ld, enum onion_type failcode, - const struct short_channel_id *channel0) + const struct short_channel_id *channel0, + const struct pubkey *dstid) { struct routing_failure *routing_failure; @@ -263,6 +264,11 @@ immediate_routing_failure(const tal_t *ctx, routing_failure->failcode = failcode; routing_failure->erring_node = ld->id; routing_failure->erring_channel = *channel0; + if (dstid) + routing_failure->channel_dir = pubkey_idx(&ld->id, dstid); + /* FIXME: Don't set at all unless we know. */ + else + routing_failure->channel_dir = 0; routing_failure->channel_update = NULL; return routing_failure; @@ -285,6 +291,8 @@ local_routing_failure(const tal_t *ctx, routing_failure->failcode = hout->failcode; routing_failure->erring_node = ld->id; routing_failure->erring_channel = payment->route_channels[0]; + routing_failure->channel_dir = pubkey_idx(&ld->id, + &payment->route_nodes[0]); routing_failure->channel_update = NULL; return routing_failure; @@ -313,6 +321,7 @@ remote_routing_failure(const tal_t *ctx, int origin_index; bool retry_plausible; bool report_to_gossipd; + int dir; routing_failure = tal(ctx, struct routing_failure); route_nodes = payment->route_nodes; @@ -333,7 +342,10 @@ remote_routing_failure(const tal_t *ctx, /* Check if at destination. */ if (origin_index == tal_count(route_nodes) - 1) { + /* FIXME: Don't set erring_channel or dir in this case! */ erring_channel = &dummy_channel; + dir = 0; + /* BOLT #4: * * - if the _final node_ is returning the error: @@ -356,6 +368,9 @@ remote_routing_failure(const tal_t *ctx, /* Report the *next* channel as failing. */ erring_channel = &route_channels[origin_index + 1]; + dir = pubkey_idx(&route_nodes[origin_index], + &route_nodes[origin_index+1]); + /* If the error is a BADONION, then it's on behalf of the * following node. */ if (failcode & BADONION) { @@ -371,6 +386,7 @@ remote_routing_failure(const tal_t *ctx, routing_failure->erring_node = *erring_node; routing_failure->erring_channel = *erring_channel; routing_failure->channel_update = channel_update; + routing_failure->channel_dir = dir; *p_retry_plausible = retry_plausible; *p_report_to_gossipd = report_to_gossipd; @@ -559,7 +575,8 @@ void payment_failed(struct lightningd *ld, const struct htlc_out *hout, fail ? &fail->erring_node : NULL, fail ? &fail->erring_channel : NULL, fail ? fail->channel_update : NULL, - failmsg); + failmsg, + fail ? fail->channel_dir : 0); /* Report to gossipd if we decided we should. */ if (report_to_gossipd) @@ -595,6 +612,7 @@ bool wait_payment(const tal_t *cxt, u8 *failupdate; char *faildetail; struct routing_failure *fail; + int faildirection; payment = wallet_payment_by_hash(tmpctx, ld->wallet, payment_hash); if (!payment) { @@ -634,7 +652,8 @@ bool wait_payment(const tal_t *cxt, &failnode, &failchannel, &failupdate, - &faildetail); + &faildetail, + &faildirection); /* Old DB might not save failure information */ if (!failonionreply && !failnode) result = sendpay_result_simple_fail(tmpctx, @@ -653,6 +672,7 @@ bool wait_payment(const tal_t *cxt, fail->erring_node = *failnode; fail->erring_channel = *failchannel; fail->channel_update = failupdate; + fail->channel_dir = faildirection; result = sendpay_result_route_failure(tmpctx, !faildestperm, fail, NULL, faildetail); } @@ -770,7 +790,8 @@ send_payment(const tal_t *ctx, /* Report routing failure to gossipd */ fail = immediate_routing_failure(ctx, ld, WIRE_UNKNOWN_NEXT_PEER, - &route[0].channel_id); + &route[0].channel_id, + 0); report_routing_failure(ld->log, ld->gossip, fail); /* Report routing failure to caller */ @@ -798,7 +819,8 @@ send_payment(const tal_t *ctx, /* Report routing failure to gossipd */ fail = immediate_routing_failure(ctx, ld, failcode, - &route[0].channel_id); + &route[0].channel_id, + &channel->peer->id); report_routing_failure(ld->log, ld->gossip, fail); /* Report routing failure to caller */ @@ -922,6 +944,8 @@ static void json_waitsendpay_on_resolve(const struct sendpay_result *r, json_add_pubkey(data, "erring_node", &fail->erring_node); json_add_short_channel_id(data, "erring_channel", &fail->erring_channel); + json_add_num(data, "erring_channel_direction", + fail->channel_dir); if (fail->channel_update) json_add_hex_talarr(data, "channel_update", fail->channel_update); diff --git a/lightningd/pay.h b/lightningd/pay.h index 294801d5f..bb586ee2c 100644 --- a/lightningd/pay.h +++ b/lightningd/pay.h @@ -19,6 +19,7 @@ struct routing_failure { enum onion_type failcode; struct pubkey erring_node; struct short_channel_id erring_channel; + int channel_dir; u8 *channel_update; }; diff --git a/wallet/db.c b/wallet/db.c index 1ec399ca5..a57f4df41 100644 --- a/wallet/db.c +++ b/wallet/db.c @@ -354,6 +354,8 @@ char *dbmigrations[] = { ", state INTEGER" ", UNIQUE(in_htlc_id, out_htlc_id)" ");", + /* Add a direction for failed payments. */ + "ALTER TABLE payments ADD faildirection INTEGER;", /* erring_direction */ NULL, }; diff --git a/wallet/wallet.c b/wallet/wallet.c index 956f80df6..4d506d62e 100644 --- a/wallet/wallet.c +++ b/wallet/wallet.c @@ -1838,7 +1838,8 @@ void wallet_payment_get_failinfo(const tal_t *ctx, struct pubkey **failnode, struct short_channel_id **failchannel, u8 **failupdate, - char **faildetail) + char **faildetail, + int *faildirection) { sqlite3_stmt *stmt; int res; @@ -1849,7 +1850,7 @@ void wallet_payment_get_failinfo(const tal_t *ctx, "SELECT failonionreply, faildestperm" " , failindex, failcode" " , failnode, failchannel" - " , failupdate, faildetail" + " , failupdate, faildetail, faildirection" " FROM payments" " WHERE payment_hash=?;"); sqlite3_bind_sha256(stmt, 1, payment_hash); @@ -1878,6 +1879,9 @@ void wallet_payment_get_failinfo(const tal_t *ctx, *failchannel = tal(ctx, struct short_channel_id); resb = sqlite3_column_short_channel_id(stmt, 5, *failchannel); assert(resb); + + /* For pre-0.6.2 dbs, direction will be 0 */ + *faildirection = sqlite3_column_int(stmt, 8); } if (sqlite3_column_type(stmt, 6) == SQLITE_NULL) *failupdate = NULL; @@ -1901,7 +1905,8 @@ void wallet_payment_set_failinfo(struct wallet *wallet, const struct pubkey *failnode, const struct short_channel_id *failchannel, const u8 *failupdate /*tal_arr*/, - const char *faildetail) + const char *faildetail, + int faildirection) { sqlite3_stmt *stmt; @@ -1915,6 +1920,7 @@ void wallet_payment_set_failinfo(struct wallet *wallet, " , failchannel=?" " , failupdate=?" " , faildetail=?" + " , faildirection=?" " WHERE payment_hash=?;"); if (failonionreply) sqlite3_bind_blob(stmt, 1, @@ -1935,8 +1941,11 @@ void wallet_payment_set_failinfo(struct wallet *wallet, struct short_channel_id *scid = tal(tmpctx, struct short_channel_id); *scid = *failchannel; sqlite3_bind_short_channel_id(stmt, 6, scid); - } else + sqlite3_bind_int(stmt, 9, faildirection); + } else { sqlite3_bind_null(stmt, 6); + sqlite3_bind_null(stmt, 9); + } if (failupdate) sqlite3_bind_blob(stmt, 7, failupdate, tal_count(failupdate), @@ -1947,7 +1956,7 @@ void wallet_payment_set_failinfo(struct wallet *wallet, faildetail, strlen(faildetail), SQLITE_TRANSIENT); - sqlite3_bind_sha256(stmt, 9, payment_hash); + sqlite3_bind_sha256(stmt, 10, payment_hash); db_exec_prepared(wallet->db, stmt); } diff --git a/wallet/wallet.h b/wallet/wallet.h index 269c2bd35..f5b749936 100644 --- a/wallet/wallet.h +++ b/wallet/wallet.h @@ -883,7 +883,8 @@ void wallet_payment_set_status(struct wallet *wallet, * wallet_payment_get_failinfo - Get failure information for a given * `payment_hash`. * - * Data is allocated as children of the given context. + * Data is allocated as children of the given context. *faildirection + * is only set if *failchannel is set non-NULL. */ void wallet_payment_get_failinfo(const tal_t *ctx, struct wallet *wallet, @@ -896,7 +897,8 @@ void wallet_payment_get_failinfo(const tal_t *ctx, struct pubkey **failnode, struct short_channel_id **failchannel, u8 **failupdate, - char **faildetail); + char **faildetail, + int *faildirection); /** * wallet_payment_set_failinfo - Set failure information for a given * `payment_hash`. @@ -910,7 +912,8 @@ void wallet_payment_set_failinfo(struct wallet *wallet, const struct pubkey *failnode, const struct short_channel_id *failchannel, const u8 *failupdate, - const char *faildetail); + const char *faildetail, + int faildirection); /** * wallet_payment_list - Retrieve a list of payments