test: fix scripts in `blockfilter_basic_test` #33162

pull UdjinM6 wants to merge 1 commits into bitcoin:master from UdjinM6:fix_scripts_blockfilter_basic_test changing 1 files +6 −6
  1. UdjinM6 commented at 10:07 AM on August 9, 2025: contributor

    std::vector fill ctor is like this:

    // Constructs a vector with `count` copies of elements with value `value`.
    explicit vector( size_type count, const T& value = T(), const Allocator& alloc = Allocator() ); // (until C++11)
    vector( size_type count, const T& value, const Allocator& alloc = Allocator() ); // (since C++11)(constexpr since C++20)
    

    https://en.cppreference.com/w/cpp/container/vector/vector.html

    i.e. std::vector<unsigned char>(0, 65) means a vector with 0 copies of 65 which feels wrong. I believe count and value were swapped in blockfilter_basic_test scripts.

  2. test: fix scripts in `blockfilter_basic_test` ca64b71ed5
  3. DrahtBot added the label Tests on Aug 9, 2025
  4. DrahtBot commented at 10:08 AM on August 9, 2025: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--006a51241073e994b41acfe9ec718e94-->

    Code Coverage & Benchmarks

    For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/33162.

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    ACK furszy, pablomartin4btc, janb84
    Concept ACK pinheadmz

    If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

    <!--5faf32d7da4f0f540f40219e4f7537a3-->

  5. pinheadmz commented at 11:23 AM on August 9, 2025: member

    concept ACK

    I think you're right! Found an example of another test with the values in the correct order:

    https://github.com/bitcoin/bitcoin/blob/2581258ec200efb173ea6449ad09b2e7f1cc02e0/src/bench/ccoins_caching.cpp#L38-L44

    It makes sense the block filter test isn't actually checking scripts, just checking the compression and retrieval mechanism.

  6. furszy commented at 10:32 PM on August 9, 2025: member

    ACK ca64b71ed5ecbef66d4bb294dfcdff638157632c

  7. DrahtBot requested review from pinheadmz on Aug 9, 2025
  8. pablomartin4btc commented at 5:00 PM on August 10, 2025: member

    ACK ca64b71ed5ecbef66d4bb294dfcdff638157632c

    This change matches how std::vector constructor is being used in other tests (src/test/transaction_tests.cpp and src/bench/ccoins_caching.cpp - last one mentioned above).

  9. janb84 commented at 10:02 AM on August 11, 2025: contributor

    ACK ca64b71ed5ecbef66d4bb294dfcdff638157632c

    Good find !

    PR fixes 8 year old error in the unit test while creating instances of std::vector<unsigned char>

    The unit test does not validate the validity of the scripts therefor it does not fail. When moving a excluded (fixed) script to the included_script array the unit test will fail (tested)

    • code review ✅
    • history review ✅
    • build & tested ✅
    • purposely broken the test ✅
  10. glozow merged this on Aug 11, 2025
  11. glozow closed this on Aug 11, 2025

  12. alexanderwiederin referenced this in commit 67b164b083 on Aug 21, 2025
  13. yuvicc referenced this in commit 22f55cf11d on Aug 26, 2025
  14. stringintech referenced this in commit ccb9779390 on Aug 27, 2025
  15. bug-castercv502 referenced this in commit 5ccbb9e900 on Sep 28, 2025
  16. stickies-v referenced this in commit a19c56cd7c on Nov 4, 2025
  17. Kino1994 referenced this in commit 2fa2caa970 on Jun 28, 2026
  18. BigcoinBGC referenced this in commit 3a3c90c413 on Jun 30, 2026
  19. bitcoin locked this on Aug 11, 2026

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-08-31 17:51 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me