gui: Enable console line edit on setClientModel

promag commented at 10:02 pm on May 29, 2019: member

Make console line edit disable by default, and only enable once RPCConsole::setClientModel is called.

fanquake added the label GUI on May 29, 2019

promag commented at 10:04 pm on May 29, 2019: member

It’s not possible to run RPC commands (not even by bitcoin-cli) if the initial rescan hasn’t completed. A long term alternative is to move this rescan after the initialization? /cc @ryanofsky

ryanofsky commented at 11:24 am on May 30, 2019: member

A long term alternative is to move this rescan after the initialization? /cc @ryanofsky

Since the rescan loop doesn’t hold onto cs_main anymore, I’d expect that RPC commands could work during a rescan, and don’t understand exactly what problem causes this crash, and what problem causes bitcoin-cli not to work right now.

Longer term, I think the main thing that needs to happen is for the rescan loop to move out of the wallet into the node (src/node/rescan.cpp or someplace like that) so when multiple wallets are loaded they are all rescanned in parallel instead of sequentially one after another other. I expect the way that would work is that during loading, each wallet would asynchronously request a rescan from the node, passing it a locator. Then after all the wallets are loaded, the node would do a single rescan from the earliest point, sending wallets the normal notifications.

In terms of implementation I would build this on top of #15719 by adding a CBlockLocator argument to ~~Chain::requestNotifications()~~ Chain::handleNotifications(). There would also need to be explicit requestRescan/abortRescan Chain methods to trigger rescans after wallets are loaded, for when users import keys or explicity request rescans.

promag commented at 3:38 pm on May 30, 2019: member

and what problem causes bitcoin-cli not to work right now.

Because the server gives RPC_IN_WARMUP:

0src/bitcoin-cli -regtest help
1error code: -28
2error message:
3Rescanning...

don’t understand exactly what problem causes this crash

Because autoCompleter is null here: https://github.com/bitcoin/bitcoin/blob/727143b65cdb801a0b755cb8b3aff97da5fe8f27/src/qt/rpcconsole.cpp#L535

I think the main thing that needs to happen is for the rescan loop to move out of the wallet into the node

This would go away right? https://github.com/bitcoin/bitcoin/blob/c7cfd20a77ce57d200b3b9e5e0dfb0d63818abdc/src/walletinitinterface.h#L22-L23

in src/qt/rpcconsole.cpp:682 in 727143b65c outdated

676@@ -677,6 +677,7 @@ void RPCConsole::setClientModel(ClientModel *model)
677         wordList.sort();
678         autoCompleter = new QCompleter(wordList, this);
679         autoCompleter->setModelSorting(QCompleter::CaseSensitivelySortedModel);
680+        ui->lineEdit->setEnabled(true);

ryanofsky commented at 3:45 pm on May 30, 2019:

Maybe add a comment saying the edit line starts off disabled to wait for rpc warmup.

promag commented at 10:26 pm on May 30, 2019:

Done.

ryanofsky approved

ryanofsky commented at 4:06 pm on May 30, 2019: member

utACK 727143b65cdb801a0b755cb8b3aff97da5fe8f27

Thanks for the explanations.

This would go away right?

I think adding a CBlockLocator argument to handleNotifications (sorry I mistakenly wrote requestNotifications before) should be the only init interface change needed for simultaneous rescanning.

The WalletInitInterface::Construct call shouldn’t be affected and isn’t directly involved in loading wallets. The Construct method is only responsible for parsing -wallet and -disablewallet options and constructing WalletClientImpl objects. Wallet loading happens later when WalletClientImpl::load is called.

fanquake added the label Needs backport on May 30, 2019

gui: Enable console line edit on setClientModel 2d8ad2f997

promag force-pushed on May 30, 2019

fanquake added this to the milestone 0.18.1 on May 31, 2019

fanquake commented at 2:06 pm on May 31, 2019: member

tACK https://github.com/bitcoin/bitcoin/pull/16122/commits/2d8ad2f99710a8981e33fe2d6ce834b0076c4e80 on macOS.

Using master (c7cfd20a77ce57d200b3b9e5e0dfb0d63818abdc) it’s possible to cause a crash during startup by opening Window -> Console typing anything into the console and hitting enter.

 0lldb Bitcoin-Qt.app
 1(lldb) target create "Bitcoin-Qt.app"
 2Current executable set to 'Bitcoin-Qt.app' (x86_64).
 3(lldb) run
 4Process 59937 launched: '/Users/michael/github/bitcoin/Bitcoin-Qt.app/Contents/MacOS/Bitcoin-Qt' (x86_64)
 52019-05-31 09:53:29.580881-0400 Bitcoin-Qt[59937:3647765] MessageTracer: Falling back to default whitelist
 6Process 59937 stopped
 7* thread [#1](/bitcoin-bitcoin/1/), name = 'bitcoin-main', queue = 'com.apple.main-thread', stop reason = EXC_BAD_ACCESS (code=1, address=0x8)
 8    frame [#0](/bitcoin-bitcoin/0/): 0x00000001011a78ba QtWidgets`QCompleter::popup() const + 10
 9QtWidgets`QCompleter::popup:
10->  0x1011a78ba <+10>: movq   0x8(%rdi), %rbx
11    0x1011a78be <+14>: movq   0x88(%rbx), %rax
12    0x1011a78c5 <+21>: testq  %rax, %rax
13    0x1011a78c8 <+24>: jne    0x1011a7942               ; <+146>
14Target 0: (Bitcoin-Qt) stopped.

Using this PR (https://github.com/bitcoin/bitcoin/pull/16122/commits/2d8ad2f99710a8981e33fe2d6ce834b0076c4e80), it’s still possible to get to the console during startup, but it’s impossible to enter or execute any commands. The console is enabled when startup completes.

laanwj merged this on Jun 3, 2019

laanwj closed this on Jun 3, 2019

laanwj referenced this in commit c3723c80da on Jun 3, 2019

promag deleted the branch on Jun 3, 2019

MarcoFalke referenced this in commit d31ae2218b on Jun 7, 2019

fanquake removed the label Needs backport on Jun 7, 2019

MarcoFalke referenced this in commit 7ed1a60193 on Jun 7, 2019

HashUnlimited referenced this in commit 302c5a8e55 on Aug 23, 2019

Bushstar referenced this in commit db41146209 on Aug 24, 2019

jasonbcox referenced this in commit 8ef3395601 on Oct 8, 2020

PastaPastaPasta referenced this in commit a1f23d077e on Jun 27, 2021

PastaPastaPasta referenced this in commit a8e0b656b5 on Jun 28, 2021

PastaPastaPasta referenced this in commit e6a0226189 on Jun 29, 2021

PastaPastaPasta referenced this in commit 85009507d7 on Jul 1, 2021

PastaPastaPasta referenced this in commit e55a34e9ed on Jul 1, 2021

PastaPastaPasta referenced this in commit 50f8552476 on Jul 12, 2021

DrahtBot locked this on Dec 16, 2021

gui: Enable console line edit on setClientModel #16122