Bitcoin-Qt: ask user to rebuild database in case of db corruption

sipa commented at 11:31 pm on January 22, 2013: member

As the current “restart with -reindex” is fine for bitcoind, but very confusing for GUI users.

Diapolo commented at 7:29 am on January 23, 2013: none

What is the flow you think of? Something like Corrupted block database detected. Do you want to restart the client to rebuild the database?. User clicks yes, client restarts with -reindex added to the command line. For such a thing to work we would also need to re-use the current command-line parameters. And the question comes from the core, I guess if there is no need to send back the users choice to the core this could be done by extending our UI interface in a form to allow a dialogbox where we process the user choice inside the GUI code.

laanwj commented at 7:35 am on January 23, 2013: member

I don’t think a client restart hack is even needed. When it detects the corrupted block database it is still in the startup phase, so the reply from the UI could trigger a rebuild immediately.

What is likely needed is a way for the core to “ask” the UI something and get back a yes/no reply.

In a more general sense, we already have this for askFee
But as this happens before the UI event loop/thread is running, it is even simpler; no need for async Qt signals. Just extend the ui interface with another init* method that shows a standard dialog and returns a the clicked button value…

If a client restart is really needed (ie, to start the database code with a clean slate), make it a dialog like “Corrupted block database detected. Do you want to rebuild the database?”. If “Yes” is clicked a flag will be set, and another dialog will be shown “On next client start, a rebuild of the database will be done. Please restart the client.” as is currently done with the wallet encryption as well.

sipa commented at 9:38 am on January 23, 2013: member

No, a client restart isn’t necessary. I would think of something like “It looks your block database is corrupted. Do you want to rebuild it now? If no, the client will exit. [Yes] [No]”. There’s some code needed in init.cpp to go back to a previous stage (basically a loop starting before the initialization of pblocktree, until after the VerifyDB call), but that’s trivial to do if some method for asking a yes/no question via the GUI is available.

laanwj commented at 11:47 am on October 24, 2013: member

I think this was implemented? If so, please close the issue :)

sipa commented at 11:48 am on October 24, 2013: member

Only for some types of corruption (the ones detected at startup in the consistency check).

Diapolo commented at 12:25 pm on October 24, 2013: none

Can someone please review #3136, as this is related. @laanwj I also think we have a problem with the ThreadSafeMessageBox function, as I’m not sure the way we implemented getting the user choice back is supported (bool pointer). As

The return value of the member function call is placed in ret. If the invocation is asynchronous, the return value cannot be evaluated.

, see https://qt-project.org/doc/qt-4.8/qmetaobject.html#invokeMethod.

Diapolo commented at 10:30 am on October 28, 2013: none

@laanwj Ping, can you take a quick look at the Qt docs to verify, if I’m correct here…

laanwj commented at 10:56 am on October 28, 2013: member

@Diapolo if you look at the ThreadSafeMessageBox source https://github.com/bitcoin/bitcoin/blob/master/src/qt/bitcoin.cpp#L56, you see that we use GUIUtil::blockingGUIThreadConnection() in case of a modal message, which never returns one of the asynchronous types.

Diapolo commented at 11:52 am on October 28, 2013: none

@laanwj You are right, we are currently only using modal messages (because all uiInterface.ThreadSafeMessageBox() calls use MSG_ERROR or MSG_WARNING).

So we have 2 types used for the connection left: if(QThread::currentThread() != qApp->thread()) -> Qt::BlockingQueuedConnection; else -> Qt::DirectConnection;

Perhaps our ThreadSafeMessageBox should always be modal then to ensure we can always rely on bool ret;?

laanwj commented at 12:06 pm on October 28, 2013: member

It only needs to be modal if a return value is desired. Otherwise it’s fine (better, even) to have it asynchronous.

Diapolo commented at 12:19 pm on October 28, 2013: none

At least no problem with our Qt code is causing my crash here, you convinced me :).

laanwj added the label Data corruption on Feb 9, 2016

laanwj removed the label Priority Medium on Apr 25, 2017

sidhujag referenced this in commit 2987897bd4 on Aug 9, 2018

MarcoFalke commented at 8:49 pm on August 7, 2019: member

You mean instead of crashing during validation, the gui will ask you to reindex instead of the “A fatal internal error occurred” message?

promag commented at 10:44 pm on November 11, 2019: member

Please assign this to me.

hebasto commented at 8:58 am on October 3, 2021: member

What is the actual status of this issue?

MarcoFalke commented at 8:49 am on October 4, 2021: member

Closing for now. I think a reindex might not be the best choice in case of corruption anyway. Better to buy stable high-end hardware.

MarcoFalke closed this on Oct 4, 2021

DrahtBot locked this on Oct 30, 2022

Bitcoin-Qt: ask user to rebuild database in case of db corruption #2202