bitcoinjs / bitcoinjs-lib

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

keypair getAddress throws error in newer versions #1420

Closed coinables closed 5 years ago

coinables commented 5 years ago

I have a bitcoinjs-lib install from May 2018 and the following code executes as expected:

> var bitcoin = require("bitcoinjs-lib");
undefined
> var network = {network: bitcoin.networks.testnet}
undefined
> var keyPair = bitcoin.ECPair.makeRandom(network)
undefined
> keyPair.getAddress();
'mm8DcVv8rXR3aLD3EYFuruoA3anrsgrzN8'

Now when I try the same thing with a fresh install of the library I receive an error that getAddress is not a function. Is there a substitute now? Same with getPublicKeyBuffer when trying to get the pubkey from a keypair.

> var bitcoin = require("bitcoinjs-lib");
undefined
> var network = {network: bitcoin.networks.testnet}
undefined
> var keyPair = bitcoin.ECPair.makeRandom(network)
undefined
> keyPair.getAddress();
TypeError: keyPair.getAddress is not a function
    at repl:1:9
    at ContextifyScript.Script.runInThisContext (vm.js:50:33)
    at REPLServer.defaultEval (repl.js:240:29)
    at bound (domain.js:301:14)
    at REPLServer.runBound [as eval] (domain.js:314:12)
    at REPLServer.onLine (repl.js:442:10)
    at emitOne (events.js:121:20)
    at REPLServer.emit (events.js:211:7)
    at REPLServer.Interface._onLine (readline.js:282:10)
    at REPLServer.Interface._line (readline.js:631:8)
coinables commented 5 years ago

Update, I did a fresh install but specified version 3.3.2 during npm install and getAddress & getPublicKeyBuffer works again.

junderw commented 5 years ago

Read the CHANGELOG

And / or read the tests.

https://github.com/bitcoinjs/bitcoinjs-lib/blob/b3def6b4006683190657ef40efa7a8bcbb78b5cd/test/integration/transactions.js#L46-L74

As you can see: ECPair having methods for addresses (something that has nothing to do with EC at all) is less logical than having a separate payments API.

bitcoin.payments.p2pkh({ pubkey: alice1.publicKey, network: regtest })

So this will return an object that lazy-loads attributes such as:

output (a Buffer of the scriptPubkey)

address (a string of the address)

hash (a Buffer of the public key hash etc.)

ECPair public key is now a getter, so keyPair.publicKey will give you a 33 or 65 byte buffer (depending on the compressed attribute, default compressed.)

Also, keyPair.privateKey will give you a 32 byte Buffer of the private key. keyPair.d BigInt no longer exists.

coinables commented 5 years ago

Thank you

ghuang123 commented 4 years ago

@coinables revert back to version 3.3.2 may not be the best solution to this problem. Some new functionality such as P2SH is not avaliable in 3.3.2. If you need to use new functionality, you will still need to upgrade.

@junderw I personally believe that getAddress API should be a first class API in bitcoinjs as it is very widely used in any kind of wallet. It would be better to make it avaliable in the payment class (since as you rightly said that address is more related to payment class), something like the following:

getAddress ( pubkey: ecKey.publicKey, network: regtest }) {

return bitcoin.payments.p2pkh({ pubkey: ecKey.publicKey, network: regtest }).address }

junderw commented 4 years ago

P2SH is not avaliable in 3.3.2

P2SH is available in v3. It's just a little hard to understand how to do it. There are test examples in the tests for that version.

getAddress API should be a first class API

This is a bad idea. By offering getAddress we are telling developers "this is the address you should use". It is better to give them the Payments API and then they can ask themselves "which payment do I want to use?"...

The function getAddress only serves to simplify something by removing choice from the developer. A new developer will see the simpler option and most will choose it. Leading everyone to use P2PKH and scratch their heads when someone asks "Why don't you use segwit?" and they think "what is segwit? I don't know, never had to think about it." whereas if they had to choose their payment from the beginning, They would know and be able to make the design decision for their app on which payment type to use.

junderw commented 4 years ago

v3 was made before segwit, and before P2SH was very popular... so assuming getAddress means "p2pkh" was the norm at that time.

ghuang123 commented 4 years ago

Very well explained. Agree! Thanks!