spesmilo / electrum

Electrum Bitcoin Wallet
https://electrum.org
MIT License
7.44k stars 3.09k forks source link

Segwit message signing is not compatible with other software #3861

Closed SomberNight closed 2 years ago

SomberNight commented 6 years ago

AFAICT there is no clear consensus how to sign messages with segwit addresses (txin type of p2wpkh-p2sh and p2wpkh)

We do one thing, and Trezor does something else. Bitcoin Core so far has this disabled. Maybe we could transition to what Trezor does, in a way that:


Electrum issue/commit

Bitcoin Core issue

Trezor issue/commit/tests

gribble PR doing what Electrum does

ecdsa commented 6 years ago

I don't mind following what trezor does. The only drawback is that we might have to change it again if Core introduces a new method

matejcik commented 5 years ago

at the moment Trezor-generated signatures will fail to verify in Electrum because how from_signature65 decodes it. That's probably worth fixing, without touching the other parts.

Giszmo commented 5 years ago

Is there any consensus yet on test vectors?

SomberNight commented 5 years ago

There have been two related new BIPs submitted in recent months:

Do we know of anyone else that is compatible with Trezor? If so, maybe we should do that too to make some progress... :)

brianddk commented 5 years ago

Perhaps on verification, you can attempt the process on both forms of the signature header-byte. It is trivial to to determine if the address presented to verify against is P2PKH, P2SH, or P2WPKH.

If your verifying against a P2WPKH address with a signature header byte between 39-42, just subtract 8 from the header byte and do the verification as it exists today. If you have a P2SH address with a signature header byte between 35 and 28, just subtract 4 from the header byte and do the verification.

This would make your message verification both BIP-0137 complaint and backward compatible with messages already generated. Perhaps ask for similar flexibility of the Trezor FW then both BIP-0137 and non-BIP-0137 signatures can be interchangeable regardless of what standard is used in their generation.

SomberNight commented 5 years ago

Regarding BIP-0322, to me it looks really scary. I am not sure how we would implement that. It seems to properly implement it, we would need a full fledged consensus validator and a script interpreter. If you read the BIP, to me, it seems evident that it was written for Bitcoin Core.

Consider this quote for example:

Verify Script with flags=consensus flags (currently P2SH, DERSIG, NULLDUMMY, CLTV, CSV, WITNESS), scriptSig=script sig, scriptPubKey=scriptPubKey, witness=witness, and sighash=sighash

or this section: https://github.com/bitcoin/bips/blob/master/bip-0322.mediawiki#consensus-and-standard-flags

How are light clients supposed to implement this?

I guess we could conceivably implement signing support via hardcoded templates for the scripts we support. About validation... well not sure; but I expect we would return INCONCLUSIVE for anything we don't recognise.

pinheadmz commented 4 years ago

With the recent merge of https://github.com/bcoin-org/bcoin/pull/802 into bcoin...

bcoin now supports Bitcoin Message Signing with bech32 addresses!

bc1q5gcvs0hq7hrtm5tt2ra3fq4jtt46f5n4t4zfn7

H+n6A3V2asu5fZ5nifIlleZvdJXPc+MiKghDgBjFPt6DcePVFEc6ZOxlhiF87oU/WxTj2uMX41uuPsFFP3qocJw=