aboutsummaryrefslogtreecommitdiff
path: root/src/validation.h
AgeCommit message (Collapse)Author
2024-03-05Rename CalculateHeadersWork to CalculateClaimedHeadersWorkGreg Sanders
2024-02-28Merge bitcoin/bitcoin#29412: p2p: Don't process mutated blocksAva Chow
d8087adc7ebd4ea05dd3843e5a92e8115fd7bbcc [test] IsBlockMutated unit tests (dergoegge) 1ed2c9829700054526156297552bb47230406098 Add transaction_identifier::size to allow Span conversion (dergoegge) 1ec6bbeb8d27d31647d1433ccb87b362f6d81f90 [validation] Cache merkle root and witness commitment checks (dergoegge) 5bf4f5ba32da4627f152b54d266df9b2aa930457 [test] Add regression test for #27608 (dergoegge) 49257c0304828a185c273fcb99742c54bbef0c8e [net processing] Don't process mutated blocks (dergoegge) 2d8495e0800f5332748cd50eaad801ff77671bba [validation] Merkle root malleation should be caught by IsBlockMutated (dergoegge) 66abce1d98115e41f394bc4f4f52341960ddc839 [validation] Introduce IsBlockMutated (dergoegge) e7669e1343aec4298fd30d539847963e6fa5619c [refactor] Cleanup merkle root checks (dergoegge) 95bddb930aa72edd40fdff52cf447202995b0dce [validation] Isolate merkle root checks (dergoegge) Pull request description: This PR proposes to check for mutated blocks early as a defense-in-depth mitigation against attacks leveraging mutated blocks. We introduce `IsBlockMutated` which catches all known forms of block malleation and use it to do an early mutation check whenever we receive a `block` message. We have observed attacks that abused mutated blocks in the past, which could have been prevented by simply not processing mutated blocks (e.g. https://github.com/bitcoin/bitcoin/pull/27608 for which a regression test is included in this PR). ACKs for top commit: achow101: ACK d8087adc7ebd4ea05dd3843e5a92e8115fd7bbcc maflcko: ACK d8087adc7ebd4ea05dd3843e5a92e8115fd7bbcc 🏄 fjahr: Code review ACK d8087adc7ebd4ea05dd3843e5a92e8115fd7bbcc sr-gi: Code review ACK https://github.com/bitcoin/bitcoin/commit/d8087adc7ebd4ea05dd3843e5a92e8115fd7bbcc Tree-SHA512: 618ff4ea7f168e10f07504d3651290efbb1bb2ab3b838ffff3527c028caf6c52dedad18d04d3dbc627977479710930e200f2dfae18a08f627efe7e64a57e535f
2024-02-27[validation] Introduce IsBlockMutateddergoegge
2024-02-13scripted-diff: Fix bitcoin_config_h includesTheCharlatan
-BEGIN VERIFY SCRIPT- regex_string='^(?!//).*(AC_APPLE_UNIVERSAL_BUILD|BOOST_PROCESS_USE_STD_FS|CHAR_EQUALS_INT8|CLIENT_VERSION_BUILD|CLIENT_VERSION_IS_RELEASE|CLIENT_VERSION_MAJOR|CLIENT_VERSION_MINOR|COPYRIGHT_HOLDERS|COPYRIGHT_HOLDERS_FINAL|COPYRIGHT_HOLDERS_SUBSTITUTION|COPYRIGHT_YEAR|ENABLE_ARM_SHANI|ENABLE_AVX2|ENABLE_EXTERNAL_SIGNER|ENABLE_SSE41|ENABLE_TRACING|ENABLE_WALLET|ENABLE_X86_SHANI|ENABLE_ZMQ|HAVE_BOOST|HAVE_BUILTIN_CLZL|HAVE_BUILTIN_CLZLL|HAVE_BYTESWAP_H|HAVE_CLMUL|HAVE_CONSENSUS_LIB|HAVE_CXX20|HAVE_DECL_BE16TOH|HAVE_DECL_BE32TOH|HAVE_DECL_BE64TOH|HAVE_DECL_BSWAP_16|HAVE_DECL_BSWAP_32|HAVE_DECL_BSWAP_64|HAVE_DECL_FORK|HAVE_DECL_FREEIFADDRS|HAVE_DECL_GETIFADDRS|HAVE_DECL_HTOBE16|HAVE_DECL_HTOBE32|HAVE_DECL_HTOBE64|HAVE_DECL_HTOLE16|HAVE_DECL_HTOLE32|HAVE_DECL_HTOLE64|HAVE_DECL_LE16TOH|HAVE_DECL_LE32TOH|HAVE_DECL_LE64TOH|HAVE_DECL_PIPE2|HAVE_DECL_SETSID|HAVE_DECL_STRERROR_R|HAVE_DEFAULT_VISIBILITY_ATTRIBUTE|HAVE_DLFCN_H|HAVE_DLLEXPORT_ATTRIBUTE|HAVE_ENDIAN_H|HAVE_EVHTTP_CONNECTION_GET_PEER_CONST_CHAR|HAVE_FDATASYNC|HAVE_GETENTROPY_RAND|HAVE_GETRANDOM|HAVE_GMTIME_R|HAVE_INTTYPES_H|HAVE_LIBADVAPI32|HAVE_LIBCOMCTL32|HAVE_LIBCOMDLG32|HAVE_LIBGDI32|HAVE_LIBIPHLPAPI|HAVE_LIBKERNEL32|HAVE_LIBOLE32|HAVE_LIBOLEAUT32|HAVE_LIBSHELL32|HAVE_LIBSHLWAPI|HAVE_LIBUSER32|HAVE_LIBUUID|HAVE_LIBWINMM|HAVE_LIBWS2_32|HAVE_MALLOC_INFO|HAVE_MALLOPT_ARENA_MAX|HAVE_MINIUPNPC_MINIUPNPC_H|HAVE_MINIUPNPC_UPNPCOMMANDS_H|HAVE_MINIUPNPC_UPNPERRORS_H|HAVE_NATPMP_H|HAVE_O_CLOEXEC|HAVE_POSIX_FALLOCATE|HAVE_PTHREAD|HAVE_PTHREAD_PRIO_INHERIT|HAVE_STDINT_H|HAVE_STDIO_H|HAVE_STDLIB_H|HAVE_STRERROR_R|HAVE_STRINGS_H|HAVE_STRING_H|HAVE_STRONG_GETAUXVAL|HAVE_SYSCTL|HAVE_SYSCTL_ARND|HAVE_SYSTEM|HAVE_SYS_ENDIAN_H|HAVE_SYS_PRCTL_H|HAVE_SYS_RESOURCES_H|HAVE_SYS_SELECT_H|HAVE_SYS_STAT_H|HAVE_SYS_SYSCTL_H|HAVE_SYS_TYPES_H|HAVE_SYS_VMMETER_H|HAVE_THREAD_LOCAL|HAVE_TIMINGSAFE_BCMP|HAVE_UNISTD_H|HAVE_VM_VM_PARAM_H|LT_OBJDIR|PACKAGE_BUGREPORT|PACKAGE_NAME|PACKAGE_STRING|PACKAGE_TARNAME|PACKAGE_URL|PACKAGE_VERSION|PTHREAD_CREATE_JOINABLE|QT_QPA_PLATFORM_ANDROID|QT_QPA_PLATFORM_COCOA|QT_QPA_PLATFORM_MINIMAL|QT_QPA_PLATFORM_WINDOWS|QT_QPA_PLATFORM_XCB|QT_STATICPLUGIN|STDC_HEADERS|STRERROR_R_CHAR_P|USE_ASM|USE_BDB|USE_DBUS|USE_NATPMP|USE_QRCODE|USE_SQLITE|USE_UPNP|_FILE_OFFSET_BITS|_LARGE_FILES)' exclusion_files=":(exclude)src/minisketch :(exclude)src/crc32c :(exclude)src/secp256k1 :(exclude)src/crypto/sha256_arm_shani.cpp :(exclude)src/crypto/sha256_avx2.cpp :(exclude)src/crypto/sha256_sse41.cpp :(exclude)src/crypto/sha256_x86_shani.cpp" git grep --perl-regexp --files-with-matches "$regex_string" -- '*.cpp' $exclusion_files | xargs git grep -L "bitcoin-config.h" | while read -r file; do line_number=$(awk -v my_file="$file" '/\/\/ file COPYING or https?:\/\/www.opensource.org\/licenses\/mit-license.php\./ {line = NR} /^\/\// && NR == line + 1 {while(getline && /^\/\//) line = NR} END {print line+1}' "$file"); sed -i "${line_number}i\\\\n\#if defined(HAVE_CONFIG_H)\\n#include <config/bitcoin-config.h>\\n\#endif" "$file"; done; git grep --perl-regexp --files-with-matches "$regex_string" -- '*.h' $exclusion_files | xargs git grep -L "bitcoin-config.h" | while read -r file; do sed -i "/#define.*_H/a \\\\n\#if defined(HAVE_CONFIG_H)\\n#include <config/bitcoin-config.h>\\n\#endif" "$file"; done; for file in $(git grep --files-with-matches 'bitcoin-config.h' -- '*.cpp' '*.h' $exclusion_files); do if ! grep -q --perl-regexp "$regex_string" $file; then sed -i '/HAVE_CONFIG_H/{N;N;N;d;}' $file; fi; done; -END VERIFY SCRIPT- The first command creates a regular expression for matching all bitcoin-config.h symbols in the following form: ^(?!//).*(AC_APPLE_UNIVERSAL_BUILD|BOOST_PROCESS_USE_STD_FS|...|_LARGE_FILES). It was generated with: ./autogen.sh && printf '^(?!//).*(%s)' $(awk '/^#undef/ {print $2}' src/config/bitcoin-config.h.in | paste -sd "|" -) The second command holds a list of files and directories that should not be processed. These include subtree directories as well as some crypto files that already get their symbols through the makefile. The third command checks for missing bitcoin-config headers in .cpp files and adds the header if it is missing. The fourth command checks for missing bitcoin-config headers in .h files and adds the header if it is missing. The fifth command checks for unneeded bitcoin-config headers in sources files and removes the header if it is unneeded.
2024-01-05Remove GetAdjustedTimedergoegge
2023-11-30Merge bitcoin/bitcoin#26762: bugfix: Make `CCheckQueue` RAII-styled (attempt 2)Andrew Chow
5b3ea5fa2e7f6dc1c9161ed8b74c9be4bd1e92dd refactor: Move `{MAX,DEFAULT}_SCRIPTCHECK_THREADS` constants (Hennadii Stepanov) 6e17b3168072ab77ed7170ab81327c017877133a refactor: Make `CCheckQueue` non-copyable and non-movable explicitly (Hennadii Stepanov) 8111e74653dc5c93cb510672d99048c3f741d8dc refactor: Drop unneeded declaration (Hennadii Stepanov) 9cf89f7a5b81197e38f58b24be0793b28fe41477 refactor: Make `CCheckQueue` constructor start worker threads (Hennadii Stepanov) d03eaacbcfb276fb638db1b423113ff43bd7ec41 Make `CCheckQueue` destructor stop worker threads (Hennadii Stepanov) be4ff3060b7b43b496dfb5a2c02b114b2b717106 Move global `scriptcheckqueue` into `ChainstateManager` class (Hennadii Stepanov) Pull request description: This PR: - makes `CCheckQueue` RAII-styled - gets rid of the global `scriptcheckqueue` - fixes https://github.com/bitcoin/bitcoin/issues/25448 The previous attempt was in https://github.com/bitcoin/bitcoin/pull/18731. ACKs for top commit: martinus: ACK 5b3ea5fa2e7 achow101: ACK 5b3ea5fa2e7f6dc1c9161ed8b74c9be4bd1e92dd TheCharlatan: ACK 5b3ea5fa2e7f6dc1c9161ed8b74c9be4bd1e92dd Tree-SHA512: 45cca846e7ed107e3930149f0b616ddbaf2648d6cde381f815331b861b5d67ab39e154883ae174b8abb1dae485bc904318c50c51e5d6b46923d89de51c5eadb0
2023-11-18blockstorage: switch from CAutoFile to AutoFileAnthony Towns
Also bump includes per suggestions from iwyu.
2023-11-07[validation] change package-fee-too-low, return wtxid(s) and effective feerateglozow
With subpackage evaluation and de-duplication, it's not always the entire package that is used in CheckFeerate. To be more helpful to the caller, specify which transactions were included in the evaluation and what the feerate was. Instead of PCKG_POLICY (which is supposed to be for package-wide errors), use PCKG_TX.
2023-11-06[refactor] use Wtxid for m_wtxids_fee_calculationsglozow
2023-10-06doc: Add and edit some comments around assumeutxoFabian Jahr
Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
2023-10-06validation: remove unused mempool param in DetectSnapshotChainstateFabian Jahr
2023-10-03refactor: Move `{MAX,DEFAULT}_SCRIPTCHECK_THREADS` constantsHennadii Stepanov
2023-10-03Make `CCheckQueue` destructor stop worker threadsHennadii Stepanov
2023-10-03Move global `scriptcheckqueue` into `ChainstateManager` classHennadii Stepanov
2023-09-30validation: assumeutxo: swap m_mempool on snapshot activationJames O'Beirne
Otherwise we will not receive transactions during background sync until restart.
2023-09-30validation: pruning for multiple chainstatesJames O'Beirne
Introduces ChainstateManager::GetPruneRange(). The prune budget is split evenly between the number of chainstates, however the prune budget may be exceeded if the resulting shares are beneath `MIN_DISK_SPACE_FOR_BLOCK_FILES`.
2023-09-30validation: indexing changes for assumeutxoJames O'Beirne
When using an assumedvalid chainstate, only process validationinterface callbacks from the background chainstate within indexes. This ensures that all indexes are built in-order. Later, we can possibly designate indexes which can be built out of order and continue their operation during snapshot use. Once the background sync has completed, restart the indexes so that they continue to index the now-validated snapshot chainstate.
2023-09-30validation: add ChainstateRoleJames O'Beirne
2023-09-30chainparams: add blockhash to AssumeutxoDataJames O'Beirne
This allows us to reference assumeutxo configuration by blockhash as well as height; this is helpful in future changes when we want to reference assumeutxo configurations before the block index is loaded.
2023-09-30assumeutxo: remove snapshot during -reindex{-chainstate}James O'Beirne
Removing a snapshot chainstate from disk (and memory) is consistent with existing reindex operations.
2023-09-30net_processing: Request assumeutxo background chain blocksSuhas Daftuar
Add new PeerManagerImpl::TryDownloadingHistoricalBlocks method and use it to request background chain blocks in addition to blocks normally requested by FindNextBlocksToDownload. Co-authored-by: Ryan Ofsky <ryan@ofsky.org> Co-authored-by: James O'Beirne <james.obeirne@gmail.com>
2023-09-26Merge bitcoin/bitcoin#28483: refactor: Return CAutoFile from ↵fanquake
BlockManager::Open*File() fa56c421be04af846f479c30749b17e6663ab418 Return CAutoFile from BlockManager::Open*File() (MarcoFalke) 9999b89cd37fb2a23c5ebcd91d9cb31d69375933 Make BufferedFile to be a CAutoFile wrapper (MarcoFalke) fa389d902fbf5fac19fba8c5d0c5e0a25f15ca63 refactor: Drop unused fclose() from BufferedFile (MarcoFalke) Pull request description: This is required for https://github.com/bitcoin/bitcoin/pull/28052, but makes sense on its own, because offloading logic to `CAutoFile` instead of re-implementing it allows to delete code and complexity. ACKs for top commit: TheCharlatan: Re-ACK fa56c421be04af846f479c30749b17e6663ab418 willcl-ark: tACK fa56c421be Tree-SHA512: fe4638f3a6bd3f9d968cfb9ae3259c9d6cd278fe2912cbc90289851311c8c781099db4c160e775960975c4739098d9af801a8d2d12603f371f8edfe134d8f85a
2023-09-23Merge bitcoin/bitcoin#28385: [refactor] rewrite ↵fanquake
DisconnectedBlockTransactions to not use boost 4313c77400eb8eaa8586db39a7e29a861772ea80 make DisconnectedBlockTransactions responsible for its own memory management (glozow) cf5f1faa037e9a40a5029cc7dd4ee61454b62466 MOVEONLY: DisconnectedBlockTransactions to its own file (glozow) 2765d6f3434c101fe2d46e9313e540aa680fbd77 rewrite DisconnectedBlockTransactions as a list + map (glozow) 79ce9f0aa46de8ff742be83fd6f68eab40e073ec add std::list to memusage (glozow) 59a35a7398f5bcb3e3805d1e4f363e4c2fb336b3 [bench] DisconnectedBlockTransactions (glozow) 925bb723ca71aa76380b769d8926c7c2ad9bbb7b [refactor] batch-add transactions to DisconnectedBlockTransactions (glozow) Pull request description: Motivation - I think it's preferable to use stdlib data structures instead of depending on boost if we can achieve the same thing. - Also see #28335 for further context/motivation. This PR simplifies that one. Things done in this PR: - Add a bench for `DisconnectedBlockTransactions` where we reorg and the new chain has {100%, 90%, 10%} of the same transactions. AFAIU in practice, it's usually close to 100%. - Rewrite `DisconnectedBlockTransactions` as a `std::list` + `unordered_map` instead of a boost multi index container. - On my machine, the bench suggests the performance is very similar. - Move `DisconnectedBlockTransactions` from txmempool.h to its own kernel/disconnected_transactions.h. This struct isn't used by txmempool and doesn't have much to do with txmempool. My guess is that it's been living there for convenience since the boost includes are there. ACKs for top commit: ismaelsadeeq: Tested ACK 4313c77400eb8eaa8586db39a7e29a861772ea80 stickies-v: ACK 4313c77400eb8eaa8586db39a7e29a861772ea80 TheCharlatan: ACK 4313c77400eb8eaa8586db39a7e29a861772ea80 Tree-SHA512: 273c80866bf3acd39b2a039dc082b7719d2d82e0940e1eb6c402f1c0992e997256722b85c7e310c9811238a770cfbdeb122ea4babbc23835d17128f214a1ef9e
2023-09-15Make BufferedFile to be a CAutoFile wrapperMarcoFalke
This refactor allows to forward some calls to the underlying CAutoFile, instead of re-implementing the logic in the buffered file.
2023-09-13Merge bitcoin/bitcoin#28251: validation: fix coins disappearing mid-package ↵fanquake
evaluation 32c1dd1ad65af0ad4d36a56d2ca32a8481237e68 [test] mempool coins disappearing mid-package evaluation (glozow) a67f460c3fd1c7eb8070623666d887eefccff0d6 [refactor] split setup in mempool_limit test (glozow) d08696120e3647b4c2cd0ae8d6e57dea12418b7c [test framework] add ability to spend only confirmed utxos (glozow) 3ea71feb11c261f002ed918f91f3434fd8a23589 [validation] don't LimitMempoolSize in any subpackage submissions (glozow) d227b7234cd4cfd7c593ffcf8e2f24573d1ebea5 [validation] return correct result when already-in-mempool tx gets evicted (glozow) 9698b81828ff98820fa49c83ca364063233374c6 [refactor] back-fill results in AcceptPackage (glozow) 8ad7ad33929ee846a55a43c55732be0cb8973060 [validation] make PackageMempoolAcceptResult members mutable (glozow) 03b87c11ca0705e1d6147b90da33ce555f9f41c8 [validation] add AcceptSubPackage to delegate Accept* calls and clean up m_view (glozow) 3f01a3dab1c4ee37fd4093b6a0a3b622f53e231d [CCoinsViewMemPool] track non-base coins and allow Reset (glozow) 7d7f7a1189432b1b6245ba25df572229870567cb [policy] check for duplicate txids in package (glozow) Pull request description: While we are evaluating a package, we split it into "subpackages" for evaluation (currently subpackages all have size 1 except the last one). If a subpackage has size 1, we may add a tx to mempool and call `LimitMempoolSize()`, which evicts transactions if the mempool gets full. We handle the case where the just-submitted transaction is evicted immediately, but we don't handle the case in which a transaction from a previous subpackage (either just submitted or already in mempool) is evicted. Mainly, since the coins created by the evicted transaction are cached in `m_view`, we don't realize the UTXO has disappeared until `CheckInputsFromMempoolAndCache` asserts that they exist. Also, the returned `PackageMempoolAcceptResult` reports that the transaction is in mempool even though it isn't anymore. Fix this by not calling `LimitMempoolSize()` until the very end, and editing the results map with "mempool full" if things fall out. Pointed out by instagibbs in https://github.com/bitcoin/bitcoin/commit/faeed687e5cde5e32750d93818dd1d4add837f24 on top of the v3 PR. ACKs for top commit: instagibbs: reACK https://github.com/bitcoin/bitcoin/pull/28251/commits/32c1dd1ad65af0ad4d36a56d2ca32a8481237e68 Tree-SHA512: 61e7f69db4712e5e5bfa27d037ab66bdd97f1bf60a8d9ffb96adb1f0609af012c810d681102ee5c7baec7b5fe8cb7c304a60c63ccc445d00d86a2b7f0e7ddb90
2023-09-13[validation] make PackageMempoolAcceptResult members mutableglozow
After the PackageMempoolAcceptResult is returned from AcceptMultipleTransactions, leave room for results to change due to LimitMempool() eviction.
2023-09-13rewrite DisconnectedBlockTransactions as a list + mapglozow
And encapsulate underlying data structures to avoid misuse. It's better to use stdlib instead of boost when we can achieve the same thing. Behavior change: the number returned by DynamicMemoryUsage for the same transactions is higher (due to change in usage or more accurate accounting), which effectively decreases the maximum amount of transactions kept for resubmission in a reorg. Co-authored-by: Cory Fields <cory-nospam-@coryfields.com>
2023-09-05Merge bitcoin/bitcoin#28195: blockstorage: Drop legacy -txindex checkfanquake
fae405556d56f6f13ce57f69a06b9ec1e825422b scripted-diff: Rename CBlockTreeDB -> BlockTreeDB (MarcoFalke) faf63039cce40f5cf8dea5a1d24945773c3433a1 Fixup style of moved code (MarcoFalke) fa65111b99627289fd47dcfaa5197e0f09b8a50e move-only: Move CBlockTreeDB to node/blockstorage (MarcoFalke) fa8685597e7302fc136f21b6dd3a4b187fa8e251 index: Drop legacy -txindex check (MarcoFalke) fa69148a0a26c5054dbccdceeac8e117bf449275 scripted-diff: Use blocks_path where possible (MarcoFalke) Pull request description: The only reason for the check was to print a warning about an increase in storage use. Now that 22.x is EOL and everyone should have migrated (or decided to not care about storage use), remove the check. Also, a move-only commit is included. (Rebased from https://github.com/bitcoin/bitcoin/pull/22242) ACKs for top commit: TheCharlatan: ACK fae405556d56f6f13ce57f69a06b9ec1e825422b, though I lack historical context to really judge the second commit fa8685597e7302fc136f21b6dd3a4b187fa8e251. stickies-v: ACK fae405556d56f6f13ce57f69a06b9ec1e825422b Tree-SHA512: 9da8f48767ae52d8e8e21c09a40c949cc0838794f1856cc5f58a91acd3f00a3bca818c8082242b3fdc9ca5badb09059570bb3870850d3807b75a8e23b5222da1
2023-08-18assumeutxo cleanup: Move IsInitialBlockDownload & NotifyHeaderTip to ↵Ryan Ofsky
ChainstateManager This change makes IsInitialBlockDownload and NotifyHeaderTip functions no longer tied to individual Chainstate objects. It makes them work with the ChainstateManager object instead so code is simpler and it is no longer possible to call them incorrectly with an inactive Chainstate. This change also makes m_cached_finished_ibd caching easier to reason about, because now there is only one cached value instead of two (for background and snapshot chainstates) so the cached IBD state now no longer gets reset when a snapshot is loaded. There should be no change in behavior because these functions were always called on the active ChainState objects. These changes were discussed previously https://github.com/bitcoin/bitcoin/pull/27746#discussion_r1246868905 and https://github.com/bitcoin/bitcoin/pull/27746#discussion_r1237552792 as possible followups for that PR.
2023-08-17Merge bitcoin/bitcoin#27675: p2p: Drop m_recently_announced_invs bloom filterfanquake
fb02ba3c5f5bcd96b5e3622ef001b8e57ce63fc0 mempool_entry: improve struct packing (Anthony Towns) 1a118062fbc4ec8f645f4ec4298d869a869c3344 net_processing: Clean up INVENTORY_BROADCAST_MAX constants (Anthony Towns) 6fa49937e488d0924044786c76b42324b659f351 test: Check tx from disconnected block is immediately requestable (glozow) e4ffabbffacc4b890d393aafcc8286916ef887d8 net_processing: don't add txids to m_tx_inventory_known_filter (Anthony Towns) 6ec1809d33bfc42b80cb6f35625dccd56be8d507 net_processing: drop m_recently_announced_invs bloom filter (Anthony Towns) a70beafdb22564043dc24fc98133fdadbaf77d8a validation: when adding txs due to a block reorg, allow immediate relay (Anthony Towns) 1e9684f39fba909b3501e9402d5b61f4bf744ff2 mempool_entry: add mempool entry sequence number (Anthony Towns) Pull request description: This PR replaces the `m_recently_announced_invs` bloom filter with a simple sequence number tracking the mempool state when we last considered sending an INV message to a node. This saves 33kB per peer (or more if we raise the rate at which we relay transactions over the network, in which case we would need to increase the size of the bloom filter proportionally). The philosophy here (compare with #18861 and #19109) is that we consider the rate limiting on INV messages to only be about saving bandwidth and not protecting privacy, and therefore after you receive an INV message, it's immediately fair game to request any transaction that was in the mempool at the time the INV message was sent. We likewise consider the BIP 133 feefilter and BIP 37 bloom filters to be bandwidth optimisations here, and treat transactions as requestable if they would have been announced without those filters. Given that philosophy, tracking the timestamp of the last INV message and comparing that against the mempool entry time allows removal of each of `m_recently_announced_invs`, `m_last_mempool_req` and `UNCONDITIONAL_RELAY_DELAY` and associated logic. ACKs for top commit: naumenkogs: ACK fb02ba3c5f5bcd96b5e3622ef001b8e57ce63fc0 amitiuttarwar: review ACK fb02ba3c5f5 glozow: reACK fb02ba3c5f5bcd96b5e3622ef001b8e57ce63fc0 Tree-SHA512: cbba5ee04c86df26b6057f3654c00a2b45ec94d354f4f157a769cecdaa0b509edaac02b3128afba39b023e82473fc5e28c915a787f84457ffe66638c6ac9c2d4
2023-08-07Remove Chainstate::LoadMempoolMarcoFalke
The 3-line function is only called once outside of tests, so it is clearer to inline it.
2023-08-03validation: when adding txs due to a block reorg, allow immediate relayAnthony Towns
2023-08-01move-only: Move CBlockTreeDB to node/blockstorageMarcoFalke
The block index (CBlockTreeDB) is required to write and read blocks, so move it to blockstorage. This allows to drop the txdb.h include from `node/blockstorage.h`. Can be reviewed with: --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space
2023-07-24Remove unused function `reliesOnAssumedValid`Suhas Daftuar
2023-07-24Cache block index entry corresponding to assumeutxo snapshot base blockhashSuhas Daftuar
This is to (a) avoid repeated lookups into the block index for an entry that should never change and (b) emphasize that the snapshot base should always exist when set and not change during the runtime of the program. Thanks to Russ Yanofsky for suggesting this approach.
2023-07-24Move CheckBlockIndex() from Chainstate to ChainstateManagerSuhas Daftuar
Also rewrite CheckBlockIndex() to perform tests on all chainstates. This increases sanity-check coverage, as any place in our code where we were invoke CheckBlockIndex() on a single chainstate will now invoke the sanity checks on all chainstates. This change also tightens up the checks on setBlockIndexCandidates and mapBlocksUnlinked, to more precisely match what we aim for even in the presence of assumed-valid blocks.
2023-07-21Move block-storage-related logic to ChainstateManagerSuhas Daftuar
Separate the notion of which blocks are stored on disk, and what data is in our block index, from what tip a chainstate might be able to get to. We can use chainstate-agnostic data to determine when to store a block on disk (primarily, an anti-DoS set of criteria) and let the chainstates figure out for themselves when a block is of interest for being a candidate tip. Note: some of the invariants in CheckBlockIndex are modified, but more work is needed (ie to move CheckBlockIndex to ChainstateManager, as most of what CheckBlockIndex is doing is checking the consistency of the block index, which is outside of Chainstate).
2023-07-14Add wrapper for adding entries to a chainstate's block index candidatesSuhas Daftuar
2023-07-14Move block-arrival information / preciousblock counters to ChainstateManagerSuhas Daftuar
Block arrival information (and the preciousblock RPC, a related concept) are both chainstate-agnostic, so these are moved to ChainstateManager. This should just be a refactor, without any observable behavior changes.
2023-07-11kernel: Remove StartShutdown calls from validation codeRyan Ofsky
This change drops the last kernel dependency on shutdown.cpp. It also adds new hooks for libbitcoinkernel applications to be able to interrupt kernel operations when the chain tip changes. This is a refactoring that does not affect behavior. (Looking at the code it can appear like the new break statement in the ActivateBestChain function is a change in behavior, but actually the previous StartShutdown call was indirectly triggering a break before, because it was causing m_chainman.m_interrupt to be true. The new code just makes the break more obvious.)
2023-07-07scripted-diff: rename 'loadblk' thread name to 'initload'furszy
The thread does not only load blocks, it loads the mempool and, in a future commit, will start the indexes as well. Also, renamed the 'ThreadImport' function to 'ImportBlocks' And the 'm_load_block' class member to 'm_thread_load'. -BEGIN VERIFY SCRIPT- sed -i "s/ThreadImport/ImportBlocks/g" $(git grep -l ThreadImport -- ':!/doc/') sed -i "s/loadblk/initload/g" $(git grep -l loadblk -- ':!/doc/release-notes/') sed -i "s/m_load_block/m_thread_load/g" $(git grep -l m_load_block) -END VERIFY SCRIPT-
2023-06-28kernel: Add fatalError method to notificationsTheCharlatan
FatalError replaces what previously was the AbortNode function in shutdown.cpp. This commit is part of the libbitcoinkernel project and further removes the shutdown's and, more generally, the kernel library's dependency on interface_ui with a kernel notification method. By removing interface_ui from the kernel library, its dependency on boost is reduced to just boost::multi_index. At the same time it also takes a step towards de-globalising the interrupt infrastructure. Co-authored-by: Russell Yanofsky <russ@yanofsky.org> Co-authored-by: TheCharlatan <seb.kung@gmail.com>
2023-06-28kernel: Pass interrupt reference to chainmanTheCharlatan
This and the following commit seek to decouple the libbitcoinkernel library from the shutdown code. As a library, it should it should have its own flexible interrupt infrastructure without relying on node-wide globals. The commit takes the first step towards this goal by de-globalising `ShutdownRequested` calls in kernel code. Co-authored-by: Russell Yanofsky <russ@yanofsky.org> Co-authored-by: TheCharlatan <seb.kung@gmail.com>
2023-06-27Merge bitcoin/bitcoin#27334: util: implement `noexcept` move assignment & ↵Andrew Chow
move ctor for `prevector` bfb9291a8661fe5b26c14ed755cfa89d27c37110 util: implement prevector's move ctor & move assignment (Martin Leitner-Ankerl) fffc86f49f4eeb811b8438bc1b7f8d9e05882c6f test: CScriptCheck is used a lot in std::vector, make sure that's efficient (Martin Leitner-Ankerl) 81f67977f543faca2dcc35846f73e2917375ae79 util: prevector's move ctor and move assignment is `noexcept` (Martin Leitner-Ankerl) d380d2877ed45cf1e75a87d822b30e4e1e21e3d4 bench: Add benchmark for prevector usage in std::vector (Martin Leitner-Ankerl) Pull request description: `prevector`'s move assignment and move constructor were not `noexcept`, which makes it inefficient to use inside STL containers like `std::vector`. That's the case e.g. for `CScriptCheck`. This PR adds `noexcept`, and also implements the move assignment & ctor, which makes it quite a bit more efficient to use prevector in an std::vector. The PR also adds a benchmark which grows an `std::vector` by adding `prevector` objects to it. merge-base: | ns/op | op/s | err% | ins/op | cyc/op | IPC | bra/op | miss% | total | benchmark |--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:---------- | 6,440.29 | 155,272.42 | 0.2% | 40,713.01 | 20,473.84 | 1.989 | 7,132.01 | 0.2% | 0.44 | `PrevectorFillVectorDirectNontrivial` | 3,213.19 | 311,217.35 | 0.7% | 35,373.01 | 10,214.07 | 3.463 | 6,945.00 | 0.2% | 0.43 | `PrevectorFillVectorDirectTrivial` | 34,749.70 | 28,777.23 | 0.1% | 364,396.05 | 110,521.94 | 3.297 | 78,568.37 | 0.1% | 0.43 | `PrevectorFillVectorIndirectNontrivial` | 32,535.05 | 30,736.09 | 0.4% | 353,823.31 | 103,464.53 | 3.420 | 79,871.80 | 0.2% | 0.40 | `PrevectorFillVectorIndirectTrivial` util: prevector's move ctor and move assignment is `noexcept`: | ns/op | op/s | err% | ins/op | cyc/op | IPC | bra/op | miss% | total | benchmark |--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:---------- | 6,603.87 | 151,426.40 | 0.2% | 23,734.01 | 21,009.63 | 1.130 | 2,445.01 | 0.3% | 0.44 | `PrevectorFillVectorDirectNontrivial` | 1,980.93 | 504,813.15 | 0.1% | 13,784.00 | 6,304.32 | 2.186 | 2,258.00 | 0.3% | 0.44 | `PrevectorFillVectorDirectTrivial` | 19,110.54 | 52,327.15 | 0.1% | 139,816.41 | 51,987.72 | 2.689 | 28,512.18 | 0.1% | 0.43 | `PrevectorFillVectorIndirectNontrivial` | 12,334.37 | 81,074.27 | 0.7% | 125,655.12 | 39,253.58 | 3.201 | 27,854.46 | 0.2% | 0.44 | `PrevectorFillVectorIndirectTrivial` util: implement prevector's move ctor & move assignment | ns/op | op/s | err% | ins/op | cyc/op | IPC | bra/op | miss% | total | benchmark |--------------------:|--------------------:|--------:|----------------:|----------------:|-------:|---------------:|--------:|----------:|:---------- | 5,262.66 | 190,018.01 | 0.2% | 20,157.01 | 16,745.26 | 1.204 | 2,445.01 | 0.3% | 0.44 | `PrevectorFillVectorDirectNontrivial` | 1,687.07 | 592,744.35 | 0.2% | 12,742.00 | 5,368.02 | 2.374 | 2,258.00 | 0.3% | 0.44 | `PrevectorFillVectorDirectTrivial` | 17,930.80 | 55,769.95 | 0.1% | 136,237.69 | 47,903.31 | 2.844 | 28,512.02 | 0.2% | 0.42 | `PrevectorFillVectorIndirectNontrivial` | 11,893.75 | 84,077.78 | 0.2% | 126,182.02 | 37,852.91 | 3.333 | 28,152.01 | 0.1% | 0.44 | `PrevectorFillVectorIndirectTrivial` As can be seen, mostly thanks to just `noexcept` the benchmark becomes about 2 times faster because `std::vector` can now use move operations instead of having to fall back to copy everything I had a look at how this change affects the other benchmarks, and they are all pretty much the same, the only noticable difference is `CCheckQueueSpeedPrevectorJob` goes from 364.56ns down to 346.21ns. ACKs for top commit: achow101: ACK bfb9291a8661fe5b26c14ed755cfa89d27c37110 jonatack: > Tested Approach ACK [bfb9291](https://github.com/bitcoin/bitcoin/commit/bfb9291a8661fe5b26c14ed755cfa89d27c37110), ~ACK modulo re-verifying the implementation of the last commit. john-moffett: Approach ACK bfb9291a8661fe5b26c14ed755cfa89d27c37110 theStack: Tested and light code-review ACK bfb9291a8661fe5b26c14ed755cfa89d27c37110 Tree-SHA512: 242995d7cb2f8ebfa73177aa690a505f189d91edeb8cea3e34a41ca507c8cb17c65fe2a4e196fdafc5c6e89b2b2222627bfc9f5c316517de0857b7b5e9c58225
2023-06-15validation: Stricter assumeutxo error handling in InvalidateCoinsDBOnDiskRyan Ofsky
Currently InvalidateCoinsDBOnDisk is calling AbortNode without an error to the caller if it fails. Change it to return just return util::Result, and update the caller to handle the error itself. This causes the secondary error to be shown below the main error instead of the other way around.
2023-06-12Merge bitcoin/bitcoin#27357: validation: Move warningcache to ↵fanquake
ChainstateManager and rename to m_warningcache 552684976b6df34ce563458f73812e6e494e3b0e validation: Move warningcache to ChainstateManager (dimitaracev) Pull request description: Removes `warningcache` and moves it to `ChainstateManager`. Also removes the respective `TODO` completely. ACKs for top commit: ajtowns: ACK 552684976b6df34ce563458f73812e6e494e3b0e dimitaracev: > ACK [5526849](https://github.com/bitcoin/bitcoin/commit/552684976b6df34ce563458f73812e6e494e3b0e) TheCharlatan: ACK 552684976b6df34ce563458f73812e6e494e3b0e ryanofsky: Code review ACK 552684976b6df34ce563458f73812e6e494e3b0e Tree-SHA512: 6869bd7aa4f0b59324e12eb8e3df47f2c9a3f3b0d9b7d45857426ec9e8b71c5573bdcf71db822f8c10aff7d8679a00a4bedc7a256c28f325e744e5d7267b41e9
2023-05-30refactor: Add stop_at_height option in ChainstateManagerTheCharlatan
Remove access to the global gArgs for the stopatheight argument and replace it by adding a field to the existing ChainstateManager Options struct. This should eventually allow users of the ChainstateManager to not rely on the global gArgs and instead pass in their own options.
2023-05-20kernel: Add progress method to notificationsTheCharlatan
This commit is part of the libbitcoinkernel project and seeks to remove the ChainstateManager's and, more generally, the kernel library's dependency on interface_ui with options methods in this and the following few commits. By removing interface_ui from the kernel library, its dependency on boost is reduced to just boost::multi_index.
2023-05-20kernel: Add notification interfaceTheCharlatan
This commit is part of the libbitcoinkernel project and seeks to remove the ChainstateManager's and, more generally, the kernel library's dependency on interface_ui with options methods in this and the following few commits. By removing interface_ui from the kernel library, its dependency on boost is reduced to just boost::multi_index. Define a new kernel notification class with virtual methods for notifying about internal kernel events. Create a new file in the node library for defining a function creating the default set of notification methods such that these do not need to be re-defined all over the codebase. As a first step, add a `blockTip` method, wrapping `uiInterface.NotifyBlockTip`.
2023-04-03Merge bitcoin/bitcoin#27254: refactor: Extract util/fs from util/systemfanquake
00e9b97f37e0bdf4c647236838c10b68b7ad5be3 refactor: Move fs.* to util/fs.* (TheCharlatan) 106b46d9d25b5228ef009fbbe6f9a7ae35090d15 Add missing fs.h includes (TheCharlatan) b202b3dd6393b415fa68e18dc49c9431dc6b58b2 Add missing cstddef include in assumptions.h (TheCharlatan) 18fb36367a28819bd5ab402344802796a1248979 refactor: Extract util/fs_helpers from util/system (Ben Woosley) Pull request description: This pull request is part of the `libbitcoinkernel` project https://github.com/bitcoin/bitcoin/issues/24303 https://github.com/bitcoin/bitcoin/projects/18 and more specifically its "Step 2: Decouple most non-consensus code from libbitcoinkernel". This commit was originally authored by empact and is taken from its parent PR #25152. #### Context There is an ongoing effort to decouple the `ArgsManager` used for command line parsing user-provided arguments from the libbitcoinkernel library (https://github.com/bitcoin/bitcoin/pull/25290, https://github.com/bitcoin/bitcoin/pull/25487, https://github.com/bitcoin/bitcoin/pull/25527, https://github.com/bitcoin/bitcoin/pull/25862, https://github.com/bitcoin/bitcoin/pull/26177, and https://github.com/bitcoin/bitcoin/pull/27125). The `ArgsManager` is defined in `system.h`. A similar pull request extracting functionality from `system.h` has been merged in https://github.com/bitcoin/bitcoin/pull/27238. #### Changes Next to providing better code organization, this PR removes some reliance of the tree of libbitcoinkernel header includes on `system.h` (and thus the `ArgsManager` definition) by moving filesystem related functions out of the `system.*` files. There is already a pair of `fs.h` / `fs.cpp` in the top-level `src/` directory. They were not combined with the files introduced here, to keep the patch cleaner and more importantly because they are often included without the utility functions. The new files are therefore named `fs_helpers` and the existing `fs` files are moved into the util directory. Further commits splitting more functionality out of `system.h` are still in #25152 and will be submitted in separate PRs once this PR has been processed. ACKs for top commit: hebasto: ACK 00e9b97f37e0bdf4c647236838c10b68b7ad5be3 Tree-SHA512: 31422f148d14ba3c843b99b1550a6fd77c77f350905ca324f93d4f97b652246bc58fa9696c64d1201979cf88733e40be02d262739bb7d417cf22bf506fdb7666