feat: forward to leader repass client to leader and leader response to client

[dinhani-cw]

PR Type

Enhancement, Bug fix, Tests

---

Description

• Updated error handling for EVM transactions by changing error type from TransactionFailed to TransactionEvmFailed.
• Added new method forward_to_leader for forwarding transactions to the leader in the consensus module.
• Implemented Deserialize for RpcClientApp.
• Updated RPC middleware and server to handle client information and forward transactions to the leader if necessary.
• Added new error types TransactionLeaderFailed and TransactionForwardToLeaderFailed.
• Removed pending_transaction module and related handling.
• Added new test case for sending raw transactions.
• Updated chain ID for tests to 3000.
• Added alias run-leader for running the leader.

---

Changes walkthrough 📝

	Relevant files
Bug fix	1 files evm.rs Update error type for EVM transaction failures                      src/eth/executor/evm.rs Changed error type from TransactionFailed to TransactionEvmFailed. +1/-1
Enhancement	9 files executor.rs Add logging and rename variables in transaction execution src/eth/executor/executor.rs Added logging for transaction execution. Renamed tx_input to tx. Updated transaction execution logic to use the new variable name. +11/-11  consensus.rs Add method to forward transactions to leader                          src/eth/follower/consensus.rs Added method forward_to_leader for forwarding transactions to the leader. Updated imports to include StratusError and RpcClientApp. +7/-9      stratus_error.rs Add new error types for leader transaction failures            src/eth/primitives/stratus_error.rs Added new error types TransactionLeaderFailed and TransactionForwardToLeaderFailed. Updated From implementation to handle TransactionLeaderFailed. +12/-2    rpc_client_app.rs Implement Deserialize for RpcClientApp                                      src/eth/rpc/rpc_client_app.rs Implemented Deserialize for RpcClientApp. +13/-0    rpc_middleware.rs Update RPC middleware to handle client information              src/eth/rpc/rpc_middleware.rs Updated RpcServiceT to handle client information in transaction tracing. Added client information to TransactionTracingIdentifiers. +14/-4    rpc_server.rs Refactor transaction forwarding and execution logic            src/eth/rpc/rpc_server.rs Updated eth_send_raw_transaction to forward transactions to the leader if not the leader. Refactored transaction parsing and execution logic. +19/-30  blockchain_client.rs Add method to forward transactions to leader                          src/infra/blockchain_client/blockchain_client.rs Added method send_raw_transaction_to_leader for forwarding transactions. Removed PendingTransaction usage. +13/-7    mod.rs Remove pending_transaction module                                                src/infra/blockchain_client/mod.rs Removed pending_transaction module. +0/-1      pending_transaction.rs Remove pending transaction handling                                            src/infra/blockchain_client/pending_transaction.rs Removed the entire file. +0/-181
Tests	2 files e2e-tx-serial-transfer.test.ts Add test case for sending raw transactions                              e2e/test/automine/e2e-tx-serial-transfer.test.ts Added a new test case for sending raw transactions. +7/-0      rpc.ts Update chain ID for tests                                                                e2e/test/helpers/rpc.ts Updated CHAIN_ID_DEC to 3000. +1/-1
Configuration changes	1 files justfile Add alias for running leader                                                          justfile Added alias run-leader for running the leader. +2/-1

---

💡 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 🔍

(Review updated until commit 8e7cb5d)

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

🧪 PR contains tests

🔒 No security concerns identified

⚡ Key issues to review Error Handling The error type has been changed from TransactionFailed to TransactionEvmFailed, but the new error type is being created with e.to_string(). Consider preserving the original error type or adding more context to the error message. Missing Span Fields The execute_local_transaction function is instrumented with tracing::instrument, but no span fields are recorded. Consider adding relevant identifiers as span fields. Logging Improvement The forward_to_leader function logs the rpc_client using tracing::info!. Ensure that the rpc_client is logged with structured fields instead of formatting the entire object. Blocking Call in Async Context The block_on call in the eth_send_raw_transaction function can block the async context. Consider using spawn_blocking_named to avoid blocking the Tokio async runtime. Error Logging When logging errors in send_raw_transaction_to_leader, ensure the original error is logged in a field called reason.

[dinhani-cw]

/describe

[github-actions]

PR Description updated to latest commit (8e7cb5d)

[github-actions]

Persistent review updated to latest commit 8e7cb5d

[github-actions]

PR Code Suggestions ✨

Latest suggestions up to 8e7cb5d

