Bitcoinjs-lib: Typescript TransactionBuilder.addOutput() should probably use String instead of Buffer for the address param

[youssefgh]

I have been facing some weird TX when trying to experiment in the Testnet see : https://test-insight.bitpay.com/tx/e5da2b1d3982c03b156d0159de2505161a37cce96cba642816550c0783c6abb0
When debugging i found that in your JS example that you pass the address to addOutput as a string :
tx.addOutput('1cMh228HTCiwS8ZsaakH8A8wze1JR5ZsP', 12000)
and in Typescript i had to do :
tx.addOutput(new Buffer('1cMh228HTCiwS8ZsaakH8A8wze1JR5ZsP'), 12000)
Am wondering if this difference is normal ?!
Your library is awesome ! good work ;)

[dcousens]

@youssefgh careful!

You are using Transaction raw, not TransactionBuilder, a Transaction in its raw form only accepts scripts of type Buffer, NOT addresses.

The above code you presented would lead to fund loss.

[dcousens]

Please see https://github.com/bitcoinjs/bitcoinjs-lib/pull/926.
Sorry about the confusion.

[dcousens]

@afk11 @fanatid @dabura667 I had some thoughts a while ago about s/Transaction/RawTransaction and s/TransactionBuilder/Transaction... should we do this for 4.0.0?
Only rarely do use cases involve purely the data-structure variant.

And additionally, I'd specifically want to have RawTransaction to be an encode/decode structure.

[youssefgh]

Think you for the fast reply !
I was actually using TransactionBuilder, and i don't have any idea about RawTransaction since it is not referenced in the index.d.ts
I found that types/bitcoinjs-lib: "^3.0.2" doesn't support string as a param and the issue seems to be fixed later in : https://github.com/DefinitelyTyped/DefinitelyTyped/commit/1eba1a5d753fb3f0ea47b2e4cbfc00fe1b221d34#diff-72285337b1f2c18b939de86defc6f63c
I will try this update typing to the latest version, i'll test this in the testnet and i'll post my result

[dabura667]

I would refrain from renaming into an old name.

ie. if we make bitcoinjslib.TransactionBuilder into bitcoinjslib.Transaction... people who might have been using bitcoinjslib.Transaction for something assuming it is bitcoinjslib.RawTransaction would be confused when re-writing for the new 4.0.0 codebase.

I would say s/Transaction/RawTransaction and s/TransactionBuilder/BuildTransaction

[youssefgh]

Updating types/bitcoinjs-lib to 3.0.5 added the support for string param and though resolved my problem
But still can't create proper P2PKH transactions using :
var transactionBuilder = new TransactionBuilder();
transactionBuilder.addInput(utxo.txid, utxo.vout);
transactionBuilder.addOutput(output.destination, 100);
Or even replacing TransactionBuilder with Transaction
The result is always transactions that have outputs that doesn't contain P2PKH locking script in them
I think this is out of the scoop of this ticket, so feel free to close it and i'll move my issue to stackoverflow
Thanks

[youssefgh]

Never mind i had to change :
var transactionBuilder = new TransactionBuilder();
to
var transactionBuilder = new TransactionBuilder(networks.testnet);
Thank you very much !
Feel free to close