Conversation
|
What is the impact of this change? looks like you created more LOC |
|
@omershlo And some |
|
ready to review & merge |
|
I don't think replacing |
|
"explicit is better than implicit" |
src/elliptic/curves/secp256_k1.rs
Outdated
| Some(BigInt::from(&y_vec[..])) | ||
| } | ||
|
|
||
| fn from_bytes(bytes: &[u8]) -> Result<Secp256k1Point, ErrorKey> { |
There was a problem hiding this comment.
this function may be the most refactored one, the logic is basically the same, except directly sending 65 or 33 bytes to secp256k1 lib
There was a problem hiding this comment.
for > 65 bytes input, the necessity of taking its first 64 bytes should be reconsidered
There was a problem hiding this comment.
though I just keep the logic as same as before
| v.extend(BigInt::to_vec(&self.y_coor().unwrap())); | ||
| v | ||
| fn pk_to_key_slice(&self) -> Vec<u8> { | ||
| self.ge.serialize_uncompressed().to_vec() |
There was a problem hiding this comment.
less computation but the vec length should always be 65 bytes
src/elliptic/curves/secp256_k1.rs
Outdated
| } | ||
| } | ||
|
|
||
| static mut CONTEXT: Option<Secp256k1<VerifyOnly>> = None; |
There was a problem hiding this comment.
replaced by lazy_static
src/elliptic/curves/secp256_k1.rs
Outdated
| impl ECScalar<SK> for Secp256k1Scalar { | ||
| fn new_random() -> Secp256k1Scalar { | ||
| let mut arr = [0u8; 32]; | ||
| thread_rng().fill(&mut arr[..]); |
There was a problem hiding this comment.
the private key may not be valid, should just use library's new function
There was a problem hiding this comment.
the probability of getting invalid key is not high, but it's not zero
src/elliptic/curves/secp256_k1.rs
Outdated
| ECPoint::add_point(self, &minus_point.get_element()) | ||
| } | ||
|
|
||
| fn from_coor(x: &BigInt, y: &BigInt) -> Secp256k1Point { |
There was a problem hiding this comment.
another function replaced by succinct refactored version
|
I've added some notes on the part with possible logic change |
|
@omershlo |
|
sure |
3 participants
fix #62