Merge #2725: [test] Rewrite rpc_wallet_encrypted_wallet_sapzkeys unit…

… test as functional test

684c1b5 test: rewrite rpc_wallet_encrypted_wallet_sapzkeys unit test as functional test (furszy)

Pull request description:

  As this test requires a complete real wallet, and not a mocked one, and accesses the node through the RPC server commands in order to perform the sapling keys/addresses checks on an encrypted wallet, it's by definition a functional test.

  This solves the unneeded headaches that we are having with the unit test. Which is failing sometimes for a missing base chainparams initialization which has nothing to do with the test purpose.

ACKs for top commit:
  random-zebra:
    ACK 684c1b5

Tree-SHA512: 5aa8a10c82b357b1d28725dd688fb1d12b12525279acde22aad3d7f90a2b10896f633bcc1c89660f181242387534c5c4d0cfb707aa16df4906a24e8fc516676a

src/test/librust/sapling_rpc_wallet_tests.cpp

@@ -498,77 +498,6 @@ BOOST_AUTO_TEST_CASE(rpc_shieldsendmany_taddr_to_sapling)
     vpwallets.erase(vpwallets.begin());
 }
 
-struct RealWalletTestingSetup : public WalletTestingSetupBase
-{
-    RealWalletTestingSetup() : WalletTestingSetupBase(CBaseChainParams::MAIN,
-                                                      "test_wallet",
-                                                      WalletDatabase::Create(fs::absolute("test_wallet", GetWalletDir()))) {};
-};
-
-BOOST_FIXTURE_TEST_CASE(rpc_wallet_encrypted_wallet_sapzkeys, RealWalletTestingSetup)
-{
-    UniValue retValue;
-    int n = 100;
-
-    {
-        LOCK(m_wallet.cs_wallet);
-        m_wallet.SetMinVersion(FEATURE_SAPLING);
-        m_wallet.SetupSPKM(false);
-    }
-    vpwallets.insert(vpwallets.begin(), &m_wallet);
-
-    // wallet should currently be empty
-    std::set<libzcash::SaplingPaymentAddress> addrs;
-    m_wallet.GetSaplingPaymentAddresses(addrs);
-    BOOST_CHECK(addrs.empty());
-
-    // create keys
-    for (int i = 0; i < n; i++) {
-        CallRPC("getnewshieldaddress");
-    }
-
-    // Verify we can list the keys imported
-    BOOST_CHECK_NO_THROW(retValue = CallRPC("listshieldaddresses"));
-    UniValue arr = retValue.get_array();
-    BOOST_CHECK((int) arr.size() == n);
-
-    // Verify that the wallet encryption RPC is disabled
-    // TODO: We don't have the experimental mode to disable the encryptwallet disable.
-    //BOOST_CHECK_THROW(CallRPC("encryptwallet passphrase"), std::runtime_error);
-
-    // Encrypt the wallet (we can't call RPC encryptwallet as that shuts down node)
-    SecureString strWalletPass;
-    strWalletPass.reserve(100);
-    strWalletPass = "hello";
-
-    PushCurrentDirectory push_dir(gArgs.GetArg("-datadir","/tmp/thisshouldnothappen"));
-    BOOST_CHECK(m_wallet.EncryptWallet(strWalletPass));
-    BOOST_CHECK(m_wallet.IsCrypted());
-
-    // Verify we can still list the keys imported
-    BOOST_CHECK_NO_THROW(retValue = CallRPC("listshieldaddresses"));
-    arr = retValue.get_array();
-    BOOST_CHECK((int) arr.size() == n);
-
-    // Try to add a new key, but we can't as the wallet is locked
-    BOOST_CHECK_THROW(CallRPC("getnewshieldaddress"), std::runtime_error);
-
-    // We can't call RPC walletpassphrase as that invokes RPCRunLater which breaks tests.
-    // So we manually unlock.
-    BOOST_CHECK(m_wallet.Unlock(strWalletPass));
-    BOOST_CHECK(m_wallet.IsCrypted());
-
-    // Now add a key
-    BOOST_CHECK_NO_THROW(CallRPC("getnewshieldaddress"));
-
-    // Verify the key has been added
-    BOOST_CHECK_NO_THROW(retValue = CallRPC("listshieldaddresses"));
-    arr = retValue.get_array();
-    BOOST_CHECK((int) arr.size() == n+1);
-
-    vpwallets.erase(vpwallets.begin());
-}
-
 BOOST_AUTO_TEST_CASE(rpc_listshieldunspent_parameters)
 {
     {

test/functional/sapling_wallet_encryption.py

@@ -0,0 +1,62 @@
+#!/usr/bin/env python3
+# Copyright (c) 2021 The PIVX developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or https://www.opensource.org/licenses/mit-license.php .
+
+from test_framework.test_framework import PivxTestFramework
+from test_framework.util import (
+    assert_equal,
+    assert_raises_rpc_error,
+    assert_true
+)
+
+# Test encrypted wallet behaviour with Sapling addresses
+class WalletSaplingTest(PivxTestFramework):
+
+    def set_test_params(self):
+        self.num_nodes = 1
+        self.setup_clean_chain = True
+
+    def run_test(self):
+        node = self.nodes[0]
+        node.generate(1)
+
+        # wallet should currently be empty
+        assert_equal(len(node.listshieldaddresses()), 0)
+        # create 100 keys
+        keys_to_generate = 100
+        for _ in range (keys_to_generate):
+            node.getnewshieldaddress()
+        # Verify we can list the created addresses
+        addr_list = node.listshieldaddresses()
+        assert_equal(len(addr_list), keys_to_generate)
+
+        password = "hello"
+        node.encryptwallet(password)
+        # assert that the wallet is encrypted
+        assert_raises_rpc_error(-15, "Error: running with an encrypted wallet, but encryptwallet was called.", node.encryptwallet, 'ff')
+
+        # now verify addresses again
+        addr_list_post_encryption = node.listshieldaddresses()
+        assert_equal(len(addr_list_post_encryption), keys_to_generate)
+        assert_equal(addr_list, addr_list_post_encryption)
+
+        # try to add a new key, but we can't as the wallet is locked
+        assert_raises_rpc_error(-13, "Error: Please enter the wallet passphrase with walletpassphrase first.", node.getnewshieldaddress)
+
+        # now unlock the wallet and try to generate the key again
+        node.walletpassphrase(password, 12000)
+        new_addr = node.getnewshieldaddress()
+        assert(new_addr is not None)
+
+        # and verify that the key has been added
+        found = False
+        addr_list_post_encryption = node.listshieldaddresses()
+        assert_equal(len(addr_list_post_encryption), keys_to_generate + 1)
+        for addr in addr_list_post_encryption:
+            if addr == new_addr:
+                found = True
+        assert_true(found, "error: shield address not found in encrypted wallet")
+
+if __name__ == '__main__':
+    WalletSaplingTest().main()

test/functional/test_runner.py

@@ -116,6 +116,7 @@
     'rpc_deprecated.py',                        # ~ 80 sec
     'interface_bitcoin_cli.py',                 # ~ 80 sec
     'mempool_packages.py',                      # ~ 63 sec
+    'sapling_wallet_encryption.py',
 
     # vv Tests less than 60s vv
     'rpc_users.py',
@@ -238,6 +239,7 @@
     'wallet_importmulti.py',
     'wallet_import_rescan.py',
     'wallet_multiwallet.py',
+    'sapling_wallet_encryption.py'
 ]
 
 # Place the lists with the longest tests (on average) first