This PR adds a new passphrase argument to createwallet which will create a wallet that is encrypted with that passphrase.
This is built on #15226 because it needs to first create an empty wallet, then encrypt the empty wallet and generate new keys that have only been stored in an encrypted state.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
Reviewers, this pull request conflicts with the following ones:
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.
Concept ACK.
Today on IRC:
29+ address1 = w0.getnewaddress()
30+
31+ self.log.info("Test disableprivatekeys creation.")
32+ self.nodes[0].createwallet('w1', True)
33+ w1 = node.get_wallet_rpc('w1')
34+ assert_raises_rpc_error(-4,"Error: Private keys are disabled for this wallet", w1.getnewaddress)
,. Please run this newly added file through black to make sure formatting is correct :-)
This fails travis:
0Assertion failed: lock cs_wallet not held in wallet/wallet.cpp:1389; locks held:
1FAIL qt/test/test_bitcoin-qt (exit status: 134)
487@@ -488,6 +488,10 @@ class WalletImpl : public Wallet
488 {
489 return MakeHandler(m_wallet.NotifyWatchonlyChanged.connect(fn));
490 }
491+ std::unique_ptr<Handler> handleCanGetAddressesChanged(CanGetAddressesChangedFn fn) override
handleCanGetAddressesChanged is a terrible method name in itself, took me at least 5 minutes to decode it, please add a comment saying it’s a handler for changes in whether addresses can be generated or not :-)
433@@ -434,7 +434,7 @@ def batch(self, requests):
434 def send_cli(self, command=None, *args, **kwargs):
435 """Run bitcoin-cli command. Deserializes returned string as python object."""
436 pos_args = [str(arg).lower() if type(arg) is bool else str(arg) for arg in args]
437- named_args = [str(key) + "=" + str(value) for (key, value) in kwargs.items()]
438+ named_args = [str(key) + "=" + (str(value).lower() if type(value) is bool else str(value)) for (key, value) in kwargs.items()]
2596 "\nCreates and loads a new wallet.\n",
2597 {
2598 {"wallet_name", RPCArg::Type::STR, /* opt */ false, /* default_val */ "", "The name for the new wallet. If this is a path, the wallet will be created at the path location."},
2599 {"disable_private_keys", RPCArg::Type::BOOL, /* opt */ true, /* default_val */ "false", "Disable the possibility of private keys (only watchonlys are possible in this mode)."},
2600 {"blank", RPCArg::Type::BOOL, /* opt */ true, /* default_val */ "false", "Create a blank wallet. A blank wallet has no keys or HD seed. One can be set using sethdseed."},
2601+ {"passphrase", RPCArg::Type::STR, /* opt */ true, /* default_val */ "", "Encrypt the wallet with this passphrase. The empty string (\"\") will result in an unencrypted wallet."},
0 {"passphrase", RPCArg::Type::STR, /* default */ "''", "Encrypt the wallet with this passphrase. The empty string (\"\") will result in an unencrypted wallet."},
Mention that the default value is the empty string "\"\"" or "''".
RPCArg::Optional::OMITTED.
testing b69db9a; src/bitcoin-cli createwallet test true false "\"", bitcoind aborts with:
02019-02-15T03:19:38Z Using BerkeleyDB version Berkeley DB 4.8.30: (April 9, 2010)
12019-02-15T03:19:38Z Using wallet /Users/michael/Library/Application Support/Bitcoin/wallets/test
22019-02-15T03:19:38Z BerkeleyEnvironment::Open: LogDir=/Users/michael/Library/Application Support/Bitcoin/wallets/test/database ErrorFile=/Users/michael/Library/Application Support/Bitcoin/wallets/test/db.log
32019-02-15T03:19:38Z init message: Loading wallet...
42019-02-15T03:19:38Z BerkeleyEnvironment::Open: LogDir=/Users/michael/Library/Application Support/Bitcoin/wallets/test/database ErrorFile=/Users/michael/Library/Application Support/Bitcoin/wallets/test/db.log
52019-02-15T03:19:38Z [test] nFileVersion = 179900
62019-02-15T03:19:38Z [test] Keys: 0 plaintext, 0 encrypted, 0 w/ metadata, 0 total. Unknown wallet records: 0
72019-02-15T03:19:38Z [test] Performing wallet upgrade to 169900
82019-02-15T03:19:38Z [test] Wallet completed loading in 111ms
92019-02-15T03:19:38Z [test] setKeyPool.size() = 0
102019-02-15T03:19:38Z [test] mapWallet.size() = 0
112019-02-15T03:19:38Z [test] mapAddressBook.size() = 0
122019-02-15T03:19:38Z [test] Encrypting Wallet with an nDeriveIterations of 253927
132019-02-15T03:19:38Z BerkeleyBatch::Rewrite: Rewriting wallet.dat...
142019-02-15T03:19:38Z BerkeleyEnvironment::Open: LogDir=/Users/michael/Library/Application Support/Bitcoin/wallets/test/database ErrorFile=/Users/michael/Library/Application Support/Bitcoin/wallets/test/db.log
15Assertion failed: (!IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS)), function GenerateNewSeed, file wallet/wallet.cpp, line 1447.
16Process 86762 stopped
17* thread [#2](/bitcoin-bitcoin/2/), name = 'bitcoin-httpworker', stop reason = signal SIGABRT
18 frame [#0](/bitcoin-bitcoin/0/): 0x00007fff5fc1423e libsystem_kernel.dylib`__pthread_kill + 10
19libsystem_kernel.dylib`__pthread_kill:
20-> 0x7fff5fc1423e <+10>: jae 0x7fff5fc14248 ; <+20>
21 0x7fff5fc14240 <+12>: movq %rax, %rdi
22 0x7fff5fc14243 <+15>: jmp 0x7fff5fc0e3b7 ; cerror_nocancel
23 0x7fff5fc14248 <+20>: retq
24Target 0: (bitcoind) stopped.
25(lldb) bt
26* thread [#2](/bitcoin-bitcoin/2/), name = 'bitcoin-httpworker', stop reason = signal SIGABRT
27 * frame [#0](/bitcoin-bitcoin/0/): 0x00007fff5fc1423e libsystem_kernel.dylib`__pthread_kill + 10
28 frame [#1](/bitcoin-bitcoin/1/): 0x00007fff5fccac1c libsystem_pthread.dylib`pthread_kill + 285
29 frame [#2](/bitcoin-bitcoin/2/): 0x00007fff5fb7d1c9 libsystem_c.dylib`abort + 127
30 frame [#3](/bitcoin-bitcoin/3/): 0x00007fff5fb45868 libsystem_c.dylib`__assert_rtn + 320
31 frame [#4](/bitcoin-bitcoin/4/): 0x00000001003031f5 bitcoind`CWallet::GenerateNewSeed(this=<unavailable>) at wallet.cpp:1447 [opt]
32 frame [#5](/bitcoin-bitcoin/5/): 0x00000001002b72e5 bitcoind`createwallet(request=<unavailable>) at rpcwallet.cpp:2649 [opt]
33 frame [#6](/bitcoin-bitcoin/6/): 0x00000001001712aa bitcoind`CRPCTable::execute(this=<unavailable>, request=0x000070000fbe9ad0) const at server.cpp:557 [opt]
34 frame [#7](/bitcoin-bitcoin/7/): 0x000000010001742e bitcoind`HTTPReq_JSONRPC(req=0x0000000118add4a0, (null)=<unavailable>) at httprpc.cpp:192 [opt]
2598@@ -2598,8 +2599,21 @@ static UniValue createwallet(const JSONRPCRequest& request)
2599 flags |= WALLET_FLAG_DISABLE_PRIVATE_KEYS;
2600 }
2601
2602+ bool create_blank = false;
2603 if (!request.params[2].isNull() && request.params[2].get_bool()) {
2604 flags |= WALLET_FLAG_BLANK_WALLET;
2605+ create_blank = true;
2606+ }
2607+ SecureString passphrase;
encryptwallet
2609+ if (!request.params[3].isNull()) {
2610+ passphrase = request.params[3].get_str().c_str();
2611+ if (!passphrase.empty()) {
2612+ if (create_blank) {
2613+ throw JSONRPCError(RPC_WALLET_ERROR, "Cannot create an encrypted empty wallet.");
2614+ }
2607+ SecureString passphrase;
2608+ passphrase.reserve(100);
2609+ if (!request.params[3].isNull()) {
2610+ passphrase = request.params[3].get_str().c_str();
2611+ if (!passphrase.empty()) {
2612+ if (create_blank) {
100+ assert_raises_rpc_error(-4, 'Cannot create an encrypted empty wallet.', self.nodes[0].createwallet, 'w6', False, True, 'thisisapassphrase')
101+ # Encrypted wallet is created
102+ self.nodes[0].createwallet('w6', False, False, 'thisisapassphrase')
103+ w6 = node.get_wallet_rpc('w6')
104+ w6.walletpassphrase('thisisapassphrase', 10)
105+ # There should only be 1000 keys
tACK 2fb944a
I’m tempted to make passphrase the second argument, although that breaks backwards compatibility, so users can more easily ignore the more advanced disable_private_keys and blank arguments.
Maybe we should just start making the RPC examples use named arguments.
2606 }
2607+ SecureString passphrase;
2608+ passphrase.reserve(100);
2609+ if (!request.params[3].isNull()) {
2610+ passphrase = request.params[3].get_str().c_str();
2611+ if (!passphrase.empty()) {
bitcoin-cli createwallet "$NAME" false false "$PASS" that forgot to set the PASS variable or spelled it differently. Better to fail in this case than go on like nothing’s wrong.
2633@@ -2620,6 +2634,25 @@ static UniValue createwallet(const JSONRPCRequest& request)
2634
2635 wallet->postInitProcess();
2636
2637+ // Encrypt the wallet if there's a passphrase
AddWallet? Currently if something fails the wallet is kept.
AddWallet(wallet).
2633@@ -2620,6 +2634,25 @@ static UniValue createwallet(const JSONRPCRequest& request)
2634
2635 wallet->postInitProcess();
2636
2637+ // Encrypt the wallet if there's a passphrase
2638+ if (!passphrase.empty() && !(flags & WALLET_FLAG_DISABLE_PRIVATE_KEYS)) {
2639+ if (!wallet->EncryptWallet(passphrase)) {
2640+ throw JSONRPCError(RPC_WALLET_ENCRYPTION_FAILED, "Error: Wallet created but failed to encrypt.");
0@@ -0,0 +1,4 @@
1+Miscellaneous RPC changes
2+------------
3+
4+- `createwallet` can now create encrypted wallets
... if a non empty passphrase is specified.
2606+ }
2607+ SecureString passphrase;
2608+ passphrase.reserve(100);
2609+ if (!request.params[3].isNull()) {
2610+ passphrase = request.params[3].get_str().c_str();
2611+ if (passphrase.empty()) {
2568@@ -2569,14 +2569,15 @@ static UniValue loadwallet(const JSONRPCRequest& request)
2569
2570 static UniValue createwallet(const JSONRPCRequest& request)
2571 {
2572- if (request.fHelp || request.params.size() < 1 || request.params.size() > 3) {
2573+ if (request.fHelp || request.params.size() < 1 || request.params.size() > 4) {
IsValidNumArgs() rather than manually setting the number of params.
2640+ throw JSONRPCError(RPC_WALLET_ENCRYPTION_FAILED, "Error: Wallet created but failed to encrypt.");
2641+ }
2642+
2643+ if (!create_blank) {
2644+ // Unlock the wallet
2645+ if (!wallet->Unlock(passphrase)) {
95@@ -96,5 +96,22 @@ def run_test(self):
96 assert_raises_rpc_error(-4, "Error: This wallet has no available keys", w5.getnewaddress)
97 assert_raises_rpc_error(-4, "Error: This wallet has no available keys", w5.getrawchangeaddress)
98
99+ self.log.info('New blank and encrypted wallets can be created')
100+ self.nodes[0].createwallet(wallet_name='wblank', disable_private_keys=False, blank=True, passphrase='thisisapassphrase')
101+ wblank = node.get_wallet_rpc('wblank')
102+ wblank.walletpassphrase('thisisapassphrase', 10)
Suggest adding the following tests:
signmessage) and testing that it triggers an error
101+ wblank = node.get_wallet_rpc('wblank')
102+ wblank.walletpassphrase('thisisapassphrase', 10)
103+
104+ self.log.info('Test creating a new encrypted wallet.')
105+ # Born encrypted wallet is created (has keys)
106+ self.nodes[0].createwallet('w6', False, False, 'thisisapassphrase')
104+ self.log.info('Test creating a new encrypted wallet.')
105+ # Born encrypted wallet is created (has keys)
106+ self.nodes[0].createwallet('w6', False, False, 'thisisapassphrase')
107+ w6 = node.get_wallet_rpc('w6')
108+ w6.walletpassphrase('thisisapassphrase', 10)
109+ # There should only be 1 keys
103+
104+ self.log.info('Test creating a new encrypted wallet.')
105+ # Born encrypted wallet is created (has keys)
106+ self.nodes[0].createwallet('w6', False, False, 'thisisapassphrase')
107+ w6 = node.get_wallet_rpc('w6')
108+ w6.walletpassphrase('thisisapassphrase', 10)
2598@@ -2598,7 +2599,20 @@ static UniValue createwallet(const JSONRPCRequest& request)
2599 flags |= WALLET_FLAG_DISABLE_PRIVATE_KEYS;
2600 }
2601
2602+ bool create_blank = false; // Indicate that we the wallet is actually supposed to be blank
IsValidNumArgs is available.
2656+ "\nCreates and loads a new wallet.\n",
2657+ {
2658+ {"wallet_name", RPCArg::Type::STR, RPCArg::Optional::NO, "The name for the new wallet. If this is a path, the wallet will be created at the path location."},
2659+ {"disable_private_keys", RPCArg::Type::BOOL, /* default */ "false", "Disable the possibility of private keys (only watchonlys are possible in this mode)."},
2660+ {"blank", RPCArg::Type::BOOL, /* default */ "false", "Create a blank wallet. A blank wallet has no keys or HD seed. One can be set using sethdseed."},
2661+ {"passphrase", RPCArg::Type::STR, RPCArg::Optional::OMITTED, "Encrypt the wallet with this passphrase."},
2703@@ -2688,6 +2704,29 @@ static UniValue createwallet(const JSONRPCRequest& request)
2704 if (!wallet) {
2705 throw JSONRPCError(RPC_WALLET_ERROR, "Wallet creation failed.");
2706 }
2707+
2708+ // Encrypt the wallet if there's a passphrase
2709+ if (!passphrase.empty() && !(flags & WALLET_FLAG_DISABLE_PRIVATE_KEYS)) {
disable_private_keys=true and passphrase=something. If they provide that input, they’re expecting the wallet to be encrypted somehow. Better to error out and say “wallets without private keys cannot be encrypted” than just swallow the passphrase silently.
95@@ -96,5 +96,28 @@ def run_test(self):
96 assert_raises_rpc_error(-4, "Error: This wallet has no available keys", w5.getnewaddress)
97 assert_raises_rpc_error(-4, "Error: This wallet has no available keys", w5.getrawchangeaddress)
98
99+ self.log.info('New blank and encrypted wallets can be created')
100+ self.nodes[0].createwallet(wallet_name='wblank', disable_private_keys=False, blank=True, passphrase='thisisapassphrase')
101+ wblank = node.get_wallet_rpc('wblank')
102+ assert_raises_rpc_error(-13, "Error: Please enter the wallet passphrase with walletpassphrase first.", wblank.signmessage, "needanargument", "test")
micronit: I always prefer named arguments in tests, so there’s no ambiguity about what the RPC method call should be. In that case it’d be ...wblank.signmessage, address="fakeaddress", message="test" here.
EDIT: assert_raises_rpc_error() itself takes an argument called message, so this doesn’t work. We should fix that (in a different PR) by making assert_raises_rpc_error’s argument something like _error_message so it doesn’t collide with any RPC arguments.
115+ # There should only be 1 key
116+ walletinfo = w6.getwalletinfo()
117+ assert_equal(walletinfo['keypoolsize'], 1)
118+ assert_equal(walletinfo['keypoolsize_hd_internal'], 1)
119+ # Empty passphrase, error
120+ assert_raises_rpc_error(-16, 'Cannot encrypt a wallet with a blank password', self.nodes[0].createwallet, 'w7', False, False, '')
micronit: again, I’d prefer named arguments here (and dropping the unneeded optional arguments):
0assert_raises_rpc_error(-16, 'Cannot encrypt a wallet with a blank password', self.nodes[0].createwallet, wallet_name='w7', passphrase='')
Looks great. utACK 662d1171d9e29964b039ba4c5bc8a2304426c003
I’ve left a few nits. Feel free to take or leave.