Merge pull request dashpay#5198 from kittywhiskers/sqlite_tests

test: adjust functional tests to account for SQLite-only builds, add CI run
vijaydasmp · Feb 17, 2023 · e50d96d · e50d96d
2 parents 0923511 + 106f2cb
commit e50d96d
Show file tree

Hide file tree

Showing 24 changed files with 212 additions and 129 deletions.
diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml
@@ -251,6 +251,13 @@ linux64_cxx20-build:
   variables:
     BUILD_TARGET: linux64_cxx20
 
+linux64_sqlite-build:
+  extends: .build-template
+  needs:
+    - x86_64-pc-linux-gnu-debug
+  variables:
+    BUILD_TARGET: linux64_sqlite
+
 linux64_fuzz-build:
   extends: .build-template
   needs:
@@ -312,6 +319,13 @@ linux64-test:
   variables:
     BUILD_TARGET: linux64
 
+linux64_sqlite-test:
+  extends: .test-template
+  needs:
+    - linux64_sqlite-build
+  variables:
+    BUILD_TARGET: linux64_sqlite
+
 linux64_tsan-test:
   extends:
     - .test-template

diff --git a/ci/dash/matrix.sh b/ci/dash/matrix.sh
@@ -30,6 +30,8 @@ elif [ "$BUILD_TARGET" = "linux64_fuzz" ]; then
   source ./ci/test/00_setup_env_native_fuzz.sh
 elif [ "$BUILD_TARGET" = "linux64_cxx20" ]; then
   source ./ci/test/00_setup_env_native_cxx20.sh
+elif [ "$BUILD_TARGET" = "linux64_sqlite" ]; then
+  source ./ci/test/00_setup_env_native_sqlite.sh
 elif [ "$BUILD_TARGET" = "linux64_nowallet" ]; then
   source ./ci/test/00_setup_env_native_nowallet.sh
 elif [ "$BUILD_TARGET" = "mac" ]; then

diff --git a/ci/test/00_setup_env_native_sqlite.sh b/ci/test/00_setup_env_native_sqlite.sh
@@ -0,0 +1,12 @@
+#!/usr/bin/env bash
+#
+# Copyright (c) 2019 The Bitcoin Core developers
+# Distributed under the MIT software license, see the accompanying
+# file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+export LC_ALL=C.UTF-8
+
+export PACKAGES="python3-zmq qtbase5-dev qttools5-dev-tools libdbus-1-dev libharfbuzz-dev"
+export DEP_OPTS="NO_BDB=1 NO_UPNP=1 DEBUG=1"
+export GOAL="install"
+export BITCOIN_CONFIG="--enable-zmq --enable-glibc-back-compat --enable-reduce-exports --with-sqlite --without-bdb LDFLAGS=-static-libstdc++"
diff --git a/src/dummywallet.cpp b/src/dummywallet.cpp
@@ -40,7 +40,6 @@ void DummyWalletInit::AddWalletOptions(ArgsManager& argsman) const
         "-rescan=<mode>",
         "-salvagewallet",
         "-spendzeroconfchange",
-        "-upgradewallet",
         "-wallet=<path>",
         "-walletbackupsdir=<dir>",
         "-walletbroadcast",

diff --git a/src/qt/forms/debugwindow.ui b/src/qt/forms/debugwindow.ui
@@ -1465,36 +1465,13 @@
         </widget>
        </item>
        <item row="5" column="0">
-        <widget class="QPushButton" name="btn_upgradewallet">
-         <property name="minimumSize">
-          <size>
-           <width>180</width>
-           <height>23</height>
-          </size>
-         </property>
-         <property name="text">
-          <string>Upgrade wallet format</string>
-         </property>
-        </widget>
-       </item>
-       <item row="5" column="1">
-        <widget class="QLabel" name="label_repair_upgrade">
-         <property name="text">
-          <string>-upgradewallet: Upgrade wallet to latest format on startup. (Note: this is NOT an update of the wallet itself!)</string>
-         </property>
-         <property name="wordWrap">
-          <bool>true</bool>
-         </property>
-        </widget>
-       </item>
-       <item row="6" column="0">
         <widget class="QPushButton" name="btn_reindex">
          <property name="text">
           <string>Rebuild index</string>
          </property>
         </widget>
        </item>
-       <item row="6" column="1">
+       <item row="5" column="1">
         <widget class="QLabel" name="label_repair_reindex">
          <property name="text">
           <string>-reindex: Rebuild block chain index from current blk000??.dat files.</string>
