Conversation
| GHC==9.2.4 | ||
| GHC==9.0.2 | ||
| GHC==8.10.7 | ||
| , GHC==9.4.2 |
There was a problem hiding this comment.
That is included in #17, which can be merged as soon as chainweb-node supports aeson-2.2, which is currently blocked on haskell-servant/servant#1695.
ghost
left a comment
ghost
left a comment
There was a problem hiding this comment.
Don't know much about the cryptography but overall the code LGTM.
| natToBytesInternal l n = go l n [] | ||
| where | ||
| go 0 _ = id | ||
| go i m = let (a, b) = quotRem m 256 in go (i - 1) a . (int b :) |
There was a problem hiding this comment.
not: put a bang on i to make GHC more likely to make this tail recursive/for the int to unpack
| where | ||
| r = a .^ ((pC + 1) `quot` 4) | ||
|
|
||
| -- | Not sure if this is faster. TODO benchmark it. |
There was a problem hiding this comment.
There is a TODO for benchmarks here, but the above comment said that it's been benchmarked. Could you amend the comments to reconcile this?
| -- | Recover public key for curve Secp256k1. | ||
| -- | ||
| -- The chance that `secondKey` is needed (i.e. there are two solution for the x | ||
| -- coordinate) is \(2^{128}\). Do we need to cover that case or can we just |
There was a problem hiding this comment.
I think you mean the inverse of this.
| (sk, pk) <- genKey | ||
| (r, s, isOddY, isSecondKey) <- sign sk msgDigest | ||
| return | ||
| $ classify isOddY "isOddY" |
|
Approved, but left a few comments of some small things I would like to see changed. I'm not an expert on the cryptography here either, but the math makes sense to me. Reading the references helped. |
2 participants
No description provided.