Remove CheckSig() dependence on CPubKey.IsValid() #3743

This was a non-obvious consensus-critical dependence; just the kind of thing that could lead to consensus bug in the future if someone ever changed CPubKey.IsValid()

How do we test this change?

There's a testcase in the unit tests that ensures that a '' CHECKSIG NOT is considered a valid transaction. The CPubKey.IsValid() function just tests size() > 0 after all, and size() returns an unsigned char.

Which is where things get interesting: CPubKey.size() is a new thing from Pieter when he created the CPubKey class; the previous code in checksig tested vchSig.empty(). Since the new CPubKey class assumes valid pubkeys are only 33 or 65 bytes, not even saving their contents otherwise, you certainly could cause a fork between current and the version prior to CPubKey by passing a non-33/65 byte pubkey that OpenSSL accepted for some reason.

So thinking about it more, maybe this actually should be NACK'd as against the spirit, however potentially dangerous, of the CPubKey class. So I'll close this and open another one just documenting the fact that IsValid() is consensus critical.

FWIW this came up from work I'm doing on python-bitcoinlib's validation machinery.