This is a follow-up to #31238#issue-2639196806, limiting the scope of the fuzz target even further, because it is still by far the most expensive one, see https://github.com/bitcoin-core/qa-assets/pull/214#issuecomment-2536009266.
Also, remove the FundTx check, as it is one of the heavier calls.
In the future, the fuzz target can fully be reworked, because it also has poor stability, see #31238 (comment)
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/31467.
See the guideline for information on the review process. A summary of reviews will appear here.
116@@ -117,9 +117,6 @@ FUZZ_TARGET(wallet_notifications, .init = initialize_setup)
117 tx.vout.emplace_back(in, wallet.GetScriptPubKey(fuzzed_data_provider));
118 // Add tx to block
119 block.vtx.emplace_back(MakeTransactionRef(tx));
120- // Check that funding the tx doesn't crash the wallet
121- a.FundTx(fuzzed_data_provider, tx);
FundTx is part of FuzzedWallet and is only used in this target. Since we’re removing this call here, maybe we could delete the entire function in FuzzedWallet.
Good point. This should either be kept (as I added it as a regression fuzz test), or the whole target can be removed.
IIUC the overhead comes from the knapsack solver running 1000 times over all inputs, which seems heavy. I wonder if there is an easy way to disable knapsack here?
I guess a middle ground could be to execute this call only ever once per fuzz input.
IIUC the overhead comes from the knapsack solver running 1000 times over all inputs, which seems heavy. I wonder if there is an easy way to disable knapsack here?
This is the 1000 iterations from ApproximateBestSubset?
ApproximateBestSubset, I think I mention it in the paper, but I remember that reducing the number of iterations by the half had no impact on its behavior.
