[gh-457] add Schnorr signature support. #461

feliciss · 2023-07-12T03:25:55Z

This PR resolves #457

vercel · 2023-07-12T03:25:58Z

Someone is attempting to deploy a commit to the Rooch Team on Vercel.

A member of the Team first needs to authorize it.

feliciss · 2023-07-13T13:04:16Z

I would like to remove traits imported from fastcrypto and generalize trait RoochSignatureInner and use it for SchnorrRoochSignature, Ed25519RoochSignature and ECDSARoochSignature.

But I encountered some difficult issues:

To satisfy AsRef<[u8]> of RoochAddress:

error[E0277]: the trait bound `secp256k1::XOnlyPublicKey: AsRef<[u8]>` is not satisfied
   --> crates/rooch-types/src/crypto.rs:812:48
    |
812 |         let received_addr = RoochAddress::from(&pk);
    |                             ------------------ ^^^ the trait `AsRef<[u8]>` is not implemented for `secp256k1::XOnlyPublicKey`
    |                             |
    |                             required by a bound introduced by this call
    |
    = help: the following other types implement trait `From<T>`:
              <address::RoochAddress as From<&T>>
              <address::RoochAddress as From<&crypto::PublicKey>>
              <address::RoochAddress as From<move_core_types::account_address::AccountAddress>>
note: required for `address::RoochAddress` to implement `From<&secp256k1::XOnlyPublicKey>`

Duplicate implementation of Secp256k1KeyPair

// TODO generalize Secp256k1KeyPair
// impl Signer<Signature> for Secp256k1KeyPair {
//     fn sign(&self, msg: &[u8]) -> Signature {
//         ECDSARoochSignature::new(self, msg).into()
//     }
// }

impl Signer<Signature> for Secp256k1KeyPair {
    fn sign(&self, msg: &[u8]) -> Signature {
        SchnorrRoochSignature::new(self, msg).into()
    }
}

They are using same key pair base for ECDSA and Schnorr signatures.

Other wreaked issues:

error[E0277]: the trait bound `OsRng: rand_core::CryptoRng` is not satisfied
   --> crates/rooch-types/src/transaction/authenticator.rs:60:50
    |
60  |         let keypair: Keypair = Keypair::generate(&mut csprng);
    |                                ----------------- ^^^^^^^^^^^ the trait `rand_core::CryptoRng` is not implemented for `OsRng`
    |                                |
    |                                required by a bound introduced by this call
    |
    = help: the following other types implement trait `rand_core::CryptoRng`:
              &'a mut R
              Box<R>
              rand::rngs::adapter::reseeding::ReseedingRng<R, Rsdr>
              rand::rngs::entropy::EntropyRng
              rand::rngs::std::StdRng
              rand::rngs::thread::ThreadRng
              rand_chacha::chacha::ChaCha12Core
              rand_chacha::chacha::ChaCha12Rng
            and 6 others
note: required by a bound in `ed25519_dalek::Keypair::generate`
    |
129 |         R: CryptoRng + RngCore,
    |            ^^^^^^^^^ required by this bound in `Keypair::generate`

error[E0277]: the trait bound `StdRng: crypto::AllowedRng` is not satisfied
    --> crates/rooch-types/src/crypto.rs:1045:27
     |
1045 |     let kp = KP::generate(&mut StdRng::from_rng(csprng).unwrap());
     |              ------------ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `crypto::AllowedRng` is not implemented for `StdRng`
     |              |
     |              required by a bound introduced by this call
     |
note: required by a bound in `KeypairTraits::generate`
    --> crates/rooch-types/src/crypto.rs:440:20
     |
440  |     fn generate<R: AllowedRng>(rng: &mut R) -> Self;
     |                    ^^^^^^^^^^ required by this bound in `KeypairTraits::generate`

I'm still solving the above problems, but I might need some reviews and hints. @jolestar @wow-sven

There're conflicts between #466 and #461, mainly for renaming spec256k1 to ECDSA in crates/rooch-types/src/transaction/authenticator.rs.

wow-sven · 2023-07-16T05:57:42Z

I would like to remove traits imported from fastcrypto and generalize trait RoochSignatureInner and use it for SchnorrRoochSignature, Ed25519RoochSignature and ECDSARoochSignature.

