From 27547ce8d412cf8f0244c636c7a24b8a116b5444 Mon Sep 17 00:00:00 2001 From: Christian Decker Date: Mon, 11 Nov 2019 17:18:10 +0100 Subject: [PATCH] pay: Allow `sendonion` callers to provide `shared_secrets` This means that c-lightning can now internally decrypt an eventual error message, and not force the caller to implement the decryption. The main difficulty was that we now have a new state (channels and nodes not specified, while shared_secrets are specified) which needed to be handled. --- lightningd/pay.c | 41 ++++++++++++++++++++++++++++++++++++++--- tests/test_pay.py | 18 ++++++++++++++++++ wallet/wallet.c | 24 ++++++++++++++---------- 3 files changed, 70 insertions(+), 13 deletions(-) diff --git a/lightningd/pay.c b/lightningd/pay.c index 1baf37359..e4e9f3290 100644 --- a/lightningd/pay.c +++ b/lightningd/pay.c @@ -362,7 +362,24 @@ remote_routing_failure(const tal_t *ctx, assert(route_nodes == NULL || origin_index < tal_count(route_nodes)); - if (origin_index == tal_count(route_nodes) - 1) { + /* Either we have both channels and nodes, or neither */ + assert((route_nodes == NULL) == (route_channels == NULL)); + + if (route_nodes == NULL) { + /* This means we have the `shared_secrets`, but cannot infer + * the erring channel and node since we don't have them. This + * can happen if the payment was initialized using `sendonion` + * and the `shared_secrets` where specified. */ + dir = 0; + erring_channel = NULL; + erring_node = NULL; + + /* We don't know if there's another route, that'd depend on + * where the failure occured and whether it was a node + * failure. Let's assume it wasn't a terminal one, and have + * the sendonion caller deal with the actual decision. */ + *pay_errcode = PAY_TRY_OTHER_ROUTE; + } else if (origin_index == tal_count(route_nodes) - 1) { /* If any channel is to blame, it's the last one. */ erring_channel = &route_channels[origin_index]; /* Single hop? */ @@ -484,7 +501,7 @@ void payment_failed(struct lightningd *ld, const struct htlc_out *hout, #else assert(payment); #endif - assert((payment->path_secrets == NULL) == (payment->route_nodes == NULL)); + assert((payment->route_channels == NULL) == (payment->route_nodes == NULL)); /* This gives more details than a generic failure message */ if (localfail) { @@ -945,12 +962,16 @@ static struct command_result *json_sendonion(struct command *cmd, struct lightningd *ld = cmd->ld; struct wallet_payment *payment; const char *label; + const jsmntok_t *secretstok, *cur; + struct secret *path_secrets; + size_t i; if (!param(cmd, buffer, params, p_req("onion", param_bin_from_hex, &onion), p_req("first_hop", param_route_hop, &first_hop), p_req("payment_hash", param_sha256, &payment_hash), p_opt("label", param_escaped_string, &label), + p_opt("shared_secrets", param_array, &secretstok), NULL)) return command_param_failed(); @@ -962,6 +983,20 @@ static struct command_result *json_sendonion(struct command *cmd, "with failcode=%d", failcode); + if (secretstok) { + path_secrets = tal_arr(cmd, struct secret, secretstok->size); + json_for_each_arr(i, cur, secretstok) { + if (!json_to_secret(buffer, cur, &path_secrets[i])) + return command_fail( + cmd, JSONRPC2_INVALID_PARAMS, + "shared_secret[%zu] isn't a valid " + "hex-encoded 32 byte secret", + i); + } + } else { + path_secrets = NULL; + } + /* FIXME if the user specified a short_channel_id, but no peer nodeid, * we need to resolve that first. */ @@ -996,10 +1031,10 @@ static struct command_result *json_sendonion(struct command *cmd, * externally, since we can't decrypt them.*/ payment->destination = NULL; payment->payment_preimage = NULL; - payment->path_secrets = NULL; payment->route_nodes = NULL; payment->route_channels = NULL; payment->bolt11 = NULL; + payment->path_secrets = tal_steal(payment, path_secrets); if (label != NULL) payment->label = tal_strdup(payment, label); diff --git a/tests/test_pay.py b/tests/test_pay.py index 3dcfe1b7f..ab24b0cfc 100644 --- a/tests/test_pay.py +++ b/tests/test_pay.py @@ -2543,3 +2543,21 @@ def test_sendonion_rpc(node_factory): pays = l1.rpc.listsendpays(payment_hash=payment_hash)['payments'] assert(len(pays) == 1 and pays[0]['status'] == 'failed' and pays[0]['payment_hash'] == payment_hash) + assert('erroronion' in pays[0]) + + # Fail onion is msg + padding = 256 + 2*2 byte lengths + 32 byte HMAC + assert(len(pays[0]['erroronion']) == (256 + 32 + 2 + 2) * 2) + + # Let's try that again, this time we give it the shared_secrets so it + # should be able to decode the error. + payment_hash = "01" * 32 + onion = l1.rpc.createonion(hops=hops, assocdata=payment_hash) + l1.rpc.sendonion(onion=onion['onion'], first_hop=first_hop, + payment_hash=payment_hash, + shared_secrets=onion['shared_secrets']) + + try: + l1.rpc.waitsendpay(payment_hash=payment_hash) + except RpcError as e: + assert(e.error['code'] == 202) + assert(e.error['message'] == "Malformed error reply") diff --git a/wallet/wallet.c b/wallet/wallet.c index 817f8a75d..e7226e7f0 100644 --- a/wallet/wallet.c +++ b/wallet/wallet.c @@ -2138,15 +2138,16 @@ void wallet_payment_store(struct wallet *wallet, db_bind_amount_msat(stmt, 3, &payment->msatoshi); db_bind_int(stmt, 4, payment->timestamp); - if (payment->route_nodes) { - assert(payment->path_secrets); - assert(payment->route_nodes); - assert(payment->route_channels); + if (payment->path_secrets != NULL) db_bind_secret_arr(stmt, 5, payment->path_secrets); + else + db_bind_null(stmt, 5); + + assert((payment->route_channels == NULL) == (payment->route_nodes == NULL)); + if (payment->route_nodes) { db_bind_node_id_arr(stmt, 6, payment->route_nodes); db_bind_short_channel_id_arr(stmt, 7, payment->route_channels); } else { - db_bind_null(stmt, 5); db_bind_null(stmt, 6); db_bind_null(stmt, 7); } @@ -2211,17 +2212,20 @@ static struct wallet_payment *wallet_stmt2payment(const tal_t *ctx, } else payment->payment_preimage = NULL; - /* Can be NULL for old db! */ - if (!db_column_is_null(stmt, 7)) { - assert(!db_column_is_null(stmt, 8)); - assert(!db_column_is_null(stmt, 9)); + /* We either used `sendpay` or `sendonion` with the `shared_secrets` + * argument. */ + if (!db_column_is_null(stmt, 7)) payment->path_secrets = db_column_secret_arr(payment, stmt, 7); + else + payment->path_secrets = NULL; + /* Either none, or both are set */ + assert(db_column_is_null(stmt, 8) == db_column_is_null(stmt, 9)); + if (!db_column_is_null(stmt, 8)) { payment->route_nodes = db_column_node_id_arr(payment, stmt, 8); payment->route_channels = db_column_short_channel_id_arr(payment, stmt, 9); } else { - payment->path_secrets = NULL; payment->route_nodes = NULL; payment->route_channels = NULL; }