From d9de40e0f0f6a191c0c7b4bd365660469032f4d6 Mon Sep 17 00:00:00 2001 From: div72 <60045611+div72@users.noreply.github.com> Date: Sat, 30 Jul 2022 01:20:46 +0300 Subject: [PATCH 01/23] build: add option for sanitizers --- configure.ac | 36 ++++++++++++++++++++++++++++++++++++ src/Makefile.am | 4 ++-- 2 files changed, 38 insertions(+), 2 deletions(-) diff --git a/configure.ac b/configure.ac index 8fa47af3b0..9afa9ba822 100755 --- a/configure.ac +++ b/configure.ac @@ -209,6 +209,12 @@ AC_ARG_ENABLE([debug], [enable_debug=$enableval], [enable_debug=no]) +dnl Enable different -fsanitize options +AC_ARG_WITH([sanitizers], + [AS_HELP_STRING([--with-sanitizers], + [comma separated list of extra sanitizers to build with (default is none enabled)])], + [use_sanitizers=$withval]) + # Turn warnings into errors AC_ARG_ENABLE([werror], [AS_HELP_STRING([--enable-werror], @@ -230,6 +236,33 @@ if test "$enable_debug" = "yes"; then fi fi +if test "$use_sanitizers" != ""; then + dnl First check if the compiler accepts flags. If an incompatible pair like + dnl -fsanitize=address,thread is used here, this check will fail. This will also + dnl fail if a bad argument is passed, e.g. -fsanitize=undfeined + AX_CHECK_COMPILE_FLAG( + [-fsanitize=$use_sanitizers], + [SANITIZER_CXXFLAGS="-fsanitize=$use_sanitizers"], + [AC_MSG_ERROR([compiler did not accept requested flags])]) + + dnl Some compilers (e.g. GCC) require additional libraries like libasan, + dnl libtsan, libubsan, etc. Make sure linking still works with the sanitize + dnl flag. This is a separate check so we can give a better error message when + dnl the sanitize flags are supported by the compiler but the actual sanitizer + dnl libs are missing. + AX_CHECK_LINK_FLAG( + [-fsanitize=$use_sanitizers], + [SANITIZER_LDFLAGS="-fsanitize=$use_sanitizers"], + [AC_MSG_ERROR([linker did not accept requested flags, you are missing required libraries])], + [], + [AC_LANG_PROGRAM([[ + #include + #include + extern "C" int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { return 0; } + __attribute__((weak)) // allow for libFuzzer linking + ]],[[]])]) +fi + ERROR_CXXFLAGS= if test "$enable_werror" = "yes"; then if test "$CXXFLAG_WERROR" = ""; then @@ -1292,6 +1325,8 @@ AC_SUBST(HARDENED_CPPFLAGS) AC_SUBST(HARDENED_LDFLAGS) AC_SUBST(PIC_FLAGS) AC_SUBST(PIE_FLAGS) +AC_SUBST(SANITIZER_CXXFLAGS) +AC_SUBST(SANITIZER_LDFLAGS) AC_SUBST(SSE42_CXXFLAGS) AC_SUBST(SSE41_CXXFLAGS) AC_SUBST(AVX2_CXXFLAGS) @@ -1398,6 +1433,7 @@ echo " with zmq = $use_zmq" echo " with test = $use_tests" echo " with bench = $use_bench" echo " with upnp = $use_upnp" +echo " sanitizers = $use_sanitizers" echo " debug enabled = $enable_debug" echo " werror = $enable_werror" echo diff --git a/src/Makefile.am b/src/Makefile.am index c521bd11fe..b92061afbc 100755 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -8,8 +8,8 @@ print-%: FORCE DIST_SUBDIRS = univalue secp256k1 -AM_LDFLAGS = ${libcurl_LIBS} $(PTHREAD_CFLAGS) $(LIBTOOL_LDFLAGS) $(HARDENED_LDFLAGS) -AM_CXXFLAGS = $(HARDENED_CXXFLAGS) $(ERROR_CXXFLAGS) +AM_LDFLAGS = ${libcurl_LIBS} $(PTHREAD_CFLAGS) $(LIBTOOL_LDFLAGS) $(HARDENED_LDFLAGS) $(SANITIZER_LDFLAGS) +AM_CXXFLAGS = $(HARDENED_CXXFLAGS) $(ERROR_CXXFLAGS) $(SANITIZER_CXXFLAGS) AM_CPPFLAGS = ${libcurl_CFLAGS} $(HARDENED_CPPFLAGS) -DSTATICLIB -DCURL_STATICLIB -DMINIUPNP_STATICLIB -DZIP_STATIC -DNN_STATIC_LIB EXTRA_LIBRARIES = From 00d6ae75fdaffcd716923e125448391ab7de6202 Mon Sep 17 00:00:00 2001 From: div72 <60045611+div72@users.noreply.github.com> Date: Sat, 30 Jul 2022 02:44:07 +0300 Subject: [PATCH 02/23] misc: fix UB in CPID hashing --- src/gridcoin/cpid.h | 18 +++++++++++++----- src/test/gridcoin/cpid_tests.cpp | 2 +- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/src/gridcoin/cpid.h b/src/gridcoin/cpid.h index b644abbd8c..c8b3df5417 100644 --- a/src/gridcoin/cpid.h +++ b/src/gridcoin/cpid.h @@ -454,9 +454,9 @@ namespace std { //! This enables the use of GRC::Cpid as a key in a std::unordered_map object. //! //! CONSENSUS: Don't use the hash produced by this routine (or by any std::hash -//! specialization) in protocol-specific implementations. It ignores endianness -//! and outputs a value with a chance of collision probably too great for usage -//! besides the intended local look-up functionality. +//! specialization) in protocol-specific implementations. It outputs a value +//! with a chance of collision probably too great for usage besides the intended +//! local look-up functionality. //! template<> struct hash @@ -473,8 +473,16 @@ struct hash // Just convert the CPID into a value that we can store in a size_t // object. CPIDs are already unique identifiers. // - return *reinterpret_cast(cpid.Raw().data()) - + *reinterpret_cast(cpid.Raw().data() + 8); + const auto& data = cpid.Raw(); + size_t ret = ((size_t)(data[0] & 255) | (size_t)(data[1] & 255) << 8 | + (size_t)(data[2] & 255) << 16 | (size_t)(data[3] & 255) << 24); + + if (sizeof(size_t) == 8) { + ret |= ((size_t)(data[4] & 255) << 32 | (size_t)(data[5] & 255) << 40 | + (size_t)(data[6] & 255) << 48 | (size_t)(data[7] & 255) << 56); + } + + return ret; } }; } diff --git a/src/test/gridcoin/cpid_tests.cpp b/src/test/gridcoin/cpid_tests.cpp index 6becf0cc51..c76af4f76a 100644 --- a/src/test/gridcoin/cpid_tests.cpp +++ b/src/test/gridcoin/cpid_tests.cpp @@ -249,7 +249,7 @@ BOOST_AUTO_TEST_CASE(it_is_hashable_to_key_a_lookup_map) std::hash hasher; // CPID halves, little endian - const size_t expected = static_cast(0x0706050403020100ull + 0x1514131211100908ull); + const size_t expected = static_cast(0x0706050403020100ull); BOOST_CHECK_EQUAL(hasher(cpid), expected); } From e33bc3589cb777f1f486e2da0849ac4661d53502 Mon Sep 17 00:00:00 2001 From: div72 <60045611+div72@users.noreply.github.com> Date: Sun, 16 Oct 2022 02:04:22 +0300 Subject: [PATCH 03/23] ci: add new builds with sanitizers --- .github/workflows/ci.yml | 4 ++++ ci/test/00_setup_env_native_asan.sh | 14 ++++++++++++++ ci/test/00_setup_env_native_tsan.sh | 14 ++++++++++++++ 3 files changed, 32 insertions(+) create mode 100644 ci/test/00_setup_env_native_asan.sh create mode 100644 ci/test/00_setup_env_native_tsan.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 9ddd104cac..6b3488812b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -23,6 +23,10 @@ jobs: script-id: native - name: x86_64 Linux [GOAL install] [GUI] [bionic] [no depends] script-id: native_old + - name: x86_64 Linux [ASan] [LSan] [UBSan] [integer] [jammy] [no depends] + script-id: native_asan + - name: x86_64 Linux [TSan] [GUI] [jammy] + script-id: native_tsan - name: macOS 10.14 [GOAL deploy] [GUI] [no tests] [focal] script-id: mac env: diff --git a/ci/test/00_setup_env_native_asan.sh b/ci/test/00_setup_env_native_asan.sh new file mode 100644 index 0000000000..4426072659 --- /dev/null +++ b/ci/test/00_setup_env_native_asan.sh @@ -0,0 +1,14 @@ +#!/usr/bin/env bash +# +# Copyright (c) 2019-2022 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or https://opensource.org/licenses/mit-license.php. + +export LC_ALL=C.UTF-8 + +export CONTAINER_NAME=ci_native_asan +export PACKAGES="clang llvm libqt5gui5 libqt5core5a qtbase5-dev libqt5dbus5 qttools5-dev qttools5-dev-tools libssl-dev libevent-dev bsdmainutils libboost-system-dev libboost-filesystem-dev libboost-iostreams-dev libboost-test-dev libboost-thread-dev libdb5.3++-dev libminiupnpc-dev libqrencode-dev libzip-dev zlib1g zlib1g-dev libcurl4 libcurl4-openssl-dev" +export DOCKER_NAME_TAG=ubuntu:22.04 +export NO_DEPENDS=1 +export GOAL="install" +export GRIDCOIN_CONFIG="--with-incompatible-bdb --with-gui=qt5 CPPFLAGS='-DARENA_DEBUG -DDEBUG_LOCKORDER' --with-sanitizers=address,integer,undefined CC=clang CXX=clang++" diff --git a/ci/test/00_setup_env_native_tsan.sh b/ci/test/00_setup_env_native_tsan.sh new file mode 100644 index 0000000000..206af61e08 --- /dev/null +++ b/ci/test/00_setup_env_native_tsan.sh @@ -0,0 +1,14 @@ +#!/usr/bin/env bash +# +# Copyright (c) 2019-2022 The Bitcoin Core developers +# Distributed under the MIT software license, see the accompanying +# file COPYING or https://opensource.org/licenses/mit-license.php. + +export LC_ALL=C.UTF-8 + +export CONTAINER_NAME=ci_native_tsan +export DOCKER_NAME_TAG=ubuntu:22.04 +export PACKAGES="clang-13 llvm-13 libc++abi-13-dev libc++-13-dev" +export DEP_OPTS="CC=clang-13 CXX='clang++-13 -stdlib=libc++'" +export GOAL="install" +export GRIDCOIN_CONFIG="CPPFLAGS='-DARENA_DEBUG -DDEBUG_LOCKORDER -DDEBUG_LOCKCONTENTION' CXXFLAGS='-g' --with-sanitizers=thread CC=clang-13 CXX='clang++-13 -stdlib=libc++'" From 57251db5017ab0f3bc9d2ac4279a4d4cf495c227 Mon Sep 17 00:00:00 2001 From: div72 <60045611+div72@users.noreply.github.com> Date: Sun, 16 Oct 2022 11:47:10 +0300 Subject: [PATCH 04/23] support: add proper includes for -DARENA_DEBUG --- src/support/lockedpool.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/support/lockedpool.cpp b/src/support/lockedpool.cpp index a2b03b3eab..2162ccb80d 100644 --- a/src/support/lockedpool.cpp +++ b/src/support/lockedpool.cpp @@ -24,6 +24,10 @@ #include #include +#ifdef ARENA_DEBUG +#include +#include +#endif LockedPoolManager* LockedPoolManager::_instance = nullptr; std::once_flag LockedPoolManager::init_flag; From 19731eee716a1ec6f953f0980ed5f39496b7e3be Mon Sep 17 00:00:00 2001 From: div72 <60045611+div72@users.noreply.github.com> Date: Sun, 16 Oct 2022 13:29:36 +0300 Subject: [PATCH 05/23] support: fix compilation with -DARENA_DEBUG --- src/support/lockedpool.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/support/lockedpool.cpp b/src/support/lockedpool.cpp index 2162ccb80d..979f0705a4 100644 --- a/src/support/lockedpool.cpp +++ b/src/support/lockedpool.cpp @@ -154,7 +154,7 @@ void Arena::walk() const printchunk(chunk.first, chunk.second, true); std::cout << std::endl; for (const auto& chunk: chunks_free) - printchunk(chunk.first, chunk.second, false); + printchunk(chunk.first, chunk.second->first, false); std::cout << std::endl; } #endif From c927e0025826cff7dd09389b905f601797dd054c Mon Sep 17 00:00:00 2001 From: div72 <60045611+div72@users.noreply.github.com> Date: Thu, 23 Mar 2023 21:39:04 +0300 Subject: [PATCH 06/23] ci: disable tsan job Rationale: The thread sanitizer CI job uses clang which has issues with compiling Qt on Linux. Disable for now until the issue can be fixed. --- .github/workflows/ci.yml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6b3488812b..ad48da6b7e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -25,8 +25,9 @@ jobs: script-id: native_old - name: x86_64 Linux [ASan] [LSan] [UBSan] [integer] [jammy] [no depends] script-id: native_asan - - name: x86_64 Linux [TSan] [GUI] [jammy] - script-id: native_tsan + # FIXME: depends is unable to compile Qt with clang. + # - name: x86_64 Linux [TSan] [GUI] [jammy] + # script-id: native_tsan - name: macOS 10.14 [GOAL deploy] [GUI] [no tests] [focal] script-id: mac env: From 29a0c093e418442b80ac1313e3e8ec9c5a2be127 Mon Sep 17 00:00:00 2001 From: div72 Date: Mon, 31 Jul 2023 18:44:26 +0300 Subject: [PATCH 07/23] ci: use suppression files for sanitizers --- ci/test/04_install.sh | 5 +++ test/sanitizer_suppressions/lsan | 2 + test/sanitizer_suppressions/tsan | 45 +++++++++++++++++++++++ test/sanitizer_suppressions/ubsan | 61 +++++++++++++++++++++++++++++++ 4 files changed, 113 insertions(+) create mode 100644 test/sanitizer_suppressions/lsan create mode 100644 test/sanitizer_suppressions/tsan create mode 100644 test/sanitizer_suppressions/ubsan diff --git a/ci/test/04_install.sh b/ci/test/04_install.sh index 288f60a7d5..d1ba771f6a 100755 --- a/ci/test/04_install.sh +++ b/ci/test/04_install.sh @@ -17,6 +17,11 @@ fi mkdir -p "${CCACHE_DIR}" mkdir -p "${PREVIOUS_RELEASES_DIR}" +export ASAN_OPTIONS="detect_stack_use_after_return=1:check_initialization_order=1:strict_init_order=1" +export LSAN_OPTIONS="suppressions=${BASE_ROOT_DIR}/test/sanitizer_suppressions/lsan" +export TSAN_OPTIONS="suppressions=${BASE_ROOT_DIR}/test/sanitizer_suppressions/tsan:halt_on_error=1:log_path=${BASE_SCRATCH_DIR}/sanitizer-output/tsan" +export UBSAN_OPTIONS="suppressions=${BASE_ROOT_DIR}/test/sanitizer_suppressions/ubsan:print_stacktrace=1:halt_on_error=1:report_error_type=1" + env | grep -E '^(GRIDCOIN_CONFIG|BASE_|QEMU_|CCACHE_|LC_ALL|BOOST_TEST_RANDOM|DEBIAN_FRONTEND|CONFIG_SHELL|(ASAN|LSAN|TSAN|UBSAN)_OPTIONS|PREVIOUS_RELEASES_DIR)' | tee /tmp/env if [[ $HOST = *-mingw32 ]]; then DOCKER_ADMIN="--cap-add SYS_ADMIN" diff --git a/test/sanitizer_suppressions/lsan b/test/sanitizer_suppressions/lsan new file mode 100644 index 0000000000..7ccb22515f --- /dev/null +++ b/test/sanitizer_suppressions/lsan @@ -0,0 +1,2 @@ +# Suppress warnings triggered in dependencies +leak:libQt5Widgets diff --git a/test/sanitizer_suppressions/tsan b/test/sanitizer_suppressions/tsan new file mode 100644 index 0000000000..82c6885bc4 --- /dev/null +++ b/test/sanitizer_suppressions/tsan @@ -0,0 +1,45 @@ +# ThreadSanitizer suppressions +# ============================ +# +# https://github.com/google/sanitizers/wiki/ThreadSanitizerSuppressions + +# race (TODO fix) +race:LoadWallet +race:WalletBatch::WriteHDChain +race:BerkeleyBatch +race:BerkeleyDatabase +race:DatabaseBatch +race:zmq::* +race:bitcoin-qt + +# deadlock (TODO fix) +# To reproduce, see: +# https://github.com/bitcoin/bitcoin/issues/19303#issuecomment-1514926359 +deadlock:Chainstate::ConnectTip + +# Intentional deadlock in tests +deadlock:sync_tests::potential_deadlock_detected + +# Wildcard for all gui tests, should be replaced with non-wildcard suppressions +race:src/qt/test/* +deadlock:src/qt/test/* + +# External libraries +# https://github.com/bitcoin/bitcoin/pull/27658#issuecomment-1547639621 +deadlock:libdb +race:libzmq + +# Intermittent issues +# ------------------- +# +# Suppressions that follow might only happen intermittently, thus they are not +# reproducible. Make sure to include a link to a full trace. + +# https://github.com/bitcoin/bitcoin/issues/20618 +race:CZMQAbstractPublishNotifier::SendZmqMessage + +# https://github.com/bitcoin/bitcoin/pull/27498#issuecomment-1517410478 +race:epoll_ctl + +# https://github.com/bitcoin/bitcoin/issues/23366 +race:std::__1::ios_base::* diff --git a/test/sanitizer_suppressions/ubsan b/test/sanitizer_suppressions/ubsan new file mode 100644 index 0000000000..68f2bee322 --- /dev/null +++ b/test/sanitizer_suppressions/ubsan @@ -0,0 +1,61 @@ +# Suppressions should use `sanitize-type:ClassName::MethodName`. + +# -fsanitize=undefined suppressions +# ================================= + +# -fsanitize=integer suppressions +# =============================== +# Dependencies +# ------------ +# Suppressions in dependencies that are developed outside this repository. +unsigned-integer-overflow:*/include/c++/ +unsigned-integer-overflow:FuzzedDataProvider::ConsumeIntegralInRange +unsigned-integer-overflow:leveldb/ +unsigned-integer-overflow:minisketch/ +unsigned-integer-overflow:test/fuzz/crypto_diff_fuzz_chacha20.cpp +implicit-integer-sign-change:*/include/boost/ +implicit-integer-sign-change:*/include/c++/ +implicit-integer-sign-change:*/new_allocator.h +implicit-integer-sign-change:crc32c/ +implicit-integer-sign-change:minisketch/ +implicit-signed-integer-truncation:*/include/c++/ +implicit-signed-integer-truncation:leveldb/ +implicit-unsigned-integer-truncation:*/include/c++/ +implicit-unsigned-integer-truncation:leveldb/ +implicit-unsigned-integer-truncation:test/fuzz/crypto_diff_fuzz_chacha20.cpp +shift-base:*/include/c++/ +shift-base:leveldb/ +shift-base:minisketch/ +shift-base:test/fuzz/crypto_diff_fuzz_chacha20.cpp +# Unsigned integer overflow occurs when the result of an unsigned integer +# computation cannot be represented in its type. Unlike signed integer overflow, +# this is not undefined behavior, but it is often unintentional. The list below +# contains files in which we expect unsigned integer overflows to occur. The +# list is used to suppress -fsanitize=integer warnings when running our CI UBSan +# job. +unsigned-integer-overflow:arith_uint256.h +unsigned-integer-overflow:common/bloom.cpp +unsigned-integer-overflow:coins.cpp +unsigned-integer-overflow:compressor.cpp +unsigned-integer-overflow:crypto/ +unsigned-integer-overflow:hash.cpp +unsigned-integer-overflow:policy/fees.cpp +unsigned-integer-overflow:prevector.h +unsigned-integer-overflow:script/interpreter.cpp +unsigned-integer-overflow:xoroshiro128plusplus.h +implicit-integer-sign-change:compat/stdin.cpp +implicit-integer-sign-change:compressor.h +implicit-integer-sign-change:crypto/ +implicit-integer-sign-change:policy/fees.cpp +implicit-integer-sign-change:prevector.h +implicit-integer-sign-change:script/bitcoinconsensus.cpp +implicit-integer-sign-change:script/interpreter.cpp +implicit-integer-sign-change:serialize.h +implicit-signed-integer-truncation:crypto/ +implicit-unsigned-integer-truncation:crypto/ +shift-base:arith_uint256.cpp +shift-base:crypto/ +shift-base:hash.cpp +shift-base:streams.h +shift-base:util/bip32.cpp +shift-base:xoroshiro128plusplus.h From c39f895391a66083a12693be70f955d2a0ba74a1 Mon Sep 17 00:00:00 2001 From: div72 Date: Thu, 7 Dec 2023 23:07:15 +0300 Subject: [PATCH 08/23] Fix memory leak in wallet tests --- src/test/wallet_tests.cpp | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/test/wallet_tests.cpp b/src/test/wallet_tests.cpp index c5e390e531..a741d2a127 100755 --- a/src/test/wallet_tests.cpp +++ b/src/test/wallet_tests.cpp @@ -13,6 +13,8 @@ using namespace std; +std::vector> wtxn; + typedef set > CoinSet; BOOST_AUTO_TEST_SUITE(wallet_tests) @@ -23,13 +25,12 @@ static vector vCoins; static void add_coin(int64_t nValue, int nAge = 6*24, bool fIsFromMe = false, int nInput=0) { static int i; - CTransaction* tx = new CTransaction; - tx->nLockTime = i++; // so all transactions get different hashes - tx->nTime = 0; - tx->vout.resize(nInput+1); - tx->vout[nInput].nValue = nValue; - CWalletTx* wtx = new CWalletTx(&wallet, *tx); - delete tx; + CTransaction tx; + tx.nLockTime = i++; // so all transactions get different hashes + tx.nTime = 0; + tx.vout.resize(nInput+1); + tx.vout[nInput].nValue = nValue; + std::unique_ptr wtx(new CWalletTx(&wallet, tx)); if (fIsFromMe) { // IsFromMe() returns (GetDebit() > 0), and GetDebit() is 0 if vin.empty(), @@ -38,15 +39,15 @@ static void add_coin(int64_t nValue, int nAge = 6*24, bool fIsFromMe = false, in wtx->fDebitCached = true; wtx->nDebitCached = 1; } - COutput output(wtx, nInput, nAge); + COutput output(wtx.get(), nInput, nAge); vCoins.push_back(output); + wtxn.emplace_back(std::move(wtx)); } static void empty_wallet() { - for(COutput& output : vCoins) - delete output.tx; vCoins.clear(); + wtxn.clear(); } static bool equal_sets(CoinSet a, CoinSet b) @@ -298,6 +299,7 @@ BOOST_AUTO_TEST_CASE(coin_selection_tests) BOOST_CHECK_NE(fails, RANDOM_REPEATS); } } + empty_wallet(); } BOOST_AUTO_TEST_SUITE_END() From 66967f0e80c28a8254106b06389a98cdc93cb7e9 Mon Sep 17 00:00:00 2001 From: div72 Date: Thu, 7 Dec 2023 23:10:48 +0300 Subject: [PATCH 09/23] miner: don't leak CBlockIndex used for testing --- src/miner.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/miner.cpp b/src/miner.cpp index bc0ef2deb7..387e891a53 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -310,8 +310,8 @@ bool CreateRestOfTheBlock(CBlock &block, CBlockIndex* pindexPrev, int nHeight = pindexPrev->nHeight + 1; // This is specifically for BlockValidateContracts, and only the nHeight is filled in. - CBlockIndex* pindex_contract_validate = new CBlockIndex(); - pindex_contract_validate->nHeight = nHeight; + CBlockIndex pindex_contract_validate; + pindex_contract_validate.nHeight = nHeight; // Create coinbase tx CTransaction &CoinBase = block.vtx[0]; @@ -385,7 +385,7 @@ bool CreateRestOfTheBlock(CBlock &block, CBlockIndex* pindexPrev, // pindex_contract_validate only has the block height filled out. // int DoS = 0; // Unused here. - if (!tx.GetContracts().empty() && !GRC::BlockValidateContracts(pindex_contract_validate, tx, DoS)) { + if (!tx.GetContracts().empty() && !GRC::BlockValidateContracts(&pindex_contract_validate, tx, DoS)) { LogPrint(BCLog::LogFlags::MINER, "%s: contract failed contextual validation. Skipped tx %s", __func__, From cfcfe24078d8e9b13e1d99873df18faaa67c7970 Mon Sep 17 00:00:00 2001 From: div72 Date: Thu, 7 Dec 2023 23:18:01 +0300 Subject: [PATCH 10/23] test: free wallet at exit of mrc_tests --- src/test/gridcoin/mrc_tests.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/src/test/gridcoin/mrc_tests.cpp b/src/test/gridcoin/mrc_tests.cpp index 4a427fd3be..779d957b3f 100644 --- a/src/test/gridcoin/mrc_tests.cpp +++ b/src/test/gridcoin/mrc_tests.cpp @@ -103,6 +103,7 @@ struct Setup { mapBlockIndex.erase(pindexGenesisBlock->GetBlockHash()); delete pindexGenesisBlock->phashBlock; + delete wallet; } }; } // Anonymous namespace From 6fd9480d4daafaa8c0537edbe638d5b3f385763e Mon Sep 17 00:00:00 2001 From: div72 Date: Fri, 8 Dec 2023 00:44:55 +0300 Subject: [PATCH 11/23] test: cleanup leveldb env Rationale: Some classes in LevelDB do not get freed unless the environment is deleted. --- src/test/test_gridcoin.cpp | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/test/test_gridcoin.cpp b/src/test/test_gridcoin.cpp index cacb3e12ec..7a9f325e3d 100644 --- a/src/test/test_gridcoin.cpp +++ b/src/test/test_gridcoin.cpp @@ -14,6 +14,8 @@ #include "random.h" #include "wallet/wallet.h" +leveldb::Env* txdb_env; + extern CWallet* pwalletMain; extern leveldb::DB *txdb; extern CClientUIInterface uiInterface; @@ -48,7 +50,7 @@ struct TestingSetup { // TODO: Refactor CTxDB to something like bitcoin's current CDBWrapper and remove this workaround. leveldb::Options db_options; - db_options.env = leveldb::NewMemEnv(leveldb::Env::Default()); // Use a memory environment to avoid polluting the production leveldb. + db_options.env = txdb_env = leveldb::NewMemEnv(leveldb::Env::Default()); // Use a memory environment to avoid polluting the production leveldb. db_options.create_if_missing = true; db_options.error_if_exists = true; assert(leveldb::DB::Open(db_options, "", &txdb).ok()); @@ -71,7 +73,9 @@ struct TestingSetup { bitdb.Flush(true); g_banman.reset(); delete txdb; + delete txdb_env; txdb = nullptr; + txdb_env = nullptr; g_mock_deterministic_tests = false; ECC_Stop(); } From c4262686c958c175d97e967bf5a555a899108593 Mon Sep 17 00:00:00 2001 From: div72 Date: Sun, 24 Dec 2023 22:59:20 +0300 Subject: [PATCH 12/23] mrc: zero initialize primitives Rationale: Avoids an UB in it_rejects_invalid_claims MRC test. --- src/gridcoin/mrc.h | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/gridcoin/mrc.h b/src/gridcoin/mrc.h index bcc966d036..578c631302 100644 --- a/src/gridcoin/mrc.h +++ b/src/gridcoin/mrc.h @@ -125,14 +125,14 @@ class MRC : public IContractPayload //! incoming reward claims and can index those calculated values without //! this field. It can be considered informational. //! - CAmount m_research_subsidy; + CAmount m_research_subsidy = 0; //! //! \brief The value of the fees charged to the MRC claimant. These will be //! subtracted from the research subsidy and distributed to the staker and //! the foundation according to protocol rules encapsulated in ComputeMRCFee(). //! - CAmount m_fee; + CAmount m_fee = 0; //! //! \brief The researcher magnitude value from the superblock at the time @@ -145,14 +145,14 @@ class MRC : public IContractPayload //! //! Previous protocol versions used the magnitude in reward calculations. //! - uint16_t m_magnitude; + uint16_t m_magnitude = 0; //! //! \brief The magnitude ratio of the network at the time of the claim. //! //! Informational. //! - double m_magnitude_unit; + double m_magnitude_unit = 0.0; //! //! \brief The hash of the last block (head of the chain) for the MRC From 29d6adbb4ba8cb979294aa416b850cfaa7794b22 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Sat, 16 Nov 2019 10:40:16 -0800 Subject: [PATCH 13/23] Fix segfault in allocator_tests/arena_tests The test uses reinterpret_cast on unallocated memory. Using this memory in printchunk as char* causes a segfault, so have printchunk take void* instead. --- src/support/lockedpool.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/support/lockedpool.cpp b/src/support/lockedpool.cpp index 979f0705a4..ef05f222b6 100644 --- a/src/support/lockedpool.cpp +++ b/src/support/lockedpool.cpp @@ -142,7 +142,7 @@ Arena::Stats Arena::stats() const } #ifdef ARENA_DEBUG -static void printchunk(char* base, size_t sz, bool used) { +static void printchunk(void* base, size_t sz, bool used) { std::cout << "0x" << std::hex << std::setw(16) << std::setfill('0') << base << " 0x" << std::hex << std::setw(16) << std::setfill('0') << sz << From 8a6579357329938290eec495ac537bfd69e83a53 Mon Sep 17 00:00:00 2001 From: div72 Date: Tue, 26 Dec 2023 16:53:45 +0300 Subject: [PATCH 14/23] test: fix suppression for script.cpp --- test/sanitizer_suppressions/ubsan | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/sanitizer_suppressions/ubsan b/test/sanitizer_suppressions/ubsan index 68f2bee322..600d2dc8a2 100644 --- a/test/sanitizer_suppressions/ubsan +++ b/test/sanitizer_suppressions/ubsan @@ -41,7 +41,7 @@ unsigned-integer-overflow:crypto/ unsigned-integer-overflow:hash.cpp unsigned-integer-overflow:policy/fees.cpp unsigned-integer-overflow:prevector.h -unsigned-integer-overflow:script/interpreter.cpp +unsigned-integer-overflow:script.cpp unsigned-integer-overflow:xoroshiro128plusplus.h implicit-integer-sign-change:compat/stdin.cpp implicit-integer-sign-change:compressor.h @@ -49,7 +49,7 @@ implicit-integer-sign-change:crypto/ implicit-integer-sign-change:policy/fees.cpp implicit-integer-sign-change:prevector.h implicit-integer-sign-change:script/bitcoinconsensus.cpp -implicit-integer-sign-change:script/interpreter.cpp +implicit-integer-sign-change:script.cpp implicit-integer-sign-change:serialize.h implicit-signed-integer-truncation:crypto/ implicit-unsigned-integer-truncation:crypto/ From 14ec4af47f136a8f8cd72a291a7025879c12a9fe Mon Sep 17 00:00:00 2001 From: div72 Date: Mon, 4 Mar 2024 15:50:21 +0300 Subject: [PATCH 15/23] test: fix memory leaks in beacon tests --- src/test/gridcoin/beacon_tests.cpp | 52 +++++++++++++++--------------- 1 file changed, 26 insertions(+), 26 deletions(-) diff --git a/src/test/gridcoin/beacon_tests.cpp b/src/test/gridcoin/beacon_tests.cpp index e216919f3a..770cddb0d7 100644 --- a/src/test/gridcoin/beacon_tests.cpp +++ b/src/test/gridcoin/beacon_tests.cpp @@ -1268,10 +1268,10 @@ BOOST_AUTO_TEST_CASE(beacon_registry_GetBeaconChainletRoot_test) tx1.nTime = int64_t {1}; uint256 tx1_hash = tx1.GetHash(); - CBlockIndex* pindex1 = new CBlockIndex; - pindex1->nVersion = 13; - pindex1->nHeight = 1; - pindex1->nTime = tx1.nTime; + CBlockIndex pindex1 {}; + pindex1.nVersion = 13; + pindex1.nHeight = 1; + pindex1.nTime = tx1.nTime; GRC::Beacon beacon1 {TestKey::Public(), tx1.nTime, tx1_hash}; beacon1.m_cpid = TestKey::Cpid(); @@ -1280,7 +1280,7 @@ BOOST_AUTO_TEST_CASE(beacon_registry_GetBeaconChainletRoot_test) beacon_payload1.m_signature = TestKey::Signature(beacon_payload1); GRC::Contract contract1 = GRC::MakeContract(3, GRC::ContractAction::ADD, beacon_payload1); - GRC::ContractContext ctx1 {contract1, tx1, pindex1}; + GRC::ContractContext ctx1 {contract1, tx1, &pindex1}; BOOST_CHECK(ctx1.m_contract.CopyPayloadAs().m_cpid == TestKey::Cpid()); BOOST_CHECK(ctx1.m_contract.CopyPayloadAs().m_beacon.m_status == GRC::BeaconStatusForStorage::PENDING); @@ -1305,18 +1305,18 @@ BOOST_AUTO_TEST_CASE(beacon_registry_GetBeaconChainletRoot_test) } // Activation - CBlockIndex* pindex2 = new CBlockIndex; - pindex2->nVersion = 13; - pindex2->nHeight = 2; - pindex2->nTime = int64_t {2}; - uint256* block2_phash = new uint256 {rng.rand256()}; - pindex2->phashBlock = block2_phash; + CBlockIndex pindex2 {}; + pindex2.nVersion = 13; + pindex2.nHeight = 2; + pindex2.nTime = int64_t {2}; + uint256 block2_phash = rng.rand256(); + pindex2.phashBlock = &block2_phash; std::vector beacon_ids {TestKey::Public().GetID()}; - registry.ActivatePending(beacon_ids, pindex2->nTime, *pindex2->phashBlock, pindex2->nHeight); + registry.ActivatePending(beacon_ids, pindex2.nTime, *pindex2.phashBlock, pindex2.nHeight); - uint256 activated_beacon_hash = Hash(*block2_phash, pending_beacons[0]->m_hash); + uint256 activated_beacon_hash = Hash(block2_phash, pending_beacons[0]->m_hash); BOOST_CHECK(registry.GetBeaconDB().size() == 2); @@ -1475,10 +1475,10 @@ BOOST_AUTO_TEST_CASE(beacon_registry_GetBeaconChainletRoot_test_2) tx1.nTime = int64_t {1}; uint256 tx1_hash = tx1.GetHash(); - CBlockIndex* pindex1 = new CBlockIndex; - pindex1->nVersion = 13; - pindex1->nHeight = 1; - pindex1->nTime = tx1.nTime; + CBlockIndex pindex1 {}; + pindex1.nVersion = 13; + pindex1.nHeight = 1; + pindex1.nTime = tx1.nTime; GRC::Beacon beacon1 {TestKey::Public(), tx1.nTime, tx1_hash}; beacon1.m_cpid = TestKey::Cpid(); @@ -1487,7 +1487,7 @@ BOOST_AUTO_TEST_CASE(beacon_registry_GetBeaconChainletRoot_test_2) beacon_payload1.m_signature = TestKey::Signature(beacon_payload1); GRC::Contract contract1 = GRC::MakeContract(3, GRC::ContractAction::ADD, beacon_payload1); - GRC::ContractContext ctx1 {contract1, tx1, pindex1}; + GRC::ContractContext ctx1 {contract1, tx1, &pindex1}; BOOST_CHECK(ctx1.m_contract.CopyPayloadAs().m_cpid == TestKey::Cpid()); BOOST_CHECK(ctx1.m_contract.CopyPayloadAs().m_beacon.m_status == GRC::BeaconStatusForStorage::PENDING); @@ -1512,18 +1512,18 @@ BOOST_AUTO_TEST_CASE(beacon_registry_GetBeaconChainletRoot_test_2) } // Activation - CBlockIndex* pindex2 = new CBlockIndex; - pindex2->nVersion = 13; - pindex2->nHeight = 2; - pindex2->nTime = int64_t {2}; - uint256* block2_phash = new uint256 {rng.rand256()}; - pindex2->phashBlock = block2_phash; + CBlockIndex pindex2 {}; + pindex2.nVersion = 13; + pindex2.nHeight = 2; + pindex2.nTime = int64_t {2}; + uint256 block2_phash = rng.rand256(); + pindex2.phashBlock = &block2_phash; std::vector beacon_ids {TestKey::Public().GetID()}; - registry.ActivatePending(beacon_ids, pindex2->nTime, *pindex2->phashBlock, pindex2->nHeight); + registry.ActivatePending(beacon_ids, pindex2.nTime, *pindex2.phashBlock, pindex2.nHeight); - uint256 activated_beacon_hash = Hash(*block2_phash, pending_beacons[0]->m_hash); + uint256 activated_beacon_hash = Hash(block2_phash, pending_beacons[0]->m_hash); BOOST_CHECK(registry.GetBeaconDB().size() == 2); From d73c0cf93eca7e3f763a51fc16a5ab2c1e50ba06 Mon Sep 17 00:00:00 2001 From: div72 Date: Mon, 4 Mar 2024 15:56:55 +0300 Subject: [PATCH 16/23] test: fix integer underflow in accounting tests --- src/test/accounting_tests.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/accounting_tests.cpp b/src/test/accounting_tests.cpp index 50259351a1..a89a266fbf 100644 --- a/src/test/accounting_tests.cpp +++ b/src/test/accounting_tests.cpp @@ -78,13 +78,13 @@ BOOST_AUTO_TEST_CASE(acc_orderupgrade) wtx.mapValue["comment"] = "y"; - --wtx.nLockTime; // Just to change the hash :) + ++wtx.nLockTime; // Just to change the hash :) pwalletMain->AddToWallet(wtx, &walletdb); vpwtx.push_back(&pwalletMain->mapWallet[wtx.GetHash()]); vpwtx[1]->nTimeReceived = (unsigned int)1333333336; wtx.mapValue["comment"] = "x"; - --wtx.nLockTime; // Just to change the hash :) + ++wtx.nLockTime; // Just to change the hash :) pwalletMain->AddToWallet(wtx, &walletdb); vpwtx.push_back(&pwalletMain->mapWallet[wtx.GetHash()]); vpwtx[2]->nTimeReceived = (unsigned int)1333333329; From 28986ad22f2db11cc54b524e2100432fcad31d6f Mon Sep 17 00:00:00 2001 From: div72 Date: Mon, 4 Mar 2024 16:24:30 +0300 Subject: [PATCH 17/23] refactor: make CBlockIndex nflags enum unsigned --- src/main.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main.h b/src/main.h index 5af4e3e47a..a204117097 100644 --- a/src/main.h +++ b/src/main.h @@ -478,7 +478,7 @@ class CBlockIndex int nHeight; unsigned int nFlags; // ppcoin: block index flags - enum + enum : uint32_t { BLOCK_PROOF_OF_STAKE = (1 << 0), // is proof-of-stake block BLOCK_STAKE_ENTROPY = (1 << 1), // entropy bit for stake modifier From 7cd0247b4d0f2e0b1e1d273351b926441cceeee0 Mon Sep 17 00:00:00 2001 From: div72 Date: Mon, 4 Mar 2024 16:24:51 +0300 Subject: [PATCH 18/23] scraper: remove redundant cntPartsRcvd inc/decrement --- src/gridcoin/scraper/scraper_net.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/gridcoin/scraper/scraper_net.cpp b/src/gridcoin/scraper/scraper_net.cpp index 25cc1360e4..e00fb4d9e4 100644 --- a/src/gridcoin/scraper/scraper_net.cpp +++ b/src/gridcoin/scraper/scraper_net.cpp @@ -139,9 +139,7 @@ int CSplitBlob::addPartData(CDataStream&& vData) { /* missing data; use the supplied data */ /* prevent calling the Complete callback FIXME: make this look better */ - cntPartsRcvd--; CSplitBlob::RecvPart(nullptr, vData); - cntPartsRcvd++; } return n; } From 6e922877ae53c0a71c03ad712b6b433e43ae0f42 Mon Sep 17 00:00:00 2001 From: div72 Date: Tue, 5 Mar 2024 13:24:06 +0300 Subject: [PATCH 19/23] superblock: eliminate UB during binary magnitude extraction --- src/gridcoin/superblock.cpp | 5 +++-- src/test/gridcoin/superblock_tests.cpp | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/gridcoin/superblock.cpp b/src/gridcoin/superblock.cpp index 4b9e7c61d7..c2f8251de6 100644 --- a/src/gridcoin/superblock.cpp +++ b/src/gridcoin/superblock.cpp @@ -456,8 +456,9 @@ class LegacySuperblockParser for (size_t x = 0; x < binary_size && binary_size - x >= 18; x += 18) { magnitudes.AddLegacy( - *reinterpret_cast(byte_ptr + x), - be16toh(*reinterpret_cast(byte_ptr + x + 16))); + Cpid(std::vector(byte_ptr + x, byte_ptr + x + 16)), + (((uint16_t)byte_ptr[x + 16] & 0xFF) << 8) | ((uint16_t)byte_ptr[x + 17] & 0xFF) + ); } return magnitudes; diff --git a/src/test/gridcoin/superblock_tests.cpp b/src/test/gridcoin/superblock_tests.cpp index 61a8a7642c..eea417b628 100644 --- a/src/test/gridcoin/superblock_tests.cpp +++ b/src/test/gridcoin/superblock_tests.cpp @@ -33,7 +33,7 @@ struct Legacy struct BinaryResearcher { std::array cpid; - int16_t magnitude; + uint16_t magnitude; }; static std::string ExtractValue(std::string data, std::string delimiter, int pos) From 2c233c1f4a152c96fc782d39004b3174427a6d92 Mon Sep 17 00:00:00 2001 From: div72 Date: Tue, 5 Mar 2024 14:17:59 +0300 Subject: [PATCH 20/23] util: avoid underflow in SplitHostPort --- src/util/strencodings.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/strencodings.cpp b/src/util/strencodings.cpp index db89881f26..d2f0a4852f 100644 --- a/src/util/strencodings.cpp +++ b/src/util/strencodings.cpp @@ -112,7 +112,7 @@ void SplitHostPort(std::string in, int &portOut, std::string &hostOut) { // if a : is found, and it either follows a [...], or no other : is in the string, treat it as port separator bool fHaveColon = colon != in.npos; bool fBracketed = fHaveColon && (in[0]=='[' && in[colon-1]==']'); // if there is a colon, and in[0]=='[', colon is not 0, so in[colon-1] is safe - bool fMultiColon = fHaveColon && (in.find_last_of(':',colon-1) != in.npos); + bool fMultiColon = fHaveColon && (colon != 0) && (in.find_last_of(':',colon-1) != in.npos); if (fHaveColon && (colon==0 || fBracketed || !fMultiColon)) { int32_t n; if (ParseInt32(in.substr(colon + 1), &n) && n > 0 && n < 0x10000) { From 8be87712216a3183fb7a086d04e43b79ffed1be8 Mon Sep 17 00:00:00 2001 From: div72 Date: Tue, 5 Mar 2024 14:18:09 +0300 Subject: [PATCH 21/23] refactor: port upstream changes for merkle --- src/consensus/merkle.cpp | 130 ++++---------------------------------- src/consensus/merkle.h | 12 +--- src/test/merkle_tests.cpp | 123 ++++++++++++++++++++++++++++++++++++ 3 files changed, 138 insertions(+), 127 deletions(-) diff --git a/src/consensus/merkle.cpp b/src/consensus/merkle.cpp index 7d15d9d52c..7fbe05adf8 100644 --- a/src/consensus/merkle.cpp +++ b/src/consensus/merkle.cpp @@ -37,118 +37,25 @@ root. */ -/* This implements a constant-space merkle root/path calculator, limited to 2^32 leaves. */ -static void MerkleComputation(const std::vector& leaves, uint256* proot, bool* pmutated, uint32_t branchpos, std::vector* pbranch) { - if (pbranch) pbranch->clear(); - if (leaves.size() == 0) { - if (pmutated) *pmutated = false; - if (proot) *proot = uint256(); - return; - } - bool mutated = false; - // count is the number of leaves processed so far. - uint32_t count = 0; - // inner is an array of eagerly computed subtree hashes, indexed by tree - // level (0 being the leaves). - // For example, when count is 25 (11001 in binary), inner[4] is the hash of - // the first 16 leaves, inner[3] of the next 8 leaves, and inner[0] equal to - // the last leaf. The other inner entries are undefined. - uint256 inner[32]; - // Which position in inner is a hash that depends on the matching leaf. - int matchlevel = -1; - // First process all leaves into 'inner' values. - while (count < leaves.size()) { - uint256 h = leaves[count]; - bool matchh = count == branchpos; - count++; - int level; - // For each of the lower bits in count that are 0, do 1 step. Each - // corresponds to an inner value that existed before processing the - // current leaf, and each needs a hash to combine it. - for (level = 0; !(count & (((uint32_t)1) << level)); level++) { - if (pbranch) { - if (matchh) { - pbranch->push_back(inner[level]); - } else if (matchlevel == level) { - pbranch->push_back(h); - matchh = true; - } +uint256 ComputeMerkleRoot(std::vector hashes, bool* mutated) { + bool mutation = false; + while (hashes.size() > 1) { + if (mutated) { + for (size_t pos = 0; pos + 1 < hashes.size(); pos += 2) { + if (hashes[pos] == hashes[pos + 1]) mutation = true; } - mutated |= (inner[level] == h); - CHash256().Write(inner[level]).Write(h).Finalize(h); - } - // Store the resulting hash at inner position level. - inner[level] = h; - if (matchh) { - matchlevel = level; - } - } - // Do a final 'sweep' over the rightmost branch of the tree to process - // odd levels, and reduce everything to a single top value. - // Level is the level (counted from the bottom) up to which we've sweeped. - int level = 0; - // As long as bit number level in count is zero, skip it. It means there - // is nothing left at this level. - while (!(count & (((uint32_t)1) << level))) { - level++; - } - uint256 h = inner[level]; - bool matchh = matchlevel == level; - while (count != (((uint32_t)1) << level)) { - // If we reach this point, h is an inner value that is not the top. - // We combine it with itself (Bitcoin's special rule for odd levels in - // the tree) to produce a higher level one. - if (pbranch && matchh) { - pbranch->push_back(h); } - CHash256().Write(h).Write(h).Finalize(h); - // Increment count to the value it would have if two entries at this - // level had existed. - count += (((uint32_t)1) << level); - level++; - // And propagate the result upwards accordingly. - while (!(count & (((uint32_t)1) << level))) { - if (pbranch) { - if (matchh) { - pbranch->push_back(inner[level]); - } else if (matchlevel == level) { - pbranch->push_back(h); - matchh = true; - } - } - CHash256().Write(inner[level]).Write(h).Finalize(h); - level++; + if (hashes.size() & 1) { + hashes.push_back(hashes.back()); } + SHA256D64(hashes[0].begin(), hashes[0].begin(), hashes.size() / 2); + hashes.resize(hashes.size() / 2); } - // Return result. - if (pmutated) *pmutated = mutated; - if (proot) *proot = h; -} - -uint256 ComputeMerkleRoot(const std::vector& leaves, bool* mutated) { - uint256 hash; - MerkleComputation(leaves, &hash, mutated, -1, nullptr); - return hash; -} - -std::vector ComputeMerkleBranch(const std::vector& leaves, uint32_t position) { - std::vector ret; - MerkleComputation(leaves, nullptr, nullptr, position, &ret); - return ret; + if (mutated) *mutated = mutation; + if (hashes.size() == 0) return uint256(); + return hashes[0]; } -uint256 ComputeMerkleRootFromBranch(const uint256& leaf, const std::vector& vMerkleBranch, uint32_t nIndex) { - uint256 hash = leaf; - for (std::vector::const_iterator it = vMerkleBranch.begin(); it != vMerkleBranch.end(); ++it) { - if (nIndex & 1) { - hash = Hash(*it, hash); - } else { - hash = Hash(hash, *it); - } - nIndex >>= 1; - } - return hash; -} uint256 BlockMerkleRoot(const CBlock& block, bool* mutated) { @@ -157,15 +64,6 @@ uint256 BlockMerkleRoot(const CBlock& block, bool* mutated) for (size_t s = 0; s < block.vtx.size(); s++) { leaves[s] = block.vtx[s].GetHash(); } - return ComputeMerkleRoot(leaves, mutated); + return ComputeMerkleRoot(std::move(leaves), mutated); } -std::vector BlockMerkleBranch(const CBlock& block, uint32_t position) -{ - std::vector leaves; - leaves.resize(block.vtx.size()); - for (size_t s = 0; s < block.vtx.size(); s++) { - leaves[s] = block.vtx[s].GetHash(); - } - return ComputeMerkleBranch(leaves, position); -} diff --git a/src/consensus/merkle.h b/src/consensus/merkle.h index 439e60ed30..beb78eeeda 100644 --- a/src/consensus/merkle.h +++ b/src/consensus/merkle.h @@ -5,15 +5,12 @@ #ifndef BITCOIN_CONSENSUS_MERKLE_H #define BITCOIN_CONSENSUS_MERKLE_H -#include #include #include "main.h" #include -uint256 ComputeMerkleRoot(const std::vector& leaves, bool* mutated = nullptr); -std::vector ComputeMerkleBranch(const std::vector& leaves, uint32_t position); -uint256 ComputeMerkleRootFromBranch(const uint256& leaf, const std::vector& branch, uint32_t position); +uint256 ComputeMerkleRoot(std::vector leaves, bool* mutated = nullptr); /* * Compute the Merkle root of the transactions in a block. @@ -21,11 +18,4 @@ uint256 ComputeMerkleRootFromBranch(const uint256& leaf, const std::vector BlockMerkleBranch(const CBlock& block, uint32_t position); - #endif // BITCOIN_CONSENSUS_MERKLE_H diff --git a/src/test/merkle_tests.cpp b/src/test/merkle_tests.cpp index 36463d42d6..4fc02a1818 100644 --- a/src/test/merkle_tests.cpp +++ b/src/test/merkle_tests.cpp @@ -9,6 +9,129 @@ BOOST_AUTO_TEST_SUITE(merkle_tests) +/* This implements a constant-space merkle root/path calculator, limited to 2^32 leaves. */ +static void MerkleComputation(const std::vector& leaves, uint256* proot, bool* pmutated, uint32_t branchpos, std::vector* pbranch) { + if (pbranch) pbranch->clear(); + if (leaves.size() == 0) { + if (pmutated) *pmutated = false; + if (proot) *proot = uint256(); + return; + } + bool mutated = false; + // count is the number of leaves processed so far. + uint32_t count = 0; + // inner is an array of eagerly computed subtree hashes, indexed by tree + // level (0 being the leaves). + // For example, when count is 25 (11001 in binary), inner[4] is the hash of + // the first 16 leaves, inner[3] of the next 8 leaves, and inner[0] equal to + // the last leaf. The other inner entries are undefined. + uint256 inner[32]; + // Which position in inner is a hash that depends on the matching leaf. + int matchlevel = -1; + // First process all leaves into 'inner' values. + while (count < leaves.size()) { + uint256 h = leaves[count]; + bool matchh = count == branchpos; + count++; + int level; + // For each of the lower bits in count that are 0, do 1 step. Each + // corresponds to an inner value that existed before processing the + // current leaf, and each needs a hash to combine it. + for (level = 0; !(count & (((uint32_t)1) << level)); level++) { + if (pbranch) { + if (matchh) { + pbranch->push_back(inner[level]); + } else if (matchlevel == level) { + pbranch->push_back(h); + matchh = true; + } + } + mutated |= (inner[level] == h); + CHash256().Write(inner[level]).Write(h).Finalize(h); + } + // Store the resulting hash at inner position level. + inner[level] = h; + if (matchh) { + matchlevel = level; + } + } + // Do a final 'sweep' over the rightmost branch of the tree to process + // odd levels, and reduce everything to a single top value. + // Level is the level (counted from the bottom) up to which we've sweeped. + int level = 0; + // As long as bit number level in count is zero, skip it. It means there + // is nothing left at this level. + while (!(count & (((uint32_t)1) << level))) { + level++; + } + uint256 h = inner[level]; + bool matchh = matchlevel == level; + while (count != (((uint32_t)1) << level)) { + // If we reach this point, h is an inner value that is not the top. + // We combine it with itself (Bitcoin's special rule for odd levels in + // the tree) to produce a higher level one. + if (pbranch && matchh) { + pbranch->push_back(h); + } + CHash256().Write(h).Write(h).Finalize(h); + // Increment count to the value it would have if two entries at this + // level had existed. + count += (((uint32_t)1) << level); + level++; + // And propagate the result upwards accordingly. + while (!(count & (((uint32_t)1) << level))) { + if (pbranch) { + if (matchh) { + pbranch->push_back(inner[level]); + } else if (matchlevel == level) { + pbranch->push_back(h); + matchh = true; + } + } + CHash256().Write(inner[level]).Write(h).Finalize(h); + level++; + } + } + // Return result. + if (pmutated) *pmutated = mutated; + if (proot) *proot = h; +} + +uint256 ComputeMerkleRoot(const std::vector& leaves, bool* mutated) { + uint256 hash; + MerkleComputation(leaves, &hash, mutated, -1, nullptr); + return hash; +} + +std::vector ComputeMerkleBranch(const std::vector& leaves, uint32_t position) { + std::vector ret; + MerkleComputation(leaves, nullptr, nullptr, position, &ret); + return ret; +} + +uint256 ComputeMerkleRootFromBranch(const uint256& leaf, const std::vector& vMerkleBranch, uint32_t nIndex) { + uint256 hash = leaf; + for (std::vector::const_iterator it = vMerkleBranch.begin(); it != vMerkleBranch.end(); ++it) { + if (nIndex & 1) { + hash = Hash(*it, hash); + } else { + hash = Hash(hash, *it); + } + nIndex >>= 1; + } + return hash; +} + +std::vector BlockMerkleBranch(const CBlock& block, uint32_t position) +{ + std::vector leaves; + leaves.resize(block.vtx.size()); + for (size_t s = 0; s < block.vtx.size(); s++) { + leaves[s] = block.vtx[s].GetHash(); + } + return ComputeMerkleBranch(leaves, position); +} + // Older version of the merkle root computation code, for comparison. static uint256 BlockBuildMerkleTree(const CBlock& block, std::vector& vMerkleTree) { From 08f65b865cc092c41ceceb966243db78eec81a0f Mon Sep 17 00:00:00 2001 From: div72 Date: Tue, 5 Mar 2024 14:33:38 +0300 Subject: [PATCH 22/23] refactor: make uint32_t casts explicit in tx tests --- src/test/transaction_tests.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp index 26809b7214..dbb899992f 100755 --- a/src/test/transaction_tests.cpp +++ b/src/test/transaction_tests.cpp @@ -106,7 +106,7 @@ BOOST_AUTO_TEST_CASE(tx_valid) break; } - COutPoint outpoint(uint256S(vinput[0].get_str()), vinput[1].get_int()); + COutPoint outpoint(uint256S(vinput[0].get_str()), uint32_t(vinput[1].get_int())); mapprevOutScriptPubKeys[outpoint] = ParseScript(vinput[2].get_str()); } @@ -175,7 +175,7 @@ BOOST_AUTO_TEST_CASE(tx_invalid) fValid = false; break; } - COutPoint outpoint(uint256S(vinput[0].get_str()), vinput[1].get_int()); + COutPoint outpoint(uint256S(vinput[0].get_str()), uint32_t(vinput[1].get_int())); mapprevOutScriptPubKeys[outpoint] = ParseScript(vinput[2].get_str()); } if (!fValid) From 0e8552dc13a9e26132a8ad74caf87d65261cd153 Mon Sep 17 00:00:00 2001 From: div72 Date: Tue, 5 Mar 2024 14:44:30 +0300 Subject: [PATCH 23/23] test: make int64_t casts explicit in msb tests --- src/test/util_tests.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp index 5ccb4fc05e..1aeccd2785 100755 --- a/src/test/util_tests.cpp +++ b/src/test/util_tests.cpp @@ -1173,7 +1173,7 @@ BOOST_AUTO_TEST_CASE(Fraction_msb_performance_test) g_timer.InitTimer("msb_test", true); for (unsigned int i = 0; i < iterations; ++i) { - msb(rand.rand64()); + msb((int64_t)rand.rand64()); } int64_t msb_test_time = g_timer.GetTimes(strprintf("msb %u iterations", iterations), "msb_test").time_since_last_check; @@ -1181,7 +1181,7 @@ BOOST_AUTO_TEST_CASE(Fraction_msb_performance_test) FastRandomContext rand2(uint256 {0}); for (unsigned int i = 0; i < iterations; ++i) { - msb2(rand2.rand64()); + msb2((int64_t)rand2.rand64()); } int64_t msb2_test_time = g_timer.GetTimes(strprintf("msb2 %u iterations", iterations), "msb_test").time_since_last_check; @@ -1189,7 +1189,7 @@ BOOST_AUTO_TEST_CASE(Fraction_msb_performance_test) FastRandomContext rand3(uint256 {0}); for (unsigned int i = 0; i < iterations; ++i) { - msb3(rand3.rand64()); + msb3((int64_t)rand3.rand64()); } int64_t msb3_test_time = g_timer.GetTimes(strprintf("msb3 %u iterations", iterations), "msb_test").time_since_last_check;