bitcoinjs / bitcoinjs-lib Goto Github PK

I've noticed ECDSA signatures are deterministic, as per 30e8b57.

Understandably a few rounds of SHA256 (used as a HMAC) on the input could be a better candidate for k than the output of a poor RNG, but what was the reasoning for this change any how?

For example: https://github.com/bitcoinjs/bitcoinjs-lib/blob/master/src/util.js#L6-L8

For example, bitcoin-ruby allows you to do this:

# use testnet so you don't acidentially your whole money!
Bitcoin.network = :testnet3

Or cryptocoinjs

PackageName.defaultNetwork = 'testnet'

If we can globally set network type then I will be able to start writing some integration tests for bitcoinjs-lib

One should use y.isEven(), y.testBit(0) doesn't work as intended.
Compressed pubkeys should be prefixed with 0x02 if y is even and 0x03 if y is odd.
Found this comparing OpenSSL and bitcoinjs results.

--- ecdsa.js    Sat Apr 07 02:32:15 2012
+++ ecdsa-new.js    Sat Apr 07 02:32:35 2012
@@ -23,7 +23,7 @@
   var enc = integerToBytes(x, 32);

   if (compressed) {
-    if (y.testBit(0)) {
+    if (y.isEven()) {
       enc.unshift(0x02);
     } else {
       enc.unshift(0x03);

BIP38 allows you to create paper wallets with private keys that have been encrypted with a password, which is an awesome way to protect your paper wallet from falling into the wrong hands. It creates a scenario where it would be easier for you to provide a copy of a paper wallet to a friend or trusted person, because that user would have to decrypt the paper wallet's private key using a password in order to spend the funds.

I like this approach more than multisig for my personal use. It's cleaner to implement, simpler to understand and does not require special wallet code to make it work.

It would be great if bitcoinjs-lib's ECKey supported importing/exporting BIP38 encrypted private keys. There is an implementation in bitaddress.org that could be used as a base for this, though it's not implemented directly in ECKey: https://github.com/pointbiz/bitaddress.org/blob/master/src/ninja.key.js#L37

The specification for BIP38 is located here:
https://github.com/bitcoin/bips/blob/master/bip-0038.mediawiki

It does have some drawbacks. It's slower and adds some code complexity because it requires scrypt. But I think it would be an awesome thing to support, if anyone was interested in taking a stab at it.

when i run jake i get a empty file

/**
 * BitcoinJS-lib v0.1.3-default
 * Copyright (c) 2011 BitcoinJS Project
 * 
 * This program is free software; you can redistribute it and/or modify
 * it under the terms of the MIT license.
 */
;
;
;
;
;
;
;
;
;
;
;
;
;
;
;
;
;
;
;
;

js> t = Bitcoin.Transaction.deserialize("0100000001aca7f3b45654c230e0886a57fb988c3044ef5e8f7f39726d305c61d5e818903c0000000000ffffffff0140420f00000000001976a914ae56b4db13554d321c402db3961187aed1bbed5b88ac00000000").serialize()
[1, 0, 0, 0, 1, 172, 167, 243, 180, 86, 84, 194, 48, 224, 136, 106, 87, 251, 152, 140, 48, 68, 239, 94, 143, 127, 57, 114, 109, 48, 92, 97, 213, 232, 24, 144, 60, 0, 0, 0, 0,  53, NaN, NaN, NaN, NaN, NaN, NaN, NaN, NaN, Script, 255, 255, 255, 255, 1, 64, 66, 15, 0, 0, 0, 0, 0, 25, 118, 169, 20, 174, 86, 180, 219, 19, 85, 77, 50, 28, 64, 45, 179, 150, 17, 135, 174, 209, 187, 237, 91, 136, 172, 0, 0, 0, 0]

This is an undesired behaviour, which makes hashes and raw transactions wrong.

We've tested also another constructors, every single one of them makes this sort of array, which is simply wrong, so we can safely assume, that this is a bug in the serialize method.

This bug applies to master branch.

var eckey = new Bitcoin.ECKey(secretKey);
var curve = getSECCurveByName('secp256k1');
var g = curve.getG();
var curvePoint = g.multiply(eckey.priv);

This last line takes about 20 milliseconds on my computer, which is a bottleneck for the product I'm developing. Would love to see if we can make it faster.

We are testing out the wallet component of bitcoinjs-lib, and fell upon a rather concerning error.

Sometimes when generating a new public-private key pair, the eckey.toWif function will returned a malformed private key that does not match its public key.

    var wallet = new Wallet();
wallet.newMasterKey();
pub = wallet.generateAddress();
var priv = wallet.getPrivateKeyForAddress(pub);
priv = priv.toWif(); // This is randomly malformed

In pressure testing, we saw this occur approximately twice every 1000 keys generated. You can test this yourself with this GIST:

https://gist.github.com/moeadham/10217078

Unfortunately, we have not done a root cause analysis to see exactly what is causing the error (we will), but if anyone is using this in production to generate public-private key pairs, I would suggest double checking your Secret key matches the public key.

moe@bitaccess

Currently Transactions addOutput does not support non-addressable outputs.

This means it is currently not possible to pay to the following standard scriptPubKey types:

• pay-to-pubkey
• pay-to-multisig

And naturally non of the non-standard outputs (OP_RETURN etc) are supported.

This appears to be predominately limited by the fact that even a TransactionOut object attempts to obtain an address:

434:    if (this.script.buffer.length > 0) this.address = this.script.getToAddress();

Hi,

I've got an issue I can reproduce with the following code.

<script src="https://raw.github.com/bitcoinjs/bitcoinjs-lib/master/build/bitcoinjs-min.js"></script>
<script>
var hex = '222cb2d3da6e9fa64d36c4580d3bf9fb81a35076c06b6b8f584c0230a00a3872';

var bytes = Crypto.util.hexToBytes(hex);
var btcKey = new Bitcoin.ECKey(bytes);
document.write(btcKey.getBitcoinAddress());
</script>

The output of this is 1BWL9833GQHYGtmvdfLD1Qxvors7LYjzBZ

However if I put the same hex string (222cb2d3da6e9fa64d36c4580d3bf9fb81a35076c06b6b8f584c0230a00a3872) into BitAddress.org http://bitaddress.org I get

175U2wVLLeCEMznpK6jnYyFdhzvmt8L9pf

BitAddress uses the same technique as above and also use BitcoinJS, so why the difference ?

Could we expose the securerandom module, since a lot of projects need the securerandom module so it would be good to have securerandom exported in bitcoinjs-lib so that we don't need to separately pull in a random number generator module

At this time, many of the constructors (or constructing functions) for ECKey, Address, and probably most of the modules, all accept a variety of parameter types, with branching to handle each possibility.

This goes against the separation of concern principles for these functions, and ensures that each function has the responsibility of correctly handling each possible set of inputs; which may be out of order.

A possible solution would be that rather than having catch-all constructor/builder functions, we could better separate the functions into their own responsibilities.

That is, rather than supply the developer with the behemoth: ECKey(hex | base64 | WIF | byteArray | base58) etc, we provide: ECKey.fromWIF, ECKey.fromBuffer and so on.

This allows for simpler verification and testing of these functions also.

I was pondering converting the code base to 2 spaces soft tabs, and doing a few more style cleanups to standardize everything in the same form.

This is super subjective, so feedback is welcome. :)

got this when I ran the tests in Firefox 25.0.1

We fixed DER integer encoding in 07f9d55, however the new encoding is still not correct. Our signed integers use a simple sign-and-magnitude representation whereas DER requires two's complement encoding.

For positive numbers these two encodings are equivalent and since there are no ECDSA signatures with negative r and s, this bug has no impact in practice. However we should fix it for correctness and perhaps refactor DER/ASN1 into its own class so that (correct) DER encoding is available for other use cases as well.

This will require configuring web hook: https://ci.testling.com/guide/quick_start#configuring-the-web-hook cc. @justmoon

Hi all,

I was going to release 0.2.0, but I have decided to postpone for 2-7 days to give people time to deal with heartbleed, and so that we can make sure the #121 fix gets included in the release. The current plan is to skip the 0.2.0 publish to NPM and go straight to 0.2.1. @justmoon believes we should push the latest code (even if it's not perfect) and I agree with him.

I want to make sure that nobody has any pending issues to solve before I do this. I would also like everyone to hold on major breaking PRs until we tag 0.2.1, so we can prevent any issues with the release.

Please let me know if you have any remaining issues that need to be addressed before this happens.

Thank you all!

This only causes issues for values >= 0xfd that are represented using multiple bytes. I encountered it while serializing multisig transactions that has large scriptSig (it works fine with regular transactions, since they usually don't have >= 0xfd values).

I attempted to reverse it myself, but it didn't work well - seems like some parts of the code expects it to return big endian?

Conventions:

The script types are currently:
https://github.com/bitcoinjs/bitcoinjs-lib/blob/master/src/script.js#L117-L133

Bitcoind uses these following names:
https://github.com/bitcoin/bitcoin/blob/master/src/script.cpp#L75-L87

I want to know what your thoughts are on following bitcoind naming conventions, I have a local branch that follow bitcoind and would like to prepare a pull request if you think it is reasonable.

Bug:

https://github.com/bitcoinjs/bitcoinjs-lib/blob/master/src/script.js#L123-L127

It is possible for the third item in a pubkeyhash script to be a OP_0, for example in this transaction:

e.g.:
http://testnet.helloblock.io/transactions/a347b4ef02173b74deb096921d8306ff7c379c254e9febaa040024b220a348ed
One of the output scripts: 76a90088ac

The above is a nonstandard script that reads: OP_DUP OP_HASH160 0 OP_EQUALVERIFY OP_CHECKSIG, I believe this also appears in a number of places on mainnet

I believe we also need:
Array.isArray(this.chunks[2]) && this.chunks[2].length === 20

I also have a local branch, and would like to prepare a pull request and these seem reasonable to you. (I haven't prepared it yet)

Currently there is no documentation nor tests nor examples of how to use a Wallet object to mkSend, how to use processOutput, etc...

// patched by bitaddress.org and Casascius for use with Bitcoin.ECKey
ECPointFp.prototype.getEncoded = function () {
var x = this.getX().toBigInteger();
var y = this.getY().toBigInteger();
var len = 32; // integerToBytes will zero pad if integer is less than 32 bytes.
//32 bytes length is required by the Bitcoin protocol.
var enc = integerToBytes(x, len);
enc.unshift(0x04);
enc = enc.concat(integerToBytes(y, len));
return enc;
};

The original Tom Wu JSBN Elliptic Curve "ECPointFp.prototype.getEncoded" function uses only the length of the X integer when calling "integerToBytes". For the purposes of the Bitcoin.ECKey library this method should pad zero bytes if the X and/or Y integer is less than 32 bytes.

Already exposed by browserify builtin

A lot of the methods in this library used a mixed API, accepting byte arrays, hex strings and other special case strings.

Is there a particular guide line we are following here?
Do we want to more closely mimic formats that are used by the bitcoind RPC API?
Or do we use what is most convenient for cross-development where appropriate? (Byte arrays? Hex? Node Buffers? (using browserify))
Note, if we used Node Buffers, we'd get built-in import/export functions using the Buffers built-in .toString('hex') decoder.

This has been touched in a few issues, so I thought I'd ask it here.

Seems like another regression. With the newest ver from git, I get this:

Bitcoin.convert.bytesToBase64(Bitcoin.Crypto.SHA256("foo", {asBytes: true}))
""

With the one from a week or two ago, it works as it should:

Bitcoin.convert.bytesToBase64(Bitcoin.Crypto.SHA256("foo", {asBytes: true}))
"LCa0a2j/xo/5m0U8HTBBNBNCLXBkg7+g+YpeiGJm564="

If this is legit, this makes me wary of updating to the newest version...as this is a pretty low level breakage (aren't there test suite cases for this??).

Here's the current state:

Istanbul is installed, so you can run this command to get the report on your local machine in the coverage dir as an HTML page:

istanbul cover ./node_modules/.bin/_mocha -- --reporter list test/*.js

The repo owner must enable travis-ci support by enabling the webhook.

Those lines in transaction.js have to be removed or commented out (especially in minified version):

181: console.log(txTmp);
186: console.log("signtx: "+Crypto.util.bytesToHex(buffer));
190: console.log("sha256_1: ", Crypto.util.bytesToHex(hash1));

As the title suggests, testnet key generation is incorrectly supported using the alternative constructor syntax.

This can be seen clearly here (the parameter version is not being carried to the resultant function).

var ECKey = function (input,compressed,version) {
    if (!(this instanceof ECKey)) { return new ECKey(input,compressed); }

After doing a fresh checkout from git of the current master (ac895ca), followed by a "npm install", I try "npm run-script compile" and get the following:

> [email protected] compile /home/user/bitcoinjs-lib
> browserify -s Bitcoin | ./node_modules/.bin/uglifyjs > bitcoinjs-min.js


stream.js:94
      throw er; // Unhandled stream error in pipe.
            ^
standalone only works with a single entry point

Maybe this is something with my environment, but this same thing worked in the past (i.e. 10-15 days ago) on the same box.

Also, note that the travisCI does not test "run-script compile", so everything looks good from it's perspective. If possible, could you add in testing of this command to it so we can catch breakages with it?

• Fix indent
• Remove trailing white space
• Get rid of the over complicated constructor, use static methods

Why not just take all the methods from convert.js and throw them into util.js? convert.js just feels like a utility file. Hopefully this can cut down on requires from other files and it's enough to require util to get any utility function you need, be it convert something or whatever.

I'd be happy to do it if you think it's a good idea :)

I was thinking about using this lib for a scrypt based coin. Would that be possible?

I'm adding version 0.1.3 of this library to cdns (pending for cdnjs, accepted at jsdelivr).

If there was an official browser-ready and minified build for version 0.2.0, I'd like to do the same with that version. Could that (or a future) release be given such a distribution file?

Even better: Could you promise to keep doing that to enable auto-updating cdnjs from this repository?

Try running mocha -R spec test/address.js, and then try running mocha -R spec test, and notice how the address tests run a lot slower.

I have no idea what's going on here..

The 93fe1b4 mass code change did not standardize the jsbn directory in src.
Therefore all the code in that folder is non-standard and not conforming to the code base conventions.

How can I use this lib to work with testnet?

This lib is available via npm, but does not work out of the box.

Does barely anything, relies on jQuery

Candidates
node: crypto.randomBytes http://nodejs.org/docs/v0.6.9/api/crypto.html#randomBytes
browser: window.getRandomValues https://developer.mozilla.org/en-US/docs/Web/API/window.crypto.getRandomValues

#143 replaces this function with createP2SHMultisigScriptSig. The old function is still referenced in transaction.js

We appear to have an implementation of Multisig:

https://github.com/bitcoinjs/bitcoinjs-lib/blob/master/src/script.js#L329
https://github.com/bitcoinjs/bitcoinjs-lib/blob/master/src/script.js#L360
https://github.com/bitcoinjs/bitcoinjs-lib/blob/master/src/transaction.js#L369

However, it is currently not tested, and it's not currently known whether it is in working condition.

Multisig is not really my forte, so I wanted to file this issue and see if anyone was interested in taking a look at multisig and getting it tested. This will go a very long way to getting our Script test coverage to 100% as well.

To sweeten the pot (and to try it out for fun), I'm issuing a 0.03 BTC bounty from the Coinpunk fund for a pull request to improve this!

How do you guys feel about adding a signature validation check to Script.createMultiSigInputScript(..) for each signature? Since its already checking the number of signatures required I feel like it should make sure the signatures are valid as well. Also, the ability to sign a partially signed multisig input script.

Or should these details be left up to the developer?

Is this project still maintained? If not would it be possible to give some other people access. There are some active developers around...

This branch seems to be in master now since a long time ago. Can we get rid of it?

As reported by @nestorsilk here: https://github.com/bitcoinjs/node-bitcoin-p2p/issues/60

For new users of BitcoinJS it is often hard to distinguish server from client-side errors. This should be fixable by adding appropriate features to the error reporting and logging system.

Upgrading from the original version of bitcoinjs-lib. There, once could call rng_get_bytes directly. I really like this version's cleanup of the namespace, however I can't find a way to get at an instance of SecureRandom (e.g. rng), or the SecureRandom object itself. It's clear from index.js that this module is not exported off of Bitcoin. Could this be changed, or is there something I'm missing?

Bitcoinjs is the standard client side bitcoin library.

To support a mature development ecosystem in bitcoin, developers need a drop in replacement for testnet addresses creation and transaction signing.

An alternative to a separate branch would be a clear supported convention in switching back and forth.