diff options
33 files changed, 147 insertions, 174 deletions
diff --git a/.cirrus.yml b/.cirrus.yml index 82aa39b296..fce3a1bb8e 100644 --- a/.cirrus.yml +++ b/.cirrus.yml @@ -195,7 +195,7 @@ task: FILE_ENV: "./ci/test/00_setup_env_i686_centos.sh" task: - name: '[previous releases, uses qt5 dev package and some depends packages, DEBUG] [unsigned char] [bionic]' + name: '[previous releases, uses qt5 dev package and some depends packages, DEBUG] [unsigned char] [buster]' previous_releases_cache: folder: "releases" << : *GLOBAL_TASK_TEMPLATE diff --git a/ci/test/00_setup_env_native_qt5.sh b/ci/test/00_setup_env_native_qt5.sh index f8b366e77f..53f933f514 100755 --- a/ci/test/00_setup_env_native_qt5.sh +++ b/ci/test/00_setup_env_native_qt5.sh @@ -7,7 +7,7 @@ export LC_ALL=C.UTF-8 export CONTAINER_NAME=ci_native_qt5 -export DOCKER_NAME_TAG=ubuntu:18.04 # Check that bionic gcc-8 can compile our C++17 and run our functional tests in python3, see doc/dependencies.md +export DOCKER_NAME_TAG=debian:buster # Check that buster gcc-8 can compile our C++17 and run our functional tests in python3, see doc/dependencies.md export PACKAGES="gcc-8 g++-8 python3-zmq qtbase5-dev qttools5-dev-tools libdbus-1-dev libharfbuzz-dev" export DEP_OPTS="NO_QT=1 NO_UPNP=1 NO_NATPMP=1 DEBUG=1 ALLOW_HOST_PACKAGES=1 CC=gcc-8 CXX=g++-8" export TEST_RUNNER_EXTRA="--previous-releases --coverage --extended --exclude feature_dbcrash" # Run extended tests so that coverage does not fail, but exclude the very slow dbcrash diff --git a/configure.ac b/configure.ac index 48684c7c98..06a208835b 100644 --- a/configure.ac +++ b/configure.ac @@ -1309,7 +1309,7 @@ else BITCOIN_QT_INIT dnl sets $bitcoin_enable_qt, $bitcoin_enable_qt_test, $bitcoin_enable_qt_dbus - BITCOIN_QT_CONFIGURE([5.9.5]) + BITCOIN_QT_CONFIGURE([5.11.3]) dnl Keep a copy of the original $QT_INCLUDES and use it when invoking qt's moc QT_INCLUDES_UNSUPPRESSED=$QT_INCLUDES diff --git a/contrib/guix/guix-build b/contrib/guix/guix-build index b38f91f3c7..3f14be7fdd 100755 --- a/contrib/guix/guix-build +++ b/contrib/guix/guix-build @@ -234,21 +234,6 @@ host_to_commonname() { # Determine the reference time used for determinism (overridable by environment) SOURCE_DATE_EPOCH="${SOURCE_DATE_EPOCH:-$(git -c log.showSignature=false log --format=%at -1)}" -# Execute "$@" in a pinned, possibly older version of Guix, for reproducibility -# across time. -time-machine() { - # shellcheck disable=SC2086 - guix time-machine --url=https://git.savannah.gnu.org/git/guix.git \ - --commit=ae03f401381e956c4c41b4cf495cbde964fa43d0 \ - --cores="$JOBS" \ - --keep-failed \ - --fallback \ - ${SUBSTITUTE_URLS:+--substitute-urls="$SUBSTITUTE_URLS"} \ - ${ADDITIONAL_GUIX_COMMON_FLAGS} ${ADDITIONAL_GUIX_TIMEMACHINE_FLAGS} \ - -- "$@" -} - - # Precious directories are those which should not be cleaned between successive # guix builds depends_precious_dir_names='SOURCES_PATH BASE_CACHE SDK_PATH' diff --git a/contrib/guix/guix-codesign b/contrib/guix/guix-codesign index 8be927bd06..94895d3fee 100755 --- a/contrib/guix/guix-codesign +++ b/contrib/guix/guix-codesign @@ -222,20 +222,6 @@ JOBS="${JOBS:-$(nproc)}" # Determine the reference time used for determinism (overridable by environment) SOURCE_DATE_EPOCH="${SOURCE_DATE_EPOCH:-$(git -c log.showSignature=false log --format=%at -1)}" -# Execute "$@" in a pinned, possibly older version of Guix, for reproducibility -# across time. -time-machine() { - # shellcheck disable=SC2086 - guix time-machine --url=https://git.savannah.gnu.org/git/guix.git \ - --commit=ae03f401381e956c4c41b4cf495cbde964fa43d0 \ - --cores="$JOBS" \ - --keep-failed \ - --fallback \ - ${SUBSTITUTE_URLS:+--substitute-urls="$SUBSTITUTE_URLS"} \ - ${ADDITIONAL_GUIX_COMMON_FLAGS} ${ADDITIONAL_GUIX_TIMEMACHINE_FLAGS} \ - -- "$@" -} - # Make sure an output directory exists for our builds OUTDIR_BASE="${OUTDIR_BASE:-${VERSION_BASE}/output}" mkdir -p "$OUTDIR_BASE" diff --git a/contrib/guix/libexec/prelude.bash b/contrib/guix/libexec/prelude.bash index 1f287e9bbb..086df48fbe 100644 --- a/contrib/guix/libexec/prelude.bash +++ b/contrib/guix/libexec/prelude.bash @@ -46,6 +46,22 @@ exit 1 fi ################ +# Execute "$@" in a pinned, possibly older version of Guix, for reproducibility +# across time. +time-machine() { + # shellcheck disable=SC2086 + guix time-machine --url=https://git.savannah.gnu.org/git/guix.git \ + --commit=ae03f401381e956c4c41b4cf495cbde964fa43d0 \ + --cores="$JOBS" \ + --keep-failed \ + --fallback \ + ${SUBSTITUTE_URLS:+--substitute-urls="$SUBSTITUTE_URLS"} \ + ${ADDITIONAL_GUIX_COMMON_FLAGS} ${ADDITIONAL_GUIX_TIMEMACHINE_FLAGS} \ + -- "$@" +} + + +################ # Set common variables ################ diff --git a/contrib/guix/manifest.scm b/contrib/guix/manifest.scm index d296eb9543..9512050ce4 100644 --- a/contrib/guix/manifest.scm +++ b/contrib/guix/manifest.scm @@ -352,7 +352,7 @@ thus should be able to compile on most platforms where these exist.") #t))))))) (define-public python-certvalidator - (let ((commit "e5bdb4bfcaa09fa0af355eb8867d00dfeecba08c")) + (let ((commit "a145bf25eb75a9f014b3e7678826132efbba6213")) (package (name "python-certvalidator") (version (git-version "0.1" "1" commit)) @@ -365,7 +365,7 @@ thus should be able to compile on most platforms where these exist.") (file-name (git-file-name name commit)) (sha256 (base32 - "18pvxkvpkfkzgvfylv0kx65pmxfcv1hpsg03cip93krfvrrl4c75")))) + "1qw2k7xis53179lpqdqyylbcmp76lj7sagp883wmxg5i7chhc96k")))) (build-system python-build-system) (propagated-inputs `(("python-asn1crypto" ,python-asn1crypto) @@ -401,11 +401,6 @@ thus should be able to compile on most platforms where these exist.") (string-append indent "@unittest.skip(\"Disabled by Guix\")\n" line))) - (substitute* "tests/test_validate.py" - (("^(.*)def test_revocation_mode_soft" line indent) - (string-append indent - "@unittest.skip(\"Disabled by Guix\")\n" - line))) #t)) (replace 'check (lambda _ @@ -490,7 +485,7 @@ and endian independent.") (license license:expat))) (define-public python-signapple - (let ((commit "b084cbbf44d5330448ffce0c7d118f75781b64bd")) + (let ((commit "9f42f3c8295d4107ee7a22e523ec17449a936f43")) (package (name "python-signapple") (version (git-version "0.1" "1" commit)) @@ -503,7 +498,7 @@ and endian independent.") (file-name (git-file-name name commit)) (sha256 (base32 - "0k7inccl2mzac3wq4asbr0kl8s4cghm8982z54kfascqg45shv01")))) + "0j1sqi0g8k2z5y56iayh5pw9yyq1r6ry3q5zy0cdy2sispiwvdnp")))) (build-system python-build-system) (propagated-inputs `(("python-asn1crypto" ,python-asn1crypto) @@ -593,24 +588,30 @@ inspecting signatures in Mach-O binaries.") ;; Git git ;; Tests - lief - ;; Native gcc 7 toolchain - gcc-toolchain-7 - (list gcc-toolchain-7 "static")) + lief) (let ((target (getenv "HOST"))) (cond ((string-suffix? "-mingw32" target) ;; Windows - (list zip + (list ;; Native GCC 10 toolchain + gcc-toolchain-10 + (list gcc-toolchain-10 "static") + zip (make-mingw-pthreads-cross-toolchain "x86_64-w64-mingw32") (make-nsis-for-gcc-10 nsis-x86_64) osslsigncode)) ((string-contains target "-linux-") - (list (cond ((string-contains target "riscv64-") + (list ;; Native GCC 7 toolchain + gcc-toolchain-7 + (list gcc-toolchain-7 "static") + (cond ((string-contains target "riscv64-") (make-bitcoin-cross-toolchain target #:base-libc glibc-2.27/bitcoin-patched #:base-kernel-headers linux-libre-headers-4.19)) (else (make-bitcoin-cross-toolchain target))))) ((string-contains target "darwin") - (list clang-toolchain-10 binutils cmake xorriso python-signapple)) + (list ;; Native GCC 10 toolchain + gcc-toolchain-10 + (list gcc-toolchain-10 "static") + clang-toolchain-10 binutils cmake xorriso python-signapple)) (else '()))))) diff --git a/doc/dependencies.md b/doc/dependencies.md index 99ff26f457..21af338119 100644 --- a/doc/dependencies.md +++ b/doc/dependencies.md @@ -20,7 +20,7 @@ These are the dependencies currently used by Bitcoin Core. You can find instruct | PCRE | | | | | [Yes](https://github.com/bitcoin/bitcoin/blob/master/depends/packages/qt.mk) | | Python (tests) | | [3.6](https://www.python.org/downloads) | | | | | qrencode | [3.4.4](https://fukuchi.org/works/qrencode) | | No | | | -| Qt | [5.15.2](https://download.qt.io/official_releases/qt/) | [5.9.5](https://github.com/bitcoin/bitcoin/issues/20104) | No | | | +| Qt | [5.15.2](https://download.qt.io/official_releases/qt/) | [5.11.3](https://github.com/bitcoin/bitcoin/pull/24132) | No | | | | SQLite | [3.32.1](https://sqlite.org/download.html) | [3.7.17](https://github.com/bitcoin/bitcoin/pull/19077) | | | | | XCB | | | | | [Yes](https://github.com/bitcoin/bitcoin/blob/master/depends/packages/qt.mk) (Linux only) | | systemtap ([tracing](tracing.md))| [4.5](https://sourceware.org/systemtap/ftp/releases/) | | | | | diff --git a/doc/i2p.md b/doc/i2p.md index ee650f3999..e45b5efb9b 100644 --- a/doc/i2p.md +++ b/doc/i2p.md @@ -65,9 +65,9 @@ logging` for more information. -onlynet=i2p ``` -Make outgoing connections only to I2P addresses. Incoming connections are not -affected by this option. It can be specified multiple times to allow multiple -network types, e.g. onlynet=onion, onlynet=i2p. +Make automatic outbound connections only to I2P addresses. Inbound and manual +connections are not affected by this option. It can be specified multiple times +to allow multiple networks, e.g. onlynet=onion, onlynet=i2p. I2P support was added to Bitcoin Core in version 22.0 and there may be fewer I2P peers than Tor or IP ones. Therefore, using I2P alone without other networks may diff --git a/doc/p2p-bad-ports.md b/doc/p2p-bad-ports.md index 0dd7d36cf4..4f717f97a2 100644 --- a/doc/p2p-bad-ports.md +++ b/doc/p2p-bad-ports.md @@ -1,6 +1,6 @@ -When Bitcoin Core automatically opens outgoing P2P connections it chooses +When Bitcoin Core automatically opens outgoing P2P connections, it chooses a peer (address and port) from its list of potential peers. This list is -populated with unchecked data, gossiped over the P2P network by other peers. +populated with unchecked data gossiped over the P2P network by other peers. A malicious actor may gossip an address:port where no Bitcoin node is listening, or one where a service is listening that is not related to the Bitcoin network. @@ -17,7 +17,7 @@ authentication are unlikely to be considered a malicious action, e.g. port 80 (http). Below is a list of "bad" ports which Bitcoin Core avoids when choosing a peer to -connect to. If a node is listening on such a port, it will likely receive less +connect to. If a node is listening on such a port, it will likely receive fewer incoming connections. 1: tcpmux diff --git a/doc/release-notes-24198.md b/doc/release-notes-24198.md new file mode 100644 index 0000000000..e41b2a8e26 --- /dev/null +++ b/doc/release-notes-24198.md @@ -0,0 +1,6 @@ +Updated RPCs +------------ + +- The `listtransactions`, `gettransaction`, and `listsinceblock` + RPC methods now include a wtxid field (hash of serialized transaction, + including witness data) for each transaction.
\ No newline at end of file diff --git a/doc/tor.md b/doc/tor.md index 086e6747bf..b7c4f7d425 100644 --- a/doc/tor.md +++ b/doc/tor.md @@ -55,10 +55,10 @@ outgoing connections, but more is possible. -seednode=X SOCKS5. In Tor mode, such addresses can also be exchanged with other P2P nodes. - -onlynet=onion Make outgoing connections only to .onion addresses. Incoming - connections are not affected by this option. This option can be - specified multiple times to allow multiple network types, e.g. - onlynet=onion, onlynet=i2p. + -onlynet=onion Make automatic outbound connections only to .onion addresses. + Inbound and manual connections are not affected by this option. + It can be specified multiple times to allow multiple networks, + e.g. onlynet=onion, onlynet=i2p. In a typical situation, this suffices to run behind a Tor proxy: diff --git a/src/addrman.cpp b/src/addrman.cpp index f91a979934..2fd8143c1c 100644 --- a/src/addrman.cpp +++ b/src/addrman.cpp @@ -246,12 +246,18 @@ void AddrManImpl::Unserialize(Stream& s_) uint8_t compat; s >> compat; + if (compat < INCOMPATIBILITY_BASE) { + throw std::ios_base::failure(strprintf( + "Corrupted addrman database: The compat value (%u) " + "is lower than the expected minimum value %u.", + compat, INCOMPATIBILITY_BASE)); + } const uint8_t lowest_compatible = compat - INCOMPATIBILITY_BASE; if (lowest_compatible > FILE_FORMAT) { throw InvalidAddrManVersionError(strprintf( "Unsupported format of addrman database: %u. It is compatible with formats >=%u, " "but the maximum supported by this version of %s is %u.", - uint8_t{format}, uint8_t{lowest_compatible}, PACKAGE_NAME, uint8_t{FILE_FORMAT})); + uint8_t{format}, lowest_compatible, PACKAGE_NAME, uint8_t{FILE_FORMAT})); } s >> nKey; diff --git a/src/bitcoin-chainstate.cpp b/src/bitcoin-chainstate.cpp index f93197350d..72b8fefcc7 100644 --- a/src/bitcoin-chainstate.cpp +++ b/src/bitcoin-chainstate.cpp @@ -256,7 +256,7 @@ epilogue: } GetMainSignals().UnregisterBackgroundSignalScheduler(); - UnloadBlockIndex(nullptr, chainman); + WITH_LOCK(::cs_main, UnloadBlockIndex(nullptr, chainman)); init::UnsetGlobals(); } diff --git a/src/init.cpp b/src/init.cpp index 3110797194..bad402e56e 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -462,7 +462,7 @@ void SetupServerArgs(ArgsManager& argsman) argsman.AddArg("-onion=<ip:port>", "Use separate SOCKS5 proxy to reach peers via Tor onion services, set -noonion to disable (default: -proxy)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-i2psam=<ip:port>", "I2P SAM proxy to reach I2P peers and accept I2P connections (default: none)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-i2pacceptincoming", "If set and -i2psam is also set then incoming I2P connections are accepted via the SAM proxy. If this is not set but -i2psam is set then only outgoing connections will be made to the I2P network. Ignored if -i2psam is not set. Listening for incoming I2P connections is done through the SAM proxy, not by binding to a local address and port (default: 1)", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); - argsman.AddArg("-onlynet=<net>", "Make automatic outgoing connections only through network <net> (" + Join(GetNetworkNames(), ", ") + "). Incoming connections are not affected by this option. This option can be specified multiple times to allow multiple networks.", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); + argsman.AddArg("-onlynet=<net>", "Make automatic outbound connections only to network <net> (" + Join(GetNetworkNames(), ", ") + "). Inbound and manual connections are not affected by this option. It can be specified multiple times to allow multiple networks.", ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-peerbloomfilters", strprintf("Support filtering of blocks and transaction with bloom filters (default: %u)", DEFAULT_PEERBLOOMFILTERS), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-peerblockfilters", strprintf("Serve compact block filters to peers per BIP 157 (default: %u)", DEFAULT_PEERBLOCKFILTERS), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); argsman.AddArg("-permitbaremultisig", strprintf("Relay non-P2SH multisig (default: %u)", DEFAULT_PERMIT_BAREMULTISIG), ArgsManager::ALLOW_ANY, OptionsCategory::CONNECTION); diff --git a/src/leveldb/util/env_posix.cc b/src/leveldb/util/env_posix.cc index 9f5863a0f3..18626b327c 100644 --- a/src/leveldb/util/env_posix.cc +++ b/src/leveldb/util/env_posix.cc @@ -850,7 +850,7 @@ class SingletonEnv { public: SingletonEnv() { #if !defined(NDEBUG) - env_initialized_.store(true, std::memory_order::memory_order_relaxed); + env_initialized_.store(true, std::memory_order_relaxed); #endif // !defined(NDEBUG) static_assert(sizeof(env_storage_) >= sizeof(EnvType), "env_storage_ will not fit the Env"); @@ -867,7 +867,7 @@ class SingletonEnv { static void AssertEnvNotInitialized() { #if !defined(NDEBUG) - assert(!env_initialized_.load(std::memory_order::memory_order_relaxed)); + assert(!env_initialized_.load(std::memory_order_relaxed)); #endif // !defined(NDEBUG) } diff --git a/src/leveldb/util/env_windows.cc b/src/leveldb/util/env_windows.cc index 1834206562..4dcba222a1 100644 --- a/src/leveldb/util/env_windows.cc +++ b/src/leveldb/util/env_windows.cc @@ -798,7 +798,7 @@ class SingletonEnv { public: SingletonEnv() { #if !defined(NDEBUG) - env_initialized_.store(true, std::memory_order::memory_order_relaxed); + env_initialized_.store(true, std::memory_order_relaxed); #endif // !defined(NDEBUG) static_assert(sizeof(env_storage_) >= sizeof(EnvType), "env_storage_ will not fit the Env"); @@ -815,7 +815,7 @@ class SingletonEnv { static void AssertEnvNotInitialized() { #if !defined(NDEBUG) - assert(!env_initialized_.load(std::memory_order::memory_order_relaxed)); + assert(!env_initialized_.load(std::memory_order_relaxed)); #endif // !defined(NDEBUG) } diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index 8a99130fd0..7392830261 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -32,35 +32,39 @@ static FILE* OpenUndoFile(const FlatFilePos& pos, bool fReadOnly = false); static FlatFileSeq BlockFileSeq(); static FlatFileSeq UndoFileSeq(); -CBlockIndex* BlockManager::LookupBlockIndex(const uint256& hash) const +CBlockIndex* BlockManager::LookupBlockIndex(const uint256& hash) +{ + AssertLockHeld(cs_main); + BlockMap::iterator it = m_block_index.find(hash); + return it == m_block_index.end() ? nullptr : &it->second; +} + +const CBlockIndex* BlockManager::LookupBlockIndex(const uint256& hash) const { AssertLockHeld(cs_main); BlockMap::const_iterator it = m_block_index.find(hash); - return it == m_block_index.end() ? nullptr : it->second; + return it == m_block_index.end() ? nullptr : &it->second; } CBlockIndex* BlockManager::AddToBlockIndex(const CBlockHeader& block) { AssertLockHeld(cs_main); - // Check for duplicate - uint256 hash = block.GetHash(); - BlockMap::iterator it = m_block_index.find(hash); - if (it != m_block_index.end()) { - return it->second; + auto [mi, inserted] = m_block_index.try_emplace(block.GetHash(), block); + if (!inserted) { + return &mi->second; } + CBlockIndex* pindexNew = &(*mi).second; - // Construct new block index object - CBlockIndex* pindexNew = new CBlockIndex(block); // We assign the sequence id to blocks only when the full data is available, // to avoid miners withholding blocks but broadcasting headers, to get a // competitive advantage. pindexNew->nSequenceId = 0; - BlockMap::iterator mi = m_block_index.insert(std::make_pair(hash, pindexNew)).first; + pindexNew->phashBlock = &((*mi).first); BlockMap::iterator miPrev = m_block_index.find(block.hashPrevBlock); if (miPrev != m_block_index.end()) { - pindexNew->pprev = (*miPrev).second; + pindexNew->pprev = &(*miPrev).second; pindexNew->nHeight = pindexNew->pprev->nHeight + 1; pindexNew->BuildSkip(); } @@ -80,8 +84,8 @@ void BlockManager::PruneOneBlockFile(const int fileNumber) AssertLockHeld(cs_main); LOCK(cs_LastBlockFile); - for (const auto& entry : m_block_index) { - CBlockIndex* pindex = entry.second; + for (auto& entry : m_block_index) { + CBlockIndex* pindex = &entry.second; if (pindex->nFile == fileNumber) { pindex->nStatus &= ~BLOCK_HAVE_DATA; pindex->nStatus &= ~BLOCK_HAVE_UNDO; @@ -199,18 +203,13 @@ CBlockIndex* BlockManager::InsertBlockIndex(const uint256& hash) return nullptr; } - // Return existing - BlockMap::iterator mi = m_block_index.find(hash); - if (mi != m_block_index.end()) { - return (*mi).second; + // Return existing or create new + auto [mi, inserted] = m_block_index.try_emplace(hash); + CBlockIndex* pindex = &(*mi).second; + if (inserted) { + pindex->phashBlock = &((*mi).first); } - - // Create new - CBlockIndex* pindexNew = new CBlockIndex(); - mi = m_block_index.insert(std::make_pair(hash, pindexNew)).first; - pindexNew->phashBlock = &((*mi).first); - - return pindexNew; + return pindex; } bool BlockManager::LoadBlockIndex( @@ -224,8 +223,8 @@ bool BlockManager::LoadBlockIndex( // Calculate nChainWork std::vector<std::pair<int, CBlockIndex*>> vSortedByHeight; vSortedByHeight.reserve(m_block_index.size()); - for (const std::pair<const uint256, CBlockIndex*>& item : m_block_index) { - CBlockIndex* pindex = item.second; + for (auto& [_, block_index] : m_block_index) { + CBlockIndex* pindex = &block_index; vSortedByHeight.push_back(std::make_pair(pindex->nHeight, pindex)); } sort(vSortedByHeight.begin(), vSortedByHeight.end()); @@ -327,10 +326,6 @@ void BlockManager::Unload() { m_blocks_unlinked.clear(); - for (const BlockMap::value_type& entry : m_block_index) { - delete entry.second; - } - m_block_index.clear(); m_blockfile_info.clear(); @@ -386,8 +381,8 @@ bool BlockManager::LoadBlockIndexDB(ChainstateManager& chainman) // Check presence of blk files LogPrintf("Checking all blk files are present...\n"); std::set<int> setBlkDataFiles; - for (const std::pair<const uint256, CBlockIndex*>& item : m_block_index) { - CBlockIndex* pindex = item.second; + for (const auto& [_, block_index] : m_block_index) { + const CBlockIndex* pindex = &block_index; if (pindex->nStatus & BLOCK_HAVE_DATA) { setBlkDataFiles.insert(pindex->nFile); } diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h index 42e46797d2..12224f7a5d 100644 --- a/src/node/blockstorage.h +++ b/src/node/blockstorage.h @@ -5,6 +5,7 @@ #ifndef BITCOIN_NODE_BLOCKSTORAGE_H #define BITCOIN_NODE_BLOCKSTORAGE_H +#include <chain.h> #include <fs.h> #include <protocol.h> // For CMessageHeader::MessageStartChars #include <sync.h> @@ -20,7 +21,6 @@ class ArgsManager; class BlockValidationState; class CBlock; class CBlockFileInfo; -class CBlockIndex; class CBlockUndo; class CChain; class CChainParams; @@ -52,7 +52,11 @@ extern bool fPruneMode; /** Number of MiB of block files that we're trying to stay below. */ extern uint64_t nPruneTarget; -typedef std::unordered_map<uint256, CBlockIndex*, BlockHasher> BlockMap; +// Because validation code takes pointers to the map's CBlockIndex objects, if +// we ever switch to another associative container, we need to either use a +// container that has stable addressing (true of all std associative +// containers), or make the key a `std::unique_ptr<CBlockIndex>` +using BlockMap = std::unordered_map<uint256, CBlockIndex, BlockHasher>; struct CBlockIndexWorkComparator { bool operator()(const CBlockIndex* pa, const CBlockIndex* pb) const; @@ -144,7 +148,8 @@ public: //! Mark one block file as pruned (modify associated database entries) void PruneOneBlockFile(const int fileNumber) EXCLUSIVE_LOCKS_REQUIRED(cs_main); - CBlockIndex* LookupBlockIndex(const uint256& hash) const EXCLUSIVE_LOCKS_REQUIRED(cs_main); + CBlockIndex* LookupBlockIndex(const uint256& hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + const CBlockIndex* LookupBlockIndex(const uint256& hash) const EXCLUSIVE_LOCKS_REQUIRED(cs_main); /** Get block file info entry for one block file */ CBlockFileInfo* GetBlockFileInfo(size_t n); diff --git a/src/qt/guiutil.cpp b/src/qt/guiutil.cpp index d6706fd009..3108c93d7c 100644 --- a/src/qt/guiutil.cpp +++ b/src/qt/guiutil.cpp @@ -883,11 +883,7 @@ void PolishProgressDialog(QProgressDialog* dialog) int TextWidth(const QFontMetrics& fm, const QString& text) { -#if (QT_VERSION >= QT_VERSION_CHECK(5, 11, 0)) return fm.horizontalAdvance(text); -#else - return fm.width(text); -#endif } void LogQtInfo() diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index 9817c80cbd..86dfbbae35 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -1753,10 +1753,10 @@ static RPCHelpMan getchaintips() std::set<const CBlockIndex*> setOrphans; std::set<const CBlockIndex*> setPrevs; - for (const std::pair<const uint256, CBlockIndex*>& item : chainman.BlockIndex()) { - if (!active_chain.Contains(item.second)) { - setOrphans.insert(item.second); - setPrevs.insert(item.second->pprev); + for (const auto& [_, block_index] : chainman.BlockIndex()) { + if (!active_chain.Contains(&block_index)) { + setOrphans.insert(&block_index); + setPrevs.insert(block_index.pprev); } } diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index c968e4d124..211153f06c 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -180,10 +180,9 @@ ChainTestingSetup::~ChainTestingSetup() m_node.banman.reset(); m_node.addrman.reset(); m_node.args = nullptr; - UnloadBlockIndex(m_node.mempool.get(), *m_node.chainman); + WITH_LOCK(::cs_main, UnloadBlockIndex(m_node.mempool.get(), *m_node.chainman)); m_node.mempool.reset(); m_node.scheduler.reset(); - m_node.chainman->Reset(); m_node.chainman.reset(); } diff --git a/src/test/validation_chainstate_tests.cpp b/src/test/validation_chainstate_tests.cpp index 1beef5cf04..b0d7389d39 100644 --- a/src/test/validation_chainstate_tests.cpp +++ b/src/test/validation_chainstate_tests.cpp @@ -72,9 +72,6 @@ BOOST_AUTO_TEST_CASE(validation_chainstate_resize_caches) // The view cache should be empty since we had to destruct to downsize. BOOST_CHECK(!c1.CoinsTip().HaveCoinInCache(outpoint)); } - - // Avoid triggering the address sanitizer. - WITH_LOCK(::cs_main, manager.Unload()); } //! Test UpdateTip behavior for both active and background chainstates. diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp index 26392e690d..5d0ec593e3 100644 --- a/src/test/validation_chainstatemanager_tests.cpp +++ b/src/test/validation_chainstatemanager_tests.cpp @@ -99,8 +99,6 @@ BOOST_AUTO_TEST_CASE(chainstatemanager) // Let scheduler events finish running to avoid accessing memory that is going to be unloaded SyncWithValidationInterfaceQueue(); - - WITH_LOCK(::cs_main, manager.Unload()); } //! Test rebalancing the caches associated with each chainstate. diff --git a/src/validation.cpp b/src/validation.cpp index 214112e2bd..d80e2576d2 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -1978,7 +1978,7 @@ bool CChainState::ConnectBlock(const CBlock& block, BlockValidationState& state, // effectively caching the result of part of the verification. BlockMap::const_iterator it = m_blockman.m_block_index.find(hashAssumeValid); if (it != m_blockman.m_block_index.end()) { - if (it->second->GetAncestor(pindex->nHeight) == pindex && + if (it->second.GetAncestor(pindex->nHeight) == pindex && pindexBestHeader->GetAncestor(pindex->nHeight) == pindex && pindexBestHeader->nChainWork >= nMinimumChainWork) { // This block is a member of the assumed verified chain and an ancestor of the best header. @@ -3035,8 +3035,8 @@ bool CChainState::InvalidateBlock(BlockValidationState& state, CBlockIndex* pind { LOCK(cs_main); - for (const auto& entry : m_blockman.m_block_index) { - CBlockIndex *candidate = entry.second; + for (auto& entry : m_blockman.m_block_index) { + CBlockIndex* candidate = &entry.second; // We don't need to put anything in our active chain into the // multimap, because those candidates will be found and considered // as we disconnect. @@ -3133,12 +3133,10 @@ bool CChainState::InvalidateBlock(BlockValidationState& state, CBlockIndex* pind // it up here, this should be an essentially unobservable error. // Loop back over all block index entries and add any missing entries // to setBlockIndexCandidates. - BlockMap::iterator it = m_blockman.m_block_index.begin(); - while (it != m_blockman.m_block_index.end()) { - if (it->second->IsValid(BLOCK_VALID_TRANSACTIONS) && it->second->HaveTxsDownloaded() && !setBlockIndexCandidates.value_comp()(it->second, m_chain.Tip())) { - setBlockIndexCandidates.insert(it->second); + for (auto& [_, block_index] : m_blockman.m_block_index) { + if (block_index.IsValid(BLOCK_VALID_TRANSACTIONS) && block_index.HaveTxsDownloaded() && !setBlockIndexCandidates.value_comp()(&block_index, m_chain.Tip())) { + setBlockIndexCandidates.insert(&block_index); } - it++; } InvalidChainFound(to_mark_failed); @@ -3157,21 +3155,19 @@ void CChainState::ResetBlockFailureFlags(CBlockIndex *pindex) { int nHeight = pindex->nHeight; // Remove the invalidity flag from this block and all its descendants. - BlockMap::iterator it = m_blockman.m_block_index.begin(); - while (it != m_blockman.m_block_index.end()) { - if (!it->second->IsValid() && it->second->GetAncestor(nHeight) == pindex) { - it->second->nStatus &= ~BLOCK_FAILED_MASK; - m_blockman.m_dirty_blockindex.insert(it->second); - if (it->second->IsValid(BLOCK_VALID_TRANSACTIONS) && it->second->HaveTxsDownloaded() && setBlockIndexCandidates.value_comp()(m_chain.Tip(), it->second)) { - setBlockIndexCandidates.insert(it->second); + for (auto& [_, block_index] : m_blockman.m_block_index) { + if (!block_index.IsValid() && block_index.GetAncestor(nHeight) == pindex) { + block_index.nStatus &= ~BLOCK_FAILED_MASK; + m_blockman.m_dirty_blockindex.insert(&block_index); + if (block_index.IsValid(BLOCK_VALID_TRANSACTIONS) && block_index.HaveTxsDownloaded() && setBlockIndexCandidates.value_comp()(m_chain.Tip(), &block_index)) { + setBlockIndexCandidates.insert(&block_index); } - if (it->second == m_chainman.m_best_invalid) { + if (&block_index == m_chainman.m_best_invalid) { // Reset invalid block marker if it was pointing to one of those. m_chainman.m_best_invalid = nullptr; } - m_chainman.m_failed_blocks.erase(it->second); + m_chainman.m_failed_blocks.erase(&block_index); } - it++; } // Remove the invalidity flag from all ancestors too. @@ -3500,7 +3496,7 @@ bool ChainstateManager::AcceptBlockHeader(const CBlockHeader& block, BlockValida if (hash != chainparams.GetConsensus().hashGenesisBlock) { if (miSelf != m_blockman.m_block_index.end()) { // Block header is already known. - CBlockIndex* pindex = miSelf->second; + CBlockIndex* pindex = &(miSelf->second); if (ppindex) *ppindex = pindex; if (pindex->nStatus & BLOCK_FAILED_MASK) { @@ -3522,7 +3518,7 @@ bool ChainstateManager::AcceptBlockHeader(const CBlockHeader& block, BlockValida LogPrint(BCLog::VALIDATION, "%s: %s prev block not found\n", __func__, hash.ToString()); return state.Invalid(BlockValidationResult::BLOCK_MISSING_PREV, "prev-blk-not-found"); } - pindexPrev = (*mi).second; + pindexPrev = &((*mi).second); if (pindexPrev->nStatus & BLOCK_FAILED_MASK) { LogPrint(BCLog::VALIDATION, "%s: %s prev block invalid\n", __func__, hash.ToString()); return state.Invalid(BlockValidationResult::BLOCK_INVALID_PREV, "bad-prevblk"); @@ -3994,13 +3990,13 @@ bool CChainState::ReplayBlocks() if (m_blockman.m_block_index.count(hashHeads[0]) == 0) { return error("ReplayBlocks(): reorganization to unknown block requested"); } - pindexNew = m_blockman.m_block_index[hashHeads[0]]; + pindexNew = &(m_blockman.m_block_index[hashHeads[0]]); if (!hashHeads[1].IsNull()) { // The old tip is allowed to be 0, indicating it's the first flush. if (m_blockman.m_block_index.count(hashHeads[1]) == 0) { return error("ReplayBlocks(): reorganization from unknown block requested"); } - pindexOld = m_blockman.m_block_index[hashHeads[1]]; + pindexOld = &(m_blockman.m_block_index[hashHeads[1]]); pindexFork = LastCommonAncestor(pindexOld, pindexNew); assert(pindexFork != nullptr); } @@ -4070,7 +4066,7 @@ void CChainState::UnloadBlockIndex() // block index state void UnloadBlockIndex(CTxMemPool* mempool, ChainstateManager& chainman) { - LOCK(cs_main); + AssertLockHeld(::cs_main); chainman.Unload(); pindexBestHeader = nullptr; if (mempool) mempool->clear(); @@ -4267,8 +4263,8 @@ void CChainState::CheckBlockIndex() // Build forward-pointing map of the entire block tree. std::multimap<CBlockIndex*,CBlockIndex*> forward; - for (const std::pair<const uint256, CBlockIndex*>& entry : m_blockman.m_block_index) { - forward.insert(std::make_pair(entry.second->pprev, entry.second)); + for (auto& [_, block_index] : m_blockman.m_block_index) { + forward.emplace(block_index.pprev, &block_index); } assert(forward.size() == m_blockman.m_block_index.size()); @@ -5056,15 +5052,6 @@ void ChainstateManager::Unload() m_best_invalid = nullptr; } -void ChainstateManager::Reset() -{ - LOCK(::cs_main); - m_ibd_chainstate.reset(); - m_snapshot_chainstate.reset(); - m_active_chainstate = nullptr; - m_snapshot_validated = false; -} - void ChainstateManager::MaybeRebalanceCaches() { AssertLockHeld(::cs_main); diff --git a/src/validation.h b/src/validation.h index 7766d77a88..cc2247239f 100644 --- a/src/validation.h +++ b/src/validation.h @@ -138,7 +138,7 @@ extern CBlockIndex *pindexBestHeader; extern const std::vector<std::string> CHECKLEVEL_DOC; /** Unload database information */ -void UnloadBlockIndex(CTxMemPool* mempool, ChainstateManager& chainman); +void UnloadBlockIndex(CTxMemPool* mempool, ChainstateManager& chainman) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); /** Run instances of script checking worker threads */ void StartScriptCheckWorkerThreads(int threads_num); /** Stop all of the script checking worker threads */ @@ -991,17 +991,13 @@ public: //! Unload block index and chain data before shutdown. void Unload() EXCLUSIVE_LOCKS_REQUIRED(::cs_main); - //! Clear (deconstruct) chainstate data. - void Reset(); - //! Check to see if caches are out of balance and if so, call //! ResizeCoinsCaches() as needed. void MaybeRebalanceCaches() EXCLUSIVE_LOCKS_REQUIRED(::cs_main); ~ChainstateManager() { LOCK(::cs_main); - UnloadBlockIndex(/* mempool */ nullptr, *this); - Reset(); + UnloadBlockIndex(/*mempool=*/nullptr, *this); } }; diff --git a/src/wallet/rpc/transactions.cpp b/src/wallet/rpc/transactions.cpp index eef2c13ee1..ad94ce4b32 100644 --- a/src/wallet/rpc/transactions.cpp +++ b/src/wallet/rpc/transactions.cpp @@ -34,6 +34,7 @@ static void WalletTxToJSON(const CWallet& wallet, const CWalletTx& wtx, UniValue } uint256 hash = wtx.GetHash(); entry.pushKV("txid", hash.GetHex()); + entry.pushKV("wtxid", wtx.GetWitnessHash().GetHex()); UniValue conflicts(UniValue::VARR); for (const uint256& conflict : wallet.GetTxConflicts(wtx)) conflicts.push_back(conflict.GetHex()); @@ -431,6 +432,7 @@ static const std::vector<RPCResult> TransactionDescriptionString() {RPCResult::Type::NUM, "blockindex", /*optional=*/true, "The index of the transaction in the block that includes it."}, {RPCResult::Type::NUM_TIME, "blocktime", /*optional=*/true, "The block time expressed in " + UNIX_EPOCH_TIME + "."}, {RPCResult::Type::STR_HEX, "txid", "The transaction id."}, + {RPCResult::Type::STR_HEX, "wtxid", "The hash of serialized transaction, including witness data."}, {RPCResult::Type::ARR, "walletconflicts", "Conflicting transaction ids.", { {RPCResult::Type::STR_HEX, "txid", "The transaction id."}, diff --git a/src/wallet/test/wallet_tests.cpp b/src/wallet/test/wallet_tests.cpp index 7693c9c0e8..c59f7e6f05 100644 --- a/src/wallet/test/wallet_tests.cpp +++ b/src/wallet/test/wallet_tests.cpp @@ -367,10 +367,10 @@ static int64_t AddTx(ChainstateManager& chainman, CWallet& wallet, uint32_t lock CBlockIndex* block = nullptr; if (blockTime > 0) { LOCK(cs_main); - auto inserted = chainman.BlockIndex().emplace(GetRandHash(), new CBlockIndex); + auto inserted = chainman.BlockIndex().emplace(std::piecewise_construct, std::make_tuple(GetRandHash()), std::make_tuple()); assert(inserted.second); const uint256& hash = inserted.first->first; - block = inserted.first->second; + block = &inserted.first->second; block->nTime = blockTime; block->phashBlock = &hash; state = TxStateConfirmed{hash, block->nHeight, /*position_in_block=*/0}; diff --git a/src/wallet/transaction.h b/src/wallet/transaction.h index 00f9c9f154..271d698e56 100644 --- a/src/wallet/transaction.h +++ b/src/wallet/transaction.h @@ -296,6 +296,7 @@ public: bool isUnconfirmed() const { return !isAbandoned() && !isConflicted() && !isConfirmed(); } bool isConfirmed() const { return state<TxStateConfirmed>(); } const uint256& GetHash() const { return tx->GetHash(); } + const uint256& GetWitnessHash() const { return tx->GetWitnessHash(); } bool IsCoinBase() const { return tx->IsCoinBase(); } // Disable copying of CWalletTx objects to prevent bugs where instances get diff --git a/test/functional/feature_addrman.py b/test/functional/feature_addrman.py index 0fdefaa9c3..5e49d0214a 100755 --- a/test/functional/feature_addrman.py +++ b/test/functional/feature_addrman.py @@ -68,6 +68,17 @@ class AddrmanTest(BitcoinTestFramework): self.start_node(0, extra_args=["-checkaddrman=1"]) assert_equal(self.nodes[0].getnodeaddresses(), []) + self.log.info("Check that addrman with negative lowest_compatible cannot be read") + self.stop_node(0) + write_addrman(peers_dat, lowest_compatible=-32) + self.nodes[0].assert_start_raises_init_error( + expected_msg=init_error( + "Corrupted addrman database: The compat value \\(0\\) is lower " + "than the expected minimum value 32.: (.+)" + ), + match=ErrorMatch.FULL_REGEX, + ) + self.log.info("Check that addrman from future is overwritten with new addrman") self.stop_node(0) write_addrman(peers_dat, lowest_compatible=111) diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py index 69f9df57d8..a7873838be 100755 --- a/test/functional/wallet_basic.py +++ b/test/functional/wallet_basic.py @@ -668,7 +668,7 @@ class WalletTest(BitcoinTestFramework): "category": baz["category"], "vout": baz["vout"]} expected_fields = frozenset({'amount', 'bip125-replaceable', 'confirmations', 'details', 'fee', - 'hex', 'time', 'timereceived', 'trusted', 'txid', 'walletconflicts'}) + 'hex', 'time', 'timereceived', 'trusted', 'txid', 'wtxid', 'walletconflicts'}) verbose_field = "decoded" expected_verbose_fields = expected_fields | {verbose_field} diff --git a/test/functional/wallet_taproot.py b/test/functional/wallet_taproot.py index f72e2026a3..d3731b135a 100755 --- a/test/functional/wallet_taproot.py +++ b/test/functional/wallet_taproot.py @@ -208,19 +208,6 @@ class WalletTaprootTest(BitcoinTestFramework): pass @staticmethod - def rand_keys(n): - ret = [] - idxes = set() - for _ in range(n): - while True: - i = random.randrange(len(KEYS)) - if not i in idxes: - break - idxes.add(i) - ret.append(KEYS[i]) - return ret - - @staticmethod def make_desc(pattern, privmap, keys, pub_only = False): pat = pattern.replace("$H", H_POINT) for i in range(len(privmap)): @@ -332,7 +319,7 @@ class WalletTaprootTest(BitcoinTestFramework): def do_test(self, comment, pattern, privmap, treefn): nkeys = len(privmap) - keys = self.rand_keys(nkeys * 4) + keys = random.sample(KEYS, nkeys * 4) self.do_test_addr(comment, pattern, privmap, treefn, keys[0:nkeys]) self.do_test_sendtoaddress(comment, pattern, privmap, treefn, keys[0:nkeys], keys[nkeys:2*nkeys]) self.do_test_psbt(comment, pattern, privmap, treefn, keys[2*nkeys:3*nkeys], keys[3*nkeys:4*nkeys]) diff --git a/test/sanitizer_suppressions/ubsan b/test/sanitizer_suppressions/ubsan index b06dd253be..e6cfe5f81a 100644 --- a/test/sanitizer_suppressions/ubsan +++ b/test/sanitizer_suppressions/ubsan @@ -62,7 +62,6 @@ implicit-integer-sign-change:script/bitcoinconsensus.cpp implicit-integer-sign-change:script/interpreter.cpp implicit-integer-sign-change:serialize.h implicit-integer-sign-change:txmempool.cpp -implicit-signed-integer-truncation:addrman.cpp implicit-signed-integer-truncation:crypto/ implicit-unsigned-integer-truncation:crypto/ shift-base:arith_uint256.cpp |