Category	Suggestion	Score
Possible issue	Add error handling for the get_chain method to provide more context in case of failure Add error handling for the get_chain method to provide more context in case of failure. src/eth/follower/consensus.rs [41] -let hash = self.get_chain()?.send_raw_transaction_to_leader(tx_data.into(), rpc_client).await?; +let chain = self.get_chain().map_err(|e| StratusError::ChainUnavailable(e.to_string()))?; +let hash = chain.send_raw_transaction_to_leader(tx_data.into(), rpc_client).await?; Suggestion importance[1-10]: 8 Why: Adding error handling for the get_chain method enhances robustness and provides more context in case of failure, which is crucial for debugging and reliability.	8
	Add a check to ensure that tx_client is not None before inserting it into the request extensions Add a check to ensure that tx_client is not None before inserting it into the request extensions. src/eth/rpc/rpc_middleware.rs [88-91] if let Some(tx_client) = tx.as_ref().and_then(|tx| tx.client.clone()) { request.extensions_mut().insert(tx_client.clone()); client = tx_client; +} else { + tracing::warn!("Transaction client is None"); } Suggestion importance[1-10]: 6 Why: Adding a check for None improves robustness and ensures that only valid clients are inserted into the request extensions. However, the impact is relatively minor as tx_client being None might be an edge case.	6
Best practice	Use a more specific error type for TransactionEvmFailed instead of converting the error to a string Consider using a more specific error type for TransactionEvmFailed instead of converting the error to a string. This will provide more context and make error handling easier. src/eth/executor/evm.rs [168] -Err(StratusError::TransactionEvmFailed(e.to_string())) +Err(StratusError::TransactionEvmFailed(e)) Suggestion importance[1-10]: 7 Why: Using a more specific error type can provide better context and improve error handling. However, this change might require additional modifications in other parts of the codebase to handle the new error type properly.	7
Enhancement	Add a log statement to indicate the start of transaction execution Add a log statement to indicate when the transaction execution starts, which will help in tracing and debugging. src/eth/executor/executor.rs [363] -tracing::info!(tx_hash = %tx.hash, "executing local transaction"); +tracing::info!(tx_hash = %tx.hash, "starting execution of local transaction"); Suggestion importance[1-10]: 5 Why: The suggested change improves logging clarity but is a minor enhancement. The existing log statement already provides useful information.	5

---

Previous suggestions

Suggestions up to commit 7617de0

Category	Suggestion	Score
Performance	Change the function parameter to take a reference instead of consuming the RpcClientApp The forward_to_leader function should take a reference to RpcClientApp instead of consuming it. This will avoid unnecessary cloning and improve performance. src/eth/consensus/mod.rs [35] -async fn forward_to_leader(&self, transaction: Bytes, rpc_client: RpcClientApp) -> anyhow::Result<Hash> { +async fn forward_to_leader(&self, transaction: Bytes, rpc_client: &RpcClientApp) -> anyhow::Result<Hash> { Suggestion importance[1-10]: 9 Why: This suggestion is correct and improves performance by avoiding unnecessary cloning. It is contextually accurate and derived from the PR code diff.	9
	Change the method call to pass a reference instead of consuming the RpcClientApp The send_raw_transaction method should take a reference to RpcClientApp instead of consuming it. This will avoid unnecessary cloning and improve performance. src/eth/consensus/raft/mod.rs [500] -let tx_hash = blockchain_client.send_raw_transaction(transaction.into(), RpcClientApp::Unknown).await?; +let tx_hash = blockchain_client.send_raw_transaction(transaction.into(), &RpcClientApp::Unknown).await?; Suggestion importance[1-10]: 9 Why: This suggestion is correct and improves performance by avoiding unnecessary cloning. It is contextually accurate and derived from the PR code diff.	9
	Change the function call to pass a reference instead of consuming the RpcClientApp The forward_to_leader function should take a reference to RpcClientApp instead of consuming it. This will avoid unnecessary cloning and improve performance. src/eth/rpc/rpc_server.rs [649] -Some(consensus) => match Handle::current().block_on(consensus.forward_to_leader(tx_data, ext.rpc_client())) { +Some(consensus) => match Handle::current().block_on(consensus.forward_to_leader(tx_data, &ext.rpc_client())) { Suggestion importance[1-10]: 9 Why: This suggestion is correct and improves performance by avoiding unnecessary cloning. It is contextually accurate and derived from the PR code diff.	9
	Change the method parameter to take a reference instead of consuming the RpcClientApp The send_raw_transaction method should take a reference to RpcClientApp instead of consuming it. This will avoid unnecessary cloning and improve performance. src/infra/blockchain_client/blockchain_client.rs [214] -pub async fn send_raw_transaction(&self, tx: EthersBytes, rpc_client: RpcClientApp) -> anyhow::Result<Hash> { +pub async fn send_raw_transaction(&self, tx: EthersBytes, rpc_client: &RpcClientApp) -> anyhow::Result<Hash> { Suggestion importance[1-10]: 9 Why: This suggestion is correct and improves performance by avoiding unnecessary cloning. It is contextually accurate and derived from the PR code diff.	9