From d85251ac6cdfc1649f122678c93390acb01dea2f Mon Sep 17 00:00:00 2001
From: Rusty Russell <rusty@rustcorp.com.au>
Date: Tue, 9 Oct 2018 19:24:52 +1030
Subject: [PATCH] db: fix up HTLCs which are missing failure information.

We don't save them to the database, so fix things up as we load them.

Next patch will actually save them into the db, and this will become
COMPAT code.

Also: call htlc_in_check() with NULL on db load, as otherwise it aborts
internally.

Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
---
 lightningd/peer_htlcs.c  | 48 ++++++++++++++++++++++++++++++++++++++++
 tests/test_connection.py |  1 -
 wallet/wallet.c          | 31 +++++++++++++++++++++++++-
 3 files changed, 78 insertions(+), 2 deletions(-)

diff --git a/lightningd/peer_htlcs.c b/lightningd/peer_htlcs.c
index 01a066376..7d8a7603f 100644
--- a/lightningd/peer_htlcs.c
+++ b/lightningd/peer_htlcs.c
@@ -1679,6 +1679,52 @@ void htlcs_notify_new_block(struct lightningd *ld, u32 height)
 	} while (removed);
 }
 
+static void fixup_hout(struct lightningd *ld, struct htlc_out *hout)
+{
+	const char *fix;
+
+	/* We didn't save HTLC failure information to the database.  So when
+	 * busy nodes restarted (y'know, our most important users!) they would
+	 * find themselves with missing fields.
+	 *
+	 * Fortunately, most of the network is honest: re-sending an old HTLC
+	 * just causes failure (though we assert() when we try to push the
+	 * failure to the incoming HTLC which has already succeeded!).
+	 */
+
+	/* We care about HTLCs being removed only, not those being added. */
+	if (hout->hstate < RCVD_REMOVE_HTLC)
+		return;
+
+	/* Successful ones are fine. */
+	if (hout->preimage)
+		return;
+
+	/* Failed ones (only happens after db fixed!) OK. */
+	if (hout->failcode || hout->failuremsg)
+		return;
+
+	/* payment_preimage for HTLC in *was* stored, so look for that. */
+	if (hout->in && hout->in->preimage) {
+		hout->preimage = tal_dup(hout, struct preimage,
+					 hout->in->preimage);
+		fix = "restoring preimage from incoming HTLC";
+	} else {
+		hout->failcode = WIRE_TEMPORARY_CHANNEL_FAILURE;
+		fix = "subsituting temporary channel failure";
+	}
+
+	log_broken(ld->log, "HTLC #%"PRIu64" (%s) "
+		   " for amount %"PRIu64
+		   " to %s"
+		   " is missing a resolution: %s.",
+		   hout->key.id, htlc_state_name(hout->hstate),
+		   hout->msatoshi,
+		   type_to_string(tmpctx, struct pubkey,
+				  &hout->key.channel->peer->id),
+		   fix);
+}
+
 /**
  * htlcs_reconnect -- Link outgoing HTLCs to their origins after initial db load
  *
@@ -1728,6 +1774,8 @@ void htlcs_reconnect(struct lightningd *ld,
 			      hout->origin_htlc_id, hout->dbid);
 #endif
 		}
+		fixup_hout(ld, hout);
+
 	}
 }
 
diff --git a/tests/test_connection.py b/tests/test_connection.py
index cf4c69f0b..debbe282a 100644
--- a/tests/test_connection.py
+++ b/tests/test_connection.py
@@ -1376,7 +1376,6 @@ def test_fulfill_incoming_first(node_factory, bitcoind):
     l3.daemon.wait_for_log('onchaind complete, forgetting peer')
 
 
-@pytest.mark.xfail(strict=True)
 def test_restart_many_payments(node_factory):
     l1 = node_factory.get_node(may_reconnect=True)
 
diff --git a/wallet/wallet.c b/wallet/wallet.c
index a7353694c..e45fffb95 100644
--- a/wallet/wallet.c
+++ b/wallet/wallet.c
@@ -1346,6 +1346,34 @@ static bool wallet_stmt2htlc_out(struct channel *channel,
 	return ok;
 }
 
+/* We didn't used to save failcore, failuremsg... */
+static void fixup_hin(struct wallet *wallet, struct htlc_in *hin)
+{
+	/* We care about HTLCs being removed only, not those being added. */
+	if (hin->hstate < SENT_REMOVE_HTLC)
+		return;
+
+	/* Successful ones are fine. */
+	if (hin->preimage)
+		return;
+
+	/* Failed ones (only happens after db fixed!) OK. */
+	if (hin->failcode || hin->failuremsg)
+		return;
+
+	hin->failcode = WIRE_TEMPORARY_CHANNEL_FAILURE;
+
+	log_broken(wallet->log, "HTLC #%"PRIu64" (%s) "
+		   " for amount %"PRIu64
+		   " from %s"
+		   " is missing a resolution:"
+		   " subsituting temporary channel failure",
+		   hin->key.id, htlc_state_name(hin->hstate),
+		   hin->msatoshi,
+		   type_to_string(tmpctx, struct pubkey,
+				  &hin->key.channel->peer->id));
+}
+
 bool wallet_htlcs_load_for_channel(struct wallet *wallet,
 				   struct channel *chan,
 				   struct htlc_in_map *htlcs_in,
@@ -1371,7 +1399,8 @@ bool wallet_htlcs_load_for_channel(struct wallet *wallet,
 		struct htlc_in *in = tal(chan, struct htlc_in);
 		ok &= wallet_stmt2htlc_in(chan, stmt, in);
 		connect_htlc_in(htlcs_in, in);
-		ok &=  htlc_in_check(in, "wallet_htlcs_load") != NULL;
+		fixup_hin(wallet, in);
+		ok &= htlc_in_check(in, NULL) != NULL;
 		incount++;
 	}
 	db_stmt_done(stmt);