But I encountered some difficult issues:

To satisfy AsRef<[u8]> of RoochAddress:

error[E0277]: the trait bound `secp256k1::XOnlyPublicKey: AsRef<[u8]>` is not satisfied
   --> crates/rooch-types/src/crypto.rs:812:48
    |
812 |         let received_addr = RoochAddress::from(&pk);
    |                             ------------------ ^^^ the trait `AsRef<[u8]>` is not implemented for `secp256k1::XOnlyPublicKey`
    |                             |
    |                             required by a bound introduced by this call
    |
    = help: the following other types implement trait `From<T>`:
              <address::RoochAddress as From<&T>>
              <address::RoochAddress as From<&crypto::PublicKey>>
              <address::RoochAddress as From<move_core_types::account_address::AccountAddress>>
note: required for `address::RoochAddress` to implement `From<&secp256k1::XOnlyPublicKey>`

Duplicate implementation of Secp256k1KeyPair

// TODO generalize Secp256k1KeyPair
// impl Signer<Signature> for Secp256k1KeyPair {
//     fn sign(&self, msg: &[u8]) -> Signature {
//         ECDSARoochSignature::new(self, msg).into()
//     }
// }

impl Signer<Signature> for Secp256k1KeyPair {
   fn sign(&self, msg: &[u8]) -> Signature {
       SchnorrRoochSignature::new(self, msg).into()
   }
}

They are using same key pair base for ECDSA and Schnorr signatures.

Other wreaked issues:

error[E0277]: the trait bound `OsRng: rand_core::CryptoRng` is not satisfied
   --> crates/rooch-types/src/transaction/authenticator.rs:60:50
    |
60  |         let keypair: Keypair = Keypair::generate(&mut csprng);
    |                                ----------------- ^^^^^^^^^^^ the trait `rand_core::CryptoRng` is not implemented for `OsRng`
    |                                |
    |                                required by a bound introduced by this call
    |
    = help: the following other types implement trait `rand_core::CryptoRng`:
              &'a mut R
              Box<R>
              rand::rngs::adapter::reseeding::ReseedingRng<R, Rsdr>
              rand::rngs::entropy::EntropyRng
              rand::rngs::std::StdRng
              rand::rngs::thread::ThreadRng
              rand_chacha::chacha::ChaCha12Core
              rand_chacha::chacha::ChaCha12Rng
            and 6 others
note: required by a bound in `ed25519_dalek::Keypair::generate`
    |
129 |         R: CryptoRng + RngCore,
    |            ^^^^^^^^^ required by this bound in `Keypair::generate`

error[E0277]: the trait bound `StdRng: crypto::AllowedRng` is not satisfied
   --> crates/rooch-types/src/crypto.rs:1045:27
    |
1045 |     let kp = KP::generate(&mut StdRng::from_rng(csprng).unwrap());
    |              ------------ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `crypto::AllowedRng` is not implemented for `StdRng`
    |              |
    |              required by a bound introduced by this call
    |
note: required by a bound in `KeypairTraits::generate`
   --> crates/rooch-types/src/crypto.rs:440:20
    |
440  |     fn generate<R: AllowedRng>(rng: &mut R) -> Self;
    |                    ^^^^^^^^^^ required by this bound in `KeypairTraits::generate`

I'm still solving the above problems, but I might need some reviews and hints. @jolestar @wow-sven

There're conflicts between #466 and #461, mainly for renaming spec256k1 to ECDSA in crates/rooch-types/src/transaction/authenticator.rs.

Duplicate implementation of Secp256k1KeyPair

Option1:
Keep making changes based on this commit （865e9dce759a71a729f5e0f527e19e7dbc209a5e） without breaking the current Fastcrypto, can be pushed to Fastcrypto

Fork fastcrypto, Add schnorr to it.

Implements the Authenticator trait for SchnorrSignature
Implements the VerifyingKey trait for XOnlyPublicKey
Implements the fastcrypto::KeyPair trait for KeyPair

ref：MystenLabs/fastcrypto#51

Option2:
Remove fastcrypto completely and reorganize instead of keeping parts. Avoid further incompatibilities later on

