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.
Co-Authored-By: Hennadii Stepanov <32963518+hebasto@users.noreply.github.com>
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32488.
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.
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
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.
