feat: improve performance by not decoding sender when dealing with imported external transactions

[dinhani-cw]

No description provided.

[github-actions]

PR Review

⏱️ Estimated effort to review [1-5]	2, because the changes are localized to specific files with a clear focus on improving performance by optimizing transaction handling. The modifications are not extensive and involve mostly refactoring and reorganization of existing logic.
🧪 Relevant tests	No
🔍 Possible issues	Possible Bug: The removal of decoding for the sender in try_from_ethers_transaction when compute_signer is false might lead to unexpected behavior if the sender's address is required downstream in the system. This change assumes that the sender's address is not needed, which should be verified.
🔒 Security concerns	No

Code feedback:

---

relevant file	src/eth/primitives/transaction_input.rs
suggestion	Consider adding error handling for the case where gas_price is None in the try_from_ethers_transaction function. Currently, it logs an error and returns, but you might want to handle this more gracefully or ensure that transactions without a gas price are filtered out earlier in the process. [important]
relevant line	None => return log_and_err!("transaction without gas_price id is not allowed"),

---

relevant file	src/eth/primitives/transaction_input.rs
suggestion	Refactor the repeated logic for converting chain_id, nonce, gas_limit, and other fields into a separate function or macro to reduce code duplication and improve maintainability. This can help centralize changes to the conversion logic in the future. [medium]
relevant line	chain_id: match value.chain_id {

---

relevant file	src/eth/primitives/transaction_input.rs
suggestion	Ensure that the from field conversion using Address::new(value.from.into()) is necessary and correctly handles all expected input cases, especially since from is directly set without additional checks when compute_signer is false. [medium]
relevant line	false => value.from.into(),

---

---

✨ Review tool usage guide:

---

Overview:
The review tool scans the PR code changes, and generates a PR review which includes several types of feedbacks, such as possible PR issues, security threats and relevant test in the PR. More feedbacks can be added by configuring the tool.

The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on any PR.

• When commenting, to edit configurations related to the review tool (pr_reviewer section), use the following template:

/review --pr_reviewer.some_config1=... --pr_reviewer.some_config2=...

• With a configuration file, use the following template:

[pr_reviewer]
some_config1=...
some_config2=...

See the review usage page for a comprehensive guide on using this tool.

[github-actions]

PR Code Suggestions

Category	Suggestions
Bug	Add error handling for missing from field in transactions. Consider handling the case where value.from might be None in the try_from_ethers_transaction function. This will prevent potential runtime errors when value.from is not provided. src/eth/primitives/transaction_input.rs [120] -false => value.from.into(), +false => value.from.ok_or(anyhow!("`from` field is missing in the transaction"))?.into(),
	Replace undefined macro with standard error handling. Replace the log_and_err! macro with a proper error handling mechanism, as the macro is not defined in the provided code snippets. This will ensure that the code compiles and provides meaningful error messages. src/eth/primitives/transaction_input.rs [126] -None => return log_and_err!("transaction without gas_price id is not allowed"), +None => return Err(anyhow!("Transaction without gas_price is not allowed")),
Maintainability	Refactor repeated conversion logic into separate functions. Refactor the repeated logic for converting chain_id, nonce, gas, and other fields into a separate function to improve code maintainability and reduce redundancy. src/eth/primitives/transaction_input.rs [130-146] -chain_id: match value.chain_id { - Some(chain_id) => Some(chain_id.try_into()?), - None => None, -}, -hash: value.hash.into(), -nonce: value.nonce.try_into()?, -signer, -from: Address::new(value.from.into()), -to: value.to.map_into(), -value: value.value.into(), -input: value.input.clone().into(), -gas_limit: value.gas.try_into()?, -gas_price, -v: value.v, -r: value.r, -s: value.s, +TransactionInput { + chain_id: convert_chain_id(value.chain_id)?, + hash: value.hash.into(), + nonce: convert_nonce(value.nonce)?, + signer, + from: convert_from(value.from)?, + to: value.to.map_into(), + value: value.value.into(), + input: value.input.clone().into(), + gas_limit: convert_gas(value.gas)?, + gas_price: convert_gas_price(value.gas_price)?, + v: value.v, + r: value.r, + s: value.s, +}
Enhancement	Ensure robustness by adding unit tests for the transaction conversion function. Add unit tests for the new try_from_ethers_transaction function to ensure that it handles various edge cases correctly, such as missing fields or invalid data. src/eth/primitives/transaction_input.rs [110] -fn try_from_ethers_transaction(value: EthersTransaction, compute_signer: bool) -> anyhow::Result<TransactionInput> { +#[cfg(test)] +mod tests { + use super::*; + #[test] + fn test_try_from_ethers_transaction_missing_from() { + let transaction = EthersTransaction { from: None, ..Default::default() }; + assert!(try_from_ethers_transaction(transaction, false).is_err()); + } + + #[test] + fn test_try_from_ethers_transaction_valid_data() { + let transaction = EthersTransaction { from: Some("0x123".into()), ..Default::default() }; + assert!(try_from_ethers_transaction(transaction, true).is_ok()); + } +} +
Best practice	Use more descriptive variable names for clarity. Consider using more descriptive variable names in the try_from_ethers_transaction function to enhance code readability and maintainability. src/eth/primitives/transaction_input.rs [112] -let signer: Address = match compute_signer { +let transaction_signer: Address = match compute_signer {

---

✨ Improve tool usage guide:

---

Overview:
The improve tool scans the PR code changes, and automatically generates suggestions for improving the PR code. The tool can be triggered automatically every time a new PR is opened, or can be invoked manually by commenting on a PR.

• When commenting, to edit configurations related to the improve tool (pr_code_suggestions section), use the following template:

/improve --pr_code_suggestions.some_config1=... --pr_code_suggestions.some_config2=...

• With a configuration file, use the following template:

[pr_code_suggestions]
some_config1=...
some_config2=...

See the improve usage page for a comprehensive guide on using this tool.

[renancloudwalk]

Nice