test: Enhance GetTxSigOpCost tests for coinbase transactions #32840
Conversation
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code Coverage & BenchmarksFor details see: https://corecheck.dev/bitcoin/bitcoin/pulls/32840. ReviewsSee the guideline for information on the review process. ConflictsReviewers, this pull request conflicts with the following ones:
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. |
871c671 to
2449a7c
Compare
|
🚧 At least one of the CI tasks failed. HintsTry to run the tests locally, according to the documentation. However, a CI failure may still
Leave a comment here, if you need help tracking down a confusing failure. |
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.
2449a7c to
0aef3d9
Compare
|
Thanks for adding these assertions! |
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.