Potentially misleading/buggy implementation in checking P2WPK and P2WSH nested in P2SH

[wanwenli]

In class CScript of core.script, the comments and the method names of
is_witness_v0_nested_keyhash and is_witness_v0_nested_scripthash are misleading and one implementation is susceptibly buggy.

Firstly, P2WPK and P2WSH cannot be distinguished in outputs, according to examples given in BIP-0141. In other words, just by looking at their outputs they are merely P2SH. Therefore I would like to propose changing both the names and comments of these two methods. The nested type cannot be told by checking "key hash" or "script hash" as their names suggest. Anyway from tracing the call hierarchy in this test case I have found that is_witness_v0_nested_keyhash is used to check redeemScript, rather than what its name and comment suggest. However is_witness_v0_nested_scripthash is not found to be used anywhere in the code base.

Most importantly, the implementation of is_witness_v0_nested_scripthash is wrong. Its logic is essentially the same as is_p2sh.

Here is an example:

The last output of transaction 44323efd5714949d516f9970899da3c0207ed1308b0deb160efe099d047f641e is of P2SH type and I use its script hex to test against your API's in IPython.

In [4]: script = CScript(bytearray.fromhex("a9148a8ceee90f9da5402de69e72055933d830fbf6b987"))

In [5]: script.is_p2sh()
Out[5]: True

In [6]: script.is_witness_v0_nested_scripthash()
Out[6]: True

This output has been spent in transaction 7927a96ff5e3915b95bdbfef648cd342bd5bd8b13be35602d4ac0e30dab95e9e as the sole input and it is found to be a generic P2SH-MULTISIG payment, not a P2WSH type.

If you agree with me, please allow me to submit a PR. Otherwise please give your suggestions. Many thanks.

[petertodd]

Yup, I think you're right here. Removing those two functions is fine by me; that pull-req is welcome!