Improve encodeSignRequest

[bh2smith]

User description

Eliminate recovery in place of hash and add chainId.

---

PR Type

Enhancement, Bug fix

---

Description

• Removed the safeMessageTxData function and eliminated the use of RecoveryData in src/lib/safe-message.ts.
• Updated the encodeSignRequest method in src/tx-manager.ts to return EncodedTxData and removed RecoveryData logic.
• Added a new EncodedTxData interface in src/types.ts to include chainId, data, and hash in evmData.

---

Changes walkthrough 📝

Relevant files

Enhancement

safe-message.ts Remove `safeMessageTxData` and `RecoveryData` usage            src/lib/safe-message.ts Removed safeMessageTxData function. Eliminated the use of RecoveryData and toPayload. +1/-26    tx-manager.ts Update `encodeSignRequest` and remove `RecoveryData`          src/tx-manager.ts Modified encodeSignRequest to return EncodedTxData. Updated requestRouter to include hash. Removed RecoveryData and related logic. +34/-28  types.ts Add `EncodedTxData` interface with `evmData` details          src/types.ts Added EncodedTxData interface. Included chainId, data, and hash in evmData. +12/-1

---

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

[mintbase-codium-pr-agent]

PR Reviewer Guide 🔍

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

🧪 No relevant tests

🔒 No security concerns identified

⚡ Key issues to review Possible Bug The method encodeSignRequest in TransactionManager now returns EncodedTxData instead of NearEthTxData. Ensure that all dependent components and systems correctly handle the new return type, especially focusing on the new evmData structure which includes chainId, data, and hash. Refactoring Needed The removal of RecoveryData and addition of hash in various methods could lead to potential issues if not handled properly across all usages. Review all usages of these methods to ensure that they are updated to work with the new structure and that there are no remnants of the old RecoveryData usage that could cause runtime errors.

[mintbase-codium-pr-agent]

PR Code Suggestions ✨

Category	Suggestion	Score
Security	Validate the chainId to prevent cross-chain replay attacks To ensure the integrity and security of the transaction, it's important to validate the chainId before using it in the evmData. This can prevent issues related to cross-chain replay attacks. src/tx-manager.ts [172] -chainId: signRequest.chainId, +chainId: validateChainId(signRequest.chainId), Suggestion importance[1-10]: 9 Why: Validating the chainId is crucial for preventing cross-chain replay attacks, which is a significant security concern. This suggestion addresses a major security issue.	9
Error handling	Add error handling for the requestRouter function to manage exceptions Consider adding error handling for the asynchronous operation this.requestRouter(signRequest, usePaymaster) to manage potential failures or exceptions gracefully. src/tx-manager.ts [161-163] -const { payload, evmMessage, hash } = await this.requestRouter( - signRequest, - usePaymaster -); +let payload, evmMessage, hash; +try { + const response = await this.requestRouter(signRequest, usePaymaster); + payload = response.payload; + evmMessage = response.evmMessage; + hash = response.hash; +} catch (error) { + console.error("Failed to process requestRouter:", error); + throw new Error("Request processing failed"); +} Suggestion importance[1-10]: 8 Why: Adding error handling improves the robustness of the code by ensuring that potential failures in the asynchronous operation are managed gracefully. This is important for maintaining application stability.	8
Possible bug	Check for null or undefined hash values before usage It's a good practice to ensure that the hash value is not null or undefined before using it in the transaction payload to avoid potential runtime errors. src/tx-manager.ts [174] -hash, +hash: hash ?? throw new Error("Hash value is missing"), Suggestion importance[1-10]: 7 Why: Ensuring that the hash value is not null or undefined before usage helps prevent potential runtime errors, improving the reliability of the code.	7
Maintainability	Refactor repeated Ethereum signing logic into a separate function To improve the readability and maintainability of the code, consider refactoring the repeated logic for handling different Ethereum signing methods into a separate function. src/tx-manager.ts [284-290] -const [_, messageOrData] = params as EthSignParams; -const message = decodeSafeMessage(messageOrData, safeInfo); -return { - evmMessage: message.safeMessageMessage, - payload: toPayload(message.safeMessageHash), - hash: message.safeMessageHash, -}; +return processEthSign(params, safeInfo); +// Define a new function +function processEthSign(params: EthSignParams, safeInfo: SafeInfo) { + const [_, messageOrData] = params; + const message = decodeSafeMessage(messageOrData, safeInfo); + return { + evmMessage: message.safeMessageMessage, + payload: toPayload(message.safeMessageHash), + hash: message.safeMessageHash, + }; +} Suggestion importance[1-10]: 6 Why: Refactoring repeated logic into a separate function improves code readability and maintainability, making it easier to manage and understand.	6