[script] Add method count_sigops

[junderw]

Planning to also add methods for the various parts of Transaction etc. to eventually allow for easier sigops calculation.

Bare multisig is making a comeback, which is causing a large amount of transactions' effective vSizes (for fee calculation) to be dependent on the sigop count.

This is a first step at making those transactions easier to estimate fees for / template blocks for etc.

[junderw]

A preview of what's to come:

junderw/rust-bitcoin@feat/getsigopcount...feat/wip-sigop-tx

This is very rough, and I'm assuming it will need to be behind the consensus flag and needs tests... but this is a rough WIP idea of where I'm trying to go with this.

Eventually, we want Esplora to return sigop based vSize as well (if it's higher).

But for now (this PR) I'm putting it off until we discuss here.

[Kixunil]

Concept ACK

[junderw]

Continuing on the thread of things that aren't rust-like, perhaps we should make two methods, get_sigop_count and get_sigop_count_legacy which used accurate = true and false respectively.

Either that or create a bool-enum that's more descriptive, like Accurate, Legacy... any thoughts?

[apoelstra]

I'd prefer two methods to an enum, but I don't feel strongly.

[tcharding]

Thanks for your contribution!

Its no biggy but I'd prefer it all in one patch because later patches change earlier patches. I added a rustdoc suggestion to test for off-by-one errors.

[tcharding]

Now we definitely want a single commit, this is going to be annoying for others to review :)

[junderw]

Done.

[tcharding]

hmmm, I don't really like the co-authored-by tag but because you put it in it forced me to look a lot closer at the code so I guess that's a win. (All I did was give some review suggestions so I totally don't need attribution, but thanks for the thought.)

To the best of my knowledge this PR mirrors the behavior in Core CScript::GetSigOpCount.

ACK 85e4672

[junderw]

I think I just invented a new underhanded way to get a review! hahahahahah /s

[Kixunil]

I like changing it to two methods, I was thinking about it before but forgot to mention it. Bit more bikeshedding: in Rust we tend to not use the get_ prefix, so maybe get rid of it? And maybe even rename the method to count_sigops to signal this has linear complexity?

(Also @tcharding you forgot to hit approve)

[junderw]

1. Made the name change. (I agree, the count_sigops name is better)
2. Removed @tcharding as Co-author. Thanks for your suggestion on the doctest!
3. Force pushed. (You can see the diff here)

[Kixunil]

ACK 638445f

Can be improved in a followup PR.

[junderw]

Follow up PR started here: 24a3352

[tcharding]

ACK 638445f

[sanket1729]

post merge ACK 1a1fe0e. Double checked with bitcoin core source.