lightningd: delay reprocessing of incoming htlcs at startup until plugins ready.

Fixes: #2923
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>

lightningd/lightningd.c

@@ -632,6 +632,7 @@ int main(int argc, char *argv[])
 	int stop_fd;
 	struct timers *timers;
 	const char *stop_response;
+	struct htlc_in_map *unprocessed_htlcs;
 
 	/*~ What happens in strange locales should stay there. */
 	setup_locale();
@@ -769,7 +770,7 @@ int main(int argc, char *argv[])
 	 *  topology is initialized since some decisions rely on being able to
 	 *  know the blockheight. */
 	db_begin_transaction(ld->wallet->db);
-	load_channels_from_wallet(ld);
+	unprocessed_htlcs = load_channels_from_wallet(ld);
 	db_commit_transaction(ld->wallet->db);
 
 	/*~ Create RPC socket: now lightning-cli can send us JSON RPC commands
@@ -780,6 +781,11 @@ int main(int argc, char *argv[])
 	 * can start talking to us. */
 	plugins_config(ld->plugins);
 
+	/*~ Process any HTLCs we were in the middle of when we exited, now
+	 * that plugins (who might want to know via htlc_accepted hook) are
+	 * active. */
+	htlcs_resubmit(ld, unprocessed_htlcs);
+
 	/*~ Activate connect daemon.  Needs to be after the initialization of
 	 * chaintopology, otherwise peers may connect and ask for
 	 * uninitialized data. */

lightningd/peer_control.c

@@ -1419,7 +1419,7 @@ void activate_peers(struct lightningd *ld)
 }
 
 /* Pull peers, channels and HTLCs from db, and wire them up. */
-void load_channels_from_wallet(struct lightningd *ld)
+struct htlc_in_map *load_channels_from_wallet(struct lightningd *ld)
 {
 	struct peer *peer;
 
@@ -1442,7 +1442,7 @@ void load_channels_from_wallet(struct lightningd *ld)
 	}
 
 	/* Now connect HTLC pointers together */
