test: Enhance GetTxSigOpCost tests for coinbase transactions #32840

1. 
   average-gary commented at 7:04 pm on June 30, 2025: none

   Added assertions to the GetTxSigOpCost test cases to verify that the witness of a coinbase transaction is not considered in the signature operation cost calculations.

   Using spendingTx.vin[0].prevout.SetNull() we create a coinbase transaction that evaluates to true for IsCoinbase(). Doing this to transactions (spendingTx in this case) that evaluate to a non-zero sigop output, we more concretely test that the witness of a coinbase transaction is not taken into account for SigOp maths.

   ---

   In my experimentation in mining software (mostly Stratum v2) I encountered the SigOps budget and began exploring the considerations as it applies to coinbase transactions. It was unclear how commitment-type addresses for coinbase were handled compared to bare script when it came to SigOp calculation. Upon further investigation, I saw that the test suite could have added vectors that clearly demonstrate that the witness for a coinbase transaction is not considered for GetTransactionSigOpCost.

   Adding these tests makes it more clear for someone in the future how SigOp maths work while exploring the intersection of SigOps and coinbase transactions.

2. DrahtBot added the label Tests on Jun 30, 2025
3. 
   DrahtBot commented at 7:04 pm on June 30, 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/32840.

   Reviews

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

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #32317 (kernel: Separate UTXO set access from validation functions by TheCharlatan)

   If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

4. average-gary force-pushed on Jun 30, 2025
5. DrahtBot added the label CI failed on Jun 30, 2025
6. 
   DrahtBot commented at 7:11 pm on June 30, 2025: contributor

   🚧 At least one of the CI tasks failed. Task lint: https://github.com/bitcoin/bitcoin/runs/45076576501 LLM reason (✨ experimental): Lint check failed due to presence of trailing whitespace in source files.

   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.

7. average-gary force-pushed on Jun 30, 2025
8. DrahtBot removed the label CI failed on Jun 30, 2025
9. 
   Sammie05 commented at 3:04 am on July 13, 2025: none

   Thanks for adding these assertions! I like how you explicitly test that coinbase witnesses don’t contribute to SigOp cost. Makes it clearer for future maintainers. LGTM 👍
10. DrahtBot added the label Needs rebase on Oct 15, 2025
11. achow101 requested review from Sjors on Oct 22, 2025
12. achow101 requested review from theStack on Oct 22, 2025
13. achow101 requested review from l0rinc on Oct 22, 2025
14. achow101 requested review from davidgumberg on Oct 22, 2025
15. test: Enhance GetTxSigOpCost tests for coinbase transactions

    Added assertions to the GetTxSigOpCost test cases to verify that the witness of a coinbase transaction is not considered in the signature operation cost calculations.
    
    Using spendingTx.vin[0].prevout.SetNull() we create a coinbase transaction that evaluates to true for IsCoinbase(). Doing this to transactions (spendingTx in this case) that evaluate to a non-zero sigop output, we more concretely test that the witness of a coinbase transaction is not taken into account for SigOp maths.

    6b6b0f4af6
16. average-gary force-pushed on Oct 23, 2025
17. DrahtBot removed the label Needs rebase on Oct 23, 2025

Contributors
average-gary DrahtBot Sammie05

Review Requested
Sjors theStack l0rinc davidgumberg

Labels
Tests