db/sql/src/db.rs (2)

Line range hint 237-272: Test could be more comprehensive

While the test verifies that old peers are discarded, it could be improved by:

1. Explicitly setting the environment variable to test configuration
2. Verifying the exact cleanup conditions

Consider enhancing the test:

     async fn peers_without_any_recent_updates_should_be_discarded_on_restarts() -> anyhow::Result<()> {
+        std::env::set_var(
+            "HOPR_DEFAULT_PEERS_DB_PERSISTENCE_AFTER_RESTART_IN_SECONDS",
+            "300" // 5 minutes
+        );
+
         let random_filename: String = rand::thread_rng()
             .sample_iter(&Alphanumeric)
             .take(15)
             .map(char::from)
             .collect();
         let random_tmp_file = format!("/tmp/{random_filename}.sqlite");

         let peer_id: PeerId = OffchainKeypair::random().public().into();
         let ma_1: Multiaddr = format!("/ip4/127.0.0.1/tcp/10000/p2p/{peer_id}").parse()?;
         let ma_2: Multiaddr = format!("/ip4/127.0.0.1/tcp/10002/p2p/{peer_id}").parse()?;

         let path = std::path::Path::new(&random_tmp_file);

         {
             let db = HoprDb::new(&path, ChainKeypair::random(), crate::db::HoprDbConfig::default()).await?;

+            // Add peer with last_seen more than 5 minutes ago
+            let old_time = std::time::SystemTime::now() - std::time::Duration::from_secs(360);
             db.add_network_peer(
                 &peer_id,
                 PeerOrigin::IncomingConnection,
                 vec![ma_1.clone(), ma_2.clone()],
                 0.0,
                 25,
-            )
-            .await?;
+            ).await?;
+            
+            db.update_network_peer(hopr_db_api::peers::PeerStatus {
+                last_seen: old_time,
+                // ... other fields ...
+            }).await?;
         }

---

274-330: Test implementation looks good but could use cleanup

The test thoroughly verifies that recent peers are retained, but there are a few improvements possible:

1. Common setup code could be extracted
2. Cleanup of temporary files is missing

Consider adding cleanup and extracting common code:

+    fn setup_test_db() -> (String, PeerId, Multiaddr, Multiaddr) {
+        let random_filename: String = rand::thread_rng()
+            .sample_iter(&Alphanumeric)
+            .take(15)
+            .map(char::from)
+            .collect();
+        let random_tmp_file = format!("/tmp/{random_filename}.sqlite");
+
+        let ofk = OffchainKeypair::random();
+        let peer_id: PeerId = ofk.public().clone().into();
+        let ma_1: Multiaddr = format!("/ip4/127.0.0.1/tcp/10000/p2p/{peer_id}").parse().unwrap();
+        let ma_2: Multiaddr = format!("/ip4/127.0.0.1/tcp/10002/p2p/{peer_id}").parse().unwrap();
+
+        (random_tmp_file, peer_id, ma_1, ma_2)
+    }

     #[async_std::test]
     async fn peers_with_a_recent_update_should_be_retained_in_the_database() -> anyhow::Result<()> {
-        let random_filename: String = rand::thread_rng()
-            .sample_iter(&Alphanumeric)
-            .take(15)
-            .map(char::from)
-            .collect();
-        let random_tmp_file = format!("/tmp/{random_filename}.sqlite");
-
-        let ofk = OffchainKeypair::random();
-        let peer_id: PeerId = ofk.public().clone().into();
-        let ma_1: Multiaddr = format!("/ip4/127.0.0.1/tcp/10000/p2p/{peer_id}").parse()?;
-        let ma_2: Multiaddr = format!("/ip4/127.0.0.1/tcp/10002/p2p/{peer_id}").parse()?;
+        let (random_tmp_file, peer_id, ma_1, ma_2) = setup_test_db();
         let path = std::path::Path::new(&random_tmp_file);

+        defer! {
+            std::fs::remove_file(&random_tmp_file).ok();
+        }

         // ... rest of the test ...
     }

