Legacy Fallback Support for Non EIP-1559 Chains

[bh2smith]

User description

Fallback to legacy transaction for non-eip1559 chains should give a bit of additional chain support.

Turns out this one particular network still doesn't support Legacy

    Received promise rejected instead of resolved
    Rejected to value: [InvalidInputRpcError: Missing or invalid parameters.
    Double check you have provided the correct parameters.·
    URL: https://rpc.startale.com/astar-zkevm
    Request body: {"method":"eth_sendRawTransaction","params":["0xe480838a3ea082520894759e10411dda5138e331b7ad5ce1b937550db7378080820ec08080"]}·
    Details: invalid sender
    Version: 2.21.1]

Unfortunately, it will not suffice to simply replace gas fee data. We may also have to change the signature (to incorporate EIP-155.

---

PR Type

enhancement, tests

---

Description

• Added getGasData function to handle gas fee estimation for both EIP-1559 and legacy transactions.
• Modified populateTx function to use getGasData for gas fee values.
• Introduced error handling for non-EIP-1559 chains.
• Added unit test for populating non-EIP-1559 transactions.

---

Changes walkthrough 📝

	Relevant files
Enhancement	transaction.ts Add legacy fallback support for non-EIP-1559 chains            src/utils/transaction.ts Added getGasData function to handle gas fee estimation for both EIP-1559 and legacy transactions. Modified populateTx function to use getGasData for gas fee values. Introduced error handling for non-EIP-1559 chains. +21/-8
Tests	utils.transaction.test.ts Add unit test for legacy transaction support                          tests/unit/utils.transaction.test.ts Added unit test for populating non-EIP-1559 transactions. Included serializeTransaction import for testing purposes. +10/-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 🔵🔵🔵⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Key issues to review Error Handling The error handling in getGasData function might not catch all relevant errors. Consider handling generic errors as well. Logging Using console.warn for logging might not be suitable for production environments. Consider using a more robust logging framework.

[mintbase-codium-pr-agent]

PR Code Suggestions ✨

Category	Suggestion	Score
Error handling	Add error handling for potential failures in fetching gas prices Add error handling for the case where provider.getGasPrice() might fail or throw an exception, to prevent unhandled promise rejections. src/utils/transaction.ts [83] -return { gasPrice: await provider.getGasPrice() }; +try { + return { gasPrice: await provider.getGasPrice() }; +} catch (error) { + logger.error('Failed to get gas price:', error); + throw error; +} Suggestion importance[1-10]: 9 Why: Adding error handling for provider.getGasPrice() prevents unhandled promise rejections, which is crucial for maintaining application stability and reliability.	9
	Add a fallback for chainId in populateTx to handle missing values Ensure that the populateTx function handles the scenario where chainId is not provided in the transaction object, to avoid potential runtime errors. src/utils/transaction.ts [54-58] return { ...transactionData, ...gasValues, gas: estimatedGas, - chainId, + chainId: tx.chainId ?? defaultChainId, }; Suggestion importance[1-10]: 8 Why: Ensuring that populateTx handles scenarios where chainId is not provided prevents potential runtime errors, improving the robustness of the function.	8
Maintainability	Refactor getGasData to improve modularity by separating fee fetching logic Refactor the getGasData function to separate concerns between fetching EIP-1559 and legacy gas data into distinct functions for better modularity and readability. src/utils/transaction.ts [73-84] export async function getGasData( provider: PublicClient ): Promise<FeeValuesEIP1559 | FeeValuesLegacy> { try { - return await provider.estimateFeesPerGas(); + return await fetchEIP1559Fees(provider); } catch (error: unknown) { if (error instanceof Eip1559FeesNotSupportedError) { - console.warn(`${error.shortMessage} Using Legacy Gas Fees`); + logger.warn(`${error.shortMessage} Using Legacy Gas Fees`); + return fetchLegacyFees(provider); } - return { gasPrice: await provider.getGasPrice() }; + throw error; } } +async function fetchEIP1559Fees(provider: PublicClient): Promise<FeeValuesEIP1559> { + return await provider.estimateFeesPerGas(); +} +async function fetchLegacyFees(provider: PublicClient): Promise<FeeValuesLegacy> { + return { gasPrice: await provider.getGasPrice() }; +} Suggestion importance[1-10]: 8 Why: Refactoring the getGasData function to separate concerns enhances code readability and maintainability, making it easier to manage and test individual parts of the code.	8
Best practice	Replace console logging with a more robust logging framework Replace the use of console.warn with a more robust logging mechanism that allows for better control over logging levels and formats, especially for production environments. src/utils/transaction.ts [81] -console.warn(`${error.shortMessage} Using Legacy Gas Fees`); +logger.warn(`${error.shortMessage} Using Legacy Gas Fees`); Suggestion importance[1-10]: 7 Why: Using a robust logging framework instead of console.warn is a best practice for better control over logging levels and formats, especially in production environments. However, it is not a critical issue.	7