gui: Enable console line edit on setClientModel #16122

pull promag wants to merge 1 commits into bitcoin:master from promag:2019-05-null-completer changing 2 files +6 −0
  1. 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.

    Fixes #16119.

  2. fanquake added the label GUI on May 29, 2019
  3. 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
  4. 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.

  5. 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

  6. 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.
  7. ryanofsky approved
  8. 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.

  9. fanquake added the label Needs backport on May 30, 2019
  10. gui: Enable console line edit on setClientModel 2d8ad2f997
  11. promag force-pushed on May 30, 2019
  12. fanquake added this to the milestone 0.18.1 on May 31, 2019
  13. 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.

  14. laanwj merged this on Jun 3, 2019
  15. laanwj closed this on Jun 3, 2019

  16. laanwj referenced this in commit c3723c80da on Jun 3, 2019
  17. promag deleted the branch on Jun 3, 2019
  18. MarcoFalke referenced this in commit d31ae2218b on Jun 7, 2019
  19. fanquake removed the label Needs backport on Jun 7, 2019
  20. MarcoFalke referenced this in commit 7ed1a60193 on Jun 7, 2019
  21. HashUnlimited referenced this in commit 302c5a8e55 on Aug 23, 2019
  22. Bushstar referenced this in commit db41146209 on Aug 24, 2019
  23. jasonbcox referenced this in commit 8ef3395601 on Oct 8, 2020
  24. PastaPastaPasta referenced this in commit a1f23d077e on Jun 27, 2021
  25. PastaPastaPasta referenced this in commit a8e0b656b5 on Jun 28, 2021
  26. PastaPastaPasta referenced this in commit e6a0226189 on Jun 29, 2021
  27. PastaPastaPasta referenced this in commit 85009507d7 on Jul 1, 2021
  28. PastaPastaPasta referenced this in commit e55a34e9ed on Jul 1, 2021
  29. PastaPastaPasta referenced this in commit 50f8552476 on Jul 12, 2021
  30. DrahtBot locked this on Dec 16, 2021


promag ryanofsky fanquake

Labels
GUI

Milestone
0.18.1


github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2024-11-17 12:12 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me