cleanup: Use the `-rpcwait` when waiting for bitcoind to warm up

[cdecker]

We currently use a rather lengthy loop to wait for bitcoind to warm up and be ready to serve RPC calls:

lightning/plugins/bcli.c

Lines 710 to 749 in 44d408c

	for (;;) {
	child = pipecmdarr(NULL, &from, &from, cast_const2(char **,cmd));
	if (child < 0) {
	if (errno == ENOENT)
	bitcoind_failure(p, "bitcoin-cli not found. Is bitcoin-cli "
	"(part of Bitcoin Core) available in your PATH?");
	plugin_err(p, "%s exec failed: %s", cmd[0], strerror(errno));
	}
	char *output = grab_fd(cmd, from);
	while ((ret = waitpid(child, &status, 0)) < 0 && errno == EINTR);
	if (ret != child)
	bitcoind_failure(p, tal_fmt(bitcoind, "Waiting for %s: %s",
	cmd[0], strerror(errno)));
	if (!WIFEXITED(status))
	bitcoind_failure(p, tal_fmt(bitcoind, "Death of %s: signal %i",
	cmd[0], WTERMSIG(status)));
	if (WEXITSTATUS(status) == 0)
	break;
	/* bitcoin/src/rpc/protocol.h:
	* RPC_IN_WARMUP = -28, //!< Client still warming up
	*/
	if (WEXITSTATUS(status) != 28) {
	if (WEXITSTATUS(status) == 1)
	bitcoind_failure(p, "Could not connect to bitcoind using"
	" bitcoin-cli. Is bitcoind running?");
	bitcoind_failure(p, tal_fmt(bitcoind, "%s exited with code %i: %s",
	cmd[0], WEXITSTATUS(status), output));
	}
	if (!printed) {
	plugin_log(p, LOG_UNUSUAL,
	"Waiting for bitcoind to warm up...");
	printed = true;
	}
	sleep(1);
	}

This could be simplified if we just call bitcoin-cli with -rpcwait which implements the wait logic internally and allows us to skip checking the returned error (if we get an error it's not the warmup error so we can pass any error up regardless). The downside is that it doesn't allow us to print the warmup ourselves, but we could easily replace that with a single one-shot timer printing the message.

[gerceboss]

@cdecker ,Is there some community where I can discuss about the doubts I have on the code of this repository ? I would like to contribute to the lightning network community. I have started reading the book Mastering the lightning network

[vincenzopalazzo]

@gerceboss you can join discord, there is the link on the Readme

[BitcoinJiuJitsu]

Open bounty on this:
https://community.corelightning.org/c/cln-bounties/3505-cleanup-use-the-rpcwait-when-waiting-for-bitcoind-to-warm-up

[NishantBansal2003]

Hey @cdecker,
I was trying to solve the above issue, but I’m stuck at the following point:

we could easily replace that with a single one-shot timer printing the message.

I tried looking at other implementations of timers but couldn’t understand if they could solve the issue. My understanding is that we should only print the message after x seconds if bitcoind hasn’t started serving RPC calls. Could you please point me to some code examples or references for implementing a timer?
Thanks!

EDIT:
I implemented something similar to the following:

static void log_warm_up(struct plugin *p)
{
    plugin_log(p, LOG_UNUSUAL, "Waiting for bitcoind to warm up...");
}

struct timers *timer = tal(cmd, struct timers);
timers_init(timer, time_mono());
new_reltimer(timer, timer, time_from_sec(30), log_warm_up, p);

From my understanding, the new_reltimer function sets up a timer to execute log_warm_up after 30 seconds.

Will this serve the purpose of a timer to print the warm-up message only if bitcoind doesn't start serving RPC calls within 30 seconds? I looked into some parts of the code and comments, and it seems like this can be used. However, I wanted to confirm if this is the correct approach or if there's a better way to implement it.

[cdecker]

As far as I remember my concern was that unlike our polling, using the rpcwait option does not allow us to enforce a deadline of 60s, i.e., it just waits indefinitely. In LN we need to have access to the Bitcoin backend and the rpcwait option just waiting silently indefinitely is bad. We exit loudly (some refer to it as crash, but we make noise so operators are notified) if we can't talk to the Bitcoin backend. A warning logged and then a call to abort() will do just that 👍

[Raimo33]

I refactored it by using -rpcwait instead of the infinite loop as suggested.

@cdecker

my concern was that unlike our polling, using the rpcwait option does not allow us to enforce a deadline of 60s, i.e., it just waits indefinitely.

As far as I understand, this is the behavior of the previous code as well. It is a for loop that just waits indefinitely with a hard coded sleep(1). Adding a 60s deadline wouldn't be much of an issue with -rpcwait as well as without. In the meantime I propose the following:

static void wait_and_check_bitcoind(struct plugin *p)
{
	int in, from, status, ret;
	pid_t child;
	const char **cmd = gather_args(bitcoind, "-rpcwait", "getnetworkinfo", NULL);
	char *output = NULL;

	child = pipecmdarr(&in, &from, &from, cast_const2(char **, cmd));

	if (bitcoind->rpcpass)
		write_all(in, bitcoind->rpcpass, strlen(bitcoind->rpcpass));
	close(in);

	if (child < 0) {
		if (errno == ENOENT)
			bitcoind_failure(p, "bitcoin-cli not found. Is bitcoin-cli "
							"(part of Bitcoin Core) available in your PATH?");
		plugin_err(p, "%s exec failed: %s", cmd[0], strerror(errno));
	}

	output = grab_fd(cmd, from);

	while ((ret = waitpid(child, &status, 0)) < 0 && errno == EINTR);
	if (ret != child)
		bitcoind_failure(p, tal_fmt(bitcoind, "Waiting for %s: %s",
							cmd[0], strerror(errno)));
	if (!WIFEXITED(status))
		bitcoind_failure(p, tal_fmt(bitcoind, "Death of %s: signal %i",
							cmd[0], WTERMSIG(status)));

	if (WEXITSTATUS(status) != 0) {
		if (WEXITSTATUS(status) == 1)
			bitcoind_failure(p, "Could not connect to bitcoind using bitcoin-cli. Is bitcoind running?");
		bitcoind_failure(p, tal_fmt(bitcoind, "%s exited with code %i: %s",
					    cmd[0], WEXITSTATUS(status), output));
	}

	parse_getnetworkinfo_result(p, output);
	tal_free(cmd);
}```