Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix offer payment failure #7839

Merged
merged 4 commits into from
Nov 20, 2024
Merged

Fix offer payment failure #7839

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
afd7553
common: provide readable explanation when onion payload is invalid.
rustyrussell Nov 20, 2024
d18147a
lightningd: send errors inside blinded paths correctly.
rustyrussell Nov 20, 2024
b05a315
test: reproduce WIRE_INVALID_ONION_PAYLOAD
vincenzopalazzo Nov 20, 2024
9932a71
offers: update block height correctly.
rustyrussell Nov 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
75 changes: 65 additions & 10 deletions common/onion_decode.c
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had to use fprintf, which is terrible.

A tell you a secret, I do it too sometimes :)

Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,12 @@
#include <ccan/array_size/array_size.h>
#include <ccan/cast/cast.h>
#include <ccan/mem/mem.h>
#include <ccan/tal/str/str.h>
#include <common/blindedpath.h>
#include <common/ecdh.h>
#include <common/onion_decode.h>
#include <common/sphinx.h>
#include <inttypes.h>
#include <sodium/crypto_aead_chacha20poly1305.h>

/* BOLT #4:
Expand Down Expand Up @@ -62,17 +64,22 @@ static u64 ceil_div(u64 a, u64 b)
return (a + b - 1) / b;
}

static bool handle_blinded_forward(struct onion_payload *p,
static bool handle_blinded_forward(const tal_t *ctx,
struct onion_payload *p,
struct amount_msat amount_in,
u32 cltv_expiry,
const struct tlv_payload *tlv,
const struct tlv_encrypted_data_tlv *enc,
u64 *failtlvtype)
u64 *failtlvtype,
const char **explanation)
{
u64 amt = amount_in.millisatoshis; /* Raw: allowed to wrap */

if (!check_nonfinal_tlv(tlv, failtlvtype))
if (!check_nonfinal_tlv(tlv, failtlvtype)) {
if (explanation)
*explanation = tal_fmt(ctx, "unexpected tlv type %"PRIu64, *failtlvtype);
return false;
}

