Skip to content
This repository has been archived by the owner on Aug 2, 2022. It is now read-only.

Replace eosjs-ecc with elliptic #579

Merged
merged 3 commits into from
Aug 9, 2019
Merged

Replace eosjs-ecc with elliptic #579

merged 3 commits into from
Aug 9, 2019

Conversation

tbfleming
Copy link
Contributor

Change Description

API Changes

  • API Changes

Documentation Additions

  • Documentation Additions

@tbfleming tbfleming marked this pull request as ready for review August 9, 2019 15:34
it('builds public keys from private when constructed', async () => {
/*
Copy link
Contributor

Choose a reason for hiding this comment

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

Rather than commenting out, I'd suggest just going ahead and deleting them.

Semi-related, if these tests didn't actually test the correctness it would be nice to see tests that do test the correctness.

Copy link
Contributor

@c0d3ster c0d3ster left a comment

Choose a reason for hiding this comment

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

Agree that we should have tests for the webauthn and changes to signature provider prior to the next release, but I won't block the PR since you need this to move forward

throw new Error('unrecognized private key format');
// todo: Verify checksum: sha256(sha256(key.data)).
// Not critical since a bad key will fail to produce an
// invalid signature anyway.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should be "fail to produce valid signature"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed

@tbfleming tbfleming merged commit 3d6c39c into develop Aug 9, 2019
@tbfleming tbfleming deleted the elliptic2 branch August 9, 2019 17:26
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants