diff --git a/doc/lightning-listforwards.7.md b/doc/lightning-listforwards.7.md index 590150a56..40531ff78 100644 --- a/doc/lightning-listforwards.7.md +++ b/doc/lightning-listforwards.7.md @@ -29,6 +29,7 @@ On success, an object containing **forwards** is returned. It is an array of ob - **received_time** (number): the UNIX timestamp when this was received - **out_channel** (short_channel_id, optional): the channel that the HTLC was forwarded to - **payment_hash** (hex, optional): payment hash sought by HTLC (always 64 characters) +- **style** (string, optional): Either a legacy onion format or a modern tlv format (one of "legacy", "tlv") If **out_channel** is present: - **fee_msat** (msat): the amount this paid in fees @@ -58,4 +59,4 @@ RESOURCES Main web site: -[comment]: # ( SHA256STAMP:abfaaa00817734d8acb77d02d7c024112c90605a8f93a134971a617ab4d383f9) +[comment]: # ( SHA256STAMP:131410f052b8a1845c8d3c7eb2d48df0fc7638e7d26817f56863815be86d8f1e) diff --git a/doc/schemas/listforwards.schema.json b/doc/schemas/listforwards.schema.json index 1f311a5d6..2f0042c46 100644 --- a/doc/schemas/listforwards.schema.json +++ b/doc/schemas/listforwards.schema.json @@ -52,6 +52,14 @@ "description": "payment hash sought by HTLC", "maxLength": 64, "minLength": 64 + }, + "style": { + "type": "string", + "enum": [ + "legacy", + "tlv" + ], + "description": "Either a legacy onion format or a modern tlv format" } }, "allOf": [ @@ -72,6 +80,7 @@ "in_msatoshi": {}, "in_msat": {}, "status": {}, + "style": {}, "received_time": {}, "resolved_time": {}, "out_channel": {}, @@ -102,6 +111,7 @@ "in_msatoshi": {}, "in_msat": {}, "status": {}, + "style": {}, "received_time": {}, "resolved_time": {}, "payment_hash": {}, @@ -132,6 +142,7 @@ "in_msatoshi": {}, "in_msat": {}, "status": {}, + "style": {}, "received_time": {}, "out_channel": {}, "payment_hash": {}, @@ -154,6 +165,7 @@ "in_msatoshi": {}, "in_msat": {}, "status": {}, + "style": {}, "received_time": {}, "out_channel": {}, "payment_hash": {}, @@ -186,6 +198,7 @@ "in_msatoshi": {}, "in_msat": {}, "status": {}, + "style": {}, "received_time": {}, "out_channel": {}, "payment_hash": {}, @@ -212,6 +225,7 @@ "in_msatoshi": {}, "in_msat": {}, "status": {}, + "style": {}, "received_time": {}, "out_channel": {}, "payment_hash": {}, diff --git a/lightningd/htlc_end.c b/lightningd/htlc_end.c index 129ba638c..bd80eecde 100644 --- a/lightningd/htlc_end.c +++ b/lightningd/htlc_end.c @@ -158,6 +158,7 @@ struct htlc_in *new_htlc_in(const tal_t *ctx, hin->failonion = NULL; hin->preimage = NULL; hin->we_filled = NULL; + hin->payload = NULL; hin->received_time = time_now(); diff --git a/lightningd/notification.c b/lightningd/notification.c index 983e9fc74..1ea485106 100644 --- a/lightningd/notification.c +++ b/lightningd/notification.c @@ -296,7 +296,8 @@ static void forward_event_notification_serialize(struct json_stream *stream, const struct amount_msat *amount_out, enum forward_status state, enum onion_wire failcode, - struct timeabs *resolved_time) + struct timeabs *resolved_time, + enum forward_style forward_style) { /* Here is more neat to initial a forwarding structure than * to pass in a bunch of parameters directly*/ @@ -324,6 +325,7 @@ static void forward_event_notification_serialize(struct json_stream *stream, cur->failcode = failcode; cur->received_time = in->received_time; cur->resolved_time = tal_steal(cur, resolved_time); + cur->forward_style = forward_style; json_format_forwarding_object(stream, "forward_event", cur); } @@ -337,7 +339,8 @@ void notify_forward_event(struct lightningd *ld, const struct amount_msat *amount_out, enum forward_status state, enum onion_wire failcode, - struct timeabs *resolved_time) + struct timeabs *resolved_time, + enum forward_style forward_style) { void (*serialize)(struct json_stream *, const struct htlc_in *, @@ -345,11 +348,12 @@ void notify_forward_event(struct lightningd *ld, const struct amount_msat *, enum forward_status, enum onion_wire, - struct timeabs *) = forward_event_notification_gen.serialize; + struct timeabs *, + enum forward_style) = forward_event_notification_gen.serialize; struct jsonrpc_notification *n = jsonrpc_notification_start(NULL, forward_event_notification_gen.topic); - serialize(n->stream, in, scid_out, amount_out, state, failcode, resolved_time); + serialize(n->stream, in, scid_out, amount_out, state, failcode, resolved_time, forward_style); jsonrpc_notification_end(n); plugins_notify(ld->plugins, take(n)); } diff --git a/lightningd/notification.h b/lightningd/notification.h index 87234ca2d..c1e314864 100644 --- a/lightningd/notification.h +++ b/lightningd/notification.h @@ -67,7 +67,8 @@ void notify_forward_event(struct lightningd *ld, const struct amount_msat *amount_out, enum forward_status state, enum onion_wire failcode, - struct timeabs *resolved_time); + struct timeabs *resolved_time, + enum forward_style forward_style); void notify_sendpay_success(struct lightningd *ld, const struct wallet_payment *payment); diff --git a/lightningd/peer_htlcs.c b/lightningd/peer_htlcs.c index 278b80527..f59d32154 100644 --- a/lightningd/peer_htlcs.c +++ b/lightningd/peer_htlcs.c @@ -465,6 +465,21 @@ static void destroy_hout_subd_died(struct htlc_out *hout) db_commit_transaction(db); } +static enum forward_style get_onion_style(const struct htlc_in *hin) +{ + /* This happens on reload from db; don't try too hard! */ + if (!hin->payload) + return FORWARD_STYLE_UNKNOWN; + + switch (hin->payload->type) { + case ONION_V0_PAYLOAD: + return FORWARD_STYLE_LEGACY; + case ONION_TLV_PAYLOAD: + return FORWARD_STYLE_TLV; + } + abort(); +} + /* This is where channeld gives us the HTLC id, and also reports if it * failed immediately. */ static void rcvd_htlc_reply(struct subd *subd, const u8 *msg, const int *fds UNUSED, @@ -503,7 +518,9 @@ static void rcvd_htlc_reply(struct subd *subd, const u8 *msg, const int *fds UNU /* here we haven't called connect_htlc_out(), * so set htlc field with NULL */ wallet_forwarded_payment_add(ld->wallet, - hout->in, NULL, NULL, + hout->in, + get_onion_style(hout->in), + NULL, NULL, FORWARD_LOCAL_FAILED, fromwire_peektype(hout->failmsg)); } @@ -662,7 +679,8 @@ static void forward_htlc(struct htlc_in *hin, if (!next || !channel_active(next)) { local_fail_in_htlc(hin, take(towire_unknown_next_peer(NULL))); wallet_forwarded_payment_add(hin->key.channel->peer->ld->wallet, - hin, next ? next->scid : NULL, NULL, + hin, get_onion_style(hin), + next ? next->scid : NULL, NULL, FORWARD_LOCAL_FAILED, WIRE_UNKNOWN_NEXT_PEER); return; @@ -757,7 +775,7 @@ static void forward_htlc(struct htlc_in *hin, fail: local_fail_in_htlc(hin, failmsg); wallet_forwarded_payment_add(ld->wallet, - hin, next->scid, hout, + hin, get_onion_style(hin), next->scid, hout, FORWARD_LOCAL_FAILED, fromwire_peektype(failmsg)); } @@ -1283,6 +1301,7 @@ static void fulfill_our_htlc_out(struct channel *channel, struct htlc_out *hout, else if (hout->in) { fulfill_htlc(hout->in, preimage); wallet_forwarded_payment_add(ld->wallet, hout->in, + get_onion_style(hout->in), hout->key.channel->scid, hout, FORWARD_SETTLED, 0); } @@ -1410,6 +1429,7 @@ static bool peer_failed_our_htlc(struct channel *channel, if (hout->in) wallet_forwarded_payment_add(ld->wallet, hout->in, + get_onion_style(hout->in), channel->scid, hout, FORWARD_FAILED, hout->failmsg @@ -1560,7 +1580,8 @@ void onchain_failed_our_htlc(const struct channel *channel, local_fail_in_htlc(hout->in, take(towire_permanent_channel_failure(NULL))); wallet_forwarded_payment_add(hout->key.channel->peer->ld->wallet, - hout->in, channel->scid, hout, + hout->in, get_onion_style(hout->in), + channel->scid, hout, FORWARD_LOCAL_FAILED, hout->failmsg ? fromwire_peektype(hout->failmsg) @@ -1726,6 +1747,7 @@ static bool update_out_htlc(struct channel *channel, if (hout->in) { wallet_forwarded_payment_add(ld->wallet, hout->in, + get_onion_style(hout->in), channel->scid, hout, FORWARD_OFFERED, 0); } @@ -2305,7 +2327,7 @@ void peer_got_revoke(struct channel *channel, const u8 *msg) // in fact, now we don't know if this htlc is a forward or localpay! wallet_forwarded_payment_add(ld->wallet, - hin, NULL, NULL, + hin, FORWARD_STYLE_UNKNOWN, NULL, NULL, FORWARD_LOCAL_FAILED, badonions[i] ? badonions[i] : fromwire_peektype(failmsgs[i])); @@ -2722,6 +2744,11 @@ void json_format_forwarding_object(struct json_stream *response, onion_wire_name(cur->failcode)); } + /* Old forwards don't have this field */ + if (cur->forward_style != FORWARD_STYLE_UNKNOWN) + json_add_string(response, "style", + forward_style_name(cur->forward_style)); + #ifdef COMPAT_V070 /* If a forwarding doesn't have received_time it was created * before we added the tracking, do not include it here. */ diff --git a/tests/test_pay.py b/tests/test_pay.py index 29235c571..96a65f971 100644 --- a/tests/test_pay.py +++ b/tests/test_pay.py @@ -5208,6 +5208,7 @@ def test_legacyonion(node_factory, bitcoind): "destination": "035d2b1192dfba134e10e540875d366ebc8bc353d5aa766b80c090b39c3a5d885d"}) wait_for(lambda: only_one(l3.rpc.listinvoices()['invoices'])['status'] == 'paid') + assert only_one(l2.rpc.listforwards()['forwards'])['style'] == 'legacy' def test_pay_manual_exclude(node_factory, bitcoind): diff --git a/wallet/db.c b/wallet/db.c index 79576b9a6..0d086fea0 100644 --- a/wallet/db.c +++ b/wallet/db.c @@ -872,6 +872,7 @@ static struct migration dbmigrations[] = { /* Default is too big; we set to max after loading */ {SQL("ALTER TABLE channels ADD htlc_maximum_msat BIGINT DEFAULT 2100000000000000"), NULL}, {SQL("ALTER TABLE channels ADD htlc_minimum_msat BIGINT DEFAULT 0"), NULL}, + {SQL("ALTER TABLE forwarded_payments ADD forward_style INTEGER DEFAULT NULL"), NULL}, }; /** diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c index b4dc58e20..27f4b148d 100644 --- a/wallet/test/run-wallet.c +++ b/wallet/test/run-wallet.c @@ -528,7 +528,8 @@ void notify_forward_event(struct lightningd *ld UNNEEDED, const struct amount_msat *amount_out UNNEEDED, enum forward_status state UNNEEDED, enum onion_wire failcode UNNEEDED, - struct timeabs *resolved_time UNNEEDED) + struct timeabs *resolved_time UNNEEDED, + enum forward_style forward_style UNNEEDED) { fprintf(stderr, "notify_forward_event called!\n"); abort(); } /* Generated stub for onchaind_funding_spent */ enum watch_result onchaind_funding_spent(struct channel *channel UNNEEDED, diff --git a/wallet/wallet.c b/wallet/wallet.c index 8a71c4113..93e52b54a 100644 --- a/wallet/wallet.c +++ b/wallet/wallet.c @@ -2534,6 +2534,7 @@ static bool wallet_stmt2htlc_in(struct channel *channel, in->status = NULL; /* FIXME: save blinding in db !*/ in->blinding = NULL; + in->payload = NULL; db_col_sha256(stmt, "payment_hash", &in->payment_hash); @@ -4228,7 +4229,8 @@ static bool wallet_forwarded_payment_update(struct wallet *w, const struct htlc_out *out, enum forward_status state, enum onion_wire failcode, - struct timeabs *resolved_time) + struct timeabs *resolved_time, + enum forward_style forward_style) { struct db_stmt *stmt; bool changed; @@ -4244,6 +4246,7 @@ static bool wallet_forwarded_payment_update(struct wallet *w, ", state=?" ", resolved_time=?" ", failcode=?" + ", forward_style=?" " WHERE in_htlc_id=?")); db_bind_amount_msat(stmt, 0, &in->msat); @@ -4268,7 +4271,12 @@ static bool wallet_forwarded_payment_update(struct wallet *w, db_bind_null(stmt, 4); } - db_bind_u64(stmt, 5, in->dbid); + /* This can happen for malformed onions, reload from db. */ + if (forward_style == FORWARD_STYLE_UNKNOWN) + db_bind_null(stmt, 5); + else + db_bind_int(stmt, 5, forward_style_in_db(forward_style)); + db_bind_u64(stmt, 6, in->dbid); db_exec_prepared_v2(stmt); changed = db_count_changes(stmt) != 0; tal_free(stmt); @@ -4277,6 +4285,7 @@ static bool wallet_forwarded_payment_update(struct wallet *w, } void wallet_forwarded_payment_add(struct wallet *w, const struct htlc_in *in, + enum forward_style forward_style, const struct short_channel_id *scid_out, const struct htlc_out *out, enum forward_status state, @@ -4292,7 +4301,7 @@ void wallet_forwarded_payment_add(struct wallet *w, const struct htlc_in *in, resolved_time = NULL; } - if (wallet_forwarded_payment_update(w, in, out, state, failcode, resolved_time)) + if (wallet_forwarded_payment_update(w, in, out, state, failcode, resolved_time, forward_style)) goto notify; stmt = db_prepare_v2(w->db, @@ -4307,7 +4316,8 @@ void wallet_forwarded_payment_add(struct wallet *w, const struct htlc_in *in, ", received_time" ", resolved_time" ", failcode" - ") VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?);")); + ", forward_style" + ") VALUES (?, ?, ?, ?, ?, ?, ?, ?, ?, ?, ?);")); db_bind_u64(stmt, 0, in->dbid); if (out) { @@ -4341,12 +4351,17 @@ void wallet_forwarded_payment_add(struct wallet *w, const struct htlc_in *in, } else { db_bind_null(stmt, 9); } + /* This can happen for malformed onions, reload from db! */ + if (forward_style == FORWARD_STYLE_UNKNOWN) + db_bind_null(stmt, 10); + else + db_bind_int(stmt, 10, forward_style_in_db(forward_style)); db_exec_prepared_v2(take(stmt)); notify: notify_forward_event(w->ld, in, scid_out, out ? &out->msat : NULL, - state, failcode, resolved_time); + state, failcode, resolved_time, forward_style); } struct amount_msat wallet_total_forward_fees(struct wallet *w) @@ -4414,6 +4429,7 @@ const struct forwarding *wallet_forwarded_payments_get(struct wallet *w, ", f.received_time" ", f.resolved_time" ", f.failcode " + ", f.forward_style " "FROM forwarded_payments f " "LEFT JOIN channel_htlcs hin ON (f.in_htlc_id = hin.id) " "WHERE (1 = ? OR f.state = ?) AND " @@ -4511,6 +4527,12 @@ const struct forwarding *wallet_forwarded_payments_get(struct wallet *w, } else { cur->failcode = 0; } + if (db_col_is_null(stmt, "f.forward_style")) { + cur->forward_style = FORWARD_STYLE_UNKNOWN; + } else { + cur->forward_style + = forward_style_in_db(db_col_int(stmt, "f.forward_style")); + } } tal_free(stmt); return results; diff --git a/wallet/wallet.h b/wallet/wallet.h index 6eea26b67..2c179b465 100644 --- a/wallet/wallet.h +++ b/wallet/wallet.h @@ -3,6 +3,7 @@ #include "config.h" #include "db.h" +#include #include #include #include @@ -159,6 +160,45 @@ static inline const char* forward_status_name(enum forward_status status) bool string_to_forward_status(const char *status_str, enum forward_status *status); +/* /!\ This is a DB ENUM, please do not change the numbering of any + * already defined elements (adding is ok) /!\ */ +enum forward_style { + FORWARD_STYLE_LEGACY = ONION_V0_PAYLOAD, + FORWARD_STYLE_TLV = ONION_TLV_PAYLOAD, + FORWARD_STYLE_UNKNOWN = 2, /* Not actually in db, safe to renumber! */ +}; + +/* Wrapper to ensure types don't change, and we don't insert/extract + * invalid ones from db */ +static inline enum forward_style forward_style_in_db(enum forward_style o) +{ + switch (o) { + case FORWARD_STYLE_LEGACY: + BUILD_ASSERT(FORWARD_STYLE_LEGACY == 0); + return o; + case FORWARD_STYLE_TLV: + BUILD_ASSERT(FORWARD_STYLE_TLV == 1); + return o; + case FORWARD_STYLE_UNKNOWN: + /* Not recorded in DB! */ + break; + } + fatal("%s: %u is invalid", __func__, o); +} + +static inline const char *forward_style_name(enum forward_style style) +{ + switch (style) { + case FORWARD_STYLE_UNKNOWN: + return "UNKNOWN"; + case FORWARD_STYLE_TLV: + return "tlv"; + case FORWARD_STYLE_LEGACY: + return "legacy"; + } + abort(); +} + /* DB wrapper to check htlc_state */ static inline enum htlc_state htlc_state_in_db(enum htlc_state s) { @@ -234,6 +274,7 @@ struct forwarding { struct short_channel_id channel_in, channel_out; struct amount_msat msat_in, msat_out, fee; struct sha256 *payment_hash; + enum forward_style forward_style; enum forward_status status; enum onion_wire failcode; struct timeabs received_time; @@ -1308,6 +1349,7 @@ struct channeltx *wallet_channeltxs_get(struct wallet *w, const tal_t *ctx, * Add of update a forwarded_payment */ void wallet_forwarded_payment_add(struct wallet *w, const struct htlc_in *in, + enum forward_style forward_style, const struct short_channel_id *scid_out, const struct htlc_out *out, enum forward_status state,