fuzz: Add tests for CCoinControl methods #34026

pull Chand-ra wants to merge 2 commits into bitcoin:master from Chand-ra:CCoinControl_Wrappers changing 1 files +42 −12
  1. Chand-ra commented at 8:54 am on December 7, 2025: none

    The ccoincontrol fuzzer misses tests for a number of CCoinControl operations. Add them.

    While at it, improve the oracle for the existing tests.

  2. DrahtBot added the label Tests on Dec 7, 2025
  3. DrahtBot commented at 8:54 am on December 7, 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/34026.

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    Stale ACK l0rinc, brunoerg

    If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.

  4. fanquake added the label Fuzzing on Dec 7, 2025
  5. in src/wallet/test/fuzz/coincontrol.cpp:84 in 70a543fc56 outdated 
    76@@ -77,6 +77,12 @@ FUZZ_TARGET(coincontrol, .init = initialize_coincontrol)
77             },
78             [&] {
79                 (void)coin_control.GetInputWeight(out_point);
80+            },
81+            [&] {
82+                (void)coin_control.GetSequence(out_point);
83+            },
84+            [&] {
85+                (void)coin_control.GetScripts(out_point);
    l0rinc commented at 11:52 am on December 9, 2025:
    These were indeed missing from https://maflcko.github.io/b-c-cov/fuzz.coverage/src/wallet/coincontrol.cpp.gcov.html - there are quite a few more, consider adding those as well
    Chand-ra commented at 1:16 pm on December 9, 2025:

    These were indeed missing from https://maflcko.github.io/b-c-cov/fuzz.coverage/src/wallet/coincontrol.cpp.gcov.html - there are quite a few more, consider adding those as well

    Do you think it would be better to add the other ones in a separate PR or is it better to add them all in this one?

    l0rinc commented at 1:45 pm on December 9, 2025:
    If it’s simple to add, I would include it here - either in the same commit, or if it’s fundamentally a different feature (it’s easier to review in isolation), it could go to a second commit.
    l0rinc commented at 11:53 am on December 11, 2025:
    Please resolve the comment
  6. l0rinc commented at 11:52 am on December 9, 2025: contributor
    code review ACK 70a543fc565f7263509891dfd23d4b9f6b64cd79
  7. Chand-ra force-pushed on Dec 9, 2025
  8. brunoerg approved
  9. brunoerg commented at 5:19 pm on December 10, 2025: contributor

    code review ACK 31904f1ba1de49890f605f1d7c907790780831ec

    It’s fine as is, but I think we could improve this harness a little bit by adding some assertions. Some quite simple examples:

    0@@ -87,9 +87,7 @@ FUZZ_TARGET(coincontrol, .init = initialize_coincontrol)
1             [&] {
2                 uint32_t sequence{fuzzed_data_provider.ConsumeIntegral<uint32_t>()};
3                 (void)coin_control.Select(out_point).SetSequence(sequence);
4-            },
5-            [&] {
6-                (void)coin_control.Select(out_point).GetSequence();
7+               assert(coin_control.Select(out_point).GetSequence() == sequence);
8             },

    0@@ -108,10 +106,8 @@ FUZZ_TARGET(coincontrol, .init = initialize_coincontrol)
1             [&] {
2                 unsigned int pos{fuzzed_data_provider.ConsumeIntegral<unsigned int>()};
3                 (void)coin_control.Select(out_point).SetPosition(pos);
4-            },
5-            [&] {
6-                (void)coin_control.Select(out_point).GetPosition();
7-            });
8+                assert(coin_control.Select(out_point).GetPosition() == pos);
9+           });
  10. DrahtBot requested review from l0rinc on Dec 10, 2025
  11. fuzz: Improve oracle for existing CCoinControl tests 901dd5e3d1
  12. fuzz: Add tests for CCoinControl methods 
    The `ccoincontrol` fuzzer misses tests for a number of CCoinControl
operations. Add them.
    b116db10a4
  13. Chand-ra force-pushed on Dec 11, 2025
  14. in src/wallet/test/fuzz/coincontrol.cpp:100 in b116db10a4 
    106+                const auto& scripts = coin_control.Select(out_point).GetScripts();
