Merge #16394: Allow createwallet to take empty passwords to make unencrypted wallets

c5d37873677551caac34752214dd491f5278c8d5 Allow createwallet to take empty passwords to make unencrypted wallets (Andrew Chow)

Pull request description:

  Allow createwallet to take the empty string as a password and interpret that as leaving the wallet unencrypted. Also warn when that happens.

  This fixes a bug where it was not possible to use the `avoid_reuse` option for new unencrypted wallets without using named arguments.Thus this allows more `createwallet` options to be added that can be set on unencrypted wallets when using positional arguments.

ACKs for top commit:
  jnewbery:
    code review ACK c5d37873677551caac34752214dd491f5278c8d5
  meshcollider:
    re-utACK c5d37873677551caac34752214dd491f5278c8d5
  ryanofsky:
    utACK c5d37873677551caac34752214dd491f5278c8d5. Changes since last review are rebasing, concatenating warning strings to avoid discarding warnings, adding release notes, and choosing an unambiguous wallet name for the test.

Tree-SHA512: 146737a728dd614ba94d4b166b27e8c9e195badd1709ccab2315afe59176d9b493dfba9b61c3ed81090f059c7e464d709deb06d99451b9a3fff667f527d6f7c9

3 files changed, 27 insertions, 7 deletions

diff --git a/doc/release-notes-16394.md b/doc/release-notes-16394.md
new file mode 100644
index 0000000000..f09cba4b6d
--- /dev/null
+++ b/doc/release-notes-16394.md
@@ -0,0 +1,4 @@
+RPC changes
+-----------
+`createwallet` now returns a warning if an empty string is used as an encryption password, and does not encrypt the wallet, instead of raising an error.
+This makes it easier to disable encryption but also specify other options when using the `bitcoin-cli` tool.
diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp
index bf72ef6b16..a86e93481c 100644
--- a/src/wallet/rpcwallet.cpp
+++ b/src/wallet/rpcwallet.cpp
@@ -2676,11 +2676,12 @@ static UniValue createwallet(const JSONRPCRequest& request)
     }
     SecureString passphrase;
     passphrase.reserve(100);
+    std::string warning;
     if (!request.params[3].isNull()) {
         passphrase = request.params[3].get_str().c_str();
         if (passphrase.empty()) {
-            // Empty string is invalid
-            throw JSONRPCError(RPC_WALLET_ENCRYPTION_FAILED, "Cannot encrypt a wallet with a blank password");
+            // Empty string means unencrypted
+            warning = "Empty string given as passphrase, wallet will not be encrypted.";
         }
     }
 
@@ -2689,9 +2690,9 @@ static UniValue createwallet(const JSONRPCRequest& request)
     }
 
     std::string error;
-    std::string warning;
+    std::string create_warning;
     std::shared_ptr<CWallet> wallet;
-    WalletCreationStatus status = CreateWallet(*g_rpc_interfaces->chain, passphrase, flags, request.params[0].get_str(), error, warning, wallet);
+    WalletCreationStatus status = CreateWallet(*g_rpc_interfaces->chain, passphrase, flags, request.params[0].get_str(), error, create_warning, wallet);
     switch (status) {
         case WalletCreationStatus::CREATION_FAILED:
             throw JSONRPCError(RPC_WALLET_ERROR, error);
@@ -2702,6 +2703,12 @@ static UniValue createwallet(const JSONRPCRequest& request)
         // no default case, so the compiler can warn about missing cases
     }
 
+    if (warning.empty()) {
+        warning = create_warning;
+    } else if (!warning.empty() && !create_warning.empty()){
+        warning += "; " + create_warning;
+    }
+
     UniValue obj(UniValue::VOBJ);
     obj.pushKV("name", wallet->GetName());
     obj.pushKV("warning", warning);
diff --git a/test/functional/wallet_createwallet.py b/test/functional/wallet_createwallet.py
index 294f90a0fa..e302e499f4 100755
--- a/test/functional/wallet_createwallet.py
+++ b/test/functional/wallet_createwallet.py
@@ -116,11 +116,20 @@ class CreateWalletTest(BitcoinTestFramework):
         walletinfo = w6.getwalletinfo()
         assert_equal(walletinfo['keypoolsize'], 1)
         assert_equal(walletinfo['keypoolsize_hd_internal'], 1)
-        # Empty passphrase, error
-        assert_raises_rpc_error(-16, 'Cannot encrypt a wallet with a blank password', self.nodes[0].createwallet, 'w7', False, False, '')
+        # Allow empty passphrase, but there should be a warning
+        resp = self.nodes[0].createwallet(wallet_name='w7', disable_private_keys=False, blank=False, passphrase='')
+        assert_equal(resp['warning'], 'Empty string given as passphrase, wallet will not be encrypted.')
+        w7 = node.get_wallet_rpc('w7')
+        assert_raises_rpc_error(-15, 'Error: running with an unencrypted wallet, but walletpassphrase was called.', w7.walletpassphrase, '', 10)
+
+        self.log.info('Test making a wallet with avoid reuse flag')
+        self.nodes[0].createwallet('w8', False, False, '', True) # Use positional arguments to check for bug where avoid_reuse could not be set for wallets without needing them to be encrypted
+        w8 = node.get_wallet_rpc('w8')
+        assert_raises_rpc_error(-15, 'Error: running with an unencrypted wallet, but walletpassphrase was called.', w7.walletpassphrase, '', 10)
+        assert_equal(w8.getwalletinfo()["avoid_reuse"], True)
 
         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')
+        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='w9', disable_private_keys=True, passphrase='thisisapassphrase')
 
 if __name__ == '__main__':
     CreateWalletTest().main()