Python API supports P2WSH

trezor / trezor-firmware

:lock: Trezor Firmware Monorepo

https://trezor.io

Other

1.29k stars 639 forks source link

Python API supports P2WSH #1355

Closed Jubertos closed 3 years ago

Jubertos commented 3 years ago

1. Is your feature request related to a problem? Please describe.

I would like to spend from P2WSH address using python API.

More detailed description

We have multiple Trezor devices, which are used as Cold Wallets. As Hot Wallet we use Bitcoin Core node.

Each Bitcoin multisig address we use is composed of:

multiple Trezor public keys
1 Hot Wallet address

To sign transactions with Trezor Cold Wallets we use the python library (v 0.7.13)

We have no problems signing transactions with our Trezors as long as address.script is 'multisig'.

But... Recently we have created a multisig, using SegWit bech32 address as Hot Wallet address. This multisig address.script is witness_v0_scripthash

We have a problem signing with our Trezors, a transaction, which spends from this address. When we try to sign this transaction using python library:

      inputs.append(proto_types.TxInputType(  
        script_type=proto_types.SPENDP2SHWITNESS,  
        address_n=path,  
        ...,  
      ))  
      ...  
      (signatures, serialized_tx) = client.sign_tx('Testnet' if TESTNET else 'Bitcoin', inputs, outputs)

it finishes without errors but produces scriptSig in this format:

        scriptSig: <signature> <publicKey>

which is actually scriptSig for P2PKH.

This is of course not what we expect.

2. Describe the solution you'd like

Native support for this P2WSH inputs.

3. Describe alternatives you've considered

A natural workaround would be to create a raw transaction using correct signatures provided by the Trezor. The scenario could look like this:

Prepare a raw transaction to be signed
Generate Trezor signature for this transaction
Assembly the final raw transaction using the produced signature

For steps 1 and 3 a general knowledge of how raw transactions are built is required but this is not a big problem. The problem is step 2. Is it possible to use for example this method:

   client.sign_message()

to create valid signatures?

If yes, could you guide us what arguments we need to provide? If not is there any other way to create valid signatures programmatically using python API?

4. Additional context

These files contain example that has been prepared on testnet. This example shows exactly the problem.

1_prev_transaction_raw.txt

This is the transaction that sends to multisg(segwit) address. Interesting is output.scriptPubKey

2_transaction_raw_unsigned.txt

Transaction that spends from multisig(segwit) address, and also from other multisig(legacy) address.

3_transaction_raw_signed_by_hot_wallet.txt

Transaction that spends from multisig(segwit) address, which is signed by Bitcoin Core The most interesting is vin.scriptSig which is correct.

4_transaction_raw_signed_by_cold_wallet.txt

Transaction that spends from multisig(segwit), signed by Trezor. The most interesting is vin.scriptSig which is NOT correct.

5_multisig_segwit_address.txt

validateaddress for segwit multisig address.

6_multisig_legacy_address.txt

validateaddress for legacy multisig address.

1_prev_transaction_raw.txt 2_transaction_raw_unsigned.txt 3_transaction_raw_signed_by_hot_wallet.txt 4_transaction_raw_signed_by_cold_wallet.txt 5_multisig_segwit_address.txt 6_multisig_legacy_address.txt

5. Last words

The help with the described workaround scenario would be greatly appreciated. Any other workaround also.

matejcik commented 3 years ago

you're supposed to be using SPENDWITNESS script type.

also bear in mind that your Python library version is several years out of date. If there's a missing feature, you might need to upgrade.

Jubertos commented 3 years ago

@matejcik Thank you for the answer, but please reopen this issue. Consider the following:

SPENDWITNESS instead of SPENDP2SHWITNESS

This does not make any difference. I will elaborate more about the actual result and expected one.

Locking script from prev transaction is P2SH (see attachment 1). Multisig address redeem script is P2WSH. So this is basically gives P2WSH nested in P2SH. The expected input is (this is what Bitcoin Core produces - see attachment 3):

{
      "txid": "016d34d6944abff2123f033e2d99aa932720cc1cd6aef62fee251bc224895049",
      "vout": 0,
      "scriptSig": {
        "asm": "00207677cf0e71ff26dc2ac7e6b93a5f6b893d39c05a1367ca100cde7a62713ddb4a",
        "hex": "2200207677cf0e71ff26dc2ac7e6b93a5f6b893d39c05a1367ca100cde7a62713ddb4a"
      },
      "txinwitness": [
        "",
        "3045022100b57e36875906add1edeb57badc92f5b4287f9e33b609fad5ae35912cdc0596ff022073338c6e14637dfc11abdb18c2396a58b8e537670d06084981799f64bcd7c3f401",
        "53210289d20d3d6e36f28537af3521ce05548fb16baf7d47ac67fbf318db6ed983b9ba21034712e706c1643ec0e9519edf093578886bac1c9c17893138a76111d9a14a2f1b2102a21047ac1a83c8cce21475240b2107a11d76212573a86f0dffe44cdd326f4dab2103430166298971c6282241ef46e866fa9077b4a48eafe4b370e94aef24050dda132102c877ef10e8dc5ca83f06263971f133f71e01e9b2a91a4184eb730026f2c3968f55ae"
      ],
      "sequence": 4294967295
    }