README.md (2)

214-214: Consider adding an example value to the environment variable documentation.

The description is clear, but adding an example value (e.g. "300" for 5 minutes) would make it more user-friendly.

- - `HOPR_DEFAULT_PEERS_DB_PERSISTENCE_AFTER_RESTART_IN_SECONDS` - cutoff duration from now to not retain the peers with older records in the peers database (e.g. after a restart)
+ - `HOPR_DEFAULT_PEERS_DB_PERSISTENCE_AFTER_RESTART_IN_SECONDS` - cutoff duration from now to not retain the peers with older records in the peers database (e.g. after a restart). Example: "300" for 5 minutes

---

Line range hint 1-24: Fix typos in documentation.

There are two typos that need to be corrected:

- The HOPR project produces multiple artifacts that allow running, maintaining and modiyfing the HOPR node.
+ The HOPR project produces multiple artifacts that allow running, maintaining and modifying the HOPR node.

- hoprd --identity /app/hoprd-db/.hopr-identity --password switzerland --init --announce --host "0.0.0.0:9091" --apiToken <MY_TOKEN> --network doufur
+ hoprd --identity /app/hoprd-db/.hopr-identity --password switzerland --init --announce --host "0.0.0.0:9091" --apiToken <MY_TOKEN> --network dufour

Configuration used: .coderabbit.yaml
Review profile: CHILL

#!/bin/bash
# Description: Check for potential circular dependencies between hopr-db-sql and hopr-platform

# Check if hopr-platform depends on hopr-db-sql
echo "Checking for circular dependencies..."
rg -l "hopr-db-sql.*workspace.*true" $(fd Cargo.toml)

# Check the dependency tree in hopr-platform
fd "Cargo.toml" | grep "platform" | xargs cat | grep "dependencies" -A 20

#!/bin/bash
# Let's check if hopr-platform depends on hopr-db-sql
rg -p "hopr-db-sql" misc/platform/Cargo.toml

# Also check if there are any other dependencies between these crates
rg -p "hopr-platform.*=.*workspace.*true" db/sql/Cargo.toml
rg -p "hopr-db-sql.*=.*workspace.*true" misc/platform/Cargo.toml

#!/bin/bash
# Check for other usages of the environment variable
echo "Checking for environment variable usage..."
rg "HOPR_.*DB_PEERS_PERSISTENCE_AFTER_RESTART" -l

# Check for potential peer connectivity related code
echo "Checking for peer connectivity related code..."
rg "add_network_peer|update_network_peer|get_network_peer" -l

# Check for related configuration or documentation
echo "Checking for related configuration..."
fd -t f "README|config|settings" -x rg -l "peer.*persistence|restart.*peer"

#!/bin/bash
# Let's examine the peer-related code more closely
echo "Checking peer-related implementations..."
rg -A 10 "add_network_peer|update_network_peer|get_network_peer" db/sql/src/peers.rs db/sql/src/db.rs

# Check for the default constant usage
echo "Checking default constant usage..."
rg "DEFAULT_PEERS_DB_PERSISTENCE_AFTER_RESTART" -l

# Check environment variable documentation
echo "Checking environment configuration documentation..."
fd README -x cat {} \;

#!/bin/bash
# Let's check the default constant value and its usage
echo "Checking DEFAULT_PEERS_DB_PERSISTENCE_AFTER_RESTART_IN_SECONDS constant..."
rg "DEFAULT_PEERS_DB_PERSISTENCE_AFTER_RESTART_IN_SECONDS" -B 2 -A 2

# Check the tests related to peer persistence
echo "Checking peer persistence tests..."
rg "peers.*persist|persist.*peer" -B 2 -A 2 db/sql/src/db.rs

# Check the implementation of network peer cleanup
echo "Checking network peer cleanup implementation..."
rg "clean.*peer|remove.*peer" -B 2 -A 2 db/sql/src/db.rs