fuzz: Properly setup wallet in wallet_fees target #32488

1. 
   maflcko commented at 9:13 pm on May 13, 2025: member

   g_wallet_ptr is destructed after the testing_setup. This is not supported and will lead to issues such as #30221 (comment) or #32409 (comment).

   This could be fixed by fixing the initialization order.

   However, the global wallet is also modified in the fuzz target, which is bad fuzzing practise.

   So instead fix it by constructing a fresh wallet for each fuzz iteration.

2. fuzz: Properly setup wallet in wallet_fees target

   Co-Authored-By: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>

   fa427ffcee
3. 
   DrahtBot commented at 9:13 pm on May 13, 2025: contributor

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

   Code Coverage & Benchmarks

   For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32488.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	brunoerg, hebasto, marcofleon

   If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

4. DrahtBot added the label Tests on May 13, 2025
5. maflcko requested review from brunoerg on May 13, 2025
6. maflcko requested review from hebasto on May 13, 2025
7. 
   maflcko commented at 9:17 pm on May 13, 2025: member

   Can be tested via:

    0diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h
    1index e32b8c7272..ea67a48e9e 100644
    2--- a/src/wallet/wallet.h
    3+++ b/src/wallet/wallet.h
    4@@ -468,6 +468,7 @@ public:
    5 
    6     ~CWallet()
    7     {
    8+    chain().relayMinFee();
    9         // Should not have slots connected at this point.
   10         assert(NotifyUnload.empty());
   11     }
   

   and:

   0rm -rf ./bld-cmake && cmake -B ./bld-cmake -DAPPEND_CXXFLAGS='-std=c++23 -O3 -g2' -DAPPEND_CFLAGS='-O3 -g2' -DCMAKE_BUILD_TYPE=Debug -DCMAKE_C_COMPILER='clang' -DCMAKE_CXX_COMPILER='clang++' -DBUILD_FOR_FUZZING=ON -DSANITIZERS=fuzzer,address,undefined,integer,float-divide-by-zero && cmake --build ./bld-cmake --parallel $( nproc )
   1UBSAN_OPTIONS="suppressions=$(pwd)/test/sanitizer_suppressions/ubsan:print_stacktrace=1:halt_on_error=1:report_error_type=1" FUZZ=wallet_fees  ./bld-cmake/bin/fuzz -runs=2
   

   Before: UndefinedBehaviorSanitizer: dynamic-type-mismatch After: Clean

8. brunoerg approved
9. 
   brunoerg commented at 9:26 pm on May 13, 2025: contributor

   code review ACK fa427ffceeefd368a1ade273501ce4b01133ad4d

   Nice! I think the initial idea was to have a global wallet because everything the target was supposed to do shouldn’t affect it. But surely it’s better to not have it globally and avoid any possible issue.

10. fanquake requested review from marcofleon on May 13, 2025
11. 
    maflcko commented at 5:00 am on May 14, 2025: member

    (side note: I would have expected that the fuzz stability was less, but https://oss-fuzz.com/fuzzer-stats?group_by=by-fuzzer&date_start=2025-04-14&date_end=2025-05-13&fuzzer=afl_bitcoin-core_wallet_fees&job=afl_asan_bitcoin-core&project=bitcoin-core claims it is 100%)
12. hebasto approved
13. 
    hebasto commented at 10:06 am on May 14, 2025: member

    ACK fa427ffceeefd368a1ade273501ce4b01133ad4d, this change fixes the issue when building the “Debug” configuration with MSVC on Windows. @maflcko

    Thank you for picking this up!

14. 
    marcofleon commented at 10:26 am on May 14, 2025: contributor

    Code review ACK fa427ffceeefd368a1ade273501ce4b01133ad4d
15. fanquake merged this on May 14, 2025
16. fanquake closed this on May 14, 2025

    ---

17. maflcko deleted the branch on May 14, 2025

Contributors
maflcko DrahtBot brunoerg hebasto marcofleon

Labels
Tests