LBRYCommunity/lbrycrd
1
0
Fork 0
Code Issues 29 Pull requests 1 Projects Releases 32 Packages Wiki Activity

Merge #16399: wallet: Improve wallet creation

Browse source 
e967cae8fa Use switch on status in RpcWallet (Fabian Jahr)
ba1f128d6c Return error for ignored passphrase through disable private keys option (Fabian Jahr)
d6649d16b5 Use strong enum for WalletCreationStatus (Fabian Jahr)
3199610ad3 Place out args at the end for CreateWallet (Fabian Jahr)

Pull request description:

  This is a follow-up PR to #16244

  The following suggestions are included:
  - Usage of `enum class` (https://github.com/bitcoin/bitcoin/pull/16244#discussion_r296434142)
  - Placing out args at the end convention (https://github.com/bitcoin/bitcoin/pull/16244#discussion_r296434172)
  - Return error when passphrase would be ignored because of disabled private keys (including functional test) (https://github.com/bitcoin/bitcoin/pull/16244#pullrequestreview-252015195)
  - Make `status` return variable of `CreateWallet` (https://github.com/bitcoin/bitcoin/pull/16244#discussion_r302107394)
  - Using a `switch` statement instead of `if/else` in `RpcWallet` (https://github.com/bitcoin/bitcoin/pull/16244#discussion_r302112502)

  Not included was:
  - "new create wallet function [could take] separate option arguments instead of wallet flags" (https://github.com/bitcoin/bitcoin/pull/16244#pullrequestreview-252015195)
  - "blank wallet and disable private keys options could be combined into a single option" (https://github.com/bitcoin/bitcoin/pull/16244#pullrequestreview-252015195)

  For these last two changes, I was not sure what an ideal solution could look like and/or this might be of slightly larger scope than the other changes, but I would be happy to work on these as well in this PR or another follow-up if I get positive feedback on that. Is there a place in the codebase that handles flags like these in a better way that I can refer to? Nonetheless, I would prefer keeping it in a separate PR unless it is a really simple change.

ACKs for top commit:
  jnewbery:
    Code review utACK e967cae8fa
  MarcoFalke:
    ACK e967cae8fa

Tree-SHA512: 3d12880ff95add9e4a5702afa26ef38080b57b216a608c113a4d0a08ba2d61142c027ba0071c6402add45db90383eee0bada12dc42820dc0d602721d7175edd5
This commit is contained in:
MarcoFalke 2019-07-29 09:36:49 -04:00
commit 74ea1f3b0f
No known key found for this signature in database
GPG key ID: D2EA4850E7528B25
4 changed files with 29 additions and 23 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

18
src/wallet/rpcwallet.cpp
View file

@ -2690,14 +2690,16 @@ static UniValue createwallet(const JSONRPCRequest& request)
std::string error;
std::string warning;
WalletCreationStatus status;
std::shared_ptr<CWallet> wallet = CreateWallet(*g_rpc_interfaces->chain, request.params[0].get_str(), error, warning, status, passphrase, flags);
if (status == WalletCreationStatus::CREATION_FAILED) {
throw JSONRPCError(RPC_WALLET_ERROR, error);
} else if (status == WalletCreationStatus::ENCRYPTION_FAILED) {
throw JSONRPCError(RPC_WALLET_ENCRYPTION_FAILED, error);
} else if (status != WalletCreationStatus::SUCCESS) {
throw JSONRPCError(RPC_WALLET_ERROR, "Wallet creation failed");
std::shared_ptr<CWallet> wallet;
WalletCreationStatus status = CreateWallet(*g_rpc_interfaces->chain, passphrase, flags, request.params[0].get_str(), error, warning, wallet);
switch (status) {
case WalletCreationStatus::CREATION_FAILED:
throw JSONRPCError(RPC_WALLET_ERROR, error);
case WalletCreationStatus::ENCRYPTION_FAILED:
throw JSONRPCError(RPC_WALLET_ENCRYPTION_FAILED, error);
case WalletCreationStatus::SUCCESS:
break;
// no default case, so the compiler can warn about missing cases
}
UniValue obj(UniValue::VOBJ);

27
src/wallet/wallet.cpp
View file

@ -161,7 +161,7 @@ std::shared_ptr<CWallet> LoadWallet(interfaces::Chain& chain, const std::string&
return LoadWallet(chain, WalletLocation(name), error, warning);
}
std::shared_ptr<CWallet> CreateWallet(interfaces::Chain& chain, const std::string& name, std::string& error, std::string& warning, WalletCreationStatus& status, const SecureString& passphrase, uint64_t wallet_creation_flags)
WalletCreationStatus CreateWallet(interfaces::Chain& chain, const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, std::string& error, std::string& warning, std::shared_ptr<CWallet>& result)
{
// Indicate that the wallet is actually supposed to be blank and not just blank to make it encrypted
bool create_blank = (wallet_creation_flags & WALLET_FLAG_BLANK_WALLET);
@ -175,39 +175,40 @@ std::shared_ptr<CWallet> CreateWallet(interfaces::Chain& chain, const std::strin
WalletLocation location(name);
if (location.Exists()) {
error = "Wallet " + location.GetName() + " already exists.";
status = WalletCreationStatus::CREATION_FAILED;
return nullptr;
return WalletCreationStatus::CREATION_FAILED;
}
// Wallet::Verify will check if we're trying to create a wallet with a duplicate name.
std::string wallet_error;
if (!CWallet::Verify(chain, location, false, wallet_error, warning)) {
error = "Wallet file verification failed: " + wallet_error;
status = WalletCreationStatus::CREATION_FAILED;
return nullptr;
return WalletCreationStatus::CREATION_FAILED;
}
// Do not allow a passphrase when private keys are disabled
if (!passphrase.empty() && (wallet_creation_flags & WALLET_FLAG_DISABLE_PRIVATE_KEYS)) {
error = "Passphrase provided but private keys are disabled. A passphrase is only used to encrypt private keys, so cannot be used for wallets with private keys disabled.";
return WalletCreationStatus::CREATION_FAILED;
}
// Make the wallet
std::shared_ptr<CWallet> wallet = CWallet::CreateWalletFromFile(chain, location, wallet_creation_flags);
if (!wallet) {
error = "Wallet creation failed";
status = WalletCreationStatus::CREATION_FAILED;
return nullptr;
return WalletCreationStatus::CREATION_FAILED;
}
// Encrypt the wallet
if (!passphrase.empty() && !(wallet_creation_flags & WALLET_FLAG_DISABLE_PRIVATE_KEYS)) {
if (!wallet->EncryptWallet(passphrase)) {
error = "Error: Wallet created but failed to encrypt.";
status = WalletCreationStatus::ENCRYPTION_FAILED;
return nullptr;
return WalletCreationStatus::ENCRYPTION_FAILED;
}
if (!create_blank) {
// Unlock the wallet
if (!wallet->Unlock(passphrase)) {
error = "Error: Wallet was encrypted but could not be unlocked";
status = WalletCreationStatus::ENCRYPTION_FAILED;
return nullptr;
return WalletCreationStatus::ENCRYPTION_FAILED;
}
// Set a seed for the wallet
@ -221,8 +222,8 @@ std::shared_ptr<CWallet> CreateWallet(interfaces::Chain& chain, const std::strin
}
AddWallet(wallet);
wallet->postInitProcess();
status = WalletCreationStatus::SUCCESS;
return wallet;
result = wallet;
return WalletCreationStatus::SUCCESS;
}
const uint32_t BIP32_HARDENED_KEY_LIMIT = 0x80000000;

4
src/wallet/wallet.h
View file

@ -50,13 +50,13 @@ std::vector<std::shared_ptr<CWallet>> GetWallets();
std::shared_ptr<CWallet> GetWallet(const std::string& name);
std::shared_ptr<CWallet> LoadWallet(interfaces::Chain& chain, const WalletLocation& location, std::string& error, std::string& warning);
enum WalletCreationStatus {
enum class WalletCreationStatus {
SUCCESS,
CREATION_FAILED,
ENCRYPTION_FAILED
};
std::shared_ptr<CWallet> CreateWallet(interfaces::Chain& chain, const std::string& name, std::string& error, std::string& warning, WalletCreationStatus& status, const SecureString& passphrase, uint64_t wallet_creation_flags);
WalletCreationStatus CreateWallet(interfaces::Chain& chain, const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, std::string& error, std::string& warning, std::shared_ptr<CWallet>& result);
//! Default for -keypool
static const unsigned int DEFAULT_KEYPOOL_SIZE = 1000;

3
test/functional/wallet_createwallet.py
View file

@ -119,5 +119,8 @@ class CreateWalletTest(BitcoinTestFramework):
# Empty passphrase, error
assert_raises_rpc_error(-16, 'Cannot encrypt a wallet with a blank password', self.nodes[0].createwallet, 'w7', False, False, '')
self.log.info('Using a passphrase with private keys disabled returns error')
assert_raises_rpc_error(-4, 'Passphrase provided but private keys are disabled. A passphrase is only used to encrypt private keys, so cannot be used for wallets with private keys disabled.', self.nodes[0].createwallet, wallet_name='w8', disable_private_keys=True, passphrase='thisisapassphrase')
if __name__ == '__main__':
CreateWalletTest().main()