@@ -1504,7 +1481,7 @@
          </property>
         </widget>
        </item>
-       <item row="7" column="1">
+       <item row="6" column="1">
         <spacer name="verticalSpacerRepair">
          <property name="orientation">
           <enum>Qt::Vertical</enum>

diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp
@@ -60,7 +60,6 @@ const TrafficGraphData::GraphRange INITIAL_TRAFFIC_GRAPH_SETTING = TrafficGraphD
 // Repair parameters
 const QString RESCAN1("-rescan=1");
 const QString RESCAN2("-rescan=2");
-const QString UPGRADEWALLET("-upgradewallet");
 const QString REINDEX("-reindex");
 
 namespace {
@@ -496,10 +495,8 @@ RPCConsole::RPCConsole(interfaces::Node& node, QWidget* parent, Qt::WindowFlags
     // Disable wallet repair options that require a wallet (enable them later when a wallet is added)
     ui->btn_rescan1->setEnabled(false);
     ui->btn_rescan2->setEnabled(false);
-    ui->btn_upgradewallet->setEnabled(false);
     connect(ui->btn_rescan1, &QPushButton::clicked, this, &RPCConsole::walletRescan1);
     connect(ui->btn_rescan2, &QPushButton::clicked, this, &RPCConsole::walletRescan2);
-    connect(ui->btn_upgradewallet, &QPushButton::clicked, this, &RPCConsole::walletUpgrade);
     connect(ui->btn_reindex, &QPushButton::clicked, this, &RPCConsole::walletReindex);
 
     // Register RPC timer interface
@@ -726,7 +723,6 @@ void RPCConsole::addWallet(WalletModel * const walletModel)
         // The only loaded wallet
         ui->btn_rescan1->setEnabled(true);
         ui->btn_rescan2->setEnabled(true);
-        ui->btn_upgradewallet->setEnabled(true);
         QString wallet_path = QString::fromStdString(GetWalletDir().string() + QDir::separator().toLatin1());
         QString wallet_name = walletModel->getWalletName().isEmpty() ? "wallet.dat" : walletModel->getWalletName();
         ui->wallet_path->setText(wallet_path + wallet_name);
@@ -736,7 +732,6 @@ void RPCConsole::addWallet(WalletModel * const walletModel)
         // No wallet recovery for multiple loaded wallets
         ui->btn_rescan1->setEnabled(false);
         ui->btn_rescan2->setEnabled(false);
-        ui->btn_upgradewallet->setEnabled(false);
         ui->wallet_path->clear();
     }
 }
@@ -750,7 +745,6 @@ void RPCConsole::removeWallet(WalletModel * const walletModel)
         // Back to the only loaded wallet
         ui->btn_rescan1->setEnabled(true);
         ui->btn_rescan2->setEnabled(true);
-        ui->btn_upgradewallet->setEnabled(true);
         WalletModel* wallet_model = ui->WalletSelector->itemData(1).value<WalletModel*>();
         QString wallet_path = QString::fromStdString(GetWalletDir().string() + QDir::separator().toLatin1());
         QString wallet_name = wallet_model->getWalletName().isEmpty() ? "wallet.dat" : wallet_model->getWalletName();
@@ -759,7 +753,6 @@ void RPCConsole::removeWallet(WalletModel * const walletModel)
         // No wallet recovery for multiple loaded wallets
         ui->btn_rescan1->setEnabled(false);
         ui->btn_rescan2->setEnabled(false);
-        ui->btn_upgradewallet->setEnabled(false);
         ui->wallet_path->clear();
     }
 }
@@ -821,12 +814,6 @@ void RPCConsole::walletRescan2()
     buildParameterlist(RESCAN2);
 }
 
