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

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

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)
 

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