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. 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.

   0aef3d931a
8. average-gary force-pushed on Jun 30, 2025
9. DrahtBot removed the label CI failed on Jun 30, 2025

Contributors
average-gary DrahtBot

Labels
Tests