-/** Restart wallet with "-upgradewallet" */
-void RPCConsole::walletUpgrade()
-{
-    buildParameterlist(UPGRADEWALLET);
-}
-
 /** Restart wallet with "-reindex" */
 void RPCConsole::walletReindex()
 {
@@ -852,7 +839,6 @@ void RPCConsole::buildParameterlist(QString arg)
     // Remove existing repair-options
     args.removeAll(RESCAN1);
     args.removeAll(RESCAN2);
-    args.removeAll(UPGRADEWALLET);
     args.removeAll(REINDEX);
 
     // Append repair parameter to command line.

diff --git a/src/qt/rpcconsole.h b/src/qt/rpcconsole.h
@@ -108,7 +108,6 @@ public Q_SLOTS:
     /** Wallet repair options */
     void walletRescan1();
     void walletRescan2();
-    void walletUpgrade();
     void walletReindex();
 
     /** Append the message to the message widget */

diff --git a/src/rpc/client.cpp b/src/rpc/client.cpp
@@ -182,6 +182,7 @@ static const CRPCConvertParam vRPCConvertParams[] =
     { "getspecialtxes", 3, "skip" },
     { "getspecialtxes", 4, "verbosity" },
     { "disconnectnode", 1, "nodeid" },
+    { "upgradewallet", 0, "version" },
     // Echo with conversion (For testing only)
     { "echojson", 0, "arg0" },
     { "echojson", 1, "arg1" },

diff --git a/src/wallet/init.cpp b/src/wallet/init.cpp
@@ -62,7 +62,6 @@ void WalletInit::AddWalletOptions(ArgsManager& argsman) const
     argsman.AddArg("-rescan=<mode>", "Rescan the block chain for missing wallet transactions on startup"
                                             " (1 = start from wallet creation time, 2 = start from genesis block)", ArgsManager::ALLOW_ANY, OptionsCategory::WALLET);
     argsman.AddArg("-spendzeroconfchange", strprintf("Spend unconfirmed change when sending transactions (default: %u)", DEFAULT_SPEND_ZEROCONF_CHANGE), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET);
-    argsman.AddArg("-upgradewallet", "Upgrade wallet to latest format on startup", ArgsManager::ALLOW_ANY, OptionsCategory::WALLET);
     argsman.AddArg("-wallet=<path>", "Specify wallet path to load at startup. Can be used multiple times to load multiple wallets. Path is to a directory containing wallet data and log files. If the path is not absolute, it is interpreted relative to <walletdir>. This only loads existing wallets and does not create new ones. For backwards compatibility this also accepts names of existing top-level data files in <walletdir>.", ArgsManager::ALLOW_ANY | ArgsManager::NETWORK_ONLY, OptionsCategory::WALLET);
     argsman.AddArg("-walletbackupsdir=<dir>", "Specify full path to directory for automatic wallet backups (must exist)", ArgsManager::ALLOW_ANY, OptionsCategory::WALLET);
     argsman.AddArg("-walletbroadcast", strprintf("Make the wallet broadcast transactions (default: %u)", DEFAULT_WALLETBROADCAST), ArgsManager::ALLOW_ANY, OptionsCategory::WALLET);
@@ -129,8 +128,6 @@ bool WalletInit::ParameterInteraction() const
         return InitError(_("You can not start a masternode with wallet enabled."));
     }
 
-    const bool is_multiwallet = gArgs.GetArgs("-wallet").size() > 1;
-
     if (gArgs.GetBoolArg("-blocksonly", DEFAULT_BLOCKSONLY) && gArgs.SoftSetBoolArg("-walletbroadcast", false)) {
         LogPrintf("%s: parameter interaction: -blocksonly=1 -> setting -walletbroadcast=0\n", __func__);
     }
@@ -146,12 +143,6 @@ bool WalletInit::ParameterInteraction() const
         gArgs.ForceRemoveArg("rescan");
     }
 
-    if (is_multiwallet) {
-        if (gArgs.GetBoolArg("-upgradewallet", false)) {
-            return InitError(strprintf(_("%s is only allowed with a single wallet file"), "-upgradewallet"));
-        }
-    }
-
     if (gArgs.GetBoolArg("-sysperms", false))
         return InitError(Untranslated("-sysperms is not allowed in combination with enabled wallet functionality"));
 

diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp
@@ -2748,7 +2748,7 @@ static UniValue loadwallet(const JSONRPCRequest& request)
     RPCHelpMan{"loadwallet",
         "\nLoads a wallet from a wallet file or directory."
         "\nNote that all wallet command-line options used when starting dashd will be"
-        "\napplied to the new wallet (eg -upgradewallet, rescan, etc).\n",
+        "\napplied to the new wallet (eg, rescan, etc).\n",
         {
             {"filename", RPCArg::Type::STR, RPCArg::Optional::NO, "The wallet directory or .dat file."},
             {"load_on_startup", RPCArg::Type::BOOL, /* default */ "null", "Save wallet name to persistent settings and load on startup. True to add wallet to startup list, false to remove, null to leave unchanged."},
@@ -4064,6 +4064,42 @@ UniValue walletcreatefundedpsbt(const JSONRPCRequest& request)
     return result;
 }
 
+static UniValue upgradewallet(const JSONRPCRequest& request)
+{
+    RPCHelpMan{"upgradewallet",
+        "\nUpgrade the wallet. Upgrades to the latest version if no version number is specified\n"
+        "New keys may be generated and a new wallet backup will need to be made.",
+        {
+            {"version", RPCArg::Type::NUM, /* default */ strprintf("%d", FEATURE_LATEST), "The version number to upgrade to. Default is the latest wallet version"}
+        },
+        RPCResults{},
+        RPCExamples{
+            HelpExampleCli("upgradewallet", "120200")
+            + HelpExampleRpc("upgradewallet", "120200")
+        }
+    }.Check(request);
+
+    std::shared_ptr<CWallet> const wallet = GetWalletForJSONRPCRequest(request);
+    if (!wallet) return NullUniValue;
+    CWallet* const pwallet = wallet.get();
+
+    RPCTypeCheck(request.params, {UniValue::VNUM}, true);
+
+    EnsureWalletIsUnlocked(pwallet);
+
+    int version = 0;
+    if (!request.params[0].isNull()) {
+        version = request.params[0].get_int();
+    }
+
+    bilingual_str error;
+    std::vector<bilingual_str> warnings;
+    if (!pwallet->UpgradeWallet(version, error, warnings)) {
+        throw JSONRPCError(RPC_WALLET_ERROR, error.original);
+    }
+    return error.original;
+}
+
 // clang-format off
 static const CRPCCommand commands[] =
 { //  category              name                        actor (function)           argNames
@@ -4123,6 +4159,7 @@ static const CRPCCommand commands[] =
     { "wallet",             "signmessage",                      &signmessage,                   {"address","message"} },
     { "wallet",             "signrawtransactionwithwallet",     &signrawtransactionwithwallet,  {"hexstring","prevtxs","sighashtype"} },
     { "wallet",             "unloadwallet",                     &unloadwallet,                  {"wallet_name", "load_on_startup"} },
+    { "wallet",             "upgradewallet",                    &upgradewallet,                 {"version"} },
     { "wallet",             "upgradetohd",                      &upgradetohd,                   {"mnemonic", "mnemonicpassphrase", "walletpassphrase", "rescan"} },
     { "wallet",             "walletlock",                       &walletlock,                    {} },
     { "wallet",             "walletpassphrasechange",           &walletpassphrasechange,        {"oldpassphrase","newpassphrase"} },

diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
@@ -4306,27 +4306,9 @@ std::shared_ptr<CWallet> CWallet::Create(interfaces::Chain& chain, const std::st
         }
     }
 
-    if (gArgs.GetBoolArg("-upgradewallet", fFirstRun))
-    {
-        int nMaxVersion = gArgs.GetArg("-upgradewallet", 0);
-        auto nMinVersion = DEFAULT_USE_HD_WALLET ? FEATURE_LATEST : FEATURE_COMPRPUBKEY;
-        if (nMaxVersion == 0) // the -upgradewallet without argument case
-        {
-            walletInstance->WalletLogPrintf("Performing wallet upgrade to %i\n", nMinVersion);
-            nMaxVersion = FEATURE_LATEST;
-            walletInstance->SetMinVersion(nMinVersion); // permanently upgrade the wallet immediately
-        }
-        else
-            walletInstance->WalletLogPrintf("Allowing wallet upgrade up to %i\n", nMaxVersion);
-        if (nMaxVersion < walletInstance->GetVersion())
-        {
-            return unload_wallet(_("Cannot downgrade wallet"));
-        }
-        walletInstance->SetMaxVersion(nMaxVersion);
-    }
-
     if (fFirstRun)
     {
+        walletInstance->SetMaxVersion(FEATURE_LATEST);
         walletInstance->SetWalletFlags(wallet_creation_flags, false);
         if (!(wallet_creation_flags & (WALLET_FLAG_DISABLE_PRIVATE_KEYS | WALLET_FLAG_BLANK_WALLET))) {
             // Create new HD chain
@@ -4596,6 +4578,27 @@ std::shared_ptr<CWallet> CWallet::Create(interfaces::Chain& chain, const std::st
     return walletInstance;
 }
 
+bool CWallet::UpgradeWallet(int version, bilingual_str& error, std::vector<bilingual_str>& warnings)
+{
+    int nMaxVersion = version;
+    auto nMinVersion = DEFAULT_USE_HD_WALLET ? FEATURE_LATEST : FEATURE_COMPRPUBKEY;
+    if (nMaxVersion == 0) {
+        WalletLogPrintf("Performing wallet upgrade to %i\n", nMinVersion);
+        nMaxVersion = FEATURE_LATEST;
+        SetMinVersion(nMinVersion); // permanently upgrade the wallet immediately
+    } else {
+        WalletLogPrintf("Allowing wallet upgrade up to %i\n", nMaxVersion);
+    }
+
+    if (nMaxVersion < GetVersion()) {
+        error = Untranslated("Cannot downgrade wallet");
+        return false;
+    }
+
+    SetMaxVersion(nMaxVersion);
+    return true;
+}
+
 void CWallet::postInitProcess()
 {
     LOCK(cs_wallet);

diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h
@@ -1241,6 +1241,9 @@ class CWallet final : public WalletStorage, public interfaces::Chain::Notificati
         LogPrintf(("%s " + fmt).c_str(), GetDisplayName(), parameters...);
     };
 
+    /** Upgrade the wallet */
+    bool UpgradeWallet(int version, bilingual_str& error, std::vector<bilingual_str>& warnings);
+
     /** Get last block processed height */
     int GetLastBlockHeight() const EXCLUSIVE_LOCKS_REQUIRED(cs_wallet)
     {

diff --git a/test/config.ini.in b/test/config.ini.in
@@ -16,6 +16,8 @@ RPCAUTH=@abs_top_srcdir@/share/rpcauth/rpcauth.py
 [components]
 # Which components are enabled. These are commented out by `configure` if they were disabled when running config.
 @ENABLE_WALLET_TRUE@ENABLE_WALLET=true
+@USE_SQLITE_TRUE@USE_SQLITE=true
+@USE_BDB_TRUE@USE_BDB=true
 @BUILD_BITCOIN_CLI_TRUE@ENABLE_CLI=true
 @BUILD_BITCOIN_WALLET_TRUE@ENABLE_WALLET_TOOL=true
 @BUILD_BITCOIND_TRUE@ENABLE_BITCOIND=true

diff --git a/test/functional/feature_filelock.py b/test/functional/feature_filelock.py
@@ -4,6 +4,8 @@
 # file COPYING or http://www.opensource.org/licenses/mit-license.php.
 """Check that it's not possible to start a second bitcoind instance using the same datadir or wallet."""
 import os
+import random
+import string
 
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.test_node import ErrorMatch
@@ -27,11 +29,14 @@ def run_test(self):
         self.nodes[1].assert_start_raises_init_error(extra_args=['-datadir={}'.format(self.nodes[0].datadir), '-noserver'], expected_msg=expected_msg)
 
         if self.is_wallet_compiled():
-            self.nodes[0].createwallet(self.default_wallet_name)
+            wallet_name = ''.join([random.choice(string.ascii_lowercase) for _ in range(6)])
+            self.nodes[0].createwallet(wallet_name=wallet_name)
             wallet_dir = os.path.join(datadir, 'wallets')
             self.log.info("Check that we can't start a second dashd instance using the same wallet")
-            expected_msg = "Error: Error initializing wallet database environment"
-            self.nodes[1].assert_start_raises_init_error(extra_args=['-walletdir={}'.format(wallet_dir), '-wallet=' + self.default_wallet_name, '-noserver'], expected_msg=expected_msg, match=ErrorMatch.PARTIAL_REGEX)
+            expected_msg = "Error: SQLiteDatabase: Unable to obtain an exclusive lock on the database, is it being used by another dashd?"
+            if self.is_bdb_compiled():
+                expected_msg = "Error: Error initializing wallet database environment"
+            self.nodes[1].assert_start_raises_init_error(extra_args=['-walletdir={}'.format(wallet_dir), '-wallet=' + wallet_name, '-noserver'], expected_msg=expected_msg, match=ErrorMatch.PARTIAL_REGEX)
 
 if __name__ == '__main__':
     FilelockTest().main()
diff --git a/test/functional/feature_notifications.py b/test/functional/feature_notifications.py
@@ -48,6 +48,12 @@ def setup_network(self):
         super().setup_network()
 
     def run_test(self):
+        if self.is_wallet_compiled():
+            # Make the wallets
+            # Ensures that node 0 and node 1 share the same wallet for the conflicting transaction tests below.
+            for i, name in enumerate(self.wallet_names):
+                self.nodes[i].createwallet(wallet_name=name, load_on_startup=True)
+
         self.log.info("test -blocknotify")
         block_count = 10
         blocks = self.nodes[1].generatetoaddress(block_count, self.nodes[1].getnewaddress() if self.is_wallet_compiled() else ADDRESS_BCRT1_UNSPENDABLE)