chain/indexer/src/block.rs:391

• generate_log_filters is a synchronous function and cannot .await. Additionally, db is not defined in this scope. Move this call into an async context where db is available or remove it.

db.ensure_logs_origin(address_topics).await?;

chain/types/src/lib.rs:2

• [nitpick] It looks like FromStr and Arc are unused in this file. Removing unused imports will keep the module clean.

use std::{str::FromStr, sync::Arc};

chain/rpc/src/indexer.rs:324

• The test uses &ranges[0].iter().for_each(...) with a ? inside an iterator, which will not compile. Rework this assertion to iterate and assert without using ? or an unnecessary &.

&ranges[0].iter().for_each(|x| assert_eq!((0, 1), filter_bounds(&x)?));

chain/rpc/src/indexer.rs (1)

332-350: ⚠️ Potential issue

Fix test code that still uses the old LogFilter API.

The test still references LogFilter::default() which should be updated to use the new filter API.

-        let ranges = split_range(LogFilter::default(), 0, 0, 2).collect::<Vec<_>>().await;
+        let filters = vec![Filter::default()];
+        let ranges = split_range(filters.clone(), 0, 0, 2).collect::<Vec<_>>().await;
         assert_eq!(1, ranges.len());
-        assert_eq!((0, 0), filter_bounds(&ranges[0])?);
+        assert_eq!((0, 0), filter_bounds(&ranges[0][0])?);

-        let ranges = split_range(LogFilter::default(), 0, 0, 1).collect::<Vec<_>>().await;
+        let ranges = split_range(filters.clone(), 0, 0, 1).collect::<Vec<_>>().await;
         assert_eq!(1, ranges.len());
-        assert_eq!((0, 0), filter_bounds(&ranges[0])?);
+        assert_eq!((0, 0), filter_bounds(&ranges[0][0])?);

-        let ranges = split_range(LogFilter::default(), 0, 3, 1).collect::<Vec<_>>().await;
+        let ranges = split_range(filters.clone(), 0, 3, 1).collect::<Vec<_>>().await;
         assert_eq!(4, ranges.len());
-        assert_eq!((0, 0), filter_bounds(&ranges[0])?);
-        assert_eq!((1, 1), filter_bounds(&ranges[1])?);
-        assert_eq!((2, 2), filter_bounds(&ranges[2])?);
-        assert_eq!((3, 3), filter_bounds(&ranges[3])?);
+        assert_eq!((0, 0), filter_bounds(&ranges[0][0])?);
+        assert_eq!((1, 1), filter_bounds(&ranges[1][0])?);
+        assert_eq!((2, 2), filter_bounds(&ranges[2][0])?);
+        assert_eq!((3, 3), filter_bounds(&ranges[3][0])?);

-        let ranges = split_range(LogFilter::default(), 0, 3, 10).collect::<Vec<_>>().await;
+        let ranges = split_range(filters, 0, 3, 10).collect::<Vec<_>>().await;
         assert_eq!(1, ranges.len());
-        assert_eq!((0, 3), filter_bounds(&ranges[0])?);
+        assert_eq!((0, 3), filter_bounds(&ranges[0][0])?);

chain/types/src/lib.rs (1)

2-2: Unused imports detected.

The pipeline shows these imports are unused and should be removed to avoid compilation warnings.

-use std::{str::FromStr, sync::Arc};

🧰 Tools

🪛 GitHub Actions: Build

[warning] 2-2: unused imports: str::FromStr and sync::Arc.

chain/rpc/src/types.rs (1)

3-3: Consider making the TODO comment more actionable.

The TODO comment mentions architectural changes but lacks specifics about timeline or implementation approach.

-/// TODO: Should be an internal type, `hopr_lib::chain` must be moved to this package
+/// TODO: Make this an internal type once `hopr_lib::chain` is moved to this package (tracked in issue #XXXX)

chain/rpc/src/indexer.rs (1)

51-56: Consider optimizing filter cloning to reduce overhead.

The filters.clone() on line 52 creates a full copy of the filters vector on each iteration. Since the filters are read-only in this context, consider using references or restructuring to avoid repeated cloning.

-            let ranged_filters = filters
-                .clone()
-                .into_iter()
-                .map(|f| f.from_block(start).to_block(end))
-                .collect::<Vec<_>>();
+            let ranged_filters = filters
+                .iter()
+                .map(|f| f.clone().from_block(start).to_block(end))
+                .collect::<Vec<_>>();

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

504-514: Remove or uncomment the debugging code.

There's commented-out code that appears to be for printing indexer state. Either remove it or uncomment it if it's needed.

-                if fetch_checksum_from_db {
-                    let last_log = block.logs.into_iter().last().unwrap();
-                    let log = db.get_log(block_id, last_log.tx_index, last_log.log_index).await.ok()?;
-
-                    // Self::print_indexer_state(block_id, log_count, log.checksum.unwrap()).await
-                } else {
-                    // Self::print_indexer_state(block_id, log_count, last_log_checksum.to_string()).await
-                }
+                if fetch_checksum_from_db {
+                    let last_log = block.logs.into_iter().last().unwrap();
+                    let log = db.get_log(block_id, last_log.tx_index, last_log.log_index).await.ok()?;
+                    // TODO: Add indexer state logging if needed
+                }

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

Learnt from: tolbrino
PR: hoprnet/hoprnet#6399
File: chain/api/Cargo.toml:50-53
Timestamp: 2024-11-01T15:26:53.058Z
Learning: In `chain/api/Cargo.toml`, it's acceptable to declare the same dependency `hopr-db-sql` in both `[dependencies]` and `[dev-dependencies]` with different features, especially when additional features are only needed for tests.

Learnt from: tolbrino
PR: hoprnet/hoprnet#6675
File: chain/indexer/src/block.rs:319-320
Timestamp: 2024-11-27T17:22:40.429Z
Learning: In the Rust file `chain/indexer/src/block.rs`, within the `process_block_by_id` function, if there is a block number mismatch in logs retrieved from the database, we use `panic!` to handle this unrecoverable error.

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

338-403: ⚠️ Potential issue

Fix critical compilation errors in generate_log_filters function.

This function has multiple compilation errors that prevent the code from building:

1. Missing imports for Filter, TransferFilter, and ApprovalFilter types
2. db is not in scope - it's a field of Self but not accessible in this static function
3. Function is not async but uses .await
4. Missing Address and other type imports

The scope and async issues were previously identified. Here's a comprehensive fix:

+use alloy::rpc::types::Filter;
+use hopr_bindings::hoprtoken::HoprToken::{Transfer as TransferFilter, Approval as ApprovalFilter};
+use hopr_primitive_types::prelude::Address;

