Skip to content
This repository has been archived by the owner on Dec 10, 2020. It is now read-only.

Update secp256k1 to v4.0.1 #68

Merged
merged 4 commits into from
May 15, 2020
Merged

Update secp256k1 to v4.0.1 #68

merged 4 commits into from
May 15, 2020

Conversation

PhilippLgh
Copy link
Contributor

No description provided.

@@ -17,7 +17,7 @@ import {

function ecdhX(publicKey: Buffer, privateKey: Buffer) {
// return (publicKey * privateKey).x
return ecdhUnsafe(publicKey, privateKey, true).slice(1)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume that this breaks the tests. The ecdhUnsafe method seems to be removed from v4. Needs investigation

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@holgerd77 any idea?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here was some issue on ecdhUnsafe from Vinay before, not sure if this is related: cryptocoinjs/secp256k1-node#138

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have posted a question on this over on the secp256k1-node library.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See answer over on the repo thread

@holgerd77
Copy link
Member

Thanks for tackling this. There are still 4 tests failing, also make sure to have a look at the linting error, always a favorite to be forgotten by everyone. 😋 (we should also add these Husky pre-push linting check we have added on the other libraries - e.g. here on MPT - turned out to become really a time and nerve saver.

@coveralls
Copy link

coveralls commented May 15, 2020

Coverage Status

Coverage decreased (-0.007%) to 87.391% when pulling 4c03f5a on update-secp256k1 into 6b72227 on master.

return pubKey
}
// @ts-ignore
return Buffer.from(ecdh(publicKey, privateKey, { hashfn }, Buffer.alloc(33)).slice(1))
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just totally unable to review here. Where did you get this code from respectively - if you wrote yourself - can you further explain?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Update: ah, just seeing your private messages on this, ok, looks good in this context! 😄

Copy link
Member

@holgerd77 holgerd77 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, great, thank you. Will merge.

@holgerd77 holgerd77 merged commit 5800e85 into master May 15, 2020
@holgerd77 holgerd77 deleted the update-secp256k1 branch May 15, 2020 15:59
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants