fuzz: wallet: add target for MigrateToDescriptor #32624

pull brunoerg wants to merge 1 commits into bitcoin:master from brunoerg:2025-05-fuzz-spkm-migration changing 1 files +140 −1
  1. brunoerg commented at 11:34 am on May 27, 2025: contributor

    This PR adds fuzz coverage for the scriptpubkeyman migration (MigrateToDescriptor). Note that it’s a test for the migration of the scriptpubkey manager, not for the whole migration process as tried in #29694, because:

    1. The wallet migration deals with DBs which is expensive for fuzzing (was getting around 3 exec/s);
    2. Mocking would require lots of refactors.

    This target loads keys, HDChain (even inactive ones), watch only and might add tons of different scripts, then calls MigrateToDescriptor. It does not play with encrypted stuff because it would make the target super slow. Also, after the migration there are some assertions that would work as a regression test for #31452, for example.

  2. DrahtBot commented at 11:34 am on May 27, 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/32624.

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    Approach ACK w0xlt
    Stale ACK maflcko

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

  3. DrahtBot added the label Tests on May 27, 2025
  4. DrahtBot added the label CI failed on May 27, 2025
  5. in src/wallet/test/fuzz/scriptpubkeyman.cpp:59 in 08b574dde2 outdated 
    51@@ -50,6 +52,13 @@ void initialize_spkm()
