aboutsummaryrefslogtreecommitdiff
path: root/src/test/mempool_tests.cpp
AgeCommit message (Collapse)Author
2024-04-25refactor: Drop util::Result operator=Ryan Ofsky
`util::Result` objects are aggregates that can hold multiple fields with different information. Currently Result objects can only hold a success value of an arbitrary type or a single bilingual_str error message. In followup PR https://github.com/bitcoin/bitcoin/pull/25722, Result objects may be able to hold both success and failure values of different types, plus error and warning messages. Having a Result::operator= assignment operator that completely erases all existing Result information before assigning new information is potentially dangerous in this case. For example, code that looks like it is assigning a warning value could erase previously-assigned success or failure values. Conversely, code that looks like it is just assigning a success or failure value could erase previously assigned error and warning messages. To prevent potential bugs like this, disable Result::operator= assignment operator. It is possible in the future we may want to re-enable operator= in limited cases (such as when implicit conversions are not used) or add a Replace() or Reset() method that mimicks default operator= behavior. Followup PR https://github.com/bitcoin/bitcoin/pull/25722 also adds a Result::Update() method providing another way to update an existing Result object. Co-authored-by: stickies-v <stickies-v@protonmail.com>
2023-11-10[refactor] remove access to mapTx.find in mempool_tests.cppglozow
2023-05-20refactor: Move system from util to common libraryTheCharlatan
Since the kernel library no longer depends on the system file, move it to the common library instead in accordance to the diagram in doc/design/libraries.md.
2023-01-05refactor: add kernel/cs_main.*fanquake
Co-authored-by: Anthony Towns <aj@erisian.com.au>
2023-01-03Merge bitcoin/bitcoin#26289: Use util::Result in for calculating mempool ↵Andrew Chow
ancestors 47c4b1f52ab8d95d7deef83050bad49d1e3e5990 mempool: log/halt when CalculateMemPoolAncestors fails unexpectedly (stickies-v) 5481f65849313ff947f38433b1ac28285a7f7694 mempool: add AssumeCalculateMemPoolAncestors helper function (stickies-v) f911bdfff95eba3793fffaf71a31cc8bfc6f80c9 mempool: use util::Result for CalculateMemPoolAncestors (stickies-v) 66e028f7399b6511f9b73b1cef54b6a6ac38a024 mempool: use util::Result for CalculateAncestorsAndCheckLimits (stickies-v) Pull request description: Upon reviewing the documentation for `CTxMemPool::CalculateMemPoolAncestors`, I noticed `setAncestors` was meant to be an `out` parameter but actually is an `in,out` parameter, as can be observed by adding `assert(setAncestors.empty());` as the first line in the function and running `make check`. This PR fixes this unexpected behaviour and introduces refactoring improvements to make intents and effects of the code more clear. ## Unexpected behaviour This behaviour occurs only in the package acceptance path, currently only triggered by `testmempoolaccept` and `submitpackage` RPCs. In `MemPoolAccept::AcceptMultipleTransactions()`, we first call `PreChecks()` and then `SubmitPackage()` with the same `Workspace ws` reference. `PreChecks` leaves `ws.m_ancestors` in a potentially non-empty state, before it is passed on to `MemPoolAccept::SubmitPackage`. `SubmitPackage` is the only place where `setAncestors` isn't guaranteed to be empty before calling `CalculateMemPoolAncestors`. The most straightforward fix is to just forcefully clear `setAncestors` at the beginning of CalculateMemPoolAncestors, which is done in the first bugfix commit. ## Improvements ### Return value instead of out-parameters This PR updates the function signatures for `CTxMemPool::CalculateMemPoolAncestors` and `CTxMemPool::CalculateAncestorsAndCheckLimits` to use a `util::Result` return type and eliminate both the `setAncestors` `in,out`-parameter as well as the error string. It simplifies the code and makes the intent and effects more explicit. ### Observability There are 7 instances where we currently call `CalculateMemPoolAncestors` without actually checking if the function succeeded because we assume that it can't fail, such as in [miner.cpp](https://github.com/bitcoin/bitcoin/blob/69b10212ea5370606c7a5aa500a70c36b4cbb58f/src/node/miner.cpp#L399). This PR adds a new wrapper `AssumeCalculateMemPoolAncestors` function that logs such unexpected failures, or in case of debug builds even halts the program. It's not crucial to the objective, more of an observability improvement that seems sensible to add on here. ACKs for top commit: achow101: ACK 47c4b1f52ab8d95d7deef83050bad49d1e3e5990 w0xlt: ACK https://github.com/bitcoin/bitcoin/pull/26289/commits/47c4b1f52ab8d95d7deef83050bad49d1e3e5990 glozow: ACK 47c4b1f52ab8d95d7deef83050bad49d1e3e5990 furszy: light code review ACK 47c4b1f5 aureleoules: ACK 47c4b1f52ab8d95d7deef83050bad49d1e3e5990 Tree-SHA512: d908dad00d1a5645eb865c4877cc0bae74b9cd3332a3641eb4a285431aef119f9fc78172d38b55c592168a73dae83242e6af3348815f7b37cbe2d448a3a58648
2022-12-24scripted-diff: Bump copyright headersHennadii Stepanov
-BEGIN VERIFY SCRIPT- ./contrib/devtools/copyright_header.py update ./ -END VERIFY SCRIPT- Commits of previous years: - 2021: f47dda2c58b5d8d623e0e7ff4e74bc352dfa83d7 - 2020: fa0074e2d82928016a43ca408717154a1c70a4db - 2019: aaaaad6ac95b402fe18d019d67897ced6b316ee0
2022-12-13mempool: use util::Result for CalculateMemPoolAncestorsstickies-v
Avoid using setAncestors outparameter, simplify function signatures and avoid creating unused dummy strings.
2022-10-24test: Use type-safe NodeSeconds for TestMemPoolEntryHelperMacroFake
2022-10-18test: Remove unused txmempool include from testsMacroFake
2022-10-05test: use NoLimits() in MempoolIndexingTeststickies-v
The (100, 1000000, 1000, 1000000) limits are arbitrarily high and don't restrict anything, they are just meant to calculate ancestors properly. Using NoLimits() makes this intent more clear and simplifies the code.
2022-06-28mempool: Make GetMinFee() with custom size protectedCarl Dong
The version of GetMinFee() with a custom size specification is and should only be used by tests. Mark it as protected and use a derived class exposing GetMinFee() as public in tests.
2022-06-15scripted-diff: test: Use CTxMemPool in TestingSetupCarl Dong
After this commit, there should be no explicit instantiation of CTxMemPool in src/test other than those in fuzz/ and setup_common -BEGIN VERIFY SCRIPT- find_regex="CTxMemPool\s+([^;({]+)(|\(\)|\{\});" \ && git grep -l -E "$find_regex" -- src/test \ | grep -v -e "^src/test/util/setup_common.cpp$" \ -e "^src/test/fuzz/" \ | xargs sed -i -E "s@$find_regex@CTxMemPool\& \1 = *Assert(m_node.mempool);@g" -END VERIFY SCRIPT-
2022-05-25tree-wide: clang-format CTxMemPool referencesCarl Dong
[META] Do this so that we can more easily grep for all actual instances of CTxMemPool construction.
2022-04-29test: Split MempoolAncestryTests into twoMacroFake
2021-12-30scripted-diff: Bump copyright headersHennadii Stepanov
-BEGIN VERIFY SCRIPT- ./contrib/devtools/copyright_header.py update ./ -END VERIFY SCRIPT- Commits of previous years: * 2020: fa0074e2d82928016a43ca408717154a1c70a4db * 2019: aaaaad6ac95b402fe18d019d67897ced6b316ee0
2021-11-19scripted-diff: Use clang-tidy syntax for C++ named argumentsMarcoFalke
-BEGIN VERIFY SCRIPT- perl -0777 -pi -e 's:((\(|\{|,)(\n| )*)\/\* ?([^=* ]+) ?\*\/ ?:\1/*\4=*/:g' $( git ls-files ./src/test ./src/wallet/test ) -END VERIFY SCRIPT-
2021-10-21[mempool] delete exists(uint256) functionglozow
Allowing callers to pass in a uint256 (which could be txid or wtxid) but then always assuming that it's a txid is a footgunny interface.
2021-04-14test: Reset mocktime in the common setupMarcoFalke
Doing it there will reduce code bloat and also ensure no test can "forget" to reset it
2019-11-06scripted-diff: test: Move setup_common to test libraryMarcoFalke
-BEGIN VERIFY SCRIPT- # Move files for f in $(git ls-files src/test/lib/); do git mv $f src/test/util/; done git mv src/test/setup_common.cpp src/test/util/ git mv src/test/setup_common.h src/test/util/ # Replace Windows paths sed -i -e 's|\\setup_common|\\util\\setup_common|g' $(git grep -l '\\setup_common') sed -i -e 's|src\\test\\lib\\|src\\test\\util\\|g' build_msvc/test_bitcoin/test_bitcoin.vcxproj # Everything else sed -i -e 's|/setup_common|/util/setup_common|g' $(git grep -l 'setup_common') sed -i -e 's|test/lib/|test/util/|g' $(git grep -l 'test/lib/') # Fix include guard sed -i -e 's|BITCOIN_TEST_SETUP_COMMON_H|BITCOIN_TEST_UTIL_SETUP_COMMON_H|g' ./src/test/util/setup_common.h sed -i -e 's|BITCOIN_TEST_LIB_|BITCOIN_TEST_UTIL_|g' $(git grep -l 'BITCOIN_TEST_LIB_') -END VERIFY SCRIPT-
2019-11-04test: add "diamond" unit test to MempoolAncestryTestsSebastian Falbesoner
Approaches #17271. If ancestors are represented more than once, check that those are not overcounted.
2019-07-22txmempool: Remove unused default value MemPoolRemovalReason::UNKNOWNMarcoFalke
2019-06-26tests: Reduce compilation time and unneccessary recompiles by removing ↵practicalswift
unused includes in tests
2019-04-11scripted-diff: Bump copyright headers in test, benchMarcoFalke
-BEGIN VERIFY SCRIPT- ./contrib/devtools/copyright_header.py update ./src/bench/ ./contrib/devtools/copyright_header.py update ./src/test/ -END VERIFY SCRIPT-
2019-04-11scripted-diff: Rename test_bitcoin to test/setup_commonMarcoFalke
-BEGIN VERIFY SCRIPT- sed -i --regexp-extended -e 's/test_bitcoin\.(h|cpp)/setup_common.\1/g' $(git grep -l test_bitcoin) git mv ./src/test/test_bitcoin.h ./src/test/setup_common.h git mv ./src/test/test_bitcoin.cpp ./src/test/setup_common.cpp sed -i -e 's/BITCOIN_TEST_TEST_BITCOIN_H/BITCOIN_TEST_SETUP_COMMON_H/g' ./src/test/setup_common.h -END VERIFY SCRIPT-
2018-12-17test: Add missing validation locksMarcoFalke
2018-12-11Removed implicit CTransaction constructor from testslucash-dev
2018-11-04scripted-diff: Move util files to separate directory.Jim Posen
-BEGIN VERIFY SCRIPT- mkdir -p src/util git mv src/util.h src/util/system.h git mv src/util.cpp src/util/system.cpp git mv src/utilmemory.h src/util/memory.h git mv src/utilmoneystr.h src/util/moneystr.h git mv src/utilmoneystr.cpp src/util/moneystr.cpp git mv src/utilstrencodings.h src/util/strencodings.h git mv src/utilstrencodings.cpp src/util/strencodings.cpp git mv src/utiltime.h src/util/time.h git mv src/utiltime.cpp src/util/time.cpp sed -i 's/<util\.h>/<util\/system\.h>/g' $(git ls-files 'src/*.h' 'src/*.cpp') sed -i 's/<utilmemory\.h>/<util\/memory\.h>/g' $(git ls-files 'src/*.h' 'src/*.cpp') sed -i 's/<utilmoneystr\.h>/<util\/moneystr\.h>/g' $(git ls-files 'src/*.h' 'src/*.cpp') sed -i 's/<utilstrencodings\.h>/<util\/strencodings\.h>/g' $(git ls-files 'src/*.h' 'src/*.cpp') sed -i 's/<utiltime\.h>/<util\/time\.h>/g' $(git ls-files 'src/*.h' 'src/*.cpp') sed -i 's/BITCOIN_UTIL_H/BITCOIN_UTIL_SYSTEM_H/g' src/util/system.h sed -i 's/BITCOIN_UTILMEMORY_H/BITCOIN_UTIL_MEMORY_H/g' src/util/memory.h sed -i 's/BITCOIN_UTILMONEYSTR_H/BITCOIN_UTIL_MONEYSTR_H/g' src/util/moneystr.h sed -i 's/BITCOIN_UTILSTRENCODINGS_H/BITCOIN_UTIL_STRENCODINGS_H/g' src/util/strencodings.h sed -i 's/BITCOIN_UTILTIME_H/BITCOIN_UTIL_TIME_H/g' src/util/time.h sed -i 's/ util\.\(h\|cpp\)/ util\/system\.\1/g' src/Makefile.am sed -i 's/utilmemory\.\(h\|cpp\)/util\/memory\.\1/g' src/Makefile.am sed -i 's/utilmoneystr\.\(h\|cpp\)/util\/moneystr\.\1/g' src/Makefile.am sed -i 's/utilstrencodings\.\(h\|cpp\)/util\/strencodings\.\1/g' src/Makefile.am sed -i 's/utiltime\.\(h\|cpp\)/util\/time\.\1/g' src/Makefile.am sed -i 's/-> util ->/-> util\/system ->/' test/lint/lint-circular-dependencies.sh sed -i 's/src\/util\.cpp/src\/util\/system\.cpp/g' test/lint/lint-format-strings.py test/lint/lint-locale-dependence.sh sed -i 's/src\/utilmoneystr\.cpp/src\/util\/moneystr\.cpp/g' test/lint/lint-locale-dependence.sh sed -i 's/src\/utilstrencodings\.\(h\|cpp\)/src\/util\/strencodings\.\1/g' test/lint/lint-locale-dependence.sh sed -i 's/src\\utilstrencodings\.cpp/src\\util\\strencodings\.cpp/' build_msvc/libbitcoinconsensus/libbitcoinconsensus.vcxproj -END VERIFY SCRIPT-
2018-08-29Merge #13792: tx pool: Avoid passing redundant hash into addUnchecked ↵Wladimir J. van der Laan
(scripted-diff) fa587773e59721e187cadc998f4dc236ad3aef0b scripted-diff: Remove unused first argument to addUnchecked (MarcoFalke) fe5c49766c0dc5beaf186d77b568361242b20d5e tx pool: Use the entry's hash instead of the one passed to addUnchecked (MarcoFalke) ddd395f968a050be5dd0ae21ba7d189b6b7f73fd Mark CTxMemPoolEntry members that should not be modified const (MarcoFalke) Pull request description: Several years ago the transaction hash was not cached. For optimization the hash was instead passed into `addUnchecked` to avoid re-calculating it. See f77654a0e9424f13cad04f82c014abd78fbb5e38 Passing in the hash is now redundant and the argument can safely be removed. Tree-SHA512: 0206b65c7a014295f67574120e8c5397bf1b1bd70c918ae1360ab093676f7f89a6f084fd2c7000a141baebfe63fe6f515559e38c4ac71810ba64f949f9c0467f
2018-08-08Merge #13780: 0.17: Pre-branch maintenanceWladimir J. van der Laan
3fc20632a3ad30809356a58d2cf0ea4a4ad4cec3 qt: Set BLOCK_CHAIN_SIZE = 220 (DrahtBot) 2b6a2f4a28792f2fe9dc1be843b1ff1ecae35e8a Regenerate manpages (DrahtBot) eb7daf4d600eeb631427c018a984a77a34aca66e Update copyright headers to 2018 (DrahtBot) Pull request description: Some trivial maintenance to avoid having to do it again after the 0.17 branch off. (The scripts to do this are in `./contrib/`) Tree-SHA512: 16b2af45e0351b1c691c5311d48025dc6828079e98c2aa2e600dc5910ee8aa01858ca6c356538150dc46fe14c8819ed8ec8e4ec9a0f682b9950dd41bc50518fa
2018-07-30scripted-diff: Remove unused first argument to addUncheckedMarcoFalke
-BEGIN VERIFY SCRIPT- git grep -l addUnchecked | xargs sed --regexp-extended -i -e 's/addUnchecked\([^)][^,]+,\s*/addUnchecked(/g' -END VERIFY SCRIPT-
2018-07-29refactor: Avoid locking tx pool cs thriceMarcoFalke
2018-07-27Update copyright headers to 2018DrahtBot
2018-07-24scripted-diff: Remove trailing whitespacesJoão Barbosa
-BEGIN VERIFY SCRIPT- sed --in-place'' --regexp-extended 's/[[:space:]]+$//g' $(git grep -I --files-with-matches --extended-regexp '[[:space:]]+$' -- src test ':!*.svg' ':!src/crypto/sha256_sse4*' ':!src/leveldb' ':!src/qt/locale' ':!src/secp256k1' ':!src/univalue') -END VERIFY SCRIPT-
2018-06-13tests: Drop variadic macroMarcoFalke
2018-06-11test: Add MempoolAncestryTestsKarl-Johan Alm
2018-05-05Add Clang thread safety analysis annotationspracticalswift
2018-05-03Make it clear which functions that are intended to be translation unit localpracticalswift
Do not share functions that are meant to be translation unit local with other translation units. Use internal linkage for those consistently.
2018-04-11test: Fix sign for expected valuesKarl-Johan Alm
A number of BOOST_CHECK_EQUAL calls would result in warnings about signs.
2018-03-21Fix typosDimitris Apostolou
2018-01-15Merge #12118: Sort mempool by min(feerate, ancestor_feerate)Wladimir J. van der Laan
0a22a52 Use mempool's ancestor sort in transaction selection (Suhas Daftuar) 7abfa53 Add test for new ancestor feerate sort behavior (Suhas Daftuar) 9a51319 Sort mempool by min(feerate, ancestor_feerate) (Suhas Daftuar) 6773f92 Refactor CompareTxMemPoolEntryByDescendantScore (Suhas Daftuar) Pull request description: This more closely approximates the desirability of a given transaction for mining, and should result in less re-sorting when transactions get removed from the mempool after being mined. I measured this as approximately a 5% speedup in removeForBlock. Tree-SHA512: ffa36b567c5dfe3e8908c545a459b6a5ec0de26e7dc81b1050dd235cac9046564b4409a3f8c5ba97bd8b30526e8fec8f78480a912e317979467f32305c3dd37b
2018-01-12Add test for new ancestor feerate sort behaviorSuhas Daftuar
2018-01-09Remove unused mempool indexSuhas Daftuar
2018-01-03Increment MIT Licence copyright header year on files modified in 2017Akira Takizawa
2017-11-16scripted-diff: Replace #include "" with #include <> (ryanofsky)MeshCollider
-BEGIN VERIFY SCRIPT- for f in \ src/*.cpp \ src/*.h \ src/bench/*.cpp \ src/bench/*.h \ src/compat/*.cpp \ src/compat/*.h \ src/consensus/*.cpp \ src/consensus/*.h \ src/crypto/*.cpp \ src/crypto/*.h \ src/crypto/ctaes/*.h \ src/policy/*.cpp \ src/policy/*.h \ src/primitives/*.cpp \ src/primitives/*.h \ src/qt/*.cpp \ src/qt/*.h \ src/qt/test/*.cpp \ src/qt/test/*.h \ src/rpc/*.cpp \ src/rpc/*.h \ src/script/*.cpp \ src/script/*.h \ src/support/*.cpp \ src/support/*.h \ src/support/allocators/*.h \ src/test/*.cpp \ src/test/*.h \ src/wallet/*.cpp \ src/wallet/*.h \ src/wallet/test/*.cpp \ src/wallet/test/*.h \ src/zmq/*.cpp \ src/zmq/*.h do base=${f%/*}/ relbase=${base#src/} sed -i "s:#include \"\(.*\)\"\(.*\):if test -e \$base'\\1'; then echo \"#include <\"\$relbase\"\\1>\\2\"; else echo \"#include <\\1>\\2\"; fi:e" $f done -END VERIFY SCRIPT-
2017-11-10tests: Add missing locks to testspracticalswift
Add missing locks to tests to satisfy lock requirements (such as EXCLUSIVE_LOCKS_REQUIRED(...) (Clang Thread Safety Analysis), AssertLockHeld(...) and implicit lock assumptions).
2017-09-11Force explicit double -> int conversion for CFeeRate constructorMatt Corallo
This resolves an issue where estimatesmartfee would return 999 sat/byte instead of 1000, due to floating point loss of precision Thanks to sipa for suggesting is_integral.
2017-03-07Merge #9548: Remove min reasonable feeWladimir J. van der Laan
ad82cb0 Remove unnecessary min fee argument in CTxMemPool constructor (Alex Morcos) 2a7b56c CBlockPolicyEstimator now uses hard coded minimum bucket feerate (Alex Morcos) ac9d3d2 Change fee estimation bucket limit variable names (Alex Morcos) Tree-SHA512: 6e3bc7df3497ed60c7620845d222063e33a0238020f5c3316e61e0eff758078588ea8dd51196ceb59aa561ba106f8cdae62cebe521adb3247108bb49f15252d6
2017-03-03[cleanup] Remove coin age priority completely.Alex Morcos
Remove GetPriority and ComputePriority. Remove internal machinery for tracking priority in CTxMemPoolEntry.
2017-03-03[test] Remove priority from testsAlex Morcos
Remove all coin age priority functionality from unit tests and RPC tests.
2017-01-17Remove unnecessary min fee argument in CTxMemPool constructorAlex Morcos
generated by cgit v1.2.3 (git 2.26.2) at 2024-12-28 23:08:56 +0000