-
Notifications
You must be signed in to change notification settings - Fork 9
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
util/signature: add WalletConnect API support #386
Conversation
Codecov Report
@@ Coverage Diff @@
## master #386 +/- ##
==========================================
- Coverage 63.65% 63.36% -0.30%
==========================================
Files 71 74 +3
Lines 9017 9163 +146
==========================================
+ Hits 5740 5806 +66
- Misses 2590 2663 +73
- Partials 687 694 +7
Continue to review full report at Codecov.
|
msg := []byte("NEO") | ||
result, err := SignMessage(p1, msg) | ||
require.NoError(t, err) | ||
//require.Equal(t, elliptic.MarshalCompressed(elliptic.P256(), p1.PublicKey.X, p1.PublicKey.Y), result.PublicKey) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
debug?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
go.mod changes look a bit strange, though
A binary in the PR. Was it planned? |
util/signature/walletconnect/sign.go
Outdated
// Verify verifies message using WalletConnect API. The returned signature | ||
// contains RFC6979 signature and 16-byte salt. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
// Verify verifies message using WalletConnect API. The returned signature | |
// contains RFC6979 signature and 16-byte salt. | |
// Verify verifies message using WalletConnect API. |
copypaste?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is nice to have some info about the signature format.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, that is good but what about "...returned..."? func does not return any signature
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
To avoid introducing new dependency (neo-go), crypto routines are used as in other code. Signed-off-by: Evgenii Stratonikov <evgeniy@nspcc.ru>
Signed-off-by: Evgenii Stratonikov evgeniy@nspcc.ru