52     MOCKED_DESC_CONVERTER.Init();
53 }
54 
55+void initialize_spkm_migration()
56+{
57+    static const auto testing_setup{MakeNoLogFileContext<const TestingSetup>()};
58+    g_setup = testing_setup.get();
59+    SelectParams(ChainType::MAIN);
    maflcko commented at 12:41 pm on May 27, 2025:
    why is this needed? Isn’t the chain already MAIN?
    brunoerg commented at 12:56 pm on May 27, 2025:
    Just removed it. Redudant.
  6. brunoerg force-pushed on May 27, 2025
  7. brunoerg commented at 12:56 pm on May 27, 2025: contributor
    08b574dde2c5f13a4c9dadcb9d8bded158c59623..5f2fb4dd333f8d1db27c78414afc84454e6ee4a5: Address #32624 (review)
  8. DrahtBot removed the label CI failed on May 27, 2025
  9. w0xlt commented at 10:26 pm on May 27, 2025: contributor
    Approach ACK
  10. brunoerg requested review from maflcko on Jun 5, 2025
  11. brunoerg force-pushed on Jun 16, 2025
  12. in src/wallet/test/fuzz/scriptpubkeyman.cpp:225 in 73439a78f3 outdated 
    220+    const auto& node{g_setup->m_node};
221+    Chainstate& chainstate{node.chainman->ActiveChainstate()};
222+    WalletContext context;
223+    auto& args{g_setup->m_args};
224+    context.args = const_cast<ArgsManager*>(&args);
225+    context.chain = g_setup->m_node.chain.get();
    maflcko commented at 1:42 pm on June 17, 2025:
    unused?
    brunoerg commented at 4:13 pm on June 17, 2025:
    Yes, thanks. It was a leftover from a previous approach, just removed it.
  13. brunoerg force-pushed on Jun 17, 2025
  14. maflcko commented at 8:44 pm on June 17, 2025: member
    lgtm ACK 6d29c9a6d1b4239a0844791d4a53599ae3a79cd0
  15. DrahtBot requested review from w0xlt on Jun 17, 2025
  16. achow101 removed review request from w0xlt on Oct 22, 2025
  17. achow101 requested review from marcofleon on Oct 22, 2025
  18. achow101 requested review from achow101 on Oct 22, 2025
  19. in src/wallet/test/fuzz/scriptpubkeyman.cpp:340 in 6d29c9a6d1 outdated 
    331+    auto result{legacy_data.MigrateToDescriptor()};
332+    size_t added_size{keys.size() + static_cast<size_t>(add_hd_chain) + static_cast<size_t>(add_inactive_hd_chain)};
333+    if (added_script > 0) {
334+        assert(result->desc_spkms.size() >= added_size);
335+    } else {
336+        assert(result->desc_spkms.size() == added_size);
    marcofleon commented at 4:21 pm on October 27, 2025: 
     0fuzz: ../../../../src/wallet/test/fuzz/scriptpubkeyman.cpp:335: void wallet::(anonymous namespace)::spkm_migration_fuzz_target(FuzzBufferType): Assertion `result->desc_spkms.size() == added_size' failed.
 1==1736884== ERROR: libFuzzer: deadly signal
 2    [#0](/bitcoin-bitcoin/0/) 0x5650eb34b2a4 in __sanitizer_print_stack_trace (/root/bitcoin/fuzzbuild/bin/fuzz+0x9fd2a4) (BuildId: 714ae02d0bf0aee30b5aead68737ef9e6f49c96a)
 3    [#1](/bitcoin-bitcoin/1/) 0x5650eb3211d8 in fuzzer::PrintStackTrace() (/root/bitcoin/fuzzbuild/bin/fuzz+0x9d31d8) (BuildId: 714ae02d0bf0aee30b5aead68737ef9e6f49c96a)
 4    [#2](/bitcoin-bitcoin/2/) 0x5650eb3075f3 in fuzzer::Fuzzer::CrashCallback() (/root/bitcoin/fuzzbuild/bin/fuzz+0x9b95f3) (BuildId: 714ae02d0bf0aee30b5aead68737ef9e6f49c96a)
 5    [#3](/bitcoin-bitcoin/3/) 0x7f07fa81d04f  (/lib/x86_64-linux-gnu/libc.so.6+0x3c04f) (BuildId: 58254ca972028402bc40624f81388d85ec95f70d)
 6    [#4](/bitcoin-bitcoin/4/) 0x7f07fa86be2b  (/lib/x86_64-linux-gnu/libc.so.6+0x8ae2b) (BuildId: 58254ca972028402bc40624f81388d85ec95f70d)
 7    [#5](/bitcoin-bitcoin/5/) 0x7f07fa81cfb1 in raise (/lib/x86_64-linux-gnu/libc.so.6+0x3bfb1) (BuildId: 58254ca972028402bc40624f81388d85ec95f70d)
 8    [#6](/bitcoin-bitcoin/6/) 0x7f07fa807471 in abort (/lib/x86_64-linux-gnu/libc.so.6+0x26471) (BuildId: 58254ca972028402bc40624f81388d85ec95f70d)
 9    [#7](/bitcoin-bitcoin/7/) 0x7f07fa807394  (/lib/x86_64-linux-gnu/libc.so.6+0x26394) (BuildId: 58254ca972028402bc40624f81388d85ec95f70d)
10    [#8](/bitcoin-bitcoin/8/) 0x7f07fa815eb1 in __assert_fail (/lib/x86_64-linux-gnu/libc.so.6+0x34eb1) (BuildId: 58254ca972028402bc40624f81388d85ec95f70d)
11    [#9](/bitcoin-bitcoin/9/) 0x5650eb6cd1c5 in wallet::(anonymous namespace)::spkm_migration_fuzz_target(std::span<unsigned char const, 18446744073709551615ul>) /root/bitcoin/fuzzbuild/src/test/fuzz/../../../../src/wallet/test/fuzz/scriptpubkeyman.cpp:335:9
12    [#10](/bitcoin-bitcoin/10/) 0x5650eb6df861 in std::function<void (std::span<unsigned char const, 18446744073709551615ul>)>::operator()(std::span<unsigned char const, 18446744073709551615ul>) const /usr/bin/../lib/gcc/x86_64-linux-gnu/12/../../../../include/c++/12/bits/std_function.h:591:9
13    [#11](/bitcoin-bitcoin/11/) 0x5650eb6df861 in test_one_input(std::span<unsigned char const, 18446744073709551615ul>) /root/bitcoin/fuzzbuild/src/test/fuzz/util/../../../../../src/test/fuzz/fuzz.cpp:88:5
14    [#12](/bitcoin-bitcoin/12/) 0x5650eb6df861 in LLVMFuzzerTestOneInput /root/bitcoin/fuzzbuild/src/test/fuzz/util/../../../../../src/test/fuzz/fuzz.cpp:216:5
15    [#13](/bitcoin-bitcoin/13/) 0x5650eb308a00 in fuzzer::Fuzzer::ExecuteCallback(unsigned char const*, unsigned long) (/root/bitcoin/fuzzbuild/bin/fuzz+0x9baa00) (BuildId: 714ae02d0bf0aee30b5aead68737ef9e6f49c96a)
16    [#14](/bitcoin-bitcoin/14/) 0x5650eb308135 in fuzzer::Fuzzer::RunOne(unsigned char const*, unsigned long, bool, fuzzer::InputInfo*, bool, bool*) (/root/bitcoin/fuzzbuild/bin/fuzz+0x9ba135) (BuildId: 714ae02d0bf0aee30b5aead68737ef9e6f49c96a)
17    [#15](/bitcoin-bitcoin/15/) 0x5650eb3098c5 in fuzzer::Fuzzer::MutateAndTestOne() (/root/bitcoin/fuzzbuild/bin/fuzz+0x9bb8c5) (BuildId: 714ae02d0bf0aee30b5aead68737ef9e6f49c96a)
18    [#16](/bitcoin-bitcoin/16/) 0x5650eb30a4c5 in fuzzer::Fuzzer::Loop(std::vector<fuzzer::SizedFile, std::allocator<fuzzer::SizedFile>>&) (/root/bitcoin/fuzzbuild/bin/fuzz+0x9bc4c5) (BuildId: 714ae02d0bf0aee30b5aead68737ef9e6f49c96a)
19    [#17](/bitcoin-bitcoin/17/) 0x5650eb2f86ef in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (/root/bitcoin/fuzzbuild/bin/fuzz+0x9aa6ef) (BuildId: 714ae02d0bf0aee30b5aead68737ef9e6f49c96a)
20    [#18](/bitcoin-bitcoin/18/) 0x5650eb321aa2 in main (/root/bitcoin/fuzzbuild/bin/fuzz+0x9d3aa2) (BuildId: 714ae02d0bf0aee30b5aead68737ef9e6f49c96a)
21    [#19](/bitcoin-bitcoin/19/) 0x7f07fa808249  (/lib/x86_64-linux-gnu/libc.so.6+0x27249) (BuildId: 58254ca972028402bc40624f81388d85ec95f70d)
22    [#20](/bitcoin-bitcoin/20/) 0x7f07fa808304 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x27304) (BuildId: 58254ca972028402bc40624f81388d85ec95f70d)
23    [#21](/bitcoin-bitcoin/21/) 0x5650eb2edae0 in _start (/root/bitcoin/fuzzbuild/bin/fuzz+0x99fae0) (BuildId: 714ae02d0bf0aee30b5aead68737ef9e6f49c96a)
24
25NOTE: libFuzzer has rudimentary signal handlers.
26      Combine libFuzzer with AddressSanitizer or similar for better crash reports.
27SUMMARY: libFuzzer: deadly signal
28MS: 1 InsertByte-; base unit: 2068efd5f9f44e2724a628cd896cc4911f0f66a7
290xff,0x0,0xfd,0xfb,0x0,0x0,0x0,0xd7,0x52,0x87,0x87,0x87,0x87,0x87,0x87,0x87,0x87,0x87,0x40,0x87,0x87,0x87,0x87,0x87,0x87,0x87,0x87,0x87,0x87,0x0,0x0,0x0,0x0,0x0,0x6f,0x0,0x0,0x13,0x87,0x87,0x87,0x87,0x28,
30\377\000\375\373\000\000\000\327R\207\207\207\207\207\207\207\207\207@\207\207\207\207\207\207\207\207\207\207\000\000\000\000\000o\000\000\023\207\207\207\207(
31artifact_prefix='./'; Test unit written to ./crash-e661f705f50b682bb8c4b81ce95935565c579253
32Base64: /wD9+wAAANdSh4eHh4eHh4eHQIeHh4eHh4eHh4cAAAAAAG8AABOHh4eHKA==

    Something being miscounted for added_size I would assume?

    brunoerg commented at 5:16 pm on October 27, 2025:

    Interesting how I didn’t get it but I think this is related to #32624 (review). The check was wrong so it was playing with not-fully-valid keys.

    edit: there is one more reason I’m checking out.

  20. in src/wallet/test/fuzz/scriptpubkeyman.cpp:264 in 6d29c9a6d1 
    259+        legacy_data.AddInactiveHDChain(hd_chain);
260+    }
261+
262+    bool watch_only = false;
263+    const auto pub_key = ConsumeDeserializable<CPubKey>(fuzzed_data_provider);
264+    if (!pub_key && !pub_key->IsFullyValid()) return;
    marcofleon commented at 4:26 pm on October 27, 2025:
    Shouldn’t this be ||? Wouldn’t want to dereference if pub_key is null.
    brunoerg commented at 5:05 pm on October 27, 2025:
    Yes, nice find!
  21. DrahtBot requested review from w0xlt on Oct 27, 2025
  22. maflcko closed this on Oct 27, 2025
  23. maflcko reopened this on Oct 27, 2025
  24. brunoerg force-pushed on Oct 27, 2025
  25. brunoerg commented at 5:16 pm on October 27, 2025: contributor
    Force-pushed addressing #32624 (review). Thanks, @marcofleon.
  26. fuzz: wallet: add target for `MigrateToDescriptor` 6436ad7f2f
  27. brunoerg force-pushed on Oct 27, 2025
  28. brunoerg commented at 6:06 pm on October 27, 2025: contributor
    There was one more case that I forgot to count in. We should consider the split chain support when verifying the result (desc_spkms). Force-pushed fixing it (6ced804 to 6436ad7).
  29. fanquake added the label Fuzzing on Oct 30, 2025


brunoerg DrahtBot maflcko w0xlt marcofleon


achow101 w0xlt

Labels
Tests Fuzzing

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2025-10-31 09:13 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me