-	htlcs_reconnect(ld, &ld->htlcs_in, &ld->htlcs_out);
+	return htlcs_reconnect(ld, &ld->htlcs_in, &ld->htlcs_out);
 }
 
 static struct command_result *json_disconnect(struct command *cmd,

lightningd/peer_control.h

@@ -90,8 +90,9 @@ void drop_to_chain(struct lightningd *ld, struct channel *channel, bool cooperat
 
 void channel_watch_funding(struct lightningd *ld, struct channel *channel);
 
-/* Pull peers, channels and HTLCs from db, and wire them up. */
-void load_channels_from_wallet(struct lightningd *ld);
+/* Pull peers, channels and HTLCs from db, and wire them up.
+ * Returns any HTLCs we have to resubmit via htlcs_resubmit. */
+struct htlc_in_map *load_channels_from_wallet(struct lightningd *ld);
 
 #if DEVELOPER
 void peer_dev_memleak(struct command *cmd);

lightningd/peer_htlcs.c

@@ -2018,17 +2018,18 @@ static void fixup_hout(struct lightningd *ld, struct htlc_out *hout)
  * For each outgoing HTLC find the incoming HTLC that triggered it. If
  * we are the origin of the transfer then we cannot resolve the
  * incoming HTLC in which case we just leave it `NULL`.
+ *
+ * Returns a map of any htlcs we need to retry.
  */
-void htlcs_reconnect(struct lightningd *ld,
-		     struct htlc_in_map *htlcs_in,
-		     struct htlc_out_map *htlcs_out)
+struct htlc_in_map *htlcs_reconnect(struct lightningd *ld,
+				    struct htlc_in_map *htlcs_in,
+				    struct htlc_out_map *htlcs_out)
 {
 	struct htlc_in_map_iter ini;
 	struct htlc_out_map_iter outi;
 	struct htlc_in *hin;
 	struct htlc_out *hout;
-	struct htlc_in_map unprocessed;
-	enum onion_type failcode COMPILER_WANTS_INIT("gcc7.4.0 bad, 8.3 OK");
+	struct htlc_in_map *unprocessed = tal(NULL, struct htlc_in_map);
 
 	/* Any HTLCs which happened to be incoming and weren't forwarded before
 	 * we shutdown/crashed: fail them now.
@@ -2036,11 +2037,11 @@ void htlcs_reconnect(struct lightningd *ld,
 	 * Note that since we do local processing synchronously, so this never
 	 * captures local payments.  But if it did, it would be a tiny corner
 	 * case. */
-	htlc_in_map_init(&unprocessed);
+	htlc_in_map_init(unprocessed);
 	for (hin = htlc_in_map_first(htlcs_in, &ini); hin;
 	     hin = htlc_in_map_next(htlcs_in, &ini)) {
 		if (hin->hstate == RCVD_ADD_ACK_REVOCATION)
-			htlc_in_map_add(&unprocessed, hin);
+			htlc_in_map_add(unprocessed, hin);
 	}
 
 	for (hout = htlc_out_map_first(htlcs_out, &outi); hout;
@@ -2081,12 +2082,22 @@ void htlcs_reconnect(struct lightningd *ld,
 #endif
 
 		if (hout->in)
-			htlc_in_map_del(&unprocessed, hout->in);
+			htlc_in_map_del(unprocessed, hout->in);
 	}
 
+	return unprocessed;
+}
+
+void htlcs_resubmit(struct lightningd *ld, struct htlc_in_map *unprocessed)
+{
+	struct htlc_in *hin;
+	struct htlc_in_map_iter ini;
+	enum onion_type failcode COMPILER_WANTS_INIT("gcc7.4.0 bad, 8.3 OK");
+
 	/* Now fail any which were stuck. */
-	for (hin = htlc_in_map_first(&unprocessed, &ini); hin;
-	     hin = htlc_in_map_next(&unprocessed, &ini)) {
+	for (hin = htlc_in_map_first(unprocessed, &ini);
+	     hin;
+	     hin = htlc_in_map_next(unprocessed, &ini)) {
 		log_unusual(hin->key.channel->log,
 			    "Replaying old unprocessed HTLC #%"PRIu64,
 			    hin->key.id);
@@ -2102,10 +2113,10 @@ void htlcs_reconnect(struct lightningd *ld,
 	}
 
 	/* Don't leak memory! */
-	htlc_in_map_clear(&unprocessed);
+	htlc_in_map_clear(unprocessed);
+	tal_free(unprocessed);
 }
 
-
 #if DEVELOPER
 static struct command_result *json_dev_ignore_htlcs(struct command *cmd,
 						    const char *buffer,

lightningd/peer_htlcs.h

@@ -63,9 +63,11 @@ void onchain_fulfilled_htlc(struct channel *channel,
 
 void htlcs_notify_new_block(struct lightningd *ld, u32 height);
 
-void htlcs_reconnect(struct lightningd *ld,
-		     struct htlc_in_map *htlcs_in,
-		     struct htlc_out_map *htlcs_out);
+struct htlc_in_map *htlcs_reconnect(struct lightningd *ld,
+				    struct htlc_in_map *htlcs_in,
+				    struct htlc_out_map *htlcs_out);
+
+void htlcs_resubmit(struct lightningd *ld, struct htlc_in_map *unprocessed);
 
 /* For HTLCs which terminate here, invoice payment calls one of these. */
 void fulfill_htlc(struct htlc_in *hin, const struct preimage *preimage);

lightningd/test/run-find_my_abspath.c

@@ -96,6 +96,9 @@ void hsm_init(struct lightningd *ld UNNEEDED)
 /* Generated stub for htlcs_notify_new_block */
 void htlcs_notify_new_block(struct lightningd *ld UNNEEDED, u32 height UNNEEDED)
 { fprintf(stderr, "htlcs_notify_new_block called!\n"); abort(); }
+/* Generated stub for htlcs_resubmit */
+void htlcs_resubmit(struct lightningd *ld UNNEEDED, struct htlc_in_map *unprocessed UNNEEDED)
+{ fprintf(stderr, "htlcs_resubmit called!\n"); abort(); }
 /* Generated stub for jsonrpc_listen */
 void jsonrpc_listen(struct jsonrpc *rpc UNNEEDED, struct lightningd *ld UNNEEDED)
 { fprintf(stderr, "jsonrpc_listen called!\n"); abort(); }
@@ -103,7 +106,7 @@ void jsonrpc_listen(struct jsonrpc *rpc UNNEEDED, struct lightningd *ld UNNEEDED
 void jsonrpc_setup(struct lightningd *ld UNNEEDED)
 { fprintf(stderr, "jsonrpc_setup called!\n"); abort(); }
 /* Generated stub for load_channels_from_wallet */
-void load_channels_from_wallet(struct lightningd *ld UNNEEDED)
+struct htlc_in_map *load_channels_from_wallet(struct lightningd *ld UNNEEDED)
 { fprintf(stderr, "load_channels_from_wallet called!\n"); abort(); }
 /* Generated stub for log_ */
 void log_(struct log *log UNNEEDED, enum log_level level UNNEEDED, bool call_notifier UNNEEDED, const char *fmt UNNEEDED, ...)

lightningd/test/run-invoice-select-inchan.c

@@ -126,9 +126,9 @@ bool htlc_is_trimmed(enum side htlc_owner UNNEEDED,
 		     enum side side UNNEEDED)
 { fprintf(stderr, "htlc_is_trimmed called!\n"); abort(); }
 /* Generated stub for htlcs_reconnect */
-void htlcs_reconnect(struct lightningd *ld UNNEEDED,
-		     struct htlc_in_map *htlcs_in UNNEEDED,
-		     struct htlc_out_map *htlcs_out UNNEEDED)
+struct htlc_in_map *htlcs_reconnect(struct lightningd *ld UNNEEDED,
+				    struct htlc_in_map *htlcs_in UNNEEDED,
+				    struct htlc_out_map *htlcs_out UNNEEDED)
 { fprintf(stderr, "htlcs_reconnect called!\n"); abort(); }
 /* Generated stub for json_add_address */
 void json_add_address(struct json_stream *response UNNEEDED, const char *fieldname UNNEEDED,

tests/test_plugin.py

@@ -466,7 +466,6 @@ def test_htlc_accepted_hook_resolve(node_factory):
     assert len(inv) == 1 and inv[0]['status'] == 'unpaid'
 
 
-@pytest.mark.xfail(strict=True)
 def test_htlc_accepted_hook_direct_restart(node_factory, executor):
     """l2 restarts while it is pondering what to do with an HTLC.
     """
@@ -492,7 +491,6 @@ def test_htlc_accepted_hook_direct_restart(node_factory, executor):
 
 
 @unittest.skipIf(not DEVELOPER, "without DEVELOPER=1, gossip v slow")
-@pytest.mark.xfail(strict=True)
 def test_htlc_accepted_hook_forward_restart(node_factory, executor):
     """l2 restarts while it is pondering what to do with an HTLC.
     """

wallet/test/run-wallet.c

@@ -1195,7 +1195,7 @@ static bool test_htlc_crud(struct lightningd *ld, const tal_t *ctx)
 		  "Failed loading HTLCs");
 	db_commit_transaction(w->db);
 
-	htlcs_reconnect(w->ld, htlcs_in, htlcs_out);
+	htlcs_resubmit(w->ld, htlcs_reconnect(w->ld, htlcs_in, htlcs_out));
 	CHECK(!wallet_err);
 
 	hin = htlc_in_map_get(htlcs_in, &in.key);