Merge bitcoin/bitcoin#31624: doc: warn that CheckBlock() underestimates sigops

ryanofsky · ryanofsky · commit f1d78a3087fb · 2025-05-14T10:05:41.000-04:00
a04f17a doc: warn that CheckBlock() underestimates sigops (Sjors Provoost) Pull request description: Counting sigops in the witness requires context that `CheckBlock()` does not have, so it only counts sigops for non-segwit transactions. It's useful to document, but it should not be a problem. The commit message contains some historical context. ACKs for top commit: ismaelsadeeq: ACK a04f17a ryanofsky: Code review ACK a04f17a Tree-SHA512: 26528367a7f3cfa8540ef0b90f7aa912c8f0bc057428f20a1fd1d4e232dac77747bc20044f0fcb0ffab8a2e1fb3dbe3dab46be749553a917744ddc7a829025cb
diff --git a/src/validation.cpp b/src/validation.cpp
@@ -4062,6 +4062,8 @@ bool CheckBlock(const CBlock& block, BlockValidationState& state, const Consensu
                                  strprintf("Transaction check failed (tx hash %s) %s", tx->GetHash().ToString(), tx_state.GetDebugMessage()));
         }
     }
+    // This underestimates the number of sigops, because unlike ConnectBlock it
+    // does not count witness and p2sh sigops.
     unsigned int nSigOps = 0;
     for (const auto& tx : block.vtx)
     {

Original file line number	Diff line number	Diff line change
`@@ -4062,6 +4062,8 @@ bool CheckBlock(const CBlock& block, BlockValidationState& state, const Consensu`
`4062`	`4062`	`strprintf("Transaction check failed (tx hash %s) %s", tx->GetHash().ToString(), tx_state.GetDebugMessage()));`
`4063`	`4063`	`}`
`4064`	`4064`	`}`
	`4065`	`+ // This underestimates the number of sigops, because unlike ConnectBlock it`
	`4066`	`+ // does not count witness and p2sh sigops.`
`4065`	`4067`	`unsigned int nSigOps = 0;`
`4066`	`4068`	`for (const auto& tx : block.vtx)`
`4067`	`4069`	`{`