aboutsummaryrefslogtreecommitdiff
path: root/src/validation.cpp
AgeCommit message (Collapse)Author
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] Cache merkle root and witness commitment checksdergoegge
Slight performance improvement by avoiding duplicate work.
2024-02-27[validation] Merkle root malleation should be caught by IsBlockMutateddergoegge
2024-02-27[validation] Introduce IsBlockMutateddergoegge
2024-02-27[refactor] Cleanup merkle root checksdergoegge
2024-02-27[validation] Isolate merkle root checksdergoegge
2024-02-20Merge bitcoin/bitcoin#29404: refactor: bitcoin-config.h includes cleanupfanquake
9d1dbbd4ceb8c04340927f5127195dc306adf3fc scripted-diff: Fix bitcoin_config_h includes (TheCharlatan) Pull request description: As mentioned in https://github.com/bitcoin/bitcoin/pull/26924#issuecomment-1403449932 and https://github.com/bitcoin/bitcoin/pull/29263#issuecomment-1922334399, it is currently not safe to remove `bitcoin-config.h` includes from headers because some unrelated file might be depending on it. See also #26972 for discussion. Solve this by including the file directly everywhere it's required, regardless of whether or not it's already included by another header. There should be no functional change here, but it will allow us to safely remove includes from headers in the future. ~I'm afraid it's a bit tedious to reproduce these commits, but it's reasonably straightforward:~ Edit: See note below ```bash # All commands executed from the src/ subdir. # Collect all tokens from bitcoin-config.h.in # Isolate the tokens and remove blank lines # Replace newlines with | and remove the last trailing one # Collect all files which use these tokens # Filter out subprojects (proper forwarding can be verified from Makefiles) # Filter out .rc files # Save to a text file git grep -E -l `grep undef config/bitcoin-config.h.in | cut -d" " -f2 | grep -v '^$' | tr '\n' '|' | sed 's/|$//'` | grep -v -e "^leveldb/" -e "^secp256k1/" -e "^crc32c/" -e "^minisketch/" -e "^Makefile" -e "\.rc$" > files-with-config-include.txt # Find all files from the above list which don't include bitcoin-config.h git grep -L -E "config/bitcoin-config.h" -- `cat files-with-config-include.txt` # Include them manually with the exception of some files in crypto: # crypto/sha256_arm_shani.cpp crypto/sha256_avx2.cpp crypto/sha256_sse41.cpp crypto/sha256_x86_shani.cpp # These are exceptions which don't use bitcoin-config.h, rather the Makefile.am adds these cppflags manually. # Commit changes. This should match the first commit of this PR. # Use the same search as above to find all files which DON'T use any config tokens git grep -E -L `grep undef config/bitcoin-config.h.in | cut -d" " -f2 | grep -v '^$' | tr '\n' '|' | sed 's/|$//'` | grep -v -e "^leveldb/" -e "^secp256k1/" -e "^crc32c/" -e "^minisketch/" -e "^Makefile" -e "\.rc$" > files-without-config-include.txt # Manually remove the includes and commit changes. This should match the second commit of this PR. ``` Edit: I'll keep this old description for posterity, but the manual approach has been replaced with a scripted diff from TheCharlatan ACKs for top commit: maflcko: ACK 9d1dbbd4ceb8c04340927f5127195dc306adf3f 🚪 TheCharlatan: ACK 9d1dbbd4ceb8c04340927f5127195dc306adf3fc hebasto: ACK 9d1dbbd4ceb8c04340927f5127195dc306adf3fc, I have reviewed the code and it looks OK. fanquake: ACK 9d1dbbd4ceb8c04340927f5127195dc306adf3fc Tree-SHA512: f11ddc4ae6a887f96b954a6b77f310558ddb271088a3fda3edc833669c4251b7f392515224bbb8e5f67eb2c799b4ffed3b07d96454e82ec635c686d0df545872
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-02-09Merge bitcoin/bitcoin#28948: v3 transaction policy for anti-pinningAva Chow
29029df5c700e6940c712028303761d91ae15847 [doc] v3 signaling in mempool-replacements.md (glozow) e643ea795e4b6fea4a6bbb3d72870ee6a4c836b1 [fuzz] v3 transactions and sigop-adjusted vsize (glozow) 1fd16b5c62f54c7f4c60122acd65d852f63d1e8b [functional test] v3 transaction submission (glozow) 27c8786ba918a42c860e6a50eaee9fdf56d7c646 test framework: Add and use option for tx-version in MiniWallet methods (MarcoFalke) 9a1fea55b29fe025355b06b45e3d77d192acc635 [policy/validation] allow v3 transactions with certain restrictions (glozow) eb8d5a2e7d939dd3ee683486e98702079e0dfcc0 [policy] add v3 policy rules (glozow) 9a29d470fbb62bbb27d517efeafe46ff03c25f54 [rpc] return full string for package_msg and package-error (glozow) 158623b8e0726dff7eae4288138f1710e727db9c [refactor] change Workspace::m_conflicts and adjacent funcs/structs to use Txid (glozow) Pull request description: See #27463 for overall package relay tracking. Delving Bitcoin discussion thread: https://delvingbitcoin.org/t/v3-transaction-policy-for-anti-pinning/340 Delving Bitcoin discussion for LN usage: https://delvingbitcoin.org/t/lightning-transactions-with-v3-and-ephemeral-anchors/418 Rationale: - There are various pinning problems with RBF and our general ancestor/descendant limits. These policies help mitigate many pinning attacks and make package RBF feasible (see #28984 which implements package RBF on top of this). I would focus the most here on Rule 3 pinning. [1][2] - Switching to a cluster-based mempool (see #27677 and #28676) requires the removal of CPFP carve out, which applications depend on. V3 + package RBF + ephemeral anchors + 1-parent-1-child package relay provides an intermediate solution. V3 policy is for "Priority Transactions." [3][4] It allows users to opt in to more restrictive topological limits for shared transactions, in exchange for the more robust fee-bumping abilities that offers. Even though we don't have cluster limits, we are able to treat these transactions as having as having a maximum cluster size of 2. Immediate benefits: - You can presign a transaction with 0 fees (not just 1sat/vB!) and add a fee-bump later. - Rule 3 pinning is reduced by a significant amount, since the attacker can only attach a maximum of 1000vB to your shared transaction. This also enables some other cool things (again see #27463 for overall roadmap): - Ephemeral Anchors - Package RBF for these 1-parent-1-child packages. That means e.g. a commitment tx + child can replace another commitment tx using the child's fees. - We can transition to a "single anchor" universe without worrying about package limit pinning. So current users of CPFP carve out would have something else to use. - We can switch to a cluster-based mempool [5] (#27677 #28676), which removes CPFP carve out [6]. [1]: Original mailing list post and discussion about RBF pinning problems https://gist.github.com/glozow/25d9662c52453bd08b4b4b1d3783b9ff, https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2022-January/019817.html [2]: A FAQ is "we need this for cluster mempool, but is this still necessary afterwards?" There are some pinning issues that are fixed here and not fully fixed in cluster mempool, so we will still want this or something similar afterward. [3]: Mailing list post for v3 https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2022-September/020937.html [4]: Original PR #25038 also contains a lot of the discussion [5]: https://delvingbitcoin.org/t/an-overview-of-the-cluster-mempool-proposal/393/7 [6]: https://delvingbitcoin.org/t/an-overview-of-the-cluster-mempool-proposal/393#the-cpfp-carveout-rule-can-no-longer-be-supported-12 ACKs for top commit: sdaftuar: ACK 29029df5c700e6940c712028303761d91ae15847 achow101: ACK 29029df5c700e6940c712028303761d91ae15847 instagibbs: ACK 29029df5c700e6940c712028303761d91ae15847 modulo that Tree-SHA512: 9664b078890cfdca2a146439f8835c9d9ab483f43b30af8c7cd6962f09aa557fb1ce7689d5e130a2ec142235dbc8f21213881baa75241c5881660f9008d68450
2024-02-08[policy/validation] allow v3 transactions with certain restrictionsglozow
Co-authored-by: Suhas Daftuar <sdaftuar@gmail.com>
2024-02-01refactor: Fix timedata includesMarcoFalke
2024-01-31Merge bitcoin/bitcoin#28956: Nuke adjusted time from validation (attempt 2)Ava Chow
ff9039f6ea876bab2c40a06a93e0dd087f445fa2 Remove GetAdjustedTime (dergoegge) Pull request description: This picks up parts of #25908. The use of adjusted time is removed from validation code while the warning to users if their clock is out of sync with the rest of the network remains. ACKs for top commit: naumenkogs: ACK ff9039f6ea876bab2c40a06a93e0dd087f445fa2 achow101: ACK ff9039f6ea876bab2c40a06a93e0dd087f445fa2 maflcko: lgtm ACK ff9039f6ea876bab2c40a06a93e0dd087f445fa2 🤽 stickies-v: ACK ff9039f6ea876bab2c40a06a93e0dd087f445fa2 Tree-SHA512: d1f6b9445c236915503fd2ea828f0d3b92285a5dbc677b168453276115e349972edbad37194d8becd9136d8e7219b576af64ec51c72bdb1923e57e405c0483fc
2024-01-23validation: move nChainTx assert down in CheckBlockIndexMartin Zumsande
There is a designated section meant for the actual consistency checks, marked by a comment.
2024-01-23doc: fix checkblockindex commentsMartin Zumsande
These exceptions are not related to situations specific to tests, but are required in general: Without the first check CheckBlockindex could fail for blocks where we only know the header. Without the second, it could fail when blocks are received out of order.
2024-01-16Merge bitcoin/bitcoin#28791: snapshots: don't core dump when running ↵Ava Chow
-checkblockindex after `loadtxoutset` cdc6ac4126b31426261605a757c52ea2dbfb2a81 snapshots: don't core dump when running -checkblockindex after `loadtxoutset` (Mark Friedenbach) Pull request description: Transaction counts aren't known for block history loaded from a snapshot. If you start with `-checkblockindex` after loading a snapshot, the bitcoin daemon will core dump. The test suite does not check for this because all the snapshots have no non-coinbase transactions (all blocks prior to the snapshot are assumed to have `nTx = 1`). Recommend for backport to 26.x ACKs for top commit: fjahr: utACK cdc6ac4126b31426261605a757c52ea2dbfb2a81 achow101: ACK cdc6ac4126b31426261605a757c52ea2dbfb2a81 pablomartin4btc: tACK cdc6ac4126b31426261605a757c52ea2dbfb2a81 Tree-SHA512: f7488a85cc29056e2ac443ce8f34aea4dfde6ba246efce82235d6a4dca2dca4344f07b93c93424b4addcb83e4cb2ae49a3ebb37d89840d42d2aeea35904cab04
2024-01-16[refactor] change Workspace::m_conflicts and adjacent funcs/structs to use Txidglozow
It's preferable to use type-safe transaction identifiers to avoid confusing txid and wtxid. The next commit will add a reference to this set; we use this opportunity to change it to Txid ahead of time instead of adding new uses of uint256.
2024-01-05Remove GetAdjustedTimedergoegge
2023-12-17wallet, mempool: propagete `checkChainLimits` error message to walletismaelsadeeq
Update CheckPackageLimits to use util::Result to pass the error message instead of out parameter. Also update test to reflect the error message from `CTxMempool` `CheckPackageLimits` output.
2023-12-01Merge bitcoin/bitcoin#28368: Fee Estimator updates from Validation ↵Andrew Chow
Interface/CScheduler thread 91504cbe0de2b74ef1aa2709761aaf0597ec66a2 rpc: `SyncWithValidationInterfaceQueue` on fee estimation RPC's (ismaelsadeeq) 714523918ba2b853fc69bee6b04a33ba0c828bf5 tx fees, policy: CBlockPolicyEstimator update from `CValidationInterface` notifications (ismaelsadeeq) dff5ad3b9944cbb56126ba37a8da180d1327ba39 CValidationInterface: modify the parameter of `TransactionAddedToMempool` (ismaelsadeeq) 91532bd38223d7d04166e05de11d0d0b55e60f13 tx fees, policy: update `CBlockPolicyEstimator::processBlock` parameter (ismaelsadeeq) bfcd401368fc0dc43827a8969a37b7e038d5ca79 CValidationInterface, mempool: add new callback to `CValidationInterface` (ismaelsadeeq) 0889e07987294d4ef2814abfca16d8e2a0c5f541 tx fees, policy: cast with static_cast instead of C-Style cast (ismaelsadeeq) a0e3eb7549d2ba4dd3af12b9ce65e29158f59078 tx fees, policy: bugfix: move `removeTx` into reason != `BLOCK` condition (ismaelsadeeq) Pull request description: This is an attempt to #11775 This Pr will enable fee estimator to listen to ValidationInterface notifications to process new transactions added and removed from the mempool. This PR includes the following changes: - Added a new callback to the Validation Interface `MempoolTransactionsRemovedForConnectedBlock`, which notifies listeners about the transactions that have been removed due to a new block being connected, along with the height at which the transactions were removed. - Modified the `TransactionAddedToMempool` callback parameter to include additional information about the transaction needed for fee estimation. - Updated `CBlockPolicyEstimator` to process transactions using` CTransactionRef` instead of `CTxMempoolEntry.` - Implemented the `CValidationInterface` interface in `CBlockPolicyEstimater` and overridden the `TransactionAddedToMempool`, `TransactionRemovedFromMempool`, and `MempoolTransactionsRemovedForConnectedBlock` methods to receive updates from their notifications. Prior to this PR, the fee estimator updates from the mempool, i.e whenever a new block is connected all transactions in the block that are in our mempool are going to be removed using the `removeForBlock` function in `txmempool.cpp`. This removal triggered updates to the fee estimator. As a result, the fee estimator would block mempool's `cs` until it finished updating every time a new block was connected. Instead of being blocked only on mempool tx removal, we were blocking on both tx removal and fee estimator updating. If we want to further improve fee estimation, or add heavy-calulation steps to it, it is currently not viable as we would be slowing down block relay in the process This PR is smaller in terms of the changes made compared to #11775, as it focuses solely on enabling fee estimator updates from the validationInterface/cscheduler thread notifications. I have not split the validation interface because, as I understand it, the rationale behind the split in #11775 was to have `MempoolInterface` signals come from the mempool and `CValidationInterface` events come from validation. I believe this separation can be achieved in a separate refactoring PR when the need arises. Also left out some commits from #11775 - Some refactoring which are no longer needed. - Handle reorgs much better in fee estimator. - Track witness hash malleation in fee estimator I believe they are a separate change that can come in a follow-up after this. ACKs for top commit: achow101: ACK 91504cbe0de2b74ef1aa2709761aaf0597ec66a2 TheCharlatan: Re-ACK 91504cbe0de2b74ef1aa2709761aaf0597ec66a2 willcl-ark: ACK 91504cbe0de2b74ef1aa2709761aaf0597ec66a2 Tree-SHA512: 846dfb9da57a8a42458827b8975722d153907fe6302ad65748d74f311e1925557ad951c3d95fe71fb90ddcc8a3710c45abb343ab86b88780871cb9c38c72c7b1
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-28Merge bitcoin/bitcoin#28579: refactor: Remove redundant checks in ↵Andrew Chow
compat/assumptions.h fa1a38470697796a1a67397a815c8f8256f59224 Move compat.h include from system.h to system.cpp (MarcoFalke) 88887531b704f3943fdb33abbdd5378ecfeee14f Move compat/assumptions.h include to one place that actually needs it (MarcoFalke) 77774110f4dd591a71441851813d59c03c9e3c78 Remove __cplusplus from compat/assumptions.h (MarcoFalke) faa3d4f1d8ecff444be53215d72e32d71d9ce138 Remove duplicate NDEBUG check from compat/assumptions.h (MarcoFalke) Pull request description: Generally, compile-time checks should be close to the code that use them. Especially, since `compat/assumptions.h` is only included in one place, where iwyu suggests to remove it. Fix all issues: * The `NDEBUG` check is used in `util/check`, so it is redundant in `compat/assumptions.h`. * The `__cplusplus` check is redundant with `doc/dependencies.md` (see commit message). * Add missing `// IWYU pragma: keep` to avoid removing the include by accident. ACKs for top commit: achow101: ACK fa1a38470697796a1a67397a815c8f8256f59224 TheCharlatan: re-ACK fa1a38470697796a1a67397a815c8f8256f59224 theuni: ACK fa1a38470697796a1a67397a815c8f8256f59224 Tree-SHA512: f8b6db84be5d8844a2267345c0b1405fcbc39b8b5eeaa24db5b8412a74145fe44cf188b6b0c39cc2b062690ed37ca5b4662473484afe28dbec6469e79961389b
2023-11-24Merge bitcoin/bitcoin#28922: Use Txid in COutpointfanquake
9e58c5bcd96e7ff2062274868814ccae0626589e Use Txid in COutpoint (dergoegge) Pull request description: This PR changes the type of the hash of a transaction outpoint from `uint256` to `Txid`. ACKs for top commit: Sjors: ACK 9e58c5bcd96e7ff2062274868814ccae0626589e stickies-v: ACK 9e58c5bcd96e7ff2062274868814ccae0626589e. A sizeable diff, but very straightforward changes. Didn't see anything controversial. Left a few nits, but nothing blocking, only if you have to retouch. TheCharlatan: ACK 9e58c5bcd96e7ff2062274868814ccae0626589e Tree-SHA512: 58f61ce1c58668f689513e62072a7775419c4d5af8f607669cd8cdc2e7be9645ba14af7f9e2d65da2670da3ec1ce7fc2a744037520caf799aba212fd1ac44b34
2023-11-22tx fees, policy: CBlockPolicyEstimator update from `CValidationInterface` ↵ismaelsadeeq
notifications `CBlockPolicyEstimator` will implement `CValidationInterface` and subscribe to its notification to process transactions added and removed from the mempool. Re-delegate calculation of `validForFeeEstimation` from validation to fee estimator. Also clean up the validForFeeEstimation arg thats no longer needed in `CTxMempool`. Co-authored-by: Matt Corallo <git@bluematt.me>
2023-11-22CValidationInterface: modify the parameter of `TransactionAddedToMempool`ismaelsadeeq
Create a new struct `NewMempoolTransactionInfo` that will be used as the new parameter of `TransactionAddedToMempool` callback.
2023-11-21Use Txid in COutpointdergoegge
2023-11-18blockstorage: switch from CAutoFile to AutoFileAnthony Towns
Also bump includes per suggestions from iwyu.
2023-11-14Use ParamsWrapper for witness serializationAnthony Towns
2023-11-13Merge bitcoin/bitcoin#28391: refactor: Simplify CTxMempool/BlockAssembler ↵fanquake
fields, remove some external mapTx access 4dd94ca18f6fc843137ffca3e6d3e97e4f19377b [refactor] remove access to mapTx in validation_block_tests (TheCharlatan) d0cd2e804ec9b278ed9699c2ae48574b1c1613b1 [refactor] rewrite BlockAssembler inBlock and failedTx as sets of txids (glozow) 55b0939cab49d50ca5bc59105b669e379d5e7f6c scripted-diff: rename vTxHashes to txns_randomized (TheCharlatan) a03aef9cec35b0d03aa63d7e8093f0420cd4b40b [refactor] rewrite vTxHashes as a vector of CTransactionRef (glozow) 938643c3b2b8e7b9aec1df34a2f8a95d616d8dd5 [refactor] remove access to mapTx in validation.cpp (glozow) 333367a9407701b5077e2457b1a6aa8ff5e4934b [txmempool] make CTxMemPoolEntry::lockPoints mutable (glozow) 1bf4855016e777dd8b424fe01750f9e3e97931a2 [refactor] use CheckPackageLimits for checkChainLimits (glozow) dbc5bdbf595e9dd0330493645ebff0b8696192a3 [refactor] remove access to mapTx.find in mempool_tests.cpp (glozow) f80909e7a31523d8f197fd650b138b9f228cd13f [refactor] remove access to mapTx in blockencodings_tests.cpp (glozow) 8892d6b744e3cbda2cf93721f573ffa7017bd898 [refactor] remove access to mapTx from rpc/mempool.cpp (glozow) fad61aa56189f98d97af1d6f70c4eb46b8f98bf0 [refactor] get wtxid from entry instead of vTxHashes (glozow) 9cd8cafb77563243af19c95e396c2fb4fc3758df [refactor] use exists() instead of mapTx.find() (glozow) 14804699e59794e61dcfb02ff1971db96e9a06ce [refactor] remove access to mapTx from policy/rbf.cpp (glozow) 1c6a73abbd1fb773c7d0036beb952b95dde8e38b [refactor] Add helper for retrieving mempool entry (TheCharlatan) 453b4813ebc74859864803e9972b58e4be76a4d6 [refactor] Add helper for iterating through mempool entries (stickies-v) Pull request description: Motivation * It seems preferable to use stdlib data structures instead of boost if they can achieve close to the same thing. * Code external to mempool should ideally use its public helper methods instead of accessing `mapTx` or its iterators directly. * Reduce the number of complex boost multi index type interactions * Also see #28335 for further context/motivation. This PR together with #28385 simplifies that one. Overview of things done in this PR: * Make `vTxHashes` a vector of transaction references instead of a pair of transaction hash and iterator. The trade off here is that the data is retrieved on the fly with `GetEntry` instead of being cached in `vTxHashes`. * Introduce `GetEntry` helper method to replace the more involved `GetIter` where applicable * Replace `mapTx` access with `CTxMemPool` helper methods * Simplify `checkChainLimits` call in `node/interfaces.cpp` * Make `CTxMemPoolEntry`s `lockPoints`mutable such that they can be changed with a const iterator directly instead of going through `mapTx` * Make `BlockAssembler`'s `inBlock` and `failedTx` sets of transaction hashes. ACKs for top commit: glozow: reACK 4dd94ca maflcko: re-ACK 4dd94ca18f6fc843137ffca3e6d3e97e4f19377b 👝 stickies-v: re-ACK 4dd94ca18f6fc843137ffca3e6d3e97e4f19377b Tree-SHA512: c4d043f2186e4fde337591883fac66cade3058173987b49502bd65cecf69207a3df1077f6626809652ab63230013167b7f39a2b39f1c5166959e5495df57065f
2023-11-10[refactor] remove access to mapTx in validation.cppglozow
2023-11-10[refactor] use exists() instead of mapTx.find()glozow
2023-11-10[refactor] Add helper for retrieving mempool entryTheCharlatan
In places where the iterator is only needed for accessing the actual entry, it should not be required to first retrieve the iterator.
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[validation] add TxValidationResult::TX_RECONSIDERABLE, TX_UNKNOWNglozow
With package validation rules, transactions that fail individually may sometimes be eligible for reconsideration if submitted as part of a (different) package. For now, that includes trasactions that failed for being too low feerate. Add a new TxValidationResult type to distinguish these failures from others. In the next commits, we will abort package validation if a tx fails for any other reason. In the future, we will also decide whether to cache failures in recent_rejects based on this result (we won't want to reject a package containing a transaction that was rejected previously for being low feerate). Package validation also sometimes elects to skip some transactions when it knows the package will not be submitted in order to quit sooner. Add a result to specify this situation; we also don't want to cache these as rejections.
2023-11-06[refactor] use Wtxid for m_wtxids_fee_calculationsglozow
2023-11-04snapshots: don't core dump when running -checkblockindex after `loadtxoutset`Mark Friedenbach
2023-11-03Merge bitcoin/bitcoin#28758: refactors for subpackage evaluationfanquake
b5a60abe8783852f5b31bc1e63b5836530410e65 MOVEONLY: CleanupTemporaryCoins into its own function (glozow) 10c0a8678cd28e7f0715e6cfa3e651903e4ad4aa [test util] CreateValidTransaction multi-in/out, configurable feerate, signal BIP125 (glozow) 6ff647a7e0d85040a6033047c5cf84f8f22b1c65 scripted-diff: rename CheckPackage to IsWellFormedPackage (glozow) da9aceba217bbded6909f06144eaa1e1a4ebcb69 [refactor] move package checks into helper functions (glozow) Pull request description: This is part of #27463. It splits off the more trivial changes from #26711 for ease of review, as requested in https://github.com/bitcoin/bitcoin/pull/26711#issuecomment-1786392253. - Split package sanitization in policy/packages.h into helper functions - Add some tests for its quirks (https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1340521597) - Rename `CheckPackage` to `IsPackageWellFormed` - Improve the `CreateValidTransaction` unit test utility to: - Configure the target feerate and return the fee paid - Signal BIP125 on transactions to enable RBF tests - Allow the specification of multiple inputs and outputs - Move `CleanupTemporaryCoins` into its own function to be reused later without duplication ACKs for top commit: dergoegge: Code review ACK b5a60abe8783852f5b31bc1e63b5836530410e65 instagibbs: ACK b5a60abe8783852f5b31bc1e63b5836530410e65 Tree-SHA512: 39d67a5f0041e381f0d0f802a98ccffbff11e44daa3a49611189d6306b03f18613d5ff16c618898d490c97a216753e99e0db231ff14d327f92c17ae4d269cfec
2023-11-02Merge bitcoin/bitcoin#28530: tests, bug fix: DisconnectedBlockTransactions ↵glozow
rewrite followups 9b3da70bd06b45482e7211aa95637a72bd115553 [test] DisconnectedBlockTransactions::DynamicMemoryUsage (glozow) b2d04479647af64ad7cf5ebfb6175251b2f6b72e bugfix: correct DisconnectedBlockTransactions memory usage (stickies-v) f4254e209801d6a790b5f0c251c0b32154a4e3cc assume duplicate transactions are not added to `iters_by_txid` (ismaelsadeeq) 29eb219c1247993378fce06c8f71aab20736c237 move only: move implementation code to disconnected_transactions.cpp (ismaelsadeeq) 81dfeddea70ae5feeaf79062585c2ff9f33c0ca3 refactor: update `MAX_DISCONNECTED_TX_POOL` from kb to bytes (ismaelsadeeq) Pull request description: This PR is a follow-up to fix review comments and a bugfix from #28385 The PR - Updated `DisconnectedBlockTransactions`'s `MAX_DISCONNECTED_TX_POOL` from kb to bytes. - Moved `DisconnectedBlockTransactions` implementation code to `kernel/disconnected_transactions.cpp`. - `AddTransactionsFromBlock` now assume duplicate transactions are not passed by asserting after inserting each transaction to `iters_by_txid`. - Included a Bug fix: In the current master we are underestimating the memory usage of `DisconnectedBlockTransactions`. * When adding and subtracting `cachedInnerUsage` we call `RecursiveDynamicUsage` with `CTransaction` which invokes this [`RecursiveDynamicUsage(const CTransaction& tx)`](https://github.com/bitcoin/bitcoin/blob/6e721c923c87abdb8d99674093d08d8c17bc52c2/src/core_memusage.h#L32) version of `RecursiveDynamicUsage`, the output of that call only account for the memory usage of the inputs and outputs of the `CTransaction`, this omits the memory usage of the `CTransaction` object and the control block. * This PR fixes this bug by calling `RecursiveDynamicUsage` with `CTransactionRef` when adding and subtracting `cachedInnerUsage` which invokes [`RecursiveDynamicUsage(const std::shared_ptr<X>& p)`](https://github.com/bitcoin/bitcoin/blob/6e721c923c87abdb8d99674093d08d8c17bc52c2/src/core_memusage.h#L67) version of `RecursiveDynamicUsage` the output of the calculation accounts for the` CTransaction` object, the control blocks, inputs and outputs memory usage. * see [comment ](https://github.com/bitcoin/bitcoin/pull/28385#discussion_r1322948452) - Added test for DisconnectedBlockTransactions memory limit. ACKs for top commit: stickies-v: ACK 9b3da70bd06b45482e7211aa95637a72bd115553 - nice work! BrandonOdiwuor: re ACK 9b3da70bd06b45482e7211aa95637a72bd115553 glozow: ACK 9b3da70bd06b45482e7211aa95637a72bd115553 Tree-SHA512: 69b9595d09f4d0209038f97081d790cea92ccf63efb94e9e372749979fcbe527f7f17a8e454720cedd12021be0c8e11cf99874625d3dafd9ec602b12dbeb4098
2023-11-01MOVEONLY: CleanupTemporaryCoins into its own functionglozow
Avoid duplicate code. This will be used at the end of every AcceptSubPackage and after PreChecks loop in AcceptPackage.
2023-11-01scripted-diff: rename CheckPackage to IsWellFormedPackageglozow
-BEGIN VERIFY SCRIPT- sed -i 's/CheckPackage(/IsWellFormedPackage(/g' $(git grep -l CheckPackage) -END VERIFY SCRIPT-
2023-11-01[refactor] move package checks into helper functionsglozow
This allows IsSorted() and IsConsistent() to be used by themselves. IsSorted() with a precomputed set is used so that we don't create this set multiple times.
2023-11-01refactor: Remove unused circular include dependency from validation.cppMarcoFalke
2023-10-30Merge bitcoin/bitcoin#28737: doc: Fix bugprone-lambda-function-name errorsfanquake
faa769db5a4c16fd171e9a39c33e245db4e7c134 Fix bugprone-lambda-function-name errors (MarcoFalke) Pull request description: Inside a lambda, `__func__` will evaluate to something like `"operator()"`. Fix this by either removing it, or by using the real name. https://clang.llvm.org/extra/clang-tidy/checks/bugprone/lambda-function-name.html ACKs for top commit: TheCharlatan: ACK faa769db5a4c16fd171e9a39c33e245db4e7c134 darosior: utACK faa769db5a4c16fd171e9a39c33e245db4e7c134 Tree-SHA512: 0b562bd4ebd7f46ca3ebabeee67851ad30bd522fa57e5010e833b163664e51f5df645ff9ca35d22c3479fb27d9267d4e5d0d417d42729bf3ccf80d7944970e4e
2023-10-26Merge bitcoin/bitcoin#28107: util: Type-safe transaction identifiersAndrew Chow
940a49978c70453e1aaf2c4a0bcb382872b844a5 Use type-safe txid types in orphanage (dergoegge) ed70e6501648466b9ca91a39b83775363e9a726d Introduce types for txids & wtxids (dergoegge) cdb14d79e809bf7d1612b21b554a9fcfb2ab1c91 [net processing] Use HasWitness over comparing (w)txids (dergoegge) Pull request description: We currently have two different identifiers for transactions: `txid` (refering to the hash of a transaction without witness data) and `wtxid` (referring to the hash of a transaction including witness data). Both are typed as `uint256` which could lead to type-safety bugs in which one transaction identifier type is passed where the other would be expected. This PR introduces explicit `Txid` and `Wtxid` types that (if used) would cause compilation errors for such type confusion bugs. (Only the orphanage is converted to use these types in this PR) ACKs for top commit: achow101: ACK 940a49978c70453e1aaf2c4a0bcb382872b844a5 stickies-v: ACK 940a49978c70453e1aaf2c4a0bcb382872b844a5 hebasto: ACK 940a49978c70453e1aaf2c4a0bcb382872b844a5, I have reviewed the code and it looks OK. instagibbs: re-ACK 940a49978c70453e1aaf2c4a0bcb382872b844a5 BrandonOdiwuor: re-ACK 940a49978c70453e1aaf2c4a0bcb382872b844a5 glozow: reACK 940a49978c Tree-SHA512: 55298d1c2bb82b7a6995e96e554571c22eaf4a89fb2a4d7a236d70e0f625e8cca62ff2490e1c179c47bd93153fe6527b56870198f026f5ee7753d64d7a424c92
2023-10-26Fix bugprone-lambda-function-name errorsMarcoFalke
Can be reviewed with --color-moved=dimmed-zebra
2023-10-20assumeutxo: Check deserialized coins for out of range valuesFabian Jahr
2023-10-20Merge bitcoin/bitcoin#28569: log: Don't log cache rebalancing in absense of ↵fanquake
a snapshot chainstate ec84f999f1408b7f1ff4498f78c33b34c30e934c log: Don't log cache rebalancing in absense of a snapshot chainstate (Fabian Jahr) Pull request description: I have noticed that this log now is always printed, even if there is no snapshot chainstate present or even was present. I think this is confusing to users that have never even thought about using assumeutxo since in that case the rebalancing is just ensuring the normal environment with one chainstate. So I suggest we don't log in absence of a snapshot chainstate. We could also think about rewording the message instead but I think this is simpler. ACKs for top commit: stickies-v: utACK ec84f999f1408b7f1ff4498f78c33b34c30e934c glozow: concept ACK ec84f999f1408b7f1ff4498f78c33b34c30e934c, don't have opinions other than removing confusing log theStack: utACK ec84f999f1408b7f1ff4498f78c33b34c30e934c Tree-SHA512: 30bbfc648e7c788106f78d52e47a3aa1e1874f65d13743643dc50bcf7f450d8330711ff9fdeac361722542da6051533153829c6d49033227ed315e111afc899f
2023-10-20log: Don't log cache rebalancing in absense of a snapshot chainstateFabian Jahr
2023-10-13refactor: update `MAX_DISCONNECTED_TX_POOL` from kb to bytesismaelsadeeq
2023-10-12Introduce types for txids & wtxidsdergoegge
2023-10-12tidy: modernize-use-emplaceMarcoFalke
generated by cgit v1.2.3 (git 2.26.2) at 2025-02-28 17:52:21 +0000