-    fn generate_log_filters(logs_handler: &U) -> FilterSet {
+    async fn generate_log_filters(logs_handler: &U, db: &Db) -> Result<FilterSet> {
         let safe_address = logs_handler.safe_address();
         let addresses_no_token = logs_handler
             .contract_addresses()
             .into_iter()
             .filter(|a| *a != logs_handler.contract_addresses_map().token)
             .collect::<Vec<_>>();
         let mut filter_base_addresses = vec![];
         let mut filter_base_topics = vec![];
         let mut address_topics = vec![];

         addresses_no_token.iter().for_each(|address| {
             let topics = logs_handler.contract_address_topics(*address);
             if !topics.is_empty() {
                 filter_base_addresses.push(alloy::primitives::Address::from(*address));
                 filter_base_topics.extend(topics);
                 for topic in topics {
                     address_topics.push((*address, alloy::primitives::B256::from_slice(topic.as_ref())))
                 }
             }
         });

         let filter_base = Filter::new().address(filter_base_addresses).topic0(filter_base_topics);
         let filter_token = Filter::new().address(alloy::primitives::Address::from(
             logs_handler.contract_addresses_map().token,
         ));

         let filter_transfer_to = filter_token
             .clone()
             .topic0(TransferFilter::signature())
             .topic2(alloy::primitives::B256::from_slice(safe_address.to_bytes32().as_ref()));

-        let filter_transfer_from = Filter::new()
+        let filter_transfer_from = filter_token
             .clone()
             .topic0(TransferFilter::signature())
             .topic1(alloy::primitives::B256::from_slice(safe_address.to_bytes32().as_ref()));

-        let filter_approval = Filter::new()
+        let filter_approval = filter_token
             .clone()
             .topic0(ApprovalFilter::signature())
             .topic1(alloy::primitives::B256::from_slice(safe_address.to_bytes32().as_ref()))
             .topic2(alloy::primitives::B256::from_slice(
                 logs_handler.contract_addresses_map().channels.to_bytes32().as_ref(),
             ));

         // Check that the contract addresses and topics are consistent with what is in the logs DB,
         // or if the DB is empty, prime it with the given addresses and topics.
         db.ensure_logs_origin(address_topics).await?;

-        FilterSet {
+        Ok(FilterSet {
             all: vec![
                 filter_base.clone(),
                 filter_transfer_from.clone(),
                 filter_transfer_to.clone(),
                 filter_approval.clone(),
             ],
             token: vec![filter_transfer_from, filter_transfer_to, filter_approval],
             no_token: vec![filter_base],
-        }
+        })
     }

You'll also need to update the call site on line 129:

-        let log_filters = Self::generate_log_filters(&logs_handler);
+        let log_filters = Self::generate_log_filters(&logs_handler, &db).await?;

🧰 Tools

🪛 GitHub Actions: Build

[error] 344-344: Cannot find type FilterSet in this scope. Suggestion: import FilterSet from hopr_chain_rpc.

---

[error] 366-367: Use of undeclared type Filter. Suggestion: import Filter from appropriate module.

---

[error] 376-381: Use of undeclared type Filter. Suggestion: import Filter from appropriate module.

---

[error] 391-391: Use of undefined value db in this scope; field db exists in Self but not accessible here. Also, .await used outside async function.

---

[error] 393-393: Cannot find struct, variant or union type FilterSet in this scope. Suggestion: import FilterSet from hopr_chain_rpc.

---

[error] 373-383: Use of undeclared types TransferFilter and ApprovalFilter. These types are not found in scope.

---

[error] 391-391: await used outside async function or block.

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

8-8: Remove unused import.

The pipeline indicates this import is unused and should be removed to clean up the code.

-use hopr_bindings::hoprtoken::HoprToken::HoprTokenEvents::Transfer;

🧰 Tools

🪛 GitHub Actions: Build

[warning] 8-8: Unused import: hopr_bindings::hoprtoken::HoprToken::HoprTokenEvents::Transfer.

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

Learnt from: tolbrino
PR: hoprnet/hoprnet#6675
File: chain/indexer/src/block.rs:319-320
Timestamp: 2024-11-27T17:22:40.429Z
Learning: In the Rust file `chain/indexer/src/block.rs`, within the `process_block_by_id` function, if there is a block number mismatch in logs retrieved from the database, we use `panic!` to handle this unrecoverable error.

chain/indexer/src/handlers.rs (3)

546-549: Fix error handling for filter misconfiguration.

The code still logs an error for filter misconfiguration but continues execution. This should return an error immediately to prevent unnecessary processing.

Apply this fix:

-error!(
-safe_address = %&self.safe_address, %from, %to,
-    "filter misconfiguration: transfer event not involving the safe");
+return Err(CoreEthereumIndexerError::ProcessError(format!(
+    "filter misconfiguration: transfer event not involving the safe - from: {}, to: {}, safe: {}",
+    from, to, self.safe_address
+)));

---

573-585: Fix RPC parameter order and error handling.

There are two issues here:

1. The parameters to get_allowance are swapped - should query allowance of safe for channels contract
2. Filter misconfiguration should return an error instead of just logging

Apply this fix:

-get_allowance::<WxHOPR>(self.addresses.channels, self.safe_address)
+get_allowance::<WxHOPR>(self.safe_address, self.addresses.channels)

---

587-589: Fix error handling for filter misconfiguration.

Similar to the transfer event, this should return an error instead of just logging.

Apply this fix:

-error!(
-    address = %&self.safe_address, %owner, %spender, allowance = %approved.value,
-    "filter misconfiguration: approval event not involving the safe and channels contract");
+return Err(CoreEthereumIndexerError::ProcessError(format!(
+    "filter misconfiguration: approval event not involving the safe and channels contract - owner: {}, spender: {}, safe: {}, channels: {}",
+    owner, spender, self.safe_address, self.addresses.channels
+)));

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

733-734: ⚠️ Potential issue

Fix the syntax error in test mock method signature.

There's a syntax error in the mock method signature that was flagged in previous reviews but hasn't been fixed yet.

Apply this diff to fix the syntax error:

-            async fn get_allowanc<C: Currency>e(&self, owner: Address, spender: Address) -> hopr_chain_rpc::errors::Result<Balance<C>>;
+            async fn get_allowance<C: Currency>(&self, owner: Address, spender: Address) -> hopr_chain_rpc::errors::Result<Balance<C>>;

---

378-392: 💡 Verification agent

🧩 Analysis chain

Verify that TransferFilter and ApprovalFilter are properly imported.

The code uses TransferFilter and ApprovalFilter types but these may not be imported based on past review comments.

---

🏁 Script executed:

#!/bin/bash
# Check if TransferFilter and ApprovalFilter are imported or available
rg -A 5 -B 5 "TransferFilter|ApprovalFilter" --type rust

Length of output: 1552

---

🏁 Script executed:

#!/bin/bash
# List all use statements in the first 200 lines of block.rs to check for TransferFilter/ApprovalFilter imports
echo "Use statements in top 200 lines of chain/indexer/src/block.rs:"
sed -n '1,200p' chain/indexer/src/block.rs | grep -n '^use'

Length of output: 721

---

🏁 Script executed:

#!/bin/bash
# Show the `use crate::{ ... }` import block in chain/indexer/src/block.rs
echo "Lines 17–40 of chain/indexer/src/block.rs:"
sed -n '17,40p' chain/indexer/src/block.rs | nl -ba

Length of output: 239

---

🏁 Script executed:

#!/bin/bash
# Print lines 1–60 of block.rs to inspect import statements and verify TransferFilter/ApprovalFilter imports
sed -n '1,60p' chain/indexer/src/block.rs

Length of output: 2431

---

Add missing imports for TransferFilter and ApprovalFilter

It appears that TransferFilter and ApprovalFilter are used in block.rs but not imported. Please add the appropriate use statements at the top of chain/indexer/src/block.rs, for example:

 use tracing::{debug, error, info, trace};
+use alloy::rpc::evm::{TransferFilter, ApprovalFilter};

 use crate::{
     IndexerConfig,
     errors::{CoreEthereumIndexerError, Result},
     traits::ChainLogHandler,
 };

This will bring both filters into scope for the calls to TransferFilter::signature() and ApprovalFilter::signature().

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

347-406: Optimize filter creation by removing redundant clone operations.

The filter generation logic is well-structured and creates appropriate filters for different contract types. However, there are redundant .clone() operations on newly created Filter instances.

Apply this diff to remove unnecessary clones:

-        let filter_transfer_from = alloy::rpc::types::Filter::new()
-            .clone()
-            .event_signature(TransferFilter::signature())
-            .topic1(alloy::primitives::B256::from_slice(safe_address.to_bytes32().as_ref()));
+        let filter_transfer_from = filter_token
+            .clone()
+            .event_signature(TransferFilter::signature())
+            .topic1(alloy::primitives::B256::from_slice(safe_address.to_bytes32().as_ref()));

-        let filter_approval = alloy::rpc::types::Filter::new()
-            .clone()
-            .event_signature(ApprovalFilter::signature())
-            .topic1(alloy::primitives::B256::from_slice(safe_address.to_bytes32().as_ref()))
-            .topic2(alloy::primitives::B256::from_slice(
-                logs_handler.contract_addresses_map().channels.to_bytes32().as_ref(),
-            ));
+        let filter_approval = filter_token
+            .clone()
+            .event_signature(ApprovalFilter::signature())
+            .topic1(alloy::primitives::B256::from_slice(safe_address.to_bytes32().as_ref()))
+            .topic2(alloy::primitives::B256::from_slice(
+                logs_handler.contract_addresses_map().channels.to_bytes32().as_ref(),
+            ));

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

Learnt from: tolbrino
PR: hoprnet/hoprnet#6675
File: chain/indexer/src/block.rs:319-320
Timestamp: 2024-11-27T17:22:40.429Z
Learning: In the Rust file `chain/indexer/src/block.rs`, within the `process_block_by_id` function, if there is a block number mismatch in logs retrieved from the database, we use `panic!` to handle this unrecoverable error.

chain/indexer/src/handlers.rs (3)

546-548: ⚠️ Potential issue

Critical: Unresolved filter misconfiguration handling.

This issue was flagged in past reviews but remains unresolved. The code logs an error for filter misconfiguration but continues execution, which can lead to unnecessary processing and confusion.

Apply this fix to return an error instead of just logging:

-                error!(
-                safe_address = %&self.safe_address, %from, %to,
-                    "filter misconfiguration: transfer event not involving the safe");
+                return Err(CoreEthereumIndexerError::ProcessError(format!(
+                    "filter misconfiguration: transfer event not involving the safe - from: {}, to: {}, safe: {}",
+                    from, to, self.safe_address
+                )));

---

587-589: ⚠️ Potential issue

Critical: Unresolved filter misconfiguration handling.

Similar to the transfer event, this filter misconfiguration issue was flagged in past reviews but remains unresolved. The code should return an error instead of just logging.

Apply this fix:

-                error!(
-                    address = %&self.safe_address, %owner, %spender, allowance = %approved.value,
-                    "filter misconfiguration: approval event not involving the safe and channels contract");
+                return Err(CoreEthereumIndexerError::ProcessError(format!(
+                    "filter misconfiguration: approval event not involving the safe and channels contract - owner: {}, spender: {}, safe: {}, channels: {}",
+                    owner, spender, self.safe_address, self.addresses.channels
+                )));

---

576-576: 💡 Verification agent

🧩 Analysis chain

Critical: Verify RPC call parameter order.

Based on past review comments, the parameters to get_allowance might be swapped. The method should query the allowance of the owner (safe) for the spender (channels), but the current order appears to be (spender, owner).

---

🏁 Script executed:

#!/bin/bash
# Description: Verify the expected parameter order for get_allowance method
# Expected: get_allowance(owner, spender) where owner=safe_address, spender=channels

# Search for get_allowance method definition and usage patterns
ast-grep --pattern 'get_allowance($owner, $spender)'

# Look for get_allowance trait definition
rg -A 5 "fn get_allowance"

Length of output: 1899

---

Fix: Correct get_allowance parameter order

The get_allowance<C>(owner, spender) signature expects the first argument to be the owner and the second to be the spender, but the call is currently swapped.

• File: chain/indexer/src/handlers.rs
Line: 576

-.get_allowance::<WxHOPR>(self.addresses.channels, self.safe_address)
+.get_allowance::<WxHOPR>(self.safe_address, self.addresses.channels)

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

735-735: Fix test mock method signature syntax error.

The method signature has a syntax error with extraneous characters in the generic parameter.

This syntax error was previously identified and should be corrected:

-            async fn get_allowanc<C: Currency>e(&self, owner: Address, spender: Address) -> hopr_chain_rpc::errors::Result<Balance<C>>;
+            async fn get_allowance<C: Currency>(&self, owner: Address, spender: Address) -> hopr_chain_rpc::errors::Result<Balance<C>>;

chain/indexer/src/handlers.rs (1)

550-558: Fix typo in error message and verify RPC integration.

There's a typo "tranpfer" that should be "transfer" in the error log. The RPC integration logic looks correct for updating balance after transfer events.

-                            error!(%error, "error getting safe balance from chain after tranpfer event");
+                            error!(%error, "error getting safe balance from chain after transfer event");

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

486-527: Individual log processing improves error isolation and data safety.

The transition from batch to individual log processing provides better error isolation and prevents data loss through immediate panic on failures. This aligns well with the PR objective of making the indexer error-safe.

Consider the performance implications of individual log processing vs batch processing, especially for blocks with many logs. You might want to implement batching at the database transaction level to maintain consistency while improving performance.

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

Learnt from: tolbrino
PR: hoprnet/hoprnet#6675
File: chain/indexer/src/block.rs:319-320
Timestamp: 2024-11-27T17:22:40.429Z
Learning: In the Rust file `chain/indexer/src/block.rs`, within the `process_block_by_id` function, if there is a block number mismatch in logs retrieved from the database, we use `panic!` to handle this unrecoverable error.

chain/rpc/src/indexer.rs (1)

317-349: ⚠️ Potential issue

Update test to use new FilterSet structure.

The test is still using the old LogFilter type instead of the new FilterSet structure, which will cause compilation errors.

-        let filters = vec![Filter::default()];
-        let ranges = split_range(filters, 0, 10, 2).collect::<Vec<_>>().await;
+        let filters = vec![Filter::default()];
+        let ranges = split_range(filters, 0, 10, 2).collect::<Vec<_>>().await;

Additionally, update the remaining test calls that reference LogFilter::default():

-        let ranges = split_range(LogFilter::default(), 0, 0, 2).collect::<Vec<_>>().await;
+        let ranges = split_range(vec![Filter::default()], 0, 0, 2).collect::<Vec<_>>().await;

chain/rpc/src/indexer.rs (3)

69-123: ⚠️ Potential issue

Fix the asymmetric sorting logic that could lead to unstable behavior.

The sorting implementation still has the same asymmetric handling of error cases identified in previous reviews. This can lead to unstable sorting behavior.

Apply this fix to ensure stable sorting:

-                // at this point we need to ensure logs are ordered by block number since that is
-                // expected by the indexer
-                results.sort_by(|a, b| {
-                    if let Ok(a) = a {
-                        if let Ok(b) = b {
-                            a.block_number.cmp(&b.block_number)
-                        } else {
-                            std::cmp::Ordering::Greater
-                        }
-                    } else {
-                        std::cmp::Ordering::Less
-                    }
-                });
+                // Filter out errors and sort successful logs by block number
+                let mut successful_logs: Vec<_> = results.into_iter().filter_map(|r| r.ok()).collect();
+                successful_logs.sort_by_key(|log| log.block_number);
+                
+                // Convert back to Results
+                results = successful_logs.into_iter().map(Ok).collect();

---

132-134: ⚠️ Potential issue

Fix infinite recursion in get_allowance method.

This method recursively calls itself, leading to a stack overflow. It should delegate to the underlying RPC implementation.

-    async fn get_allowance<C: Currency>(&self, owner: Address, spender: Address) -> Result<Balance<C>> {
-        self.get_allowance::<C>(owner, spender).await
-    }
+    async fn get_allowance<C: Currency>(&self, owner: Address, spender: Address) -> Result<Balance<C>> {
+        self.get_allowance::<C>(owner, spender).await
+    }

Note: This should call the actual RPC method, not itself. The implementation needs to delegate to the underlying contract or RPC client.

---

136-138: ⚠️ Potential issue

Fix infinite recursion in get_balance method.

This method also recursively calls itself. Replace the call to self.get_balance(...) with the appropriate contract or RPC client invocation.

-    async fn get_balance<C: Currency + Send>(&self, address: Address) -> Result<Balance<C>> {
-        self.get_balance::<C>(address).await
-    }
+    async fn get_balance<C: Currency + Send>(&self, address: Address) -> Result<Balance<C>> {
+        // Delegate to the underlying RPC implementation
+        // This should call the actual contract method or RPC client
+        todo!("Implement actual RPC call to get balance")
+    }

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

chain/indexer/src/handlers.rs (1)

3002-3005: ⚠️ Potential issue

Compile error – SystemTime::as_unix_timestamp is not in std

SystemTime has no as_unix_timestamp() in the standard library.
Use duration_since(UNIX_EPOCH)?.as_secs() instead.

-        let timestamp = SystemTime::now();
-        let encoded_data = (U256::from(timestamp.as_unix_timestamp().as_secs())).abi_encode();
+        let timestamp_secs = SystemTime::now()
+            .duration_since(SystemTime::UNIX_EPOCH)
+            .expect("time went backwards")
+            .as_secs();
+        let encoded_data = U256::from(timestamp_secs).abi_encode();

This fixes compilation of the on_channel_closure_initiated test.

chain/indexer/src/handlers.rs (1)

625-648: 🛠️ Refactor suggestion

Same transactional deadlock risk in token Transfer / Approval handlers

get_hopr_balance and get_hopr_allowance are awaited while tx is open.
Move the RPC calls outside the transaction (see previous comment) or collect the values from the emitted event instead of an extra RPC.
Failure to do so keeps the DB locked for the full RPC latency and will reproduce the “database is locked” error under concurrent load.

Also applies to: 669-682

chain/indexer/src/handlers.rs (2)

1035-1066: collect_log_event retains the DB transaction across network awaits

The closure passed to perform awaits process_log_event, which (see above) may await chain RPCs.
Therefore the transaction initiated at begin_transaction() can live for hundreds of milliseconds – enough for other tasks to block on the global SQLite write lock.

Consider:

1. Decoding the log and doing all RPC work first.
2. Wrapping only the final, quick DB mutations in begin_transaction.

This architectural change will remove the tight coupling between external IO and DB write locks.

---

1124-1184: Mock implementation leaks lifetimes & duplicates trait – simplify

The ClonableMockOperations wrapper plus the manual delegation of every trait
method is verbose and error-prone (e.g. when the trait gains another method).

A simpler pattern:

type ClonableMockOperations = Arc<MockIndexerRpcOperations>;

#[async_trait]
impl HoprIndexerRpcOperations for ClonableMockOperations {
    async fn block_number(&self) -> _ { self.block_number().await }
    // macro or `delegate` crate could auto-forward remaining methods
}

Or use mockall::predicate::* with Arc<dyn HoprIndexerRpcOperations> directly,
eliminating the boiler-plate.

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

sdk/python/localcluster/anvil.py (1)

30-38: command.split() mangles multi-line strings – use shlex.split or a list

The f-string contains new-lines and indent spaces; str.split() will turn them into empty/garbage tokens that land in subprocess.run, making the CLI harder to read and brittle.

-        command = f"""
-            bash scripts/run-local-anvil.sh
-            ...
-        """
-        run(
-            command.split(),
+        command = [
+            "bash",
+            "scripts/run-local-anvil.sh",
+            *([] if state is AnvilState.DUMP else ["-s"]),
+            "-l", str(self.log_file),
+            "-c", str(self.cfg_file),
+            "-p", str(self.port),
+            *(["-ls", str(self.state_file)] if state is AnvilState.LOAD else ["-ds", str(self.state_file)]),
+            *(["-sp"] if self.use_staking_proxy else []),
+        ]
+        run(
+            command,
             ...
         )

This removes the whitespace quirks and is safer across shells and paths with spaces.

sdk/python/localcluster/node.py (1)

107-110: Guard against missing PATH in environment

Accessing os.environ["PATH"] raises KeyError in rare, containerised environments.
Safer:

-            "PATH": os.environ["PATH"],
+            "PATH": os.environ.get("PATH", ""),

scripts/run-local-anvil.sh (1)

55-101: Option parsing: avoid accidental double-shift bugs

-sp) sets use_staking_proxy="true" and performs shift.
Because -sp carries no argument, that single shift is correct, but any future refactor that appends a value (e.g., -sp true|false) would silently eat the next flag. Consider the no-arg options pattern used elsewhere:

-    use_staking_proxy="true"
-    shift
+    use_staking_proxy="true"
+    shift

No immediate action required, just flagging for future-proofing.

sdk/python/localcluster/main_process.py (2)

46-48: Break overly-long constructor call for Ruff E501

Ruff flags this line (E501 130>120). Readability also benefits from one-arg-per-line formatting.

-    anvil = Anvil(ANVIL_FOLDER.joinpath("anvil.log"), ANVIL_CONFIG_FILE, ANVIL_FOLDER.joinpath("anvil.state.json"), base_port, not test_mode)
+    anvil = Anvil(
+        ANVIL_FOLDER.joinpath("anvil.log"),
+        ANVIL_CONFIG_FILE,
+        ANVIL_FOLDER.joinpath("anvil.state.json"),
+        base_port,
+        not test_mode,
+    )

🧰 Tools

🪛 Ruff (0.11.9)

47-47: Line too long (130 > 120)

(E501)

---

79-83: Hard-coded 1 s sleep risks flakiness

cluster.enable_network_registry() fires an on-chain tx and then you await asyncio.sleep(1).
Block mining & confirmation on Anvil can be >1 s under load, leading to race conditions with subsequent funding/registration.

Consider polling the CLI exit code/receipt or waiting for hopli to return once the registry status is confirmed instead of an arbitrary sleep.

sdk/python/localcluster/cluster.py (1)

101-128: CLI flag name mismatch & unused captured output

The new hopli network-registry toggle sub-command expects --enable with a Boolean value (see enable: bool in hopli/src/network_registry.rs).
Your call passes the flag but no value; this only sets enable=true, which is fine, but capturing stdout/stderr without logging them discards useful diagnostics.

1. Decide whether to pass an explicit value for clarity, e.g. --enable true.
2. Log result.stdout/result.stderr when check=True succeeds to aid debugging.

hopli/src/network_registry.rs (1)

48-56: Docstring examples are outdated

The sample shows
--ena true and repeats “syncs eligibility”, but the implemented flag is
--enable (boolean) for the new toggle command.

Updating the comment avoids confusing CLI users.

chain/rpc/src/indexer.rs (2)

90-98: Unbounded concurrency – protect the RPC provider

then_concurrent spawns an unbounded number of get_logs calls (one per filter per chunk).
On fast-sync this can issue hundreds of concurrent HTTP requests and overwhelm the node or the provider, again surfacing as I/O timeouts and flakey smoke tests.

Consider limiting concurrency:

-                let mut results = futures::stream::iter(subrange_filters)
-                    .then_concurrent(|filter| async move {
+                let concurrency = self.cfg.max_concurrent_log_requests; // add to config
+                let mut results = futures::stream::iter(subrange_filters)
+                    .then_concurrent_with_limit(concurrency, |filter| async move {

A sane limit (e.g. 8-16) keeps throughput high without saturating the provider.

---

310-335: Variable naming in test helper is flipped – hurts readability

Inside filter_bounds, to holds the from block and from holds the to block.
This double inversion makes the helper hard to reason about.

-            let to = filter.block_option.get_from_block()...
-            let from = filter.block_option.get_to_block()...
-            let next = (to, from);
+            let from = filter.block_option.get_from_block()...
+            let to   = filter.block_option.get_to_block()...
+            let next = (from, to);

No functional change but saves the next reader from a mental back-flip.

chain/indexer/src/handlers.rs (1)

941-953: Opportunity: fetch balance & allowance concurrently

get_hopr_balance and get_hopr_allowance are independent RPCs.
Once they run outside the DB transaction you can shave one network RTT by firing them in parallel:

let (balance_res, allowance_res) = tokio::join!(
    self.rpc_operations.get_hopr_balance(self.safe_address),
    self.rpc_operations.get_hopr_allowance(self.safe_address, self.addresses.channels)
);

Apply where both values are required (ChannelBalanceIncreased, Transfer, …).
Not critical but low-hanging latency improvement.

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

Learnt from: tolbrino
PR: hoprnet/hoprnet#7213
File: chain/rpc/src/indexer.rs:132-142
Timestamp: 2025-06-04T09:14:43.735Z
Learning: CodeRabbit sometimes flags false positive infinite recursion warnings when trait implementation methods delegate to concrete methods with the same name on the implementing struct. This is a valid delegation pattern, not actual recursion.

ethereum/contracts/test/scripts/DeployNodeManagement.t.sol (1)

24-26: 🛠️ Refactor suggestion

The test body is commented out – no assertions are executed

test_Run() currently performs no action, so the new staking-proxy path isn’t exercised.
Either enable the call and add at least one assertion (e.g., check that the deployed Safe points to the staking proxy) or remove the placeholder test to keep the suite clean.

-    // deployNodeSafeScriptContract.run();
+        address safe = deployNodeSafeScriptContract.run();
+        assertTrue(safe != address(0), "Safe deployment failed");

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

515-533: Per-log set_log_processed still a write-lock hot-spot

This loop issues one DB transaction per log, the exact pattern that triggered the smoke-test deadlock (#7216).
Please batch the updates (e.g. set_logs_processed_bulk(&[…])) or mark them after the loop within a single transaction.
See earlier review on the same lines – the concern remains unresolved.

---

399-402: ⚠️ Potential issue

event_signature called with Vec<B256> → does not compile

alloy::rpc::types::Filter::event_signature expects impl Into<Option<B256>>, i.e. a single topic hash, not a vector.
Passing filter_base_topics (a Vec) will fail at compile-time.

-        let filter_base = alloy::rpc::types::Filter::new()
-            .address(filter_base_addresses)
-            .event_signature(filter_base_topics);
+        // Build one filter per signature instead of passing the whole Vec
+        let base_filters: Vec<_> = filter_base_topics
+            .into_iter()
+            .map(|sig| {
+                alloy::rpc::types::Filter::new()
+                    .address(filter_base_addresses.clone())
+                    .event_signature(sig)
+            })
+            .collect();

You will also need to adjust the FilterSet construction a few lines below to use base_filters instead of the single filter_base variable.
Failing to fix this will block compilation entirely.

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

406-421: Minor: avoid redundant clones and unnecessary allocations

filter_token.clone() is called twice; you could reuse the first clone (or use filter_token.clone() only once and derive the others from it) to save an allocation.
Not critical, but easy to streamline.

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

Learnt from: tolbrino
PR: hoprnet/hoprnet#6675
File: chain/indexer/src/block.rs:319-320
Timestamp: 2024-11-27T17:22:40.429Z
Learning: In the Rust file `chain/indexer/src/block.rs`, within the `process_block_by_id` function, if there is a block number mismatch in logs retrieved from the database, we use `panic!` to handle this unrecoverable error.

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

402-404: event_signature is called with a Vec<B256> – this will not compile

alloy::rpc::types::Filter::event_signature accepts impl Into<Option<B256>>, i.e. a single topic hash, not a list.
Passing filter_base_topics (a Vec) therefore breaks the build.

-        let filter_base = alloy::rpc::types::Filter::new()
-            .address(filter_base_addresses)
-            .event_signature(filter_base_topics);
+        // Build one filter per event signature to satisfy the API
+        let base_filters: Vec<_> = filter_base_topics
+            .into_iter()
+            .map(|sig| {
+                alloy::rpc::types::Filter::new()
+                    .address(filter_base_addresses.clone())
+                    .event_signature(sig)
+            })
+            .collect();

You will also need to push all base_filters into the returned FilterSet.
Failing to fix this stops the whole crate from compiling.

---

297-303: Token logs are never inserted but still marked as processed → guaranteed panic

logs_vec filters out token-contract logs before insertion, yet process_block
later iterates over the original block.logs (which still contains them) and
calls db.set_log_processed(log).

The DB does not have those rows, set_log_processed returns an error, and the
code panics – reviving the very crash this PR tries to eliminate.

Quick, minimal fix:

-                        let logs_vec = logs
-                            .into_iter()
-                            .filter(|log| log.address != logs_handler.contract_addresses_map().token)
-                            .collect();
+                        let filtered: Vec<_> = logs
+                            .into_iter()
+                            .filter(|log| log.address != logs_handler.contract_addresses_map().token)
+                            .collect();
+
+                        // keep both DB store and downstream processing in sync
+                        block.logs = filtered.iter().cloned().collect();
+                        let logs_vec = filtered;

Without this adjustment every token Transfer / Approval will crash the
indexer.

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

Learnt from: tolbrino
PR: hoprnet/hoprnet#6675
File: chain/indexer/src/block.rs:319-320
Timestamp: 2024-11-27T17:22:40.429Z
Learning: In the Rust file `chain/indexer/src/block.rs`, within the `process_block_by_id` function, if there is a block number mismatch in logs retrieved from the database, we use `panic!` to handle this unrecoverable error.