107+                assert(scripts.first && *scripts.first == script);
108+            },
109+            [&] {
110+                const CScriptWitness script_wit{ConsumeScriptWitness(fuzzed_data_provider)};
111+                (void)coin_control.Select(out_point).SetScriptWitness(script_wit);
    l0rinc commented at 11:56 am on December 11, 2025:

    SetScriptWitness is already void:

    0                coin_control.Select(out_point).SetScriptWitness(script_wit);
  15. in src/wallet/test/fuzz/coincontrol.cpp:71 in b116db10a4 
    77+                assert(!coin_control.HasSelected());
78             },
79             [&] {
80-                (void)coin_control.ListSelected();
81+                const std::vector<COutPoint> selected = coin_control.ListSelected();
82+                assert(selected.empty() == !coin_control.HasSelected());
    l0rinc commented at 11:59 am on December 11, 2025:
    this is just duplicating the actual implementation, not very useful
  16. in src/wallet/test/fuzz/coincontrol.cpp:72 in b116db10a4 
    78             },
79             [&] {
80-                (void)coin_control.ListSelected();
81+                const std::vector<COutPoint> selected = coin_control.ListSelected();
82+                assert(selected.empty() == !coin_control.HasSelected());
83+                for (const auto& out : selected)
    l0rinc commented at 11:59 am on December 11, 2025:
    please add braces to multiline loops
  17. in src/wallet/test/fuzz/coincontrol.cpp:78 in b116db10a4 
    84+                    assert(coin_control.IsSelected(out));
85             },
86             [&] {
87                 int64_t weight{fuzzed_data_provider.ConsumeIntegral<int64_t>()};
88                 (void)coin_control.SetInputWeight(out_point, weight);
89+                assert(coin_control.GetInputWeight(out_point) == weight);
    l0rinc commented at 12:01 pm on December 11, 2025:
    when can GetInputWeight return nullptr? can we exercise that path as well?
  18. in src/wallet/test/fuzz/coincontrol.cpp:96 in b116db10a4 
    102+            [&] {
103+                const CScript script{ConsumeScript(fuzzed_data_provider)};
104+                (void)coin_control.Select(out_point).SetScriptSig(script);
105+                assert(coin_control.Select(out_point).HasScripts());
106+                const auto& scripts = coin_control.Select(out_point).GetScripts();
107+                assert(scripts.first && *scripts.first == script);
    l0rinc commented at 12:03 pm on December 11, 2025:

    Given the std::optional’s operator== overload, wouldn’t this suffice

    0                assert(scripts.first == script);

    ?

  19. in src/wallet/test/fuzz/coincontrol.cpp:94 in b116db10a4 
    100+                assert(coin_control.Select(out_point).GetSequence() == sequence);
101+            },
102+            [&] {
103+                const CScript script{ConsumeScript(fuzzed_data_provider)};
104+                (void)coin_control.Select(out_point).SetScriptSig(script);
105+                assert(coin_control.Select(out_point).HasScripts());
    l0rinc commented at 12:08 pm on December 11, 2025:
    Select mutates m_selection_pos - we could do selection separately
  20. in src/wallet/test/fuzz/coincontrol.cpp:1 in b116db10a4
    l0rinc commented at 12:09 pm on December 11, 2025:
    HasSelectedOrder() and GetSelectionPos() from header are still untested
  21. in src/wallet/test/fuzz/coincontrol.cpp:47 in b116db10a4 
    42@@ -43,40 +43,70 @@ FUZZ_TARGET(coincontrol, .init = initialize_coincontrol)
43                 }
44                 out_point = *optional_out_point;
45             },
46-            [&] {
47-                (void)coin_control.HasSelected();
    l0rinc commented at 12:10 pm on December 11, 2025:
    you could leave these to untangle coverage from context
  22. in src/wallet/test/fuzz/coincontrol.cpp:108 in b116db10a4 
    115             },
116             [&] {
117-                (void)coin_control.GetInputWeight(out_point);
118+                auto& input = coin_control.Select(out_point);
119+                unsigned int pos{fuzzed_data_provider.ConsumeIntegral<unsigned int>()};
120+                input.SetPosition(pos);
    l0rinc commented at 12:11 pm on December 11, 2025:
    doesn’t this overwrite the value change by coin_control.Select?
  23. l0rinc changes_requested


Chand-ra DrahtBot l0rinc brunoerg

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-12-11 15:13 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me