gui: Add dynamic wallets support #13100

pull promag wants to merge 6 commits into bitcoin:master from promag:2018-04-ui-open-wallet changing 14 files +237 −41
  1. promag commented at 11:01 PM on April 26, 2018: member

    This PR adds a menu entry Open Wallet to support loading existing wallets in the UI.

    Fixes #7675.

  2. fanquake added the label GUI on Apr 27, 2018
  3. ken2812221 commented at 1:33 AM on April 27, 2018: contributor

    If the wallet folder name contains Chinese, it will create a new folder that has same wallet file with a weird name on Windows, maybe the encoding issue (Not related)

    Also, the seperator is incorrect on Windows image

  4. promag force-pushed on Apr 27, 2018
  5. promag commented at 8:36 AM on April 27, 2018: member

    @ken2812221 does the problem exists if you use the RPC command?

  6. ken2812221 commented at 8:49 AM on April 27, 2018: contributor

    It shows wallet not found on RPC. (Not related)

  7. promag commented at 8:55 AM on April 27, 2018: member

    @ken2812221 but it does exists right? What if you launch the application with -wallet=d:\...?

  8. ken2812221 commented at 9:14 AM on April 27, 2018: contributor

    -wallet=d:\錢包 does works, but the name displayed in Qt is incorrect image

    And it crash when I quit Qt

    Checked that this isn't an issue on Linux.

  9. ryanofsky commented at 12:47 PM on April 27, 2018: member

    Maybe windows or boost is using big5 encoding. which ends in a right square bracket ]

    >>> '錢包'.encode('big5')
