bitcoinjs / bitcoinjs-lib

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

TransactionBuilder.buildIncomplete() loses the P2WPKH prevOutScript information #1315

Closed calvinlauyh closed 5 years ago

calvinlauyh commented 5 years ago

I have a scenario where there is a program A that will generate a transaction of multiple P2WPKH inputs and distribute to many different person, each possessing another program B to sign the transaction, once signed, then pass the partly signed tx to next one until everyone is signed.

Here is the pseudo code:

Program A

const txBuilder = new bitcoin.TransactionBuilder();
txBuilder.addInput(txHash1, 0, null, aliceP2wpkh.output);
txBuilder.addInput(txHash2, 5, null, bobP2wpkh.output);
txBuilder.addInput(txHash3, 2, null, tomP2wpkh.output);
txBuilder.addOutput(peterAddress, 10000000);

const incompleteTxHex = txBuilder.buildIncomplete().toHex();
// This incomplete transaction hex will be distributed to Alice, Bob and Tom for sign

Program B

const tx = bitcoin.Transaction.fromHex(incompleteTxHex);
const txBuilder = bitcoin.TransactionBuilder.fromTransaction(tx, network);
txBuilder.sign(inputIndex, keyPair, null, null, satoshiAmount.toNumber());

const partlySignedTxHex = txBuilder.buildIncomplete().toHex();
// This partly signed transaction hex will be distributed to the next one to sign, until everyone sign it

However, this does not work and the bitcoin-cli will return non-mandatory-script-verify-flag (Witness requires empty scriptSig) (code 64) when trying to send it.

I have changed the program to addInput and sign in the same program and it can be broadcasted.

After some investigation, I realize when I do txBuild.buildIncomplete() it seems to lose the prevOutScript of the previous P2PKH output. This make sense because it is not supposed to be inside the transaction hex. But I am wondering how I can let the client (program B) "patch" those information back to the inputs before signing?

Thanks

junderw commented 5 years ago

this is a compound problem:

  1. p2wpkh uses p2pkh for its prevOutScript
  2. raw Transactions are limited in the info we can store.

for other formats, TransactionBuilder is able to accurately guess the prevoutscript.

there is no way to fix this.

So you must hard code your program B to sign as p2wpkh explicitly.

Or

Wait until we merge BIP174 support

calvinlauyh commented 5 years ago

Hi @junderw, may I know a bit more of the first solution you suggested? What do you mean by hard code the program B to sign as p2wpkh explictly? Do you mean to call addInput inside program B and sign it so that witness data are kept inside the raw transaction hex?

junderw commented 5 years ago

This is extremely hackey... and difficult... but it passes and verifies.

https://github.com/junderw/bitcoinjs-lib/commit/c22564801644f3f101e4fdc8bf60ea062575fb5a

Let me know if you have any questions about it.

calvinlauyh commented 5 years ago

Sorry, there has been some time before I get back to my project.

I have tried with the above code snippet and it works well. Thanks for you help.

Just by skimming the methods, the snippet seems to be using the public interface exposed by the class. Do you foresee it will be easily broken when this library further develops and enhances in features?

Sorry I know it is hard to predict code changes, but I am trying to get more sense how reliable I can use the above used functions in my project. Thank you. @junderw

junderw commented 5 years ago

setWitness and hashFor are vital methods and will not likely change in the future, hashFor will likely just add new functions like hashForWitnessV1 when v1 comes out, etc.