lightningd: add "style" to listforwards.

Suggested-by: @t-bast
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Changelog-Added: JSON-RPC: `listforwards` has new entry `style`, currently "legacy" or "tlv".

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: <https://github.com/ElementsProject/lightning>
 
-[comment]: # ( SHA256STAMP:abfaaa00817734d8acb77d02d7c024112c90605a8f93a134971a617ab4d383f9)
+[comment]: # ( SHA256STAMP:131410f052b8a1845c8d3c7eb2d48df0fc7638e7d26817f56863815be86d8f1e)

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": {},

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();
 

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));
 }

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);

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. */

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):

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},
 };
 
 /**

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,

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;

wallet/wallet.h

@@ -3,6 +3,7 @@
 
 #include "config.h"
 #include "db.h"
+#include <common/onion.h>
 #include <common/penalty_base.h>
 #include <common/utxo.h>
 #include <common/wallet.h>
@@ -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,