b'\xbf\xfa\xa5]'
>>> '錢包'.encode('big5').decode('utf8', 'replace')
'���]'
  10. promag commented at 2:08 PM on April 27, 2018: member

    @ken2812221 thanks for the review and bug report. Let's move the bug discussion to #13103.

  11. in src/qt/bitcoingui.cpp:750 in f61aff5796 outdated 
     706 | @@ -701,6 +707,20 @@ void BitcoinGUI::openClicked()
 707 |      }
 708 |  }
 709 |  
 710 | +void BitcoinGUI::openWalletClicked()
 711 | +{
 712 | +    QString fileName = QFileDialog::getExistingDirectory(this, tr("Open Wallet"),
    ken2812221 commented at 10:56 AM on April 29, 2018:

    Use QDir::toNativeSeparators to convert / to \ on Windows

    promag commented at 4:33 PM on May 2, 2018:

    Done.

    unknown commented at 5:01 AM on June 12, 2018:

    ^^

  12. in src/interfaces/node.cpp:238 in f61aff5796 outdated 
     233 | +    }
 234 | +    std::pair<std::unique_ptr<Wallet>, std::string> loadWallet(const std::string& wallet_file) override
 235 | +    {
 236 | +#ifdef ENABLE_WALLET
 237 | +        try {
 238 | +            std::string dummy_warning;
    jnewbery commented at 6:12 PM on April 30, 2018:

    This seems almost exactly duplicated from loadwallet() in rpcwallet.cpp. Does it make sense to factor it out to avoid repetition?

    promag commented at 6:20 PM on April 30, 2018:

    Yeah, I would like to move to WalletManager

    promag commented at 4:32 PM on May 2, 2018:

    @jnewbery done in #9888dc2.

  13. in src/interfaces/node.cpp:254 in f61aff5796 outdated 
     249 | +            AddWallet(wallet);
 250 | +
 251 | +            wallet->postInitProcess();
 252 | +            return { MakeWallet(*wallet), std::string() };
 253 | +        } catch (...) {
 254 | +            return { std::unique_ptr<Wallet>(), "ops" };
    jnewbery commented at 6:14 PM on April 30, 2018:

    I don't understand why you're passing back an error string if it's always the string ops

    promag commented at 6:21 PM on April 30, 2018:

    Ops, it's unfinished. Not sure if it should return the pair.

    promag commented at 4:32 PM on May 2, 2018:

    Removed, changed the return type.

  14. in src/qt/bitcoingui.cpp:717 in f61aff5796 outdated 
     712 | +    QString fileName = QFileDialog::getExistingDirectory(this, tr("Open Wallet"),
 713 | +                                                QString(),
 714 | +                                                QFileDialog::ShowDirsOnly
 715 | +                                                | QFileDialog::DontResolveSymlinks);
 716 | +    if (fileName.isEmpty()) return;
 717 | +    const auto& result = m_node.loadWallet(fileName.toStdString());
    jnewbery commented at 6:15 PM on April 30, 2018:

    The use of auto here makes it difficult to understand the result.second below.

    promag commented at 4:33 PM on May 2, 2018:

    No an issue now, return value is not necessary.

  15. meshcollider commented at 9:12 AM on May 2, 2018: contributor

    Concept ACK

  16. promag force-pushed on May 2, 2018
  17. in src/qt/bitcoingui.cpp:720 in 462bf7fd69 outdated 
     715 | +                                                QFileDialog::ShowDirsOnly
 716 | +                                                | QFileDialog::DontResolveSymlinks);
 717 | +    if (fileName.isEmpty()) return;
 718 | +
 719 | +    std::vector<std::string> errors;
 720 | +    m_node.loadWallet(QDir::toNativeSeparators(fileName).toStdString(), errors);
    promag commented at 9:37 AM on May 3, 2018:

    @jnewbery @laanwj ATM the wallet is loaded in the UI thread, should I refactor to load it in the background?

    jnewbery commented at 8:04 PM on May 3, 2018:

    I think that's a good future enhancement. Not necessary in the initial version IMO.

  18. jnewbery commented at 8:03 PM on May 3, 2018: member

    Please update description to say that this fixes #7675

  19. jnewbery added this to the "In progress" column in a project
  20. promag force-pushed on May 4, 2018
  21. promag commented at 10:06 PM on May 4, 2018: member

    Please update description to say that this fixes #7675

    Done. Also rebased.

  22. promag force-pushed on May 23, 2018
  23. promag force-pushed on May 23, 2018
  24. promag commented at 11:57 AM on May 23, 2018: member

    Rebased on master.

  25. jnewbery commented at 1:29 PM on May 23, 2018: member

    wallet_multiwallet.py fails

  26. promag force-pushed on May 23, 2018
  27. MarcoFalke renamed this:
    Add menu entry to open wallet
    gui: Add menu entry to open wallet
    on May 23, 2018
  28. promag force-pushed on May 24, 2018
  29. promag commented at 1:21 PM on May 24, 2018: member

    Rebased.

  30. in src/qt/bitcoingui.cpp:751 in a27b30050f outdated 
     713 | @@ -707,6 +714,24 @@ void BitcoinGUI::openClicked()
 714 |      }
 715 |  }
 716 |  
 717 | +void BitcoinGUI::openWalletClicked()
 718 | +{
 719 | +    QString fileName = QFileDialog::getExistingDirectory(this, tr("Open Wallet"),
 720 | +                                                QString(),
    jnewbery commented at 2:51 PM on May 24, 2018:

    Suggestion: start in the data directory for the current network (mainnet/testnet/regtest) instead of the home directory, since that's where the wallet files and folders will usually be (and because it may be difficult to traverse down into .bitcoin).

  31. in src/qt/bitcoingui.cpp:721 in a27b30050f outdated 
     713 | @@ -707,6 +714,24 @@ void BitcoinGUI::openClicked()
 714 |      }
 715 |  }
 716 |  
 717 | +void BitcoinGUI::openWalletClicked()
 718 | +{
 719 | +    QString fileName = QFileDialog::getExistingDirectory(this, tr("Open Wallet"),
 720 | +                                                QString(),
 721 | +                                                QFileDialog::ShowDirsOnly
    jnewbery commented at 2:51 PM on May 24, 2018:

    Why only show directories and not symlinks? Wallets can be .dat files.

    promag commented at 3:48 PM on May 24, 2018:

    Right.

  32. unknown approved
  33. promag force-pushed on Jun 23, 2018
  34. DrahtBot added the label Needs rebase on Jun 25, 2018
  35. promag renamed this:
    gui: Add menu entry to open wallet
    gui: Add dynamic wallets support
    on Jul 1, 2018
  36. jnewbery commented at 6:48 PM on July 9, 2018: member

    @promag - what's the status of this? It'd be great to have this in v0.17, but feature freeze is next week.

  37. promag force-pushed on Jul 10, 2018
  38. promag force-pushed on Jul 10, 2018
  39. promag force-pushed on Jul 10, 2018
  40. promag commented at 2:20 AM on July 10, 2018: member

    @jnewbery agree, please test, I'll address your comments as soon as I can.

  41. promag force-pushed on Jul 10, 2018
  42. DrahtBot removed the label Needs rebase on Jul 10, 2018
  43. in src/qt/bitcoingui.cpp:327 in 6741c10ce4 outdated 
     323 | @@ -322,6 +324,15 @@ void BitcoinGUI::createActions()
 324 |      openAction = new QAction(platformStyle->TextColorIcon(":/icons/open"), tr("Open &URI..."), this);
 325 |      openAction->setStatusTip(tr("Open a bitcoin: URI or payment request"));
 326 |  
 327 | +    m_new_wallet_action = new QAction(platformStyle->TextColorIcon(":/icons/open"), tr("New &Wallet..."), this);
    Sjors commented at 1:11 PM on July 10, 2018:

    IIUC the & sets the keyboard shortcut, so these should be unique (or absent).

    promag commented at 2:08 PM on July 10, 2018:

    Then I think it's best to omit shortcuts for now, WDYT?

    Sjors commented at 2:13 PM on July 10, 2018:

    I'm fine with either no shortcuts or unique ones.

  44. Sjors changes_requested
  45. Sjors commented at 1:30 PM on July 10, 2018: member

    On macOS when opening a wallet or creating a new wallet it suggests the directory from which opened QT by default. This also leads to strangeness in the dropdown list:

    <img width="409" alt="paths" src="https://user-images.githubusercontent.com/10217/42512739-b47745ca-8455-11e8-8a56-7c4ad118085a.png">

    The default should be the datadir, if that can be passed into the dialog.

    For creating new wallet, I think a better approach is to ask for a wallet name, and not offer the user the option to put it in a different directory for the UI.

    For opening an existing wallet I have a light preference for just scanning the default directory for wallets and showing a list of wallets to choose from. This also prevents confusion around wallets that are files vs. wallets that are directories. Offering multiple wallets and offering the ability to put wallets anywhere in the filesystem are two different features.

    Code looks good to me, but I'm not qualified to tell if the memory / pointer management stuff is correct.

    macOS threw a warning:

    objc[17648]: Class FIFinderSyncExtensionHost is implemented in both /System/Library/PrivateFrameworks/FinderKit.framework/Versions/A/FinderKit (0x7fff86ae1c90) and /System/Library/PrivateFrameworks/FileProvider.framework/OverrideBundles/FinderSyncCollaborationFileProviderOverride.bundle/Contents/MacOS/FinderSyncCollaborationFileProviderOverride (0x113143cd8). One of the two will be used. Which one is undefined.
  46. promag commented at 2:09 PM on July 10, 2018: member

    @Sjors thanks for testing!

    The default should be the datadir,

    Or walletdir?

  47. Sjors commented at 2:14 PM on July 10, 2018: member

    Or walletdir?

    If present, probably yes. Afaik it's only created if there was no datadir at launch time, so at least in normal circumstances if /wallets exists, it should be non-confusing to use that as a default.

  48. fanquake added this to the "Blockers" column in a project
  49. in src/wallet/wallet.cpp:124 in 6741c10ce4 outdated 
     120 | +
 121 | +    wallet->postInitProcess();
 122 | +    return wallet;
 123 | +}
 124 | +
 125 | +std::shared_ptr<CWallet> LoadWallet(const std::string& wallet_file, std::string& error, std::string& warning)
    Empact commented at 3:56 PM on July 15, 2018:

    nit: there's a lot of common code between LoadWallet and CreateWallet

  50. DrahtBot added the label Needs rebase on Jul 20, 2018
  51. promag commented at 1:25 AM on July 22, 2018: member

    With #9662 merged, do we want the same feature in the UI?

  52. promag force-pushed on Jul 22, 2018
  53. promag commented at 1:28 AM on July 22, 2018: member

    Rebased.

  54. Sjors commented at 10:46 AM on July 22, 2018: member

    A check box for watch-only in the GUI can probably wait. Not sure if it's useful, but not against it either.

  55. DrahtBot removed the label Needs rebase on Jul 22, 2018
  56. in src/qt/bitcoingui.cpp:363 in cb89573eba outdated 
     357 | @@ -347,6 +358,9 @@ void BitcoinGUI::createActions()
 358 |          connect(usedSendingAddressesAction, SIGNAL(triggered()), walletFrame, SLOT(usedSendingAddresses()));
 359 |          connect(usedReceivingAddressesAction, SIGNAL(triggered()), walletFrame, SLOT(usedReceivingAddresses()));
 360 |          connect(openAction, SIGNAL(triggered()), this, SLOT(openClicked()));
 361 | +        connect(m_new_wallet_action, SIGNAL(triggered()), this, SLOT(newWallet()));
 362 | +        connect(m_open_wallet_action, SIGNAL(triggered()), this, SLOT(openWallet()));
 363 | +        connect(m_close_wallet_action, SIGNAL(triggered()), this, SLOT(closeWallet()));
    MarcoFalke commented at 3:29 PM on July 22, 2018:

    Could use the new syntax here?

  57. jnewbery commented at 8:56 PM on July 23, 2018: member

    Fails wallet_disableprivatekeys.py. The wallet_creation_flags are not being passed through to CreateWalletFromFile(), and so this regresses being able to create a watch-only wallet.

    With #9662 merged, do we want the same feature in the UI?

    Later. Let's just try to get the basic functionality into the GUI first!

  58. promag force-pushed on Jul 24, 2018
  59. promag commented at 8:49 PM on July 24, 2018: member

    @jnewbery thanks for the hint, fixed.

  60. Sjors commented at 8:53 AM on July 25, 2018: member

    I'm a bit confused by git here: Github shows 737ecac as the most recent commit, but when I fetch your branch it's not there. Travis seems to ignore it as well.

  61. promag commented at 9:23 AM on July 25, 2018: member

    @Sjors the last commit in the branch is 940e445, github orders by author date IIUC.

  62. in src/qt/bitcoingui.cpp:709 in 940e44556f outdated 
     703 | @@ -681,6 +704,54 @@ void BitcoinGUI::openClicked()
 704 |      }
 705 |  }
 706 |  
 707 | +void BitcoinGUI::newWallet()
 708 | +{
 709 | +    QString file_name = QFileDialog::getSaveFileName(this, tr("New Wallet"));
    jnewbery commented at 9:37 PM on July 26, 2018:

    This still defaults to the directory that qt is running in. Can we change that to the wallet directory?

  63. in src/qt/bitcoingui.cpp:724 in 940e44556f outdated 
     719 | +    }
 720 | +}
 721 | +
 722 | +void BitcoinGUI::openWallet()
 723 | +{
 724 | +    QString file_name = QFileDialog::getExistingDirectory(this, tr("Open Wallet"), QString(), QFileDialog::DontResolveSymlinks);
    jnewbery commented at 9:38 PM on July 26, 2018:

    Why is this getExistingDirectory()? A wallet can also be a file I think.

    Also defaults to the directory that qt is running in.

  64. in src/wallet/wallet.cpp:84 in 940e44556f outdated 
      78 | @@ -79,6 +79,74 @@ std::shared_ptr<CWallet> GetWallet(const std::string& name)
  79 |      return nullptr;
  80 |  }
  81 |  
  82 | +static void GetWalletNameAndPath(const std::string& wallet_file, std::string& wallet_name, fs::path& wallet_path) {
    jnewbery commented at 9:38 PM on July 26, 2018:

    I find this function quite inscrutable. Can you add some comments?

    promag commented at 8:52 PM on September 7, 2018:

    Sure!

  65. laanwj removed this from the "Blockers" column in a project
  66. DrahtBot added the label Needs rebase on Aug 21, 2018
  67. laanwj added this to the "Blockers" column in a project
  68. PierreRochard commented at 11:13 PM on August 25, 2018: contributor

    Concept ACK, I read through the changes and manually tested the GUI. Ditto to @jnewbery's comments

  69. MarcoFalke removed this from the "Blockers" column in a project
  70. MarcoFalke commented at 7:35 PM on August 29, 2018: member

    Removing from High-priority due to conflicts

  71. Sjors commented at 8:14 PM on September 7, 2018: member

    I suggest adding a warning, once a user has selected a wallet file to open, that you should never ever use wallet files downloaded from strangers.

  72. jnewbery commented at 8:31 PM on September 7, 2018: member

    @promag - are you still actively working on this PR?

  73. promag commented at 8:54 PM on September 7, 2018: member

    I suggest adding a warning, once a user has selected a wallet file to open, that you should never ever use wallet files downloaded from strangers. @Sjors do you think it should be done in this PR?

    @promag - are you still actively working on this PR? @jnewbery yes, I have some changes to submit.

  74. jnewbery commented at 9:23 PM on September 7, 2018: member

    yes, I have some changes to submit.

    Great! Looking forward to seeing them :grin:

  75. Sjors commented at 9:29 AM on September 8, 2018: member

    in this PR?

    No, but worth creating an up for grabs issue for.

  76. jnewbery commented at 5:37 PM on September 11, 2018: member

    @promag - it'd be great to get this in for v0.18. It's the last piece in https://github.com/bitcoin/bitcoin/projects/2.

    Longer term, one of my motivations for #13059 was to have bitcoind/qt not create a default wallet at startup if one didn't exist. This PR is the final blocker for that to happen.

  77. gui: Factor out WalletModel::getDisplayName() b70b3452a1
  78. gui: Move private slot BitcoinApplication::removeWallet to lambda 295f9797cf
  79. wallet: Add CreateWallet and LoadWallet functions 919c5e22b0
  80. interfaces: Add dynamic wallets support 2577ab48f4
  81. gui: Add dynamic wallets support 1bb5612db8
  82. promag commented at 10:03 PM on September 21, 2018: member

    Related #11485 (comment)

  83. wip: Some fixups a7a8be8d1f
  84. promag force-pushed on Sep 21, 2018
  85. DrahtBot removed the label Needs rebase on Sep 21, 2018
  86. DrahtBot commented at 7:21 AM on September 22, 2018: member

    <!--e57a25ab6845829454e8d69fc972939a-->Reviewers, this pull request conflicts with the following ones:

    • #14451 (Add BIP70 deprecation warning and allow building GUI without BIP70 support by jameshilliard)
    • #14437 (Refactor: Start to separate wallet from node by ryanofsky)
    • #14350 (Add WalletInfo class by promag)
    • #14291 (wallet: Add ListWalletDir utility function by promag)
    • #13926 ([Tools] bitcoin-wallet - a tool for creating and managing wallets offline by jnewbery)
    • #13756 (wallet: -avoidreuse feature for improved privacy by kallewoof)
    • #11625 (Add BitcoinApplication & RPCConsole tests by ryanofsky)
    • #10973 (Refactor: separate wallet from node by ryanofsky)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

  87. practicalswift commented at 6:07 PM on September 22, 2018: contributor

    This PR does not compile after rebasing on master :-)

  88. Sjors commented at 11:51 AM on September 25, 2018: member

    @jnewbery wrote:

    Longer term [...] to have bitcoind/qt not create a default wallet at startup if one didn't exist. This PR is the final blocker for that to happen.

    That would be nice indeed. I find it quite annoying that you can't start bitcoind without any wallet loaded. And I have too many wallet.dat files floating around that I need to carefully check are actually safe to delete :-)

    I'll test again after rebase.

    Re #11485 listing wallets, this could be added later. It's nice to just show a list of wallet names that can be loaded, avoiding the file dialog.

    For this PR it should be enough if the file dialog opens in walletdir, or datadir if that doesn't exist. (the latter is impossible now, so maybe a source code comment warning would suffice).

    Creating a new wallet should probably create a data and/or wallet dir, though it's easier if #13761 is merged first.

  89. jnewbery commented at 12:26 PM on September 25, 2018: member

    @promag - what's the status here? This PR seems to have stalled.

  90. promag commented at 12:58 PM on September 25, 2018: member

    @jnewbery I'm rebasing on #14291. Current implementation is more suitable to external wallets.

  91. promag commented at 1:27 PM on September 25, 2018: member

    @Sjors @jnewbery this is what it looks like:

    <img width="425" alt="screen shot 2018-09-25 at 12 43 04" src="https://user-images.githubusercontent.com/3534524/46017109-9925fe00-c0ce-11e8-8635-ace219b7f603.png">

    For this PR it should be enough if the file dialog opens in walletdir, or datadir if that doesn't exist.

    If we handle wallets in the -walletdir differently, then, for external wallets, I think we should point the file dialog to QDesktopServices::DocumentsLocation or whatever.

  92. Sjors commented at 1:37 PM on September 25, 2018: member

    That looks pretty nice. I'd move Open External Wallet to the bottom and just call it Other.... Indeed it makes sense that doesn't use walletdir.

  93. jonasschnelli commented at 6:46 PM on September 25, 2018: contributor

    Doesn't build on linux/win gitian:

      CXXLD    test/test_bitcoin.exe
  AR       qt/libbitcoinqt.a
  OBJCXXLD qt/bitcoin-qt.exe