feliciss · 2023-07-16T20:27:53Z

I would like to remove traits imported from fastcrypto and generalize trait RoochSignatureInner and use it for SchnorrRoochSignature, Ed25519RoochSignature and ECDSARoochSignature.
But I encountered some difficult issues:

To satisfy AsRef<[u8]> of RoochAddress:

error[E0277]: the trait bound `secp256k1::XOnlyPublicKey: AsRef<[u8]>` is not satisfied
   --> crates/rooch-types/src/crypto.rs:812:48
    |
812 |         let received_addr = RoochAddress::from(&pk);
    |                             ------------------ ^^^ the trait `AsRef<[u8]>` is not implemented for `secp256k1::XOnlyPublicKey`
    |                             |
    |                             required by a bound introduced by this call
    |
    = help: the following other types implement trait `From<T>`:
              <address::RoochAddress as From<&T>>
              <address::RoochAddress as From<&crypto::PublicKey>>
              <address::RoochAddress as From<move_core_types::account_address::AccountAddress>>
note: required for `address::RoochAddress` to implement `From<&secp256k1::XOnlyPublicKey>`

Duplicate implementation of Secp256k1KeyPair

// TODO generalize Secp256k1KeyPair
// impl Signer<Signature> for Secp256k1KeyPair {
//     fn sign(&self, msg: &[u8]) -> Signature {
//         ECDSARoochSignature::new(self, msg).into()
//     }
// }

impl Signer<Signature> for Secp256k1KeyPair {
   fn sign(&self, msg: &[u8]) -> Signature {
       SchnorrRoochSignature::new(self, msg).into()
   }
}

They are using same key pair base for ECDSA and Schnorr signatures.
Other wreaked issues:

error[E0277]: the trait bound `OsRng: rand_core::CryptoRng` is not satisfied
   --> crates/rooch-types/src/transaction/authenticator.rs:60:50
    |
60  |         let keypair: Keypair = Keypair::generate(&mut csprng);
    |                                ----------------- ^^^^^^^^^^^ the trait `rand_core::CryptoRng` is not implemented for `OsRng`
    |                                |
    |                                required by a bound introduced by this call
    |
    = help: the following other types implement trait `rand_core::CryptoRng`:
              &'a mut R
              Box<R>
              rand::rngs::adapter::reseeding::ReseedingRng<R, Rsdr>
              rand::rngs::entropy::EntropyRng
              rand::rngs::std::StdRng
              rand::rngs::thread::ThreadRng
              rand_chacha::chacha::ChaCha12Core
              rand_chacha::chacha::ChaCha12Rng
            and 6 others
note: required by a bound in `ed25519_dalek::Keypair::generate`
    |
129 |         R: CryptoRng + RngCore,
    |            ^^^^^^^^^ required by this bound in `Keypair::generate`

error[E0277]: the trait bound `StdRng: crypto::AllowedRng` is not satisfied
   --> crates/rooch-types/src/crypto.rs:1045:27
    |
1045 |     let kp = KP::generate(&mut StdRng::from_rng(csprng).unwrap());
    |              ------------ ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `crypto::AllowedRng` is not implemented for `StdRng`
    |              |
    |              required by a bound introduced by this call
    |
note: required by a bound in `KeypairTraits::generate`
   --> crates/rooch-types/src/crypto.rs:440:20
    |
440  |     fn generate<R: AllowedRng>(rng: &mut R) -> Self;
    |                    ^^^^^^^^^^ required by this bound in `KeypairTraits::generate`

I'm still solving the above problems, but I might need some reviews and hints. @jolestar @wow-sven
There're conflicts between #466 and #461, mainly for renaming spec256k1 to ECDSA in crates/rooch-types/src/transaction/authenticator.rs.

Duplicate implementation of Secp256k1KeyPair

Option1: Keep making changes based on this commit （865e9dce759a71a729f5e0f527e19e7dbc209a5e） without breaking the current Fastcrypto, can be pushed to Fastcrypto

Fork fastcrypto, Add schnorr to it.

Implements the Authenticator trait for SchnorrSignature
Implements the VerifyingKey trait for XOnlyPublicKey
Implements the fastcrypto::KeyPair trait for KeyPair

