Remove instanceof since we can't actually create the ecurve.Point

[glorat]

No description provided.

[glorat]

This is how I'm fixing my own issue #255

[coveralls]

Coverage decreased (-0.14%) when pulling 201a30e on glorat:master into dce2690 on bitcoinjs:master.

[dcousens]

Could also be replaced with just Q.affineX which is a property with lazy evaluating side effects.

As to why this stops sinon from detecting that TransactionBuilder.sign has been called with the correct arguments? I have absolutely no idea.

[glorat]

It beats me too.

As discussed over IRC, since signing is deterministic, one could replace all the sinon interception with an assertion that the signature payload is as expected. This would be a stronger test than the assertion that the sign method was called.

Unfortunately, that's beyond the scope of my minimal volunteerism for now!

[weilu]

It sounds to me it's more like a problem that sinon exposed which needs to be addressed over at the ecurve project.

[dcousens]

As discussed in #280, the reason this fails without the affineX is due to its lazy evaluation, resulting in a deep equality difference; which is used by calledWith in sinon.

edit: resolved this problem separately

[dcousens]

@glorat does #257 help your situation?

[weilu]

@glorat can you clean up (squash/rebase) the git history so we can get this merged?

[dcousens]

And remove the toString
On Sep 22, 2014 10:55 PM, "Wei Lu" [email protected] wrote:

@glorat https://github.com/glorat can you clean up (squash/rebase) the
git history so we can get this merged?

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

[glorat]

That will be an exercise for my new git skills! Will try...

[glorat]

Cleaned up as requested and CI passes this time thanks to the other changes.

I'm agnostic as to whether you guys go with this or something else like #257. For my part, I'll be using this patch locally since it works for me.

[weilu]

@dcousens the change looks good to me. Recently I have actually bumped into something like this -- our staging doesn't work because of an instanceof check: https://hive-js.herokuapp.com/. Shall we do a global remove, or alternatively replace them with verbose duck typing method checks?

[dcousens]

I feel like it's inconsistent to only do it here. Thoughts on params object for HDNode constructor?
On Sep 27, 2014 1:20 PM, "Wei Lu" [email protected] wrote:

@dcousens https://github.com/dcousens the change looks good to me.
Recently I have actually bumped into something like this -- our staging
doesn't work because of an instanceof check:
https://hive-js.herokuapp.com/. Shall we do a global remove, or
alternatively replace them with verbose duck typing method checks?

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

[weilu]

Why HDNode?

[dcousens]

if (K instanceof BigInteger) {

[glorat]

Closing this now as there are other proposals that are looking better