test,bench: validate CheckTransaction’s duplicate input detection in broader context #31699

1. 
   l0rinc commented at 2:03 pm on January 21, 2025: contributor

   This PR follows up on #31682, isolating its testing and benchmarking improvements to focus on CheckTransaction’s duplicate input detection and performance in realistic contexts, avoiding the controversial consensus code change.

   First commit adds tests to validate CheckTransaction’s ordering-based duplicate detection against the hash-based methods used elsewhere in the codebase.

   Second commit replaces overly-specific microbenchmarks with ProcessTransactionBench, which implicitly measures CheckTransaction as part of the full validation pipeline.

2. 
   DrahtBot commented at 2:03 pm on January 21, 2025: contributor

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

   Code Coverage & Benchmarks

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

   Reviews

   See the guideline for information on the review process. A summary of reviews will appear here.

   Conflicts

   No conflicts as of last run.

3. l0rinc force-pushed on Jan 21, 2025
4. 
   DrahtBot commented at 2:10 pm on January 21, 2025: contributor

   🚧 At least one of the CI tasks failed. Debug: https://github.com/bitcoin/bitcoin/runs/35933961993

   Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

   • Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

   • A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

   • An intermittent issue.

   Leave a comment here, if you need help tracking down a confusing failure.

5. DrahtBot added the label CI failed on Jan 21, 2025
6. l0rinc force-pushed on Jan 21, 2025
7. laanwj added the label Tests on Jan 21, 2025
8. DrahtBot removed the label CI failed on Jan 21, 2025
9. l0rinc force-pushed on Jan 21, 2025
10. test: validate duplicate detection in `CheckTransaction`

    The `CheckTransaction` validation function in https://github.com/bitcoin/bitcoin/blob/master/src/consensus/tx_check.cpp#L41-L45 relies on a correct ordering relation for detecting duplicate transaction inputs.
    
    This update to the tests ensures that:
    * Accurate detection of duplicates: Beyond trivial cases (e.g., two identical inputs), duplicates are detected correctly in more complex scenarios.
    * Consistency across methods: Both sorted sets and hash-based sets behave identically when detecting duplicates for `COutPoint` and related values.
    * Robust ordering and equality relations: The function maintains expected behavior for ordering and equality checks.
    
    Using randomized testing with shuffled inputs, the enhanced test validates that `CheckTransaction` remains robust and reliable across various input configurations. It confirms identical behavior to a hashing-based duplicate detection mechanism, ensuring consistency and correctness.

    8b9754e47f
11. bench: add `ProcessTransactionBench` to measure `CheckBlock` in context

    The newly introduced `ProcessTransactionBench` incorporates multiple steps in the validation pipeline, offering a more comprehensive view of `CheckBlock` performance within a realistic transaction validation context.
    
    Previous microbenchmarks, such as DeserializeAndCheckBlockTest and DuplicateInputs, focused on isolated aspects of transaction and block validation. While these tests provided valuable insights for targeted profiling, they lacked context regarding the broader validation process, where interactions between components play a critical role.

    d90a0b8c90
12. l0rinc force-pushed on Jan 21, 2025
13. DrahtBot added the label CI failed on Jan 21, 2025
14. 
    DrahtBot commented at 3:35 pm on January 21, 2025: contributor

    🚧 At least one of the CI tasks failed. Debug: https://github.com/bitcoin/bitcoin/runs/35939718499

    Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

    • Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.

    • A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.

    • An intermittent issue.

    Leave a comment here, if you need help tracking down a confusing failure.

15. DrahtBot removed the label CI failed on Jan 21, 2025
16. l0rinc closed this on Jan 23, 2025

    ---

Contributors
l0rinc DrahtBot

Labels
Tests