But Python API produces this (see attachment 4):

{
      "txid": "016d34d6944abff2123f033e2d99aa932720cc1cd6aef62fee251bc224895049",
      "vout": 0,
      "scriptSig": {
        "asm": "3045022100cd569e7f8e5fb6490ef3240d68abdd147f0c3ce4e77bcfd59f587b9a14848bdb02207fe31882d22518af84d3961809b6133c06b34ce6f91c1e579dcff0c58b7519d3[ALL] 034712e706c1643ec0e9519edf093578886bac1c9c17893138a76111d9a14a2f1b",
        "hex": "483045022100cd569e7f8e5fb6490ef3240d68abdd147f0c3ce4e77bcfd59f587b9a14848bdb02207fe31882d22518af84d3961809b6133c06b34ce6f91c1e579dcff0c58b7519d30121034712e706c1643ec0e9519edf093578886bac1c9c17893138a76111d9a14a2f1b"
      },
      "sequence": 4294967295
    }

ScriptSig is simply for P2PKH, so how could this be valid? It's not.

Old python lib

I'm aware of this fact. If a newer version can solve this problem then I will be happy to upgrade to a newer version. But this requires upgrading our environment and requires substantial amount of work. So we need to be sure that upgrading actually makes any sense, and the newest version will work properly.

So far we have been told by the support, that this is not supported at all.

prusnak commented 3 years ago

But this requires upgrading our environment and requires substantial amount of work.

You don't need to test this in your production environment. Just test it on your workstation.

matejcik commented 3 years ago

@Jubertos please provide a code snippet that can reproduce your issue. what you show above is not nearly enough data for me to look into

Jubertos commented 3 years ago

@prusnak @matejcik

I have created a simple code snippet for the newest python API (0.12.2). If you need anything else please let me know.

trezor_snippet.tar.gz

matejcik commented 3 years ago

it seems your Trezor firmware is wildly out of date.

For me, the script crashes with the following error: trezorlib.exceptions.TrezorFailure: DataError: signing.c:1166:Segwit input without amount

if I modify the script by adding amount (add amount=int(decimal.Decimal(tx_in["amount"]) * decimal.Decimal(1e8)), at line 175), and add my own pubkey to the list, the following transaction is produced:

010000000001020dcc23ccdb3665779a7657577cd33c5c0660e969ea2ffaf5438c58306ebc9dd201000000fd410100483045022100a9312e98422f8c8d8e1391b2cb854adceeaea5e0beae624f0d8721afc66ec72f02203b8c4f9c14d3d7129f67931ac76c2a63d9dbbf40bf13e7d9dd86edf5cda0900c01473044022079be75240f2a1519559fc3eacfdbeee123403466879d720b9e7f68a45a2b3bef0220585ea1c34766d57e54103ec5102c8fd0925ee2bc1a847797176da9e878d21448014cad53210367e0abbd78d5b8d4ff6a6934d4c98101f272462d97bd49a2a508bf2e91131acf21034712e706c1643ec0e9519edf093578886bac1c9c17893138a76111d9a14a2f1b2102a21047ac1a83c8cce21475240b2107a11d76212573a86f0dffe44cdd326f4dab2103430166298971c6282241ef46e866fa9077b4a48eafe4b370e94aef24050dda13210329446142884a33e6100603c6eead5300ec5cf438437f8374ea76dda2daced12855aeffffffff49508924c21b25ee2ff6aed61ccc202793aa992d3e033f12f2bf4a94d6346d010000000000ffffffff0240420f00000000001976a9140d7ca1e17524d6c96209cf503fb053613cecf8f688acc0b606000000000017a914fa1cee87827866df00431b5c9a3506b5e31ef4ed8700040047304402204eed04a9fe23bf57ec4119cddeef7b6bf6d7cd6ce6a18c177557cbfd9ef6711502205211d304b0a4a09c7b7b8327c015d87ea6b7d6873489e85a385d63f23edc9bf601493045022100b57e36875906add1edeb57badc92f5b4287f9e33b609fad5ae35912cdc0596ff022073338c6e14637dfc11abdb18c2396a58b8e537670d06084981799f64bcd7c3f40101ad53210367e0abbd78d5b8d4ff6a6934d4c98101f272462d97bd49a2a508bf2e91131acf21034712e706c1643ec0e9519edf093578886bac1c9c17893138a76111d9a14a2f1b2102a21047ac1a83c8cce21475240b2107a11d76212573a86f0dffe44cdd326f4dab2103430166298971c6282241ef46e866fa9077b4a48eafe4b370e94aef24050dda132102c877ef10e8dc5ca83f06263971f133f71e01e9b2a91a4184eb730026f2c3968f55ae00000000

