Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
cdecker opened this issue Feb 11, 2020 · 5 comments · May be fixed by #7967
Open

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

cdecker opened this issue Feb 11, 2020 · 5 comments · May be fixed by #7967
Labels
cleanup good first issue good for onboarding

Comments

@cdecker
Copy link
Member

cdecker commented Feb 11, 2020

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
Copy link

@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
Copy link
Contributor

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

@BitcoinJiuJitsu
Copy link

@NishantBansal2003
Copy link

NishantBansal2003 commented Dec 23, 2024

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
Copy link
Member Author

cdecker commented Jan 3, 2025

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 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup good first issue good for onboarding
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants