code audit

[fanatid]

Need another eyes for code check. Any feedback appreciated.

[dcousens]

AFAIK, an address is a human-readable easily shared equivalent of the original script. Assigning an address to a pay-to-pubkey and raw-multisig script is actually strongly discouraged since it can't be readily mapped to its transaction out without re-looking up the information (making the address useless).

Is there any reason you provided the derived addresses in https://github.com/fanatid/script2addresses/blob/master/src/index.js#L124-L175?

[fanatid]

The main reason why I wrote this module was extracting all available addresses from output scripts for chromanode which is blockexplorer bitcore-node has same behavior for pay-to-pubkey

[dcousens]

Sure, some block explorers and libraries have done it, but that doesn't mean the behaviour makes sense.

[dcousens]

Remembering, there is no such thing as a "multisig" address, or a "pubkey" address. There are only [currently] two address types, P2PKH, and P2SH. IMHO, alluding to anything else is just confusing for users and developers alike.

[fanatid]

But what you think block explorer should display instead derived addresses from pay-to-pubkey and raw-multisig? raw script?

[dcousens]

@fanatid indeed. The only meaningful purpose of showing the "addresses" of those scripts is to misinform users such that they use the P2SH/P2PKH variants instead, forcing anyone who wants to spend those funds to also use the P2SH/P2PKH variants.

If that is your intention, then by all means, but, it should be an intentional illusion on part of an application, and not something done by "default" in a library.

[dcousens]

Imagine the scenario where a user visits a pay-to-pubkey transaction out on a block explorer, and the P2PKH QR code is displayed. The user, whom was given this link by a friend, assumes, "I'll send him a mBTC", scans the QR, and sends it. Now, presumably unless the friend has a wallet that is smart enough to listen to both his pay-to-pubkey and the equivalent P2PKH, he will likely never know he has been given that mBTC. It may sound unlikely, but posit the same situation with a raw-multisig, and the situation sounds a lot more plausible.

[fanatid]

OK. I think, that I agree with you. Any ideas how interface should look? BTW, I think that package should be able convert input scripts (multisig in p2sh) to pubkeys, what do you think?

[dcousens]

input scripts (multisig in p2sh) to pubkeys, what do you think?

Do you mean

bitcoin.script.isMultisigOutput(redeemScript) && 
bitcoin.script.decompile(redeemScript).chunks.slice(1, -2).map(ECPubKey.fromBuffer)

?

[dcousens]

To get that P2SH, maybe we should make an easy way to do it in the core library: bitcoin.script.decodeRedeemScript => (redeemScriptSig, redeemScriptPubKey)?

Then its just the 1 liner above for users.

[fanatid]

input scripts (multisig in p2sh) to pubkeys, what do you think?

Do you mean

bitcoin.script.isMultisigOutput(redeemScript) && 
bitcoin.script.decompile(redeemScript).chunks.slice(1, -2).map(ECPubKey.fromBuffer)

?

Yep, but instead redeemScript shoule be redeemScriptPubKey and function should return buffers instead bitcoinjs-lib public keys.

Under core library you mean bitcoinjs-lib? script2addresses doesn't uses any bitcoin library

In my mind library should give next info about script:

• input or output?
• script type: p2pk, p2pkh, p2sh, multisig, data, unknow
• if script type is p2sh, what p2sh: p2pk, p2pkh, multisig, unknow
• p2pkh, p2sh hashes/addresses if available
• public keys if available

All of this out from script2addresses context, don't you want move bitcoinjs-lib/script to separate package with this features?

[dcousens]

don't you want move bitcoinjs-lib/script to separate package with this features?

Absolutely, but by public keys do you mean EC ready or just their equivalent Buffer?

[fanatid]

I think it should be a buffer https://github.com/cryptocoinjs/secp256k1-node also use buffers