libbitcoin_server.a(libbitcoin_server_a-node.o): In function `getWalletDir':
/home/ubuntu/build/bitcoin/distsrc-i686-w64-mingw32/src/interfaces/node.cpp:226: undefined reference to `GetWalletDir[abi:cxx11]()'
collect2: error: ld returned 1 exit status
Makefile:3904: recipe for target 'qt/bitcoin-qt.exe' failed
make[2]: *** [qt/bitcoin-qt.exe] Error 1
make[2]: Leaving directory '/home/ubuntu/build/bitcoin/distsrc-i686-w64-mingw32/src'
Makefile:10188: recipe for target 'all-recursive' failed
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory '/home/ubuntu/build/bitcoin/distsrc-i686-w64-mingw32/src'
Makefile:774: recipe for target 'all-recursive' failed
make: *** [all-recursive] Error 1
Failed run an application inside container

      AR       qt/libbitcoinqt.a
  OBJCXXLD qt/bitcoin-qt
libbitcoin_server.a(libbitcoin_server_a-node.o): In function `interfaces::(anonymous namespace)::NodeImpl::getWalletDir()':
/home/ubuntu/build/bitcoin/distsrc-i686-pc-linux-gnu/src/interfaces/node.cpp:226: undefined reference to `GetWalletDir[abi:cxx11]()'
collect2: error: ld returned 1 exit status
Makefile:3904: recipe for target 'qt/bitcoin-qt' failed
make[2]: *** [qt/bitcoin-qt] Error 1
make[2]: Leaving directory '/home/ubuntu/build/bitcoin/distsrc-i686-pc-linux-gnu/src'
Makefile:10188: recipe for target 'all-recursive' failed
make[1]: *** [all-recursive] Error 1
make[1]: Leaving directory '/home/ubuntu/build/bitcoin/distsrc-i686-pc-linux-gnu/src'
Makefile:774: recipe for target 'all-recursive' failed
make: *** [all-recursive] Error 1
Failed run an application inside container
  94. DrahtBot commented at 9:52 AM on October 20, 2018: member

    <!--cf906140f33d8803c4a75a2196329ecb-->Needs rebase

  95. DrahtBot added the label Needs rebase on Oct 20, 2018
  96. meshcollider commented at 6:21 AM on November 3, 2018: contributor

    It'd be nice to get this in to 0.18, is that feasible? Needs a rebase

    Overall this is looking good, concept ACK with some code review, I'll review more once rebased/the comments above have been addressed and the WIP fixup commit is done

  97. meshcollider added the label Wallet on Nov 9, 2018
  98. fanquake added this to the milestone 0.18.0 on Nov 10, 2018
  99. Sjors commented at 5:42 PM on December 10, 2018: member

    For a followup, once #11082 is merged, we should add a wallet= entry to it once a wallet is created.

    Noteable changes that may cause a headache during rebase:

    • #14350 (by yourself, WalletLocation needed in various places)
    • #14437 (you need to pass interfaces::Chain& chain to CreateWallet, which needs it for Verify)
  100. in src/qt/bitcoingui.cpp:335 in a7a8be8d1f 
     331 | @@ -330,6 +332,15 @@ void BitcoinGUI::createActions()
 332 |      openAction = new QAction(platformStyle->TextColorIcon(":/icons/open"), tr("Open &URI..."), this);
 333 |      openAction->setStatusTip(tr("Open a bitcoin: URI or payment request"));
 334 |  
 335 | +    m_new_wallet_action = new QAction(platformStyle->TextColorIcon(":/icons/open"), tr("New &Wallet"), this);
    luke-jr commented at 11:10 AM on January 2, 2019:

    If we're going to use icons, I think they ought to be different...

  101. luke-jr commented at 11:13 AM on January 2, 2019: member

    Why is there a separate File and Wallet menu in the screenshot above? They should be the same menu...

  102. jonasschnelli commented at 6:33 AM on January 4, 2019: contributor

    @promag: care to rebase/pick this up again?

  103. Sjors commented at 4:10 PM on January 17, 2019: member

    A lot of this has been moved to smaller pull requests. E.g. there's now a separate PR for just the open wallet feature #15153. I suppose "unloading" a wallet would be fairly trivial on top of that.

    For my own work on #14912 I'd love to have a (proof of concept) "create wallet" PR to build on top of. Ideally it would a dialog, with a check box for the read-only feature (and later for the empty wallet feature in #14938).

  104. jnewbery commented at 4:20 PM on January 17, 2019: member

    Feature freeze is on Feb 15th. I'd really love to have this in v0.18. @promag - any chance of a rebase so we can start testing & reviewing?

  105. promag commented at 4:55 PM on January 17, 2019: member

    @jnewbery please see #15101 and then #15153.

  106. jonasschnelli referenced this in commit 63144335be on Jan 18, 2019
  107. jnewbery commented at 8:37 PM on January 18, 2019: member

    Can this PR be closed? I think all changes are covered by other PRs, and issue #13059 tracks the total progress.

  108. promag closed this on Jan 18, 2019
  109. promag deleted the branch on Jan 18, 2019
  110. promag commented at 8:40 PM on January 18, 2019: member

    Thanks for updating that @jnewbery.

  111. jonasschnelli removed this from the "In progress" column in a project
  112. laanwj removed the label Needs rebase on Oct 24, 2019
  113. PastaPastaPasta referenced this in commit bc3d461c44 on Sep 8, 2021
  114. PastaPastaPasta referenced this in commit 85503c821b on Sep 10, 2021
  115. PastaPastaPasta referenced this in commit 90e8487af9 on Sep 10, 2021
  116. PastaPastaPasta referenced this in commit 43744dc130 on Sep 18, 2021
  117. PastaPastaPasta referenced this in commit 68ad472996 on Sep 18, 2021
  118. PastaPastaPasta referenced this in commit 4197db6a7e on Sep 18, 2021
  119. thelazier referenced this in commit 76c77bc265 on Sep 25, 2021
  120. MarcoFalke locked this on Dec 16, 2021
Contributors
promagken2812221ryanofskyjnewberymeshcolliderSjorsEmpactMarcoFalkePierreRochardDrahtBotpracticalswiftjonasschnelliluke-jr
Labels
GUIWallet

Milestone
0.18.0

Linked (view graph)
#7675 Qt GUI Feature Request: Load wallet file#13103 Invalid wallet path with Chinese characters in windows #15101 gui: Add WalletController
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: 2026-04-29 09:15 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me