which decodes, as expected:

        {
            "txid": "016d34d6944abff2123f033e2d99aa932720cc1cd6aef62fee251bc224895049",
            "vout": 0,
            "scriptSig": {
                "asm": "",
                "hex": ""
            },
            "txinwitness": [
                "",
                "304402204eed04a9fe23bf57ec4119cddeef7b6bf6d7cd6ce6a18c177557cbfd9ef6711502205211d304b0a4a09c7b7b8327c015d87ea6b7d6873489e85a385d63f23edc9bf601",
                "3045022100b57e36875906add1edeb57badc92f5b4287f9e33b609fad5ae35912cdc0596ff022073338c6e14637dfc11abdb18c2396a58b8e537670d06084981799f64bcd7c3f40101",
                "53210367e0abbd78d5b8d4ff6a6934d4c98101f272462d97bd49a2a508bf2e91131acf21034712e706c1643ec0e9519edf093578886bac1c9c17893138a76111d9a14a2f1b2102a21047ac1a83c8cce21475240b2107a11d76212573a86f0dffe44cdd326f4dab2103430166298971c6282241ef46e866fa9077b4a48eafe4b370e94aef24050dda132102c877ef10e8dc5ca83f06263971f133f71e01e9b2a91a4184eb730026f2c3968f55ae"
            ],
            "sequence": 4294967295
        }

i'm afraid I can't advise any further, because I can't find a firmware old enough that does not produce this error

Jubertos commented 3 years ago

Hello @matejcik,

Thank you very much for checking the code snippet I uploaded.

Well, the output you have received is different indeed. And it clearly proves that the firmware is one of the problems. But unfortunately the produced transaction is still wrong.

Why?

That what you received conforms to the format of P2WSH.

    ScriptPubKey: 0 <32-byte-redeemScriptHash>
    ScriptSig: (empty)
    Witness: <witness items> <redeemScript>

    RedeemScript: (any)

And this will not work. Let's look at the locking script:

    a914ad91e77783fee699a7415008266c8b75a2bc425887

which is:

    OP_HASH160 ad91e77783fee699a7415008266c8b75a2bc4258 OP_EQUAL

So in our case we have P2WSH nested in P2SH:

    ScriptPubKey: OP_HASH160 <20-byte-P2SH-RedeemScriptHash> OP_EQUAL
    ScriptSig: <0 <32-byte-P2WSH-RedeemScriptHash>>
    Witness: <witness items> <P2WSH-RedeemScript>

    P2WSH RedeemScript: (any)

So scriptSig cannot be empty. The correct scriptSig is (check attachment 5 which is output of validateaddress):

2200207677cf0e71ff26dc2ac7e6b93a5f6b893d39c05a1367ca100cde7a62713ddb4a

which is easy to confirm:

hash160 (00207677cf0e71ff26dc2ac7e6b93a5f6b893d39c05a1367ca100cde7a62713ddb4a) = ad91e77783fee699a7415008266c8b75a2bc4258

So basically P2SH execution needs to be valid to execute nested P2WSH logic.

And again, this is the format produced by the Bitcoin Core:

{
      "txid": "016d34d6944abff2123f033e2d99aa932720cc1cd6aef62fee251bc224895049",
      "vout": 0,
      "scriptSig": {
        "asm": "00207677cf0e71ff26dc2ac7e6b93a5f6b893d39c05a1367ca100cde7a62713ddb4a",
        "hex": "2200207677cf0e71ff26dc2ac7e6b93a5f6b893d39c05a1367ca100cde7a62713ddb4a"
      },
      "txinwitness": [
        "",
        "3045022100b57e36875906add1edeb57badc92f5b4287f9e33b609fad5ae35912cdc0596ff022073338c6e14637dfc11abdb18c2396a58b8e537670d06084981799f64bcd7c3f401",
        "53210289d20d3d6e36f28537af3521ce05548fb16baf7d47ac67fbf318db6ed983b9ba21034712e706c1643ec0e9519edf093578886bac1c9c17893138a76111d9a14a2f1b2102a21047ac1a83c8cce21475240b2107a11d76212573a86f0dffe44cdd326f4dab2103430166298971c6282241ef46e866fa9077b4a48eafe4b370e94aef24050dda132102c877ef10e8dc5ca83f06263971f133f71e01e9b2a91a4184eb730026f2c3968f55ae"
      ],
      "sequence": 4294967295
    }

