lightningd/io_loop_with_timers.c: Move mainloop to its own source file, have chaintopology use it

lightningd/Makefile

@@ -69,6 +69,7 @@ LIGHTNINGD_SRC :=				\
 	lightningd/hsm_control.c		\
 	lightningd/htlc_end.c			\
 	lightningd/invoice.c			\
+	lightningd/io_loop_with_timers.c	\
 	lightningd/json.c			\
 	lightningd/json_stream.c		\
 	lightningd/jsonrpc.c			\

lightningd/chaintopology.c

@@ -22,6 +22,7 @@
 #include <inttypes.h>
 #include <lightningd/channel_control.h>
 #include <lightningd/gossip_control.h>
+#include <lightningd/io_loop_with_timers.h>
 
 /* Mutual recursion via timer. */
 static void try_extend_tip(struct chain_topology *topo);
@@ -894,7 +895,7 @@ void setup_topology(struct chain_topology *topo,
 	start_fee_estimate(topo);
 
 	/* Once it gets initial block, it calls io_break() and we return. */
-	io_loop(NULL, NULL);
+	io_loop_with_timers(topo->ld);
 }
 
 void begin_topology(struct chain_topology *topo)

lightningd/io_loop_with_timers.c

@@ -0,0 +1,36 @@
+#include "lightningd/io_loop_with_timers.h"
+
+#include <ccan/io/io.h>
+#include <ccan/timer/timer.h>
+#include <common/timeout.h>
+#include <lightningd/lightningd.h>
+#include <wallet/db.h>
+#include <wallet/wallet.h>
+
+void *io_loop_with_timers(struct lightningd *ld)
+{
+	void *retval = NULL;
+	struct timer *expired;
+
+	while (!retval) {
+		/* ~ccan/io's io_loop() continuously calls
+		 * io_poll_lightningd() for all file descriptors registered
+		 * with it, then calls their callbacks or closes them if they
+		 * fail, as appropriate.
+		 *
+		 * It will only exit if there's an expired timer, *or* someone
+		 * calls io_break, or if there are no more file descriptors
+		 * (which never happens in our code). */
+		retval = io_loop(&ld->timers, &expired);
+
+		/*~ Notice that timers are called here in the event loop like
+		 * anything else, so there are no weird concurrency issues. */
+		if (expired) {
+			db_begin_transaction(ld->wallet->db);
+			timer_expired(ld, expired);
+			db_commit_transaction(ld->wallet->db);
+		}
+	}
+
+	return retval;
+}

lightningd/io_loop_with_timers.h

@@ -0,0 +1,10 @@
+#ifndef LIGHTNING_LIGHTNINGD_IO_LOOP_WITH_TIMERS_H
+#define LIGHTNING_LIGHTNINGD_IO_LOOP_WITH_TIMERS_H
+
+#include "config.h"
+
+struct lightningd;
+
+void *io_loop_with_timers(struct lightningd *ld);
+
+#endif /* LIGHTNING_LIGHTNINGD_IO_LOOP_WITH_TIMERS_H */

lightningd/lightningd.c

@@ -70,6 +70,7 @@
 #include <lightningd/channel_control.h>
 #include <lightningd/connect_control.h>
 #include <lightningd/invoice.h>
+#include <lightningd/io_loop_with_timers.h>
 #include <lightningd/jsonrpc.h>
 #include <lightningd/log.h>
 #include <lightningd/onchain_control.h>
@@ -810,30 +811,12 @@ int main(int argc, char *argv[])
 
 	/*~ The root of every backtrace (almost).  This is our main event
 	 *  loop. */
-	for (;;) {
-		/* ~ccan/io's io_loop() continuously calls
-		 * io_poll_lightningd() for all file descriptors registered
-		 * with it, then calls their callbacks or closes them if they
-		 * fail, as appropriate.
-		 *
-		 * It will only exit if there's an expired timer, *or* someone
-		 * calls io_break, or if there are no more file descriptors
-		 * (which never happens in our code). */
-		struct timer *expired;
-		void *v = io_loop(&ld->timers, &expired);
-
-		/*~ We use io_break(ld) to shut down. */
-		if (v == ld)
-			break;
-
-		/*~ Notice that timers are called here in the event loop like
-		 * anything else, so there are no weird concurrency issues. */
-		if (expired) {
-			db_begin_transaction(ld->wallet->db);
-			timer_expired(ld, expired);
-			db_commit_transaction(ld->wallet->db);
-		}
-	}
+	void *io_loop_ret = io_loop_with_timers(ld);
+	/*~ io_loop_with_timers will only exit if we call io_break.
+	 *  At this point in code, we should use io_break(ld) to
+	 *  shut down.
+	 */
+	assert(io_loop_ret == ld);
 
 	shutdown_subdaemons(ld);
 

lightningd/test/run-find_my_abspath.c

@@ -2,6 +2,7 @@
 int unused_main(int argc, char *argv[]);
 #include "../../common/base32.c"
 #include "../../common/wireaddr.c"
+#include "../io_loop_with_timers.c"
 #include "../lightningd.c"
 #include "../subd.c"
 #include <common/amount.h>

tests/test_misc.py

@@ -1399,3 +1399,38 @@ def test_newaddr(node_factory):
     both = l1.rpc.newaddr('all')
     assert both['p2sh-segwit'].startswith('2')
     assert both['bech32'].startswith('bcrt1')
+
+
+def test_bitcoind_fail_first(node_factory, bitcoind, executor):
+    """Make sure we handle spurious bitcoin-cli failures during startup
+
+    See [#2687](https://github.com/ElementsProject/lightning/issues/2687) for
+    details
+
+    """
+    # Do not start the lightning node since we need to instrument bitcoind
+    # first.
+    l1 = node_factory.get_node(start=False)
+
+    # Instrument bitcoind to fail some queries first.
+    def mock_fail(*args):
+        raise ValueError()
+
+    l1.daemon.rpcproxy.mock_rpc('getblock', mock_fail)
+    l1.daemon.rpcproxy.mock_rpc('estimatesmartfee', mock_fail)
+
+    f = executor.submit(l1.start)
+
+    wait_for(lambda: l1.daemon.running)
+    # Make sure it fails on the first `getblock` call (need to use `is_in_log`
+    # since the `wait_for_log` in `start` sets the offset)
+    wait_for(lambda: l1.daemon.is_in_log(
+        r'getblock [a-z0-9]* false exited with status 1'))
+    wait_for(lambda: l1.daemon.is_in_log(
+        r'estimatesmartfee 2 CONSERVATIVE exited with status 1'))
+
+    # Now unset the mock, so calls go through again
+    l1.daemon.rpcproxy.mock_rpc('getblock', None)
+    l1.daemon.rpcproxy.mock_rpc('estimatesmartfee', None)
+
+    f.result()