diff options
35 files changed, 702 insertions, 294 deletions
diff --git a/.gitignore b/.gitignore index 23b6090265..3e5d284aa3 100644 --- a/.gitignore +++ b/.gitignore @@ -9,7 +9,7 @@ src/bitcoin-gui src/bitcoin-node src/bitcoin-tx src/bitcoin-wallet -src/test/fuzz +src/test/fuzz/* !src/test/fuzz/*.* src/test/test_bitcoin src/qt/test/test_bitcoin-qt diff --git a/.travis.yml b/.travis.yml index 2d69ad7d23..f9932cfaca 100644 --- a/.travis.yml +++ b/.travis.yml @@ -72,14 +72,12 @@ jobs: FILE_ENV="./ci/test/00_setup_env_arm.sh" QEMU_USER_CMD="" -# s390 build was disabled temporarily because of disk space issues on the Travis VM -# -# - stage: test -# name: 'S390x [GOAL: install] [buster] [unit tests, functional tests]' -# arch: s390x # Can disable QEMU_USER_CMD and run the tests natively without qemu -# env: >- -# FILE_ENV="./ci/test/00_setup_env_s390x.sh" -# QEMU_USER_CMD="" + - stage: test + name: 'S390x [GOAL: install] [buster] [unit tests, functional tests]' + arch: s390x # Can disable QEMU_USER_CMD and run the tests natively without qemu + env: >- + FILE_ENV="./ci/test/00_setup_env_s390x.sh" + QEMU_USER_CMD="" - stage: test name: 'Win64 [GOAL: deploy] [unit tests, no gui, no functional tests]' diff --git a/ci/test/00_setup_env_native_fuzz.sh b/ci/test/00_setup_env_native_fuzz.sh index 43ee219ef9..31f437f0e8 100644 --- a/ci/test/00_setup_env_native_fuzz.sh +++ b/ci/test/00_setup_env_native_fuzz.sh @@ -15,3 +15,4 @@ export RUN_FUNCTIONAL_TESTS=false export RUN_FUZZ_TESTS=true export GOAL="install" export BITCOIN_CONFIG="--enable-fuzz --with-sanitizers=fuzzer,address,undefined CC=clang CXX=clang++" +export CCACHE_SIZE=200M diff --git a/ci/test/00_setup_env_native_fuzz_with_valgrind.sh b/ci/test/00_setup_env_native_fuzz_with_valgrind.sh index c27d525003..e06a40eb23 100644 --- a/ci/test/00_setup_env_native_fuzz_with_valgrind.sh +++ b/ci/test/00_setup_env_native_fuzz_with_valgrind.sh @@ -16,3 +16,4 @@ export RUN_FUZZ_TESTS=true export FUZZ_TESTS_CONFIG="--valgrind" export GOAL="install" export BITCOIN_CONFIG="--enable-fuzz --with-sanitizers=fuzzer CC=clang CXX=clang++" +export CCACHE_SIZE=200M diff --git a/ci/test/00_setup_env_native_msan.sh b/ci/test/00_setup_env_native_msan.sh index cc583edf17..6a4979990b 100644 --- a/ci/test/00_setup_env_native_msan.sh +++ b/ci/test/00_setup_env_native_msan.sh @@ -20,3 +20,4 @@ export GOAL="install" export BITCOIN_CONFIG="--enable-wallet --with-sanitizers=memory --with-asm=no --prefix=${BASE_ROOT_DIR}/depends/x86_64-pc-linux-gnu/ CC=clang CXX=clang++ CFLAGS='${MSAN_FLAGS}' CXXFLAGS='${MSAN_AND_LIBCXX_FLAGS}' BDB_LIBS='-L${BDB_PREFIX}/lib -ldb_cxx-4.8' BDB_CFLAGS='-I${BDB_PREFIX}/include'" export USE_MEMORY_SANITIZER="true" export RUN_FUNCTIONAL_TESTS="false" +export CCACHE_SIZE=250M diff --git a/configure.ac b/configure.ac index c6b8aafef9..f11f2b2059 100644 --- a/configure.ac +++ b/configure.ac @@ -785,6 +785,7 @@ if test x$use_hardening != xno; then AX_CHECK_LINK_FLAG([[-Wl,--high-entropy-va]], [HARDENED_LDFLAGS="$HARDENED_LDFLAGS -Wl,--high-entropy-va"],, [[$LDFLAG_WERROR]]) AX_CHECK_LINK_FLAG([[-Wl,-z,relro]], [HARDENED_LDFLAGS="$HARDENED_LDFLAGS -Wl,-z,relro"],, [[$LDFLAG_WERROR]]) AX_CHECK_LINK_FLAG([[-Wl,-z,now]], [HARDENED_LDFLAGS="$HARDENED_LDFLAGS -Wl,-z,now"],, [[$LDFLAG_WERROR]]) + AX_CHECK_LINK_FLAG([[-Wl,-z,separate-code]], [HARDENED_LDFLAGS="$HARDENED_LDFLAGS -Wl,-z,separate-code"],, [[$LDFLAG_WERROR]]) AX_CHECK_LINK_FLAG([[-fPIE -pie]], [PIE_FLAGS="-fPIE"; HARDENED_LDFLAGS="$HARDENED_LDFLAGS -pie"],, [[$CXXFLAG_WERROR]]) case $host in diff --git a/contrib/devtools/security-check.py b/contrib/devtools/security-check.py index ca587ca9e5..dc74de9198 100755 --- a/contrib/devtools/security-check.py +++ b/contrib/devtools/security-check.py @@ -40,25 +40,48 @@ def get_ELF_program_headers(executable): stdout = run_command([READELF_CMD, '-l', '-W', executable]) in_headers = False - count = 0 headers = [] for line in stdout.splitlines(): if line.startswith('Program Headers:'): in_headers = True + count = 0 if line == '': in_headers = False if in_headers: if count == 1: # header line - ofs_typ = line.find('Type') - ofs_offset = line.find('Offset') - ofs_flags = line.find('Flg') - ofs_align = line.find('Align') - if ofs_typ == -1 or ofs_offset == -1 or ofs_flags == -1 or ofs_align == -1: + header = [x.strip() for x in line.split()] + ofs_typ = header.index('Type') + ofs_flags = header.index('Flg') + # assert readelf output is what we expect + if ofs_typ == -1 or ofs_flags == -1: raise ValueError('Cannot parse elfread -lW output') elif count > 1: - typ = line[ofs_typ:ofs_offset].rstrip() - flags = line[ofs_flags:ofs_align].rstrip() - headers.append((typ, flags)) + splitline = [x.strip() for x in line.split()] + typ = splitline[ofs_typ] + if not typ.startswith('[R'): # skip [Requesting ...] + splitline = [x.strip() for x in line.split()] + flags = splitline[ofs_flags] + # check for 'R', ' E' + if splitline[ofs_flags + 1] is 'E': + flags += ' E' + headers.append((typ, flags, [])) + count += 1 + + if line.startswith(' Section to Segment mapping:'): + in_mapping = True + count = 0 + if line == '': + in_mapping = False + if in_mapping: + if count == 1: # header line + ofs_segment = line.find('Segment') + ofs_sections = line.find('Sections...') + if ofs_segment == -1 or ofs_sections == -1: + raise ValueError('Cannot parse elfread -lW output') + elif count > 1: + segment = int(line[ofs_segment:ofs_sections].strip()) + sections = line[ofs_sections:].strip().split() + headers[segment][2].extend(sections) count += 1 return headers @@ -68,7 +91,7 @@ def check_ELF_NX(executable) -> bool: ''' have_wx = False have_gnu_stack = False - for (typ, flags) in get_ELF_program_headers(executable): + for (typ, flags, _) in get_ELF_program_headers(executable): if typ == 'GNU_STACK': have_gnu_stack = True if 'W' in flags and 'E' in flags: # section is both writable and executable @@ -82,7 +105,7 @@ def check_ELF_RELRO(executable) -> bool: Dynamic section must have BIND_NOW flag ''' have_gnu_relro = False - for (typ, flags) in get_ELF_program_headers(executable): + for (typ, flags, _) in get_ELF_program_headers(executable): # Note: not checking flags == 'R': here as linkers set the permission differently # This does not affect security: the permission flags of the GNU_RELRO program # header are ignored, the PT_LOAD header determines the effective permissions. @@ -113,6 +136,62 @@ def check_ELF_Canary(executable) -> bool: ok = True return ok +def check_ELF_separate_code(executable): + ''' + Check that sections are appropriately separated in virtual memory, + based on their permissions. This checks for missing -Wl,-z,separate-code + and potentially other problems. + ''' + EXPECTED_FLAGS = { + # Read + execute + '.init': 'R E', + '.plt': 'R E', + '.plt.got': 'R E', + '.plt.sec': 'R E', + '.text': 'R E', + '.fini': 'R E', + # Read-only data + '.interp': 'R', + '.note.gnu.property': 'R', + '.note.gnu.build-id': 'R', + '.note.ABI-tag': 'R', + '.gnu.hash': 'R', + '.dynsym': 'R', + '.dynstr': 'R', + '.gnu.version': 'R', + '.gnu.version_r': 'R', + '.rela.dyn': 'R', + '.rela.plt': 'R', + '.rodata': 'R', + '.eh_frame_hdr': 'R', + '.eh_frame': 'R', + '.qtmetadata': 'R', + '.gcc_except_table': 'R', + '.stapsdt.base': 'R', + # Writable data + '.init_array': 'RW', + '.fini_array': 'RW', + '.dynamic': 'RW', + '.got': 'RW', + '.data': 'RW', + '.bss': 'RW', + } + # For all LOAD program headers get mapping to the list of sections, + # and for each section, remember the flags of the associated program header. + flags_per_section = {} + for (typ, flags, sections) in get_ELF_program_headers(executable): + if typ == 'LOAD': + for section in sections: + assert(section not in flags_per_section) + flags_per_section[section] = flags + # Spot-check ELF LOAD program header flags per section + # If these sections exist, check them against the expected R/W/E flags + for (section, flags) in flags_per_section.items(): + if section in EXPECTED_FLAGS: + if EXPECTED_FLAGS[section] != flags: + return False + return True + def get_PE_dll_characteristics(executable) -> int: '''Get PE DllCharacteristics bits''' stdout = run_command([OBJDUMP_CMD, '-x', executable]) @@ -225,7 +304,8 @@ CHECKS = { ('PIE', check_ELF_PIE), ('NX', check_ELF_NX), ('RELRO', check_ELF_RELRO), - ('Canary', check_ELF_Canary) + ('Canary', check_ELF_Canary), + ('separate_code', check_ELF_separate_code), ], 'PE': [ ('DYNAMIC_BASE', check_PE_DYNAMIC_BASE), diff --git a/contrib/devtools/test-security-check.py b/contrib/devtools/test-security-check.py index 629eba4f28..ec2d886653 100755 --- a/contrib/devtools/test-security-check.py +++ b/contrib/devtools/test-security-check.py @@ -31,15 +31,17 @@ class TestSecurityChecks(unittest.TestCase): cc = 'gcc' write_testcode(source) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-zexecstack','-fno-stack-protector','-Wl,-znorelro','-no-pie','-fno-PIE']), + self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-zexecstack','-fno-stack-protector','-Wl,-znorelro','-no-pie','-fno-PIE', '-Wl,-z,separate-code']), (1, executable+': failed PIE NX RELRO Canary')) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-fno-stack-protector','-Wl,-znorelro','-no-pie','-fno-PIE']), + self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-fno-stack-protector','-Wl,-znorelro','-no-pie','-fno-PIE', '-Wl,-z,separate-code']), (1, executable+': failed PIE RELRO Canary')) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-fstack-protector-all','-Wl,-znorelro','-no-pie','-fno-PIE']), + self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-fstack-protector-all','-Wl,-znorelro','-no-pie','-fno-PIE', '-Wl,-z,separate-code']), (1, executable+': failed PIE RELRO')) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-fstack-protector-all','-Wl,-znorelro','-pie','-fPIE']), + self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-fstack-protector-all','-Wl,-znorelro','-pie','-fPIE', '-Wl,-z,separate-code']), (1, executable+': failed RELRO')) - self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-fstack-protector-all','-Wl,-zrelro','-Wl,-z,now','-pie','-fPIE']), + self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-fstack-protector-all','-Wl,-zrelro','-Wl,-z,now','-pie','-fPIE', '-Wl,-z,noseparate-code']), + (1, executable+': failed separate_code')) + self.assertEqual(call_security_check(cc, source, executable, ['-Wl,-znoexecstack','-fstack-protector-all','-Wl,-zrelro','-Wl,-z,now','-pie','-fPIE', '-Wl,-z,separate-code']), (0, '')) def test_PE(self): diff --git a/depends/packages/native_cctools.mk b/depends/packages/native_cctools.mk index 072d3828a6..5022ed980f 100644 --- a/depends/packages/native_cctools.mk +++ b/depends/packages/native_cctools.mk @@ -1,8 +1,8 @@ package=native_cctools -$(package)_version=4da2f3b485bcf4cef526f30c0b8c0bcda99cdbb4 +$(package)_version=55562e4073dea0fbfd0b20e0bf69ffe6390c7f97 $(package)_download_path=https://github.com/tpoechtrager/cctools-port/archive $(package)_file_name=$($(package)_version).tar.gz -$(package)_sha256_hash=a2d491c0981cef72fee2b833598f20f42a6c44a7614a61c439bda93d56446fec +$(package)_sha256_hash=e51995a843533a3dac155dd0c71362dd471597a2d23f13dff194c6285362f875 $(package)_build_subdir=cctools ifeq ($(strip $(FORCE_USE_SYSTEM_CLANG)),) $(package)_clang_version=8.0.0 @@ -63,9 +63,10 @@ endef endif define $(package)_set_vars - $(package)_config_opts=--target=$(host) --disable-lto-support --with-libtapi=$($(package)_extract_dir) + $(package)_config_opts=--target=$(host) --with-libtapi=$($(package)_extract_dir) $(package)_ldflags+=-Wl,-rpath=\\$$$$$$$$\$$$$$$$$ORIGIN/../lib ifeq ($(strip $(FORCE_USE_SYSTEM_CLANG)),) + $(package)_config_opts+=--enable-lto-support --with-llvm-config=$($(package)_extract_dir)/toolchain/bin/llvm-config $(package)_cc=$($(package)_extract_dir)/toolchain/bin/clang $(package)_cxx=$($(package)_extract_dir)/toolchain/bin/clang++ else diff --git a/src/Makefile.test.include b/src/Makefile.test.include index 637d1d2f6e..f20e51a8a5 100644 --- a/src/Makefile.test.include +++ b/src/Makefile.test.include @@ -275,6 +275,7 @@ BITCOIN_TESTS =\ test/uint256_tests.cpp \ test/util_tests.cpp \ test/validation_block_tests.cpp \ + test/validation_chainstate_tests.cpp \ test/validation_chainstatemanager_tests.cpp \ test/validation_flush_tests.cpp \ test/validationinterface_tests.cpp \ diff --git a/src/coins.cpp b/src/coins.cpp index 7b76c13f98..5de2ed7810 100644 --- a/src/coins.cpp +++ b/src/coins.cpp @@ -245,6 +245,14 @@ bool CCoinsViewCache::HaveInputs(const CTransaction& tx) const return true; } +void CCoinsViewCache::ReallocateCache() +{ + // Cache should be empty when we're calling this. + assert(cacheCoins.size() == 0); + cacheCoins.~CCoinsMap(); + ::new (&cacheCoins) CCoinsMap(); +} + static const size_t MIN_TRANSACTION_OUTPUT_WEIGHT = WITNESS_SCALE_FACTOR * ::GetSerializeSize(CTxOut(), PROTOCOL_VERSION); static const size_t MAX_OUTPUTS_PER_BLOCK = MAX_BLOCK_WEIGHT / MIN_TRANSACTION_OUTPUT_WEIGHT; diff --git a/src/coins.h b/src/coins.h index a3f34bb0ee..a3e241ac90 100644 --- a/src/coins.h +++ b/src/coins.h @@ -318,6 +318,13 @@ public: //! Check whether all prevouts of the transaction are present in the UTXO set represented by this view bool HaveInputs(const CTransaction& tx) const; + //! Force a reallocation of the cache map. This is required when downsizing + //! the cache because the map's allocator may be hanging onto a lot of + //! memory despite having called .clear(). + //! + //! See: https://stackoverflow.com/questions/42114044/how-to-release-unordered-map-memory + void ReallocateCache(); + private: /** * @note this is marked const, but may actually append to `cacheCoins`, increasing diff --git a/src/init.cpp b/src/init.cpp index acf9f8bd91..1c89e2eeff 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -1534,7 +1534,7 @@ bool AppInitMain(const util::Ref& context, NodeContext& node) int64_t nCoinDBCache = std::min(nTotalCache / 2, (nTotalCache / 4) + (1 << 23)); // use 25%-50% of the remainder for disk cache nCoinDBCache = std::min(nCoinDBCache, nMaxCoinsDBCache << 20); // cap total coins db cache nTotalCache -= nCoinDBCache; - nCoinCacheUsage = nTotalCache; // the rest goes to in-memory cache + int64_t nCoinCacheUsage = nTotalCache; // the rest goes to in-memory cache int64_t nMempoolSizeMax = gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000; LogPrintf("Cache configuration:\n"); LogPrintf("* Using %.1f MiB for block index database\n", nBlockTreeDBCache * (1.0 / 1024 / 1024)); @@ -1563,6 +1563,9 @@ bool AppInitMain(const util::Ref& context, NodeContext& node) try { LOCK(cs_main); chainman.InitializeChainstate(); + chainman.m_total_coinstip_cache = nCoinCacheUsage; + chainman.m_total_coinsdb_cache = nCoinDBCache; + UnloadBlockIndex(); // new CBlockTreeDB tries to delete the existing file, which @@ -1646,7 +1649,7 @@ bool AppInitMain(const util::Ref& context, NodeContext& node) } // The on-disk coinsdb is now in a good state, create the cache - chainstate->InitCoinsCache(); + chainstate->InitCoinsCache(nCoinCacheUsage); assert(chainstate->CanFlushToDisk()); if (!is_coinsview_empty(chainstate)) { diff --git a/src/net_processing.cpp b/src/net_processing.cpp index f63abca847..ca701a7e5b 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -157,9 +157,6 @@ std::map<uint256, std::map<uint256, COrphanTx>::iterator> g_orphans_by_wtxid GUA void EraseOrphansFor(NodeId peer); -/** Increase a node's misbehavior score. */ -void Misbehaving(NodeId nodeid, int howmuch, const std::string& message="") EXCLUSIVE_LOCKS_REQUIRED(cs_main); - // Internal stuff namespace { /** Number of nodes with fSyncStarted. */ @@ -1062,23 +1059,22 @@ unsigned int LimitOrphanTxSize(unsigned int nMaxOrphans) * Increment peer's misbehavior score. If the new value >= DISCOURAGEMENT_THRESHOLD, mark the node * to be discouraged, meaning the peer might be disconnected and added to the discouragement filter. */ -void Misbehaving(NodeId pnode, int howmuch, const std::string& message) EXCLUSIVE_LOCKS_REQUIRED(cs_main) +void Misbehaving(const NodeId pnode, const int howmuch, const std::string& message) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { - if (howmuch == 0) - return; + assert(howmuch > 0); - CNodeState *state = State(pnode); - if (state == nullptr) - return; + CNodeState* const state = State(pnode); + if (state == nullptr) return; state->nMisbehavior += howmuch; - std::string message_prefixed = message.empty() ? "" : (": " + message); + const std::string message_prefixed = message.empty() ? "" : (": " + message); if (state->nMisbehavior >= DISCOURAGEMENT_THRESHOLD && state->nMisbehavior - howmuch < DISCOURAGEMENT_THRESHOLD) { - LogPrint(BCLog::NET, "%s: %s peer=%d (%d -> %d) DISCOURAGE THRESHOLD EXCEEDED%s\n", __func__, state->name, pnode, state->nMisbehavior-howmuch, state->nMisbehavior, message_prefixed); + LogPrint(BCLog::NET, "Misbehaving: peer=%d (%d -> %d) DISCOURAGE THRESHOLD EXCEEDED%s\n", pnode, state->nMisbehavior - howmuch, state->nMisbehavior, message_prefixed); state->m_should_discourage = true; - } else - LogPrint(BCLog::NET, "%s: %s peer=%d (%d -> %d)%s\n", __func__, state->name, pnode, state->nMisbehavior-howmuch, state->nMisbehavior, message_prefixed); + } else { + LogPrint(BCLog::NET, "Misbehaving: peer=%d (%d -> %d)%s\n", pnode, state->nMisbehavior - howmuch, state->nMisbehavior, message_prefixed); + } } /** @@ -1799,7 +1795,7 @@ inline void static SendBlockTransactions(const CBlock& block, const BlockTransac for (size_t i = 0; i < req.indexes.size(); i++) { if (req.indexes[i] >= block.vtx.size()) { LOCK(cs_main); - Misbehaving(pfrom.GetId(), 100, strprintf("Peer %d sent us a getblocktxn with out-of-bounds tx indices", pfrom.GetId())); + Misbehaving(pfrom.GetId(), 100, "getblocktxn with out-of-bounds tx indices"); return; } resp.txn[i] = block.vtx[req.indexes[i]]; @@ -1848,7 +1844,7 @@ static void ProcessHeadersMessage(CNode& pfrom, CConnman& connman, ChainstateMan UpdateBlockAvailability(pfrom.GetId(), headers.back().GetHash()); if (nodestate->nUnconnectingHeaders % MAX_UNCONNECTING_HEADERS == 0) { - Misbehaving(pfrom.GetId(), 20); + Misbehaving(pfrom.GetId(), 20, strprintf("%d non-connecting headers", nodestate->nUnconnectingHeaders)); } return; } @@ -2307,7 +2303,7 @@ void ProcessMessage( if (pfrom.nVersion != 0) { LOCK(cs_main); - Misbehaving(pfrom.GetId(), 1); + Misbehaving(pfrom.GetId(), 1, "redundant version message"); return; } @@ -2468,7 +2464,7 @@ void ProcessMessage( if (pfrom.nVersion == 0) { // Must have a version message before anything else LOCK(cs_main); - Misbehaving(pfrom.GetId(), 1); + Misbehaving(pfrom.GetId(), 1, "non-version message before version handshake"); return; } @@ -2535,7 +2531,7 @@ void ProcessMessage( if (!pfrom.fSuccessfullyConnected) { // Must have a verack message before anything else LOCK(cs_main); - Misbehaving(pfrom.GetId(), 1); + Misbehaving(pfrom.GetId(), 1, "non-verack message before version handshake"); return; } @@ -3203,7 +3199,7 @@ void ProcessMessage( ReadStatus status = partialBlock.InitData(cmpctblock, vExtraTxnForCompact); if (status == READ_STATUS_INVALID) { MarkBlockAsReceived(pindex->GetBlockHash()); // Reset in-flight state in case Misbehaving does not result in a disconnect - Misbehaving(pfrom.GetId(), 100, strprintf("Peer %d sent us invalid compact block\n", pfrom.GetId())); + Misbehaving(pfrom.GetId(), 100, "invalid compact block"); return; } else if (status == READ_STATUS_FAILED) { // Duplicate txindexes, the block is now in-flight, so just request it @@ -3336,7 +3332,7 @@ void ProcessMessage( ReadStatus status = partialBlock.FillBlock(*pblock, resp.txn); if (status == READ_STATUS_INVALID) { MarkBlockAsReceived(resp.blockhash); // Reset in-flight state in case Misbehaving does not result in a disconnect - Misbehaving(pfrom.GetId(), 100, strprintf("Peer %d sent us invalid compact block/non-matching block transactions\n", pfrom.GetId())); + Misbehaving(pfrom.GetId(), 100, "invalid compact block/non-matching block transactions"); return; } else if (status == READ_STATUS_FAILED) { // Might have collided, fall back to getdata now :( @@ -3605,7 +3601,7 @@ void ProcessMessage( { // There is no excuse for sending a too-large filter LOCK(cs_main); - Misbehaving(pfrom.GetId(), 100); + Misbehaving(pfrom.GetId(), 100, "too-large bloom filter"); } else if (pfrom.m_tx_relay != nullptr) { @@ -3639,7 +3635,7 @@ void ProcessMessage( } if (bad) { LOCK(cs_main); - Misbehaving(pfrom.GetId(), 100); + Misbehaving(pfrom.GetId(), 100, "bad filteradd message"); } return; } @@ -3723,32 +3719,32 @@ void ProcessMessage( */ bool PeerLogicValidation::MaybeDiscourageAndDisconnect(CNode& pnode) { - NodeId peer_id{pnode.GetId()}; + const NodeId peer_id{pnode.GetId()}; { LOCK(cs_main); - CNodeState &state = *State(peer_id); + CNodeState& state = *State(peer_id); // There's nothing to do if the m_should_discourage flag isn't set if (!state.m_should_discourage) return false; - // Reset m_should_discourage state.m_should_discourage = false; } // cs_main if (pnode.HasPermission(PF_NOBAN)) { - // Peer has the NOBAN permission flag - log but don't disconnect + // We never disconnect or discourage peers for bad behavior if they have the NOBAN permission flag LogPrintf("Warning: not punishing noban peer %d!\n", peer_id); return false; } if (pnode.m_manual_connection) { - // Peer is a manual connection - log but don't disconnect + // We never disconnect or discourage manual peers for bad behavior LogPrintf("Warning: not punishing manually connected peer %d!\n", peer_id); return false; } if (pnode.addr.IsLocal()) { - // Peer is on a local address. Disconnect this peer, but don't discourage the local address + // We disconnect local peers for bad behavior but don't discourage (since that would discourage + // all peers on the same local address) LogPrintf("Warning: disconnecting but not discouraging local peer %d!\n", peer_id); pnode.fDisconnect = true; return true; diff --git a/src/netaddress.cpp b/src/netaddress.cpp index 0aaba440b8..0874b8dcea 100644 --- a/src/netaddress.cpp +++ b/src/netaddress.cpp @@ -28,19 +28,35 @@ CNetAddr::CNetAddr() void CNetAddr::SetIP(const CNetAddr& ipIn) { + m_net = ipIn.m_net; memcpy(ip, ipIn.ip, sizeof(ip)); } +void CNetAddr::SetLegacyIPv6(const uint8_t ipv6[16]) +{ + if (memcmp(ipv6, pchIPv4, sizeof(pchIPv4)) == 0) { + m_net = NET_IPV4; + } else if (memcmp(ipv6, pchOnionCat, sizeof(pchOnionCat)) == 0) { + m_net = NET_ONION; + } else if (memcmp(ipv6, g_internal_prefix, sizeof(g_internal_prefix)) == 0) { + m_net = NET_INTERNAL; + } else { + m_net = NET_IPV6; + } + memcpy(ip, ipv6, 16); +} + void CNetAddr::SetRaw(Network network, const uint8_t *ip_in) { switch(network) { case NET_IPV4: + m_net = NET_IPV4; memcpy(ip, pchIPv4, 12); memcpy(ip+12, ip_in, 4); break; case NET_IPV6: - memcpy(ip, ip_in, 16); + SetLegacyIPv6(ip_in); break; default: assert(!"invalid network"); @@ -66,6 +82,7 @@ bool CNetAddr::SetInternal(const std::string &name) if (name.empty()) { return false; } + m_net = NET_INTERNAL; unsigned char hash[32] = {}; CSHA256().Write((const unsigned char*)name.data(), name.size()).Finalize(hash); memcpy(ip, g_internal_prefix, sizeof(g_internal_prefix)); @@ -89,6 +106,7 @@ bool CNetAddr::SetSpecial(const std::string &strName) std::vector<unsigned char> vchAddr = DecodeBase32(strName.substr(0, strName.size() - 6).c_str()); if (vchAddr.size() != 16-sizeof(pchOnionCat)) return false; + m_net = NET_ONION; memcpy(ip, pchOnionCat, sizeof(pchOnionCat)); for (unsigned int i=0; i<16-sizeof(pchOnionCat); i++) ip[i + sizeof(pchOnionCat)] = vchAddr[i]; @@ -123,15 +141,9 @@ bool CNetAddr::IsBindAny() const return true; } -bool CNetAddr::IsIPv4() const -{ - return (memcmp(ip, pchIPv4, sizeof(pchIPv4)) == 0); -} +bool CNetAddr::IsIPv4() const { return m_net == NET_IPV4; } -bool CNetAddr::IsIPv6() const -{ - return (!IsIPv4() && !IsTor() && !IsInternal()); -} +bool CNetAddr::IsIPv6() const { return m_net == NET_IPV6; } bool CNetAddr::IsRFC1918() const { @@ -165,50 +177,54 @@ bool CNetAddr::IsRFC5737() const bool CNetAddr::IsRFC3849() const { - return GetByte(15) == 0x20 && GetByte(14) == 0x01 && GetByte(13) == 0x0D && GetByte(12) == 0xB8; + return IsIPv6() && GetByte(15) == 0x20 && GetByte(14) == 0x01 && + GetByte(13) == 0x0D && GetByte(12) == 0xB8; } bool CNetAddr::IsRFC3964() const { - return (GetByte(15) == 0x20 && GetByte(14) == 0x02); + return IsIPv6() && GetByte(15) == 0x20 && GetByte(14) == 0x02; } bool CNetAddr::IsRFC6052() const { static const unsigned char pchRFC6052[] = {0,0x64,0xFF,0x9B,0,0,0,0,0,0,0,0}; - return (memcmp(ip, pchRFC6052, sizeof(pchRFC6052)) == 0); + return IsIPv6() && memcmp(ip, pchRFC6052, sizeof(pchRFC6052)) == 0; } bool CNetAddr::IsRFC4380() const { - return (GetByte(15) == 0x20 && GetByte(14) == 0x01 && GetByte(13) == 0 && GetByte(12) == 0); + return IsIPv6() && GetByte(15) == 0x20 && GetByte(14) == 0x01 && GetByte(13) == 0 && + GetByte(12) == 0; } bool CNetAddr::IsRFC4862() const { static const unsigned char pchRFC4862[] = {0xFE,0x80,0,0,0,0,0,0}; - return (memcmp(ip, pchRFC4862, sizeof(pchRFC4862)) == 0); + return IsIPv6() && memcmp(ip, pchRFC4862, sizeof(pchRFC4862)) == 0; } bool CNetAddr::IsRFC4193() const { - return ((GetByte(15) & 0xFE) == 0xFC); + return IsIPv6() && (GetByte(15) & 0xFE) == 0xFC; } bool CNetAddr::IsRFC6145() const { static const unsigned char pchRFC6145[] = {0,0,0,0,0,0,0,0,0xFF,0xFF,0,0}; - return (memcmp(ip, pchRFC6145, sizeof(pchRFC6145)) == 0); + return IsIPv6() && memcmp(ip, pchRFC6145, sizeof(pchRFC6145)) == 0; } bool CNetAddr::IsRFC4843() const { - return (GetByte(15) == 0x20 && GetByte(14) == 0x01 && GetByte(13) == 0x00 && (GetByte(12) & 0xF0) == 0x10); + return IsIPv6() && GetByte(15) == 0x20 && GetByte(14) == 0x01 && + GetByte(13) == 0x00 && (GetByte(12) & 0xF0) == 0x10; } bool CNetAddr::IsRFC7343() const { - return (GetByte(15) == 0x20 && GetByte(14) == 0x01 && GetByte(13) == 0x00 && (GetByte(12) & 0xF0) == 0x20); + return IsIPv6() && GetByte(15) == 0x20 && GetByte(14) == 0x01 && + GetByte(13) == 0x00 && (GetByte(12) & 0xF0) == 0x20; } bool CNetAddr::IsHeNet() const @@ -222,10 +238,7 @@ bool CNetAddr::IsHeNet() const * * @see CNetAddr::SetSpecial(const std::string &) */ -bool CNetAddr::IsTor() const -{ - return (memcmp(ip, pchOnionCat, sizeof(pchOnionCat)) == 0); -} +bool CNetAddr::IsTor() const { return m_net == NET_ONION; } bool CNetAddr::IsLocal() const { @@ -235,7 +248,7 @@ bool CNetAddr::IsLocal() const // IPv6 loopback (::1/128) static const unsigned char pchLocal[16] = {0,0,0,0,0,0,0,0,0,0,0,0,0,0,0,1}; - if (memcmp(ip, pchLocal, 16) == 0) + if (IsIPv6() && memcmp(ip, pchLocal, 16) == 0) return true; return false; @@ -259,12 +272,12 @@ bool CNetAddr::IsValid() const // header20 vectorlen3 addr26 addr26 addr26 header20 vectorlen3 addr26 addr26 addr26... // so if the first length field is garbled, it reads the second batch // of addr misaligned by 3 bytes. - if (memcmp(ip, pchIPv4+3, sizeof(pchIPv4)-3) == 0) + if (IsIPv6() && memcmp(ip, pchIPv4+3, sizeof(pchIPv4)-3) == 0) return false; // unspecified IPv6 address (::/128) unsigned char ipNone6[16] = {}; - if (memcmp(ip, ipNone6, 16) == 0) + if (IsIPv6() && memcmp(ip, ipNone6, 16) == 0) return false; // documentation IPv6 address @@ -311,7 +324,7 @@ bool CNetAddr::IsRoutable() const */ bool CNetAddr::IsInternal() const { - return memcmp(ip, g_internal_prefix, sizeof(g_internal_prefix)) == 0; + return m_net == NET_INTERNAL; } enum Network CNetAddr::GetNetwork() const @@ -322,13 +335,7 @@ enum Network CNetAddr::GetNetwork() const if (!IsRoutable()) return NET_UNROUTABLE; - if (IsIPv4()) - return NET_IPV4; - - if (IsTor()) - return NET_ONION; - - return NET_IPV6; + return m_net; } std::string CNetAddr::ToStringIP() const @@ -362,12 +369,12 @@ std::string CNetAddr::ToString() const bool operator==(const CNetAddr& a, const CNetAddr& b) { - return (memcmp(a.ip, b.ip, 16) == 0); + return a.m_net == b.m_net && memcmp(a.ip, b.ip, 16) == 0; } bool operator<(const CNetAddr& a, const CNetAddr& b) { - return (memcmp(a.ip, b.ip, 16) < 0); + return a.m_net < b.m_net || (a.m_net == b.m_net && memcmp(a.ip, b.ip, 16) < 0); } /** @@ -813,7 +820,7 @@ CSubNet::CSubNet(const CNetAddr &addr): */ bool CSubNet::Match(const CNetAddr &addr) const { - if (!valid || !addr.IsValid()) + if (!valid || !addr.IsValid() || network.m_net != addr.m_net) return false; for(int x=0; x<16; ++x) if ((addr.ip[x] & netmask[x]) != network.ip[x]) diff --git a/src/netaddress.h b/src/netaddress.h index f2daad7fb6..0365907d44 100644 --- a/src/netaddress.h +++ b/src/netaddress.h @@ -16,21 +16,50 @@ #include <string> #include <vector> +/** + * A network type. + * @note An address may belong to more than one network, for example `10.0.0.1` + * belongs to both `NET_UNROUTABLE` and `NET_IPV4`. + * Keep these sequential starting from 0 and `NET_MAX` as the last entry. + * We have loops like `for (int i = 0; i < NET_MAX; i++)` that expect to iterate + * over all enum values and also `GetExtNetwork()` "extends" this enum by + * introducing standalone constants starting from `NET_MAX`. + */ enum Network { + /// Addresses from these networks are not publicly routable on the global Internet. NET_UNROUTABLE = 0, + + /// IPv4 NET_IPV4, + + /// IPv6 NET_IPV6, + + /// TORv2 NET_ONION, + + /// A set of dummy addresses that map a name to an IPv6 address. These + /// addresses belong to RFC4193's fc00::/7 subnet (unique-local addresses). + /// We use them to map a string or FQDN to an IPv6 address in CAddrMan to + /// keep track of which DNS seeds were used. NET_INTERNAL, + /// Dummy value to indicate the number of NET_* constants. NET_MAX, }; -/** IP address (IPv6, or IPv4 using mapped IPv6 range (::FFFF:0:0/96)) */ +/** + * Network address. + */ class CNetAddr { protected: + /** + * Network to which this address belongs. + */ + Network m_net{NET_IPV6}; + unsigned char ip[16]; // in network byte order uint32_t scopeId{0}; // for scoped/link-local ipv6 addresses @@ -40,6 +69,14 @@ class CNetAddr void SetIP(const CNetAddr& ip); /** + * Set from a legacy IPv6 address. + * Legacy IPv6 address may be a normal IPv6 address, or another address + * (e.g. IPv4) disguised as IPv6. This encoding is used in the legacy + * `addr` encoding. + */ + void SetLegacyIPv6(const uint8_t ipv6[16]); + + /** * Set raw IPv4 or IPv6 address (in network byte order) * @note Only NET_IPV4 and NET_IPV6 are allowed for network. */ @@ -100,7 +137,27 @@ class CNetAddr friend bool operator!=(const CNetAddr& a, const CNetAddr& b) { return !(a == b); } friend bool operator<(const CNetAddr& a, const CNetAddr& b); - SERIALIZE_METHODS(CNetAddr, obj) { READWRITE(obj.ip); } + /** + * Serialize to a stream. + */ + template <typename Stream> + void Serialize(Stream& s) const + { + s << ip; + } + + /** + * Unserialize from a stream. + */ + template <typename Stream> + void Unserialize(Stream& s) + { + unsigned char ip_temp[sizeof(ip)]; + s >> ip_temp; + // Use SetLegacyIPv6() so that m_net is set correctly. For example + // ::FFFF:0102:0304 should be set as m_net=NET_IPV4 (1.2.3.4). + SetLegacyIPv6(ip_temp); + } friend class CSubNet; }; diff --git a/src/rest.cpp b/src/rest.cpp index 8cb594a03b..7130625d5c 100644 --- a/src/rest.cpp +++ b/src/rest.cpp @@ -68,13 +68,32 @@ static bool RESTERR(HTTPRequest* req, enum HTTPStatusCode status, std::string me } /** - * Get the node context mempool. + * Get the node context. * - * Set the HTTP error and return nullptr if node context - * mempool is not found. + * @param[in] req The HTTP request, whose status code will be set if node + * context is not found. + * @returns Pointer to the node context or nullptr if not found. + */ +static NodeContext* GetNodeContext(const util::Ref& context, HTTPRequest* req) +{ + NodeContext* node = context.Has<NodeContext>() ? &context.Get<NodeContext>() : nullptr; + if (!node) { + RESTERR(req, HTTP_INTERNAL_SERVER_ERROR, + strprintf("%s:%d (%s)\n" + "Internal bug detected: Node context not found!\n" + "You may report this issue here: %s\n", + __FILE__, __LINE__, __func__, PACKAGE_BUGREPORT)); + return nullptr; + } + return node; +} + +/** + * Get the node context mempool. * - * @param[in] req the HTTP request - * return pointer to the mempool or nullptr if no mempool found + * @param[in] req The HTTP request, whose status code will be set if node + * context mempool is not found. + * @returns Pointer to the mempool or nullptr if no mempool found. */ static CTxMemPool* GetMemPool(const util::Ref& context, HTTPRequest* req) { @@ -371,10 +390,13 @@ static bool rest_tx(const util::Ref& context, HTTPRequest* req, const std::strin g_txindex->BlockUntilSyncedToCurrentChain(); } - CTransactionRef tx; + const NodeContext* const node = GetNodeContext(context, req); + if (!node) return false; uint256 hashBlock = uint256(); - if (!GetTransaction(hash, tx, Params().GetConsensus(), hashBlock)) + const CTransactionRef tx = GetTransaction(/* block_index */ nullptr, node->mempool, hash, Params().GetConsensus(), hashBlock); + if (!tx) { return RESTERR(req, HTTP_NOT_FOUND, hashStr + " not found"); + } switch (rf) { case RetFormat::BINARY: { diff --git a/src/rpc/rawtransaction.cpp b/src/rpc/rawtransaction.cpp index d5e902cadd..70caf6009a 100644 --- a/src/rpc/rawtransaction.cpp +++ b/src/rpc/rawtransaction.cpp @@ -157,6 +157,8 @@ static UniValue getrawtransaction(const JSONRPCRequest& request) }, }.Check(request); + const NodeContext& node = EnsureNodeContext(request.context); + bool in_active_chain = true; uint256 hash = ParseHashV(request.params[0], "parameter 1"); CBlockIndex* blockindex = nullptr; @@ -188,9 +190,9 @@ static UniValue getrawtransaction(const JSONRPCRequest& request) f_txindex_ready = g_txindex->BlockUntilSyncedToCurrentChain(); } - CTransactionRef tx; uint256 hash_block; - if (!GetTransaction(hash, tx, Params().GetConsensus(), hash_block, blockindex)) { + const CTransactionRef tx = GetTransaction(blockindex, node.mempool, hash, Params().GetConsensus(), hash_block); + if (!tx) { std::string errmsg; if (blockindex) { if (!(blockindex->nStatus & BLOCK_HAVE_DATA)) { @@ -245,10 +247,11 @@ static UniValue gettxoutproof(const JSONRPCRequest& request) for (unsigned int idx = 0; idx < txids.size(); idx++) { const UniValue& txid = txids[idx]; uint256 hash(ParseHashV(txid, "txid")); - if (setTxids.count(hash)) - throw JSONRPCError(RPC_INVALID_PARAMETER, std::string("Invalid parameter, duplicated txid: ")+txid.get_str()); - setTxids.insert(hash); - oneTxid = hash; + if (setTxids.count(hash)) { + throw JSONRPCError(RPC_INVALID_PARAMETER, std::string("Invalid parameter, duplicated txid: ") + txid.get_str()); + } + setTxids.insert(hash); + oneTxid = hash; } CBlockIndex* pblockindex = nullptr; @@ -281,11 +284,11 @@ static UniValue gettxoutproof(const JSONRPCRequest& request) LOCK(cs_main); - if (pblockindex == nullptr) - { - CTransactionRef tx; - if (!GetTransaction(oneTxid, tx, Params().GetConsensus(), hashBlock) || hashBlock.IsNull()) + if (pblockindex == nullptr) { + const CTransactionRef tx = GetTransaction(/* block_index */ nullptr, /* mempool */ nullptr, oneTxid, Params().GetConsensus(), hashBlock); + if (!tx || hashBlock.IsNull()) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Transaction not yet in block"); + } pblockindex = LookupBlockIndex(hashBlock); if (!pblockindex) { throw JSONRPCError(RPC_INTERNAL_ERROR, "Transaction index corrupt"); @@ -293,15 +296,19 @@ static UniValue gettxoutproof(const JSONRPCRequest& request) } CBlock block; - if(!ReadBlockFromDisk(block, pblockindex, Params().GetConsensus())) + if (!ReadBlockFromDisk(block, pblockindex, Params().GetConsensus())) { throw JSONRPCError(RPC_INTERNAL_ERROR, "Can't read block from disk"); + } unsigned int ntxFound = 0; - for (const auto& tx : block.vtx) - if (setTxids.count(tx->GetHash())) + for (const auto& tx : block.vtx) { + if (setTxids.count(tx->GetHash())) { ntxFound++; - if (ntxFound != setTxids.size()) + } + } + if (ntxFound != setTxids.size()) { throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Not all transactions found in specified or retrieved block"); + } CDataStream ssMB(SER_NETWORK, PROTOCOL_VERSION | SERIALIZE_TRANSACTION_NO_WITNESS); CMerkleBlock mb(block, setTxids); diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp index 5fa128d62d..9978d084d5 100644 --- a/src/script/descriptor.cpp +++ b/src/script/descriptor.cpp @@ -825,8 +825,9 @@ std::unique_ptr<PubkeyProvider> ParsePubkey(uint32_t key_exp_index, const Span<c return nullptr; } if (origin_split.size() == 1) return ParsePubkeyInner(key_exp_index, origin_split[0], permit_uncompressed, out, error); - if (origin_split[0].size() < 1 || origin_split[0][0] != '[') { - error = strprintf("Key origin start '[ character expected but not found, got '%c' instead", origin_split[0][0]); + if (origin_split[0].empty() || origin_split[0][0] != '[') { + error = strprintf("Key origin start '[ character expected but not found, got '%c' instead", + origin_split[0].empty() ? /** empty, implies split char */ ']' : origin_split[0][0]); return nullptr; } auto slash_split = Split(origin_split[0].subspan(1), '/'); @@ -896,7 +897,7 @@ std::unique_ptr<DescriptorImpl> ParseScript(uint32_t key_exp_index, Span<const c providers.emplace_back(std::move(pk)); key_exp_index++; } - if (providers.size() < 1 || providers.size() > 16) { + if (providers.empty() || providers.size() > 16) { error = strprintf("Cannot have %u keys in multisig; must have between 1 and 16 keys, inclusive", providers.size()); return nullptr; } else if (thres < 1) { diff --git a/src/span.h b/src/span.h index 841f1eadf7..79f13c9203 100644 --- a/src/span.h +++ b/src/span.h @@ -151,6 +151,7 @@ public: return m_data[m_size - 1]; } constexpr std::size_t size() const noexcept { return m_size; } + constexpr bool empty() const noexcept { return size() == 0; } CONSTEXPR_IF_NOT_DEBUG C& operator[](std::size_t pos) const noexcept { ASSERT_IF_DEBUG(size() > pos); diff --git a/src/streams.h b/src/streams.h index e1d1b0eab2..6ce8065da8 100644 --- a/src/streams.h +++ b/src/streams.h @@ -814,18 +814,6 @@ public: return true; } - bool Seek(uint64_t nPos) { - long nLongPos = nPos; - if (nPos != (uint64_t)nLongPos) - return false; - if (fseek(src, nLongPos, SEEK_SET)) - return false; - nLongPos = ftell(src); - nSrcPos = nLongPos; - nReadPos = nLongPos; - return true; - } - //! prevent reading beyond a certain position //! no argument removes the limit bool SetLimit(uint64_t nPos = std::numeric_limits<uint64_t>::max()) { diff --git a/src/test/fuzz/buffered_file.cpp b/src/test/fuzz/buffered_file.cpp index 6bbd13eb5c..e575640be5 100644 --- a/src/test/fuzz/buffered_file.cpp +++ b/src/test/fuzz/buffered_file.cpp @@ -31,7 +31,7 @@ void test_one_input(const std::vector<uint8_t>& buffer) if (opt_buffered_file && fuzzed_file != nullptr) { bool setpos_fail = false; while (fuzzed_data_provider.ConsumeBool()) { - switch (fuzzed_data_provider.ConsumeIntegralInRange<int>(0, 5)) { + switch (fuzzed_data_provider.ConsumeIntegralInRange<int>(0, 4)) { case 0: { std::array<uint8_t, 4096> arr{}; try { @@ -41,20 +41,16 @@ void test_one_input(const std::vector<uint8_t>& buffer) break; } case 1: { - opt_buffered_file->Seek(fuzzed_data_provider.ConsumeIntegralInRange<uint64_t>(0, 4096)); - break; - } - case 2: { opt_buffered_file->SetLimit(fuzzed_data_provider.ConsumeIntegralInRange<uint64_t>(0, 4096)); break; } - case 3: { + case 2: { if (!opt_buffered_file->SetPos(fuzzed_data_provider.ConsumeIntegralInRange<uint64_t>(0, 4096))) { setpos_fail = true; } break; } - case 4: { + case 3: { if (setpos_fail) { // Calling FindByte(...) after a failed SetPos(...) call may result in an infinite loop. break; @@ -65,7 +61,7 @@ void test_one_input(const std::vector<uint8_t>& buffer) } break; } - case 5: { + case 4: { ReadFromStream(fuzzed_data_provider, *opt_buffered_file); break; } diff --git a/src/test/netbase_tests.cpp b/src/test/netbase_tests.cpp index ea3e633cc2..591b4ce49a 100644 --- a/src/test/netbase_tests.cpp +++ b/src/test/netbase_tests.cpp @@ -138,6 +138,14 @@ BOOST_AUTO_TEST_CASE(onioncat_test) } +BOOST_AUTO_TEST_CASE(embedded_test) +{ + CNetAddr addr1(ResolveIP("1.2.3.4")); + CNetAddr addr2(ResolveIP("::FFFF:0102:0304")); + BOOST_CHECK(addr2.IsIPv4()); + BOOST_CHECK_EQUAL(addr1.ToString(), addr2.ToString()); +} + BOOST_AUTO_TEST_CASE(subnet_test) { @@ -158,12 +166,13 @@ BOOST_AUTO_TEST_CASE(subnet_test) BOOST_CHECK(ResolveSubNet("1.2.2.1/24").Match(ResolveIP("1.2.2.4"))); BOOST_CHECK(ResolveSubNet("1.2.2.110/31").Match(ResolveIP("1.2.2.111"))); BOOST_CHECK(ResolveSubNet("1.2.2.20/26").Match(ResolveIP("1.2.2.63"))); - // All-Matching IPv6 Matches arbitrary IPv4 and IPv6 + // All-Matching IPv6 Matches arbitrary IPv6 BOOST_CHECK(ResolveSubNet("::/0").Match(ResolveIP("1:2:3:4:5:6:7:1234"))); // But not `::` or `0.0.0.0` because they are considered invalid addresses BOOST_CHECK(!ResolveSubNet("::/0").Match(ResolveIP("::"))); BOOST_CHECK(!ResolveSubNet("::/0").Match(ResolveIP("0.0.0.0"))); - BOOST_CHECK(ResolveSubNet("::/0").Match(ResolveIP("1.2.3.4"))); + // Addresses from one network (IPv4) don't belong to subnets of another network (IPv6) + BOOST_CHECK(!ResolveSubNet("::/0").Match(ResolveIP("1.2.3.4"))); // All-Matching IPv4 does not Match IPv6 BOOST_CHECK(!ResolveSubNet("0.0.0.0/0").Match(ResolveIP("1:2:3:4:5:6:7:1234"))); // Invalid subnets Match nothing (not even invalid addresses) diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 24c0d6382b..0cd991f453 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -142,7 +142,7 @@ TestingSetup::TestingSetup(const std::string& chainName, const std::vector<const ::ChainstateActive().InitCoinsDB( /* cache_size_bytes */ 1 << 23, /* in_memory */ true, /* should_wipe */ false); assert(!::ChainstateActive().CanFlushToDisk()); - ::ChainstateActive().InitCoinsCache(); + ::ChainstateActive().InitCoinsCache(1 << 23); assert(::ChainstateActive().CanFlushToDisk()); if (!LoadGenesisBlock(chainparams)) { throw std::runtime_error("LoadGenesisBlock failed."); diff --git a/src/test/validation_chainstate_tests.cpp b/src/test/validation_chainstate_tests.cpp new file mode 100644 index 0000000000..f531b393b8 --- /dev/null +++ b/src/test/validation_chainstate_tests.cpp @@ -0,0 +1,76 @@ +// Copyright (c) 2020 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 <random.h> +#include <uint256.h> +#include <consensus/validation.h> +#include <sync.h> +#include <test/util/setup_common.h> +#include <validation.h> + +#include <vector> + +#include <boost/test/unit_test.hpp> + +BOOST_FIXTURE_TEST_SUITE(validation_chainstate_tests, TestingSetup) + +//! Test resizing coins-related CChainState caches during runtime. +//! +BOOST_AUTO_TEST_CASE(validation_chainstate_resize_caches) +{ + ChainstateManager manager; + + //! Create and add a Coin with DynamicMemoryUsage of 80 bytes to the given view. + auto add_coin = [](CCoinsViewCache& coins_view) -> COutPoint { + Coin newcoin; + uint256 txid = InsecureRand256(); + COutPoint outp{txid, 0}; + newcoin.nHeight = 1; + newcoin.out.nValue = InsecureRand32(); + newcoin.out.scriptPubKey.assign((uint32_t)56, 1); + coins_view.AddCoin(outp, std::move(newcoin), false); + + return outp; + }; + + ENTER_CRITICAL_SECTION(cs_main); + CChainState& c1 = manager.InitializeChainstate(); + LEAVE_CRITICAL_SECTION(cs_main); + c1.InitCoinsDB( + /* cache_size_bytes */ 1 << 23, /* in_memory */ true, /* should_wipe */ false); + WITH_LOCK(::cs_main, c1.InitCoinsCache(1 << 23)); + + // Add a coin to the in-memory cache, upsize once, then downsize. + { + LOCK(::cs_main); + auto outpoint = add_coin(c1.CoinsTip()); + + // Set a meaningless bestblock value in the coinsview cache - otherwise we won't + // flush during ResizecoinsCaches() and will subsequently hit an assertion. + c1.CoinsTip().SetBestBlock(InsecureRand256()); + + BOOST_CHECK(c1.CoinsTip().HaveCoinInCache(outpoint)); + + c1.ResizeCoinsCaches( + 1 << 24, // upsizing the coinsview cache + 1 << 22 // downsizing the coinsdb cache + ); + + // View should still have the coin cached, since we haven't destructed the cache on upsize. + BOOST_CHECK(c1.CoinsTip().HaveCoinInCache(outpoint)); + + c1.ResizeCoinsCaches( + 1 << 22, // downsizing the coinsview cache + 1 << 23 // upsizing the coinsdb cache + ); + + // 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()); +} + +BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp index 0d149285ad..f99191b08f 100644 --- a/src/test/validation_chainstatemanager_tests.cpp +++ b/src/test/validation_chainstatemanager_tests.cpp @@ -28,13 +28,11 @@ BOOST_AUTO_TEST_CASE(chainstatemanager) // Create a legacy (IBD) chainstate. // - ENTER_CRITICAL_SECTION(cs_main); - CChainState& c1 = manager.InitializeChainstate(); - LEAVE_CRITICAL_SECTION(cs_main); + CChainState& c1 = *WITH_LOCK(::cs_main, return &manager.InitializeChainstate()); chainstates.push_back(&c1); c1.InitCoinsDB( /* cache_size_bytes */ 1 << 23, /* in_memory */ true, /* should_wipe */ false); - WITH_LOCK(::cs_main, c1.InitCoinsCache()); + WITH_LOCK(::cs_main, c1.InitCoinsCache(1 << 23)); BOOST_CHECK(!manager.IsSnapshotActive()); BOOST_CHECK(!manager.IsSnapshotValidated()); @@ -57,12 +55,13 @@ BOOST_AUTO_TEST_CASE(chainstatemanager) // Create a snapshot-based chainstate. // ENTER_CRITICAL_SECTION(cs_main); - CChainState& c2 = manager.InitializeChainstate(GetRandHash()); + CChainState& c2 = *WITH_LOCK(::cs_main, + return &manager.InitializeChainstate(GetRandHash())); LEAVE_CRITICAL_SECTION(cs_main); chainstates.push_back(&c2); c2.InitCoinsDB( /* cache_size_bytes */ 1 << 23, /* in_memory */ true, /* should_wipe */ false); - WITH_LOCK(::cs_main, c2.InitCoinsCache()); + WITH_LOCK(::cs_main, c2.InitCoinsCache(1 << 23)); // Unlike c1, which doesn't have any blocks. Gets us different tip, height. c2.LoadGenesisBlock(chainparams); BlockValidationState _; @@ -104,4 +103,58 @@ BOOST_AUTO_TEST_CASE(chainstatemanager) WITH_LOCK(::cs_main, manager.Unload()); } +//! Test rebalancing the caches associated with each chainstate. +BOOST_AUTO_TEST_CASE(chainstatemanager_rebalance_caches) +{ + ChainstateManager manager; + size_t max_cache = 10000; + manager.m_total_coinsdb_cache = max_cache; + manager.m_total_coinstip_cache = max_cache; + + std::vector<CChainState*> chainstates; + + // Create a legacy (IBD) chainstate. + // + ENTER_CRITICAL_SECTION(cs_main); + CChainState& c1 = manager.InitializeChainstate(); + LEAVE_CRITICAL_SECTION(cs_main); + chainstates.push_back(&c1); + c1.InitCoinsDB( + /* cache_size_bytes */ 1 << 23, /* in_memory */ true, /* should_wipe */ false); + + { + LOCK(::cs_main); + c1.InitCoinsCache(1 << 23); + c1.CoinsTip().SetBestBlock(InsecureRand256()); + manager.MaybeRebalanceCaches(); + } + + BOOST_CHECK_EQUAL(c1.m_coinstip_cache_size_bytes, max_cache); + BOOST_CHECK_EQUAL(c1.m_coinsdb_cache_size_bytes, max_cache); + + // Create a snapshot-based chainstate. + // + ENTER_CRITICAL_SECTION(cs_main); + CChainState& c2 = manager.InitializeChainstate(GetRandHash()); + LEAVE_CRITICAL_SECTION(cs_main); + chainstates.push_back(&c2); + c2.InitCoinsDB( + /* cache_size_bytes */ 1 << 23, /* in_memory */ true, /* should_wipe */ false); + + { + LOCK(::cs_main); + c2.InitCoinsCache(1 << 23); + c2.CoinsTip().SetBestBlock(InsecureRand256()); + manager.MaybeRebalanceCaches(); + } + + // Since both chainstates are considered to be in initial block download, + // the snapshot chainstate should take priority. + BOOST_CHECK_CLOSE(c1.m_coinstip_cache_size_bytes, max_cache * 0.05, 1); + BOOST_CHECK_CLOSE(c1.m_coinsdb_cache_size_bytes, max_cache * 0.05, 1); + BOOST_CHECK_CLOSE(c2.m_coinstip_cache_size_bytes, max_cache * 0.95, 1); + BOOST_CHECK_CLOSE(c2.m_coinsdb_cache_size_bytes, max_cache * 0.95, 1); + +} + BOOST_AUTO_TEST_SUITE_END() diff --git a/src/test/validation_flush_tests.cpp b/src/test/validation_flush_tests.cpp index a863e3a4d5..7111fc3380 100644 --- a/src/test/validation_flush_tests.cpp +++ b/src/test/validation_flush_tests.cpp @@ -21,7 +21,7 @@ BOOST_AUTO_TEST_CASE(getcoinscachesizestate) BlockManager blockman{}; CChainState chainstate{blockman}; chainstate.InitCoinsDB(/*cache_size_bytes*/ 1 << 10, /*in_memory*/ true, /*should_wipe*/ false); - WITH_LOCK(::cs_main, chainstate.InitCoinsCache()); + WITH_LOCK(::cs_main, chainstate.InitCoinsCache(1 << 10)); CTxMemPool tx_pool{}; constexpr bool is_64_bit = sizeof(void*) == 8; diff --git a/src/txdb.cpp b/src/txdb.cpp index 047560f45d..72460e7c69 100644 --- a/src/txdb.cpp +++ b/src/txdb.cpp @@ -10,6 +10,7 @@ #include <random.h> #include <shutdown.h> #include <uint256.h> +#include <util/memory.h> #include <util/system.h> #include <util/translation.h> #include <util/vector.h> @@ -39,35 +40,45 @@ struct CoinEntry { } -CCoinsViewDB::CCoinsViewDB(fs::path ldb_path, size_t nCacheSize, bool fMemory, bool fWipe) : db(ldb_path, nCacheSize, fMemory, fWipe, true) +CCoinsViewDB::CCoinsViewDB(fs::path ldb_path, size_t nCacheSize, bool fMemory, bool fWipe) : + m_db(MakeUnique<CDBWrapper>(ldb_path, nCacheSize, fMemory, fWipe, true)), + m_ldb_path(ldb_path), + m_is_memory(fMemory) { } + +void CCoinsViewDB::ResizeCache(size_t new_cache_size) { + // Have to do a reset first to get the original `m_db` state to release its + // filesystem lock. + m_db.reset(); + m_db = MakeUnique<CDBWrapper>( + m_ldb_path, new_cache_size, m_is_memory, /*fWipe*/ false, /*obfuscate*/ true); } bool CCoinsViewDB::GetCoin(const COutPoint &outpoint, Coin &coin) const { - return db.Read(CoinEntry(&outpoint), coin); + return m_db->Read(CoinEntry(&outpoint), coin); } bool CCoinsViewDB::HaveCoin(const COutPoint &outpoint) const { - return db.Exists(CoinEntry(&outpoint)); + return m_db->Exists(CoinEntry(&outpoint)); } uint256 CCoinsViewDB::GetBestBlock() const { uint256 hashBestChain; - if (!db.Read(DB_BEST_BLOCK, hashBestChain)) + if (!m_db->Read(DB_BEST_BLOCK, hashBestChain)) return uint256(); return hashBestChain; } std::vector<uint256> CCoinsViewDB::GetHeadBlocks() const { std::vector<uint256> vhashHeadBlocks; - if (!db.Read(DB_HEAD_BLOCKS, vhashHeadBlocks)) { + if (!m_db->Read(DB_HEAD_BLOCKS, vhashHeadBlocks)) { return std::vector<uint256>(); } return vhashHeadBlocks; } bool CCoinsViewDB::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock) { - CDBBatch batch(db); + CDBBatch batch(*m_db); size_t count = 0; size_t changed = 0; size_t batch_size = (size_t)gArgs.GetArg("-dbbatchsize", nDefaultDbBatchSize); @@ -105,7 +116,7 @@ bool CCoinsViewDB::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock) { mapCoins.erase(itOld); if (batch.SizeEstimate() > batch_size) { LogPrint(BCLog::COINDB, "Writing partial batch of %.2f MiB\n", batch.SizeEstimate() * (1.0 / 1048576.0)); - db.WriteBatch(batch); + m_db->WriteBatch(batch); batch.Clear(); if (crash_simulate) { static FastRandomContext rng; @@ -122,14 +133,14 @@ bool CCoinsViewDB::BatchWrite(CCoinsMap &mapCoins, const uint256 &hashBlock) { batch.Write(DB_BEST_BLOCK, hashBlock); LogPrint(BCLog::COINDB, "Writing final batch of %.2f MiB\n", batch.SizeEstimate() * (1.0 / 1048576.0)); - bool ret = db.WriteBatch(batch); + bool ret = m_db->WriteBatch(batch); LogPrint(BCLog::COINDB, "Committed %u changed transaction outputs (out of %u) to coin database...\n", (unsigned int)changed, (unsigned int)count); return ret; } size_t CCoinsViewDB::EstimateSize() const { - return db.EstimateSize(DB_COIN, (char)(DB_COIN+1)); + return m_db->EstimateSize(DB_COIN, (char)(DB_COIN+1)); } CBlockTreeDB::CBlockTreeDB(size_t nCacheSize, bool fMemory, bool fWipe) : CDBWrapper(GetDataDir() / "blocks" / "index", nCacheSize, fMemory, fWipe) { @@ -156,7 +167,7 @@ bool CBlockTreeDB::ReadLastBlockFile(int &nFile) { CCoinsViewCursor *CCoinsViewDB::Cursor() const { - CCoinsViewDBCursor *i = new CCoinsViewDBCursor(const_cast<CDBWrapper&>(db).NewIterator(), GetBestBlock()); + CCoinsViewDBCursor *i = new CCoinsViewDBCursor(const_cast<CDBWrapper&>(*m_db).NewIterator(), GetBestBlock()); /* It seems that there are no "const iterators" for LevelDB. Since we only need read operations on it, use a const-cast to get around that restriction. */ @@ -335,7 +346,7 @@ public: * Currently implemented: from the per-tx utxo model (0.8..0.14.x) to per-txout. */ bool CCoinsViewDB::Upgrade() { - std::unique_ptr<CDBIterator> pcursor(db.NewIterator()); + std::unique_ptr<CDBIterator> pcursor(m_db->NewIterator()); pcursor->Seek(std::make_pair(DB_COINS, uint256())); if (!pcursor->Valid()) { return true; @@ -346,7 +357,7 @@ bool CCoinsViewDB::Upgrade() { LogPrintf("[0%%]..."); /* Continued */ uiInterface.ShowProgress(_("Upgrading UTXO database").translated, 0, true); size_t batch_size = 1 << 24; - CDBBatch batch(db); + CDBBatch batch(*m_db); int reportDone = 0; std::pair<unsigned char, uint256> key; std::pair<unsigned char, uint256> prev_key = {DB_COINS, uint256()}; @@ -380,9 +391,9 @@ bool CCoinsViewDB::Upgrade() { } batch.Erase(key); if (batch.SizeEstimate() > batch_size) { - db.WriteBatch(batch); + m_db->WriteBatch(batch); batch.Clear(); - db.CompactRange(prev_key, key); + m_db->CompactRange(prev_key, key); prev_key = key; } pcursor->Next(); @@ -390,8 +401,8 @@ bool CCoinsViewDB::Upgrade() { break; } } - db.WriteBatch(batch); - db.CompactRange({DB_COINS, uint256()}, key); + m_db->WriteBatch(batch); + m_db->CompactRange({DB_COINS, uint256()}, key); uiInterface.ShowProgress("", 100, false); LogPrintf("[%s].\n", ShutdownRequested() ? "CANCELLED" : "DONE"); return !ShutdownRequested(); diff --git a/src/txdb.h b/src/txdb.h index 488c24f935..0cf7e2f1b8 100644 --- a/src/txdb.h +++ b/src/txdb.h @@ -39,11 +39,16 @@ static const int64_t max_filter_index_cache = 1024; //! Max memory allocated to coin DB specific cache (MiB) static const int64_t nMaxCoinsDBCache = 8; +// Actually declared in validation.cpp; can't include because of circular dependency. +extern RecursiveMutex cs_main; + /** CCoinsView backed by the coin database (chainstate/) */ class CCoinsViewDB final : public CCoinsView { protected: - CDBWrapper db; + std::unique_ptr<CDBWrapper> m_db; + fs::path m_ldb_path; + bool m_is_memory; public: /** * @param[in] ldb_path Location in the filesystem where leveldb data will be stored. @@ -60,6 +65,9 @@ public: //! Attempt to update from an older database format. Returns whether an error occurred. bool Upgrade(); size_t EstimateSize() const override; + + //! Dynamically alter the underlying leveldb cache size. + void ResizeCache(size_t new_cache_size) EXCLUSIVE_LOCKS_REQUIRED(cs_main); }; /** Specialization of CCoinsViewCursor to iterate over a CCoinsViewDB */ diff --git a/src/validation.cpp b/src/validation.cpp index 5aa3d315d5..545aa0f2d3 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -139,7 +139,6 @@ bool fPruneMode = false; bool fRequireStandard = true; bool fCheckBlockIndex = false; bool fCheckpointsEnabled = DEFAULT_CHECKPOINTS_ENABLED; -size_t nCoinCacheUsage = 5000 * 300; uint64_t nPruneTarget = 0; int64_t nMaxTipAge = DEFAULT_MAX_TIP_AGE; @@ -1089,45 +1088,33 @@ bool AcceptToMemoryPool(CTxMemPool& pool, TxValidationState &state, const CTrans return AcceptToMemoryPoolWithTime(chainparams, pool, state, tx, GetTime(), plTxnReplaced, bypass_limits, nAbsurdFee, test_accept); } -/** - * Return transaction in txOut, and if it was found inside a block, its hash is placed in hashBlock. - * If blockIndex is provided, the transaction is fetched from the corresponding block. - */ -bool GetTransaction(const uint256& hash, CTransactionRef& txOut, const Consensus::Params& consensusParams, uint256& hashBlock, const CBlockIndex* const block_index) +CTransactionRef GetTransaction(const CBlockIndex* const block_index, const CTxMemPool* const mempool, const uint256& hash, const Consensus::Params& consensusParams, uint256& hashBlock) { LOCK(cs_main); - if (!block_index) { - CTransactionRef ptx = mempool.get(hash); - if (ptx) { - txOut = ptx; - return true; - } - - if (g_txindex) { - return g_txindex->FindTx(hash, hashBlock, txOut); - } - } else { + if (block_index) { CBlock block; if (ReadBlockFromDisk(block, block_index, consensusParams)) { for (const auto& tx : block.vtx) { if (tx->GetHash() == hash) { - txOut = tx; hashBlock = block_index->GetBlockHash(); - return true; + return tx; } } } + return nullptr; } - - return false; + if (mempool) { + CTransactionRef ptx = mempool->get(hash); + if (ptx) return ptx; + } + if (g_txindex) { + CTransactionRef tx; + if (g_txindex->FindTx(hash, hashBlock, tx)) return tx; + } + return nullptr; } - - - - - ////////////////////////////////////////////////////////////////////////////// // // CBlock and CBlockIndex @@ -1284,9 +1271,10 @@ void CChainState::InitCoinsDB( leveldb_name, cache_size_bytes, in_memory, should_wipe); } -void CChainState::InitCoinsCache() +void CChainState::InitCoinsCache(size_t cache_size_bytes) { assert(m_coins_views != nullptr); + m_coinstip_cache_size_bytes = cache_size_bytes; m_coins_views->InitCache(); } @@ -2243,7 +2231,7 @@ CoinsCacheSizeState CChainState::GetCoinsCacheSizeState(const CTxMemPool& tx_poo { return this->GetCoinsCacheSizeState( tx_pool, - nCoinCacheUsage, + m_coinstip_cache_size_bytes, gArgs.GetArg("-maxmempool", DEFAULT_MAX_MEMPOOL_SIZE) * 1000000); } @@ -4318,7 +4306,7 @@ bool CVerifyDB::VerifyDB(const CChainParams& chainparams, CCoinsView *coinsview, } } // check level 3: check for inconsistencies during memory-only disconnect of tip blocks - if (nCheckLevel >= 3 && (coins.DynamicMemoryUsage() + ::ChainstateActive().CoinsTip().DynamicMemoryUsage()) <= nCoinCacheUsage) { + if (nCheckLevel >= 3 && (coins.DynamicMemoryUsage() + ::ChainstateActive().CoinsTip().DynamicMemoryUsage()) <= ::ChainstateActive().m_coinstip_cache_size_bytes) { assert(coins.GetBestBlock() == pindex->GetBlockHash()); DisconnectResult res = ::ChainstateActive().DisconnectBlock(block, pindex, coins); if (res == DISCONNECT_FAILED) { @@ -4981,6 +4969,39 @@ std::string CChainState::ToString() tip ? tip->nHeight : -1, tip ? tip->GetBlockHash().ToString() : "null"); } +bool CChainState::ResizeCoinsCaches(size_t coinstip_size, size_t coinsdb_size) +{ + if (coinstip_size == m_coinstip_cache_size_bytes && + coinsdb_size == m_coinsdb_cache_size_bytes) { + // Cache sizes are unchanged, no need to continue. + return true; + } + size_t old_coinstip_size = m_coinstip_cache_size_bytes; + m_coinstip_cache_size_bytes = coinstip_size; + m_coinsdb_cache_size_bytes = coinsdb_size; + CoinsDB().ResizeCache(coinsdb_size); + + LogPrintf("[%s] resized coinsdb cache to %.1f MiB\n", + this->ToString(), coinsdb_size * (1.0 / 1024 / 1024)); + LogPrintf("[%s] resized coinstip cache to %.1f MiB\n", + this->ToString(), coinstip_size * (1.0 / 1024 / 1024)); + + BlockValidationState state; + const CChainParams& chainparams = Params(); + + bool ret; + + if (coinstip_size > old_coinstip_size) { + // Likely no need to flush if cache sizes have grown. + ret = FlushStateToDisk(chainparams, state, FlushStateMode::IF_NEEDED); + } else { + // Otherwise, flush state to disk and deallocate the in-memory coins map. + ret = FlushStateToDisk(chainparams, state, FlushStateMode::ALWAYS); + CoinsTip().ReallocateCache(); + } + return ret; +} + std::string CBlockFileInfo::ToString() const { return strprintf("CBlockFileInfo(blocks=%u, size=%u, heights=%u...%u, time=%s...%s)", nBlocks, nSize, nHeightFirst, nHeightLast, FormatISO8601Date(nTimeFirst), FormatISO8601Date(nTimeLast)); @@ -5289,3 +5310,33 @@ void ChainstateManager::Reset() m_active_chainstate = nullptr; m_snapshot_validated = false; } + +void ChainstateManager::MaybeRebalanceCaches() +{ + if (m_ibd_chainstate && !m_snapshot_chainstate) { + LogPrintf("[snapshot] allocating all cache to the IBD chainstate\n"); + // Allocate everything to the IBD chainstate. + m_ibd_chainstate->ResizeCoinsCaches(m_total_coinstip_cache, m_total_coinsdb_cache); + } + else if (m_snapshot_chainstate && !m_ibd_chainstate) { + LogPrintf("[snapshot] allocating all cache to the snapshot chainstate\n"); + // Allocate everything to the snapshot chainstate. + m_snapshot_chainstate->ResizeCoinsCaches(m_total_coinstip_cache, m_total_coinsdb_cache); + } + else if (m_ibd_chainstate && m_snapshot_chainstate) { + // If both chainstates exist, determine who needs more cache based on IBD status. + // + // Note: shrink caches first so that we don't inadvertently overwhelm available memory. + if (m_snapshot_chainstate->IsInitialBlockDownload()) { + m_ibd_chainstate->ResizeCoinsCaches( + m_total_coinstip_cache * 0.05, m_total_coinsdb_cache * 0.05); + m_snapshot_chainstate->ResizeCoinsCaches( + m_total_coinstip_cache * 0.95, m_total_coinsdb_cache * 0.95); + } else { + m_snapshot_chainstate->ResizeCoinsCaches( + m_total_coinstip_cache * 0.05, m_total_coinsdb_cache * 0.05); + m_ibd_chainstate->ResizeCoinsCaches( + m_total_coinstip_cache * 0.95, m_total_coinsdb_cache * 0.95); + } + } +} diff --git a/src/validation.h b/src/validation.h index acadf151c5..53503768aa 100644 --- a/src/validation.h +++ b/src/validation.h @@ -127,7 +127,6 @@ extern bool g_parallel_script_checks; extern bool fRequireStandard; extern bool fCheckBlockIndex; extern bool fCheckpointsEnabled; -extern size_t nCoinCacheUsage; /** A fee rate smaller than this is considered zero fee (for relaying, mining and transaction creation) */ extern CFeeRate minRelayTxFee; /** If the tip is older than this (in seconds), the node is considered to be in initial block download. */ @@ -164,8 +163,19 @@ bool LoadGenesisBlock(const CChainParams& chainparams); void UnloadBlockIndex(); /** Run an instance of the script checking thread */ void ThreadScriptCheck(int worker_num); -/** Retrieve a transaction (from memory pool, or from disk, if possible) */ -bool GetTransaction(const uint256& hash, CTransactionRef& tx, const Consensus::Params& params, uint256& hashBlock, const CBlockIndex* const blockIndex = nullptr); +/** + * Return transaction from the block at block_index. + * If block_index is not provided, fall back to mempool. + * If mempool is not provided or the tx couldn't be found in mempool, fall back to g_txindex. + * + * @param[in] block_index The block to read from disk, or nullptr + * @param[in] mempool If block_index is not provided, look in the mempool, if provided + * @param[in] hash The txid + * @param[in] consensusParams The params + * @param[out] hashBlock The hash of block_index, if the tx was found via block_index + * @returns The tx if found, otherwise nullptr + */ +CTransactionRef GetTransaction(const CBlockIndex* const block_index, const CTxMemPool* const mempool, const uint256& hash, const Consensus::Params& consensusParams, uint256& hashBlock); /** * Find the best known block, and make it the tip of the block chain * @@ -521,7 +531,7 @@ public: //! Initialize the in-memory coins cache (to be done after the health of the on-disk database //! is verified). - void InitCoinsCache() EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + void InitCoinsCache(size_t cache_size_bytes) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); //! @returns whether or not the CoinsViews object has been fully initialized and we can //! safely flush this object to disk. @@ -570,6 +580,17 @@ public: //! Destructs all objects related to accessing the UTXO set. void ResetCoinsViews() { m_coins_views.reset(); } + //! The cache size of the on-disk coins view. + size_t m_coinsdb_cache_size_bytes{0}; + + //! The cache size of the in-memory coins view. + size_t m_coinstip_cache_size_bytes{0}; + + //! Resize the CoinsViews caches dynamically and flush state to disk. + //! @returns true unless an error occurred during the flush. + bool ResizeCoinsCaches(size_t coinstip_size, size_t coinsdb_size) + EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + /** * Update the on-disk chain state. * The caches and indexes are flushed depending on the mode we're called with @@ -786,6 +807,14 @@ public: //! chainstate to avoid duplicating block metadata. BlockManager m_blockman GUARDED_BY(::cs_main); + //! The total number of bytes available for us to use across all in-memory + //! coins caches. This will be split somehow across chainstates. + int64_t m_total_coinstip_cache{0}; + // + //! The total number of bytes available for us to use across all leveldb + //! coins databases. This will be split somehow across chainstates. + int64_t m_total_coinsdb_cache{0}; + //! Instantiate a new chainstate and assign it based upon whether it is //! from a snapshot. //! @@ -874,6 +903,10 @@ public: //! 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); }; /** DEPRECATED! Please use node.chainman instead. May only be used in validation.cpp internally */ diff --git a/src/wallet/bdb.cpp b/src/wallet/bdb.cpp index 1953be2d54..a8719806ab 100644 --- a/src/wallet/bdb.cpp +++ b/src/wallet/bdb.cpp @@ -32,12 +32,12 @@ void CheckUniqueFileid(const BerkeleyEnvironment& env, const std::string& filena int ret = db.get_mpf()->get_fileid(fileid.value); if (ret != 0) { - throw std::runtime_error(strprintf("BerkeleyBatch: Can't open database %s (get_fileid failed with %d)", filename, ret)); + throw std::runtime_error(strprintf("BerkeleyDatabase: Can't open database %s (get_fileid failed with %d)", filename, ret)); } for (const auto& item : env.m_fileids) { if (fileid == item.second && &fileid != &item.second) { - throw std::runtime_error(strprintf("BerkeleyBatch: Can't open database %s (duplicates fileid %s from %s)", filename, + throw std::runtime_error(strprintf("BerkeleyDatabase: Can't open database %s (duplicates fileid %s from %s)", filename, HexStr(std::begin(item.second.value), std::end(item.second.value)), item.first)); } } @@ -97,9 +97,8 @@ void BerkeleyEnvironment::Close() fDbEnvInit = false; for (auto& db : m_databases) { - auto count = mapFileUseCount.find(db.first); - assert(count == mapFileUseCount.end() || count->second == 0); BerkeleyDatabase& database = db.second.get(); + assert(database.m_refcount <= 0); if (database.m_db) { database.m_db->close(0); database.m_db.reset(); @@ -232,16 +231,6 @@ BerkeleyEnvironment::BerkeleyEnvironment() fMockDb = true; } -bool BerkeleyEnvironment::Verify(const std::string& strFile) -{ - LOCK(cs_db); - assert(mapFileUseCount.count(strFile) == 0); - - Db db(dbenv.get(), 0); - int result = db.verify(strFile.c_str(), nullptr, nullptr, 0); - return result == 0; -} - BerkeleyBatch::SafeDbt::SafeDbt() { m_dbt.set_flags(DB_DBT_MALLOC); @@ -295,7 +284,11 @@ bool BerkeleyDatabase::Verify(bilingual_str& errorStr) if (fs::exists(file_path)) { - if (!env->Verify(strFile)) { + assert(m_refcount == 0); + + Db db(env->dbenv.get(), 0); + int result = db.verify(strFile.c_str(), nullptr, nullptr, 0); + if (result != 0) { errorStr = strprintf(_("%s corrupt. Try using the wallet tool bitcoin-wallet to salvage or restoring a backup."), file_path); return false; } @@ -316,6 +309,8 @@ BerkeleyDatabase::~BerkeleyDatabase() { if (env) { LOCK(cs_db); + env->CloseDb(strFile); + assert(!m_db); size_t erased = env->m_databases.erase(strFile); assert(erased == 1); env->m_fileids.erase(strFile); @@ -324,13 +319,27 @@ BerkeleyDatabase::~BerkeleyDatabase() BerkeleyBatch::BerkeleyBatch(BerkeleyDatabase& database, const char* pszMode, bool fFlushOnCloseIn) : pdb(nullptr), activeTxn(nullptr), m_cursor(nullptr), m_database(database) { + database.AddRef(); + database.Open(pszMode); fReadOnly = (!strchr(pszMode, '+') && !strchr(pszMode, 'w')); fFlushOnClose = fFlushOnCloseIn; env = database.env.get(); - if (database.IsDummy()) { + pdb = database.m_db.get(); + strFile = database.strFile; + bool fCreate = strchr(pszMode, 'c') != nullptr; + if (fCreate && !Exists(std::string("version"))) { + bool fTmp = fReadOnly; + fReadOnly = false; + Write(std::string("version"), CLIENT_VERSION); + fReadOnly = fTmp; + } +} + +void BerkeleyDatabase::Open(const char* pszMode) +{ + if (IsDummy()){ return; } - const std::string &strFilename = database.strFile; bool fCreate = strchr(pszMode, 'c') != nullptr; unsigned int nFlags = DB_THREAD; @@ -341,10 +350,9 @@ BerkeleyBatch::BerkeleyBatch(BerkeleyDatabase& database, const char* pszMode, bo LOCK(cs_db); bilingual_str open_err; if (!env->Open(open_err)) - throw std::runtime_error("BerkeleyBatch: Failed to open database environment."); + throw std::runtime_error("BerkeleyDatabase: Failed to open database environment."); - pdb = database.m_db.get(); - if (pdb == nullptr) { + if (m_db == nullptr) { int ret; std::unique_ptr<Db> pdb_temp = MakeUnique<Db>(env->dbenv.get(), 0); @@ -353,60 +361,33 @@ BerkeleyBatch::BerkeleyBatch(BerkeleyDatabase& database, const char* pszMode, bo DbMpoolFile* mpf = pdb_temp->get_mpf(); ret = mpf->set_flags(DB_MPOOL_NOFILE, 1); if (ret != 0) { - throw std::runtime_error(strprintf("BerkeleyBatch: Failed to configure for no temp file backing for database %s", strFilename)); + throw std::runtime_error(strprintf("BerkeleyDatabase: Failed to configure for no temp file backing for database %s", strFile)); } } ret = pdb_temp->open(nullptr, // Txn pointer - fMockDb ? nullptr : strFilename.c_str(), // Filename - fMockDb ? strFilename.c_str() : "main", // Logical db name + fMockDb ? nullptr : strFile.c_str(), // Filename + fMockDb ? strFile.c_str() : "main", // Logical db name DB_BTREE, // Database type nFlags, // Flags 0); if (ret != 0) { - throw std::runtime_error(strprintf("BerkeleyBatch: Error %d, can't open database %s", ret, strFilename)); + throw std::runtime_error(strprintf("BerkeleyDatabase: Error %d, can't open database %s", ret, strFile)); } + m_file_path = (env->Directory() / strFile).string(); // Call CheckUniqueFileid on the containing BDB environment to // avoid BDB data consistency bugs that happen when different data // files in the same environment have the same fileid. - // - // Also call CheckUniqueFileid on all the other g_dbenvs to prevent - // bitcoin from opening the same data file through another - // environment when the file is referenced through equivalent but - // not obviously identical symlinked or hard linked or bind mounted - // paths. In the future a more relaxed check for equal inode and - // device ids could be done instead, which would allow opening - // different backup copies of a wallet at the same time. Maybe even - // more ideally, an exclusive lock for accessing the database could - // be implemented, so no equality checks are needed at all. (Newer - // versions of BDB have an set_lk_exclusive method for this - // purpose, but the older version we use does not.) - for (const auto& env : g_dbenvs) { - CheckUniqueFileid(*env.second.lock().get(), strFilename, *pdb_temp, this->env->m_fileids[strFilename]); - } + CheckUniqueFileid(*env, strFile, *pdb_temp, this->env->m_fileids[strFile]); - pdb = pdb_temp.release(); - database.m_db.reset(pdb); + m_db.reset(pdb_temp.release()); - if (fCreate && !Exists(std::string("version"))) { - bool fTmp = fReadOnly; - fReadOnly = false; - Write(std::string("version"), CLIENT_VERSION); - fReadOnly = fTmp; - } } - database.AddRef(); - strFile = strFilename; } } -void BerkeleyDatabase::Open(const char* mode) -{ - throw std::logic_error("BerkeleyDatabase does not implement Open. This function should not be called."); -} - void BerkeleyBatch::Flush() { if (activeTxn) @@ -427,6 +408,12 @@ void BerkeleyDatabase::IncrementUpdateCounter() ++nUpdateCounter; } +BerkeleyBatch::~BerkeleyBatch() +{ + Close(); + m_database.RemoveRef(); +} + void BerkeleyBatch::Close() { if (!pdb) @@ -439,8 +426,6 @@ void BerkeleyBatch::Close() if (fFlushOnClose) Flush(); - - m_database.RemoveRef(); } void BerkeleyEnvironment::CloseDb(const std::string& strFile) @@ -464,8 +449,8 @@ void BerkeleyEnvironment::ReloadDbEnv() AssertLockNotHeld(cs_db); std::unique_lock<RecursiveMutex> lock(cs_db); m_db_in_use.wait(lock, [this](){ - for (auto& count : mapFileUseCount) { - if (count.second > 0) return false; + for (auto& db : m_databases) { + if (db.second.get().m_refcount > 0) return false; } return true; }); @@ -493,11 +478,11 @@ bool BerkeleyDatabase::Rewrite(const char* pszSkip) while (true) { { LOCK(cs_db); - if (!env->mapFileUseCount.count(strFile) || env->mapFileUseCount[strFile] == 0) { + if (m_refcount <= 0) { // Flush log data to the dat file env->CloseDb(strFile); env->CheckpointLSN(strFile); - env->mapFileUseCount.erase(strFile); + m_refcount = -1; bool fSuccess = true; LogPrintf("BerkeleyBatch::Rewrite: Rewriting %s...\n", strFile); @@ -581,10 +566,11 @@ void BerkeleyEnvironment::Flush(bool fShutdown) return; { LOCK(cs_db); - std::map<std::string, int>::iterator mi = mapFileUseCount.begin(); - while (mi != mapFileUseCount.end()) { - std::string strFile = (*mi).first; - int nRefCount = (*mi).second; + bool no_dbs_accessed = true; + for (auto& db_it : m_databases) { + std::string strFile = db_it.first; + int nRefCount = db_it.second.get().m_refcount; + if (nRefCount < 0) continue; LogPrint(BCLog::WALLETDB, "BerkeleyEnvironment::Flush: Flushing %s (refcount = %d)...\n", strFile, nRefCount); if (nRefCount == 0) { // Move log data to the dat file @@ -595,14 +581,15 @@ void BerkeleyEnvironment::Flush(bool fShutdown) if (!fMockDb) dbenv->lsn_reset(strFile.c_str(), 0); LogPrint(BCLog::WALLETDB, "BerkeleyEnvironment::Flush: %s closed\n", strFile); - mapFileUseCount.erase(mi++); - } else - mi++; + nRefCount = -1; + } else { + no_dbs_accessed = false; + } } LogPrint(BCLog::WALLETDB, "BerkeleyEnvironment::Flush: Flush(%s)%s took %15dms\n", fShutdown ? "true" : "false", fDbEnvInit ? "" : " database not started", GetTimeMillis() - nStart); if (fShutdown) { char** listp; - if (mapFileUseCount.empty()) { + if (no_dbs_accessed) { dbenv->log_archive(&listp, DB_ARCH_REMOVE); Close(); if (!fMockDb) { @@ -623,13 +610,12 @@ bool BerkeleyDatabase::PeriodicFlush() if (!lockDb) return false; // Don't flush if any databases are in use - for (const auto& use_count : env->mapFileUseCount) { - if (use_count.second > 0) return false; + for (auto& it : env->m_databases) { + if (it.second.get().m_refcount > 0) return false; } // Don't flush if there haven't been any batch writes for this database. - auto it = env->mapFileUseCount.find(strFile); - if (it == env->mapFileUseCount.end()) return false; + if (m_refcount < 0) return false; LogPrint(BCLog::WALLETDB, "Flushing %s\n", strFile); int64_t nStart = GetTimeMillis(); @@ -637,7 +623,7 @@ bool BerkeleyDatabase::PeriodicFlush() // Flush wallet file so it's self contained env->CloseDb(strFile); env->CheckpointLSN(strFile); - env->mapFileUseCount.erase(it); + m_refcount = -1; LogPrint(BCLog::WALLETDB, "Flushed %s %dms\n", strFile, GetTimeMillis() - nStart); @@ -653,12 +639,11 @@ bool BerkeleyDatabase::Backup(const std::string& strDest) const { { LOCK(cs_db); - if (!env->mapFileUseCount.count(strFile) || env->mapFileUseCount[strFile] == 0) + if (m_refcount <= 0) { // Flush log data to the dat file env->CloseDb(strFile); env->CheckpointLSN(strFile); - env->mapFileUseCount.erase(strFile); // Copy wallet file fs::path pathSrc = env->Directory() / strFile; @@ -840,16 +825,18 @@ bool BerkeleyBatch::HasKey(CDataStream&& key) void BerkeleyDatabase::AddRef() { LOCK(cs_db); - ++env->mapFileUseCount[strFile]; + if (m_refcount < 0) { + m_refcount = 1; + } else { + m_refcount++; + } } void BerkeleyDatabase::RemoveRef() { - { - LOCK(cs_db); - --env->mapFileUseCount[strFile]; - } - env->m_db_in_use.notify_all(); + LOCK(cs_db); + m_refcount--; + if (env) env->m_db_in_use.notify_all(); } std::unique_ptr<DatabaseBatch> BerkeleyDatabase::MakeBatch(const char* mode, bool flush_on_close) diff --git a/src/wallet/bdb.h b/src/wallet/bdb.h index ef3b81d4d6..982423f00e 100644 --- a/src/wallet/bdb.h +++ b/src/wallet/bdb.h @@ -52,7 +52,6 @@ private: public: std::unique_ptr<DbEnv> dbenv; - std::map<std::string, int> mapFileUseCount; std::map<std::string, std::reference_wrapper<BerkeleyDatabase>> m_databases; std::unordered_map<std::string, WalletDatabaseFileId> m_fileids; std::condition_variable_any m_db_in_use; @@ -67,8 +66,6 @@ public: bool IsDatabaseLoaded(const std::string& db_filename) const { return m_databases.find(db_filename) != m_databases.end(); } fs::path Directory() const { return strPath; } - bool Verify(const std::string& strFile); - bool Open(bilingual_str& error); void Close(); void Flush(bool fShutdown); @@ -100,7 +97,6 @@ class BerkeleyBatch; **/ class BerkeleyDatabase : public WalletDatabase { - friend class BerkeleyBatch; public: /** Create dummy DB handle */ BerkeleyDatabase() : WalletDatabase(), env(nullptr) @@ -166,11 +162,12 @@ public: /** Database pointer. This is initialized lazily and reset during flushes, so it can be null. */ std::unique_ptr<Db> m_db; + std::string strFile; + /** Make a BerkeleyBatch connected to this database */ std::unique_ptr<DatabaseBatch> MakeBatch(const char* mode = "r+", bool flush_on_close = true) override; private: - std::string strFile; /** Return whether this database handle is a dummy for testing. * Only to be used at a low level, application should ideally not care @@ -220,7 +217,7 @@ protected: public: explicit BerkeleyBatch(BerkeleyDatabase& database, const char* pszMode = "r+", bool fFlushOnCloseIn=true); - ~BerkeleyBatch() override { Close(); } + ~BerkeleyBatch() override; BerkeleyBatch(const BerkeleyBatch&) = delete; BerkeleyBatch& operator=(const BerkeleyBatch&) = delete; diff --git a/test/functional/test_framework/mininode.py b/test/functional/test_framework/mininode.py index f68c1a9ddd..07811667a8 100755 --- a/test/functional/test_framework/mininode.py +++ b/test/functional/test_framework/mininode.py @@ -283,9 +283,13 @@ class P2PInterface(P2PConnection): def __init__(self): super().__init__() - # Track number of messages of each type received and the most recent - # message of each type + # Track number of messages of each type received. + # Should be read-only in a test. self.message_count = defaultdict(int) + + # Track the most recent message of each type. + # To wait for a message to be received, pop that message from + # this and use wait_until. self.last_message = {} # A count of the number of ping messages we've sent to the node @@ -472,7 +476,7 @@ class P2PInterface(P2PConnection): def wait_for_verack(self, timeout=60): def test_function(): - return self.message_count["verack"] + return "verack" in self.last_message self.wait_until(test_function, timeout=timeout) diff --git a/test/functional/wallet_multiwallet.py b/test/functional/wallet_multiwallet.py index 88beef1034..a54396cad3 100755 --- a/test/functional/wallet_multiwallet.py +++ b/test/functional/wallet_multiwallet.py @@ -120,7 +120,7 @@ class MultiWalletTest(BitcoinTestFramework): # should not initialize if one wallet is a copy of another shutil.copyfile(wallet_dir('w8'), wallet_dir('w8_copy')) - exp_stderr = r"BerkeleyBatch: Can't open database w8_copy \(duplicates fileid \w+ from w8\)" + exp_stderr = r"BerkeleyDatabase: Can't open database w8_copy \(duplicates fileid \w+ from w8\)" self.nodes[0].assert_start_raises_init_error(['-wallet=w8', '-wallet=w8_copy'], exp_stderr, match=ErrorMatch.PARTIAL_REGEX) # should not initialize if wallet file is a symlink @@ -258,10 +258,10 @@ class MultiWalletTest(BitcoinTestFramework): assert_raises_rpc_error(-4, "Wallet file verification failed. Error loading wallet wallet.dat. Duplicate -wallet filename specified.", self.nodes[0].loadwallet, 'wallet.dat') # Fail to load if one wallet is a copy of another - assert_raises_rpc_error(-4, "BerkeleyBatch: Can't open database w8_copy (duplicates fileid", self.nodes[0].loadwallet, 'w8_copy') + assert_raises_rpc_error(-4, "BerkeleyDatabase: Can't open database w8_copy (duplicates fileid", self.nodes[0].loadwallet, 'w8_copy') # Fail to load if one wallet is a copy of another, test this twice to make sure that we don't re-introduce #14304 - assert_raises_rpc_error(-4, "BerkeleyBatch: Can't open database w8_copy (duplicates fileid", self.nodes[0].loadwallet, 'w8_copy') + assert_raises_rpc_error(-4, "BerkeleyDatabase: Can't open database w8_copy (duplicates fileid", self.nodes[0].loadwallet, 'w8_copy') # Fail to load if wallet file is a symlink |