Skip to content

Conversation

@dcousens
Copy link
Contributor

Most of this has been previously vetted, but this is the [almost] all inclusive 2.0.0 PR.

@weilu did we want to split it up into smaller PR's for simpler review over the next few days?

@dcousens dcousens added this to the 2.0.0 milestone Nov 26, 2014
@dcousens
Copy link
Contributor Author

I could probably squash the ECPair commits given the API has not existed previously to this.

@dcousens
Copy link
Contributor Author

Squashed into 0f86931 and 8b1d135, addition then replacement of ECKey/ECPubKey commits.

@weilu
Copy link
Contributor

weilu commented Nov 27, 2014

A single PR is fine. What's up with Travis CI?

@dcousens
Copy link
Contributor Author

@weilu sinon seems to be broken in 0.11

@dcousens dcousens force-pushed the 2.0.0 branch 2 times, most recently from c2132e6 to 5e62b61 Compare November 27, 2014 23:58
@dcousens
Copy link
Contributor Author

Rebased on HEAD, squashed Transaction test and ECPair integration test fixes.

@dcousens
Copy link
Contributor Author

@weilu reference sinonjs/sinon#613

Added a commit to ignore node 0.11 until sinon is fixed upstream

@dcousens
Copy link
Contributor Author

Rebased on HEAD

@dcousens
Copy link
Contributor Author

Fixed block.js tests to use RawTransaction in 967d570.
Added consistent import/export hashType checking and corresponding tests into ECSignature

@dcousens dcousens force-pushed the 2.0.0 branch 2 times, most recently from f1e8442 to 2746027 Compare November 28, 2014 03:05
@coveralls
Copy link

Coverage Status

Coverage increased (+0.83%) when pulling 2746027 on 2.0.0 into 110cb86 on master.

@dcousens dcousens force-pushed the 2.0.0 branch 2 times, most recently from 093e8f8 to c536926 Compare November 29, 2014 01:37
@dcousens
Copy link
Contributor Author

Will add a commit to ignore node 0.11 if sinon isn't fixed before we want to merge merge this.

@dcousens
Copy link
Contributor Author

dcousens commented Dec 1, 2014

Rebased on #323

@coveralls
Copy link

Coverage Status

Coverage increased (+0.98%) when pulling 25c2c42 on 2.0.0 into b1cb56c on master.

@weilu
Copy link
Contributor

weilu commented Dec 7, 2014

var bitcoin = require("bitcoinjs-lib")

Backward incompatible changes:

removed

  • bitcoin.crypto.HmacSHA256 // use require('crypto').createHmac instead
  • bitcoin.crypto.HmacSHA512 // use require('crypto').createHmac instead
  • bitcoin.base58check // use require("bs58check") instead
  • bitcoin.Wallet // build your own
  • bitcoin.HDNode.fromBuffer // use `bitcoin.HDNode.fromBase58 instead
  • bitcoin.HDNode.fromHex // use `bitcoin.HDNode.fromBase58 instead
  • bitcoin.HDNode.toBuffer // use `bitcoin.HDNode.toBase58 instead
  • bitcoin.HDNode.toHex // use `bitcoin.HDNode.toBase58 instead
  • bitcoin.Transaction.prototype.sign // use bitcoin.TransactionBuilder.prototype.sign instead
  • bitcoin.Transaction.prototype.signInput // use bitcoin.TransactionBuilder.prototype.sign instead
  • bitcoin.Transaction.prototype.validateInput // ?? roll your own?
  • bitcoin.ECKey // use bitcoin.ECPair instead
  • bitcoin.ECPubKey // use bitcoin.ECPair instead

updated

  • bitcoin.Message -> bitcoin.message
  • bitcoin.Transaction -> bitcoin.RawTransaction
  • bitcoin.TransactionBuilder -> bitcoin.Transaction
  • bitcoin.HDNode.toBase58(isPrivate) -> bitcoin.HDNode.prototype.neutered().toBase58() for exporting a public key and bitcoin.HDNode.prototype.toBase58() for exporting a private key
  • bitcoin.scripts.dataOutput -> bitcoin.nullDataOutput
  • Transaction.prototype.hashForSignature(prevOutScript, inIndex, hashType) -> Transaction.prototype.hashForSignature(inIndex, prevOutScript, hashType)
  • Transaction.prototype.addInput(hash, ...): hash could be a string, Transaction or Buffer -> hash can only be a Buffer. For TransactionBuilder.prototype.addInput(prevTx, ...), prevTx can still be a string, Transaction or Buffer.
  • Transaction.prototype.addOutput(scriptPubKey, ...): scriptPubKey could be a string, Address or Buffer -> scriptPubKey can only be a Buffer. For TransactionBuilder.prototype.addOutput(scriptPubKey, ...), scriptPubKey can be a string, Address or Buffer.

Writing up the change notes makes me think that the bitcoin.Transaction -> bitcoin.RawTransaction, bitcoin.TransactionBuilder -> bitcoin.Transaction change should come in a separate major bump. It can be really confusing for people otherwise. @dcousens thoughts?

@dcousens
Copy link
Contributor Author

dcousens commented Dec 8, 2014