It contains scriptSig which is missing in the transaction you received during your testing.

Please let me know what you think about this, and also if you need anything from my side.

prusnak commented 3 years ago

@Jubertos Are you running the latest version of the firmware?

Jubertos commented 3 years ago

Hello @prusnak,

Nope. I'm not running the latest version of the firmware. That's why @matejcik received a different transaction than I did. So I'm aware of this.

But what is the most important, the transaction received by @matejcik is not correct, and I tried to explain in the previous answer why I do believe it is not correct.

Maybe @matejcik can confirm that the test he did, was using the newest firmware.

Jubertos commented 3 years ago

Hello @matejcik and @prusnak,

Please, give an update on this. I need to be aware of the current status.

So basically I believe that it should be clear now, that P2WSH nested in P2SH is not supported (arguments in this comment)

The issue is closed, but it should be opened now IMHO. Unless you:

still believe that the transaction received by @matejcik is correct
even though it is not correct you are not going to provide support for P2WSH nested in P2SH

Anyway, please help me understand the current situation to plan my next steps.

prusnak commented 3 years ago

Please, give an update on this.

Please test with the latest version of the firmware and latest version of python-trezor. We are not going to spend our time investigating bugs in outdated version of the software.

matejcik commented 3 years ago

@Jubertos you might have been clearer at start that you actually want (segwit) nested in P2SH then the appropriate script type is SPENDP2SHWITNESS, which you were originally using.

using that with your snippet produces the following transaction:

010000000001020dcc23ccdb3665779a7657577cd33c5c0660e969ea2ffaf5438c58306ebc9dd201000000fd630100483045022100e20863f540c7e61bcf07b27e5bc6b21811272dce477d9f507ffc3ec05479a025022032acddad7ff8070c8d964ffcc62db30dbe4c358da09da87766182e2e7a2edb1701473044022079be75240f2a1519559fc3eacfdbeee123403466879d720b9e7f68a45a2b3bef0220585ea1c34766d57e54103ec5102c8fd0925ee2bc1a847797176da9e878d21448014ccf5321030e669acac1f280d1ddf441cd2ba5e97417bf2689e4bbec86df4f831bf9f7ffd0210289d20d3d6e36f28537af3521ce05548fb16baf7d47ac67fbf318db6ed983b9ba21034712e706c1643ec0e9519edf093578886bac1c9c17893138a76111d9a14a2f1b2102a21047ac1a83c8cce21475240b2107a11d76212573a86f0dffe44cdd326f4dab2103430166298971c6282241ef46e866fa9077b4a48eafe4b370e94aef24050dda13210329446142884a33e6100603c6eead5300ec5cf438437f8374ea76dda2daced12856aeffffffff49508924c21b25ee2ff6aed61ccc202793aa992d3e033f12f2bf4a94d6346d010000000023220020ba05889f4141a0990726f7b8c5c52a1ba5731f6ecd245997494b2bf056ca716fffffffff0240420f00000000001976a9140d7ca1e17524d6c96209cf503fb053613cecf8f688acc0b606000000000017a914fa1cee87827866df00431b5c9a3506b5e31ef4ed87000400483045022100b80d62d38a4e817642f65afaa0d9a11bf35c9ed2ca6e04a2b027230d726503c902200988a8b645556d4cc3f97d061b6b6576677b841ea89cbb4578aeb303331b46de01493045022100b57e36875906add1edeb57badc92f5b4287f9e33b609fad5ae35912cdc0596ff022073338c6e14637dfc11abdb18c2396a58b8e537670d06084981799f64bcd7c3f40101cf5321030e669acac1f280d1ddf441cd2ba5e97417bf2689e4bbec86df4f831bf9f7ffd0210289d20d3d6e36f28537af3521ce05548fb16baf7d47ac67fbf318db6ed983b9ba21034712e706c1643ec0e9519edf093578886bac1c9c17893138a76111d9a14a2f1b2102a21047ac1a83c8cce21475240b2107a11d76212573a86f0dffe44cdd326f4dab2103430166298971c6282241ef46e866fa9077b4a48eafe4b370e94aef24050dda132102c877ef10e8dc5ca83f06263971f133f71e01e9b2a91a4184eb730026f2c3968f56ae00000000

