Remove/deprecate classify*

[dcousens]

They are ambiguous, and the result is therefore dependent on the internal order of checks made.

There is too much intrinsic context required that the function parameters would become a mess if we attempted to capture it.

We should remove them and leave people to decide on what suits them.

[dcousens]

See https://github.com/bitcoinjs/bitcoinjs-lib/pull/898

[afk11]

have mixed thoughts on this, could be good to discuss!

I like having a way to classify the standard script types (script-hash types, and the simple common scripts p2pk p2pkh multisig).

classifying on the bases of what's in the scriptSig should die and never return :) a substitute is just to classify the txOut's script, and if P2SH, P2WSH, or P2SH|P2WSH is indicated only then peek at the input script/witness for a script to compare against (& since it's all script-hash commitments it's easy to check if they're wrong before checking if multisig/etc)

So I would say classifyOutput is quite safe once the user is dealing with standard enough scripts, but classifyInput isn't robust against future soft-forks that look like P2SH and so on.

[dcousens]

@afk11 a great point, but, I wonder if classifyOutput will be as necessary if we have a stronger address/script abstraction?

[junderw]

classifyOutput is great.

Perhaps add an optional arg to classifyOutput that accepts the input/witness data.

if they pass in a P2SH output and no input/witness = P2SH if they pass in P2SH and input info = Multisig (P2SH) if they pass in P2SH and witness info = Multisig (P2WSH) if they pass in P2SH and input and witness info = Multisig (P2WSH) (P2SH)

etc. etc.

Seems a little complicated though.

[afk11]

@junderw: it's interesting that classifying P2SH output script alone gives you something with the address, but giving it each further detail could make it more 'complete'..

This kind of classification is being used in places in the transaction builder, in buildInput https://github.com/bitcoinjs/bitcoinjs-lib/blob/master/src/transaction_builder.js#L409 and I know if we started requiring the txOut in there, it would make prepareInput a much nicer piece of code:

https://github.com/bitcoinjs/bitcoinjs-lib/blob/master/src/transaction_builder.js#L274 all those if branches would be known as part of this classification..

@dcousens fair point. I know I was keener for output scripts only, but even input scripts/witness stacks would be pretty cool to build if we did deal with this in the right way. cus if we had a completely classified [P2SH|P2WSH|multisig], and we know that the innermost script is some standard type, and not nonstandard, we would say that buildStack works with it too.. buildInput is very generic, and takes care of the representing a stack for a certain representation (whether it's p2sh and/or p2wsh)

It would be interesting to start fleshing out an API to deal with this, since as there's definitely a few parts I can see that are somewhat connected to templates.. making script-hash scripts/addresses/witness programs from a bare script; building/extracting stacks for known types (P2PKH/P2PK/multisig) with the script-hash addons (redeemscript in sig, witnessscript in stack); classifying scripts to check them (p2sh -> p2wsh -> multisig hash checks, etc)

[junderw]

@afk11 Also, down the road with MAST I think inputs will require output data in some sort of format that will be conducive to placing in the MAST script portions and merkle nodes.

I tend to agree with requiring locking scripts (scriptPubkey, redeemScript, or witnessScript) for input classification.

[afk11]

@junderw: Yes, it does seem be the place for it.. To completely classify a script which was a BIP114 bare witness output (scriptPubKey only) maybe you'd have to provide a partial merkle tree & node, or the full set of scripts.

This is exactly the data the signer would need as well (in addition to other redeemScripts/witnessScript), but normally I'd just expect the signer to know what to provide.. it would be nice to be able to produce it from raw scripts (first time address generation), or reproduce it from the scriptPubKey (the reverse)

Would be interesting to put the code for converting between forms here too. Script w/ all mast scripts w/ and chosen branch.. or reduce the information so only a single branch could be redeemed.. etc

[dcousens]

Our classify* function could attempt to parse each script using the new functions defined in the https://github.com/bitcoinjs/playground, and seeing if any attempts are successful.

It will always be order dependent. It will never be 100%.

This is simply the state of affairs with classification.

[dcousens]

Will review/reopen if necessary in a few months post 4.0.0.