PAYTOP2SHWITNESS detection through getAddressScriptType

[sidhujag]

Hello,

I think there may be a bug in getAddressScriptType which is not correctly identifying PAYTOP2SHWITNESS in the case if address_n is not passed into deriveOutputScript's output parameter:

https://github.com/trezor/connect/blob/f71a8e256bf8a51c9bf835291e4969d994d56aff/src/js/core/methods/helpers/signtxVerify.js#L112

getAddressScriptType: https://github.com/trezor/connect/blob/f71a8e256bf8a51c9bf835291e4969d994d56aff/src/js/utils/addressUtils.js#L95

The logic should be:

function() {
  if not bech32(address):
     decodeBase58Check(address):
         if version is p2pkh:
            return PAYTOADDRESS
        else if version is p2sh:
            return PAYTOSCRIPTHASH
  else:
     decodeBech32(address):
         if length is 20:
            return PAYTOWITNESS
         else if length is 32:
            return PAYTOP2SHWITNESS
  return PAYTOADDRESS
}

Incidentally PAYTOP2SHWITNESS is not P2WSH right, is there support for P2WSH in Trezor yet?

[szymonlesisz]

hi @sidhujag,

P2WSH is supported, case with p2wsh output is tested here

PAYTOP2SHWITNESS refers to change output (with address_n) SegWit over P2SH

getAddressScriptType returns PAYTOWITNESS for both p2wpkh and p2wsh addresses and output script is derived by deriveBech32Output function

[sidhujag]

PAYTOWITNESS for both p2wpkh and p2wsh

ok great and p2wsh inputs being spent will also just be SPENDWITNESS using bip84? (84 as first byte in address_n of input)

[szymonlesisz]

exactly, there is not test for that in connect (i will add it) but here is the example in python

[szymonlesisz]

i guess i can close it then