enha: implement Consensus trait for Importer

[carneiro-cw]

PR Type

Enhancement, Bug fix

---

Description

• Enhanced the Consensus trait by adding default methods for should_serve and forward, along with new methods get_chain and lag.
• Implemented the Consensus trait for Importer, Raft, and SimpleConsensus structs.
• Modified the RpcContext and RPC server to handle an optional consensus.
• Updated the run function in main.rs to initialize consensus as an Option.

---

Changes walkthrough 📝

Relevant files

Enhancement

mod.rs Enhance `Consensus` trait with default methods and error handling src/eth/consensus/mod.rs Implemented default methods for should_serve and forward in the Consensus trait. Added get_chain and lag methods to the Consensus trait. Introduced error handling and logging for lag computation and transaction forwarding. +46/-3    mod.rs Implement `get_chain` and `lag` for `Raft` struct                src/eth/consensus/raft/mod.rs Implemented get_chain and lag methods for Raft struct. Removed redundant should_forward method. +9/-4      mod.rs Implement `get_chain` and `lag` for `SimpleConsensus` struct src/eth/consensus/simple_consensus/mod.rs Implemented get_chain and lag methods for SimpleConsensus struct. Removed redundant should_forward and should_serve methods. +9/-45    importer.rs Implement `Consensus` trait for `Importer` struct                src/eth/importer/importer.rs Implemented Consensus trait for Importer struct. Added lag and get_chain methods to Importer. +13/-0    rpc_context.rs Make `consensus` field optional in `RpcContext`                    src/eth/rpc/rpc_context.rs Changed consensus field in RpcContext to be an Option. +1/-1      rpc_server.rs Handle optional `consensus` in RPC server                                src/eth/rpc/rpc_server.rs Updated RPC server to handle optional consensus. Added checks for None value in consensus before calling methods. +9/-4      main.rs Update `run` function to handle optional `consensus`          src/main.rs Updated run function to initialize consensus as an Option. Removed initialization of SimpleConsensus. +2/-7

---

💡 PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

[github-actions]

PR Reviewer Guide 🔍

⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Key issues to review Error Logging In the should_serve method, the error is logged but not returned. Consider returning the error for better error handling and visibility. Dynamic Field Logging In the forward method, dynamic fields are being logged directly in the message. These should be added as tracing fields instead. Unimplemented Methods The get_chain and lag methods are marked as todo!(). Ensure these methods are implemented before merging. Unwrap Usage The lag method uses unwrap on EXTERNAL_RPC_CURRENT_BLOCK. Consider using expect with a meaningful message instead. Dynamic Field Logging In the eth_send_raw_transaction function, dynamic fields are being logged directly in the message. These should be added as tracing fields instead.

[github-actions]

PR Code Suggestions ✨

Category	Suggestion	Score
Possible bug	Fix a recursive call in the forward function to prevent a stack overflow The forward function in the Raft implementation of the Consensus trait calls itself recursively, which will result in a stack overflow. It should call the forward method from the trait instead. src/eth/consensus/raft/mod.rs [620] -let (tx_hash, url) = self.forward(transaction).await?; +let (tx_hash, url) = Consensus::forward(self, transaction).await?; Suggestion importance[1-10]: 10 Why: This suggestion addresses a critical bug that could cause a stack overflow, which is essential for the stability of the application.	10
	Replace the not function with the ! operator to correct the syntax The not function used in the stratus_health function is not a standard Rust function. It should be replaced with the ! operator. src/eth/rpc/rpc_server.rs [254] -if not(should_serve) { +if !should_serve { Suggestion importance[1-10]: 9 Why: The not function is not standard in Rust, and using the ! operator is correct syntax. This fix is crucial for the code to compile and run correctly.	9
Maintainability	Correct a typo in an error message The error message "validator and replica are too far appart" contains a typo. The word "appart" should be corrected to "apart". src/eth/consensus/mod.rs [27] -tracing::info!(?lag, "validator and replica are too far appart"); +tracing::info!(?lag, "validator and replica are too far apart"); Suggestion importance[1-10]: 8 Why: Correcting typos in error messages improves code readability and professionalism. This is a minor but important fix for maintainability.	8
Best practice	Remove explicit return statements in the should_serve function to make the code more idiomatic The should_serve function can be improved by removing the explicit return statements, making the code more idiomatic. src/eth/consensus/mod.rs [20-30] -return false; +false ... -return should_serve; +should_serve Suggestion importance[1-10]: 7 Why: Removing explicit return statements makes the code more idiomatic and easier to read, which is a best practice in Rust.	7