which decodes to:

            "txid": "016d34d6944abff2123f033e2d99aa932720cc1cd6aef62fee251bc224895049",
            "vout": 0,
            "scriptSig": {
                "asm": "0020ba05889f4141a0990726f7b8c5c52a1ba5731f6ecd245997494b2bf056ca716f",
                "hex": "220020ba05889f4141a0990726f7b8c5c52a1ba5731f6ecd245997494b2bf056ca716f"
            },
            "txinwitness": [
                "",
                "3045022100b80d62d38a4e817642f65afaa0d9a11bf35c9ed2ca6e04a2b027230d726503c902200988a8b645556d4cc3f97d061b6b6576677b841ea89cbb4578aeb303331b46de01",
                "3045022100b57e36875906add1edeb57badc92f5b4287f9e33b609fad5ae35912cdc0596ff022073338c6e14637dfc11abdb18c2396a58b8e537670d06084981799f64bcd7c3f40101",
                "5321030e669acac1f280d1ddf441cd2ba5e97417bf2689e4bbec86df4f831bf9f7ffd0210289d20d3d6e36f28537af3521ce05548fb16baf7d47ac67fbf318db6ed983b9ba21034712e706c1643ec0e9519edf093578886bac1c9c17893138a76111d9a14a2f1b2102a21047ac1a83c8cce21475240b2107a11d76212573a86f0dffe44cdd326f4dab2103430166298971c6282241ef46e866fa9077b4a48eafe4b370e94aef24050dda132102c877ef10e8dc5ca83f06263971f133f71e01e9b2a91a4184eb730026f2c3968f56ae"
            ],
            "sequence": 4294967295

which seems to match your Bitcoin Core format pretty closely

Jubertos commented 3 years ago

@matejcik thank you for the latest post

The format looks great. The unlocking script is set up.

I wanted to test this myself so I updated the firmware of one of our trezors to the newest version which is 1.9.3 (we use Trezor One).

I modified the code snippet to set amount for the inputs (similar to what you have done yourself). I have run the snippet code with:

Trezor One (firmware 1.9.3) - newest
python API lib version 0.12.2 - newest

Unfortunately I cannot receive the result you have received:

Traceback (most recent call last):
  File "trezor_snippet.py", line 239, in <module>
    main()
  File "trezor_snippet.py", line 139, in main
    (signatures, serialized_tx) = btc.sign_tx(client, 'Testnet', inputs, outputs, prev_txes=txes)
  File "/home/jubertos/code/Btc/trezor_snippet/venv/lib/python3.7/site-packages/trezorlib/tools.py", line 231, in wrapped_f
    return f(client, *args, **kwargs)
  File "/home/jubertos/code/Btc/trezor_snippet/venv/lib/python3.7/site-packages/trezorlib/btc.py", line 248, in sign_tx
    res = client.call(messages.TxAck(tx=msg))
  File "/home/jubertos/code/Btc/trezor_snippet/venv/lib/python3.7/site-packages/trezorlib/tools.py", line 231, in wrapped_f
    return f(client, *args, **kwargs)
  File "/home/jubertos/code/Btc/trezor_snippet/venv/lib/python3.7/site-packages/trezorlib/client.py", line 223, in call
    raise exceptions.TrezorFailure(resp)
trezorlib.exceptions.TrezorFailure: ProcessError: Failed to compile input

BTW: this script does not fail with unupgraded devices

So I need to ask:

which type of trezor device have you used during testing? One or T?
which firmware version have you used?

prusnak commented 3 years ago

BTW: this script does not fail with unupgraded devices

Can you share the trezor_snippet.py again? My guess is you are not passing input amounts.

matejcik commented 3 years ago

no, they're hitting path protection now. @Jubertos make sure you use matching SLIP44 id in your path (the script claims 44h/0h) for the coin (script claims Testnet, which should be 44h/1h).

The way it is written, you're signing a fully valid Bitcoin transaction with Bitcoin keys, but it shows "TEST" on the Trezor screen. Which is obviously something that is not allowed.

Jubertos commented 3 years ago

@prusnak No, the amounts for inputs are set correctly.

@matejcik You are right. Path protection is the reason of the error. Could be more descriptive though. I did two tests:

one by just adding a 'correct path' public key to the test case from the code snippet (this changes scriptSig)
second with a fresh multisig address composed of 'correct path public keys' to confirm that scriptSig is produced as expected All went fine.

So it seems that Trezor python API works fine with P2WSH nested in P2SH.

Thank you for the support. Especially big thanks to you @matejcik. Your help was crucial for us. You are the man !