tests: Add fuzzing harness for CCoinsViewCache #18867

pull practicalswift wants to merge 1 commits into bitcoin:master from practicalswift:fuzzers-coins_view changing 3 files +313 −0
  1. practicalswift commented at 1:06 PM on May 4, 2020: contributor

    Add fuzzing harness for CCoinsViewCache.

    See doc/fuzzing.md for information on how to fuzz Bitcoin Core. Don't forget to contribute any coverage increasing inputs you find to the Bitcoin Core fuzzing corpus repo.

    Happy fuzzing :)

  2. fanquake added the label Tests on May 4, 2020
  3. in src/test/fuzz/coins_view.cpp:65 in b5f971a16e outdated 
      59 | +            bool expected_code_path = false;
  60 | +            try {
  61 | +                coins_view_cache.AddCoin(random_out_point, std::move(coin), fuzzed_data_provider.ConsumeBool());
  62 | +                expected_code_path = true;
  63 | +            } catch (const std::logic_error& e) {
  64 | +                if (e.what() == std::string{"Attempted to overwrite an unspent coin (when possible_overwrite is false)"}) {
    MarcoFalke commented at 1:22 PM on May 4, 2020: 
                    if (e.what() != std::string{"Attempted to overwrite an unspent coin (when possible_overwrite is false)"}) raise e;

    Could expected_code_path be removed with this fixup? If yes, that seems preferable, as it is less code.

    practicalswift commented at 1:36 PM on May 4, 2020:

    The current formulation is intentional. The problem with doing it the way you suggest is that the throw; line (I assume raise e; was a typo :)) will not be covered. That would be bad for src/test/fuzz/ where I want literally 100% line coverage to be able to see fuzzing harness gaps where our inputs are not able to reach :)

  4. in src/test/fuzz/coins_view.cpp:80 in b5f971a16e outdated 
      75 | +        case 2: {
  76 | +            coins_view_cache.SetBestBlock(ConsumeUInt256(fuzzed_data_provider));
  77 | +            break;
  78 | +        }
  79 | +        case 3: {
  80 | +            Coin coin;
    MarcoFalke commented at 1:24 PM on May 4, 2020: 
                Coin move_to;
  5. jb55 commented at 3:42 PM on May 4, 2020: member

    nice, Concept ACK

  7. practicalswift commented at 8:12 PM on May 18, 2020: contributor

    @MarcoFalke Would you mind reviewing? :)

  8. practicalswift force-pushed on May 22, 2020
  9. practicalswift commented at 2:50 PM on May 22, 2020: contributor

    Rebased! :)

  10. in src/test/fuzz/coins_view.cpp:33 in ab7af9323e outdated 
      28 | +const Coin EMPTY_COIN{};
  29 | +
  30 | +bool operator==(const Coin& a, const Coin& b)
  31 | +{
  32 | +    if (a.IsSpent() && b.IsSpent()) return true;
  33 | +    return static_cast<bool>(a.fCoinBase) == static_cast<bool>(b.fCoinBase) && a.nHeight == b.nHeight && a.out == b.out;
    MarcoFalke commented at 2:50 PM on May 24, 2020:

    any reason to use static cast here? this seems to silence any sanitizers that check for overflow if the cast value was not 0 or 1

  11. in src/test/fuzz/coins_view.cpp:61 in ab7af9323e outdated 
      56 | +                break;
  57 | +            }
  58 | +            Coin coin = random_coin;
  59 | +            bool expected_code_path = false;
  60 | +            try {
  61 | +                coins_view_cache.AddCoin(random_out_point, std::move(coin), fuzzed_data_provider.ConsumeBool());
    MarcoFalke commented at 3:05 PM on May 24, 2020: 
                    coins_view_cache.AddCoin(random_out_point, std::move(coin), possible_overwrite);
  12. in src/test/fuzz/coins_view.cpp:67 in ab7af9323e outdated 
      60 | +            try {
  61 | +                coins_view_cache.AddCoin(random_out_point, std::move(coin), fuzzed_data_provider.ConsumeBool());
  62 | +                expected_code_path = true;
  63 | +            } catch (const std::logic_error& e) {
  64 | +                if (e.what() == std::string{"Attempted to overwrite an unspent coin (when possible_overwrite is false)"}) {
  65 | +                    expected_code_path = true;
    MarcoFalke commented at 3:05 PM on May 24, 2020: 
                        assert(possible_overwrite);

    Could make this check a bit stricter?

  13. in src/test/fuzz/coins_view.cpp:138 in ab7af9323e outdated 
     133 | +                if (fuzzed_data_provider.ConsumeBool()) {
 134 | +                    const std::optional<COutPoint> opt_out_point = ConsumeDeserializable<COutPoint>(fuzzed_data_provider);
 135 | +                    if (!opt_out_point) {
 136 | +                        break;
 137 | +                    }
 138 | +                    random_out_point = *opt_out_point;
    MarcoFalke commented at 3:15 PM on May 24, 2020:

    What is this doing? How is this different from the fuzz engine running case 6 -> case 9?

  14. in src/test/fuzz/coins_view.cpp:143 in ab7af9323e outdated 
     143 | +            try {
 144 | +                coins_view_cache.BatchWrite(coins_map, fuzzed_data_provider.ConsumeBool() ? ConsumeUInt256(fuzzed_data_provider) : coins_view_cache.GetBestBlock());
 145 | +                expected_code_path = true;
 146 | +            } catch (const std::logic_error& e) {
 147 | +                if (e.what() == std::string{"FRESH flag misapplied to coin that exists in parent cache"}) {
 148 | +                    expected_code_path = true;
    MarcoFalke commented at 3:20 PM on May 24, 2020:

    Note to myself only: I thought about putting a more strict assert here, but the exception doesn't give any hints which coin failed, so I don't think this is possible for now.

  15. in src/test/fuzz/coins_view.cpp:225 in ab7af9323e outdated 
     222 | +            try {
 223 | +                AddCoins(coins_view_cache, transaction, fuzzed_data_provider.ConsumeIntegral<int>(), fuzzed_data_provider.ConsumeBool());
 224 | +                expected_code_path = true;
 225 | +            } catch (const std::logic_error& e) {
 226 | +                if (e.what() == std::string{"Attempted to overwrite an unspent coin (when possible_overwrite is false)"}) {
 227 | +                    expected_code_path = true;
    MarcoFalke commented at 3:28 PM on May 24, 2020:

    assert(possible_overwrite);

  16. in src/test/fuzz/coins_view.cpp:239 in ab7af9323e outdated 
     234 | +            (void)AreInputsStandard(CTransaction{random_mutable_transaction}, coins_view_cache);
 235 | +            break;
 236 | +        }
 237 | +        case 2: {
 238 | +            TxValidationState state;
 239 | +            CAmount tx_fee;
    MarcoFalke commented at 3:30 PM on May 24, 2020: 
                CAmount tx_fee_out;
  17. in src/test/fuzz/coins_view.cpp:247 in ab7af9323e outdated 
     242 | +                // Avoid:
 243 | +                // consensus/tx_verify.cpp:171: bool Consensus::CheckTxInputs(const CTransaction &, TxValidationState &, const CCoinsViewCache &, int, CAmount &): Assertion `!coin.IsSpent()' failed.
 244 | +                break;
 245 | +            }
 246 | +            try {
 247 | +                (void)Consensus::CheckTxInputs(transaction, state, coins_view_cache, fuzzed_data_provider.ConsumeIntegralInRange<int>(0, std::numeric_limits<int>::max()), tx_fee);
    MarcoFalke commented at 3:32 PM on May 24, 2020:

    can assert(MoneyRange(tx_fee_out)); when check was successful?

  18. MarcoFalke approved
  19. MarcoFalke commented at 3:34 PM on May 24, 2020: member

    ACK. Left some ideas for more asserts

    68dd5ee2c0934b6cb8f595b4a97ba6e5d5e2a31a

  20. tests: Add fuzzing harness for CCoinsViewCache f9b22e3bdb
  21. practicalswift force-pushed on May 25, 2020
  22. practicalswift commented at 10:07 AM on May 25, 2020: contributor

    @MarcoFalke Thanks a lot for a very good review! All feedback addressed :)

  23. in src/test/fuzz/coins_view.cpp:59 in f9b22e3bdb 
      54 | +        case 0: {
  55 | +            if (random_coin.IsSpent()) {
  56 | +                break;
  57 | +            }
  58 | +            Coin coin = random_coin;
  59 | +            bool expected_code_path = false;
    MarcoFalke commented at 11:05 AM on May 26, 2020:

    why is this needed?

    MarcoFalke commented at 11:08 AM on May 26, 2020:

    Are you saying that the only exception that is allowed to be thrown is the one that is caught by the error string? Fair enough.

    practicalswift commented at 11:20 AM on May 26, 2020:

    Exactly :)

  24. MarcoFalke commented at 11:08 AM on May 26, 2020: member

    ACK f9b22e3bdb

  25. MarcoFalke commented at 11:09 AM on May 26, 2020: member

    ACK f9b22e3bdb 📫

    <details><summary>Show signature and timestamp</summary>

    Signature:

    -----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA512

ACK f9b22e3bdb 📫
-----BEGIN PGP SIGNATURE-----

iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
pUhWqQv/XMjZcKGe3IffrT2bXwemMhi9fKPwO9pJcYGnW6+V/8iHWrTF44+knH5Z
k+gO6xywZRkw2LwHFQ2DRK8YYQex+AtAK4gsMu13bGZn6NoJToqgjfN4FZzZafzn
ZJmyOlPP2jO2GKAh3ur0ymUVRB5IvuitTnkjHJpFYgv0T+fFEEmrT+IwB6Gj+c9i
vjRTqU1Pg4eYxtS/zygb1faD6IjN2QUK/uiIV6wdy1aXBVxivx3/p7lCgN9J7M16
TrgvkdTX8Ox4mDMHV+bxmmDKPkDM2Z3ZmAlG2fr4l6ZQF57wQZ4InXRLJS/Rqx/Z
R3UzNufWomLHuwvuOyCTuoVuEjk6MmRmBYxa1mTD1JdC1zEdDBCGrB6Yc/TUzHKI
oUSIDxnvkOq4NGetBBuR0KwWuMOvpGfywV5zryVeMYvD4FtbrIauzDCF9Khjwqtf
Znhrmdf70sQDm8kU36/w64Iph54oERYYsV4DbhxqeGJQ+K1j6YKzOIvR7bPsS7ZX
k+eVRXEB
=x9vw
-----END PGP SIGNATURE-----

    Timestamp of file with hash ead5a215f8a35eb59ebc41b959be5926bd52e9eb70983fd9272038e5d010f341 -

    </details>

  26. MarcoFalke merged this on May 26, 2020
  27. MarcoFalke closed this on May 26, 2020
  28. sidhujag referenced this in commit 4426e28416 on May 27, 2020
  29. deadalnix referenced this in commit 6332444c07 on Jan 26, 2021
  30. practicalswift deleted the branch on Apr 10, 2021
  31. kittywhiskers referenced this in commit c881c38441 on May 7, 2022
  32. kittywhiskers referenced this in commit 815fc1ee8e on May 7, 2022
  33. kittywhiskers referenced this in commit 3782696ea4 on Jun 14, 2022
  34. kittywhiskers referenced this in commit 78c617e757 on Jun 14, 2022
  35. kittywhiskers referenced this in commit 75ed624ce4 on Jun 14, 2022
  36. kittywhiskers referenced this in commit 78993aed83 on Jun 18, 2022
  37. kittywhiskers referenced this in commit 2c6b999160 on Jun 18, 2022
  38. kittywhiskers referenced this in commit dee47502c2 on Jul 4, 2022
  39. kittywhiskers referenced this in commit d431f22691 on Jul 4, 2022
  40. kittywhiskers referenced this in commit 08c9364a0a on Jul 6, 2022
  41. kittywhiskers referenced this in commit 597d670a55 on Jul 6, 2022
  42. kittywhiskers referenced this in commit 9d7d2f96a6 on Jul 6, 2022
  43. PastaPastaPasta referenced this in commit eefdae1a53 on Jul 12, 2022
  44. kittywhiskers referenced this in commit 96b7d0373d on Jul 13, 2022
  45. kittywhiskers referenced this in commit 3356a43466 on Jul 13, 2022
  46. kittywhiskers referenced this in commit 09825e4678 on Jul 15, 2022
  47. PastaPastaPasta referenced this in commit 30d6584cb6 on Jul 17, 2022
  48. DrahtBot locked this on Aug 16, 2022
Contributors
practicalswiftMarcoFalkejb55laanwj
Labels
Tests
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-04-16 15:14 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me