chain/indexer/src/block.rs (4)

153-157: Consider gracefully handling RPC update failures.
Calling Self::update_chain_head can cause a panic on RPC failure. Depending on production requirements, you may want to catch and retry rather than terminating the process.

---

167-172: Enum usage for fast sync modes is clear.
The FastSyncMode enum helps make the fast sync states explicit. Consider adding doc comments or deriving Debug to facilitate future logging and debugging.

---

203-231: Conditional fast-sync block is well-structured but watch performance.
The loop processing each block in log_block_numbers is synchronous. For large ranges, this might block other tasks. If performance becomes a concern, consider batching or chunking to allow partial yields.

---

255-255: Redundant logging comment.
This commented line repeats the log's purpose. Consider removing or consolidating the comment to reduce clutter.

METRICS.md (1)

69-69: Minor punctuation fix.
The line defining hopr_indexer_data_source has a trailing colon after the backticks, which may trigger a style or punctuation alert. Consider refining as follows for clarity:

- - `hopr_indexer_data_source`: Current data source of the Indexer, keys: `source`
+ - `hopr_indexer_data_source` - Current data source of the Indexer, keys: `source`

🧰 Tools

🪛 LanguageTool

[uncategorized] ~69-~69: Loose punctuation mark.
Context: ...r checksum. - hopr_indexer_data_source: Current data source of the Indexer, key...

(UNLIKELY_OPENING_PUNCTUATION)

transport/protocol/src/heartbeat/config.rs (1)

13-13: Verify if 6 seconds is sufficient for heartbeat timeout.

Reducing the timeout from 15s to 6s will help detect node unavailability faster but could lead to more frequent retries and increased network traffic if the timeout is too aggressive. Please ensure that 6 seconds is sufficient for typical network conditions and latency between nodes.

Consider the following factors:

• Network latency between nodes in different geographical locations
• Impact on nodes behind NAT or with poor connectivity
• Potential increase in network traffic due to more frequent retries

transport/network/src/constants.rs (1)

13-13: Verify system resources can handle increased parallel pings.

Increasing DEFAULT_MAX_PARALLEL_PINGS from 14 to 25 will allow more concurrent network operations, potentially improving responsiveness. However, this also increases resource usage (network sockets, memory, CPU).

Consider the following factors:

• Impact on system resources (memory, CPU, network sockets)
• Network bandwidth consumption with more concurrent pings
• Potential for network congestion or rate limiting

transport/p2p/src/constants.rs (2)

2-2: Verify impact of reduced idle connection timeout.

Reducing the idle timeout from 10 minutes to 5 minutes will free up resources sooner but might cause more frequent reconnections. Please ensure this doesn't negatively impact network stability.

Consider monitoring:

• Frequency of reconnections
• Impact on network stability
• Resource usage patterns

---

14-14: Verify system capacity for increased concurrent peer negotiations.

Doubling the concurrent inbound peer count from 255 to 512 significantly increases the system's peer handling capacity but also increases resource requirements.

Consider:

• Memory usage per peer negotiation
• System limits on file descriptors
• Impact on other concurrent operations

transport/protocol/src/config.rs (2)

10-12: Ensure proper handling of None case for winning probability.

The change to make outgoing_ticket_winning_prob optional is good for flexibility, but ensure that the code handling this value properly falls back to the network value when None.

Consider:

• Default value handling in all code paths
• Error handling for invalid network values
• Documentation of fallback behavior

---

13-14: Verify validation of optional ticket price.

The new outgoing_ticket_price field allows overriding network prices, but ensure proper validation of the price values to prevent potential issues.

Consider adding:

• Validation for minimum/maximum price bounds
• Documentation of price override behavior
• Logging when price override is used

transport/protocol/src/msg/processor.rs (1)

213-230: Consider adding error logging for network win probability fallback.

While the fallback logic is sound, consider adding error logging when falling back to the default probability to help with debugging.

 async fn determine_actual_outgoing_win_prob(&self) -> f64 {
     let network_win_prob = self
         .db
         .get_network_winning_probability()
         .await
         .inspect_err(|error| error!(%error, "failed to determine current network winning probability"))
         .ok();

+    if network_win_prob.is_none() {
+        warn!("Using default winning probability due to missing network value");
+    }

     self.cfg
         .outgoing_ticket_win_prob
         .or(network_win_prob)
         .unwrap_or(DEFAULT_OUTGOING_TICKET_WIN_PROB)
 }

hoprd/rest-api/src/alias.rs (2)

112-147: Consider batching database queries for better performance.

