aboutsummaryrefslogtreecommitdiff
path: root/src/test/fuzz
AgeCommit message (Collapse)Author
2024-07-24refactor: Replace ParseHashStr with FromHexMarcoFalke
No need to have two functions with different names that achieve the exact same thing.
2024-07-23fuzz: Speed up PickValue in txorphanMarcoFalke
Co-Authored-By: l0rinc <pap.lorinc@gmail.com>
2024-07-22Fix lint-spelling warningsLőrinc
These warnings were often polluting the CI output, e.g. https://github.com/bitcoin/bitcoin/pull/30499/checks?check_run_id=27745036545 > ./test/lint/lint-spelling.py before the change: ``` doc/design/libraries.md:100: targetted ==> targeted doc/developer-notes.md:495: dependant ==> dependent src/bench/sign_transaction.cpp:49: hashIn ==> hashing, hash in src/bitcoin-chainstate.cpp:213: hashIn ==> hashing, hash in src/bitcoin-chainstate.cpp:213: hashIn ==> hashing, hash in src/coins.cpp:24: viewIn ==> viewing, view in src/coins.cpp:24: viewIn ==> viewing, view in src/coins.cpp:29: viewIn ==> viewing, view in src/coins.cpp:29: viewIn ==> viewing, view in src/coins.h:44: outIn ==> outing, out in src/coins.h:44: outIn ==> outing, out in src/coins.h:45: outIn ==> outing, out in src/coins.h:45: outIn ==> outing, out in src/coins.h:215: viewIn ==> viewing, view in src/coins.h:220: viewIn ==> viewing, view in src/primitives/transaction.h:37: hashIn ==> hashing, hash in src/primitives/transaction.h:37: hashIn ==> hashing, hash in src/protocol.cpp:51: hashIn ==> hashing, hash in src/protocol.cpp:51: hashIn ==> hashing, hash in src/protocol.h:497: hashIn ==> hashing, hash in src/qt/forms/optionsdialog.ui:344: incomin ==> incoming src/qt/optionsdialog.cpp:445: proxys ==> proxies src/rpc/mining.cpp:987: hashIn ==> hashing, hash in src/rpc/mining.cpp:987: hashIn ==> hashing, hash in src/script/interpreter.h:298: amountIn ==> amounting, amount in src/script/interpreter.h:298: amountIn ==> amounting, amount in src/script/interpreter.h:299: amountIn ==> amounting, amount in src/script/interpreter.h:299: amountIn ==> amounting, amount in src/script/sigcache.h:70: amountIn ==> amounting, amount in src/script/sigcache.h:70: amountIn ==> amounting, amount in src/signet.cpp:144: amountIn ==> amounting, amount in src/test/fuzz/util/net.cpp:386: occured ==> occurred src/test/fuzz/util/net.cpp:398: occured ==> occurred src/util/vecdeque.h:79: deques ==> dequeues src/util/vecdeque.h:160: deques ==> dequeues src/util/vecdeque.h:184: deques ==> dequeues src/util/vecdeque.h:194: deques ==> dequeues src/validation.cpp:2130: re-declared ==> redeclared src/validation.h:348: outIn ==> outing, out in src/validation.h:349: outIn ==> outing, out in test/functional/wallet_bumpfee.py:851: atleast ==> at least ```
2024-07-19fuzz: Deglobalize signature cache in sigcache testTheCharlatan
The body of the fuzz test should ideally be a pure function. If data is persisted in the cache over many iterations, and there is a crash, reproducing it from the input might be difficult.
2024-07-19fuzz: Limit parse_univalue input lengthMarcoFalke
2024-07-19fuzz: Use BasicTestingSetup for coins_view targetTheCharlatan
2024-07-19test: Add arguments for creating a slimmer setupTheCharlatan
Adds more testing options for creating an environment without networking and a validation interface. This is useful for improving the performance of the utxo snapshot fuzz test, which constructs a new TestingSetup on each iteration.
2024-07-16Merge bitcoin/bitcoin#28263: Add fuzz test for FSChaCha20Poly1305, ↵merge-script
AEADChacha20Poly1305 8607773750e60931e51a33e48cd077a1dedf9db3 Add fuzz test for FSChaCha20Poly1305 (stratospher) c807f3322897ca8c0da114556e5936e389da5059 Add fuzz test for AEADChacha20Poly1305 (stratospher) Pull request description: This PR adds fuzz tests for `AEADChaCha20Poly1305` and `FSChaCha20Poly1305` introduced in #28008. Run using: ``` $ FUZZ=crypto_aeadchacha20poly1305 src/test/fuzz/fuzz $ FUZZ=crypto_fschacha20poly1305 src/test/fuzz/fuzz ``` ACKs for top commit: dergoegge: tACK 8607773750e60931e51a33e48cd077a1dedf9db3 marcofleon: Tested ACK 8607773750e60931e51a33e48cd077a1dedf9db3. Ran both targets for ~200 CPU hours. Coverage of intended targets looks good to me. The simulation of damaged keys and checks that follow seem useful as well. Tree-SHA512: b6b85661d896e653caeed330f941fde665fc2bbd97ecd340808a3f365c469fe9134aa77316569a771dc36d1158cac1a5f76700bcfc45fff12aef07562e48feb9
2024-07-15Merge bitcoin/bitcoin#30407: test: [refactor] Pass TestOptsmerge-script
fa690c8e532672f7ab53be6f7a0bb3070858745e test: [refactor] Pass TestOpts (MarcoFalke) Pull request description: Currently optional test context setup settings are passed by adding a new optional argument to the constructors. For example `extra_args`. This is problematic, because: * Adding more optional settings in the future requires touching all affected constructors, increasing their verbosity. * Setting only a later option requires setting the earlier ones. * Clang-tidy named args passed to `std::make_unique` are not checked. Fix all issues by adding a new struct `TestOpts`, which holds all options. Notes: * The chain type is not an option in the struct for now, because the default values vary. * The struct holds all possible test options globally. Not all fields may be used by all constructors. Albeit harmless, it is up to the test author to not set a field that is unused. ACKs for top commit: kevkevinpal: utACK [fa690c8](https://github.com/bitcoin/bitcoin/pull/30407/commits/fa690c8e532672f7ab53be6f7a0bb3070858745e) dergoegge: utACK fa690c8e532672f7ab53be6f7a0bb3070858745e TheCharlatan: Nice, ACK fa690c8e532672f7ab53be6f7a0bb3070858745e Tree-SHA512: 8db8efa5dff854a73757d3f454f8f902e41bb4358f5f9bae29dbb3e251e20ee93489605de51d0822ba31d97835cd15526a29c075278dd6a8bbde26134feb4f49
2024-07-15Merge bitcoin/bitcoin#30197: fuzz: bound some miniscript operations to avoid ↵merge-script
fuzz timeouts bc34bc288824978ef4b98e8802b47cb863c8a8c2 fuzz: limit the number of nested wrappers in descriptors (Antoine Poinsot) 8d7340105f5299e9a45e84f1704b8b4545cb85f0 fuzz: limit the number of sub-fragments per fragment for descriptors (Antoine Poinsot) Pull request description: Some of the logic in the miniscript module is quadratic. It only becomes an issue for very large uninteresting descriptors (like a `thresh` with 130k sub-fragments or a fragment with more than 60k nested `j:` wrappers). This PR fixes the two types of fuzz timeouts reported by Marco in https://github.com/bitcoin/bitcoin/issues/28812 by trying to pinpoint the problematic descriptors through a simple analysis of the string, without limiting the size of the string itself. This is the same approach as was adopted for limiting the depth of derivation paths. ACKs for top commit: dergoegge: utACK bc34bc288824978ef4b98e8802b47cb863c8a8c2 stickies-v: Light ACK bc34bc288824978ef4b98e8802b47cb863c8a8c2 marcofleon: Code review ACK bc34bc288824978ef4b98e8802b47cb863c8a8c2. The added comments are useful, thanks for those. Tested on the three inputs in https://github.com/bitcoin/bitcoin/issues/28812 that caused the timeouts. Tree-SHA512: 8811c7b225684c5ecc1eb1256cf39dfa60d4518161e70210086c8a01b38927481ebe747af86aa5f4803187672d43fadabcfdfbf4e3b049738d629a25143f0e77
2024-07-15Add fuzz test for FSChaCha20Poly1305stratospher
2024-07-15Add fuzz test for AEADChacha20Poly1305stratospher
2024-07-15Merge bitcoin/bitcoin#30412: MiniMiner: use FeeFrac in AncestorFeerateComparatormerge-script
09370529fb9f6d06f6d16bdb1fb336f7a265d8ba fuzz: mini_miner_selection fixups. (glozow) de273d53004f48e4c8c965f7ce0bd375fd8d0d69 MiniMiner: use FeeFrac in AncestorFeerateComparator (glozow) Pull request description: Closes #30284. Closes #30367, see https://github.com/bitcoin/bitcoin/issues/30367#issuecomment-2217459257 Previously, we were only comparing feerates up to 1/1000 precision, since CFeeRate comparison just looks at their respective nSatoshisPerK. This could lead to MiniMiner selecting packages in the wrong order (i.e. by txid) if their feerates were less than 0.001sat/vB different. Fix this by creating + comparing `FeeFrac`s instead. Also, `FeeFrac::Mul` doesn't have the overflow problem. Also added a few minor fuzzer fixups that caught my eye while I was debugging this. ACKs for top commit: ismaelsadeeq: Tested ACK 09370529fb9f6d06f6d16bdb1fb336f7a265d8ba murchandamus: ACK 09370529fb9f6d06f6d16bdb1fb336f7a265d8ba with nits dergoegge: tACK 09370529fb9f6d06f6d16bdb1fb336f7a265d8ba Tree-SHA512: e5b6d6c3f7289f30cd8280d0a47cd852d0180b83d1b27ff9514f50c97103b0f069484e48cba2ca3a57419beadc1996c1b9dd8d0a0f34bc4f4223d8adaf414ce5
2024-07-14fuzz: limit the number of nested wrappers in descriptorsAntoine Poinsot
The script building logic performs a quadratic number of copies in the number of nested wrappers in the miniscript. Limit the number of nested wrappers to avoid fuzz timeouts. Thanks to Marco Falke for reporting the fuzz timeouts and providing a minimal input to reproduce.
2024-07-14fuzz: limit the number of sub-fragments per fragment for descriptorsAntoine Poinsot
This target may call into logic quadratic over the number of sub-fragments. Limit the number of sub-fragments to keep the runtime reasonable. Thanks to Marco Falke for reporting the fuzz timeouts with a minimized input.
2024-07-10util: Catch translation string errors at compile timeMarcoFalke
2024-07-09Merge bitcoin/bitcoin#30396: random: add benchmarks and drop unnecessary ↵Ava Chow
Shuffle function 6ecda04fefad980872c72fba89844393f5581120 random: drop ad-hoc Shuffle in favor of std::shuffle (Pieter Wuille) da28a26aae3178fb7663efbe20bb650857ace775 bench random: benchmark more functions, and add InsecureRandomContext (Pieter Wuille) 0a9bbc64c157a314e5472ecd98300e30b12d3fdf random bench refactor: move to new bench/random.cpp (Pieter Wuille) Pull request description: This adds benchmarks for various operations on `FastRandomContext` and `InsecureRandomContext`, and then removes the ad-hoc `Shuffle` functions, now that it appears that standard library `std::shuffle` has comparable performance. The other reason for keeping `Shuffle`, namely the fact that libstdc++ used self-move (which debug mode panics on) has been fixed as well (see https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1658344049). ACKs for top commit: achow101: ACK 6ecda04fefad980872c72fba89844393f5581120 hodlinator: ACK 6ecda04fefad980872c72fba89844393f5581120 dergoegge: Code review ACK 6ecda04fefad980872c72fba89844393f5581120 Tree-SHA512: 2560b7312410581ff2b9bd0716e0f1558d910b5eadb9544785c972384985ac0f11f72d6b2797cfe2e7eb71fa57c30cffd98cc009cb4ee87a18b1524694211417
2024-07-09Merge bitcoin/bitcoin#30329: fuzz: improve utxo_snapshot targetRyan Ofsky
de71d4dece604907afc4fc26b7788e9c1a4cbecb fuzz: improve utxo_snapshot target (Martin Zumsande) Pull request description: Add the possibility of giving more guidance to the creation of the metadata and/or coins, so that the fuzzer gets the chance to reach more error conditions in ActivateSnapshot and sometimes successfully creates a valid snapshot. This also changes the asserts for the success case that were outdated (after #29370) and only didn't result in a crash because the fuzzer wasn't able to reach this code before. ACKs for top commit: maflcko: re-ACK de71d4dece604907afc4fc26b7788e9c1a4cbecb 🎆 fjahr: utACK de71d4dece604907afc4fc26b7788e9c1a4cbecb TheCharlatan: ACK de71d4dece604907afc4fc26b7788e9c1a4cbecb Tree-SHA512: 346974d594164544d8cd3df7d8362c905fd93116215e9f5df308dfdac55bab04d727bfd7fd001cf11318682d11ee329b4b4a43308124c04d64b67840ab8a58a0
2024-07-09fuzz: mini_miner_selection fixups.glozow
Delete asserts that are redundant with the == assert. Add assertion that the coinbase isn't already in mock_template_txids.
2024-07-08Merge bitcoin/bitcoin#30141: kernel: De-globalize validation cachesRyan Ofsky
606a7ab862470413ced400aa68a94fd37c8ad3d3 kernel: De-globalize signature cache (TheCharlatan) 66d74bfc45ae0f743084475ac3bbfb4355bb6ec2 Expose CSignatureCache class in header (TheCharlatan) 021d38822c0e6a1b9497bcb20401c5c37e1bb84d kernel: De-globalize script execution cache hasher (TheCharlatan) 13a3661aba95b54b822c99ecbb695b14a22536d2 kernel: De-globalize script execution cache (TheCharlatan) ab14d1d6a4a8ef5fe5013150e6c5ebcb5f5e4ea9 validation: Don't error if maxsigcachesize exceeds uint32::max (TheCharlatan) Pull request description: The validation caches are currently setup independently from where the rest of the validation code is initialized. This makes their ownership semantics unclear. There is also no clear enforcement on when and in what order they need to be initialized. The caches are always initialized in the `BasicTestingSetup` although a number of tests don't actually need them. Solve this by moving the caches from global scope into the `ChainstateManager` class. This simplifies the usage of the kernel library by no longer requiring manual setup of the caches prior to using the `ChainstateManager`. Tests that need to access the caches can instantiate them independently. --- This pull request is part of the [libbitcoinkernel project](https://github.com/bitcoin/bitcoin/issues/27587). ACKs for top commit: stickies-v: re-ACK 606a7ab862470413ced400aa68a94fd37c8ad3d3 glozow: reACK 606a7ab ryanofsky: Code review ACK 606a7ab862470413ced400aa68a94fd37c8ad3d3. Just small formatting, include, and static_assert changes since last review. Tree-SHA512: e7f3ee41406e3b233832bb67dc3a63c4203b5367e5daeed383df9cb590f227fcc62eae31311029c077d5e81b273a37a88a364db3dee2efe91bb3b9c9ddc8a42e
2024-07-08Merge bitcoin/bitcoin#30263: build: Bump clang minimum supported version to 16merge-script
fa8f53273c7e5965620d31a8c3fe5f223cb76888 refactor: Remove no longer needed clang-15 workaround for std::span (MarcoFalke) 9999dbc1bd171931f02266d7c1a5cfd39f49238e fuzz: Clarify Apple-Clang-16 workaround (MarcoFalke) fa7462c67ab9b6d45484ce92b44d03f812627d6e build: Bump clang minimum supported version to 16 (MarcoFalke) Pull request description: Most supported operating systems ship with clang-16 (or later), so bump the minimum to that and allow new code to drop workarounds for previous clang bugs. For reference: * https://packages.debian.org/bookworm/clang-16 * https://packages.ubuntu.com/noble/clang (clang-18) * CentOS-like 8/9 Stream: All Clang versions from 16 to 17 * FreeBSD 12/13: All Clang versions from 16 to 18 * OpenSuse Tumbleweed ships with https://software.opensuse.org/package/clang (`clang18`); No idea about OpenSuse Leap On operating systems where the clang version is not shipped by default, the user would have to use GCC, or install clang in a different way. For example: * https://packages.debian.org/bookworm/g++ (g++-12) * https://packages.ubuntu.com/jammy/g++ (g++-11) * https://apt.llvm.org/, or nix, or guix, or compile clang from source, ... **Ubuntu 22.04 LTS does not ship with clang-16**, so one of the above workarounds is needed there. macOS 13 is unaffected, and the previous minimum requirement of Xcode15.0 remains, see also https://github.com/bitcoin/bitcoin/blame/b1ba1b178f501daa1afdd91f9efec34e5ec1e294/.github/workflows/ci.yml#L93. For macOS 11 (Big Sur) and 12 (Monterey) you need to install a more recent version of llvm, this remains unchanged as well, see https://github.com/bitcoin/bitcoin/blame/b1ba1b178f501daa1afdd91f9efec34e5ec1e294/doc/build-osx.md#L54. ACKs for top commit: hebasto: ACK fa8f53273c7e5965620d31a8c3fe5f223cb76888, I have reviewed the code and it looks OK. TheCharlatan: Re-ACK fa8f53273c7e5965620d31a8c3fe5f223cb76888 stickies-v: ACK fa8f53273c7e5965620d31a8c3fe5f223cb76888 Tree-SHA512: 18b79f88301a63bb5e367d2f52fffccd5fb84409061800158e51051667f6581a4cd71d4859d4cfa6d23e47e92963ab637e5ad87e3170ed23b5bebfbe99e759e2
2024-07-08test: [refactor] Pass TestOptsMarcoFalke
2024-07-06random: drop ad-hoc Shuffle in favor of std::shufflePieter Wuille
Benchmarks show it is no longer faster with modern standard C++ libraries, and the debug-mode failure due to self-move has been fixed as well.
2024-07-05kernel: De-globalize signature cacheTheCharlatan
Move its ownership to the ChainstateManager class. Next to simplifying usage of the kernel library by no longer requiring manual setup of the cache prior to using validation code, it also slims down the amount of memory allocated by BasicTestingSetup. Use this opportunity to make SignatureCache RAII styled Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
2024-07-04fuzz: improve utxo_snapshot targetMartin Zumsande
Add the possibility of giving more guidance to the creation of the metadata and/or coins, so that the fuzzer gets the chance to reach more error conditions in ActivateSnapshot and sometimes successfully creates a valid snapshot. This also changes the asserts for the success case that were outdated, and only didn't result in a crash because the fuzzer wasn't able to reach this code before.
2024-07-04Merge bitcoin/bitcoin#29625: Several randomness improvementsmerge-script
ce8094246ee95232e9d84f7e37f3c0a43ef587ce random: replace construct/assign with explicit Reseed() (Pieter Wuille) 2ae392d561ecfdf81855e6df6b9ad3d8843cdfa2 random: use LogError for init failure (Pieter Wuille) 97e16f57042cab07e5e73f6bed19feec2006e4f7 tests: make fuzz tests (mostly) deterministic with fixed seed (Pieter Wuille) 2c91330dd68064e402e8eceea3df9474bb7afd48 random: cleanup order, comments, static (Pieter Wuille) 8e31cf9c9b5e9fdd01e8b220c08a3ccde5cf584c net, net_processing: use existing RNG objects more (Pieter Wuille) d5fcbe966bc501db8bf6a3809633f0b82e6ae547 random: improve precision of MakeExponentiallyDistributed (Pieter Wuille) cfb0dfe2cf0b46f3ea9e62992ade989860f086c8 random: convert GetExponentialRand into rand_exp_duration (Pieter Wuille) 4eaa239dc3e189369d59144b524cb2808cbef8c3 random: convert GetRand{Micros,Millis} into randrange (Pieter Wuille) 82de1b80d95fc9447e64c098dcadb6b8a2f1f2ee net: use GetRandMicros for cache expiration (Pieter Wuille) ddc184d999d7e1a87efaf6bcb222186f0dcd87ec random: get rid of GetRand by inlining (Pieter Wuille) e2d1f84858485650ff743753ffa5c679f210a992 random: make GetRand() support entire range (incl. max) (Pieter Wuille) 810cdf6b4e12a1fdace7998d75b4daf8b67d7028 tests: overhaul deterministic test randomness (Pieter Wuille) 6cfdc5b104caf9952393f9dac2a36539d964077f random: convert XoRoShiRo128PlusPlus into full RNG (Pieter Wuille) 8cc2f45065fc1864f879248d1e1444588e27076b random: move XoRoShiRo128PlusPlus into random module (Pieter Wuille) 8f5ac0d0b608bdf396d8f2d758a792f869c2cd2a xoroshiro128plusplus: drop comment about nonexisting copy() (Pieter Wuille) 8924f5120f66269c04633167def01f82c74ea730 random: modernize XoRoShiRo128PlusPlus a bit (Pieter Wuille) ddb7d26cfd96c1f626def4755e0e1b5aaac94d3e random: add RandomMixin::randbits with compile-known bits (Pieter Wuille) 21ce9d8658fed0d3e4552e8b02a6902cb31c572e random: Improve RandomMixin::randbits (Pieter Wuille) 9b14d3d2da05f74ffb6a2ac20b7d9efefbe29634 random: refactor: move rand* utilities to RandomMixin (Pieter Wuille) 40dd86fc3b60d7a67a9720a84a685f16e3f05b06 random: use BasicByte concept in randbytes (Pieter Wuille) 27cefc7fd6a6a159779f572f4c3a06170f955ed8 random: add a few noexcepts to FastRandomContext (Pieter Wuille) b3b382dde202ad508baf553817c5b38fdd2d4a0c random: move rand256() and randbytes() to .h file (Pieter Wuille) 493a2e024e845e623e202e3eefe1cc2010e9b514 random: write rand256() in function of fillrand() (Pieter Wuille) Pull request description: This PR contains a number of vaguely-related improvements to the random module. The specific changes and more detailed rationale is in the commit messages, but the highlights are: * `XoRoShiRo128PlusPlus` (previously a test-only RNG) moves to random.h and becomes `InsecureRandomContext`, which is even faster than `FastRandomContext` but non-cryptographic. It also gets all helper randomness functions (`randrange`, `fillrand`, ...), making it a lot more succinct to use. * During tests, **all** randomness is made deterministic (except for `GetStrongRandBytes`) but non-repeating (like `GetRand()` used to be when `g_mock_deterministic_tests` was used), either fixed, or from a random seed (overridden by env var). * Several infrequently used top-level functions (`GetRandMillis`, `GetRandMicros`, `GetExponentialRand`) are converted into member functions of `FastRandomContext` (and `InsecureRandomContext`). * `GetRand<T>()` (without argument) can now return the maximum value of the type (previously e.g. `GetRand<uint32_t>()` would never return 0xffffffff). ACKs for top commit: achow101: ACK ce8094246ee95232e9d84f7e37f3c0a43ef587ce maflcko: re-ACK ce8094246ee95232e9d84f7e37f3c0a43ef587ce 🐈 hodlinator: ACK ce8094246ee95232e9d84f7e37f3c0a43ef587ce dergoegge: utACK ce8094246ee95232e9d84f7e37f3c0a43ef587ce Tree-SHA512: 79bc0cbafaf27e95012c1ce2947a8ca6f9a3c78af5f1f16e69354b6fc9b987a28858adf4cd356dc5baf21163e9af8dcc24e70f8d7173be870e8a3ddcdd47c02c
2024-07-02Merge bitcoin/bitcoin#30272: doc: use TRUC instead of v3 and add release noteAva Chow
926b8e39dcbc0a3a8a75ef0a29bdca2bf738d746 [doc] add release note for TRUC (glozow) 19a9b90617419f68d0f1c90ee115b5220be99a16 use version=3 instead of v3 in debug strings (glozow) 881fac8e609be17eb71bd9a54c0284b304e2e2e2 scripted-diff: change names from V3 to TRUC (glozow) a573dd261748d2a80560f73db08f7dca788c7fcf [doc] replace mentions of v3 with TRUC (glozow) 089b5757dff39a9a06cdb625aaced9beeb72958d rename mempool_accept_v3.py to mempool_truc.py (glozow) f543852a89d93441645250c40c3980aeb0c3b664 rename policy/v3_policy.* to policy/truc_policy.* (glozow) Pull request description: Adds a release note for TRUC policy which will be live in v28.0. For clarity, replaces mentions of "v3" with "TRUC" in most places. Suggested in - https://github.com/bitcoin/bitcoin/pull/29496#discussion_r1629749583 - https://github.com/bitcoin/bitcoin/pull/29496#discussion_r1624500904 I changed error strings from "v3-violation" to "TRUC-violation" but left v3 in the debug strings because I think it might be clearer for somebody who is debugging. Similarly, I left some variables unchanged because I think they're more descriptive this way, e.g. `tx_v3_from_v2_and_v3`. I'm happy to debate places that should or shouldn't be documented differently in this PR, whatever is clearest to everyone. ACKs for top commit: instagibbs: reACK https://github.com/bitcoin/bitcoin/pull/30272/commits/926b8e39dcbc0a3a8a75ef0a29bdca2bf738d746 achow101: ACK 926b8e39dcbc0a3a8a75ef0a29bdca2bf738d746 ismaelsadeeq: Code review ACK 926b8e39dcbc0a3a8a75ef0a29bdca2bf738d746 Tree-SHA512: 16c88add0a29dc6d1236c4d45f34a17b850f6727b231953cbd52eb9f7268d1d802563eadfc8b7928c94ed3d7a615275dd103e57e81439ebf3ba2b12efa1e42af
2024-07-02Merge bitcoin/bitcoin#30267: assumeutxo: Check snapshot base block is not in ↵Ava Chow
invalid chain 2f9bde69f45c7a9fdcf0c65f9e1305391a6f1f28 test: Remove unnecessary restart in assumeutxo test (Fabian Jahr) 19ce3d407ef546fa50d18b2ffbd67b7417797064 assumeutxo: Check snapshot base block is not marked invalid (Fabian Jahr) 80315c011863d69e7785673283e4c9033fbcd5ac refactor: Move early loadtxoutset checks into ActiveSnapshot (Fabian Jahr) Pull request description: This was discovered in a discussion in #29996 If the base block of the snapshot is marked invalid or part of an invalid chain, we currently still load the snapshot and get stuck in a weird state where we have the snapshot chainstate but it will never connect to our valid chain. While this scenario is highly unlikely to occur on mainnet, it still seems good to prevent this inconsistent state. The behavior change described above is in the second commit. The first commit refactors the early checks in the `loadtxoutset` RPC by moving them into `ActivateSnapshot()` in order to have the chance to cover them by unit tests in the future and have a more consistent interface. Previously checks were spread out between `rpc/blockchain.cpp` and `validation.cpp`. In order to be able to return the error message to users of the RPC, the return type of `ActivateSnapshot()` is changed from `bool` to `util::Result`. The third commit removes an unnecessary restart introduced in #29428. ACKs for top commit: mzumsande: re-ACK 2f9bde6 alfonsoromanz: Re-ACK 2f9bde69f45c7a9fdcf0c65f9e1305391a6f1f28. The RPC code looks much cleaner after the refactor. Also, it seems very useful to get the error message in the RPC response rather than having to rely on the logs in some scenarios if you are an RPC user. achow101: ACK 2f9bde69f45c7a9fdcf0c65f9e1305391a6f1f28 Tree-SHA512: 5328dd88c3c7be3f1be97c9eef52ac3666c27188c30a798b3e949f3ffcb83be075127c107e4046f7f39f961a79911ea3d61b61f3c11e451b3e4c541c264eeed4
2024-07-02scripted-diff: change names from V3 to TRUCglozow
-BEGIN VERIFY SCRIPT- sed -i 's/SingleV3Checks/SingleTRUCChecks/g' $(git grep -l 'SingleV3Checks') sed -i 's/PackageV3Checks/PackageTRUCChecks/g' $(git grep -l 'PackageV3Checks') sed -i 's/PV3C/PTRUCC/g' src/policy/truc_policy.h sed -i 's/V3_MAX_VSIZE/TRUC_MAX_VSIZE/g' $(git grep -l 'V3_MAX_VSIZE') sed -i 's/V3_CHILD_MAX_VSIZE/TRUC_CHILD_MAX_VSIZE/g' $(git grep -l 'V3_CHILD_MAX_VSIZE') sed -i 's/V3_DESCENDANT_LIMIT/TRUC_DESCENDANT_LIMIT/g' $(git grep -l 'V3_DESCENDANT_LIMIT') sed -i 's/V3_ANCESTOR_LIMIT/TRUC_ANCESTOR_LIMIT/g' $(git grep -l 'V3_ANCESTOR_LIMIT') sed -i 's/CheckMempoolV3Invariants/CheckMempoolTRUCInvariants/g' $(git grep -l 'CheckMempoolV3Invariants') -END VERIFY SCRIPT-
2024-07-02[doc] replace mentions of v3 with TRUCglozow
Keep mentions of v3 in debug strings to help people who might not know that TRUC is applied when version=3. Also keep variable names in tests, as it is less verbose to keep v3 and v2.
2024-07-02Merge bitcoin/bitcoin#30344: kernel: remove mempool_persistglozow
f1478c05458562a9bef5c2ba43959d758e7b4745 mempool: move LoadMempool/DumpMempool to node (Cory Fields) 6d242ff1e9ca02fd8f5cb3ffe82dfb48a52366cc kernel: remove mempool_persist.cpp (Cory Fields) Pull request description: DumpMempool/LoadMempool are not necessary for the kernel. Noticed while working on instantiated logging. I suppose these could have been left in on purpose, but I'm assuming it was probably just an oversight. ACKs for top commit: TheCharlatan: Re-ACK f1478c05458562a9bef5c2ba43959d758e7b4745 glozow: ACK f1478c0545 stickies-v: ACK f1478c05458562a9bef5c2ba43959d758e7b4745 Tree-SHA512: 5825da0cf2e67470524eb6ebe397eb90755a368469a25f184df99ab935b3eb6d89eb802b41a6c3661e869bba3bbfa8ba9d95281bc75ebbf790ec5d9d1f79c66f
2024-07-01random: replace construct/assign with explicit Reseed()Pieter Wuille
2024-07-01tests: make fuzz tests (mostly) deterministic with fixed seedPieter Wuille
2024-07-01tests: overhaul deterministic test randomnessPieter Wuille
The existing code provides two randomness mechanisms for test purposes: - g_insecure_rand_ctx (with its wrappers InsecureRand*), which during tests is initialized using either zeros (SeedRand::ZEROS), or using environment-provided randomness (SeedRand::SEED). - g_mock_deterministic_tests, which controls some (but not all) of the normal randomness output if set, but then makes it extremely predictable (identical output repeatedly). Replace this with a single mechanism, which retains the SeedRand modes to control all randomness. There is a new internal deterministic PRNG inside the random module, which is used in GetRandBytes() when in test mode, and which is also used to initialize g_insecure_rand_ctx. This means that during tests, all random numbers are made deterministic. There is one exception, GetStrongRandBytes(), which even in test mode still uses the normal PRNG state. This probably opens the door to removing a lot of the ad-hoc "deterministic" mode functions littered through the codebase (by simply running relevant tests in SeedRand::ZEROS mode), but this isn't done yet.
2024-07-01random: convert XoRoShiRo128PlusPlus into full RNGPieter Wuille
Convert XoRoShiRo128PlusPlus into a full RandomMixin-based RNG class, providing all utility functionality that FastRandomContext has. In doing so, it is renamed to InsecureRandomContext, highlighting its non-cryptographic nature. To do this, a fillrand fallback is added to RandomMixin (where it is used by InsecureRandomContext), but FastRandomContext still uses its own fillrand.
2024-07-01random: move XoRoShiRo128PlusPlus into random modulePieter Wuille
This is preparation for making it more generally accessible.
2024-07-01Merge bitcoin/bitcoin#30237: test: Add Compact Block Encoding test ↵glozow
`ReceiveWithExtraTransactions` covering non-empty `extra_txn` 55eea003af24169c883e1761beb997e151845225 test: Make blockencodings_tests deterministic (AngusP) 4c99301220ab44e98d0d0e1cc8d774d96a25b7aa test: Add ReceiveWithExtraTransactions Compact Block receive test. (AngusP) 4621e7cc8f8e2b71393a2b30d5dbe56165bfb854 test: refactor: Rename extra_txn to const empty_extra_txn as it is empty in all test cases (AngusP) Pull request description: This test uses the `extra_txn` (`vExtraTxnForCompact`) vector of optional orphan/conflicted/etc. transactions to provide transactions to a PartiallyDownloadedBlock that are not otherwise present in the mempool, and check that they are used. This also covers a former nullptr deref bug that was fixed in #29752 (bf031a517c79cec5b43420bcd40291ab0e9f68a8) where the `extra_txn` vec/circular-buffer was null-initialized and not yet filled when dereferenced in `PartiallyDownloadedBlock::InitData`. ACKs for top commit: marcofleon: Code review ACK 55eea003af24169c883e1761beb997e151845225. I ran the `blockencodings` unit test and no issues with the new test case. dergoegge: Code review ACK 55eea003af24169c883e1761beb997e151845225 glozow: ACK 55eea003af24169c883e1761beb997e151845225 Tree-SHA512: d7909c212bb069e1f6184b26390a5000dcc5f2b18e49b86cceccb9f1ec4f874dd43bc9bc92abd4207c71dd78112ba58400042c230c42e93afe55ba51b943262c
2024-07-01Merge bitcoin/bitcoin#30273: fuzz: FuzzedSock::Recv() don't lose bytes from ↵merge-script
MSG_PEEK read 4d81b4de339efbbb68c9785203b699e6e12ecd83 fuzz: FuzzedSock::Recv() don't lose bytes from MSG_PEEK read (Vasil Dimov) b51d75ea97ee0d01ee586e40a30cb68c0bf7ffd3 fuzz: simplify FuzzedSock::m_peek_data (Vasil Dimov) Pull request description: Problem: If `FuzzedSock::Recv(N, MSG_PEEK)` is called then `N` bytes would be retrieved from the fuzz provider, saved in `m_peek_data` and returned to the caller (ok). If after this `FuzzedSock::Recv(M, 0)` is called where `M < N` then the first `M` bytes from `m_peek_data` would be returned to the caller (ok), but the remaining `N - M` bytes in `m_peek_data` would be discarded/lost (not ok). They must be returned by a subsequent `Recv()`. To resolve this, only remove the head `N` bytes from `m_peek_data`. --- This is a followup to https://github.com/bitcoin/bitcoin/pull/30211, more specifically: https://github.com/bitcoin/bitcoin/pull/30211#discussion_r1633199919 https://github.com/bitcoin/bitcoin/pull/30211#discussion_r1633216366 ACKs for top commit: marcofleon: ACK 4d81b4de339efbbb68c9785203b699e6e12ecd83. Tested this with the I2P fuzz target and there's no loss in coverage. I think overall this is an improvement in the robustness of `Recv` in `FuzzedSock`. dergoegge: Code review ACK 4d81b4de339efbbb68c9785203b699e6e12ecd83 brunoerg: utACK 4d81b4de339efbbb68c9785203b699e6e12ecd83 Tree-SHA512: 73b5cb396784652447874998850e45899e8cba49dcd2cc96b2d1f63be78e48201ab88a76cf1c3cb880abac57af07f2c65d673a1021ee1a577d0496c3a4b0c5dd
2024-06-26mempool: move LoadMempool/DumpMempool to nodeCory Fields
2024-06-26fuzz: Clarify Apple-Clang-16 workaroundMarcoFalke
2024-06-20Merge bitcoin/bitcoin#30202: netbase: extend CreateSock() to support ↵Ava Chow
creating arbitrary sockets 1245d1388b003c46092937def7041917aecec8de netbase: extend CreateSock() to support creating arbitrary sockets (Vasil Dimov) Pull request description: Allow the callers of `CreateSock()` to pass all 3 arguments to the `socket(2)` syscall. This makes it possible to create sockets of any domain/type/protocol. In addition to extending arguments, some extra safety checks were put in place. The need for this came up during the discussion in https://github.com/bitcoin/bitcoin/pull/30043#discussion_r1618837102 ACKs for top commit: achow101: ACK 1245d1388b003c46092937def7041917aecec8de tdb3: re ACK 1245d1388b003c46092937def7041917aecec8de theStack: re-ACK 1245d1388b003c46092937def7041917aecec8de Tree-SHA512: cc86b56121293ac98959aed0ed77812d20702ed7029b5a043586f46e74295779c5354bb0d5f9e80be6c29e535df980d34c1dbf609064fb7ea3e5ca0f0ed54d6b
2024-06-19test: Make blockencodings_tests deterministicAngusP
refactor: CBlockHeaderAndShortTxIDs constructor now always takes an explicit nonce. test: Make blockencodings_tests deterministic using fixed seed providing deterministic CBlockHeaderAndShortTxID nonces and dummy transaction IDs. Fixes very rare flaky test failures, where the ShortIDs of test transactions collide, leading to `READ_STATUS_FAILED` from PartiallyDownloadedBlock::InitData and/or `IsTxAvailable` giving `false` when the transaction should actually be available. * Use a new `FastRandomContext` with a fixed seed in each test, to ensure 'random' uint256s used as fake prevouts are deterministic, so in-turn test txids and short IDs are deterministic and don't collide causing very rare but flaky test failures. * Add new test-only/internal initializer for `CBlockHeaderAndShortTxIDs` that takes a specified nonce to further ensure determinism and avoid rare but undesireable short ID collisions. In a test context this nonce is set to a fixed known-good value. Normally it is random, as previously. Flaky test failures can be reproduced with: ```patch diff --git a/src/blockencodings.cpp b/src/blockencodings.cpp index 695e8d806a..64d635a97a 100644 --- a/src/blockencodings.cpp +++ b/src/blockencodings.cpp @@ -44,7 +44,8 @@ void CBlockHeaderAndShortTxIDs::FillShortTxIDSelector() const { uint64_t CBlockHeaderAndShortTxIDs::GetShortID(const Wtxid& wtxid) const { static_assert(SHORTTXIDS_LENGTH == 6, "shorttxids calculation assumes 6-byte shorttxids"); - return SipHashUint256(shorttxidk0, shorttxidk1, wtxid) & 0xffffffffffffL; + // return SipHashUint256(shorttxidk0, shorttxidk1, wtxid) & 0xffffffffffffL; + return SipHashUint256(shorttxidk0, shorttxidk1, wtxid) & 0x0f; } ``` to increase the likelihood of a short ID collision; and running ```shell set -e; n=0; while (( n++ < 5000 )); do src/test/test_bitcoin --run_test=blockencodings_tests; done ```
2024-06-19refactor: Move early loadtxoutset checks into ActiveSnapshotFabian Jahr
Also changes the return type of ActiveSnapshot to allow returning the error message to the user of the loadtxoutset RPC.
2024-06-18fuzz: have package_rbf always make small txnsGreg Sanders
The fuzz target is generating a large amount of transactions, but the core of the logic is ConsumeTxMemPoolEntry making the mempool entries for adding to the mempool. Since ConsumeTxMemPoolEntry generates its own transaction "vsize", we can improve efficiency of the target by explicitly creating very small transactions, reducing the hashing and memory burden.
2024-06-18rename policy/v3_policy.* to policy/truc_policy.*glozow
2024-06-17Merge bitcoin/bitcoin#28984: Cluster size 2 package rbfAva Chow
94ed4fbf8e1a396c650b5134d396d6c0be35ce10 Add release note for size 2 package rbf (Greg Sanders) afd52d8e63ed323a159ea49fd1f10542abeacb97 doc: update package RBF comment (Greg Sanders) 6e3c4394cfadf32c06c8c4732d136ca10c316721 mempool: Improve logging of replaced transactions (Greg Sanders) d3466e4cc5051c314873dd14ec8f7a88494c0780 CheckPackageMempoolAcceptResult: Check package rbf invariants (Greg Sanders) 316d7b63c97144ba3e21201315c784852210f8ff Fuzz: pass mempool to CheckPackageMempoolAcceptResult (Greg Sanders) 4d15bcf448eb3c4451b63e8f78cc61f3f9f9b639 [test] package rbf (glozow) dc21f61c72e5a97d974ca2c5cb70b8328f4fab2a [policy] package rbf (Suhas Daftuar) 5da396781589177d4ceb3b4b59c9f309a5e4d029 PackageV3Checks: Relax assumptions (Greg Sanders) Pull request description: Allows any 2 transaction package with no in-mempool ancestors to do package RBF when directly conflicting with other mempool clusters of size two or less. Proposed validation steps: 1) If the transaction package is of size 1, legacy rbf rules apply. 2) Otherwise the transaction package consists of a (parent, child) pair with no other in-mempool ancestors (or descendants, obviously), so it is also going to create a cluster of size 2. If larger, fail. 3) The package rbf may not evict more than 100 transactions from the mempool(bip125 rule 5) 4) The package is a single chunk 5) Every directly conflicted mempool transaction is connected to at most 1 other in-mempool transaction (ie the cluster size of the conflict is at most 2). 6) Diagram check: We ensure that the replacement is strictly superior, improving the mempool 7) The total fee of the package, minus the total fee of what is being evicted, is at least the minrelayfee * size of the package (equivalent to bip125 rule 3 and 4) Post-cluster mempool this will likely be expanded to general package rbf, but this is what we can safely support today. ACKs for top commit: achow101: ACK 94ed4fbf8e1a396c650b5134d396d6c0be35ce10 glozow: reACK 94ed4fbf8e via range-diff ismaelsadeeq: re-ACK 94ed4fbf8e1a396c650b5134d396d6c0be35ce10 theStack: Code-review ACK 94ed4fbf8e1a396c650b5134d396d6c0be35ce10 murchandamus: utACK 94ed4fbf8e1a396c650b5134d396d6c0be35ce10 Tree-SHA512: 9bd383e695964f362f147482bbf73b1e77c4d792bda2e91d7f30d74b3540a09146a5528baf86854a113005581e8c75f04737302517b7d5124296bd7a151e3992
2024-06-14fuzz: FuzzedSock::Recv() don't lose bytes from MSG_PEEK readVasil Dimov
Problem: If `FuzzedSock::Recv(N, MSG_PEEK)` is called then `N` bytes would be retrieved from the fuzz provider, saved in `m_peek_data` and returned to the caller (ok). If after this `FuzzedSock::Recv(M, 0)` is called where `M < N` then the first `M` bytes from `m_peek_data` would be returned to the caller (ok), but the remaining `N - M` bytes in `m_peek_data` would be discarded/lost (not ok). They must be returned by a subsequent `Recv()`. To resolve this, only remove the head `N` bytes from `m_peek_data`.
2024-06-14fuzz: simplify FuzzedSock::m_peek_dataVasil Dimov
`FuzzedSock::m_peek_data` need not be an optional of a vector. It can be just a vector whereas an empty vector denotes "no peek data".
2024-06-14netbase: extend CreateSock() to support creating arbitrary socketsVasil Dimov
Allow the callers of `CreateSock()` to pass all 3 arguments to the `socket(2)` syscall. This makes it possible to create sockets of any domain/type/protocol.
2024-06-13Fuzz: pass mempool to CheckPackageMempoolAcceptResultGreg Sanders
generated by cgit v1.2.3 (git 2.26.2) at 2025-03-16 15:17:13 +0000