search

bitcoinjs / bitcoinjs-lib

A javascript Bitcoin library for node.js and browsers.
MIT License
5.67k stars 2.1k forks source link

Can it be better - "non-segwit inputs now require passing the whole previous tx as Buffer" ? #1560

Closed Overtorment closed 4 years ago

Overtorment commented 4 years ago

I'm refactoring my BIP44 implementation to create transactions through PSBT and stumbled upon this. Is that the only way to add non-segwit inputs? That would require for me to do another network request to fetch txhex by txid, and it feels like we can do better than that - we already know txid, vout, amount and locking script of the input .

Im staring at test/integration/transactions.spec.ts:27 and it looks like this transaction hex is actually crafted on the fly, which is kinda what I want but feels ugly.

junderw commented 4 years ago

Sorry, this is what PSBT requires.

If we remove that requirement we are not PSBT, we are a new thing...

The reason why it is needed is to guarantee the input value.

Segwit signs the input value. So a wallet doesn't need to check or trust anything else, all it needs to do is sign the PSBT and it is guaranteed that the value of the input is the value it signed.

Non-segwit inputs only sign the input value VIA the txid of the previous transaction. So in order for the same "you only need the PSBT to guarantee the data you sign" you must verify the txid hash is accurate (by hashing the full tx by yourself from the PSBT) and then you must verify the value of the output in that transaction. Because you sign the txid which is a hash that contains the value, and you verified the value there is no way you can be tricked.

Is it possible to skip this check? Yes. But you will need to fork bitcoinjs-lib to remove the check AND you must NEVER export the PSBT string, since you will just confuse users when any other wallet gives them errors saying "WTF why doesn't your nonsegwit input have nonSegwitUtxo full tx? This is invalid, ERROR ERROR!"

junderw commented 4 years ago

In your wallet's case, you are trusting the server heavily.

That is a security design that you have chosen. But that PSBT doesn't allow.

My only advice would be to change the server to return full transactions with any non-segwit utxo unspent info.

Overtorment commented 4 years ago

Thanks for the explanation! For now I made a workaround by fetching and providing complete txhexes, but Im thinking about actually forking and patching bitcoinjs for this specific case - to skip checks. My BIP44 implementation is considered legacy and is not going to export PSBT, at least I don't plan that.

junderw commented 4 years ago

It should be an easy fix. We allow segwit inputs to sign with nonWitnessUtxo, so I think all you need to do is change a few lines to allow non-segwit to sign with witnessUtxo