chore: use rwlock for peers

[renancloudwalk]

No description provided.

[github-actions]

PR Review 🔍

⏱️ Estimated effort to review [1-5]	3, because the PR involves changes to concurrency mechanisms which require careful consideration to ensure thread safety and performance. The changes are not extensive but are critical.
🧪 Relevant tests	No
⚡ Possible issues	Possible Bug: The use of RwLock may introduce race conditions if not handled properly. Specifically, the transition from Mutex to RwLock needs to ensure that all read and write operations are correctly managed to prevent data corruption.
🔒 Security concerns	No

Code feedback:

---

relevant file	src/eth/consensus/mod.rs
suggestion	Consider handling potential errors from RwLock read and write operations. Currently, the code assumes successful locking without error handling, which might lead to panics in case of failures. Adding error handling will make the system more robust. [important]
relevant line	let peers = consensus.peers.read().await;

---

relevant file	src/eth/consensus/mod.rs
suggestion	Ensure that all instances where peers are accessed are properly switched from Mutex to RwLock semantics. This includes not only locking mechanisms but also considering the scope of locks to avoid deadlocks or excessive blocking, especially in write operations. [important]
relevant line	let mut peers_lock = consensus.peers.write().await;

---

relevant file	src/eth/consensus/mod.rs
suggestion	Optimize the use of RwLock by minimizing the scope of write locks. For example, consider if some of the write locks can be downgraded to read locks if only read operations are performed after certain points. This can improve performance by reducing lock contention. [medium]
relevant line	let mut peers_lock = consensus.peers.write().await;

---

relevant file	src/eth/consensus/mod.rs
suggestion	Review and possibly refactor the logic to ensure that the transition from Mutex to RwLock does not alter the intended logic of the program, especially in concurrent environments. This might involve more detailed analysis and testing to catch subtle bugs related to concurrency. [important]
relevant line	peers: Arc>>,

---

[github-actions]

PR Code Suggestions ✨

Category	Suggestion	Score
Possible issue	Review and balance the use of read and write locks when using RwLock to optimize concurrency and prevent deadlocks When replacing Mutex with RwLock, ensure that the use of read and write locks is correctly balanced to avoid deadlocks and to maximize concurrency. src/eth/consensus/mod.rs [157-315] -let peers = consensus.peers.read().await; -let mut peers_lock = consensus.peers.write().await; +let peers = consensus.peers.read().await; // Ensure this read lock is properly used +let mut peers_lock = consensus.peers.write().await; // Review the necessity and usage of write locks Suggestion importance[1-10]: 8 Why: Ensuring the correct balance of read and write locks is crucial for preventing deadlocks and maximizing concurrency, which is important for the performance and reliability of the system.	8
	Add error handling during the initialization of peers to prevent potential runtime issues Ensure proper error handling when initializing peers with RwLock::new(HashMap::new()) to avoid potential runtime panics from unwrapping errors. src/eth/consensus/mod.rs [114] -let peers = Arc::new(RwLock::new(HashMap::new())); +let peers = Arc::new(RwLock::new(HashMap::new())); // Consider adding error handling here Suggestion importance[1-10]: 3 Why: The initialization of peers with RwLock::new(HashMap::new()) is unlikely to fail, so adding error handling here is not critical. However, it could improve robustness.	3
Maintainability	Encapsulate the creation of peers in a separate function to improve maintainability Replace the direct use of Arc::new(RwLock::new(...)) with a factory function that encapsulates the creation logic, enhancing maintainability and testability. src/eth/consensus/mod.rs [114] -let peers = Arc::new(RwLock::new(HashMap::new())); +let peers = create_peers(); +// Assuming create_peers() is a function that returns `Arc<RwLock<HashMap<PeerAddress, Peer>>>` Suggestion importance[1-10]: 6 Why: Encapsulating the creation logic in a factory function can enhance maintainability and testability, making the code easier to manage and extend.	6
Best practice	Improve code clarity and avoid potential conflicts by using a more specific import name for RwLock Consider using a more specific import for RwLock to avoid potential namespace conflicts and improve code clarity. src/eth/consensus/mod.rs [22] -use tokio::sync::RwLock; +use tokio::sync::RwLock as TokioRwLock; Suggestion importance[1-10]: 5 Why: Using a more specific import name can help avoid potential namespace conflicts and improve code clarity, but it is not crucial for functionality.	5