diff options
59 files changed, 202 insertions, 190 deletions
diff --git a/ci/test/00_setup_env_i686_centos.sh b/ci/test/00_setup_env_i686_centos.sh index e8f963c8a9..8a931d44e5 100755 --- a/ci/test/00_setup_env_i686_centos.sh +++ b/ci/test/00_setup_env_i686_centos.sh @@ -12,6 +12,7 @@ export CI_IMAGE_NAME_TAG=quay.io/centos/centos:stream8 export CI_BASE_PACKAGES="gcc-c++ glibc-devel.x86_64 libstdc++-devel.x86_64 glibc-devel.i686 libstdc++-devel.i686 ccache libtool make git python38 python38-pip which patch lbzip2 xz procps-ng dash rsync coreutils bison" export PIP_PACKAGES="pyzmq" export GOAL="install" +export NO_WERROR=1 # GCC 8 export BITCOIN_CONFIG="--enable-zmq --with-gui=qt5 --enable-reduce-exports" export CONFIG_SHELL="/bin/dash" export TEST_RUNNER_ENV="LC_ALL=en_US.UTF-8" diff --git a/ci/test/00_setup_env_native_nowallet_libbitcoinkernel.sh b/ci/test/00_setup_env_native_nowallet_libbitcoinkernel.sh index 08bb5d1eab..06bc2401c5 100755 --- a/ci/test/00_setup_env_native_nowallet_libbitcoinkernel.sh +++ b/ci/test/00_setup_env_native_nowallet_libbitcoinkernel.sh @@ -13,4 +13,5 @@ export PACKAGES="-t buster-backports python3-zmq clang-8 llvm-8 libc++abi-8-dev export APPEND_APT_SOURCES_LIST="deb http://deb.debian.org/debian buster-backports main" export DEP_OPTS="NO_WALLET=1 CC=clang-8 CXX='clang++-8 -stdlib=libc++'" export GOAL="install" +export NO_WERROR=1 export BITCOIN_CONFIG="--enable-reduce-exports CC=clang-8 CXX='clang++-8 -stdlib=libc++' --enable-experimental-util-chainstate --with-experimental-kernel-lib --enable-shared" diff --git a/ci/test/00_setup_env_native_qt5.sh b/ci/test/00_setup_env_native_qt5.sh index 3f39185ae8..5cc0addd33 100755 --- a/ci/test/00_setup_env_native_qt5.sh +++ b/ci/test/00_setup_env_native_qt5.sh @@ -15,6 +15,7 @@ export TEST_RUNNER_EXTRA="--previous-releases --coverage --extended --exclude fe export RUN_UNIT_TESTS_SEQUENTIAL="true" export RUN_UNIT_TESTS="false" export GOAL="install" +export NO_WERROR=1 export DOWNLOAD_PREVIOUS_RELEASES="true" export BITCOIN_CONFIG="--enable-zmq --with-libs=no --with-gui=qt5 --enable-reduce-exports \ --enable-debug CFLAGS=\"-g0 -O2 -funsigned-char\" CXXFLAGS=\"-g0 -O2 -funsigned-char\" CC=gcc-8 CXX=g++-8" diff --git a/ci/test/01_base_install.sh b/ci/test/01_base_install.sh new file mode 100755 index 0000000000..c2469d7ca9 --- /dev/null +++ b/ci/test/01_base_install.sh @@ -0,0 +1,33 @@ +#!/usr/bin/env bash +# +# Copyright (c) 2018-2022 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 + +CI_EXEC_ROOT () { bash -c "$*"; } +export -f CI_EXEC_ROOT + +if [ -n "$DPKG_ADD_ARCH" ]; then + CI_EXEC_ROOT dpkg --add-architecture "$DPKG_ADD_ARCH" +fi + +if [[ $CI_IMAGE_NAME_TAG == *centos* ]]; then + ${CI_RETRY_EXE} CI_EXEC_ROOT dnf -y install epel-release + ${CI_RETRY_EXE} CI_EXEC_ROOT dnf -y --allowerasing install "$CI_BASE_PACKAGES" "$PACKAGES" +elif [ "$CI_USE_APT_INSTALL" != "no" ]; then + if [[ "${ADD_UNTRUSTED_BPFCC_PPA}" == "true" ]]; then + # Ubuntu 22.04 LTS and Debian 11 both have an outdated bpfcc-tools packages. + # The iovisor PPA is outdated as well. The next Ubuntu and Debian releases will contain updated + # packages. Meanwhile, use an untrusted PPA to install an up-to-date version of the bpfcc-tools + # package. + # TODO: drop this once we can use newer images in GCE + CI_EXEC_ROOT add-apt-repository ppa:hadret/bpfcc + fi + if [[ -n "${APPEND_APT_SOURCES_LIST}" ]]; then + CI_EXEC_ROOT echo "${APPEND_APT_SOURCES_LIST}" \>\> /etc/apt/sources.list + fi + ${CI_RETRY_EXE} CI_EXEC_ROOT apt-get update + ${CI_RETRY_EXE} CI_EXEC_ROOT apt-get install --no-install-recommends --no-upgrade -y "$PACKAGES" "$CI_BASE_PACKAGES" +fi diff --git a/ci/test/04_install.sh b/ci/test/04_install.sh index 53fe6d961f..05bef79a3d 100755 --- a/ci/test/04_install.sh +++ b/ci/test/04_install.sh @@ -33,7 +33,12 @@ if [ -z "$DANGER_RUN_CI_ON_HOST" ]; then # the name isn't important, so long as we use the same UID LOCAL_USER=nonroot - ${CI_RETRY_EXE} docker pull "$CI_IMAGE_NAME_TAG" + DOCKER_BUILDKIT=1 ${CI_RETRY_EXE} docker build \ + --file "${BASE_ROOT_DIR}/ci/test_imagefile" \ + --build-arg "CI_IMAGE_NAME_TAG=${CI_IMAGE_NAME_TAG}" \ + --build-arg "FILE_ENV=${FILE_ENV}" \ + --tag="${CONTAINER_NAME}" \ + "${BASE_ROOT_DIR}" if [ -n "${RESTART_CI_DOCKER_BEFORE_RUN}" ] ; then echo "Restart docker before run to stop and clear all containers started with --rm" @@ -49,7 +54,7 @@ if [ -z "$DANGER_RUN_CI_ON_HOST" ]; then -w $BASE_ROOT_DIR \ --env-file /tmp/env \ --name $CONTAINER_NAME \ - $CI_IMAGE_NAME_TAG) + $CONTAINER_NAME) export CI_CONTAINER_ID # Create a non-root user inside the container which matches the local user. @@ -63,6 +68,7 @@ if [ -z "$DANGER_RUN_CI_ON_HOST" ]; then export CI_EXEC_CMD_PREFIX="docker exec -u $LOCAL_UID $CI_CONTAINER_ID" else echo "Running on host system without docker wrapper" + "${BASE_ROOT_DIR}/ci/test/01_base_install.sh" fi CI_EXEC () { @@ -76,29 +82,6 @@ export -f CI_EXEC_ROOT CI_EXEC mkdir -p "${BINS_SCRATCH_DIR}" -if [ -n "$DPKG_ADD_ARCH" ]; then - CI_EXEC_ROOT dpkg --add-architecture "$DPKG_ADD_ARCH" -fi - -if [[ $CI_IMAGE_NAME_TAG == *centos* ]]; then - ${CI_RETRY_EXE} CI_EXEC_ROOT dnf -y install epel-release - ${CI_RETRY_EXE} CI_EXEC_ROOT dnf -y --allowerasing install "$CI_BASE_PACKAGES" "$PACKAGES" -elif [ "$CI_USE_APT_INSTALL" != "no" ]; then - if [[ "${ADD_UNTRUSTED_BPFCC_PPA}" == "true" ]]; then - # Ubuntu 22.04 LTS and Debian 11 both have an outdated bpfcc-tools packages. - # The iovisor PPA is outdated as well. The next Ubuntu and Debian releases will contain updated - # packages. Meanwhile, use an untrusted PPA to install an up-to-date version of the bpfcc-tools - # package. - # TODO: drop this once we can use newer images in GCE - CI_EXEC_ROOT add-apt-repository ppa:hadret/bpfcc - fi - if [[ -n "${APPEND_APT_SOURCES_LIST}" ]]; then - CI_EXEC_ROOT echo "${APPEND_APT_SOURCES_LIST}" \>\> /etc/apt/sources.list - fi - ${CI_RETRY_EXE} CI_EXEC_ROOT apt-get update - ${CI_RETRY_EXE} CI_EXEC_ROOT apt-get install --no-install-recommends --no-upgrade -y "$PACKAGES" "$CI_BASE_PACKAGES" -fi - if [ -n "$PIP_PACKAGES" ]; then if [ "$CI_OS_NAME" == "macos" ]; then sudo -H pip3 install --upgrade pip diff --git a/ci/test_imagefile b/ci/test_imagefile new file mode 100644 index 0000000000..4854708d1a --- /dev/null +++ b/ci/test_imagefile @@ -0,0 +1,10 @@ +ARG CI_IMAGE_NAME_TAG +FROM ${CI_IMAGE_NAME_TAG} + +ARG FILE_ENV +ENV FILE_ENV=${FILE_ENV} + +COPY ./ci/retry/retry /usr/bin/retry +COPY ./ci/test/00_setup_env.sh ./${FILE_ENV} ./ci/test/01_base_install.sh /ci_base_install/ci/test/ + +RUN ["bash", "-c", "cd /ci_base_install/ && set -o errexit && source ./ci/test/00_setup_env.sh && ./ci/test/01_base_install.sh"] diff --git a/doc/reduce-memory.md b/doc/reduce-memory.md index 097cc9f001..25205258b8 100644 --- a/doc/reduce-memory.md +++ b/doc/reduce-memory.md @@ -18,7 +18,7 @@ The size of some in-memory caches can be reduced. As caches trade off memory usa - Since `0.14.0`, unused memory allocated to the mempool (default: 300MB) is shared with the UTXO cache, so when trying to reduce memory usage you should limit the mempool, with the `-maxmempool` command line argument. -- To disable most of the mempool functionality there is the `-blocksonly` option. This will reduce the default memory usage to 5MB and make the client opt out of receiving (and thus relaying) transactions, except from whitelisted peers and as part of blocks. +- To disable most of the mempool functionality there is the `-blocksonly` option. This will reduce the default memory usage to 5MB and make the client opt out of receiving (and thus relaying) transactions, except from peers who have the `relay` permission set (e.g. whitelisted peers), and as part of blocks. - Do not use this when using the client to broadcast transactions as any transaction sent will stick out like a sore thumb, affecting privacy. When used with the wallet it should be combined with `-walletbroadcast=0` and `-spendzeroconfchange=0`. Another mechanism for broadcasting outgoing transactions (if any) should be used. diff --git a/src/.clang-tidy b/src/.clang-tidy index 3ad463e99b..b2c1b49588 100644 --- a/src/.clang-tidy +++ b/src/.clang-tidy @@ -9,6 +9,7 @@ performance-for-range-copy, performance-move-const-arg, performance-no-automatic-move, performance-unnecessary-copy-initialization, +readability-const-return-type, readability-redundant-declaration, readability-redundant-string-init, ' diff --git a/src/Makefile.test_util.include b/src/Makefile.test_util.include index a4e8b3f842..8496b3698a 100644 --- a/src/Makefile.test_util.include +++ b/src/Makefile.test_util.include @@ -10,6 +10,7 @@ EXTRA_LIBRARIES += \ TEST_UTIL_H = \ test/util/blockfilter.h \ test/util/chainstate.h \ + test/util/json.h \ test/util/logging.h \ test/util/mining.h \ test/util/net.h \ @@ -28,6 +29,7 @@ libtest_util_a_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) $(BOOST_CPPFLAGS) libtest_util_a_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS) libtest_util_a_SOURCES = \ test/util/blockfilter.cpp \ + test/util/json.cpp \ test/util/logging.cpp \ test/util/mining.cpp \ test/util/net.cpp \ diff --git a/src/addrman.h b/src/addrman.h index 2a074268be..4985fc764c 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -106,7 +106,7 @@ public: * @param[in] in_new Select addresses only from one table (true = new, false = tried, nullopt = both) * @return Number of unique addresses that match specified options. */ - size_t Size(std::optional<Network> net = {}, std::optional<bool> in_new = {}) const; + size_t Size(std::optional<Network> net = std::nullopt, std::optional<bool> in_new = std::nullopt) const; /** * Attempt to add one or more addresses to addrman's new table. diff --git a/src/arith_uint256.h b/src/arith_uint256.h index a6065dd9bd..c710fe9471 100644 --- a/src/arith_uint256.h +++ b/src/arith_uint256.h @@ -58,7 +58,7 @@ public: explicit base_uint(const std::string& str); - const base_uint operator~() const + base_uint operator~() const { base_uint ret; for (int i = 0; i < WIDTH; i++) @@ -66,7 +66,7 @@ public: return ret; } - const base_uint operator-() const + base_uint operator-() const { base_uint ret; for (int i = 0; i < WIDTH; i++) @@ -171,7 +171,7 @@ public: return *this; } - const base_uint operator++(int) + base_uint operator++(int) { // postfix operator const base_uint ret = *this; @@ -188,7 +188,7 @@ public: return *this; } - const base_uint operator--(int) + base_uint operator--(int) { // postfix operator const base_uint ret = *this; @@ -199,16 +199,16 @@ public: int CompareTo(const base_uint& b) const; bool EqualTo(uint64_t b) const; - friend inline const base_uint operator+(const base_uint& a, const base_uint& b) { return base_uint(a) += b; } - friend inline const base_uint operator-(const base_uint& a, const base_uint& b) { return base_uint(a) -= b; } - friend inline const base_uint operator*(const base_uint& a, const base_uint& b) { return base_uint(a) *= b; } - friend inline const base_uint operator/(const base_uint& a, const base_uint& b) { return base_uint(a) /= b; } - friend inline const base_uint operator|(const base_uint& a, const base_uint& b) { return base_uint(a) |= b; } - friend inline const base_uint operator&(const base_uint& a, const base_uint& b) { return base_uint(a) &= b; } - friend inline const base_uint operator^(const base_uint& a, const base_uint& b) { return base_uint(a) ^= b; } - friend inline const base_uint operator>>(const base_uint& a, int shift) { return base_uint(a) >>= shift; } - friend inline const base_uint operator<<(const base_uint& a, int shift) { return base_uint(a) <<= shift; } - friend inline const base_uint operator*(const base_uint& a, uint32_t b) { return base_uint(a) *= b; } + friend inline base_uint operator+(const base_uint& a, const base_uint& b) { return base_uint(a) += b; } + friend inline base_uint operator-(const base_uint& a, const base_uint& b) { return base_uint(a) -= b; } + friend inline base_uint operator*(const base_uint& a, const base_uint& b) { return base_uint(a) *= b; } + friend inline base_uint operator/(const base_uint& a, const base_uint& b) { return base_uint(a) /= b; } + friend inline base_uint operator|(const base_uint& a, const base_uint& b) { return base_uint(a) |= b; } + friend inline base_uint operator&(const base_uint& a, const base_uint& b) { return base_uint(a) &= b; } + friend inline base_uint operator^(const base_uint& a, const base_uint& b) { return base_uint(a) ^= b; } + friend inline base_uint operator>>(const base_uint& a, int shift) { return base_uint(a) >>= shift; } + friend inline base_uint operator<<(const base_uint& a, int shift) { return base_uint(a) <<= shift; } + friend inline base_uint operator*(const base_uint& a, uint32_t b) { return base_uint(a) *= b; } friend inline bool operator==(const base_uint& a, const base_uint& b) { return memcmp(a.pn, b.pn, sizeof(a.pn)) == 0; } friend inline bool operator!=(const base_uint& a, const base_uint& b) { return memcmp(a.pn, b.pn, sizeof(a.pn)) != 0; } friend inline bool operator>(const base_uint& a, const base_uint& b) { return a.CompareTo(b) > 0; } diff --git a/src/bench/gcs_filter.cpp b/src/bench/gcs_filter.cpp index 51fbe15760..0af4ee98fe 100644 --- a/src/bench/gcs_filter.cpp +++ b/src/bench/gcs_filter.cpp @@ -5,7 +5,7 @@ #include <bench/bench.h> #include <blockfilter.h> -static const GCSFilter::ElementSet GenerateGCSTestElements() +static GCSFilter::ElementSet GenerateGCSTestElements() { GCSFilter::ElementSet elements; diff --git a/src/bench/wallet_loading.cpp b/src/bench/wallet_loading.cpp index 2f7dc53b0c..6b09adcc9d 100644 --- a/src/bench/wallet_loading.cpp +++ b/src/bench/wallet_loading.cpp @@ -24,7 +24,7 @@ using wallet::WALLET_FLAG_DESCRIPTORS; using wallet::WalletContext; using wallet::WalletDatabase; -static const std::shared_ptr<CWallet> BenchLoadWallet(std::unique_ptr<WalletDatabase> database, WalletContext& context, DatabaseOptions& options) +static std::shared_ptr<CWallet> BenchLoadWallet(std::unique_ptr<WalletDatabase> database, WalletContext& context, DatabaseOptions& options) { bilingual_str error; std::vector<bilingual_str> warnings; diff --git a/src/blockfilter.cpp b/src/blockfilter.cpp index fc6dde20f9..88c7526b9e 100644 --- a/src/blockfilter.cpp +++ b/src/blockfilter.cpp @@ -247,21 +247,10 @@ bool BlockFilter::BuildParams(GCSFilter::Params& params) const uint256 BlockFilter::GetHash() const { - const std::vector<unsigned char>& data = GetEncodedFilter(); - - uint256 result; - CHash256().Write(data).Finalize(result); - return result; + return Hash(GetEncodedFilter()); } uint256 BlockFilter::ComputeHeader(const uint256& prev_header) const { - const uint256& filter_hash = GetHash(); - - uint256 result; - CHash256() - .Write(filter_hash) - .Write(prev_header) - .Finalize(result); - return result; + return Hash(GetHash(), prev_header); } diff --git a/src/external_signer.cpp b/src/external_signer.cpp index 8a3e17a292..5524b943f4 100644 --- a/src/external_signer.cpp +++ b/src/external_signer.cpp @@ -16,7 +16,7 @@ ExternalSigner::ExternalSigner(const std::string& command, const std::string chain, const std::string& fingerprint, const std::string name): m_command(command), m_chain(chain), m_fingerprint(fingerprint), m_name(name) {} -const std::string ExternalSigner::NetworkArg() const +std::string ExternalSigner::NetworkArg() const { return " --chain " + m_chain; } diff --git a/src/external_signer.h b/src/external_signer.h index e40fd7f010..90f07478e3 100644 --- a/src/external_signer.h +++ b/src/external_signer.h @@ -24,7 +24,7 @@ private: //! Bitcoin mainnet, testnet, etc std::string m_chain; - const std::string NetworkArg() const; + std::string NetworkArg() const; public: //! @param[in] command the command which handles interaction with the external signer diff --git a/src/index/blockfilterindex.cpp b/src/index/blockfilterindex.cpp index 07b4cdc06b..59bf6d34cf 100644 --- a/src/index/blockfilterindex.cpp +++ b/src/index/blockfilterindex.cpp @@ -157,9 +157,7 @@ bool BlockFilterIndex::ReadFilterFromDisk(const FlatFilePos& pos, const uint256& std::vector<uint8_t> encoded_filter; try { filein >> block_hash >> encoded_filter; - uint256 result; - CHash256().Write(encoded_filter).Finalize(result); - if (result != hash) return error("Checksum mismatch in filter decode."); + if (Hash(encoded_filter) != hash) return error("Checksum mismatch in filter decode."); filter = BlockFilter(GetFilterType(), block_hash, std::move(encoded_filter), /*skip_decode_check=*/true); } catch (const std::exception& e) { diff --git a/src/kernel/mempool_options.h b/src/kernel/mempool_options.h index a5a0bae86d..beb5fca5e9 100644 --- a/src/kernel/mempool_options.h +++ b/src/kernel/mempool_options.h @@ -18,7 +18,7 @@ class CBlockPolicyEstimator; /** Default for -maxmempool, maximum megabytes of mempool memory usage */ static constexpr unsigned int DEFAULT_MAX_MEMPOOL_SIZE_MB{300}; -//** Default for -maxmempool when blocksonly is set */ +/** Default for -maxmempool when blocksonly is set */ static constexpr unsigned int DEFAULT_BLOCKSONLY_MAX_MEMPOOL_SIZE_MB{5}; /** Default for -mempoolexpiry, expiration time for mempool transactions in hours */ static constexpr unsigned int DEFAULT_MEMPOOL_EXPIRY_HOURS{336}; diff --git a/src/key.cpp b/src/key.cpp index 33913ed461..3a3f0b2bc2 100644 --- a/src/key.cpp +++ b/src/key.cpp @@ -245,8 +245,7 @@ bool CKey::VerifyPubKey(const CPubKey& pubkey) const { unsigned char rnd[8]; std::string str = "Bitcoin key verification\n"; GetRandBytes(rnd); - uint256 hash; - CHash256().Write(MakeUCharSpan(str)).Write(rnd).Finalize(hash); + uint256 hash{Hash(str, rnd)}; std::vector<unsigned char> vchSig; Sign(hash, vchSig); return pubkey.Verify(hash, vchSig); diff --git a/src/logging/timer.h b/src/logging/timer.h index ea0821dede..993ba99c25 100644 --- a/src/logging/timer.h +++ b/src/logging/timer.h @@ -12,6 +12,7 @@ #include <util/types.h> #include <chrono> +#include <optional> #include <string> @@ -28,14 +29,14 @@ public: std::string prefix, std::string end_msg, BCLog::LogFlags log_category = BCLog::LogFlags::ALL, - bool msg_on_completion = true) : - m_prefix(std::move(prefix)), - m_title(std::move(end_msg)), - m_log_category(log_category), - m_message_on_completion(msg_on_completion) + bool msg_on_completion = true) + : m_prefix(std::move(prefix)), + m_title(std::move(end_msg)), + m_log_category(log_category), + m_message_on_completion(msg_on_completion) { this->Log(strprintf("%s started", m_title)); - m_start_t = GetTime<std::chrono::microseconds>(); + m_start_t = std::chrono::steady_clock::now(); } ~Timer() @@ -60,24 +61,25 @@ public: std::string LogMsg(const std::string& msg) { - const auto end_time = GetTime<std::chrono::microseconds>() - m_start_t; - if (m_start_t.count() <= 0) { + const auto end_time{std::chrono::steady_clock::now()}; + if (!m_start_t) { return strprintf("%s: %s", m_prefix, msg); } + const auto duration{end_time - *m_start_t}; if constexpr (std::is_same<TimeType, std::chrono::microseconds>::value) { - return strprintf("%s: %s (%iμs)", m_prefix, msg, end_time.count()); + return strprintf("%s: %s (%iμs)", m_prefix, msg, Ticks<std::chrono::microseconds>(duration)); } else if constexpr (std::is_same<TimeType, std::chrono::milliseconds>::value) { - return strprintf("%s: %s (%.2fms)", m_prefix, msg, end_time.count() * 0.001); + return strprintf("%s: %s (%.2fms)", m_prefix, msg, Ticks<MillisecondsDouble>(duration)); } else if constexpr (std::is_same<TimeType, std::chrono::seconds>::value) { - return strprintf("%s: %s (%.2fs)", m_prefix, msg, end_time.count() * 0.000001); + return strprintf("%s: %s (%.2fs)", m_prefix, msg, Ticks<SecondsDouble>(duration)); } else { static_assert(ALWAYS_FALSE<TimeType>, "Error: unexpected time type"); } } private: - std::chrono::microseconds m_start_t{}; + std::optional<std::chrono::steady_clock::time_point> m_start_t{}; //! Log prefix; usually the name of the function this was created in. const std::string m_prefix; diff --git a/src/qt/optionsmodel.cpp b/src/qt/optionsmodel.cpp index cd0a1a19ee..00b7952ca4 100644 --- a/src/qt/optionsmodel.cpp +++ b/src/qt/optionsmodel.cpp @@ -31,7 +31,7 @@ const char *DEFAULT_GUI_PROXY_HOST = "127.0.0.1"; -static const QString GetDefaultProxyAddress(); +static QString GetDefaultProxyAddress(); /** Map GUI option ID to node setting name. */ static const char* SettingName(OptionsModel::OptionID option) @@ -308,7 +308,7 @@ static std::string ProxyString(bool is_set, QString ip, QString port) return is_set ? QString(ip + ":" + port).toStdString() : ""; } -static const QString GetDefaultProxyAddress() +static QString GetDefaultProxyAddress() { return QString("%1:%2").arg(DEFAULT_GUI_PROXY_HOST).arg(DEFAULT_GUI_PROXY_PORT); } diff --git a/src/qt/overviewpage.cpp b/src/qt/overviewpage.cpp index c9caec39cf..3cd5f4061c 100644 --- a/src/qt/overviewpage.cpp +++ b/src/qt/overviewpage.cpp @@ -262,7 +262,6 @@ void OverviewPage::setWalletModel(WalletModel *model) // Set up transaction list filter.reset(new TransactionFilterProxy()); filter->setSourceModel(model->getTransactionTableModel()); - filter->setLimit(NUM_ITEMS); filter->setDynamicSortFilter(true); filter->setSortRole(Qt::EditRole); filter->setShowInactive(false); @@ -271,6 +270,10 @@ void OverviewPage::setWalletModel(WalletModel *model) ui->listTransactions->setModel(filter.get()); ui->listTransactions->setModelColumn(TransactionTableModel::ToAddress); + connect(filter.get(), &TransactionFilterProxy::rowsInserted, this, &OverviewPage::LimitTransactionRows); + connect(filter.get(), &TransactionFilterProxy::rowsRemoved, this, &OverviewPage::LimitTransactionRows); + connect(filter.get(), &TransactionFilterProxy::rowsMoved, this, &OverviewPage::LimitTransactionRows); + LimitTransactionRows(); // Keep up to date with wallet setBalance(model->getCachedBalance()); connect(model, &WalletModel::balanceChanged, this, &OverviewPage::setBalance); @@ -299,6 +302,16 @@ void OverviewPage::changeEvent(QEvent* e) QWidget::changeEvent(e); } +// Only show most recent NUM_ITEMS rows +void OverviewPage::LimitTransactionRows() +{ + if (filter && ui->listTransactions && ui->listTransactions->model() && filter.get() == ui->listTransactions->model()) { + for (int i = 0; i < filter->rowCount(); ++i) { + ui->listTransactions->setRowHidden(i, i >= NUM_ITEMS); + } + } +} + void OverviewPage::updateDisplayUnit() { if (walletModel && walletModel->getOptionsModel()) { diff --git a/src/qt/overviewpage.h b/src/qt/overviewpage.h index 2ca38b78dd..5c487ee116 100644 --- a/src/qt/overviewpage.h +++ b/src/qt/overviewpage.h @@ -60,6 +60,7 @@ private: std::unique_ptr<TransactionFilterProxy> filter; private Q_SLOTS: + void LimitTransactionRows(); void updateDisplayUnit(); void handleTransactionClicked(const QModelIndex &index); void updateAlerts(const QString &warnings); diff --git a/src/qt/rpcconsole.cpp b/src/qt/rpcconsole.cpp index 843cd46d13..b46a3c039b 100644 --- a/src/qt/rpcconsole.cpp +++ b/src/qt/rpcconsole.cpp @@ -780,8 +780,8 @@ void RPCConsole::addWallet(WalletModel * const walletModel) { // use name for text and wallet model for internal data object (to allow to move to a wallet id later) ui->WalletSelector->addItem(walletModel->getDisplayName(), QVariant::fromValue(walletModel)); - if (ui->WalletSelector->count() == 2 && !isVisible()) { - // First wallet added, set to default so long as the window isn't presently visible (and potentially in use) + if (ui->WalletSelector->count() == 2) { + // First wallet added, set to default to match wallet RPC behavior ui->WalletSelector->setCurrentIndex(1); } if (ui->WalletSelector->count() > 2) { diff --git a/src/qt/transactionfilterproxy.cpp b/src/qt/transactionfilterproxy.cpp index 3cc0cc839d..173fd326a3 100644 --- a/src/qt/transactionfilterproxy.cpp +++ b/src/qt/transactionfilterproxy.cpp @@ -88,25 +88,8 @@ void TransactionFilterProxy::setWatchOnlyFilter(WatchOnlyFilter filter) invalidateFilter(); } -void TransactionFilterProxy::setLimit(int limit) -{ - this->limitRows = limit; -} - void TransactionFilterProxy::setShowInactive(bool _showInactive) { this->showInactive = _showInactive; invalidateFilter(); } - -int TransactionFilterProxy::rowCount(const QModelIndex &parent) const -{ - if(limitRows != -1) - { - return std::min(QSortFilterProxyModel::rowCount(parent), limitRows); - } - else - { - return QSortFilterProxyModel::rowCount(parent); - } -} diff --git a/src/qt/transactionfilterproxy.h b/src/qt/transactionfilterproxy.h index 8e5f72d764..73c4f21426 100644 --- a/src/qt/transactionfilterproxy.h +++ b/src/qt/transactionfilterproxy.h @@ -42,14 +42,9 @@ public: void setMinAmount(const CAmount& minimum); void setWatchOnlyFilter(WatchOnlyFilter filter); - /** Set maximum number of rows returned, -1 if unlimited. */ - void setLimit(int limit); - /** Set whether to show conflicted transactions. */ void setShowInactive(bool showInactive); - int rowCount(const QModelIndex &parent = QModelIndex()) const override; - protected: bool filterAcceptsRow(int source_row, const QModelIndex & source_parent) const override; @@ -60,7 +55,6 @@ private: quint32 typeFilter; WatchOnlyFilter watchOnlyFilter{WatchOnlyFilter_All}; CAmount minAmount{0}; - int limitRows{-1}; bool showInactive{true}; }; diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp index 72b9f2230e..8991bda340 100644 --- a/src/script/descriptor.cpp +++ b/src/script/descriptor.cpp @@ -1833,17 +1833,17 @@ DescriptorCache DescriptorCache::MergeAndDiff(const DescriptorCache& other) return diff; } -const ExtPubKeyMap DescriptorCache::GetCachedParentExtPubKeys() const +ExtPubKeyMap DescriptorCache::GetCachedParentExtPubKeys() const { return m_parent_xpubs; } -const std::unordered_map<uint32_t, ExtPubKeyMap> DescriptorCache::GetCachedDerivedExtPubKeys() const +std::unordered_map<uint32_t, ExtPubKeyMap> DescriptorCache::GetCachedDerivedExtPubKeys() const { return m_derived_xpubs; } -const ExtPubKeyMap DescriptorCache::GetCachedLastHardenedExtPubKeys() const +ExtPubKeyMap DescriptorCache::GetCachedLastHardenedExtPubKeys() const { return m_last_hardened_xpubs; } diff --git a/src/script/descriptor.h b/src/script/descriptor.h index 16ee2f6d97..cb3b366acf 100644 --- a/src/script/descriptor.h +++ b/src/script/descriptor.h @@ -66,11 +66,11 @@ public: bool GetCachedLastHardenedExtPubKey(uint32_t key_exp_pos, CExtPubKey& xpub) const; /** Retrieve all cached parent xpubs */ - const ExtPubKeyMap GetCachedParentExtPubKeys() const; + ExtPubKeyMap GetCachedParentExtPubKeys() const; /** Retrieve all cached derived xpubs */ - const std::unordered_map<uint32_t, ExtPubKeyMap> GetCachedDerivedExtPubKeys() const; + std::unordered_map<uint32_t, ExtPubKeyMap> GetCachedDerivedExtPubKeys() const; /** Retrieve all cached last hardened xpubs */ - const ExtPubKeyMap GetCachedLastHardenedExtPubKeys() const; + ExtPubKeyMap GetCachedLastHardenedExtPubKeys() const; /** Combine another DescriptorCache into this one. * Returns a cache containing the items from the other cache unknown to current cache diff --git a/src/streams.h b/src/streams.h index ed9af308c9..8788343809 100644 --- a/src/streams.h +++ b/src/streams.h @@ -358,14 +358,6 @@ public: nType{nTypeIn}, nVersion{nVersionIn} {} - template <typename... Args> - CDataStream(int nTypeIn, int nVersionIn, Args&&... args) - : nType{nTypeIn}, - nVersion{nVersionIn} - { - ::SerializeMany(*this, std::forward<Args>(args)...); - } - int GetType() const { return nType; } void SetVersion(int n) { nVersion = n; } int GetVersion() const { return nVersion; } diff --git a/src/test/addrman_tests.cpp b/src/test/addrman_tests.cpp index cf7bb776cc..586cec4081 100644 --- a/src/test/addrman_tests.cpp +++ b/src/test/addrman_tests.cpp @@ -947,8 +947,6 @@ BOOST_AUTO_TEST_CASE(load_addrman_corrupted) } catch (const std::exception&) { exceptionThrown = true; } - // Even though de-serialization failed addrman is not left in a clean state. - BOOST_CHECK(addrman1.Size() == 1); BOOST_CHECK(exceptionThrown); // Test that ReadFromStream fails if peers.dat is corrupt diff --git a/src/test/base58_tests.cpp b/src/test/base58_tests.cpp index 0101bcc372..601caf8102 100644 --- a/src/test/base58_tests.cpp +++ b/src/test/base58_tests.cpp @@ -5,6 +5,7 @@ #include <test/data/base58_encode_decode.json.h> #include <base58.h> +#include <test/util/json.h> #include <test/util/setup_common.h> #include <util/strencodings.h> #include <util/vector.h> @@ -16,8 +17,6 @@ using namespace std::literals; -UniValue read_json(const std::string& jsondata); - BOOST_FIXTURE_TEST_SUITE(base58_tests, BasicTestingSetup) // Goal: test low-level base58 encoding functionality diff --git a/src/test/fuzz/miniscript.cpp b/src/test/fuzz/miniscript.cpp index 1d6a8d89e4..d5667e0cf3 100644 --- a/src/test/fuzz/miniscript.cpp +++ b/src/test/fuzz/miniscript.cpp @@ -104,7 +104,7 @@ struct ScriptParserContext { return key.data; } - const std::vector<unsigned char> ToPKHBytes(const Key& key) const + std::vector<unsigned char> ToPKHBytes(const Key& key) const { if (key.is_hash) return key.data; const auto h = Hash160(key.data); diff --git a/src/test/fuzz/util.h b/src/test/fuzz/util.h index af1d65cd38..c14f633029 100644 --- a/src/test/fuzz/util.h +++ b/src/test/fuzz/util.h @@ -47,7 +47,7 @@ size_t CallOneOf(FuzzedDataProvider& fuzzed_data_provider, Callables... callable template <typename Collection> auto& PickValue(FuzzedDataProvider& fuzzed_data_provider, Collection& col) { - const auto sz = col.size(); + auto sz{col.size()}; assert(sz >= 1); auto it = col.begin(); std::advance(it, fuzzed_data_provider.ConsumeIntegralInRange<decltype(sz)>(0, sz - 1)); diff --git a/src/test/key_io_tests.cpp b/src/test/key_io_tests.cpp index fb0a07934d..a400afee71 100644 --- a/src/test/key_io_tests.cpp +++ b/src/test/key_io_tests.cpp @@ -8,6 +8,7 @@ #include <key.h> #include <key_io.h> #include <script/script.h> +#include <test/util/json.h> #include <test/util/setup_common.h> #include <util/strencodings.h> @@ -15,8 +16,6 @@ #include <univalue.h> -UniValue read_json(const std::string& jsondata); - BOOST_FIXTURE_TEST_SUITE(key_io_tests, BasicTestingSetup) // Goal: check that parsed keys match test payload diff --git a/src/test/key_tests.cpp b/src/test/key_tests.cpp index d14fda7351..edf28cfbfc 100644 --- a/src/test/key_tests.cpp +++ b/src/test/key_tests.cpp @@ -205,8 +205,7 @@ BOOST_AUTO_TEST_CASE(key_key_negation) unsigned char rnd[8]; std::string str = "Bitcoin key verification\n"; GetRandBytes(rnd); - uint256 hash; - CHash256().Write(MakeUCharSpan(str)).Write(rnd).Finalize(hash); + uint256 hash{Hash(str, rnd)}; // import the static test key CKey key = DecodeSecret(strSecret1C); diff --git a/src/test/logging_tests.cpp b/src/test/logging_tests.cpp index 022e33f99d..beb9398c74 100644 --- a/src/test/logging_tests.cpp +++ b/src/test/logging_tests.cpp @@ -75,20 +75,9 @@ struct LogSetup : public BasicTestingSetup { BOOST_AUTO_TEST_CASE(logging_timer) { - SetMockTime(1); auto micro_timer = BCLog::Timer<std::chrono::microseconds>("tests", "end_msg"); - SetMockTime(2); - BOOST_CHECK_EQUAL(micro_timer.LogMsg("test micros"), "tests: test micros (1000000μs)"); - - SetMockTime(1); - auto ms_timer = BCLog::Timer<std::chrono::milliseconds>("tests", "end_msg"); - SetMockTime(2); - BOOST_CHECK_EQUAL(ms_timer.LogMsg("test ms"), "tests: test ms (1000.00ms)"); - - SetMockTime(1); - auto sec_timer = BCLog::Timer<std::chrono::seconds>("tests", "end_msg"); - SetMockTime(2); - BOOST_CHECK_EQUAL(sec_timer.LogMsg("test secs"), "tests: test secs (1.00s)"); + const std::string_view result_prefix{"tests: msg ("}; + BOOST_CHECK_EQUAL(micro_timer.LogMsg("msg").substr(0, result_prefix.size()), result_prefix); } BOOST_FIXTURE_TEST_CASE(logging_LogPrintf_, LogSetup) diff --git a/src/test/merkle_tests.cpp b/src/test/merkle_tests.cpp index bba103d1b0..74e01fc2a5 100644 --- a/src/test/merkle_tests.cpp +++ b/src/test/merkle_tests.cpp @@ -60,7 +60,7 @@ static void MerkleComputation(const std::vector<uint256>& leaves, uint256* proot } } mutated |= (inner[level] == h); - CHash256().Write(inner[level]).Write(h).Finalize(h); + h = Hash(inner[level], h); } // Store the resulting hash at inner position level. inner[level] = h; @@ -86,7 +86,7 @@ static void MerkleComputation(const std::vector<uint256>& leaves, uint256* proot if (pbranch && matchh) { pbranch->push_back(h); } - CHash256().Write(h).Write(h).Finalize(h); + h = Hash(h, h); // Increment count to the value it would have if two entries at this // level had existed. count += ((uint32_t{1}) << level); @@ -101,7 +101,7 @@ static void MerkleComputation(const std::vector<uint256>& leaves, uint256* proot matchh = true; } } - CHash256().Write(inner[level]).Write(h).Finalize(h); + h = Hash(inner[level], h); level++; } } diff --git a/src/test/script_tests.cpp b/src/test/script_tests.cpp index d4cede8f7c..22f6cfd164 100644 --- a/src/test/script_tests.cpp +++ b/src/test/script_tests.cpp @@ -15,6 +15,7 @@ #include <script/sign.h> #include <script/signingprovider.h> #include <streams.h> +#include <test/util/json.h> #include <test/util/setup_common.h> #include <test/util/transaction_utils.h> #include <util/strencodings.h> @@ -41,18 +42,6 @@ static const unsigned int gFlags = SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_STRICTENC; unsigned int ParseScriptFlags(std::string strFlags); std::string FormatScriptFlags(unsigned int flags); -UniValue read_json(const std::string& jsondata) -{ - UniValue v; - - if (!v.read(jsondata) || !v.isArray()) - { - BOOST_ERROR("Parse error."); - return UniValue(UniValue::VARR); - } - return v.get_array(); -} - struct ScriptErrorDesc { ScriptError_t err; diff --git a/src/test/serialize_tests.cpp b/src/test/serialize_tests.cpp index f583109e16..09f77d2b61 100644 --- a/src/test/serialize_tests.cpp +++ b/src/test/serialize_tests.cpp @@ -237,7 +237,8 @@ BOOST_AUTO_TEST_CASE(class_methods) BOOST_CHECK(methodtest2 == methodtest3); BOOST_CHECK(methodtest3 == methodtest4); - CDataStream ss2(SER_DISK, PROTOCOL_VERSION, intval, boolval, stringval, charstrval, txval); + CDataStream ss2{SER_DISK, PROTOCOL_VERSION}; + ss2 << intval << boolval << stringval << charstrval << txval; ss2 >> methodtest3; BOOST_CHECK(methodtest3 == methodtest4); } diff --git a/src/test/sighash_tests.cpp b/src/test/sighash_tests.cpp index 1ce694b8c6..368f9e6047 100644 --- a/src/test/sighash_tests.cpp +++ b/src/test/sighash_tests.cpp @@ -10,6 +10,7 @@ #include <serialize.h> #include <streams.h> #include <test/data/sighash.json.h> +#include <test/util/json.h> #include <test/util/setup_common.h> #include <util/strencodings.h> #include <util/system.h> @@ -21,8 +22,6 @@ #include <univalue.h> -UniValue read_json(const std::string& jsondata); - // Old script.cpp SignatureHash function uint256 static SignatureHashOld(CScript scriptCode, const CTransaction& txTo, unsigned int nIn, int nHashType) { diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index 69b03e07bf..d00de9df9b 100644 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -21,6 +21,7 @@ #include <script/signingprovider.h> #include <script/standard.h> #include <streams.h> +#include <test/util/json.h> #include <test/util/script.h> #include <test/util/transaction_utils.h> #include <util/strencodings.h> @@ -37,9 +38,6 @@ typedef std::vector<unsigned char> valtype; -// In script_tests.cpp -UniValue read_json(const std::string& jsondata); - static CFeeRate g_dust{DUST_RELAY_TX_FEE}; static bool g_bare_multi{DEFAULT_PERMIT_BAREMULTISIG}; diff --git a/src/test/util/json.cpp b/src/test/util/json.cpp new file mode 100644 index 0000000000..ad3c346c84 --- /dev/null +++ b/src/test/util/json.cpp @@ -0,0 +1,17 @@ +// Copyright (c) 2023 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include <test/util/json.h> + +#include <string> +#include <util/check.h> + +#include <univalue.h> + +UniValue read_json(const std::string& jsondata) +{ + UniValue v; + Assert(v.read(jsondata) && v.isArray()); + return v.get_array(); +} diff --git a/src/test/util/json.h b/src/test/util/json.h new file mode 100644 index 0000000000..5b1026762e --- /dev/null +++ b/src/test/util/json.h @@ -0,0 +1,14 @@ +// Copyright (c) 2023 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_TEST_UTIL_JSON_H +#define BITCOIN_TEST_UTIL_JSON_H + +#include <string> + +#include <univalue.h> + +UniValue read_json(const std::string& jsondata); + +#endif // BITCOIN_TEST_UTIL_JSON_H diff --git a/src/test/versionbits_tests.cpp b/src/test/versionbits_tests.cpp index 4a42cec4af..91383ee4a5 100644 --- a/src/test/versionbits_tests.cpp +++ b/src/test/versionbits_tests.cpp @@ -13,7 +13,7 @@ /* Define a virtual block time, one block per 10 minutes after Nov 14 2014, 0:55:36am */ static int32_t TestTime(int nHeight) { return 1415926536 + 600 * nHeight; } -static const std::string StateName(ThresholdState state) +static std::string StateName(ThresholdState state) { switch (state) { case ThresholdState::DEFINED: return "DEFINED"; diff --git a/src/txmempool.cpp b/src/txmempool.cpp index aa04f8a4d0..378123ce0f 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -1128,7 +1128,7 @@ void CTxMemPool::SetLoadTried(bool load_tried) } -const std::string RemovalReasonToString(const MemPoolRemovalReason& r) noexcept +std::string RemovalReasonToString(const MemPoolRemovalReason& r) noexcept { switch (r) { case MemPoolRemovalReason::EXPIRY: return "expiry"; diff --git a/src/txmempool.h b/src/txmempool.h index 51b8af3286..2c3cb7e9db 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -237,7 +237,7 @@ enum class MemPoolRemovalReason { REPLACED, //!< Removed for replacement }; -const std::string RemovalReasonToString(const MemPoolRemovalReason& r) noexcept; +std::string RemovalReasonToString(const MemPoolRemovalReason& r) noexcept; /** * CTxMemPool stores valid-according-to-the-current-best-chain transactions diff --git a/src/util/fees.cpp b/src/util/fees.cpp index cbefe18dbb..8ada02ce54 100644 --- a/src/util/fees.cpp +++ b/src/util/fees.cpp @@ -49,7 +49,7 @@ std::string FeeModes(const std::string& delimiter) return Join(FeeModeMap(), delimiter, [&](const std::pair<std::string, FeeEstimateMode>& i) { return i.first; }); } -const std::string InvalidEstimateModeErrorMessage() +std::string InvalidEstimateModeErrorMessage() { return "Invalid estimate_mode parameter, must be one of: \"" + FeeModes("\", \"") + "\""; } diff --git a/src/util/fees.h b/src/util/fees.h index 9ef2389d3e..10ba1e4f85 100644 --- a/src/util/fees.h +++ b/src/util/fees.h @@ -13,6 +13,6 @@ enum class FeeReason; bool FeeModeFromString(const std::string& mode_string, FeeEstimateMode& fee_estimate_mode); std::string StringForFeeReason(FeeReason reason); std::string FeeModes(const std::string& delimiter); -const std::string InvalidEstimateModeErrorMessage(); +std::string InvalidEstimateModeErrorMessage(); #endif // BITCOIN_UTIL_FEES_H diff --git a/src/util/system.cpp b/src/util/system.cpp index 309595ff5b..0cea283ece 100644 --- a/src/util/system.cpp +++ b/src/util/system.cpp @@ -242,7 +242,7 @@ static std::optional<util::SettingsValue> InterpretValue(const KeyInfo& key, con ArgsManager::ArgsManager() = default; ArgsManager::~ArgsManager() = default; -const std::set<std::string> ArgsManager::GetUnsuitableSectionOnlyArgs() const +std::set<std::string> ArgsManager::GetUnsuitableSectionOnlyArgs() const { std::set<std::string> unsuitables; @@ -262,7 +262,7 @@ const std::set<std::string> ArgsManager::GetUnsuitableSectionOnlyArgs() const return unsuitables; } -const std::list<SectionInfo> ArgsManager::GetUnrecognizedSections() const +std::list<SectionInfo> ArgsManager::GetUnrecognizedSections() const { // Section names to be recognized in the config file. static const std::set<std::string> available_sections{ diff --git a/src/util/system.h b/src/util/system.h index 671491f2ff..c053adf8c3 100644 --- a/src/util/system.h +++ b/src/util/system.h @@ -250,12 +250,12 @@ protected: * on the command line or in a network-specific section in the * config file. */ - const std::set<std::string> GetUnsuitableSectionOnlyArgs() const; + std::set<std::string> GetUnsuitableSectionOnlyArgs() const; /** * Log warnings for unrecognized section names in the config file. */ - const std::list<SectionInfo> GetUnrecognizedSections() const; + std::list<SectionInfo> GetUnrecognizedSections() const; struct Command { /** The command (if one has been registered with AddCommand), or empty */ diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp index 900cb0474a..d344c8bfbd 100644 --- a/src/validationinterface.cpp +++ b/src/validationinterface.cpp @@ -17,7 +17,7 @@ #include <unordered_map> #include <utility> -const std::string RemovalReasonToString(const MemPoolRemovalReason& r) noexcept; +std::string RemovalReasonToString(const MemPoolRemovalReason& r) noexcept; /** * MainSignalsImpl manages a list of shared_ptr<CValidationInterface> callbacks. diff --git a/src/wallet/rpc/transactions.cpp b/src/wallet/rpc/transactions.cpp index 4a076a7040..e590aa1f08 100644 --- a/src/wallet/rpc/transactions.cpp +++ b/src/wallet/rpc/transactions.cpp @@ -397,7 +397,7 @@ static void ListTransactions(const CWallet& wallet, const CWalletTx& wtx, int nM } -static const std::vector<RPCResult> TransactionDescriptionString() +static std::vector<RPCResult> TransactionDescriptionString() { return{{RPCResult::Type::NUM, "confirmations", "The number of confirmations for the transaction. Negative confirmations means the\n" "transaction conflicted that many blocks ago."}, diff --git a/src/wallet/scriptpubkeyman.cpp b/src/wallet/scriptpubkeyman.cpp index d8fb926c9a..59cf87355b 100644 --- a/src/wallet/scriptpubkeyman.cpp +++ b/src/wallet/scriptpubkeyman.cpp @@ -1664,7 +1664,7 @@ std::set<CKeyID> LegacyScriptPubKeyMan::GetKeys() const return set_address; } -const std::unordered_set<CScript, SaltedSipHasher> LegacyScriptPubKeyMan::GetScriptPubKeys() const +std::unordered_set<CScript, SaltedSipHasher> LegacyScriptPubKeyMan::GetScriptPubKeys() const { LOCK(cs_KeyStore); std::unordered_set<CScript, SaltedSipHasher> spks; @@ -2650,17 +2650,17 @@ void DescriptorScriptPubKeyMan::WriteDescriptor() } } -const WalletDescriptor DescriptorScriptPubKeyMan::GetWalletDescriptor() const +WalletDescriptor DescriptorScriptPubKeyMan::GetWalletDescriptor() const { return m_wallet_descriptor; } -const std::unordered_set<CScript, SaltedSipHasher> DescriptorScriptPubKeyMan::GetScriptPubKeys() const +std::unordered_set<CScript, SaltedSipHasher> DescriptorScriptPubKeyMan::GetScriptPubKeys() const { return GetScriptPubKeys(0); } -const std::unordered_set<CScript, SaltedSipHasher> DescriptorScriptPubKeyMan::GetScriptPubKeys(int32_t minimum_index) const +std::unordered_set<CScript, SaltedSipHasher> DescriptorScriptPubKeyMan::GetScriptPubKeys(int32_t minimum_index) const { LOCK(cs_desc_man); std::unordered_set<CScript, SaltedSipHasher> script_pub_keys; diff --git a/src/wallet/scriptpubkeyman.h b/src/wallet/scriptpubkeyman.h index d74388b3e8..4399ac2087 100644 --- a/src/wallet/scriptpubkeyman.h +++ b/src/wallet/scriptpubkeyman.h @@ -36,7 +36,7 @@ class WalletStorage { public: virtual ~WalletStorage() = default; - virtual const std::string GetDisplayName() const = 0; + virtual std::string GetDisplayName() const = 0; virtual WalletDatabase& GetDatabase() const = 0; virtual bool IsWalletFlagSet(uint64_t) const = 0; virtual void UnsetBlankWalletFlag(WalletBatch&) = 0; @@ -243,7 +243,7 @@ public: virtual uint256 GetID() const { return uint256(); } /** Returns a set of all the scriptPubKeys that this ScriptPubKeyMan watches */ - virtual const std::unordered_set<CScript, SaltedSipHasher> GetScriptPubKeys() const { return {}; }; + virtual std::unordered_set<CScript, SaltedSipHasher> GetScriptPubKeys() const { return {}; }; /** Prepends the wallet name in logging output to ease debugging in multi-wallet use cases */ template<typename... Params> @@ -512,7 +512,7 @@ public: const std::map<CKeyID, int64_t>& GetAllReserveKeys() const { return m_pool_key_to_index; } std::set<CKeyID> GetKeys() const override; - const std::unordered_set<CScript, SaltedSipHasher> GetScriptPubKeys() const override; + std::unordered_set<CScript, SaltedSipHasher> GetScriptPubKeys() const override; /** Get the DescriptorScriptPubKeyMans (with private keys) that have the same scriptPubKeys as this LegacyScriptPubKeyMan. * Does not modify this ScriptPubKeyMan. */ @@ -641,9 +641,9 @@ public: void AddDescriptorKey(const CKey& key, const CPubKey &pubkey); void WriteDescriptor(); - const WalletDescriptor GetWalletDescriptor() const EXCLUSIVE_LOCKS_REQUIRED(cs_desc_man); - const std::unordered_set<CScript, SaltedSipHasher> GetScriptPubKeys() const override; - const std::unordered_set<CScript, SaltedSipHasher> GetScriptPubKeys(int32_t minimum_index) const; + WalletDescriptor GetWalletDescriptor() const EXCLUSIVE_LOCKS_REQUIRED(cs_desc_man); + std::unordered_set<CScript, SaltedSipHasher> GetScriptPubKeys() const override; + std::unordered_set<CScript, SaltedSipHasher> GetScriptPubKeys(int32_t minimum_index) const; int32_t GetEndRange() const; bool GetDescriptorString(std::string& out, const bool priv) const; diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 53321e98ee..f056c54734 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -43,7 +43,7 @@ static_assert(WALLET_INCREMENTAL_RELAY_FEE >= DEFAULT_INCREMENTAL_RELAY_FEE, "wa BOOST_FIXTURE_TEST_SUITE(wallet_tests, WalletTestingSetup) -static const std::shared_ptr<CWallet> TestLoadWallet(WalletContext& context) +static std::shared_ptr<CWallet> TestLoadWallet(WalletContext& context) { DatabaseOptions options; options.create_flags = WALLET_FLAG_DESCRIPTORS; diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index aadad258a9..5a92dbe428 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3871,10 +3871,7 @@ std::optional<MigrationData> CWallet::GetDescriptorsForLegacy(bilingual_str& err AssertLockHeld(cs_wallet); LegacyScriptPubKeyMan* legacy_spkm = GetLegacyScriptPubKeyMan(); - if (!legacy_spkm) { - error = _("Error: This wallet is already a descriptor wallet"); - return std::nullopt; - } + assert(legacy_spkm); std::optional<MigrationData> res = legacy_spkm->MigrateToDescriptor(); if (res == std::nullopt) { @@ -4170,6 +4167,11 @@ bool DoMigration(CWallet& wallet, WalletContext& context, bilingual_str& error, util::Result<MigrationResult> MigrateLegacyToDescriptor(std::shared_ptr<CWallet>&& wallet, WalletContext& context) { + // Before anything else, check if there is something to migrate. + if (!wallet->GetLegacyScriptPubKeyMan()) { + return util::Error{_("Error: This wallet is already a descriptor wallet")}; + } + MigrationResult res; bilingual_str error; std::vector<bilingual_str> warnings; diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h index 108db11d32..f104a15f98 100644 --- a/src/wallet/wallet.h +++ b/src/wallet/wallet.h @@ -821,7 +821,8 @@ public: bool IsLegacy() const; /** Returns a bracketed wallet name for displaying in logs, will return [default wallet] if the wallet has no name */ - const std::string GetDisplayName() const override { + std::string GetDisplayName() const override + { std::string wallet_name = GetName().length() == 0 ? "default wallet" : GetName(); return strprintf("[%s]", wallet_name); }; diff --git a/src/wallet/wallettool.cpp b/src/wallet/wallettool.cpp index f93b666bd5..96537b9873 100644 --- a/src/wallet/wallettool.cpp +++ b/src/wallet/wallettool.cpp @@ -47,7 +47,7 @@ static void WalletCreate(CWallet* wallet_instance, uint64_t wallet_creation_flag wallet_instance->TopUpKeyPool(); } -static const std::shared_ptr<CWallet> MakeWallet(const std::string& name, const fs::path& path, const ArgsManager& args, DatabaseOptions options) +static std::shared_ptr<CWallet> MakeWallet(const std::string& name, const fs::path& path, const ArgsManager& args, DatabaseOptions options) { DatabaseStatus status; bilingual_str error; diff --git a/test/functional/wallet_migration.py b/test/functional/wallet_migration.py index 688ac98617..72c5fe7b84 100755 --- a/test/functional/wallet_migration.py +++ b/test/functional/wallet_migration.py @@ -163,6 +163,10 @@ class WalletMigrationTest(BitcoinTestFramework): assert_equal(basic2.getbalance(), basic2_balance) self.assert_list_txs_equal(basic2.listtransactions(), basic2_txs) + # Now test migration on a descriptor wallet + self.log.info("Test \"nothing to migrate\" when the user tries to migrate a wallet with no legacy data") + assert_raises_rpc_error(-4, "Error: This wallet is already a descriptor wallet", basic2.migratewallet) + def test_multisig(self): default = self.nodes[0].get_wallet_rpc(self.default_wallet_name) |