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

use secp256k1 library for signing #815

Closed
wants to merge 7 commits into from
Closed

use secp256k1 library for signing #815

wants to merge 7 commits into from

Conversation

@pedrouid
Copy link
Member

pedrouid commented Feb 5, 2020

The Problem

The Solution

Checklist:

  • I am making this PR against staging, not master
  • My code follows the code style of this project.
  • I have described any backwards-incompatibility implications above.
  • I have highlighted which parts of the code should be reviewed most carefully.
  • If my change requires a change to the documentation, I have updated it accordingly.
@todo-tracker

This comment has been minimized.

Copy link

todo-tracker bot commented Feb 5, 2020

Hey, pedrouid

We noticed you made changes to a file with a TODO on it.
These are set to make sure potential Technical Debt doesn't get forgotten.
While you're here take a shot at turning a listed TODO into a TODONE!

Id Name File Priority
86 clean up types from restore, without the any typing things modules/client/src/channelProvider.ts Normal
87 remove when using only store package modules/client/src/channelProvider.ts Normal

button

@LayneHaber LayneHaber self-requested a review Feb 6, 2020
@todo-tracker

This comment has been minimized.

Copy link

todo-tracker bot commented Feb 6, 2020

Hey, LayneHaber

It looks like you made some changes to this pull request!
These changes included files that have a TODO linked to them.
Click the button below to view details on all the TODO for this pull request, and while you're here take a shot at turning a TODO into a TODONE!

Id Name File Priority
65 correct? how can i use allowance? modules/client/src/controllers/DepositController.ts Normal
84 Currently, this test always fails because when promise is never rejected when modules/test-runner/src/swap/swap.test.ts Normal
91 fix race condition, the following assertion randomly fails modules/test-runner/src/transfer/getLinkedTransfer.test.ts Normal
93 update this to include the intermediares modules/types/src/messaging.ts Normal
94 update this to include the intermediares modules/types/src/messaging.ts Normal
98 this passes locally when running `make test-integration, and when modules/test-runner/src/swap/swap.test.ts Normal

button

LayneHaber added 2 commits Feb 6, 2020
doh
modules/client/src/lib/utils.ts Outdated Show resolved Hide resolved
pedrouid added 3 commits Feb 7, 2020
@todo-tracker

This comment has been minimized.

Copy link

todo-tracker bot commented Feb 7, 2020

Hey, pedrouid

It looks like you made some changes to this pull request!
These changes included files that have a TODO linked to them.
Click the button below to view details on all the TODO for this pull request, and while you're here take a shot at turning a TODO into a TODONE!

Id Name File Priority
64 expose remove listener modules/node/src/transfer/transfer.service.ts Normal
68 rename to "get" when in branch to publish new client modules/node/src/transfer/transfer.provider.ts Normal
71 merge: modules/client/src/node.ts Normal
81 dont listen to linked transfer app in default listener, only modules/client/src/controllers/ResolveConditionController.ts Normal
82 how can we access / verify the meta here? modules/client/src/controllers/ResolveConditionController.ts Normal
83 should add once to top level client modules/test-runner/src/util/helpers/fundChannel.ts Normal
85 events for withdrawal commitments! issue 698 modules/test-runner/src/withdraw/withdraw.test.ts Normal
95 remove modules/client/src/controllers/ResolveConditionController.ts Normal
96 remove modules/client/src/controllers/ResolveConditionController.ts Normal
97 remove modules/client/src/controllers/ResolveConditionController.ts Normal
100 why only one token allowed in virtual? modules/types/src/protocol.ts Normal
101 move this to install modules/node/src/appRegistry/appRegistry.service.ts Normal
102 this doesn't work with the new paradigm, we won't know this info modules/node/src/appRegistry/appRegistry.service.ts Normal

button

@pedrouid

This comment has been minimized.

Copy link
Member Author

pedrouid commented Feb 7, 2020

This was breaking with ConnextBenchmarking and even simply loading the DaiCard, I'm unsure how this even passed all the tests. We should investigate the case where signing breaks but all the tests are passing.

@pedrouid pedrouid closed this Feb 7, 2020
@pedrouid pedrouid deleted the secp256k1-node branch Feb 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.