fix: fixing review comments

Signed-off-by: Simon Paitrault <[email protected]>

crates/topos-p2p/src/behaviour/gossip.rs

@@ -255,7 +255,7 @@ impl NetworkBehaviour for Behaviour {
                     _ => {}
                 },
                 gossipsub::Event::Subscribed { peer_id, topic } => {
-                    debug!("Subscribed to {:?} with {peer_id}", topic);
+                    debug!("{peer_id} subscribed to {:?}", topic);
 
                     // If the behaviour isn't already healthy we check if this event
                     // triggers a switch to healthy
@@ -268,7 +268,7 @@ impl NetworkBehaviour for Behaviour {
                     }
                 }
                 gossipsub::Event::Unsubscribed { peer_id, topic } => {
-                    debug!("Unsubscribed from {:?} with {peer_id}", topic);
+                    debug!("{peer_id} unsubscribed from {:?}", topic);
                 }
                 gossipsub::Event::GossipsubNotSupported { peer_id } => {
                     debug!("Gossipsub not supported by {:?}", peer_id);

crates/topos-p2p/src/runtime/handle_event.rs

@@ -193,6 +193,7 @@ impl EventHandler<SwarmEvent<ComposedEvent>> for Runtime {
         let behaviour = self.swarm.behaviour();
 
         if let Some(event) = self.healthy_status_changed() {
+            debug!("Healthy status changed: {:?}", event);
             _ = self.event_sender.send(event).await;
         }
 

crates/topos-tce-api/src/graphql/builder.rs

@@ -62,7 +62,7 @@ impl ServerBuilder {
             .take()
             .expect("Cannot build GraphQL server without a FullNode store");
 
-        let fullnode_store = store.get_fullnode_store();
+        let fullnode_store = store.fullnode_store();
         let runtime = self
             .runtime
             .take()

crates/topos-tce-broadcast/src/tests/task.rs

@@ -1,4 +1,4 @@
-use std::{future::IntoFuture, str::FromStr, sync::Arc, time::Duration};
+use std::{future::IntoFuture, sync::Arc, time::Duration};
 
 use rstest::rstest;
 use tokio::{
@@ -11,6 +11,7 @@ use topos_tce_storage::validator::ValidatorStore;
 use topos_test_sdk::{
     certificates::create_certificate_chain,
     constants::{SOURCE_SUBNET_ID_1, TARGET_SUBNET_ID_1},
+    crypto::message_signer,
     storage::create_validator_store,
 };
 
@@ -26,6 +27,7 @@ async fn start_with_ungossiped_cert(
     #[future(awt)]
     #[from(create_validator_store)]
     validatore_store: Arc<ValidatorStore>,
+    message_signer: Arc<MessageSigner>,
 ) {
     let certificate = create_certificate_chain(SOURCE_SUBNET_ID_1, &[TARGET_SUBNET_ID_1], 1)
         .pop()
@@ -40,10 +42,6 @@ async fn start_with_ungossiped_cert(
     };
     let (event_sender, mut event_receiver) = mpsc::channel(2);
     let (broadcast_sender, _) = broadcast::channel(1);
-    let message_signer = Arc::new(
-        MessageSigner::from_str("122f3ae6ade1fd136b292cea4f6243c7811160352c8821528547a1fe7c459daf")
-            .unwrap(),
-    );
     let need_gossip = true;
     let subscriptions = SubscriptionsView::default();
 

crates/topos-tce-broadcast/src/tests/task_manager.rs

@@ -1,4 +1,4 @@
-use std::{str::FromStr, sync::Arc};
+use std::sync::Arc;
 
 use rstest::rstest;
 use tokio::{
@@ -12,6 +12,7 @@ use topos_tce_storage::validator::ValidatorStore;
 use topos_test_sdk::{
     certificates::create_certificate_chain,
     constants::{SOURCE_SUBNET_ID_1, TARGET_SUBNET_ID_1},
+    crypto::message_signer,
     storage::create_validator_store,
 };
 
@@ -23,6 +24,7 @@ async fn can_start(
     #[future(awt)]
     #[from(create_validator_store)]
     validator_store: Arc<ValidatorStore>,
+    message_signer: Arc<MessageSigner>,
 ) {
     let (message_sender, message_receiver) = mpsc::channel(1);
     let (event_sender, _) = mpsc::channel(1);
@@ -35,11 +37,6 @@ async fn can_start(
         delivery_threshold: 1,
     };
 
-    let message_signer = Arc::new(
-        MessageSigner::from_str("122f3ae6ade1fd136b292cea4f6243c7811160352c8821528547a1fe7c459daf")
-            .unwrap(),
-    );
-
     let manager = TaskManager::new(
         message_receiver,
         SubscriptionsView::default(),

crates/topos-tce-storage/src/fullnode/mod.rs

@@ -85,7 +85,7 @@ impl FullNodeStore {
     }
 
     /// Await for a [`LockGuards`] for the given certificate id
-    pub(crate) async fn get_certificate_lock_guard(
+    pub(crate) async fn certificate_lock_guard(
         &self,
         certificate_id: CertificateId,
     ) -> OwnedMutexGuard<()> {
@@ -97,7 +97,7 @@ impl FullNodeStore {
     }
 
     /// Await for a [`LockGuards`] for the given subnet id
-    pub(crate) async fn get_subnet_lock_guard(&self, subnet_id: SubnetId) -> OwnedMutexGuard<()> {
+    pub(crate) async fn subnet_lock_guard(&self, subnet_id: SubnetId) -> OwnedMutexGuard<()> {
         self.subnet_lock_guards
             .get_lock(subnet_id)
             .await
@@ -114,11 +114,11 @@ impl WriteStore for FullNodeStore {
     ) -> Result<CertificatePositions, StorageError> {
         // Lock resources for concurrency issues
         let _cert_guard = self
-            .get_certificate_lock_guard(certificate.certificate.id)
+            .certificate_lock_guard(certificate.certificate.id)
             .await;
 
         let _subnet_guard = self
-            .get_subnet_lock_guard(certificate.certificate.source_subnet_id)
+            .subnet_lock_guard(certificate.certificate.source_subnet_id)
             .await;
 
         let subnet_id = certificate.certificate.source_subnet_id;

crates/topos-tce-storage/src/tests/pending_certificates.rs

@@ -91,7 +91,7 @@ async fn adding_pending_certificate_already_delivered(store: Arc<ValidatorStore>
         .is_err());
 }
 
-/// This test is covering a corner case which involve the delivery of a prev certificate
+/// This test is covering a corner case which involves the delivery of a prev certificate
 /// and a child certificate.
 ///
 /// The scenario is this one:
@@ -123,7 +123,7 @@ mod concurrency {
         // The lock guard simulate the start of the certificate insertion in the table.
         let lock_guard_certificate = store
             .fullnode_store
-            .get_certificate_lock_guard(parent.certificate.id)
+            .certificate_lock_guard(parent.certificate.id)
             .await;
 
         tokio::spawn(async move {
@@ -149,7 +149,7 @@ mod concurrency {
         // The lock guard simulate the start of the certificate insertion in the table.
         let lock_guard_subnet = store
             .fullnode_store
-            .get_subnet_lock_guard(cert.certificate.source_subnet_id)
+            .subnet_lock_guard(cert.certificate.source_subnet_id)
             .await;
 
         tokio::spawn(async move {
@@ -188,7 +188,7 @@ mod concurrency {
         // The lock guard simulate the start of the certificate insertion in the table.
         let lock_guard_subnet = store
             .fullnode_store
-            .get_subnet_lock_guard(cert.certificate.source_subnet_id)
+            .subnet_lock_guard(cert.certificate.source_subnet_id)
             .await;
 
         tokio::spawn(async move {

crates/topos-tce-storage/src/validator/mod.rs

@@ -119,7 +119,7 @@ impl ValidatorStore {
     }
 
     /// Returns the [`FullNodeStore`] used by the [`ValidatorStore`]
-    pub fn get_fullnode_store(&self) -> Arc<FullNodeStore> {
+    pub fn fullnode_store(&self) -> Arc<FullNodeStore> {
         self.fullnode_store.clone()
     }
 
@@ -290,12 +290,12 @@ impl ValidatorStore {
         &self,
         certificate: &Certificate,
     ) -> Result<Option<PendingCertificateId>, StorageError> {
-        // A lock guard is asked during the insertion of a pending certificate
-        // to avoid race condition when a certificate is being inserted and added
-        // to the pending pool at the same time
+        // A lock guard is taken during the insertion of a pending certificate (C1)
+        // to avoid race condition when this certificate C1 is delivered by the network
+        // and in the process of being inserted into the precedence tables.
         let _certificate_guard = self
             .fullnode_store
-            .get_certificate_lock_guard(certificate.id)
+            .certificate_lock_guard(certificate.id)
             .await;
 
         if self.get_certificate(&certificate.id)?.is_some() {
@@ -320,12 +320,12 @@ impl ValidatorStore {
             ));
         }
 
-        // A lock guard is asked during the insertion of a pending certificate
+        // A lock guard is taken during the insertion of a pending certificate
         // to avoid race condition when a certificate is being added to the
         // pending pool while its parent is currently being inserted as delivered
         let _prev_certificate_guard = self
             .fullnode_store
-            .get_certificate_lock_guard(certificate.prev_id)
+            .certificate_lock_guard(certificate.prev_id)
             .await;
 
         let prev_delivered = certificate.prev_id == INITIAL_CERTIFICATE_ID

crates/topos-tce/src/lib.rs

@@ -107,7 +107,7 @@ pub async fn run(
     let validator_store = ValidatorStore::new(path)
         .map_err(|error| format!("Unable to create validator store: {error}"))?;
 
-    let fullnode_store = validator_store.get_fullnode_store();
+    let fullnode_store = validator_store.fullnode_store();
 
     let storage_client = StorageClient::new(validator_store.clone());
 

crates/topos-test-sdk/src/crypto.rs

@@ -0,0 +1,9 @@
+use std::{str::FromStr, sync::Arc};
+
+use rstest::fixture;
+use topos_crypto::messages::MessageSigner;
+
+#[fixture(key = "122f3ae6ade1fd136b292cea4f6243c7811160352c8821528547a1fe7c459daf")]
+pub fn message_signer(key: &str) -> Arc<MessageSigner> {
+    Arc::new(MessageSigner::from_str(key).unwrap())
+}

crates/topos-test-sdk/src/lib.rs

@@ -1,5 +1,6 @@
 pub mod certificates;
 
+pub mod crypto;
 pub mod networking;
 pub mod p2p;
 pub mod sequencer;

crates/topos-test-sdk/src/tce/mod.rs

@@ -44,6 +44,7 @@ use self::p2p::{bootstrap_network, create_network_worker};
 use self::protocol::{create_reliable_broadcast_client, create_reliable_broadcast_params};
 use self::public_api::create_public_api;
 use self::synchronizer::create_synchronizer;
+use crate::crypto::message_signer;
 use crate::p2p::local_peer;
 use crate::storage::create_fullnode_store;
 use crate::storage::create_validator_store;
@@ -174,10 +175,6 @@ impl NodeConfig {
     }
 }
 
-fn default_message_signer() -> Arc<MessageSigner> {
-    Arc::new(MessageSigner::new(&[5u8; 32]).unwrap())
-}
-
 #[derive(Clone)]
 struct DummyService {}
 
@@ -207,9 +204,8 @@ pub fn create_dummy_router() -> Router {
     peers = &[],
     certificates = &[],
     validator_id = ValidatorId::default(),
-    validators = HashSet::default(),
-    message_signer = default_message_signer())
-]
+    validators = HashSet::default()
+)]
 pub async fn start_node(
     certificates: &[CertificateDelivered],
     config: NodeConfig,