/* BOLT #4:
* - If it is not the final node:
Expand All @@ -81,6 +88,8 @@ static bool handle_blinded_forward(struct onion_payload *p,
* contain either `short_channel_id` or `next_node_id`.
*/
if (!enc->short_channel_id && !enc->next_node_id) {
if (explanation)
*explanation = tal_fmt(ctx, "neither short_channel_id nor next_node_id present");
*failtlvtype = TLV_PAYLOAD_ENCRYPTED_RECIPIENT_DATA;
return false;
}
Expand All @@ -104,6 +113,8 @@ static bool handle_blinded_forward(struct onion_payload *p,
* contain `payment_relay`.
*/
if (!enc->payment_relay) {
if (explanation)
*explanation = tal_fmt(ctx, "missing payment_relay");
*failtlvtype = TLV_PAYLOAD_ENCRYPTED_RECIPIENT_DATA;
return false;
}
Expand All @@ -117,13 +128,18 @@ static bool handle_blinded_forward(struct onion_payload *p,
return true;
}

static bool handle_blinded_terminal(struct onion_payload *p,
static bool handle_blinded_terminal(const tal_t *ctx,
struct onion_payload *p,
const struct tlv_payload *tlv,
const struct tlv_encrypted_data_tlv *enc,
u64 *failtlvtype)
u64 *failtlvtype,
const char **explanation)
{
if (!check_final_tlv(tlv, failtlvtype))
if (!check_final_tlv(tlv, failtlvtype)) {
if (explanation)
*explanation = tal_fmt(ctx, "unexpected tlv type %"PRIu64, *failtlvtype);
return false;
}

/* BOLT #4:
* - MUST return an error if `amt_to_forward`, `outgoing_cltv_value`
Expand All @@ -132,16 +148,22 @@ static bool handle_blinded_terminal(struct onion_payload *p,
* for the payment.
*/
if (!tlv->amt_to_forward) {
if (explanation)
*explanation = tal_fmt(ctx, "missing amt_to_forward");
*failtlvtype = TLV_PAYLOAD_AMT_TO_FORWARD;
return false;
}

if (!tlv->outgoing_cltv_value) {
if (explanation)
*explanation = tal_fmt(ctx, "missing outgoing_cltv_value");
*failtlvtype = TLV_PAYLOAD_OUTGOING_CLTV_VALUE;
return false;
}

if (!tlv->total_amount_msat) {
if (explanation)
*explanation = tal_fmt(ctx, "missing total_amount_msat");
*failtlvtype = TLV_PAYLOAD_TOTAL_AMOUNT_MSAT;
return false;
}
Expand Down Expand Up @@ -173,7 +195,8 @@ struct onion_payload *onion_decode(const tal_t *ctx,
struct amount_msat amount_in,
u32 cltv_expiry,
u64 *failtlvtype,
size_t *failtlvpos)
size_t *failtlvpos,
const char **explanation)
{
struct onion_payload *p = tal(ctx, struct onion_payload);
const u8 *cursor = rs->raw_payload;
Expand All @@ -191,6 +214,8 @@ struct onion_payload *onion_decode(const tal_t *ctx,
if (!cursor || len > max) {
*failtlvtype = 0;
*failtlvpos = tal_bytelen(rs->raw_payload);
if (explanation)
*explanation = tal_fmt(ctx, "Too short for initial length");
return tal_free(p);
}

Expand All @@ -201,6 +226,8 @@ struct onion_payload *onion_decode(const tal_t *ctx,
TLVS_ARRAY_SIZE_tlv_payload,
p->tlv, &p->tlv->fields, accepted_extra_tlvs,
failtlvpos, failtlvtype)) {
if (explanation)
*explanation = tal_fmt(ctx, "Unparseable TLV");
return tal_free(p);
}

Expand All @@ -225,12 +252,16 @@ struct onion_payload *onion_decode(const tal_t *ctx,
if (path_key) {
if (p->tlv->current_path_key) {
*failtlvtype = TLV_PAYLOAD_CURRENT_PATH_KEY;
if (explanation)
*explanation = tal_fmt(ctx, "current_path_key was present");
goto field_bad;
}
p->path_key = tal_dup(p, struct pubkey, path_key);
} else {
if (!p->tlv->current_path_key) {
*failtlvtype = TLV_PAYLOAD_CURRENT_PATH_KEY;
if (explanation)
*explanation = tal_fmt(ctx, "current_path_key was not present");
goto field_bad;
}
p->path_key = tal_dup(p, struct pubkey,
Expand All @@ -248,6 +279,8 @@ struct onion_payload *onion_decode(const tal_t *ctx,
enc = decrypt_encrypted_data(tmpctx, &p->blinding_ss,
p->tlv->encrypted_recipient_data);
if (!enc) {
if (explanation)
*explanation = tal_fmt(ctx, "encrypted_recipient_data decryption failed");
*failtlvtype = TLV_PAYLOAD_ENCRYPTED_RECIPIENT_DATA;
goto field_bad;
}
Expand All @@ -259,6 +292,10 @@ struct onion_payload *onion_decode(const tal_t *ctx,
* `encrypted_recipient_data.payment_constraints.max_cltv_expiry`.
*/
if (cltv_expiry > enc->payment_constraints->max_cltv_expiry) {
if (explanation)
*explanation = tal_fmt(ctx, "cltv_expiry %u > payment_constraint %u",
cltv_expiry,
enc->payment_constraints->max_cltv_expiry);
*failtlvtype = TLV_PAYLOAD_ENCRYPTED_RECIPIENT_DATA;
goto field_bad;
}
Expand All @@ -271,6 +308,10 @@ struct onion_payload *onion_decode(const tal_t *ctx,
*/
if (amount_msat_less(amount_in,
amount_msat(enc->payment_constraints->htlc_minimum_msat))) {
if (explanation)
*explanation = tal_fmt(ctx, "amount_in %s < payment_constraint min %"PRIu64,
fmt_amount_msat(tmpctx, amount_in),
enc->payment_constraints->htlc_minimum_msat);
*failtlvtype = TLV_PAYLOAD_ENCRYPTED_RECIPIENT_DATA;
goto field_bad;
}
Expand Down Expand Up @@ -299,21 +340,26 @@ struct onion_payload *onion_decode(const tal_t *ctx,
/* No features, this is easy */
if (!memeqzero(enc->allowed_features,
tal_bytelen(enc->allowed_features))) {
if (explanation)
*explanation = tal_fmt(ctx, "non-zero allowed_features (%s)",
tal_hex(tmpctx, enc->allowed_features));
*failtlvtype = TLV_PAYLOAD_ENCRYPTED_RECIPIENT_DATA;
goto field_bad;
}

if (enc->short_channel_id && enc->next_node_id) {
if (explanation)
*explanation = tal_fmt(ctx, "both scid and next_node_id present");
*failtlvtype = TLV_PAYLOAD_ENCRYPTED_RECIPIENT_DATA;
goto field_bad;
}

if (!p->final) {
if (!handle_blinded_forward(p, amount_in, cltv_expiry,
p->tlv, enc, failtlvtype))
if (!handle_blinded_forward(ctx, p, amount_in, cltv_expiry,
p->tlv, enc, failtlvtype, explanation))
goto field_bad;
} else {
if (!handle_blinded_terminal(p, p->tlv, enc, failtlvtype))
if (!handle_blinded_terminal(ctx, p, p->tlv, enc, failtlvtype, explanation))
goto field_bad;
}

Expand All @@ -336,6 +382,9 @@ struct onion_payload *onion_decode(const tal_t *ctx,
* is present.
*/
if (path_key || p->tlv->current_path_key) {
if (explanation)
*explanation = tal_fmt(ctx, "%s set outside blinded route",
path_key ? "update_add_htlc->path_key" : "current_path_key");
*failtlvtype = TLV_PAYLOAD_ENCRYPTED_RECIPIENT_DATA;
goto field_bad;
}
Expand All @@ -348,10 +397,14 @@ struct onion_payload *onion_decode(const tal_t *ctx,
* `outgoing_cltv_value` are not present.
*/
if (!p->tlv->amt_to_forward) {
if (explanation)
*explanation = tal_fmt(ctx, "missing amt_to_forward");
*failtlvtype = TLV_PAYLOAD_AMT_TO_FORWARD;
goto field_bad;
}
if (!p->tlv->outgoing_cltv_value) {
if (explanation)
*explanation = tal_fmt(ctx, "missing outgoing_cltv_value");
*failtlvtype = TLV_PAYLOAD_OUTGOING_CLTV_VALUE;
goto field_bad;
}
Expand All @@ -367,6 +420,8 @@ struct onion_payload *onion_decode(const tal_t *ctx,
*/
if (!p->final) {
if (!p->tlv->short_channel_id) {
if (explanation)
*explanation = tal_fmt(ctx, "missing short_channel_id");
*failtlvtype = TLV_PAYLOAD_SHORT_CHANNEL_ID;
goto field_bad;
}
Expand Down
4 changes: 3 additions & 1 deletion common/onion_decode.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
* @cltv_expiry: Incoming HTLC cltv_expiry
* @failtlvtype: (out) the tlv type which failed to parse.
* @failtlvpos: (out) the offset in the tlv which failed to parse.
* @explanation: (out) if non-NULL, a string describing the problem on failure.
*
* If the payload is not valid, returns NULL.
*/
Expand All @@ -26,5 +27,6 @@ struct onion_payload *onion_decode(const tal_t *ctx,
struct amount_msat amount_in,
u32 cltv_expiry,
u64 *failtlvtype,
size_t *failtlvpos);
size_t *failtlvpos,
const char **explanation);
#endif /* LIGHTNING_COMMON_ONION_DECODE_H */
3 changes: 3 additions & 0 deletions common/test/run-blindedpath_onion.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,9 @@ struct amount_asset amount_sat_to_asset(struct amount_sat *sat UNNEEDED, const u
/* Generated stub for amount_tx_fee */
struct amount_sat amount_tx_fee(u32 fee_per_kw UNNEEDED, size_t weight UNNEEDED)
{ fprintf(stderr, "amount_tx_fee called!\n"); abort(); }
/* Generated stub for fmt_amount_msat */
char *fmt_amount_msat(const tal_t *ctx UNNEEDED, struct amount_msat msat UNNEEDED)
{ fprintf(stderr, "fmt_amount_msat called!\n"); abort(); }
/* Generated stub for fromwire_amount_msat */
struct amount_msat fromwire_amount_msat(const u8 **cursor UNNEEDED, size_t *max UNNEEDED)
{ fprintf(stderr, "fromwire_amount_msat called!\n"); abort(); }
Expand Down
3 changes: 3 additions & 0 deletions common/test/run-onion-test-vector.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,9 @@ struct tlv_encrypted_data_tlv *decrypt_encrypted_data(const tal_t *ctx UNNEEDED,
/* Generated stub for ecdh */
void ecdh(const struct pubkey *point UNNEEDED, struct secret *ss UNNEEDED)
{ fprintf(stderr, "ecdh called!\n"); abort(); }
/* Generated stub for fmt_amount_msat */
char *fmt_amount_msat(const tal_t *ctx UNNEEDED, struct amount_msat msat UNNEEDED)
{ fprintf(stderr, "fmt_amount_msat called!\n"); abort(); }
/* Generated stub for fromwire_amount_msat */
struct amount_msat fromwire_amount_msat(const u8 **cursor UNNEEDED, size_t *max UNNEEDED)
{ fprintf(stderr, "fromwire_amount_msat called!\n"); abort(); }
Expand Down
3 changes: 3 additions & 0 deletions common/test/run-sphinx.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,9 @@ struct tlv_encrypted_data_tlv *decrypt_encrypted_data(const tal_t *ctx UNNEEDED,
/* Generated stub for ecdh */
void ecdh(const struct pubkey *point UNNEEDED, struct secret *ss UNNEEDED)
{ fprintf(stderr, "ecdh called!\n"); abort(); }
/* Generated stub for fmt_amount_msat */
char *fmt_amount_msat(const tal_t *ctx UNNEEDED, struct amount_msat msat UNNEEDED)
{ fprintf(stderr, "fmt_amount_msat called!\n"); abort(); }
/* Generated stub for fromwire_amount_msat */
struct amount_msat fromwire_amount_msat(const u8 **cursor UNNEEDED, size_t *max UNNEEDED)
{ fprintf(stderr, "fromwire_amount_msat called!\n"); abort(); }
Expand Down
8 changes: 5 additions & 3 deletions lightningd/pay.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1844,6 +1844,7 @@ static struct command_result *json_injectpaymentonion(struct command *cmd,
const u8 *failmsg;
struct htlc_out *hout;
const struct wallet_payment *prev_payment;
const char *explanation;

if (!param_check(cmd, buffer, params,
p_req("onion", param_bin_from_hex, &onion),
Expand Down Expand Up @@ -1915,12 +1916,13 @@ static struct command_result *json_injectpaymentonion(struct command *cmd,
cmd->ld->accept_extra_tlv_types,
*msat, *cltv,
&failtlvtype,
&failtlvpos);
&failtlvpos,
&explanation);
if (!payload) {
return command_fail(cmd, JSONRPC2_INVALID_PARAMS,
"Onion decode for %s failed at type %"PRIu64" offset %zu",
"Onion decode for %s failed at type %"PRIu64" offset %zu (%s)",
tal_hex(tmpctx, rs->raw_payload),
failtlvtype, failtlvpos);
failtlvtype, failtlvpos, explanation);
}

if (payload->final) {
Expand Down
Loading
Loading