aboutsummaryrefslogtreecommitdiff
path: root/src/test
AgeCommit message (Collapse)Author
24 min.Merge bitcoin/bitcoin#31391: util: Drop boost posix_time in ParseISO8601DateTimeHEADmastermerge-script
faf70cc9941ce2b0ce4fd48ecfdbe28194adb8ba Remove wallet::ParseISO8601DateTime, use ParseISO8601DateTime instead (MarcoFalke) 2222aecd5f8059785e655da7b7e3fcc59204245c util: Implement ParseISO8601DateTime based on C++20 (MarcoFalke) Pull request description: `boost::posix_time` in `ParseISO8601DateTime` has many issues: * It parses random strings that are clearly invalid and returns a time value for them, see [1] below. * None of the separators `-`, or `:`, or `T`, or `Z` are validated. * It may crash when running under a hardened C++ library, see https://github.com/bitcoin/bitcoin/issues/28917. * It has been unmaintained for years, so reporting or fixing any issues will most likely be useless. * It pulls in a third-party dependency, when the functionality is already included in vanilla C++20. Fix all issues by replacing it with a simple helper function written in C++20. Fixes https://github.com/bitcoin/bitcoin/issues/28917. [1] The following patch passes on current master: ```diff diff --git a/src/wallet/test/rpc_util_tests.cpp b/src/wallet/test/rpc_util_tests.cpp index 32f6f5ab46..c1c94c7116 100644 --- a/src/wallet/test/rpc_util_tests.cpp +++ b/src/wallet/test/rpc_util_tests.cpp @@ -12,6 +12,14 @@ BOOST_AUTO_TEST_SUITE(wallet_util_tests) BOOST_AUTO_TEST_CASE(util_ParseISO8601DateTime) { + BOOST_CHECK_EQUAL(ParseISO8601DateTime("964296"), 242118028800); + BOOST_CHECK_EQUAL(ParseISO8601DateTime("244622"), 15023836800); + BOOST_CHECK_EQUAL(ParseISO8601DateTime("+INfINITy"), 9223372036854); + BOOST_CHECK_EQUAL(ParseISO8601DateTime("7000802 01"), 158734166400); + BOOST_CHECK_EQUAL(ParseISO8601DateTime("7469-2 +INfINITy"), 9223372036854); + BOOST_CHECK_EQUAL(ParseISO8601DateTime("maXimum-datE-time"), 253402300799); + BOOST_CHECK_EQUAL(ParseISO8601DateTime("577737 114maXimum-datE-time"), 253402300799); + BOOST_CHECK_EQUAL(ParseISO8601DateTime("1970-01-01T00:00:00Z"), 0); BOOST_CHECK_EQUAL(ParseISO8601DateTime("1960-01-01T00:00:00Z"), 0); BOOST_CHECK_EQUAL(ParseISO8601DateTime("2000-01-01T00:00:01Z"), 946684801); ``` ACKs for top commit: hebasto: ACK faf70cc9941ce2b0ce4fd48ecfdbe28194adb8ba, I have reviewed the code and it looks OK. dergoegge: utACK faf70cc9941ce2b0ce4fd48ecfdbe28194adb8ba Tree-SHA512: 9dd745a356d04acf6200e13a6af52c51a9e2a0eeccea110093ce5da147b3c669c0eda918e46db0164c081a78c8feae3fe557a4759bea18449a8ff2d090095931
13 hoursMerge bitcoin/bitcoin#31096: Package validation: accept packages of size 1Ava Chow
32fc59796f74a2941772b5ec2755b1319132cd9c rpc: Allow single transaction through submitpackage (glozow) Pull request description: There's no particular reason to restrict single transaction submissions with submitpackage. This change relaxes the RPC checks as enables the `AcceptPackage` flow to accept packages of a single transaction. Resolves #31085 ACKs for top commit: naumenkogs: ACK https://github.com/bitcoin/bitcoin/commit/32fc59796f74a2941772b5ec2755b1319132cd9c achow101: ACK 32fc59796f74a2941772b5ec2755b1319132cd9c glozow: ACK 32fc59796f74a2941772b5ec2755b1319132cd9c Tree-SHA512: ffed353bfdca610ffcfd53b40b76da05ffc26df6bac4b0421492e067bede930380e03399d2e2d1d17f0e88fb91cd8eb376e3aabebbabcc724590bf068d09807c
38 hours[validation] Make script error messages uniform for parallel/single validationPieter Wuille
This makes the debug output mostly the same for -par=1 and parallel validation runs. Of course, parallel validation is non-deterministic in what error it may encounter first if there are multiple issues. Also, the way certain script-related and non-script-related checks are performed differs between the two modes still, which may result in discrepancies.
38 hours[checkqueue] support user-defined return type through std::optionalPieter Wuille
The check type function now needs to return a std::optional<R> for some type R, and the check queue overall will return std::nullopt if all individual checks return that, or one of the non-nullopt values if there is at least one. For most tests, we use R=int, but for the actual validation code, we make it return the ScriptError.
46 hoursRemove wallet::ParseISO8601DateTime, use ParseISO8601DateTime insteadMarcoFalke
2 daysutil: Implement ParseISO8601DateTime based on C++20MarcoFalke
7 daysMerge bitcoin/bitcoin#30708: rpc: add getdescriptoractivityAva Chow
37a5c5d83664c31d83fc649d3c8c858bd5f10f21 doc: update descriptors.md for getdescriptoractivity (James O'Beirne) ee3ce6a4f4d35afe7fcab16eff419a6788b02170 test: rpc: add no address case for getdescriptoractivity (James O'Beirne) 811f76f3a511d20750046319b390e225a1151caa rpc: add getdescriptoractivity (James O'Beirne) 25fe087de59e967ce968d35ed77138325eb9a9fa rpc: move-only: move ScriptPubKeyDoc to utils (James O'Beirne) Pull request description: The RPC command `scanblocks` provides a useful way to get a set of blockhashes that have activity relevant to a set of descriptors (`relevant_blocks`). However actually extracting the activity from those blocks is left as an exercise to the end user. This process involves not only generating the (potentially ranged) set of scripts for the descriptor set on the client side (maybe via `deriveaddresses`), but then the user must retrieve each block's contents one-by-one using `getblock <hash>`, which is transmitted over a network link. And that's all before they perform the actual search over block content. There's even more work required to incorporate unconfirmed transactions. This PR introduces an RPC `getdescriptoractivity` that [dovetails](https://bitcoin-irc.chaincode.com/bitcoin-core-dev/2024-08-16#1046393;) with `scanblocks` output, handling the process described above. Users specify the blockhashes (perhaps from `relevant_blocks`) and a set of descriptors; they are then given all spend/receive activity in that set of blocks. This is a very useful tool when implementing lightweight wallets that want neither to require a third-party indexer like electrs, nor the overhead of creating and managing watch-only wallets in Core. This allows Core to be more easily used in a "stateless" manner by wallets, with potentially many nodes interchangeably acting as backends. ### Example usage ``` % ./src/bitcoin-cli scanblocks start \ '["addr(bc1p0cp0vyag6snlta2l7c4am3rue7eef9f72l7uhx52m4v27vfydx9s8tfs7t)"]' \ 857263 { "from_height": 857263, "to_height": 858263, "relevant_blocks": [ "00000000000000000002bc5cc78f5b0913a5230a8f4b0d5060bc9a60900a5a88", "00000000000000000001c5291ed6a40c06d3db5c8fb738567654b24a14b24ecb" ], "completed": true } % ./src/bitcoin-cli getdescriptoractivity \ '["00000000000000000002bc5cc78f5b0913a5230a8f4b0d5060bc9a60900a5a88", "00000000000000000001c5291ed6a40c06d3db5c8fb738567654b24a14b24ecb"]' \ '["addr(bc1p0cp0vyag6snlta2l7c4am3rue7eef9f72l7uhx52m4v27vfydx9s8tfs7t)"]' { "activity": [ { "type": "receive", "amount": 0.00002900, "blockhash": "00000000000000000002bc5cc78f5b0913a5230a8f4b0d5060bc9a60900a5a88", "height": 857907, "txid": "c9d34f202c1f66d80cae76f305350f5fdde910b97cf6ae6bf79f5bcf2a337d06", "vout": 254, "output_spk": { "asm": "1 7e02f613a8d427f5f55ff62bddc47ccfb394953e57fdcb9a8add58af3124698b", "desc": "rawtr(7e02f613a8d427f5f55ff62bddc47ccfb394953e57fdcb9a8add58af3124698b)#yewcd80j", "hex": "51207e02f613a8d427f5f55ff62bddc47ccfb394953e57fdcb9a8add58af3124698b", "address": "bc1p0cp0vyag6snlta2l7c4am3rue7eef9f72l7uhx52m4v27vfydx9s8tfs7t", "type": "witness_v1_taproot" } }, { "type": "spend", "amount": 0.00002900, "blockhash": "00000000000000000001c5291ed6a40c06d3db5c8fb738567654b24a14b24ecb", "height": 858260, "spend_txid": "7f61d1b248d4ee46376f9c6df272f63fbb0c17039381fb23ca5d90473b823c36", "spend_vin": 0, "prevout_txid": "c9d34f202c1f66d80cae76f305350f5fdde910b97cf6ae6bf79f5bcf2a337d06", "prevout_vout": 254, "prevout_spk": { "asm": "1 7e02f613a8d427f5f55ff62bddc47ccfb394953e57fdcb9a8add58af3124698b", "desc": "rawtr(7e02f613a8d427f5f55ff62bddc47ccfb394953e57fdcb9a8add58af3124698b)#yewcd80j", "hex": "51207e02f613a8d427f5f55ff62bddc47ccfb394953e57fdcb9a8add58af3124698b", "address": "bc1p0cp0vyag6snlta2l7c4am3rue7eef9f72l7uhx52m4v27vfydx9s8tfs7t", "type": "witness_v1_taproot" } } ] } ``` ACKs for top commit: instagibbs: reACK 37a5c5d83664c31d83fc649d3c8c858bd5f10f21 achow101: ACK 37a5c5d83664c31d83fc649d3c8c858bd5f10f21 tdb3: Code review and light retest ACK 37a5c5d83664c31d83fc649d3c8c858bd5f10f21 rkrux: re-ACK 37a5c5d83664c31d83fc649d3c8c858bd5f10f21 Tree-SHA512: 04aa51e329c6c2ed72464b9886281d5ebd7511a8a8e184ea81249033a4dad535a12829b1010afc2da79b344ea8b5ab8ed47e426d0bf2eb78ab395d20b1da8dbb
7 daysrpc: add getdescriptoractivityJames O'Beirne
8 daysMerge bitcoin/bitcoin#31364: refactor: Fix remaining clang-tidy ↵Ava Chow
performance-unnecessary-copy-initialization errors 3305972f7bfd78181566e4297891c2dd7cae0f2b refactor: Fix remaining clang-tidy performance-unnecessary-copy-initialization errors (Lőrinc) Pull request description: A follow-up of https://github.com/bitcoin/bitcoin/pull/31305. The `clang-tidy` check can be run via: ```bash cmake -B build -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -DCMAKE_EXPORT_COMPILE_COMMANDS=ON -DBUILD_BENCH=ON -DBUILD_FUZZ_BINARY=ON -DBUILD_FOR_FUZZING=ON && cmake --build build -j$(nproc) run-clang-tidy -quiet -p build -j $(nproc) -checks='-*,performance-unnecessary-copy-initialization' | grep -v 'clang-tidy' ``` ACKs for top commit: maflcko: review ACK 3305972f7bfd78181566e4297891c2dd7cae0f2 🏀 achow101: ACK 3305972f7bfd78181566e4297891c2dd7cae0f2b theuni: ACK the much more constrained 3305972f7bfd78181566e4297891c2dd7cae0f2b. hebasto: ACK 3305972f7bfd78181566e4297891c2dd7cae0f2b, tested with clang 19.1.5 + clang-tidy. Tree-SHA512: 64dc3b35f33b7ac064ebf9e56e9f0ceca5d26681a1379dcd2168987960020fe1a282ec4de8c353c82ddf0a534a4866b607fc691e690010c6cea78887045897fb
8 daysMerge bitcoin/bitcoin#31305: refactor: Fix remaining clang-tidy ↵Ava Chow
performance-inefficient-vector errors 11f3bc229ccd4b20191855fb1df882cfa6145264 refactor: Reserve vectors in fuzz tests (Lőrinc) 152fefe7a22b7da3cfe2815083634bece9c5654e refactor: Preallocate PrevectorFillVector(In)Direct without vector resize (Lőrinc) a774c7a339c26b1409c9a9572d2b52810ee64062 refactor: Fix remaining clang-tidy performance-inefficient-vector errors (Lőrinc) Pull request description: PR inspired by https://github.com/bitcoin/bitcoin/pull/29608#issuecomment-2437847307 (and https://github.com/bitcoin/bitcoin/pull/29458, https://github.com/bitcoin/bitcoin/pull/29606, https://github.com/bitcoin/bitcoin/pull/29607, https://github.com/bitcoin/bitcoin/pull/30093). The `clang-tidy` check can be run via: ```bash cmake -B build -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -DCMAKE_EXPORT_COMPILE_COMMANDS=ON -DBUILD_BENCH=ON -DBUILD_FUZZ_BINARY=ON -DBUILD_FOR_FUZZING=ON && cmake --build build -j$(nproc) run-clang-tidy -quiet -p build -j $(nproc) -checks='-*,performance-inefficient-vector-operation' | grep -v 'clang-tidy' ``` which revealed 3 tests and 1 prod warning (+ fuzz and benching, found by hebasto). Even though the tests aren't performance critical, getting rid of these warnings (for which the checks were already enabled via https://github.com/bitcoin/bitcoin/blob/master/src/.clang-tidy#L18, see below), the fix was quite simple. <details> <summary>clang-tidy -list-checks</summary> ```bash cd src && clang-tidy -list-checks | grep 'vector' performance-inefficient-vector-operation ``` </details> <details> <summary>Output before the change</summary> ``` src/test/rpc_tests.cpp:434:9: error: 'emplace_back' is called inside a loop; consider pre-allocating the container capacity before the loop [performance-inefficient-vector-operation,-warnings-as-errors] 433 | for (int64_t i = 0; i < 100; i++) { 434 | feerates.emplace_back(1 ,1); | ^ src/test/checkqueue_tests.cpp:366:13: error: 'emplace_back' is called inside a loop; consider pre-allocating the container capacity before the loop [performance-inefficient-vector-operation,-warnings-as-errors] 365 | for (size_t i = 0; i < 3; ++i) { 366 | tg.emplace_back( | ^ src/test/cuckoocache_tests.cpp:231:9: error: 'emplace_back' is called inside a loop; consider pre-allocating the container capacity before the loop [performance-inefficient-vector-operation,-warnings-as-errors] 228 | for (uint32_t x = 0; x < 3; ++x) 229 | /** Each thread is emplaced with x copy-by-value 230 | */ 231 | threads.emplace_back([&, x] { | ^ src/rpc/output_script.cpp:127:17: error: 'push_back' is called inside a loop; consider pre-allocating the container capacity before the loop [performance-inefficient-vector-operation,-warnings-as-errors] 126 | for (unsigned int i = 0; i < keys.size(); ++i) { 127 | pubkeys.push_back(HexToPubKey(keys[i].get_str())); | ^ ``` And the fuzz and benchmarks, noticed by hebasto: https://github.com/bitcoin/bitcoin/pull/31305#issuecomment-2483124499 </details> ACKs for top commit: maflcko: review ACK 11f3bc229ccd4b20191855fb1df882cfa6145264 🎦 achow101: ACK 11f3bc229ccd4b20191855fb1df882cfa6145264 theuni: ACK 11f3bc229ccd4b20191855fb1df882cfa6145264 hebasto: ACK 11f3bc229ccd4b20191855fb1df882cfa6145264, tested with clang 19.1.5 + clang-tidy. Tree-SHA512: 41691c19f35c63b922a95407617a54f9bff1af3f95f99d15642064f321df038aeb1ae5f061f854ed913f69036807cc28fa6222b2ff4c24ef43b909027fa0f9b3
9 daysrpc: Allow single transaction through submitpackageglozow
And under the hood suppoert single transactions in AcceptPackage. This simplifies user experience and paves the way for reducing number of codepaths for transaction acceptance in the future. Co-Authored-By: instagibbs <gsanders87@gmail.com>
9 daysrefactor: Fix remaining clang-tidy ↵Lőrinc
performance-unnecessary-copy-initialization errors
9 daysrefactor: Reserve vectors in fuzz testsLőrinc
* Since the main LIMITED_WHILE stated `outpoints.size() < 200'000`, I've presized outpoints accordingly. * `tx_mut.vin` and `tx_mut.vout` weren't caught by the clang-tidy, but addressed them anyway.
9 daysrefactor: Fix remaining clang-tidy performance-inefficient-vector errorsLőrinc
9 daysMerge bitcoin/bitcoin#31279: policy: ephemeral dust followupsglozow
466e4df3fb83ef82b6add22e202e3a70dbf83a12 assert_mempool_contents: assert not duplicates expected (Greg Sanders) ea5db2f26920bce7caf85e5c1b70a527cc3b82c2 functional: only generate required blocks for test (Greg Sanders) d033acb608391f3ba95864cdaa7025cc00888ea2 fuzz: package_eval: let fuzzer run out input in main tx creation loop (Greg Sanders) ba35a570c5d4ade342cb32630ffaa5f5bdd5e826 CheckEphemeralSpends: return boolean, and set child state and txid outparams (Greg Sanders) cf0cee1617c0bf065b295a9807a4c7de0558393d func: add note about lack of 1P1C propagation in tree submitpackage (Greg Sanders) 84242903043bb14fca917790c9381c411817c9f7 unit test: ephemeral_tests is using a dust relay rate, not minrelay (Greg Sanders) d9cfa5fc4eb03fb425fd5d46d3b72db72fbc3243 CheckEphemeralSpends: no need to iterate inputs if no parent dust (Greg Sanders) 87b26e3dc07b283cb05064ccde179c6777397ce8 func: rename test_free_relay to test_no_minrelay_fee (Greg Sanders) e5709a4a41ecd8c7b1e695871c1a6153864e76ae func: slight elaboration on submitpackage restriction (Greg Sanders) 08e969bd1076c99e0b43ecd01dd790b9ebd04d0a RPC: only enforce dust rules on priority when standardness active (Greg Sanders) ca050d12e76f61af7e60fa564dd04db08f2b8f38 unit test: adapt to changing MAX_DUST_OUTPUTS_PER_TX (Greg Sanders) 7c3490169c9e20375d3f525f81798fcced01a30a fuzz: package_eval: move last_tx inside txn ctor (Greg Sanders) 445eaed182a714e65ee2fe679ecdf7a86055313b fuzz: use optional status instead of should_rbf_eph_spend (Greg Sanders) 4dfdf615b9dbdc2204347029bea1db974a88e392 fuzz: remove unused TransactionsDelta validation interface (Greg Sanders) 09ce926e4a14f183cfab387d2531519e000ea176 func: cleanup reorg test comment (Greg Sanders) 768a0c1889e57ae8bb3596ac7aa9fd2b1ecab9fa func: cleanup test_dustrelay comments (Greg Sanders) bedca1cb6633f4b9a5f8f532f27e084f23f04a2e fuzz: Directly place transactions in vector (Greg Sanders) c041ad6eccb5aae87648cf510257a06f711b1bc3 fuzz: explain package eval coin tracking better (Greg Sanders) bc0d98ea6126ea95526c2b70721131764c6ff3a7 fuzz: remove dangling reference to GetEntry (Greg Sanders) 15b6cbf07f5c3db650a0a8cccf46d3fbe031aef0 unit test: make dust index less magical (Greg Sanders) 5fbcfd12b8f508c87740883435800b6260fa308b unit test: assert txid returned on CheckEphemeralSpends failures (Greg Sanders) ef94d84b4e469d8dbd63e63598d3b8d53595c695 bench: remove unnecessary CMTxn constructors (Greg Sanders) c5c10fd317c6b4c033f3001757e6975b8b9a4942 ephemeral policy doxygen cleanup (Greg Sanders) dd9044b8d4624fb7ffd432b6b89ab99290957a3e ephemeral policy: IWYU (Greg Sanders) c6859ce2de7531e42fc304b69d74ca0d8e99ea29 Move+rename GetDustIndexes -> GetDust (Greg Sanders) 62016b32300123a44599e649b4f35a3a0f32565f Use std::ranges for ephemeral policy checks (Greg Sanders) 3ed930a1f41f7d7160c6ede5dcf3d4d5f1cfa876 Have HasDust and PreCheckValidEphemeralTx take CTransaction (Greg Sanders) 04a614bf9a7bb6abad150a3edf8938358f54d55b Rename CheckValidEphemeralTx to PreCheckEphemeralTx (Greg Sanders) cbf1a47d6062ec2c2c4a788636e8c950a0271997 CheckEphemeralSpends: only compute txid of tx when needed (Greg Sanders) Pull request description: Follow-up to https://github.com/bitcoin/bitcoin/pull/30239 Here are the parent PR's comments that should be addressed by this PR: https://github.com/bitcoin/bitcoin/pull/30239/files#r1834529646 https://github.com/bitcoin/bitcoin/pull/30239/files#r1831247308 https://github.com/bitcoin/bitcoin/pull/30239/files#r1832622481 https://github.com/bitcoin/bitcoin/pull/30239/files#r1831195216 https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1835805164 https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1835805164 https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1834639096 https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1834624976 https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1834619709 https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1834610434 https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1834504436 https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1834500036 https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1832985488 https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1830929809 https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1832376920 https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1832755799 https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1832492686 https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1832980576 https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1832784278 https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1837989979 https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1830996993 https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1830997947 https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1830012890 https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1830037288 https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1830977092 https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1832622481 https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1834726168 https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1832453654 https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1848488226 ACKs for top commit: naumenkogs: ACK 466e4df3fb83ef82b6add22e202e3a70dbf83a12 hodlinator: ACK 466e4df3fb83ef82b6add22e202e3a70dbf83a12 theStack: lgtm ACK 466e4df3fb83ef82b6add22e202e3a70dbf83a12 glozow: utACK 466e4df3fb83ef82b6add22e202e3a70dbf83a12 Tree-SHA512: 89106f695755c238b84e0996b89446c0733e10a94c867f656d516d26697d2efe38dfc332188b8589a0a26a3d2bd2c88c6ab70c108e187ce5bfcb91bbf3fb0391
14 daysfuzz: package_eval: let fuzzer run out input in main tx creation loopGreg Sanders
14 daysCheckEphemeralSpends: return boolean, and set child state and txid outparamsGreg Sanders
14 daysunit test: ephemeral_tests is using a dust relay rate, not minrelayGreg Sanders
14 daysunit test: adapt to changing MAX_DUST_OUTPUTS_PER_TXGreg Sanders
14 daysfuzz: package_eval: move last_tx inside txn ctorGreg Sanders
14 daysfuzz: use optional status instead of should_rbf_eph_spendGreg Sanders
14 daysfuzz: remove unused TransactionsDelta validation interfaceGreg Sanders
14 daysfuzz: Directly place transactions in vectorGreg Sanders
14 daysfuzz: explain package eval coin tracking betterGreg Sanders
14 daysfuzz: remove dangling reference to GetEntryGreg Sanders
14 daysunit test: make dust index less magicalGreg Sanders
14 daysunit test: assert txid returned on CheckEphemeralSpends failuresGreg Sanders
Simplify nullopt checks
14 daysMove+rename GetDustIndexes -> GetDustGreg Sanders
Use to replace HasDust and where appropraite
14 daysfuzz: Implement G_TEST_GET_FULL_NAMEHodlinator
When BasicTestingSetup is used in fuzz-tests it will now create test directories containing the fuzz target names. Example: /tmp/test_common bitcoin/tx_package_eval/153d7906294f7d0606a7/ This is already implemented for bench and unit tests.
14 daysMerge bitcoin/bitcoin#31122: cluster mempool: Implement changeset interface ↵glozow
for mempool 5736d1ddacc4019101e7a5170dd25efbc63b622a tracing: pass if replaced by tx/pkg to tracepoint (0xb10c) a4ec07f1944999c2eead41d08d7dd4fc3aa71243 doc: add comments for CTxMemPool::ChangeSet (Suhas Daftuar) 83f814b1d1100baac9dca9c176f89b0ec2555dbc Remove m_all_conflicts from SubPackageState (Suhas Daftuar) d3c8e7dfb63f7986a1f9654ea2393aabe3cd78da Ensure that we don't add duplicate transactions in rbf fuzz tests (Suhas Daftuar) d7dc9fd2f7bc675256687b9c55fdbec9cc8ac781 Move CalculateChunksForRBF() to the mempool changeset (Suhas Daftuar) 284a1d33f1dcbc3b3404ea40a948ff6600239613 Move prioritisation into changeset (Suhas Daftuar) 446b08b599bc492bbec10ccc2292aee6f90c58e7 Don't distinguish between direct conflicts and all conflicts when doing cluster-size-2-rbf checks (Suhas Daftuar) b53041021abc4f9ee7203341413e8676e2d5a7ca Duplicate transactions are not permitted within a changeset (Suhas Daftuar) b447416fddcb8c8647391502cca3dbfd1552e02e Public mempool removal methods Assume() no changeset is outstanding (Suhas Daftuar) 2b30f4d36c86f775ac637b171d27d42a02309c5b Make RemoveStaged() private (Suhas Daftuar) 18829194ca68152ac0b38d34e94b9265ee74c410 Enforce that there is only one changeset at a time (Suhas Daftuar) 7fb62f7db60c7d793828ae45f87bc3f5c63cc989 Apply mempool changeset transactions directly into the mempool (Suhas Daftuar) 34b6c5833d11ea84fbd4b891e06408f6f4ca6fac Clean up FinalizeSubpackage to avoid workspace-specific information (Suhas Daftuar) 57983b8add72a04721d3f2050c063a3c4d8683ed Move LimitMempoolSize to take place outside FinalizeSubpackage (Suhas Daftuar) 01e145b9758f1df14a7ea18058ba9577bf88e459 Move changeset from workspace to subpackage (Suhas Daftuar) 802214c0832de00f24268183f7763fa984ba7903 Introduce mempool changesets (Suhas Daftuar) 87d92fa340195d9c87be3d023ca133b90b3b7d4e test: Add unit test coverage of package rbf + prioritisetransaction (Suhas Daftuar) 15d982f91e6b0f145c9dd4edf29827cfabb37a3f Add package hash to package-rbf log message (Suhas Daftuar) Pull request description: part of cluster mempool: #30289 It became clear while working on cluster mempool that it would be helpful for transaction validation if we could consider a full set of proposed changes to the mempool -- consisting of a set of transactions to add, and a set of transactions (ie conflicts) to simultaneously remove -- and perform calculations on what the mempool would look like if the proposed changes were to be applied. Two specific examples of where we'd like to do this: - Determining if ancestor/descendant/TRUC limits would be violated (in the future, cluster limits) if either a single transaction or a package of transactions were to be accepted - Determining if an RBF would make the mempool "better", however that idea is defined, both in the single transaction and package of transaction cases In preparation for cluster mempool, I have pulled this reworking of the mempool interface out of #28676 so it can be reviewed on its own. I have not re-implemented ancestor/descendant limits to be run through the changeset, since with cluster mempool those limits will be going away, so this seems like wasted effort. However, I have rebased #28676 on top of this branch so reviewers can see what the new mempool interface could look like in the cluster mempool setting. There are some minor behavior changes here, which I believe are inconsequential: - In the package validation setting, transactions would be added to the mempool before the `ConsensusScriptChecks()` are run. In theory, `ConsensusScriptChecks()` should always pass if the `PolicyScriptChecks()` have passed and it's just a belt-and-suspenders for us, but if somehow they were to diverge then there could be some small behavior change from adding transactions and then removing them, versus never adding them at all. - The error reporting on `CheckConflictTopology()` has slightly changed due to no longer distinguishing between direct conflicts and indirect conflicts. I believe this should be entirely inconsequential because there shouldn't be a logical difference between those two ideas from the perspective of this function, but I did have to update some error strings in some tests. - Because, in a package setting, RBFs now happen as part of the entire package being accepted, the logging has changed slightly because we do not know which transaction specifically evicted a given removed transaction. - Specifically, the "package hash" is now used to reference the set of transactions that are being accepted, rather than any single txid. The log message relating to package RBF that happen in the `TXPACKAGES` category has been updated as well to include the package hash, so that it's possible to see which specific set of transactions are being referenced by that package hash. - Relatedly, the tracepoint logging in the package rbf case has been updated as well to reference the package hash, rather than a transaction hash. ACKs for top commit: naumenkogs: ACK https://github.com/bitcoin/bitcoin/commit/5736d1ddacc4019101e7a5170dd25efbc63b622a instagibbs: ACK 5736d1ddacc4019101e7a5170dd25efbc63b622a ismaelsadeeq: reACK 5736d1ddacc4019101e7a5170dd25efbc63b622a glozow: ACK 5736d1ddacc Tree-SHA512: 21810872e082920d337c89ac406085aa71c5f8e5151ab07aedf41e6601f60a909b22fbf462ef3b735d5d5881e9b76142c53957158e674dd5dfe6f6aabbdf630b
14 daysdoc: add copyright header to p2p_headers_presyncfanquake
14 daysMerge bitcoin/bitcoin#31213: fuzz: Fix difficulty target generation in ↵merge-script
`p2p_headers_presync` a6ca8f324396522e9748c9a7bbefb3bf1c74a436 fuzz: Fix difficulty target generation in p2p_headers_presync (marcofleon) fa327c77e34e0cfb7994842c23f539ab11bf5d3b util: Add ConsumeArithUInt256InRange fuzzing helper (marcofleon) Pull request description: In the `p2p_headers_presync` fuzz target, this assertion failed: ``` assert(total_work < chainman.MinimumChainWork()); ``` Input that triggered the failure: [p2ppresync_crash.txt](https://github.com/user-attachments/files/17620203/p2ppresync_crash.txt) The test previously used `ConsumeIntegralInRange` to generate header difficulty targets within a hardcoded range. The fuzzer found specific values in that range that correspond to very low thresholds due to how [`SetCompact`][setcompact-link] works. The total work of a long enough test chain ended up exceeding `MinimumChainWork`. Fix this by adding a new `ConsumeArithUInt256InRange` helper function and use it in the fuzz test to generate target values within the originally intended range. The target is then converted to an `nBits` value using `GetCompact()`. For some more context, see https://github.com/bitcoin/bitcoin/pull/30918. [setcompact-link]: https://github.com/bitcoin/bitcoin/blob/6463117a29294f6ddc9fafecfd1e9023956cc41b/src/arith_uint256.h#L251-L271 ACKs for top commit: instagibbs: ACK https://github.com/bitcoin/bitcoin/pull/31213/commits/a6ca8f324396522e9748c9a7bbefb3bf1c74a436 dergoegge: Code review ACK a6ca8f324396522e9748c9a7bbefb3bf1c74a436 brunoerg: code review ACK a6ca8f324396522e9748c9a7bbefb3bf1c74a436 Tree-SHA512: 92013d9d37bd3f11992ee678ba9745196efbdc4d773fd14994116629260bea46ffc9fa3923d443af7b623d39c6211900ce98a349c62ad1976e12312c37ef9df0
2024-11-20Merge bitcoin/bitcoin#31317: test: Revert to random path elementmerge-script
faaaf59f71ede057b2c1d369ef8db973c2f2dbc2 test: Make g_rng_temp_path rand, not dependent on SeedRandomForTest (MarcoFalke) fa80b08fef0eaa600339caa678fdf80a8aec3ce3 test: Revert to random path element (MarcoFalke) Pull request description: The randomness in the path element is required to allow a single fuzz test to run in parallel. Previous releases used a uint256 random value, but 10 random bytes should be sufficient as well, while avoiding a `MAX_PATH` violation on Windows. The issue was introduced by myself, by suggesting to use the current time in https://github.com/bitcoin/bitcoin/pull/31000#discussion_r1835351305. ACKs for top commit: kevkevinpal: reACK https://github.com/bitcoin/bitcoin/commit/faaaf59f71ede057b2c1d369ef8db973c2f2dbc2 hodlinator: ACK faaaf59f71ede057b2c1d369ef8db973c2f2dbc2 tdb3: re ACK faaaf59f71ede057b2c1d369ef8db973c2f2dbc2 dergoegge: ACK faaaf59f71ede057b2c1d369ef8db973c2f2dbc2 Tree-SHA512: f12256c8b353618291030f71bf36eab97a25ffeaa28e36a5f2c6718dfc1fbbc8548c71475edec53d59026f2a779a05778db83f0530dd3e1d1faf6e4fc0ee7d70
2024-11-19test: Make g_rng_temp_path rand, not dependent on SeedRandomForTestMarcoFalke
2024-11-18test: Revert to random path elementMarcoFalke
2024-11-17build: Temporarily disable compiling `fuzz/utxo_snapshot.cpp` with MSVCHennadii Stepanov
Visual Studio 2022 version 17.12 introduced a bug that causes an internal compiler error. See: https://github.com/bitcoin/bitcoin/issues/31303.
2024-11-14refactor: Avoid std::string format stringsMarcoFalke
Pass literal format strings instead of std::string so formats can be checked at compile time. Co-authored-by: Ryan Ofsky <ryan@ofsky.org> Co-authored-by: stickies-v <stickies-v@protonmail.com>
2024-11-14Merge bitcoin/bitcoin#31174: tinyformat: Add compile-time checking for ↵merge-script
literal format strings fe39acf88ff552bfc4a502c99774375b91824bb1 tinyformat: Add compile-time checking for literal format strings (Ryan Ofsky) 184f34f2d0fa2e56ad594966b2b99ff4cf840d95 util: Support dynamic width & precision in ConstevalFormatString (Ryan Ofsky) Pull request description: Add compile-time checking for literal format strings passed to `strprintf` and `tfm::format` to make sure the right number of format arguments are passed. There is still no compile-time checking if non-literal `std::string` or `bilingual_str` format strings are passed, but this is improved in other PRs: - [#31061](https://github.com/bitcoin/bitcoin/pull/31061) implements compile-time checking for translated strings - [#31072](https://github.com/bitcoin/bitcoin/pull/31072) increases compile-time checking by using literal strings as format strings, instead of `std::string` and `bilingual_str` - [#31149](https://github.com/bitcoin/bitcoin/pull/31149) may drop the `std::string` overload for `strprintf` to [require](https://github.com/bitcoin/bitcoin/pull/31149#issuecomment-2444579999) compile-time checking ACKs for top commit: maflcko: re-ACK fe39acf88ff552bfc4a502c99774375b91824bb1 🕐 l0rinc: ACK fe39acf88ff552bfc4a502c99774375b91824bb1 hodlinator: re-ACK fe39acf88ff552bfc4a502c99774375b91824bb1 Tree-SHA512: f1ddef0c96b9468c5ffe31b42dc19f1922583dd14f2e180b618d992c98614c5cc7db9f9cd917ef503f833bbc7dbec78e4489d0035416dce6840837e1d66d87cb
2024-11-14Merge bitcoin/bitcoin#31269: validation: Remove RECENT_CONSENSUS_CHANGE ↵merge-script
validation result e80e4c6ff91e27d7d40f099a2d7942c29085234c validation: Remove RECENT_CONSENSUS_CHANGE validation result (TheCharlatan) Pull request description: The *_RECENT_CONSENSUS_CHANGE variants in the validation result enumerations were always unused. They seem to have been kept around speculatively for a soft fork after segwit, however they were never used for taproot either. This points at them not having a clear purpose. Based on the original pull requests' comments their usage was never entirely clear: https://github.com/bitcoin/bitcoin/pull/11639#issuecomment-370234133 https://github.com/bitcoin/bitcoin/pull/15141#discussion_r271039747 Since they are part of the validation interface and need to be exposed by the kernel library keeping them around may also be confusing to future users of the library. ACKs for top commit: sipa: ACK e80e4c6ff91e27d7d40f099a2d7942c29085234c naumenkogs: ACK https://github.com/bitcoin/bitcoin/commit/e80e4c6ff91e27d7d40f099a2d7942c29085234c dergoegge: ACK e80e4c6ff91e27d7d40f099a2d7942c29085234c ajtowns: ACK e80e4c6ff91e27d7d40f099a2d7942c29085234c Tree-SHA512: 0af17c4435bb1b5a4f43600da30545cbbe95a7d642419cabdefabfb82b9335d92262c1c48be7ca2f2a024078ae9447161228b6f951d2f508a51159a31947fb54
2024-11-13Merge bitcoin/bitcoin#31000: bench: add support for custom data directoryAva Chow
fa66e0887ca1a1445d8b18ba1fadb12b2d911048 bench: add support for custom data directory (furszy) ad9c2cceda9cd893c0f754e49f7fca6e417ee95f test, bench: specialize working directory name (furszy) Pull request description: Expands the benchmark framework with the existing `-testdatadir` arg, enabling the ability to change the benchmark data directory. This is useful for running benchmarks on different storage devices, and not just under the OS `/tmp/` directory. A good use case is #28574, where we are benchmarking the wallet migration process on an HDD. ACKs for top commit: maflcko: re-ACK fa66e0887ca1a1445d8b18ba1fadb12b2d911048 achow101: ACK fa66e0887ca1a1445d8b18ba1fadb12b2d911048 tdb3: re ACK fa66e0887ca1a1445d8b18ba1fadb12b2d911048 hodlinator: re-ACK fa66e0887ca1a1445d8b18ba1fadb12b2d911048 pablomartin4btc: re-ACK fa66e0887ca1a1445d8b18ba1fadb12b2d911048 Tree-SHA512: 4e87206c07e26fe193c07074ae9eb0cc9c70a58aeea8cf27d18fb5425d77e4b00dbe0e6d6a75c17b427744e9066458b9a84e5ef7b0420f02a4fccb9c5ef4dacc
2024-11-13Ensure that we don't add duplicate transactions in rbf fuzz testsSuhas Daftuar
2024-11-13Move CalculateChunksForRBF() to the mempool changesetSuhas Daftuar
2024-11-13Move prioritisation into changesetSuhas Daftuar
2024-11-13Don't distinguish between direct conflicts and all conflicts when doing ↵Suhas Daftuar
cluster-size-2-rbf checks
2024-11-13Apply mempool changeset transactions directly into the mempoolSuhas Daftuar
Rather than individually calling addUnchecked for each transaction added in a changeset (after removing all the to-be-removed transactions), instead we can take advantage of boost::multi_index's splicing features to extract and insert entries directly from the staging multi_index into mapTx. This has the immediate advantage of saving allocation overhead for mempool entries which have already been allocated once. This also means that the memory locations of mempool entries will not change when transactions go from staging to the main mempool. Additionally, eliminate addUnchecked and require all new transactions to enter the mempool via a CTxMemPoolChangeSet.
2024-11-13test: Add unit test coverage of package rbf + prioritisetransactionSuhas Daftuar
2024-11-13Merge bitcoin/bitcoin#31235: addrman: cap the `max_pct` to not exceed the ↵merge-script
maximum number of addresses 9c5775c331e02dab06c78ecbb58488542d16dda7 addrman: cap the `max_pct` to not exceed the maximum number of addresses (brunoerg) Pull request description: Fixes #31234 This PR fixes a bad alloc issue in `GetAddresses` by capping the value `max_pct`. In practice, values greater than 100 should be treated as 100 since it's the percentage of addresses to return. Also, it limites the value `max_pct` in connman target to exercise values between 0 and 100. ACKs for top commit: adamandrews1: Code Review ACK https://github.com/bitcoin/bitcoin/commit/9c5775c331e02dab06c78ecbb58488542d16dda7 marcofleon: Tested ACK 9c5775c331e02dab06c78ecbb58488542d16dda7. Reproduced the crash on master and checked that this fixed it. The checks added to `GetAddr_` look reasonable. mzumsande: Code Review ACK 9c5775c331e02dab06c78ecbb58488542d16dda7 vasild: ACK 9c5775c331e02dab06c78ecbb58488542d16dda7 Tree-SHA512: 2957ae561ccc37df71f43c1863216d2e563522ea70b9a4baee6990e0b4a1ddadccabdcb9115c131a9a57480367b5ebdd03e0e3d4c8583792e2b7d1911a0a06d3
2024-11-12fuzz: Fix difficulty target generation in p2p_headers_presyncmarcofleon
The hardcoded nBits range would occasionally produce values for the difficulty target that were too low, causing the total work of the test chain to exceed MinimumChainWork. This fix uses ConsumeArithUInt256InRange to properly generate targets that will produce header chains with less work than MinimumChainWork.
2024-11-12test: unit test for CheckEphemeralSpendsGreg Sanders
2024-11-12fuzz: add ephemeral_package_eval harnessGreg Sanders
Works a bit harder to get ephemeral dust transactions into the mempool.