The current implementation makes individual database queries for each alias. Consider batching these queries to reduce database load.

 pub(super) async fn aliases_addresses(State(state): State<Arc<InternalState>>) -> impl IntoResponse {
     let aliases = state.hoprd_db.get_aliases().await;

     match aliases {
         Ok(aliases) => {
-            let aliases = aliases
-                .into_iter()
-                .map(|alias| (alias.alias.clone(), alias.peer_id.clone()))
-                .collect::<HashMap<_, _>>();
+            let peer_ids: Vec<_> = aliases.iter().map(|alias| alias.peer_id.clone()).collect();
+            let addresses_future = state.hopr.peer_resolver().batch_resolve_addresses(peer_ids);
+            let addresses = addresses_future.await?;
+            
+            let aliases_addresses = aliases.into_iter()
+                .zip(addresses)
+                .map(|(alias, address)| (alias.alias, address.to_string()))
+                .collect::<HashMap<_, _>>();

-            let aliases_addresses_futures = aliases.into_iter().map(|(alias, peer_id)| {
-                let hopr = state.hopr.clone();
-                async move {
-                    let peer_or_address = match PeerOrAddress::from_str(peer_id.as_str()) {
-                        Ok(destination) => destination,
-                        Err(_) => return (alias, ApiErrorStatus::PeerNotFound.to_string()),
-                    };
-
-                    match HoprIdentifier::new_with(peer_or_address, hopr.peer_resolver()).await {
-                        Ok(destination) => (alias, destination.address.to_string()),
-                        Err(_) => (alias, ApiErrorStatus::PeerNotFound.to_string()),
-                    }
-                }
-            });
-
-            let aliases_addresses = futures::future::join_all(aliases_addresses_futures)
-                .await
-                .into_iter()
-                .collect::<HashMap<_, _>>();

             (StatusCode::OK, Json(aliases_addresses)).into_response()
         }
         Err(DbError::BackendError(_)) => (StatusCode::NOT_FOUND, ApiErrorStatus::AliasNotFound).into_response(),
         Err(_) => (StatusCode::INTERNAL_SERVER_ERROR, ApiErrorStatus::DatabaseError).into_response(),
     }
 }

---

272-307: Consider adding rate limiting for address resolution.

The endpoint performs address resolution which could be resource-intensive. Consider adding rate limiting to prevent abuse.

transport/network/src/ping.rs (2)

63-63: Consider documenting the reason for increasing max_parallel_pings.

The default value for max_parallel_pings has been increased from 14 to 25. While this change might be intentional, it would be helpful to document the reasoning behind this increase.

-    #[default = 25]
+    /// Default value increased from 14 to 25 to improve network throughput
+    #[default = 25]

---

573-573: Consider reducing the sleep duration.

The sleep duration of 0.5 seconds in the URL check might be unnecessarily long. Consider reducing it to improve responsiveness.

-                        await asyncio.sleep(0.5)
+                        await asyncio.sleep(0.1)

sdk/python/api/hopr.py (1)

166-166: Remove extraneous f-string prefix.

The f-string prefix is unnecessary as there are no placeholders in the string.

-        endpoint = f"aliases_addresses" if return_address else f"aliases"
+        endpoint = "aliases_addresses" if return_address else "aliases"

🧰 Tools

🪛 Ruff (0.8.2)

166-166: f-string without any placeholders

Remove extraneous f prefix

(F541)

---

166-166: f-string without any placeholders

Remove extraneous f prefix

(F541)

db/sql/src/protocol.rs (1)

341-347: Consider adding documentation for ticket price calculation.

The ticket price calculation logic has been updated, but the comments could be more detailed about the formula and its implications.

-            // The ticket price from the oracle times my node's position on the
-            // path is the acceptable minimum
+            // The minimum acceptable ticket price is calculated as:
+            // oracle_ticket_price * path_position
+            // This ensures that nodes further along the path receive proportionally higher compensation

hopli/README.md (2)

257-264: Add language specifiers to code blocks and secure key handling note.

The environment variables section should:

1. Use language specifiers for code blocks
2. Include a security note about private key handling

Apply these changes:

-```
+```bash
 export PATH="./target/release:${PATH}"
 export RUST_BACKTRACE=full
 export HOPRD_NETWORK=rotsee
 export IDENTITY_PASSWORD=<your-secure-password-here> # Replace with your own secure password
 export PRIVATE_KEY=0x0000000000000000000000000000000000000000000000000000000000000000 # Replace with Deployer Private Key
 export MANAGER_PRIVATE_KEY=0x0000000000000000000000000000000000000000000000000000000000000000 # Replace with Manager Private Key

