Skip to content

Commit

Permalink
chore: address review
Browse files Browse the repository at this point in the history
  • Loading branch information
tbraun96 committed Dec 17, 2024
1 parent 525e183 commit 49c7194
Show file tree
Hide file tree
Showing 9 changed files with 106 additions and 137 deletions.
34 changes: 17 additions & 17 deletions crates/crypto/core/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,45 +133,45 @@ pub trait KeyType: Sized + 'static {

#[macro_export]
macro_rules! impl_crypto_tests {
($ecdsa_type:ty, $signing_key:ty, $signature:ty) => {
($crypto_type:ty, $signing_key:ty, $signature:ty) => {
use $crate::KeyType;
#[test]
fn test_key_generation() {
// Test random key generation
let secret = <$ecdsa_type>::generate_with_seed(None).unwrap();
let _public = <$ecdsa_type>::public_from_secret(&secret);
let secret = <$crypto_type>::generate_with_seed(None).unwrap();
let _public = <$crypto_type>::public_from_secret(&secret);
}

#[test]
fn test_signing_and_verification() {
let mut secret = <$ecdsa_type>::generate_with_seed(None).unwrap();
let public = <$ecdsa_type>::public_from_secret(&secret);
let mut secret = <$crypto_type>::generate_with_seed(None).unwrap();
let public = <$crypto_type>::public_from_secret(&secret);

// Test normal signing
let message = b"Hello, world!";
let signature = <$ecdsa_type>::sign_with_secret(&mut secret, message).unwrap();
let signature = <$crypto_type>::sign_with_secret(&mut secret, message).unwrap();
assert!(
<$ecdsa_type>::verify(&public, message, &signature),
<$crypto_type>::verify(&public, message, &signature),
"Signature verification failed"
);

// Test pre-hashed signing
let hashed_msg = [42u8; 32];
let signature =
<$ecdsa_type>::sign_with_secret_pre_hashed(&mut secret, &hashed_msg).unwrap();
<$crypto_type>::sign_with_secret_pre_hashed(&mut secret, &hashed_msg).unwrap();

// Verify with wrong message should fail
let wrong_message = b"Wrong message";
assert!(
!<$ecdsa_type>::verify(&public, wrong_message, &signature),
!<$crypto_type>::verify(&public, wrong_message, &signature),
"Verification should fail with wrong message"
);
}

#[test]
fn test_key_serialization() {
let secret = <$ecdsa_type>::generate_with_seed(None).unwrap();
let public = <$ecdsa_type>::public_from_secret(&secret);
let secret = <$crypto_type>::generate_with_seed(None).unwrap();
let public = <$crypto_type>::public_from_secret(&secret);

// Test signing key serialization
let serialized = serde_json::to_string(&secret).unwrap();
Expand All @@ -192,9 +192,9 @@ macro_rules! impl_crypto_tests {

#[test]
fn test_signature_serialization() {
let mut secret = <$ecdsa_type>::generate_with_seed(None).unwrap();
let mut secret = <$crypto_type>::generate_with_seed(None).unwrap();
let message = b"Test message";
let signature = <$ecdsa_type>::sign_with_secret(&mut secret, message).unwrap();
let signature = <$crypto_type>::sign_with_secret(&mut secret, message).unwrap();

// Test signature serialization
let serialized = serde_json::to_string(&signature).unwrap();
Expand All @@ -207,10 +207,10 @@ macro_rules! impl_crypto_tests {

#[test]
fn test_key_comparison() {
let secret1 = <$ecdsa_type>::generate_with_seed(None).unwrap();
let secret2 = <$ecdsa_type>::generate_with_seed(None).unwrap();
let public1 = <$ecdsa_type>::public_from_secret(&secret1);
let public2 = <$ecdsa_type>::public_from_secret(&secret2);
let secret1 = <$crypto_type>::generate_with_seed(None).unwrap();
let secret2 = <$crypto_type>::generate_with_seed(None).unwrap();
let public1 = <$crypto_type>::public_from_secret(&secret1);
let public2 = <$crypto_type>::public_from_secret(&secret2);

// Test Ord implementation
assert!(public1 != public2, "Different keys should not be equal");
Expand Down
6 changes: 3 additions & 3 deletions crates/crypto/sp-core/src/sp_core_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -205,15 +205,15 @@ macro_rules! impl_sp_core_key_type {
}
}

impl std::ops::Deref for [<$key_type Pair>] {
impl gadget_std::ops::Deref for [<$key_type Pair>] {
type Target = $pair_type;

fn deref(&self) -> &Self::Target {
&self.0
}
}

impl std::ops::DerefMut for [<$key_type Pair>] {
impl gadget_std::ops::DerefMut for [<$key_type Pair>] {
fn deref_mut(&mut self) -> &mut Self::Target {
&mut self.0
}
Expand All @@ -225,7 +225,7 @@ macro_rules! impl_sp_core_key_type {
/// Implements both pair/public and signature traits for a given sp_core crypto type
macro_rules! impl_sp_core_crypto {
($key_type:ident, $module:ident) => {
impl_sp_core_pair_public!($key_type, sp_core::$module::Pair, $module::Public);
impl_sp_core_pair_public!($key_type, sp_core::$module::Pair, sp_core::$module::Public);
impl_sp_core_signature!($key_type, sp_core::$module::Pair);
impl_sp_core_key_type!($key_type, sp_core::$module::Pair);
};
Expand Down
32 changes: 16 additions & 16 deletions crates/networking/src/gossip.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
clippy::exhaustive_enums
)]

use crate::key_types::{CryptoKeyPair, CryptoPublicKey, CryptoSignature};
use crate::key_types::{KeyPair, PublicKey, Signature};
use crate::Error;
use async_trait::async_trait;
use gadget_crypto::hashing::blake3_256;
Expand Down Expand Up @@ -46,8 +46,8 @@ pub type InboundMapping = (IdentTopic, UnboundedSender<Vec<u8>>, Arc<AtomicUsize

pub struct NetworkServiceWithoutSwarm<'a> {
pub inbound_mapping: &'a [InboundMapping],
pub crypto_peer_id_to_libp2p_id: Arc<RwLock<BTreeMap<CryptoPublicKey, PeerId>>>,
pub crypto_secret_key: &'a CryptoKeyPair,
pub public_key_to_libp2p_id: Arc<RwLock<BTreeMap<PublicKey, PeerId>>>,
pub secret_key: &'a KeyPair,
pub connected_peers: Arc<AtomicUsize>,
pub span: tracing::Span,
pub my_id: PeerId,
Expand All @@ -61,8 +61,8 @@ impl<'a> NetworkServiceWithoutSwarm<'a> {
NetworkService {
swarm,
inbound_mapping: self.inbound_mapping,
crypto_peer_id_to_libp2p_id: &self.crypto_peer_id_to_libp2p_id,
crypto_secret_key: self.crypto_secret_key,
public_key_to_libp2p_id: &self.public_key_to_libp2p_id,
secret_key: self.secret_key,
connected_peers: self.connected_peers.clone(),
span: &self.span,
my_id: self.my_id,
Expand All @@ -73,9 +73,9 @@ impl<'a> NetworkServiceWithoutSwarm<'a> {
pub struct NetworkService<'a> {
pub swarm: &'a mut libp2p::Swarm<MyBehaviour>,
pub inbound_mapping: &'a [InboundMapping],
pub crypto_peer_id_to_libp2p_id: &'a Arc<RwLock<BTreeMap<CryptoPublicKey, PeerId>>>,
pub public_key_to_libp2p_id: &'a Arc<RwLock<BTreeMap<PublicKey, PeerId>>>,
pub connected_peers: Arc<AtomicUsize>,
pub crypto_secret_key: &'a CryptoKeyPair,
pub secret_key: &'a KeyPair,
pub span: &'a tracing::Span,
pub my_id: PeerId,
}
Expand Down Expand Up @@ -265,7 +265,7 @@ pub struct GossipHandle {
pub tx_to_outbound: UnboundedSender<IntraNodePayload>,
pub rx_from_inbound: Arc<Mutex<tokio::sync::mpsc::UnboundedReceiver<Vec<u8>>>>,
pub connected_peers: Arc<AtomicUsize>,
pub crypto_peer_id_to_libp2p_id: Arc<RwLock<BTreeMap<CryptoPublicKey, PeerId>>>,
pub public_key_to_libp2p_id: Arc<RwLock<BTreeMap<PublicKey, PeerId>>>,
pub recent_messages: parking_lot::Mutex<LruCache<[u8; 32], ()>>,
pub my_id: PeerId,
}
Expand All @@ -283,8 +283,8 @@ impl GossipHandle {
}

/// Returns an ordered vector of public keys of the peers that are connected to the gossipsub topic.
pub async fn peers(&self) -> Vec<CryptoPublicKey> {
self.crypto_peer_id_to_libp2p_id
pub async fn peers(&self) -> Vec<PublicKey> {
self.public_key_to_libp2p_id
.read()
.await
.keys()
Expand Down Expand Up @@ -325,8 +325,8 @@ pub struct GossipMessage {
#[derive(Serialize, Deserialize, Debug)]
pub enum MyBehaviourRequest {
Handshake {
crypto_public_key: CryptoPublicKey,
signature: CryptoSignature,
public_key: PublicKey,
signature: Signature,
},
Message {
topic: String,
Expand All @@ -338,8 +338,8 @@ pub enum MyBehaviourRequest {
#[derive(Serialize, Deserialize, Debug)]
pub enum MyBehaviourResponse {
Handshaked {
crypto_public_key: CryptoPublicKey,
crypto_signature: CryptoSignature,
public_key: PublicKey,
signature: Signature,
},
MessageHandled,
}
Expand Down Expand Up @@ -381,11 +381,11 @@ impl Network for GossipHandle {

async fn send_message(&self, message: ProtocolMessage) -> Result<(), Error> {
let message_type = if let Some(ParticipantInfo {
crypto_public_key: Some(to),
public_key: Some(to),
..
}) = message.recipient
{
let pub_key_to_libp2p_id = self.crypto_peer_id_to_libp2p_id.read().await;
let pub_key_to_libp2p_id = self.public_key_to_libp2p_id.read().await;
gadget_logging::trace!("Handshake count: {}", pub_key_to_libp2p_id.len());
let libp2p_id = pub_key_to_libp2p_id
.get(&to)
Expand Down
14 changes: 7 additions & 7 deletions crates/networking/src/handlers/connections.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#![allow(unused_results, clippy::used_underscore_binding)]

use crate::gossip::{MyBehaviourRequest, NetworkService};
use crate::key_types::CryptoKeyCurve;
use crate::key_types::Curve;
use gadget_crypto::{hashing::blake3_256, KeyType};
use itertools::Itertools;
use libp2p::PeerId;
Expand All @@ -15,7 +15,7 @@ impl NetworkService<'_> {
) {
gadget_logging::debug!("Connection established");
if !self
.crypto_peer_id_to_libp2p_id
.public_key_to_libp2p_id
.read()
.await
.iter()
Expand All @@ -24,13 +24,13 @@ impl NetworkService<'_> {
let my_peer_id = *self.swarm.local_peer_id();
let msg = my_peer_id.to_bytes();
let hash = blake3_256(&msg);
match <CryptoKeyCurve as KeyType>::sign_with_secret_pre_hashed(
&mut self.crypto_secret_key.clone(),
match <Curve as KeyType>::sign_with_secret_pre_hashed(
&mut self.secret_key.clone(),
&hash,
) {
Ok(signature) => {
let handshake = MyBehaviourRequest::Handshake {
crypto_public_key: self.crypto_secret_key.public(),
public_key: self.secret_key.public(),
signature,
};
self.swarm
Expand Down Expand Up @@ -63,7 +63,7 @@ impl NetworkService<'_> {
.behaviour_mut()
.gossipsub
.remove_explicit_peer(&peer_id);
let mut pub_key_to_libp2p_id = self.crypto_peer_id_to_libp2p_id.write().await;
let mut pub_key_to_libp2p_id = self.public_key_to_libp2p_id.write().await;
let len_initial = 0;
pub_key_to_libp2p_id.retain(|_, id| *id != peer_id);
if pub_key_to_libp2p_id.len() == len_initial + 1 {
Expand Down Expand Up @@ -111,7 +111,7 @@ impl NetworkService<'_> {
error: libp2p::swarm::DialError,
) {
if let libp2p::swarm::DialError::Transport(addrs) = error {
let read = self.crypto_peer_id_to_libp2p_id.read().await;
let read = self.public_key_to_libp2p_id.read().await;
for (addr, err) in addrs {
if let Some(peer_id) = get_peer_id_from_multiaddr(&addr) {
if !read.values().contains(&peer_id) {
Expand Down
37 changes: 16 additions & 21 deletions crates/networking/src/handlers/p2p.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#![allow(unused_results)]

use crate::gossip::{MyBehaviourRequest, MyBehaviourResponse, NetworkService};
use crate::key_types::CryptoKeyCurve;
use crate::key_types::Curve;
use gadget_crypto::hashing::blake3_256;
use gadget_crypto::KeyType;
use gadget_std::string::ToString;
Expand Down Expand Up @@ -85,25 +85,24 @@ impl NetworkService<'_> {
use crate::gossip::MyBehaviourRequest::{Handshake, Message};
let result = match req {
Handshake {
crypto_public_key,
public_key,
signature,
} => {
gadget_logging::trace!("Received handshake from peer: {peer}");
// Verify the signature
let msg = peer.to_bytes();
let hash = blake3_256(&msg);
let valid =
<CryptoKeyCurve as KeyType>::verify(&crypto_public_key, &hash, &signature);
let valid = <Curve as KeyType>::verify(&public_key, &hash, &signature);
if !valid {
gadget_logging::warn!("Invalid signature from peer: {peer}");
let _ = self.swarm.disconnect_peer_id(peer);
return;
}
if self
.crypto_peer_id_to_libp2p_id
.public_key_to_libp2p_id
.write()
.await
.insert(crypto_public_key, peer)
.insert(public_key, peer)
.is_none()
{
let _ = self.connected_peers.fetch_add(1, Ordering::Relaxed);
Expand All @@ -112,15 +111,15 @@ impl NetworkService<'_> {
let my_peer_id = self.swarm.local_peer_id();
let msg = my_peer_id.to_bytes();
let hash = blake3_256(&msg);
match <CryptoKeyCurve as KeyType>::sign_with_secret_pre_hashed(
&mut self.crypto_secret_key.clone(),
match <Curve as KeyType>::sign_with_secret_pre_hashed(
&mut self.secret_key.clone(),
&hash,
) {
Ok(signature) => self.swarm.behaviour_mut().p2p.send_response(
channel,
MyBehaviourResponse::Handshaked {
crypto_public_key: self.crypto_secret_key.public(),
crypto_signature: signature,
public_key: self.secret_key.public(),
signature,
},
),
Err(e) => {
Expand Down Expand Up @@ -168,32 +167,28 @@ impl NetworkService<'_> {
use crate::gossip::MyBehaviourResponse::{Handshaked, MessageHandled};
match message {
Handshaked {
crypto_public_key,
crypto_signature,
public_key,
signature,
} => {
gadget_logging::trace!("Received handshake-ack message from peer: {peer}");
let msg = peer.to_bytes();
let hash = blake3_256(&msg);
let valid = <CryptoKeyCurve as KeyType>::verify(
&crypto_public_key,
&hash,
&crypto_signature,
);
let valid = <Curve as KeyType>::verify(&public_key, &hash, &signature);
if !valid {
gadget_logging::warn!("Invalid signature from peer: {peer}");
// TODO: report this peer.
self.crypto_peer_id_to_libp2p_id
self.public_key_to_libp2p_id
.write()
.await
.remove(&crypto_public_key);
.remove(&public_key);
let _ = self.swarm.disconnect_peer_id(peer);
return;
}
if self
.crypto_peer_id_to_libp2p_id
.public_key_to_libp2p_id
.write()
.await
.insert(crypto_public_key, peer)
.insert(public_key, peer)
.is_none()
{
let _ = self.connected_peers.fetch_add(1, Ordering::Relaxed);
Expand Down
Loading

0 comments on commit 49c7194

Please sign in to comment.