Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Switch to noble-secp256k1 #145

Merged
merged 3 commits into from
Apr 1, 2021
Merged

Switch to noble-secp256k1 #145

merged 3 commits into from
Apr 1, 2021

Conversation

mds1
Copy link
Collaborator

@mds1 mds1 commented Apr 1, 2021

(Built off the branch in #133, so that needs to be merged first, then we'll rebase this against the master)

  • Implements noble-secp256k1 for all ECC operations
  • A bit of cleanup (remove unneeded padHex method)
  • Need to double check that we're not using elliptic behind the scenes from an ethers method
  • We also change the shared secret computation, such that the shared secret is now the hash of the resulting point, instead of the point itself. This is preferred as in theory it should make the output symmetric keys more uniformly distributed. Consequently, this PR is a breaking change and once merged, past sends will no longer display in the UI when building locally

@mds1 mds1 requested a review from apbendi April 1, 2021 04:49
@mds1 mds1 changed the base branch from master to add-frontend-features April 1, 2021 14:13
@mds1 mds1 changed the base branch from add-frontend-features to master April 1, 2021 14:14
Copy link
Member

@apbendi apbendi left a comment

Choose a reason for hiding this comment

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

Looks great! Running the tests I got the following error:

1) Utilities
@umbra/umbra-js:        Recipient identifier lookups
@umbra/umbra-js:          looks up recipients by address:
@umbra/umbra-js:      Error: Transaction not found. Are the provider and transaction hash on the same network?
@umbra/umbra-js:       at /Users/bendi/Development/umbra/umbra-js/src/utils/utils.ts:46:11
@umbra/umbra-js:       at step (src/utils/utils.ts:36:23)
@umbra/umbra-js:       at Object.next (src/utils/utils.ts:17:53)
@umbra/umbra-js:       at fulfilled (src/utils/utils.ts:8:58)
@umbra/umbra-js:       at processTicksAndRejections (internal/process/task_queues.js:97:5)
@umbra/umbra-js: error Command failed with exit code 1.

which is possibly related to this warning?

@umbra/umbra-js: ========= NOTICE =========
@umbra/umbra-js: Request-Rate Exceeded  (this message will not be repeated)
@umbra/umbra-js: The default API keys for each service are provided as a highly-throttled,
@umbra/umbra-js: community resource for low-traffic projects and early prototyping.
@umbra/umbra-js: While your application will continue to function, we highly recommended
@umbra/umbra-js: signing up for your own API keys to improve performance, increase your
@umbra/umbra-js: request rate/limit and enable other perks, such as metrics and advanced APIs.
@umbra/umbra-js: For more details: https://docs.ethers.io/api-keys/
@umbra/umbra-js: ==========================

Can we add an API key as a required env var setting to prevent this?

@mds1
Copy link
Collaborator Author

mds1 commented Apr 1, 2021

Added info about the above test failure to #104 (comment) and will track it there

@mds1 mds1 merged commit b8c8473 into master Apr 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants