Bitcoinjs-lib: Should P2PKH still be the default `getAddress`?

[dcousens]

Following on from discussion in https://github.com/bitcoinjs/bitcoinjs-lib/issues/779.

Is getAddress a sane assumption now that P2SH is highly prevalent, and SegWit introduces P2WPKH and P2WSH ... and bech32 soon...

For now, I vote we deprecated getAddress asap, and remove it in 4.0.0... thoughts?
What do we promote as the idiomatic flow for getting addresses?
How we do we even start that conversation...

[afk11]

Yea it's probably a good idea to make people think twice about it.

There's a similar situation if we exposed 'var scripthash = script.getScriptHash()' or something. The current route is to make use of the primitives directly (mostly because scripts are buffers I guess). Maybe a script.getHash160() and script.getSha256() for P2SH and P2WSH respectively?

ECPair could expose getHash160() ?

• baddress.p2pkh

  • .fromHash()
  • .fromKey()
  • .fromOutputScript()

• baddress.p2wpkh

  • .fromHash()
  • .fromKey()
  • .fromOutputScript()

• baddress.p2sh

  • .fromHash()
  • .fromRedeemScript()
  • .fromOutputScript()

• baddress.p2wsh

  • .fromHash()
  • .fromWitnessScript()
  • .fromOutputScript()

maybe setting us up for:

• baddress.fromOutputScript()
• baddress.toOutputScript()

Random thoughts:

• If we encourage the fromKey/fromScript when people generate addresses, it's a chance to enforce (meaning: reject) certain things before an address is produced. For example, segwit requires compressed keys everywhere - something we can enforce in baddress.p2wpkh.fromKey & baddress.p2wsh.fromWitnessScript (or maybe a more ideal place)

• fromKey/fromScript will require hashing, so after the initial generation fromHash/fromOutputScript might be preferred (hmm, if ECPair is immutable, maybe we can cache the hash160/sha256 as they're generated? something similar for script's?)

• something that's often handy is the ability to go from address instance to scriptPubKey..

[dcousens]

bitcoin.address.P2WPKH.fromKeyPair(keyPair) ... could work.

Will read again tomorrow, but good ideas!

[dcousens]

And it would/could be consistent with our script template interface.

[dcousens]

How does a bech32 address fit into this?

[afk11]

I guess address classes need to indicate what serialization they are, since to date we've only had base58? Hopefully not too messy..

[dcousens]

I think your proposal is solid, with fromKey being fromKeyPair.

@afk11 I don't think the serialization classes will be an issue, it'll simply be baddress.P2PKH.toString/fromString functions. (or encode/decode?)

baddress.fromBase58Check will be Base58Check only.
baddress.fromString will allow anything.
baddress.fromBech32 will be Bech32 only.

How do we deal with the "QR code" optimized address schemes (aka, uppercase bech32)?

toQRString?

edit: maybe encode/decode?, then it could be abstract-encoding compliant-ish?

[dabura667]

Everything sounds great so far...

I would like the prototypes on the ECPair as an option though.

[dabura667]

.toP2shP2wpkh() .toP2wpkh()

[dcousens]

@dabura667 I don't think the ECPair options are necessary if the bitcoin.address templates offer bitcoin.address.*.fromKeyPair.
It it short and sweet and non-ambiguous.

I think we could remove .getAddress.

[dcousens]

I think this [breaking] change should align with a renaming of bitcoin.script.* templates to be identical.

I personally prefer p2wpkh over witnessPubKeyHash, but, what do others think?

[afk11]

p2wpkh is nicer, shorter, though notice it's a break for script.*, but not the worst thing if this goes into a major release.

not sure if changing the default is a good idea, but could gradually remove support for 'key.getAddress()' over time so people are forced to think about it?

[dcousens]

@afk11 I think the general consensus is to phase out P2PKH as soon as possible.

This requires a major bump anyway, lets do both now [in 4.0.0]. Keeping .getAddress() is only serving to confuse IMHO.

[dcousens]

Consensus is remove .getAddress with new streamline/compact interfaces that support all script types.

https://github.com/bitcoinjs/bitcoinjs-lib/pull/854 will serve as in the reminder/issue in place until the new API is ready.