Implementation of secp256k1 Group #844
Conversation
There was a problem hiding this comment.
Thanks for this contribution! Overall looks good:
- I focused so far on the group related code, see comments
- Please add tests similarly to the ones of bls12381_group_tests.rs
- Note that I've sent [DKG] Clean up old versions of DKG & ECIES #845 to clean up the DKG code. I don't expect major changes needed because of that in the current PR, so mostly FYI.
fastcrypto-tbls/src/ecies_v0.rs
Outdated
| @@ -76,6 +76,10 @@ where | |||
| proof, | |||
| } | |||
| } | |||
|
|
|||
| pub fn as_element(&self) -> &G::ScalarType { | |||
There was a problem hiding this comment.
In general the raw secret key should not be used not via APIs, do we really need this?
There was a problem hiding this comment.
My main use was from an external library that needed to convert the private key to a k256::ecdsa::SigningKey. But I can change the external library instead, and remove as_element() from here. Alternatively, we could also implement From<PrivateKey<G>> for k256::ecdsa::SigningKey, but it is not necessary for now.
There was a problem hiding this comment.
Not sure I follow - the ECIES private key is not related to the DKG private key/share, you can generate it independently when creating a party
| } | ||
|
|
||
| impl FromTrustedByteArray<SCALAR_BYTE_LENGTH> for Scalar { | ||
| fn from_trusted_byte_array(bytes: &[u8; SCALAR_BYTE_LENGTH]) -> FastCryptoResult<Self> { |
There was a problem hiding this comment.
please document the expected format
| } | ||
|
|
||
| impl FromTrustedByteArray<PROJECTIVE_POINT_BYTE_LENGTH> for ProjectivePoint { | ||
| fn from_trusted_byte_array( |
There was a problem hiding this comment.
Please document the format here
|
|
||
| impl ToFromByteArray<PROJECTIVE_POINT_BYTE_LENGTH> for ProjectivePoint { | ||
| fn from_byte_array(bytes: &[u8; PROJECTIVE_POINT_BYTE_LENGTH]) -> FastCryptoResult<Self> { | ||
| Self::from_trusted_byte_array(bytes) |
There was a problem hiding this comment.
this will unwrap for invalid points? let's return error instead
| fn hash_to_group_element(msg: &[u8]) -> Self { | ||
| let domain = "secp256k1_XMD:SHA-256_SSWU_RO_".as_bytes(); | ||
| let mut u = [FieldElement::ZERO]; | ||
| hash_to_field::<ExpandMsgXmd<Sha256>, FieldElement>(&[msg], &[domain], &mut u) |
There was a problem hiding this comment.
Please add a link to the relevant standard this is based on
| } | ||
|
|
||
| serialize_deserialize_with_to_from_byte_array!(ProjectivePoint); | ||
| generate_bytes_representation!( |
There was a problem hiding this comment.
I guess that you don't need this - we mainly use XAsBytes for external interfaces
…perimental' feature
This PR implements the secp256k1 group by wrapping around the k256 crate. This enhancement enables the use of the DKG protocol in fastcrypto-tbls to produce shared keys on the secp256k1 curve.
Motivation: We would like to use the fastcrypto-tbls DKG protocol to produce shares for a key that can sign transactions on Bitcoin, which natively supports secp256k1. However, the DKG protocol implementation currently only supports the BLS12-381 curve. By implementing the secp256k1 group, we enable the DKG protocol to work in conjunction with a signature aggregation protocol (e.g., FROST) to produce aggregated signatures that can be natively verified on Bitcoin.
Changes: