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


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-05-02 03:12 UTC

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