db: put scids in forwards even if we didn't actually send.

If the peer is not connected, or other error which means we don't actually create an outgoing HTLC, we don't record the short_channel_id. This is unhelpful! Pass the scid down to the wallet code, and explicitly hand the scid and amount down to the notification code rather than handing it the htlc_out (which it doesn't need). Changelog-Changed: JSON API: `listforwards` now shows `out_channel` even if we couldn't forward. Signed-off-by: Rusty Russell <[email protected]>
rsouvik · Feb 13, 2020 · fc4d01c · fc4d01c
1 parent 5e2053f
commit fc4d01c
Show file tree

Hide file tree

Showing 7 changed files with 41 additions and 19 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -26,6 +26,7 @@ This release named by Vasil Dimov @vasild.
 
  - JSON API: The hooks `db_write`, `invoice_payment`, and `rpc_command` now accept `{ "result": "continue" }` to mean "do default action". ([3475](https://github.com/ElementsProject/lightning/pull/3475))
  - Plugin: Multiple plugins can now register for the htlc_accepted hook. ([3489](https://github.com/ElementsProject/lightning/pull/3489))
+ - JSON API: `listforwards` now shows `out_channel` even if we couldn't forward.
  - JSON API: `funchannel_cancel`: only the opener of a fundchannel can cancel the channel open ([3336](https://github.com/ElementsProject/lightning/pull/3336))
  - JSON API: `sendpay` optional `msatoshi` param for non-MPP (if set), must be the exact amount sent to the final recipient. ([3470](https://github.com/ElementsProject/lightning/pull/3470))
  - JSON API: `waitinvoice` now returns error code 903 to designate that the invoice expired during wait, instead of the previous -2 ([3441](https://github.com/ElementsProject/lightning/pull/3441))

diff --git a/lightningd/notification.c b/lightningd/notification.c
@@ -176,7 +176,8 @@ void notify_channel_opened(struct lightningd *ld, struct node_id *node_id,
 
 static void forward_event_notification_serialize(struct json_stream *stream,
 						 const struct htlc_in *in,
-						 const struct htlc_out *out,
+						 const struct short_channel_id *scid_out,
+						 const struct amount_msat *amount_out,
 						 enum forward_status state,
 						 enum onion_type failcode,
 						 struct timeabs *resolved_time)
@@ -186,10 +187,16 @@ static void forward_event_notification_serialize(struct json_stream *stream,
 	struct forwarding *cur = tal(tmpctx, struct forwarding);
 	cur->channel_in = *in->key.channel->scid;
 	cur->msat_in = in->msat;
-	if (out) {
-		cur->channel_out = *out->key.channel->scid;
-		cur->msat_out = out->msat;
-		assert(amount_msat_sub(&cur->fee, in->msat, out->msat));
+	if (scid_out) {
+		cur->channel_out = *scid_out;
+		if (amount_out) {
+			cur->msat_out = *amount_out;
+			assert(amount_msat_sub(&cur->fee,
+					       in->msat, *amount_out));
+		} else {
+			cur->msat_out = AMOUNT_MSAT(0);
+			cur->fee = AMOUNT_MSAT(0);
+		}
 	} else {
 		cur->channel_out.u64 = 0;
 		cur->msat_out = AMOUNT_MSAT(0);
@@ -209,21 +216,23 @@ REGISTER_NOTIFICATION(forward_event,
 
 void notify_forward_event(struct lightningd *ld,
 			  const struct htlc_in *in,
-			  const struct htlc_out *out,
+			  const struct short_channel_id *scid_out,
+			  const struct amount_msat *amount_out,
 			  enum forward_status state,
 			  enum onion_type failcode,
 			  struct timeabs *resolved_time)
 {
 	void (*serialize)(struct json_stream *,
 			  const struct htlc_in *,
-			  const struct htlc_out *,
+			  const struct short_channel_id *,
+			  const struct amount_msat *,
 			  enum forward_status,
 			  enum onion_type,
 			  struct timeabs *) = forward_event_notification_gen.serialize;
 
 	struct jsonrpc_notification *n
 		= jsonrpc_notification_start(NULL, forward_event_notification_gen.topic);
-	serialize(n->stream, in, out, state, failcode, resolved_time);
+	serialize(n->stream, in, scid_out, amount_out, state, failcode, resolved_time);
 	jsonrpc_notification_end(n);
 	plugins_notify(ld->plugins, take(n));
 }

diff --git a/lightningd/notification.h b/lightningd/notification.h
@@ -53,7 +53,10 @@ void notify_channel_opened(struct lightningd *ld, struct node_id *node_id,
 
 void notify_forward_event(struct lightningd *ld,
 			  const struct htlc_in *in,
-			  const struct htlc_out *out,
+			  /* May be NULL if we don't know. */
+			  const struct short_channel_id *scid_out,
+			  /* May be NULL. */
+			  const struct amount_msat *amount_out,
 			  enum forward_status state,
 			  enum onion_type failcode,
 			  struct timeabs *resolved_time);

diff --git a/lightningd/peer_htlcs.c b/lightningd/peer_htlcs.c
@@ -404,7 +404,7 @@ 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,
+					 hout->in, NULL, NULL,
 					 FORWARD_LOCAL_FAILED,
 					 failure_code);
 		}
@@ -514,7 +514,7 @@ static void forward_htlc(struct htlc_in *hin,
 	if (!next || !next->scid) {
 		local_fail_htlc(hin, WIRE_UNKNOWN_NEXT_PEER, NULL);
 		wallet_forwarded_payment_add(hin->key.channel->peer->ld->wallet,
-					 hin, NULL,
+					 hin, next ? next->scid : NULL, NULL,
 					 FORWARD_LOCAL_FAILED,
 					 hin->failcode);
 		return;
@@ -603,7 +603,7 @@ static void forward_htlc(struct htlc_in *hin,
 fail:
 	local_fail_htlc(hin, failcode, next->scid);
 	wallet_forwarded_payment_add(ld->wallet,
-				 hin, hout,
+				 hin, next->scid, hout,
 				 FORWARD_LOCAL_FAILED,
 				 hin->failcode);
 }
@@ -637,7 +637,7 @@ static void channel_resolve_reply(struct subd *gossip, const u8 *msg,
 	if (!peer_id) {
 		local_fail_htlc(gr->hin, WIRE_UNKNOWN_NEXT_PEER, NULL);
 		wallet_forwarded_payment_add(gr->hin->key.channel->peer->ld->wallet,
-					 gr->hin, NULL,
+					 gr->hin, &gr->next_channel, NULL,
 					 FORWARD_LOCAL_FAILED,
 					 gr->hin->failcode);
 		tal_free(gr);
@@ -1009,7 +1009,8 @@ static void fulfill_our_htlc_out(struct channel *channel, struct htlc_out *hout,
 		payment_succeeded(ld, hout, preimage);
 	else if (hout->in) {
 		fulfill_htlc(hout->in, preimage);
-		wallet_forwarded_payment_add(ld->wallet, hout->in, hout,
+		wallet_forwarded_payment_add(ld->wallet, hout->in,
+					     hout->key.channel->scid, hout,
 					     FORWARD_SETTLED, 0);
 	}
 }
@@ -1102,6 +1103,7 @@ static bool peer_failed_our_htlc(struct channel *channel,
 
 	if (hout->in)
 		wallet_forwarded_payment_add(ld->wallet, hout->in,
+					     channel->scid,
 					 hout, FORWARD_FAILED, hout->failcode);
 
 	return true;
@@ -1141,7 +1143,7 @@ void onchain_failed_our_htlc(const struct channel *channel,
 		local_fail_htlc(hout->in, WIRE_PERMANENT_CHANNEL_FAILURE,
 				hout->key.channel->scid);
 		wallet_forwarded_payment_add(hout->key.channel->peer->ld->wallet,
-					 hout->in, hout,
+					 hout->in, channel->scid, hout,
 					 FORWARD_LOCAL_FAILED,
 					 hout->failcode);
 	}
@@ -1266,7 +1268,8 @@ static bool update_out_htlc(struct channel *channel,
 						      hout->msat);
 
 		if (hout->in) {
-			wallet_forwarded_payment_add(ld->wallet, hout->in, hout,
+			wallet_forwarded_payment_add(ld->wallet, hout->in,
+						     channel->scid, hout,
 						     FORWARD_OFFERED, 0);
 		}
 
@@ -1760,7 +1763,7 @@ void peer_got_revoke(struct channel *channel, const u8 *msg)
 		local_fail_htlc(hin, failcodes[i], NULL);
 		// in fact, now we don't know if this htlc is a forward or localpay!
 		wallet_forwarded_payment_add(ld->wallet,
-					 hin, NULL,
+					 hin, NULL, NULL,
 					 FORWARD_LOCAL_FAILED,
 					 hin->failcode);
 	}

diff --git a/wallet/test/run-wallet.c b/wallet/test/run-wallet.c
@@ -388,7 +388,10 @@ void notify_disconnect(struct lightningd *ld UNNEEDED, struct node_id *nodeid UN
 /* Generated stub for notify_forward_event */
 void notify_forward_event(struct lightningd *ld UNNEEDED,
 			  const struct htlc_in *in UNNEEDED,
-			  const struct htlc_out *out UNNEEDED,
+			  /* May be NULL if we don't know. */
+			  const struct short_channel_id *scid_out UNNEEDED,
+			  /* May be NULL. */
+			  const struct amount_msat *amount_out UNNEEDED,
 			  enum forward_status state UNNEEDED,
 			  enum onion_type failcode UNNEEDED,
 			  struct timeabs *resolved_time UNNEEDED)

diff --git a/wallet/wallet.c b/wallet/wallet.c
@@ -3365,6 +3365,7 @@ static bool wallet_forwarded_payment_update(struct wallet *w,
 }
 
 void wallet_forwarded_payment_add(struct wallet *w, const struct htlc_in *in,
+				  const struct short_channel_id *scid_out,
 				  const struct htlc_out *out,
 				  enum forward_status state,
 				  enum onion_type failcode)
@@ -3432,7 +3433,8 @@ void wallet_forwarded_payment_add(struct wallet *w, const struct htlc_in *in,
 	db_exec_prepared_v2(take(stmt));
 
 notify:
-	notify_forward_event(w->ld, in, out, state, failcode, resolved_time);
+	notify_forward_event(w->ld, in, scid_out, out ? &out->msat : NULL,
+			     state, failcode, resolved_time);
 }
 
 struct amount_msat wallet_total_forward_fees(struct wallet *w)

diff --git a/wallet/wallet.h b/wallet/wallet.h
@@ -1173,6 +1173,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,
+				  const struct short_channel_id *scid_out,
 				  const struct htlc_out *out,
 				  enum forward_status state,
 				  enum onion_type failcode);