lightningd: allow htlc_set_fail to take empty msg to send incorrect_or_unknown_payment_details

This message is supposed to include the msat amount received.  But this is
obviously per-HTLC, and we hacked it to use the value for the first one.

And we add logging whenever we fail an HTLC set, since we removed logging
by not calling failmsg_incorrect_or_unknown() (which, now, no longer needs
to log).

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
This commit is contained in:
Rusty Russell 2024-10-22 11:09:46 +10:30
parent fdb3f5fe1b
commit 41610d7bab
6 changed files with 46 additions and 37 deletions

View file

@ -1,6 +1,7 @@
#include "config.h" #include "config.h"
#include <common/features.h> #include <common/features.h>
#include <common/timeout.h> #include <common/timeout.h>
#include <lightningd/chaintopology.h>
#include <lightningd/channel.h> #include <lightningd/channel.h>
#include <lightningd/htlc_set.h> #include <lightningd/htlc_set.h>
#include <lightningd/invoice.h> #include <lightningd/invoice.h>
@ -41,16 +42,29 @@ static void timeout_htlc_set(struct htlc_set *set)
htlc_set_fail(set, take(towire_mpp_timeout(NULL))); htlc_set_fail(set, take(towire_mpp_timeout(NULL)));
} }
void htlc_set_fail(struct htlc_set *set, const u8 *failmsg TAKES) void htlc_set_fail_(struct htlc_set *set, const u8 *failmsg TAKES,
const char *file, int line)
{ {
/* Don't let local_fail_in_htlc take! */ /* Don't let local_fail_in_htlc take! */
if (taken(failmsg)) if (taken(failmsg))
tal_steal(set, failmsg); tal_steal(set, failmsg);
for (size_t i = 0; i < tal_count(set->htlcs); i++) { for (size_t i = 0; i < tal_count(set->htlcs); i++) {
const u8 *this_failmsg;
/* Don't remove from set */ /* Don't remove from set */
tal_del_destructor2(set->htlcs[i], htlc_set_hin_destroyed, set); tal_del_destructor2(set->htlcs[i], htlc_set_hin_destroyed, set);
local_fail_in_htlc(set->htlcs[i], failmsg);
if (tal_bytelen(failmsg) == 0)
this_failmsg = towire_incorrect_or_unknown_payment_details(tmpctx, set->htlcs[i]->msat, get_block_height(set->ld->topology));
else
this_failmsg = failmsg;
log_debug(set->htlcs[i]->key.channel->log,
"failing with %s: %s:%u",
onion_wire_name(fromwire_peektype(this_failmsg)),
file, line);
local_fail_in_htlc(set->htlcs[i], this_failmsg);
} }
tal_free(set); tal_free(set);
} }
@ -76,6 +90,7 @@ static struct htlc_set *new_htlc_set(struct lightningd *ld,
struct htlc_set *set; struct htlc_set *set;
set = tal(ld, struct htlc_set); set = tal(ld, struct htlc_set);
set->ld = ld;
set->total_msat = total_msat; set->total_msat = total_msat;
set->payment_hash = hin->payment_hash; set->payment_hash = hin->payment_hash;
set->so_far = AMOUNT_MSAT(0); set->so_far = AMOUNT_MSAT(0);
@ -122,7 +137,8 @@ void htlc_set_add(struct lightningd *ld,
/* If we insist on a payment secret, it must always have it */ /* If we insist on a payment secret, it must always have it */
if (feature_is_set(details->features, COMPULSORY_FEATURE(OPT_PAYMENT_SECRET)) if (feature_is_set(details->features, COMPULSORY_FEATURE(OPT_PAYMENT_SECRET))
&& !payment_secret) { && !payment_secret) {
log_debug(ld->log, "Missing payment_secret, but required for %s", log_debug(hin->key.channel->log,
"Missing payment_secret, but required for %s",
fmt_sha256(tmpctx, &hin->payment_hash)); fmt_sha256(tmpctx, &hin->payment_hash));
local_fail_in_htlc(hin, local_fail_in_htlc(hin,
take(failmsg_incorrect_or_unknown(NULL, ld, hin->msat))); take(failmsg_incorrect_or_unknown(NULL, ld, hin->msat)));
@ -149,6 +165,8 @@ void htlc_set_add(struct lightningd *ld,
/* We check this now, since we want to fail with this as soon /* We check this now, since we want to fail with this as soon
* as possible, to avoid other probing attacks. */ * as possible, to avoid other probing attacks. */
if (!payment_secret) { if (!payment_secret) {
log_debug(hin->key.channel->log,
"Missing payment_secret, but required for MPP");
local_fail_in_htlc(hin, take(failmsg_incorrect_or_unknown(NULL, ld, hin->msat))); local_fail_in_htlc(hin, take(failmsg_incorrect_or_unknown(NULL, ld, hin->msat)));
return; return;
} }
@ -214,8 +232,7 @@ void htlc_set_add(struct lightningd *ld,
* - MUST require `payment_secret` for all HTLCs in the set. */ * - MUST require `payment_secret` for all HTLCs in the set. */
/* This catches the case of the first payment in a set. */ /* This catches the case of the first payment in a set. */
if (!payment_secret) { if (!payment_secret) {
htlc_set_fail(set, htlc_set_fail(set, NULL);
take(failmsg_incorrect_or_unknown(NULL, ld, hin->msat)));
return; return;
} }
} }

View file

@ -14,6 +14,7 @@ struct lightningd;
/* Set of incoming HTLCs for multi-part-payments */ /* Set of incoming HTLCs for multi-part-payments */
struct htlc_set { struct htlc_set {
struct lightningd *ld;
struct amount_msat total_msat, so_far; struct amount_msat total_msat, so_far;
struct sha256 payment_hash; struct sha256 payment_hash;
struct htlc_in **htlcs; struct htlc_in **htlcs;
@ -48,8 +49,12 @@ void htlc_set_add(struct lightningd *ld,
struct amount_msat total_msat, struct amount_msat total_msat,
const struct secret *payment_secret); const struct secret *payment_secret);
/* Fail every htlc in the set: frees set */ /* Fail every htlc in the set: frees set. If failmsg is NULL/zero-length,
void htlc_set_fail(struct htlc_set *set, const u8 *failmsg TAKES); * it sends each one a WIRE_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS. */
#define htlc_set_fail(set, failmsg) \
htlc_set_fail_((set), (failmsg), __FILE__, __LINE__)
void htlc_set_fail_(struct htlc_set *set, const u8 *failmsg TAKES,
const char *file, int line);
/* Fulfill every htlc in the set: frees set */ /* Fulfill every htlc in the set: frees set */
void htlc_set_fulfill(struct htlc_set *set, const struct preimage *preimage); void htlc_set_fulfill(struct htlc_set *set, const struct preimage *preimage);

View file

@ -223,9 +223,9 @@ static void invoice_payload_remove_set(struct htlc_set *set,
payload->set = NULL; payload->set = NULL;
} }
/* Returns magic value to send generic incorrect_or_unknown_payment_details */
static const u8 *hook_gives_failmsg(const tal_t *ctx, static const u8 *hook_gives_failmsg(const tal_t *ctx,
struct lightningd *ld, struct lightningd *ld,
const struct htlc_in *hin,
const char *buffer, const char *buffer,
const jsmntok_t *toks) const jsmntok_t *toks)
{ {
@ -242,7 +242,9 @@ static const u8 *hook_gives_failmsg(const tal_t *ctx,
if (json_tok_streq(buffer, resulttok, "continue")) { if (json_tok_streq(buffer, resulttok, "continue")) {
return NULL; return NULL;
} else if (json_tok_streq(buffer, resulttok, "reject")) { } else if (json_tok_streq(buffer, resulttok, "reject")) {
return failmsg_incorrect_or_unknown(ctx, ld, hin->msat); /* htlc_set_fail makes this a per-htlc
* incorrect_or_unknown_payment_details */
return tal_arr(ctx, u8, 0);
} else } else
fatal("Invalid invoice_payment hook result: %.*s", fatal("Invalid invoice_payment hook result: %.*s",
toks[0].end - toks[0].start, buffer); toks[0].end - toks[0].start, buffer);
@ -274,8 +276,7 @@ invoice_payment_hooks_done(struct invoice_payment_hook_payload *payload STEALS)
/* If invoice gets paid meanwhile (plugin responds out-of-order?) then /* If invoice gets paid meanwhile (plugin responds out-of-order?) then
* we can also fail */ * we can also fail */
if (!invoices_find_by_label(ld->wallet->invoices, &inv_dbid, payload->label)) { if (!invoices_find_by_label(ld->wallet->invoices, &inv_dbid, payload->label)) {
htlc_set_fail(payload->set, take(failmsg_incorrect_or_unknown( htlc_set_fail(payload->set, NULL);
NULL, ld, payload->set->htlcs[0]->msat)));
return; return;
} }
@ -283,8 +284,7 @@ invoice_payment_hooks_done(struct invoice_payment_hook_payload *payload STEALS)
if (!invoices_resolve(ld->wallet->invoices, inv_dbid, payload->msat, if (!invoices_resolve(ld->wallet->invoices, inv_dbid, payload->msat,
payload->label, payload->outpoint)) { payload->label, payload->outpoint)) {
if (payload->set) if (payload->set)
htlc_set_fail(payload->set, take(failmsg_incorrect_or_unknown( htlc_set_fail(payload->set, NULL);
NULL, ld, payload->set->htlcs[0]->msat)));
return; return;
} }
@ -316,8 +316,7 @@ invoice_payment_deserialize(struct invoice_payment_hook_payload *payload,
if (payload->set) { if (payload->set) {
/* Did we have a hook result? */ /* Did we have a hook result? */
failmsg = hook_gives_failmsg(NULL, ld, failmsg = hook_gives_failmsg(NULL, ld, buffer, toks);
payload->set->htlcs[0], buffer, toks);
if (failmsg) { if (failmsg) {
htlc_set_fail(payload->set, take(failmsg)); htlc_set_fail(payload->set, take(failmsg));
return false; return false;

View file

@ -275,13 +275,10 @@ void local_fail_in_htlc(struct htlc_in *hin, const u8 *failmsg TAKES)
} }
/* Helper to create (common) WIRE_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS */ /* Helper to create (common) WIRE_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS */
const u8 *failmsg_incorrect_or_unknown_(const tal_t *ctx, const u8 *failmsg_incorrect_or_unknown(const tal_t *ctx,
struct lightningd *ld, struct lightningd *ld,
struct amount_msat msat, struct amount_msat msat)
const char *file, int line)
{ {
log_debug(ld->log, "WIRE_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS: %s:%u",
file, line);
return towire_incorrect_or_unknown_payment_details( return towire_incorrect_or_unknown_payment_details(
ctx, msat, ctx, msat,
get_block_height(ld->topology)); get_block_height(ld->topology));

View file

@ -62,11 +62,7 @@ void local_fail_in_htlc_needs_update(struct htlc_in *hin,
const struct short_channel_id *failmsg_scid); const struct short_channel_id *failmsg_scid);
/* Helper to create (common) WIRE_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS */ /* Helper to create (common) WIRE_INCORRECT_OR_UNKNOWN_PAYMENT_DETAILS */
#define failmsg_incorrect_or_unknown(ctx, ld, msat) \ const u8 *failmsg_incorrect_or_unknown(const tal_t *ctx,
failmsg_incorrect_or_unknown_((ctx), (ld), (msat), __FILE__, __LINE__) struct lightningd *ld,
struct amount_msat msat);
const u8 *failmsg_incorrect_or_unknown_(const tal_t *ctx,
struct lightningd *ld,
struct amount_msat msat,
const char *file, int line);
#endif /* LIGHTNING_LIGHTNINGD_PEER_HTLCS_H */ #endif /* LIGHTNING_LIGHTNINGD_PEER_HTLCS_H */

View file

@ -251,12 +251,6 @@ u8 *encrypt_tlv_encrypted_data(const tal_t *ctx UNNEEDED,
struct pubkey *node_alias) struct pubkey *node_alias)
{ fprintf(stderr, "encrypt_tlv_encrypted_data called!\n"); abort(); } { fprintf(stderr, "encrypt_tlv_encrypted_data called!\n"); abort(); }
/* Generated stub for failmsg_incorrect_or_unknown_ */
const u8 *failmsg_incorrect_or_unknown_(const tal_t *ctx UNNEEDED,
struct lightningd *ld UNNEEDED,
struct amount_msat msat UNNEEDED,
const char *file UNNEEDED, int line UNNEEDED)
{ fprintf(stderr, "failmsg_incorrect_or_unknown_ called!\n"); abort(); }
/* Generated stub for fatal */ /* Generated stub for fatal */
void fatal(const char *fmt UNNEEDED, ...) void fatal(const char *fmt UNNEEDED, ...)
{ fprintf(stderr, "fatal called!\n"); abort(); } { fprintf(stderr, "fatal called!\n"); abort(); }
@ -375,9 +369,10 @@ bool htlc_is_trimmed(enum side htlc_owner UNNEEDED,
/* Generated stub for htlc_max_possible_send */ /* Generated stub for htlc_max_possible_send */
struct amount_msat htlc_max_possible_send(const struct channel *channel UNNEEDED) struct amount_msat htlc_max_possible_send(const struct channel *channel UNNEEDED)
{ fprintf(stderr, "htlc_max_possible_send called!\n"); abort(); } { fprintf(stderr, "htlc_max_possible_send called!\n"); abort(); }
/* Generated stub for htlc_set_fail */ /* Generated stub for htlc_set_fail_ */
void htlc_set_fail(struct htlc_set *set UNNEEDED, const u8 *failmsg TAKES UNNEEDED) void htlc_set_fail_(struct htlc_set *set UNNEEDED, const u8 *failmsg TAKES UNNEEDED,
{ fprintf(stderr, "htlc_set_fail called!\n"); abort(); } const char *file UNNEEDED, int line UNNEEDED)
{ fprintf(stderr, "htlc_set_fail_ called!\n"); abort(); }
/* Generated stub for htlc_set_fulfill */ /* Generated stub for htlc_set_fulfill */
void htlc_set_fulfill(struct htlc_set *set UNNEEDED, const struct preimage *preimage UNNEEDED) void htlc_set_fulfill(struct htlc_set *set UNNEEDED, const struct preimage *preimage UNNEEDED)
{ fprintf(stderr, "htlc_set_fulfill called!\n"); abort(); } { fprintf(stderr, "htlc_set_fulfill called!\n"); abort(); }