trezor / connect

:link: A platform for easy integration of Trezor into 3rd party services
Other
349 stars 262 forks source link

Accept P2WSH outputs. #520

Closed nodech closed 4 years ago

nodech commented 4 years ago

If you have P2WSH output for transaction, code will still try to create P2WPKH instead and throw Output ${i} scripts differ..

Code that generates incorrect script for p2wsh is in js/core/methods/helpers/signtxVerify.js: https://github.com/trezor/connect/blob/6b568b2f6828951fac64009a4d542f5391e220ca/src/js/core/methods/helpers/signtxVerify.js#L56

example solution could look like this:

const deriveBech32Output = (program: Buffer): Buffer => {
    // see https://github.com/bitcoin/bips/blob/master/bip-0173.mediawiki#Segwit_address_format
    // address derivation + test vectors

    // ideally we would also have program version with this, but
    // currently it's fixed to version 0.
    if (program.length !== 32 && program.length !== 20) {
        throw new Error('deriveBech32Output: Unknown size for witness program v0.');
    }

    const scriptSig = Buffer.alloc(program.length + 2);
    scriptSig[0] = 0;
    scriptSig[1] = program.length;
    program.copy(scriptSig, 2);
    return scriptSig;
};

Is there other output type I am missing that accepts p2wsh outputs ?

bucko13 commented 4 years ago

+1 on this issue. I'm getting the same error when trying to sign a tx with a bech32, P2WSH address in the output.

bucko13 commented 4 years ago

Also want to add that this seems to be a regression. The same transaction is able to be signed when using connect v^7, but fails with the Output ${i} scripts differ. error when using v8.

szymonlesisz commented 4 years ago

could you please provide some code example?

bucko13 commented 4 years ago

The issue itself is pretty straightforward in that if you try and sign a tx that pays to a P2WSH output (program length of 32 rather than the P2WPKH length of 20) then the signing will fail with the same error message indicated by OP.

I've put together a test app you can use to verify yourself though in codesandbox: https://codesandbox.io/s/trezor-connect-signing-bug-pdh4e

I can also verify that updating deriveBech32Output with @nodar-chkuaselidze 's fix solves the issue.

szymonlesisz commented 4 years ago

wow, great example :) PR merged, thanks! i've also added some unit testing

CaveRock commented 2 years ago

I am having an issue with using trezor-connect to send a p2wsh multisig change output. I have setup a repeatable example here:

example

I have not been able to find any examples on how to use trezor connect to send a change output to a p2wsh 2of3 change address.

In my example above the change is instead being sent to a pay-to-witness-pubkey-hash for the keypath instead of using the multisig.

My apologies in advance if I am using the incorrect place for this post. Additionally apologies in the instance I have made some elementary error.