ref：MystenLabs/fastcrypto#51

Option2: Remove fastcrypto completely and reorganize instead of keeping parts. Avoid further incompatibilities later on

Thanks. 🙂 I'll do Option 2 to decouple it from fastcrypto. I don't think it's a good idea to keep dependencies on commercial libraries.

jolestar · 2023-07-17T01:18:54Z

fastcrypto is in Apache2/MIT license. I think Option 1 is better.

feliciss · 2023-07-17T01:26:18Z

fastcrypto is in Apache2/MIT license. I think Option 1 is better.

I can add Option 1 temporarily, but in the long term view, it may need to decouple from fastcrypto if that repo isn't being maintained.

feliciss · 2023-07-17T02:09:57Z

fastcrypto is in Apache2/MIT license. I think Option 1 is better.

Can you @wow-sven @jolestar make a fork the repo fastcrypto? I do not have access to the team account rooch-network on GitHub. Or I can fork to my own and submit it later to the team repo.

jolestar · 2023-07-17T02:32:58Z

fastcrypto is in Apache2/MIT license. I think Option 1 is better.

Can you @wow-sven @jolestar make a fork the repo fastcrypto? I do not have access to the team account rooch-network on GitHub. Or I can fork to my own and submit it later to the team repo.

https://github.com/rooch-network/fastcrypto

vercel · 2023-07-18T04:34:26Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment

Name	Status	Preview	Comments	Updated (UTC)
rooch	⬜️ Ignored (Inspect)			Jul 19, 2023 3:21am

feliciss · 2023-07-18T04:39:05Z

@wow-sven @jolestar I can almost finish it, but, there's another duplicate implementation. Do you have any idea to resolve it?

Code:

#[derive(Debug, Clone, PartialEq, Eq, JsonSchema)]
// TODO From here
// #[derive(Debug, Clone, PartialEq, Eq, From, JsonSchema)]
pub enum PublicKey {
    Ed25519(Ed25519PublicKeyAsBytes),
    Ecdsa(Secp256k1PublicKeyAsBytes),
    Schnorr(SchnorrPublicKeyAsBytes),
}

// TODO resolve From conflicts
impl From<BytesRepresentation<32>> for PublicKey {
    fn from(pk: BytesRepresentation<32>) -> Self {
        PublicKey::Ed25519(pk)
    }
}

impl AsRef<[u8]> for PublicKey {
    fn as_ref(&self) -> &[u8] {
        match self {
            PublicKey::Ed25519(pk: &BytesRepresentation<32>) => &pk.0,
            PublicKey::Ecdsa(pk: &BytesRepresentation<33>) => &pk.0,
            PublicKey::Schnorr(pk: &BytesRepresentation<32>) => &pk.0,
        }
    }
}

There're two hidden implementations of &BytesRepresentation<32> here.

One solution I can think of is to abstract BytesRepresentation<32> with BytesRepresentation<ed25519>, BytesRepresentation<ecdsa>, BytesRepresentation<schnorr>, respectively.

I'll try implementing it later.

feliciss · 2023-07-18T23:23:01Z

It's ready for review now.

jolestar · 2023-07-19T02:05:54Z

crates/rooch-framework/src/natives/rooch_framework/crypto/schnorr.rs

Need a Move file to export the native function

jolestar · 2023-07-19T02:07:22Z

Cargo.toml

@@ -109,7 +109,7 @@ enum_dispatch = "^0.3"
 ethereum-types = "0.14.1"
 ethers = { version = "2.0.7", features = ["legacy"] }
 eyre = "0.6.8"
-fastcrypto = { git = "https://github.com/MystenLabs/fastcrypto", rev = "c27e87fb539c88b6fe1eac7dd82561254bb1e07a" }
+fastcrypto = { git = "https://github.com/feliciss/fastcrypto", branch = "schnorr" }


rooch-network/fastcrypto#1 has merged, use /rooch-network/fastcrypto

feliciss force-pushed the #457 branch 2 times, most recently from d0af4c0 to 2149b61 Compare July 13, 2023 12:45

feliciss force-pushed the #457 branch from 2149b61 to 865e9dc Compare July 13, 2023 13:06

