Wait for bitcoind if it's gone backwards, don't abort. #7342
Conversation
rustyrussell
force-pushed
the
guilt/bitcoind-backwards-wait
branch
2 times, most recently
from
42b5f99
to
b196d57
Compare
There was a problem hiding this comment.
Mh What is the main reason that makes #6924 difficult to implement with the current mainline?
I do this in folgore all the time by using last_height given by the getchaininfo RPC In #6924 and blocking till the bitcoind is not fully sync.
Do you think that we need all this pile of change while we are under rc?
The code is ridiculously fragile. As I said, every time we tried, we broke something else.
That only works because we do not time you out, but it's silly: we should handle this internally.
No, that's why it's set to 24.08 milestone? |
rustyrussell
force-pushed
the
guilt/bitcoind-backwards-wait
branch
from
b196d57
to
cb319e2
Compare
I agree
Ah sorry I read it wrong! |
rustyrussell
force-pushed
the
guilt/bitcoind-backwards-wait
branch
from
cb319e2
to
cfdc3b4
Compare
ShahanaFarooqui
force-pushed
the
guilt/bitcoind-backwards-wait
branch
from
cfdc3b4
to
b5b834a
Compare
|
ACK b5b834a |
rustyrussell
force-pushed
the
guilt/bitcoind-backwards-wait
branch
3 times, most recently
from
8dc5386
to
7ab549d
Compare
Several callers stash this, but we have it, so make it explicit: here's the block you asked for. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
The current code is confusing: there are polling loops, but we wait for them to run once. Be explicit: make the calls once, then start the loops in begin_topology. This removes various chain_topology struct members which only exist for startup. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
It was a weird arbitrary bool passed through from the caller, which we no longer need. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Fixes: ElementsProject#6924 Changelog-Changed: lightningd: we wait for bitcoind if it has somehow gone backwards (as long as header height is still ok). Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Handling half in main() and half here was a mess. And the name "max_blockheight" was poor: it was the max in the db, or UINT32_MAX, but then we changed it depending on what height we wanted to start at. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
checkchain_timer is run by chaintopology, so why have the pointer in bitcoind? And since we free the timers, we don't need them to self-disable by checking the stopped flag. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
That way if you free the context, you free the call (i.e. the callback will not be called). Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
…ile shutting down. I've never seen this, but the race looks possible so we should close it. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
Found by very slow CI: the two io_breaks() can combine into one, so we block waiting for the second initialization which never happens. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
rustyrussell
force-pushed
the
guilt/bitcoind-backwards-wait
branch
from
7ab549d
to
2e7f8e2
Compare
@ShahanaFarooqui and I spent about 2-3 hours trying to fix this, but the code was fragile and we kept breaking existing cases.
So instead, I started by reworking the code to make it simple, then I change it. The result is much neater, and will serve us well for any future changes.
Fixes: #6924