fuzz: Extend `spend` coverage

Chand-ra commented at 3:12 pm on January 12, 2026: none

Make the wallet/spend fuzz target stateful and extend coverage for several missing methods.

fuzz: Refactor `wallet_create_transaction` to support multiple actions

Update the `wallet_create_transaction` fuzz target to move the
transaction creation logic inside a `CallOneOf` block.

This refactoring is a structural preparation for stateful fuzzing,
allowing commits to easily add and interleave other wallet
operations within the same loop.

5fb868abe7

fuzz: lock inputs to verify correct handling of user-mandated inputs

Move the `CCoinControl` parameter configuration inside the fuzzing
loop to allow mutation of settings between operations. Additionally,
introduce logic to randomly `Select` or `UnSelect` specific available
outpoints within `coin_control`.

This allows the fuzzer to simulate scenarios where a user manually
mandates specific inputs.

9c1a53a48f

DrahtBot added the label Fuzzing on Jan 12, 2026

DrahtBot commented at 3:12 pm on January 12, 2026: 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/34264.

Reviews

See the guideline for information on the review process. A summary of reviews will appear here.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#34032 (util: Add some more Unexpected and Expected methods by maflcko)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

DrahtBot added the label CI failed on Jan 12, 2026

DrahtBot commented at 5:24 pm on January 12, 2026: contributor

🚧 At least one of the CI tasks failed. Task tidy: https://github.com/bitcoin/bitcoin/actions/runs/20924396327/job/60120935082 LLM reason (✨ experimental): CI failure due to clang-tidy errors in spend.cpp (unnecessary temporary object in emplace_back and unused-return-value).

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

brunoerg commented at 6:30 pm on January 12, 2026: contributor

From CI:

 0[515/729][5.2s] clang-tidy-21 -p=/home/admin/actions-runner/_work/_temp/build -quiet -load=/tidy-build/libbitcoin-tidy.so /home/admin/actions-runner/_work/_temp/src/test/fuzz/block_index_tree.cpp
 11126 warnings generated.
 2+ echo '^^^ ⚠️ Failure generated from clang-tidy'
 3+ false
 4
 5[516/729][8.1s] clang-tidy-21 -p=/home/admin/actions-runner/_work/_temp/build -quiet -load=/tidy-build/libbitcoin-tidy.so /home/admin/actions-runner/_work/_temp/src/wallet/test/fuzz/spend.cpp
 6/home/admin/actions-runner/_work/_temp/src/wallet/test/fuzz/spend.cpp:125:41: error: unnecessary temporary object created while calling emplace_back [modernize-use-emplace,-warnings-as-errors]
 7  125 |                     tx.vin.emplace_back(CTxIn(out));
 8      |                                         ^~~~~~   ~
 9/home/admin/actions-runner/_work/_temp/src/wallet/test/fuzz/spend.cpp:152:23: error: the value returned by this function should not be disregarded; neglecting it may lead to errors [bugprone-unused-return-value,-warnings-as-errors]
10  152 |                 (void)FundTransaction(*fuzzed_wallet.wallet, tx, recipients, change_pos, lockUnspents, coin_control);
11      |                       ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
122392 warnings generated.
13
14[517/729][3.6s] clang-tidy-21 -p=/home/admin/actions-runner/_work/_temp/build -quiet -load=/tidy-build/libbitcoin-tidy.so /home/admin/actions-runner/_work/_temp/src/qt/bitcoinaddressvalidator.cpp
15^^^ ⚠️ Failure generated from clang-tidy
16Command '['docker', 'exec', '88e904189f27a153218451cb9cc2cf6a3600de9821f8b631509e657c4e1d73df', '/home/admin/actions-runner/_work/_temp/ci/test/03_test_script.sh']' returned non-zero exit status 1.

in src/wallet/test/fuzz/spend.cpp:125 in e3619a36d8

127+                CMutableTransaction tx;
128+                tx.nLockTime = fuzzed_data_provider.ConsumeIntegral<unsigned int>();
129+                int num_inputs = fuzzed_data_provider.ConsumeIntegralInRange<int>(0, available_outpoints.size());
130+                for (int i = 0; i < num_inputs; i++) {
131+                    auto out = PickValue(fuzzed_data_provider, available_outpoints);
132+                    tx.vin.emplace_back(CTxIn(out));

frankomosh commented at 7:32 am on January 13, 2026:

Could simplify to tx.vin.emplace_back(out)

in src/wallet/test/fuzz/spend.cpp:152 in e3619a36d8

154+                }
155+                std::optional<unsigned int> change_pos;
156+                if (fuzzed_data_provider.ConsumeBool()) change_pos = fuzzed_data_provider.ConsumeIntegral<unsigned int>();
157+                bool lockUnspents = fuzzed_data_provider.ConsumeBool();
158+
159+                (void)FundTransaction(*fuzzed_wallet.wallet, tx, recipients, change_pos, lockUnspents, coin_control);

frankomosh commented at 7:48 am on January 13, 2026:

Also, should FundTransaction have wallet lock protection i.e wrap it in LOCK(fuzzed_wallet.wallet -> cs_wallet), as has been done in ListCoins ?

Chand-ra commented at 3:43 pm on January 13, 2026:

Not really, FundTransaction() doesn’t require an exclusive lock unlike ListCoins() does.

in src/wallet/test/fuzz/spend.cpp:128 in e3619a36d8

130+                for (int i = 0; i < num_inputs; i++) {
131+                    auto out = PickValue(fuzzed_data_provider, available_outpoints);
132+                    tx.vin.emplace_back(CTxIn(out));
133+                }
134+                std::vector<CRecipient> recipients;
135+                LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 50) {

frankomosh commented at 8:08 am on January 13, 2026:

I notice that this logic appears twice. Would it make sense to extract it into a lambda helper at the top of the function?

frankomosh commented at 8:11 am on January 13, 2026: contributor

I think that the variables int next_locktime and CAmount all_values become dead code after you’ve refactored? Also some inline nits below.

Chand-ra commented at 3:48 pm on January 13, 2026: none

I think that the variables int next_locktime and CAmount all_values become dead code after you’ve refactored? Also some inline nits below.

I believe next_locktime and all_values are still necessary in the setup loop. next_locktime ensures that we generate unique TXIDs and all_values is used to enforce the MAX_MONEY cap so we don’t overflow the wallet balance during setup.

fuzz: Add coverage for `FundTransaction()`

Extend the target to cover `FundTransaction()`.

80b4cf2bef

fuzz: Add coverage for `ListCoins()`

Extend the fuzz target to cover `ListCoins()`.

6242b127a2

Chand-ra force-pushed on Jan 13, 2026

DrahtBot removed the label CI failed on Jan 13, 2026

fuzz: Extend spend coverage #34264

Code Coverage & Benchmarks

Reviews

Conflicts

fuzz: Extend `spend` coverage #34264