bitcoin.Transaction.prototype.validateInput
This is as simple as using hashForSignature and ECPair.prototype.validate. What could be added is the hashType to use the existing hashType modifier if it already exists at that vout.

How does a separate major version bump help?

@dcousens
Copy link
Contributor Author

dcousens commented Feb 5, 2015

Rebased on #329

@dcousens
Copy link
Contributor Author

dcousens commented Mar 2, 2015

I've rebased on master.

This should be good to merge quite quickly, as it is almost all just removals, with the addition of bufferutils.equal and the movement of parameter type coercion to TransactionBuilder specifically.

I also removed ECPair, as will put it in another PR for review.

@dcousens dcousens changed the title 2.0.0 pre-2.0.0, deprecations Mar 2, 2015
@dcousens
Copy link
Contributor Author

dcousens commented Mar 2, 2015

Will merge before end of tonight, post-merge review is welcome if you miss the merge, but most of this has already been reviewed several times, so it should just be a final check over.

@dcousens dcousens self-assigned this Mar 2, 2015
@dcousens dcousens force-pushed the 2.0.0 branch 2 times, most recently from 2d0ca4e to cb3c291 Compare March 2, 2015 07:06
@coveralls
Copy link

Coverage Status

Coverage increased (+1.04%) to 98.82% when pulling 77f81cf on 2.0.0 into 0bba215 on master.

@weilu
Copy link
Contributor

weilu commented Mar 2, 2015

I'll review this again today.

On Mon, Mar 2, 2015 at 3:52 PM, Daniel Cousens notifications@github.com
wrote:

I've rebased on master.

This should be good to merge quite quickly, as it is almost all just
removals, with the addition of bufferutils.equal and the movement of
parameter type coercion to TransactionBuilder specifically.

Removed ECPair (will put it in another PR for review).


Reply to this email directly or view it on GitHub
#317 (comment)
.

@dcousens
Copy link
Contributor Author

dcousens commented Mar 2, 2015

Sounds good. Will freeze as of now.

edit: Was just fixing a test accidentally broken in the commit before 4668cb1. Good to go now.

@coveralls
Copy link

Coverage Status

Coverage increased (+1.04%) to 98.82% when pulling 4668cb1 on 2.0.0 into 0bba215 on master.

@dcousens
Copy link
Contributor Author

dcousens commented Mar 2, 2015

Change log as of this PR.


Backward incompatible changes:

removed

  • bitcoin.crypto.HmacSHA256 // use require('crypto').createHmac instead
  • bitcoin.crypto.HmacSHA512 // use require('crypto').createHmac instead
  • bitcoin.base58check // use require("bs58check") instead
  • bitcoin.Wallet // build your own
  • bitcoin.HDNode.fromBuffer // use `bitcoin.HDNode.fromBase58 instead
  • bitcoin.HDNode.fromHex // use `bitcoin.HDNode.fromBase58 instead
  • bitcoin.HDNode.toBuffer // use `bitcoin.HDNode.toBase58 instead
  • bitcoin.HDNode.toHex // use `bitcoin.HDNode.toBase58 instead
  • bitcoin.Transaction.prototype.sign // use bitcoin.TransactionBuilder.prototype.sign instead
  • bitcoin.Transaction.prototype.signInput // use bitcoin.TransactionBuilder.prototype.sign instead
  • bitcoin.Transaction.prototype.validateInput // use hashForSignature and ecdsa verify

updated

  • bitcoin.Message -> bitcoin.message
  • bitcoin.HDNode.toBase58(isPrivate) -> bitcoin.HDNode.prototype.neutered().toBase58() for exporting a extended public key and bitcoin.HDNode.prototype.toBase58() for exporting an extended private key
  • bitcoin.scripts.dataOutput -> bitcoin.nullDataOutput
  • Transaction.prototype.hashForSignature(prevOutScript, inIndex, hashType) -> Transaction.prototype.hashForSignature(inIndex, prevOutScript, hashType)
  • Transaction.prototype.addInput(hash, ...): hash could be a string, Transaction or Buffer -> hash can now only be a Buffer.
  • Transaction.prototype.addOutput(scriptPubKey, ...): scriptPubKey could be a string, Address or Buffer -> scriptPubKey can now only be a Buffer.
  • TransactionBuilder API unchanged.

@weilu
Copy link
Contributor

weilu commented Mar 2, 2015

LGTM

TransactionBuilder API unchanged.

If it's not changed, it should not be under updated section

weilu added a commit that referenced this pull request Mar 2, 2015
@weilu weilu merged commit b132714 into master Mar 2, 2015
@dcousens
Copy link
Contributor Author

dcousens commented Mar 2, 2015

It was an amendment to what you had, which specified changes, even though
it was unchanged.
On 3 Mar 2015 4:24 am, "Wei Lu" notifications@github.com wrote:

LGTM

TransactionBuilder API unchanged.

If it's not changed, it should not be under updated section


Reply to this email directly or view it on GitHub
#317 (comment)
.

@dcousens dcousens mentioned this pull request Mar 2, 2015
@dcousens dcousens deleted the 2.0.0 branch April 28, 2015 06:41
@dcousens dcousens mentioned this pull request Aug 13, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants