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 +62 −3
  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:106 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. Chand-ra force-pushed on Dec 11, 2025
  12. 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);
  13. 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
  14. 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
  15. in src/wallet/test/fuzz/coincontrol.cpp:96 in b116db10a4 outdated 
    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?
    Chand-ra commented at 6:34 am on December 12, 2025:
    GetInputWeight() returns a std::optional<int64_t>, so I don’t think it can return a nullptr. It does return a std::nullopt when the coin is not selected, but I think adding a check for that would be too close to duplicating the implementation.
    l0rinc commented at 8:41 am on December 12, 2025:
    I meant nullopt of course. We should at least exercise that code path.
  16. 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);

    ?

  17. 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
  18. in src/wallet/test/fuzz/coincontrol.cpp:1 in b116db10a4 outdated
    l0rinc commented at 12:09 pm on December 11, 2025:
    HasSelectedOrder() and GetSelectionPos() from header are still untested
  19. 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
  20. in src/wallet/test/fuzz/coincontrol.cpp:137 in b116db10a4 outdated 
    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?
    Chand-ra commented at 6:41 am on December 12, 2025:

    Yes, that is correct and intentional.

    Select() assigns a default sequential position, but the goal of this test is to exercise the SetPosition() setter directly. We want to verify that the PreselectedInput object handles arbitrary position values correctly, even if they differ from the default sequence assigned by Select().

  21. l0rinc changes_requested
  22. Chand-ra force-pushed on Dec 12, 2025
  23. Chand-ra force-pushed on Dec 17, 2025
  24. brunoerg commented at 2:11 pm on December 17, 2025: contributor

    The improved harness from this PR increased the mutation score of the coin_control target compared to the master branch. See:

    The only uncaught mutant is:

     0diff --git a/src/wallet/coincontrol.cpp b/src/wallet/coincontrol.cpp
 1index 873c5ab383..e83f8d451a 100644
 2--- a/src/wallet/coincontrol.cpp
 3+++ b/src/wallet/coincontrol.cpp
 4@@ -25,7 +25,7 @@ bool CCoinControl::IsSelected(const COutPoint& outpoint) const
 5 bool CCoinControl::IsExternalSelected(const COutPoint& outpoint) const
 6 {
 7     const auto it = m_selected.find(outpoint);
 8-    return it != m_selected.end() && it->second.HasTxOut();
 9+    return it != m_selected.end() || it->second.HasTxOut();
10 }

    which can be killed with the following diff - it checks that when there is no tx out for a given input, it’s not external selected:

     0                 auto& input = coin_control.Select(out_point);
 1-                input.SetTxOut(tx_out);
 2-                assert(input.HasTxOut());
 3-                assert(input.GetTxOut() == tx_out);
 4-                assert(coin_control.IsExternalSelected(out_point));
 5+                const auto set_tx_out{fuzzed_data_provider.ConsumeBool()};
 6+                if (set_tx_out) {
 7+                    input.SetTxOut(tx_out);
 8+                }
 9+                auto has_tx_out{input.HasTxOut()};
10+                auto is_external_selected{coin_control.IsExternalSelected(out_point)};
11+                if (set_tx_out) {
12+                    assert(has_tx_out);
13+                    assert(input.GetTxOut() == tx_out);
14+                    assert(is_external_selected);
15+                } else if (!has_tx_out) {
16+                    assert(!is_external_selected);
17+                }
18             },
  25. fuzz: Improve oracle for existing CCoinControl tests b84b30f1c8
  26. fuzz: Add tests for CCoinControl methods 
    The `ccoincontrol` fuzzer misses tests for a number of CCoinControl
operations. Add them.
    80e30376e6
  27. Chand-ra force-pushed on Dec 18, 2025
  28. Chand-ra requested review from l0rinc on Dec 19, 2025


Chand-ra DrahtBot l0rinc brunoerg


l0rinc

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