+> Security Note: Never commit private keys to version control. Use environment files or secure key management systems in production.

<details>
<summary>🧰 Tools</summary>

<details>
<summary>🪛 markdownlint-cli2 (0.17.2)</summary>

257-257: Fenced code blocks should have a language specified
null

(MD040, fenced-code-language)

</details>

</details>

---

`270-272`: **Add language specifiers to command code blocks.**

The command examples should include language specifiers for better syntax highlighting.

Apply these changes:

```diff
-```
+```bash
 hopli identity create --identity-directory "./new_identities" --identity-prefix nodes_ --number 1

- +bash
hopli safe-module create --network ${HOPRD_NETWORK} --contracts-root "./ethereum/contracts" --provider-url https://gnosis.rpc-provider.prod.hoprnet.link --identity-directory "./new_identities" --allowance 10.5 --hopr-amount 1 --native-amount 0.1

-```
+```bash
hopli identity read --identity-directory "./new_identities" --identity-prefix nodes_

Also applies to: 276-278, 282-284

<details>
<summary>🧰 Tools</summary>

<details>
<summary>🪛 markdownlint-cli2 (0.17.2)</summary>

270-270: Fenced code blocks should have a language specified
null

(MD040, fenced-code-language)

</details>

</details>

</blockquote></details>

</blockquote></details>

<details>
<summary>🛑 Comments failed to post (3)</summary><blockquote>

<details>
<summary>scripts/generate-python-sdk.sh (1)</summary><blockquote>

22-22: _:warning: Potential issue_

**Fix potential security issue in rm command.**

The `rm -rf "${TARGET_DIR}"/*` command could be dangerous if `TARGET_DIR` is empty or unset, despite the `-u` option in `set`. Use `"${TARGET_DIR:?}"/*` to ensure safer execution.

```diff
-rm -rf "${TARGET_DIR}"/*
+rm -rf "${TARGET_DIR:?}"/*

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

rm -rf "${TARGET_DIR:?}"/*

🧰 Tools

🪛 Shellcheck (0.10.0)

[warning] 22-22: Use "${var:?}" to ensure this never expands to /* .

(SC2115)

deploy/compose/hoprd_data/hoprd.cfg.yaml (1)

18-18: 💡 Verification agent

🧩 Analysis chain

LGTM! Property renamed for better clarity.

The property on_close_redeem_single_tickets_value_min has been renamed to minimum_redeem_ticket_value for better clarity.

Run the following script to check for any remaining usages of the old property name:

---

🏁 Script executed:

#!/bin/bash
# Description: Check for any remaining usages of the old property name.
rg -i "on_close_redeem_single_tickets_value_min"

Length of output: 178

---

Config update approved, but update legacy comment for consistency

• The HOPR configuration file (deploy/compose/hoprd_data/hoprd.cfg.yaml) has been updated correctly: the property has been renamed from on_close_redeem_single_tickets_value_min to minimum_redeem_ticket_value, which improves clarity.
• A legacy reference to the old property still exists in a comment in logic/strategy/src/auto_redeeming.rs ("This is not used for cases where on_close_redeem_single_tickets_value_min applies."). To avoid confusion, please update or remove this comment to reflect the new naming.

deploy/compose/README.md (1)

65-68: 🛠️ Refactor suggestion

Specify safe port range for GnosisVPN sessions.

While the documentation explains how to add UDP ports for GnosisVPN sessions, it should specify the safe port range to use (outside ephemeral range) to align with the PR objective.

Add a note about the safe port range:

- For example: `11111:11111/udp`
+ For example: `1422:1422/udp`
+
+ Note: Use ports in the range 1422-1429 for GnosisVPN sessions to avoid conflicts with ephemeral ports (typically 32768-60999 on Linux).

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

To test GnosisVPN, you need to have an open session with a node. Follow these steps to make the necessary changes:

- `services.hoprd.ports`: Add an additional UDP port under the ports variable to open a session. For example: `1422:1422/udp`

  Note: Use ports in the range 1422-1429 for GnosisVPN sessions to avoid conflicts with ephemeral ports (typically 32768-60999 on Linux).

🧰 Tools

🪛 LanguageTool

[uncategorized] ~67-~67: Loose punctuation mark.
Context: ...ssary changes: - services.hoprd.ports: Add an additional UDP port under the po...

(UNLIKELY_OPENING_PUNCTUATION)

---

[style] ~67-~67: This phrase might be redundant. Consider either removing or replacing the adjective ‘additional’.
Context: ...ary changes: - services.hoprd.ports: Add an additional UDP port under the ports variable to open a...

(ADD_AN_ADDITIONAL)