feliciss changed the title ~~[gh-457] draft Schnorr signature support.~~ [gh-457] abstract signatures and add Schnorr support. Jul 14, 2023

jolestar requested a review from wow-sven July 14, 2023 15:39

feliciss force-pushed the #457 branch from 865e9dc to 86af97a Compare July 18, 2023 04:34

feliciss force-pushed the #457 branch from 86af97a to ab22fde Compare July 18, 2023 04:34

Feliciss added 5 commits July 18, 2023 15:49

[rooch-networkgh-457] draft Schnorr signature support.

a5368cd

[rooch-networkgh-457] remove binding from fastcrypto.

65f8536

[rooch-networkgh-457] flat code, custom traits, rename modules.

62a43c3

[rooch-networkgh-457] add schnorr scheme rust.

3afff62

[rooch-networkgh-457] introduce functions to replace from trait

6326d1c

feliciss force-pushed the #457 branch from ab22fde to 6326d1c Compare July 18, 2023 21:49

feliciss marked this pull request as ready for review July 18, 2023 23:20

[rooch-networkgh-457] add schnorr to rooch framework.

b9729e3

feliciss force-pushed the #457 branch from 740f174 to b9729e3 Compare July 18, 2023 23:21

jolestar reviewed Jul 19, 2023

View reviewed changes

[rooch-networkgh-457] resolve repo and add schnorr.move

9921453

feliciss force-pushed the #457 branch from 5f246bc to 9921453 Compare July 19, 2023 03:21

feliciss changed the title ~~[gh-457] abstract signatures and add Schnorr support.~~ [gh-457] add Schnorr signature support. Jul 19, 2023

jolestar approved these changes Jul 19, 2023

View reviewed changes

jolestar merged commit ffcbde9 into rooch-network:main Jul 19, 2023
3 checks passed

feliciss deleted the #457 branch July 19, 2023 04:11

jolestar mentioned this pull request Jul 21, 2023

[CLI] Integration move explain command to explain the error code #529

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[gh-457] add Schnorr signature support. #461

[gh-457] add Schnorr signature support. #461

feliciss commented Jul 12, 2023

vercel bot commented Jul 12, 2023

feliciss commented Jul 13, 2023

wow-sven commented Jul 16, 2023 •

edited

Loading

feliciss commented Jul 16, 2023

jolestar commented Jul 17, 2023

feliciss commented Jul 17, 2023 •

edited

Loading

feliciss commented Jul 17, 2023 •

edited

Loading

jolestar commented Jul 17, 2023

vercel bot commented Jul 18, 2023 •

edited

Loading

feliciss commented Jul 18, 2023 •

edited

Loading

feliciss commented Jul 18, 2023

jolestar Jul 19, 2023

feliciss Jul 19, 2023

jolestar Jul 19, 2023

feliciss Jul 19, 2023

[gh-457] add Schnorr signature support. #461

[gh-457] add Schnorr signature support. #461

Conversation

feliciss commented Jul 12, 2023

vercel bot commented Jul 12, 2023

feliciss commented Jul 13, 2023

wow-sven commented Jul 16, 2023 • edited Loading

feliciss commented Jul 16, 2023

jolestar commented Jul 17, 2023

feliciss commented Jul 17, 2023 • edited Loading

feliciss commented Jul 17, 2023 • edited Loading

jolestar commented Jul 17, 2023

vercel bot commented Jul 18, 2023 • edited Loading

feliciss commented Jul 18, 2023 • edited Loading

feliciss commented Jul 18, 2023

jolestar Jul 19, 2023

Choose a reason for hiding this comment

feliciss Jul 19, 2023

Choose a reason for hiding this comment

jolestar Jul 19, 2023

Choose a reason for hiding this comment

feliciss Jul 19, 2023

Choose a reason for hiding this comment

wow-sven commented Jul 16, 2023 •

edited

Loading

feliciss commented Jul 17, 2023 •

edited

Loading

feliciss commented Jul 17, 2023 •

edited

Loading

vercel bot commented Jul 18, 2023 •

edited

Loading

feliciss commented Jul 18, 2023 •

edited

Loading