fuzz: add mempool_utils.cpp #26250

1. 
   fanquake commented at 2:49 pm on October 4, 2022: member

   Moving the heavy (Boost) mempool code out of fuzz/util.h. Means that (for ex) a crypto_common fuzz unit doesn’t need to care about seeing endless Boost headers. This results in a ~10% speedup (for me) when compiling the fuzz tests. Your results may vary.
2. fanquake added the label Tests on Oct 4, 2022
3. in src/Makefile.test_fuzz.include:20 in 53366df01e outdated

   16@@ -17,5 +17,6 @@ libtest_fuzz_a_CPPFLAGS = $(AM_CPPFLAGS) $(BITCOIN_INCLUDES) $(BOOST_CPPFLAGS)
   17 libtest_fuzz_a_CXXFLAGS = $(AM_CXXFLAGS) $(PIE_FLAGS)
   18 libtest_fuzz_a_SOURCES = \
   19   test/fuzz/fuzz.cpp \
   20+  test/fuzz/mempool_utils.cpp \
   

   ---

   ---

   

   maflcko commented at 3:06 pm on October 4, 2022:

   I wonder if we can nest more folders. Maybe src/test/fuzz/util/mempool.cpp to differentiate utils from tests?

   ---

   

   fanquake commented at 4:53 pm on October 4, 2022:

   Added some nesting.
4. in src/test/fuzz/mempool_utils.cpp:20 in 53366df01e outdated

   15+{
   16+    // Avoid:
   17+    // policy/feerate.cpp:28:34: runtime error: signed integer overflow: 34873208148477500 * 1000 cannot be represented in type 'long'
   18+    //
   19+    // Reproduce using CFeeRate(348732081484775, 10).GetFeePerK()
   20+    const CAmount fee = std::min<CAmount>(ConsumeMoney(fuzzed_data_provider), std::numeric_limits<CAmount>::max() / static_cast<CAmount>(100000));
   

   ---

   ---

   

   maflcko commented at 3:08 pm on October 4, 2022:

   unrelated:

   0    const CAmount fee{ConsumeMoney(fuzzed_data_provider, /*max=*/std::numeric_limits<CAmount>::max() / CAmount{100'000})};
   

   It should be possible to just pass the max. (Can do in a separate commit, if you want)

   ---

   

   fanquake commented at 4:53 pm on October 4, 2022:

   Added another commit for this.
5. 
   maflcko commented at 3:08 pm on October 4, 2022: member

   lgtm
6. fanquake force-pushed on Oct 4, 2022
7. fuzz: add util/mempool/h.cpp

   Moving the mempool code (Boost) out of util.h, results in a ~10% speedup
   (for me) when compiling the fuzz tests.

   eb15569280
8. fuzz: pass max fee into ConsumeTxMemPoolEntry 8a6b6dfcd8
9. fanquake force-pushed on Oct 4, 2022
10. 
    DrahtBot commented at 8:23 pm on October 4, 2022: contributor

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #17786 (refactor: Nuke policy/fees->mempool circular dependencies by hebasto)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

11. in src/test/fuzz/util/mempool.h:9 in eb15569280 outdated

     6-#define BITCOIN_TEST_FUZZ_MEMPOOL_UTILS_H
     7+#ifndef BITCOIN_TEST_FUZZ_UTIL_MEMPOOL_H
     8+#define BITCOIN_TEST_FUZZ_UTIL_MEMPOOL_H
     9 
    10+#include <primitives/transaction.h>
    11+#include <test/fuzz/FuzzedDataProvider.h>
    

    ---

    ---

    

    maflcko commented at 8:27 am on October 5, 2022:

    those two includes could be fwd-decls, but it doesn’t matter because everywhere where this header is included, the full includes already exist
12. maflcko approved
13. 
    maflcko commented at 8:28 am on October 5, 2022: member

    review ACK 8a6b6dfcd8d26b62c3a13beba72440635fcc5e19 🍮

    Signature:

     0-----BEGIN PGP SIGNED MESSAGE-----
     1Hash: SHA512
     2
     3review ACK 8a6b6dfcd8d26b62c3a13beba72440635fcc5e19 🍮
     4-----BEGIN PGP SIGNATURE-----
     5
     6iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
     7pUgJuAv+MyuzOk4R07Q6iwAbpfd78pnXvv/5J2YAj5mpWWifj9ZAJL72sYk/K1I8
     85K4ukFrthMelgGVV7Hkz7KcSEQu/rRpjHRRILFuYsnxGwhVYi4RCH0w+s4jrotBc
     9onA2qBJaft8ULZtZg1wDS+Lkl8VLQ9bwkYmUkZgJf9vaUQwMYFlO70QkqidubjcR
    10+lslpo9UkFD4uH8q9YLClPvGWWxpjB/6x8/CKDEVt7Yfu1cQypX6kmgrG4T2Kmgi
    116FrJDAM0ECts/di/V1TLhRsGdQrSMhDxx0UawygzgaLTBc38ezG7sE17ram+ems+
    12cpE+mDhWu56QWG6hCrpIyRH2pkDcjymAwIjfEug9EromErGocKKpY1nK5BVaqNbl
    13KKhrNnZ4x+DBhvgBxCiGzfGFP2oGgDAKPY4Ke22LjdpdC1szqBcLMdWVyPCKrJBc
    14VzypwHcwe8rGkqZaVmGzIGB7PSEfJCEhvscA31Z7vFBsuHqTiz/OROgV+J4+/qPr
    15fQ8WY0tK
    16=GxLp
    17-----END PGP SIGNATURE-----
    

14. maflcko merged this on Oct 5, 2022
15. maflcko closed this on Oct 5, 2022

    ---

16. 
    glozow commented at 8:41 am on October 5, 2022: member

    post-merge concept ACK
17. sidhujag referenced this in commit 05b8fd05f9 on Oct 5, 2022
18. fanquake deleted the branch on Oct 10, 2022
19. bitcoin locked this on Oct 10, 2023

Contributors
fanquake maflcko DrahtBot glozow

Labels
Tests