fix: differentiate legacy and segwit types for p2sh when using Descriptor::scriptpubkey_type

[qlrd]

• fix scriptpubkey_type return p2sh when using p2sh-p2wsh #93;

Add a check for p2sh-p2wsh, p2sh-p2wpkh and raise an exception for
unknow types.

With this fix i was able to do a proper recognition of nested segwit multisig wallet when using this method to load a descriptor.

[odudex]

The commit from this pull request has now been merged into Embit's develop branch.
Edit: The change was reverted and removed from develop until the below issues are solved.

[jdlcdl]

Note: To include this in release-notes since this alters api/expected-return where existing projects may be expecting "p2sh"

[odudex]

Another issue:
In order to use owns() method from descriptor.py and correctly detect change and self transfers in nested types, script_type() from script.py must be updated accordingly to cover detailed p2sh: p2sh-p2wpkh and p2sh-p2wsh

Edit: I believe script_type() can't retrieve more information than it does now. So another approach should be used

[tadeubas]

How Specter DIY and SS does that if it is not implemented?

[qlrd]

In order to use owns() method from descriptor.py and correctly detect change and self transfers in nested types, script_type() from script.py must be updated accordingly to cover detailed p2sh: p2sh-p2wpkh and p2sh-p2wsh

I think that something like this could be used in owns (but didn't tested):

- if psbt_scope.script_pubkey.script_type() != self.scriptpubkey_type():
+ if psbt_scope.script_pubkey.script_type() not in self.scriptpubkey_type():

I believe script_type() can't retrieve more information than it does now. So another approach should be used

yup, but during the sign procedure we can check, as a lib, if there's redeem_script and redeem_script.script_type() to assert p2sh, p2sh-p2wpkh, p2sh-p2wsh (i effectively did that).

How Specter DIY and SS does that if it is not implemented?

It's better to ask @kdmukai and @k9ert.

[odudex]

I think that something like this could be used in owns (but didn't tested):

- if psbt_scope.script_pubkey.script_type() != self.scriptpubkey_type():
+ if psbt_scope.script_pubkey.script_type() not in self.scriptpubkey_type():

It seems to be a good idea!

yup, but during the sign procedure we can check, as a lib, if there's redeem_script and redeem_script.script_type() to assert p2sh, p2sh-p2wpkh, p2sh-p2wsh (i effectively did that).

With the above idea it might not be necessary.

[qlrd]

I think that something like this could be used in owns (but didn't tested):

- if psbt_scope.script_pubkey.script_type() != self.scriptpubkey_type():
+ if psbt_scope.script_pubkey.script_type() not in self.scriptpubkey_type():

It seems to be a good idea!

Changed. Also some test cases for singlesig and nested contexts (need to think in more cases and add some taproot cases).

[qlrd]

I think that something like this could be used in owns (but didn't tested):

- if psbt_scope.script_pubkey.script_type() != self.scriptpubkey_type():
+ if psbt_scope.script_pubkey.script_type() not in self.scriptpubkey_type():

It seems to be a good idea!

yup, but during the sign procedure we can check, as a lib, if there's redeem_script and redeem_script.script_type() to assert p2sh, p2sh-p2wpkh, p2sh-p2wsh (i effectively did that).

With the above idea it might not be necessary.

Also started with some signet PSBTs for testing Descriptor.owns

[odudex]

These changes are not required anymore, am I right @qlrd ?