From 590b2db88ea3a5512ed782c632d2988e7557dda4 Mon Sep 17 00:00:00 2001 From: Rusty Russell Date: Fri, 21 Feb 2020 15:38:39 +1030 Subject: [PATCH] lightningd: make local htlc failures pass a wiremsg for errors, not a failcode. Unfortunately the invoice_payment_hook can give us a failcode, so I simply restrict it to the two sensible ones. Signed-off-by: Rusty Russell Changelog-deprecated: plugins: invoice_payment_hook "failure_code" only handles simple cases now, use "failure_message". --- lightningd/htlc_set.c | 28 ++++++++----- lightningd/htlc_set.h | 2 +- lightningd/invoice.c | 45 ++++++++++++--------- lightningd/peer_htlcs.c | 30 +++++++++----- lightningd/peer_htlcs.h | 6 ++- lightningd/test/run-invoice-select-inchan.c | 9 ++++- 6 files changed, 78 insertions(+), 42 deletions(-) diff --git a/lightningd/htlc_set.c b/lightningd/htlc_set.c index b9485919f..05bbd0089 100644 --- a/lightningd/htlc_set.c +++ b/lightningd/htlc_set.c @@ -14,7 +14,7 @@ static void htlc_set_hin_destroyed(struct htlc_in *hin, /* Don't try to re-fail this HTLC! */ tal_arr_remove(&set->htlcs, i); /* Kind of the correct failure code. */ - htlc_set_fail(set, WIRE_MPP_TIMEOUT); + htlc_set_fail(set, take(towire_mpp_timeout(NULL))); return; } } @@ -35,15 +35,19 @@ static void destroy_htlc_set(struct htlc_set *set, */ static void timeout_htlc_set(struct htlc_set *set) { - htlc_set_fail(set, WIRE_MPP_TIMEOUT); + htlc_set_fail(set, take(towire_mpp_timeout(NULL))); } -void htlc_set_fail(struct htlc_set *set, enum onion_type failcode) +void htlc_set_fail(struct htlc_set *set, const u8 *failmsg TAKES) { + /* Don't let local_fail_in_htlc take! */ + if (taken(failmsg)) + tal_steal(set, failmsg); + for (size_t i = 0; i < tal_count(set->htlcs); i++) { /* Don't remove from set */ tal_del_destructor2(set->htlcs[i], htlc_set_hin_destroyed, set); - fail_htlc(set->htlcs[i], failcode); + local_fail_in_htlc(set->htlcs[i], failmsg); } tal_free(set); } @@ -101,7 +105,8 @@ void htlc_set_add(struct lightningd *ld, details = invoice_check_payment(tmpctx, ld, &hin->payment_hash, total_msat, payment_secret); if (!details) { - fail_htlc(hin, WIRE_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS); + local_fail_in_htlc(hin, + take(failmsg_incorrect_or_unknown(NULL, hin))); return; } @@ -125,7 +130,7 @@ void htlc_set_add(struct lightningd *ld, /* We check this now, since we want to fail with this as soon * as possible, to avoid other probing attacks. */ if (!payment_secret) { - fail_htlc(hin, WIRE_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS); + local_fail_in_htlc(hin, take(failmsg_incorrect_or_unknown(NULL, hin))); return; } tal_arr_expand(&set->htlcs, hin); @@ -147,7 +152,9 @@ void htlc_set_add(struct lightningd *ld, &set->total_msat), type_to_string(tmpctx, struct amount_msat, &total_msat)); - htlc_set_fail(set, WIRE_FINAL_INCORRECT_HTLC_AMOUNT); + htlc_set_fail(set, + take(towire_final_incorrect_htlc_amount(NULL, + hin->msat))); return; } @@ -164,7 +171,9 @@ void htlc_set_add(struct lightningd *ld, &set->so_far), type_to_string(tmpctx, struct amount_msat, &hin->msat)); - htlc_set_fail(set, WIRE_FINAL_INCORRECT_HTLC_AMOUNT); + htlc_set_fail(set, + take(towire_final_incorrect_htlc_amount(NULL, + hin->msat))); return; } @@ -183,7 +192,8 @@ void htlc_set_add(struct lightningd *ld, * - MUST require `payment_secret` for all HTLCs in the set. */ /* This catches the case of the first payment in a set. */ if (!payment_secret) { - htlc_set_fail(set, WIRE_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS); + htlc_set_fail(set, + take(failmsg_incorrect_or_unknown(NULL, hin))); return; } } diff --git a/lightningd/htlc_set.h b/lightningd/htlc_set.h index 264a52a7b..fa6c070c1 100644 --- a/lightningd/htlc_set.h +++ b/lightningd/htlc_set.h @@ -49,7 +49,7 @@ void htlc_set_add(struct lightningd *ld, const struct secret *payment_secret); /* Fail every htlc in the set: frees set */ -void htlc_set_fail(struct htlc_set *set, enum onion_type failcode); +void htlc_set_fail(struct htlc_set *set, const u8 *failmsg TAKES); /* Fulfill every htlc in the set: frees set */ void htlc_set_fulfill(struct htlc_set *set, const struct preimage *preimage); diff --git a/lightningd/invoice.c b/lightningd/invoice.c index 5f27ff886..6be2f17fd 100644 --- a/lightningd/invoice.c +++ b/lightningd/invoice.c @@ -162,10 +162,11 @@ static void invoice_payload_remove_set(struct htlc_set *set, payload->set = NULL; } -static bool hook_gives_failcode(struct log *log, - const char *buffer, - const jsmntok_t *toks, - enum onion_type *failcode) +static const u8 *hook_gives_failmsg(const tal_t *ctx, + struct log *log, + const struct htlc_in *hin, + const char *buffer, + const jsmntok_t *toks) { const jsmntok_t *resulttok; const jsmntok_t *t; @@ -173,15 +174,14 @@ static bool hook_gives_failcode(struct log *log, /* No plugin registered on hook at all? */ if (!buffer) - return false; + return NULL; resulttok = json_get_member(buffer, toks, "result"); if (resulttok) { if (json_tok_streq(buffer, resulttok, "continue")) { - return false; + return NULL; } else if (json_tok_streq(buffer, resulttok, "reject")) { - *failcode = WIRE_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS; - return true; + return failmsg_incorrect_or_unknown(ctx, hin); } else fatal("Invalid invoice_payment hook result: %.*s", toks[0].end - toks[0].start, buffer); @@ -213,13 +213,16 @@ static bool hook_gives_failcode(struct log *log, fatal("Invalid invoice_payment_hook failure_code: %.*s", toks[0].end - toks[1].start, buffer); - /* UPDATE isn't valid for final nodes to return, and I think - * we assert elsewhere that we don't do this! */ - if (val & UPDATE) - fatal("Invalid invoice_payment_hook UPDATE failure_code: %.*s", - toks[0].end - toks[1].start, buffer); - *failcode = val; - return true; + /* FIXME: Allow hook to return its own failmsg directly? */ + if (val == WIRE_TEMPORARY_NODE_FAILURE) + return towire_temporary_node_failure(ctx); + if (val != WIRE_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS) + log_broken(hin->key.channel->log, + "invoice_payment hook returned failcode %u," + " changing to incorrect_or_unknown_payment_details", + val); + + return failmsg_incorrect_or_unknown(ctx, hin); } static void @@ -229,7 +232,7 @@ invoice_payment_hook_cb(struct invoice_payment_hook_payload *payload, { struct lightningd *ld = payload->ld; struct invoice invoice; - enum onion_type failcode; + const u8 *failmsg; /* We notify here to benefit from the payload and because the hook callback is * called even if the hook is not registered. */ @@ -249,14 +252,16 @@ invoice_payment_hook_cb(struct invoice_payment_hook_payload *payload, /* If invoice gets paid meanwhile (plugin responds out-of-order?) then * we can also fail */ if (!wallet_invoice_find_by_label(ld->wallet, &invoice, payload->label)) { - failcode = WIRE_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS; - htlc_set_fail(payload->set, failcode); + htlc_set_fail(payload->set, take(failmsg_incorrect_or_unknown( + NULL, payload->set->htlcs[0]))); return; } /* Did we have a hook result? */ - if (hook_gives_failcode(ld->log, buffer, toks, &failcode)) { - htlc_set_fail(payload->set, failcode); + failmsg = hook_gives_failmsg(NULL, ld->log, + payload->set->htlcs[0], buffer, toks); + if (failmsg) { + htlc_set_fail(payload->set, take(failmsg)); return; } diff --git a/lightningd/peer_htlcs.c b/lightningd/peer_htlcs.c index e5297ab09..557864b5c 100644 --- a/lightningd/peer_htlcs.c +++ b/lightningd/peer_htlcs.c @@ -295,12 +295,21 @@ static void local_fail_htlc(struct htlc_in *hin, enum onion_type failcode, fail_in_htlc(hin, failcode, NULL, out_channel_update); } -void fail_htlc(struct htlc_in *hin, enum onion_type failcode) +void local_fail_in_htlc(struct htlc_in *hin, const u8 *failmsg TAKES) { - assert(failcode); - /* Final hop never sends an UPDATE. */ - assert(!(failcode & UPDATE)); - local_fail_htlc(hin, failcode, NULL); + /* FIXME: pass failmsg through! */ + local_fail_htlc(hin, fromwire_peektype(failmsg), NULL); + if (taken(failmsg)) + tal_free(failmsg); +} + +/* Helper to create (common) WIRE_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS */ +const u8 *failmsg_incorrect_or_unknown(const tal_t *ctx, + const struct htlc_in *hin) +{ + return towire_incorrect_or_unknown_payment_details( + ctx, hin->msat, + get_block_height(hin->key.channel->owner->ld->topology)); } /* localfail are for handing to the local payer if it's local. */ @@ -443,7 +452,7 @@ static void handle_localpay(struct htlc_in *hin, struct amount_msat total_msat, const struct secret *payment_secret) { - enum onion_type failcode; + const u8 *failmsg; struct lightningd *ld = hin->key.channel->peer->ld; /* BOLT #4: @@ -467,7 +476,7 @@ static void handle_localpay(struct htlc_in *hin, * * The amount in the HTLC doesn't match the value in the onion. */ - failcode = WIRE_FINAL_INCORRECT_HTLC_AMOUNT; + failmsg = towire_final_incorrect_htlc_amount(NULL, hin->msat); goto fail; } @@ -480,7 +489,8 @@ static void handle_localpay(struct htlc_in *hin, * The CLTV expiry in the HTLC doesn't match the value in the onion. */ if (!check_cltv(hin, hin->cltv_expiry, outgoing_cltv_value, 0)) { - failcode = WIRE_FINAL_INCORRECT_CLTV_EXPIRY; + failmsg = towire_final_incorrect_cltv_expiry(NULL, + hin->cltv_expiry); goto fail; } @@ -497,7 +507,7 @@ static void handle_localpay(struct htlc_in *hin, hin->cltv_expiry, get_block_height(ld->topology), ld->config.cltv_final); - failcode = WIRE_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS; + failmsg = failmsg_incorrect_or_unknown(NULL, hin); goto fail; } @@ -505,7 +515,7 @@ static void handle_localpay(struct htlc_in *hin, return; fail: - fail_htlc(hin, failcode); + local_fail_in_htlc(hin, take(failmsg)); } /* diff --git a/lightningd/peer_htlcs.h b/lightningd/peer_htlcs.h index 14b6e172f..99df35a64 100644 --- a/lightningd/peer_htlcs.h +++ b/lightningd/peer_htlcs.h @@ -71,11 +71,15 @@ void htlcs_resubmit(struct lightningd *ld, /* For HTLCs which terminate here, invoice payment calls one of these. */ void fulfill_htlc(struct htlc_in *hin, const struct preimage *preimage); -void fail_htlc(struct htlc_in *hin, enum onion_type failcode); +void local_fail_in_htlc(struct htlc_in *hin, const u8 *failmsg TAKES); /* This json process will be used as the serialize method for * forward_event_notification_gen and be used in * `listforwardings_add_forwardings()`. */ void json_format_forwarding_object(struct json_stream *response, const char *fieldname, const struct forwarding *cur); + +/* Helper to create (common) WIRE_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS */ +const u8 *failmsg_incorrect_or_unknown(const tal_t *ctx, + const struct htlc_in *hin); #endif /* LIGHTNING_LIGHTNINGD_PEER_HTLCS_H */ diff --git a/lightningd/test/run-invoice-select-inchan.c b/lightningd/test/run-invoice-select-inchan.c index 496bb164f..c4ca7976b 100644 --- a/lightningd/test/run-invoice-select-inchan.c +++ b/lightningd/test/run-invoice-select-inchan.c @@ -92,6 +92,10 @@ char *encode_scriptpubkey_to_addr(const tal_t *ctx UNNEEDED, const struct chainparams *chainparams UNNEEDED, const u8 *scriptPubkey UNNEEDED) { fprintf(stderr, "encode_scriptpubkey_to_addr called!\n"); abort(); } +/* Generated stub for failmsg_incorrect_or_unknown */ +const u8 *failmsg_incorrect_or_unknown(const tal_t *ctx UNNEEDED, + const struct htlc_in *hin UNNEEDED) +{ fprintf(stderr, "failmsg_incorrect_or_unknown called!\n"); abort(); } /* Generated stub for fatal */ void fatal(const char *fmt UNNEEDED, ...) { fprintf(stderr, "fatal called!\n"); abort(); } @@ -147,7 +151,7 @@ bool htlc_is_trimmed(enum side htlc_owner UNNEEDED, enum side side UNNEEDED) { fprintf(stderr, "htlc_is_trimmed called!\n"); abort(); } /* Generated stub for htlc_set_fail */ -void htlc_set_fail(struct htlc_set *set UNNEEDED, enum onion_type failcode UNNEEDED) +void htlc_set_fail(struct htlc_set *set UNNEEDED, const u8 *failmsg TAKES UNNEEDED) { fprintf(stderr, "htlc_set_fail called!\n"); abort(); } /* Generated stub for htlc_set_fulfill */ void htlc_set_fulfill(struct htlc_set *set UNNEEDED, const struct preimage *preimage UNNEEDED) @@ -436,6 +440,9 @@ u8 *towire_hsm_sign_invoice(const tal_t *ctx UNNEEDED, const u8 *u5bytes UNNEEDE /* Generated stub for towire_onchain_dev_memleak */ u8 *towire_onchain_dev_memleak(const tal_t *ctx UNNEEDED) { fprintf(stderr, "towire_onchain_dev_memleak called!\n"); abort(); } +/* Generated stub for towire_temporary_node_failure */ +u8 *towire_temporary_node_failure(const tal_t *ctx UNNEEDED) +{ fprintf(stderr, "towire_temporary_node_failure called!\n"); abort(); } /* Generated stub for txfilter_add_scriptpubkey */ void txfilter_add_scriptpubkey(struct txfilter *filter UNNEEDED, const u8 *script TAKES UNNEEDED) { fprintf(stderr, "txfilter_add_scriptpubkey called!\n"); abort(); }