fix: remove term from peers

[renancloudwalk]

No description provided.

[github-actions]

PR Review 🔍

⏱️ Estimated effort to review [1-5]	2, because the changes are localized to a specific part of the code, mainly involving the removal of the term field from the Peer struct and its usage. The changes are straightforward and do not involve complex logic changes.
🧪 Relevant tests	No
⚡ Possible issues	Possible Bug: The removal of the term field from the Peer struct might affect other parts of the system where term is expected to be present. This could lead to runtime errors if not all references to term are properly removed or handled.
🔒 Security concerns	No

Code feedback:

---

relevant file	src/eth/consensus/mod.rs
suggestion	Ensure that all instances where term might still be accessed are either removed or refactored. This includes checking any serialized formats, logs, or external interfaces that might expect the term field. [important]
relevant line	- term: u64,

---

relevant file	src/eth/consensus/mod.rs
suggestion	Since the role field is used in a conditional comment (FIXME), consider implementing the logic to handle different roles dynamically rather than hardcoding it to Follower. This might involve refactoring the initialization of role based on the current state or configuration. [important]
relevant line	role: Role::Follower, // FIXME it won't be always follower, we need to check the leader or candidates

---

relevant file	src/eth/consensus/mod.rs
suggestion	Add detailed logging for error scenarios, especially when handling unexpected status codes in the match StatusCode::try_from(response.status) block. This will help in debugging and maintaining the system effectively. [medium]
relevant line	_ => Err(anyhow!("Unexpected status code: {:?}", response.status)),

---

relevant file	src/eth/consensus/mod.rs
suggestion	Consider adding back the logging of state changes including the term if it's still relevant in other contexts or ensure that all necessary state information is logged after the removal of term. [medium]
relevant line	tracing::info!(match_index = peer.match_index, next_index = peer.next_index, role = ?peer.role, "current follower state on election"); //TODO also move this to metrics

---

[github-actions]

PR Code Suggestions ✨

Category	Suggestion	Score
Performance	Use asynchronous locking mechanisms to improve concurrency handling To avoid potential race conditions or deadlocks, consider using asynchronous locking mechanisms or other non-blocking approaches when working with Arc<Mutex<...>> especially in a highly concurrent context like blockchain consensus. src/eth/consensus/mod.rs [544] -receiver: Arc::new(Mutex::new(consensus.broadcast_sender.subscribe())), +receiver: Arc::new(tokio::sync::Mutex::new(consensus.broadcast_sender.subscribe())), Suggestion importance[1-10]: 9 Why: This suggestion addresses a critical performance and safety issue by recommending the use of asynchronous locking mechanisms. This change is highly relevant in a concurrent context like blockchain consensus and can prevent potential race conditions or deadlocks.	9
Enhancement	Implement dynamic role assignment for peers Consider implementing a mechanism to dynamically determine the role of a peer instead of hardcoding it as Role::Follower. This could be done by checking the current state or election results to assign the correct role. src/eth/consensus/mod.rs [543] -role: Role::Follower, // FIXME it won't be always follower, we need to check the leader or candidates +role: determine_peer_role(), // Implement this function to dynamically assign role based on state or election Suggestion importance[1-10]: 8 Why: This suggestion addresses a significant issue by proposing a dynamic role assignment mechanism, which is crucial for the correct functioning of the consensus algorithm. It directly improves the flexibility and correctness of the code.	8
Possible issue	Refactor functionalities dependent on the removed term field Since the term field is removed from the Peer struct, ensure that all functionalities relying on the term are updated or refactored to work without it. This might involve updating consensus algorithms or state management that previously depended on the term. src/eth/consensus/mod.rs [139] -// term field removed +// Ensure all dependent functionalities are refactored Suggestion importance[1-10]: 7 Why: The suggestion correctly identifies the need to refactor functionalities that depended on the removed term field. Ensuring that all dependent functionalities are updated is important for maintaining the integrity of the consensus mechanism.	7
	Add alternative logging information for elections in place of the removed term The logging statement for the peer's state during elections is missing the term information which was present before. If the term is still relevant for logging or debugging, consider adding an alternative way to log election-related information. src/eth/consensus/mod.rs [663] -tracing::info!(match_index = peer.match_index, next_index = peer.next_index, role = ?peer.role, "current follower state on election"); +tracing::info!(match_index = peer.match_index, next_index = peer.next_index, role = ?peer.role, election_cycle = get_current_election_cycle(), "current follower state on election"); Suggestion importance[1-10]: 6 Why: This suggestion is valid as it proposes an alternative way to log important election-related information, which is useful for debugging and monitoring. However, the exact implementation of get_current_election_cycle() is not provided, which slightly reduces the score.	6