Any reason why? As described in the spec, the sigChecks count for a failing scripts is unspecified. It really shouldn't make a difference under any circumstances (if it matters, then other much more important things have gone horribly wrong).

Can you be more specific?

The required behaviour is: scan *all* signatures (not just scan some of signatures like in the main loop above), and if *any* of them is non-null then increment sigchecks by nKeysCount, once.

No, but I do plan to do integer arithmetic (and not modulo arithmetic/bitwise manips) with the sigchecks count, and the common wisdom is that you don't use modulo ints ("unsigned") just because an arithmetical quantity is nonnegative.

That's true, it will compile, but I'd like to avoid gcc throwing up a bunch of warnings about unused function coming from this header when it's only static.

For aggregation, my plan is like so:

A fresh ScriptExecutionMetrics is generated for every call to VerifyScript (i.e., a new one for every input and for every distinct set of flags). So, at first aggregation only happens across the sequential EvalScript calls that happen on the same stack. Each ScriptExecutionMetrics needs to be unshared and starts fresh for that input, so that any per-input limitations know what happened for that input specifically and separately.

In CScriptChecks::operator(), once the VerifyScript is done it can then merge the per-input metrics into a coarser aggregator, which can be one of two types:

• add the per-input metrics to some transaction-aggregate metrics, during mempool validation (would be needed for doing transaction vbytes size calculation); once CheckInputs is done running all inputs, throw the aggregate metrics into the script cache (as relevant).
• subtract the per-input metrics from a block-aggregate atomic 'metrics-remaining', during block parallel validation; if the atomic metrics-remaining falls below 0 then bail.

Because you want the signature check and the accounting of signature check to be together.

The only situation where it makes a difference is if you use the count in a case where nullfail is not present. Making the code more complex to support something that never happens is a bad idea.

As I described in telegram, that's the plan eventually after sigops is removed -- calculate sigchecks on all blocks, both historical and current.

By using this limit, it means that CPU usage per block will be limited even if the node somehow gets sidetracked onto an alternate history chain prior to nullfail, that is designed to introduce high CPU usage. Yes, it can happen- this could happen maybe because the user disabled checkpoints, or because the checkpoints don't work.

FWIW I believe this way is perfectly clear -- "are all signatures null? if not, then add N sigchecks". Putting something into the loop makes it much more awkward since the loop never performs N iterations, except the special case of N==M and success. If you have a better way to code this, please show me the code.

It cannot, there is a checkpoint.

linter save me please :-)

Sure, see D4754.

Yeah this is a good idea.

Yeah that's odd. The linter seems to insist on them (if I remove them, they get put back).