fuzz: Minor improvements to tx_package_eval target #28825

pull instagibbs wants to merge 2 commits into bitcoin:master from instagibbs:2023-11-package_fuzz_impr changing 2 files +30 −4

instagibbs commented at 5:11 PM on November 8, 2023: member

Exercises DIFFERENT_WITNESS by using "blank" WSH() and allowing witness to determine wtxid, and attempts to make invalid/duplicate inputs.
DrahtBot commented at 5:11 PM on November 8, 2023: contributor



The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Code Coverage

For detailed information about the code coverage, see the test coverage report.



Reviews

See the guideline for information on the review process.

Type Reviewers

ACK dergoegge

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
DrahtBot added the label Tests on Nov 8, 2023
fanquake requested review from dergoegge on Nov 8, 2023
fanquake requested review from glozow on Nov 8, 2023

Type	Reviewers
ACK	dergoegge

in src/test/fuzz/package_eval.cpp:48 in de541ea2f7 outdated

  43 | +           uint256 hash;
  44 | +           CSHA256().Write(EMPTY.data(), EMPTY.size()).Finalize(hash.begin());
  45 | +           return hash;
  46 | +       }())};
  47 | +static const std::vector<std::vector<uint8_t>> P2WSH_EMPTY_TRUE_STACK{{static_cast<uint8_t>(OP_TRUE)}, {}};
  48 | +static const std::vector<std::vector<uint8_t>> P2WSH_EMPTY_TWO_STACK{{static_cast<uint8_t>(OP_2)}, {}};

dergoegge commented at 9:20 AM on November 9, 2023:

Maybe put this in test/util/script.h?

instagibbs commented at 2:07 PM on November 9, 2023:

done

fuzz: allow reaching MempoolAcceptResult::ResultType::DIFFERENT_WITNESS in tx_package_eval target a0626ccdad
fuzz: allow fake and duplicate inputs in tx_package_eval target 6a917918b7
instagibbs force-pushed on Nov 9, 2023
in src/test/fuzz/package_eval.cpp:217 in 6a917918b7
```
 212 | +                    tx_mut.vin.push_back(tx_mut.vin.back());
 213 | +                }
 214 | +
 215 | +                // Refer to a non-existant input
 216 | +                if (fuzzed_data_provider.ConsumeBool()) {
 217 | +                    tx_mut.vin.emplace_back();
```
glozow commented at 2:08 PM on November 10, 2023:

is this supposed to be empty?

instagibbs commented at 6:36 PM on November 10, 2023:

yes, with the idea that the fuzzer won't be able to generate the preimage to all genuinely create null uint256/prevout
dergoegge approved
dergoegge commented at 10:16 AM on November 13, 2023: member

https://dergoegge.github.io/bitcoin-coverage/pr28825/fuzz.coverage/src/validation.cpp.gcov.html

Coverage looks good to me ACK 6a917918b76eef154c6757fe9ecf7713d526c3dd
fanquake requested review from maflcko on Nov 13, 2023

in src/test/util/script.h:31 in 6a917918b7

  26 | +           uint256 hash;
  27 | +           CSHA256().Write(EMPTY.data(), EMPTY.size()).Finalize(hash.begin());
  28 | +           return hash;
  29 | +       }())};
  30 | +static const std::vector<std::vector<uint8_t>> P2WSH_EMPTY_TRUE_STACK{{static_cast<uint8_t>(OP_TRUE)}, {}};
  31 | +static const std::vector<std::vector<uint8_t>> P2WSH_EMPTY_TWO_STACK{{static_cast<uint8_t>(OP_2)}, {}};

maflcko commented at 10:34 AM on November 13, 2023:

static const std::vector<std::vector<uint8_t>> P2WSH_EMPTY_TRUE_STACK{{uint8_t{OP_TRUE}}, {}};
static const std::vector<std::vector<uint8_t>> P2WSH_EMPTY_TWO_STACK{{uint8_t{OP_2}}, {}};

nit: Use the safe cast for new code, where possible, over the narrowing one.

Also, could use EMPTY instead of {}. But anything is fine here.

instagibbs commented at 2:29 PM on November 13, 2023:

will do if I touch again

maflcko approved
maflcko commented at 10:35 AM on November 13, 2023: member

Nice
fanquake assigned glozow on Nov 13, 2023
fanquake merged this on Nov 16, 2023
fanquake closed this on Nov 16, 2023
bitcoin locked this on Nov 15, 2024

Contributors

Labels

Linked (view graph)

#27463 Package Relay Project Tracking

fuzz: Minor improvements to tx_package_eval target #28825

Code Coverage

Reviews