fix/PRO-3396/userOp-retry-sending-options #183
Conversation
|
Error: Could not generate a valid Mermaid diagram after multiple attempts. Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
npm error Exit handler never called! ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
src/types/EtherspotTransactionKit.ts (1)
205-209: Well-designed type definition for retry options.The
SendOptionstype is well-structured with appropriate optional properties. Consider adding JSDoc comments to document expected value ranges and behavior, especially formaxRetriesandfeeMultiplier.Consider adding JSDoc documentation:
+/** + * Options for configuring transaction sending behavior + */ export type SendOptions = { + /** Enable retry mechanism when transaction fails due to low fees */ retryOnFeeTooLow?: boolean; + /** Maximum number of retry attempts (should be positive) */ maxRetries?: number; + /** Fee multiplier for retries (should be > 1.0) */ feeMultiplier?: number; };CHANGELOG.md (1)
7-7: Fix typo in changelog entry.There's a typo in the changelog description: "useOp.maxFeePerGas" should be "userOp.maxFeePerGas" to match standard UserOperation naming conventions.
Apply this fix:
-- Added `SendOptions` to the `send()` function, allowing an optional retry mechanism if the `useOp.maxFeePerGas` and `userOp.maxPriorityFeePerGas` are too low. +- Added `SendOptions` to the `send()` function, allowing an optional retry mechanism if the `userOp.maxFeePerGas` and `userOp.maxPriorityFeePerGas` are too low.src/providers/EtherspotTransactionKitContextProvider.tsx (2)
219-231: Extract duplicated fee multiplier calculation.The conversion of
feeMultiplierto BigInt is duplicated. Consider extracting it to improve maintainability.attempt++; + const feeMultiplierBigInt = BigInt(Math.floor(feeMultiplier * 100)); + const maxFee = BigInt(userOp.maxFeePerGas.toString()); - const maxFeeMultiplier = BigInt(Math.floor(feeMultiplier * 100)); - const maxNewFee = (maxFee * maxFeeMultiplier) / BigInt(100); + const maxNewFee = (maxFee * feeMultiplierBigInt) / BigInt(100); const maxPriorityFee = BigInt( userOp.maxPriorityFeePerGas.toString() ); - const maxFeePriorityMultiplier = BigInt( - Math.floor(feeMultiplier * 100) - ); const maxPriorityNewFee = - (maxPriorityFee * maxFeePriorityMultiplier) / BigInt(100); + (maxPriorityFee * feeMultiplierBigInt) / BigInt(100);
210-210:Details
❓ Verification inconclusive
Consider more robust error detection.
The current implementation relies on string matching for error detection. If the SDK provides error codes or specific error types, consider using those for more reliable error detection.
Let me check if the SDK provides more structured error information:
🏁 Script executed:
#!/bin/bash # Search for error handling patterns in the SDK usage rg -A 5 "getUserOpReceipt|send\(" --type ts | grep -E "(error|Error|catch)" -A 3Length of output: 1272
🏁 Script executed:
#!/bin/bash # Locate the definition of parseEtherspotErrorMessage to inspect its implementation rg -n "parseEtherspotErrorMessage" -A10Length of output: 4163
Consider using structured SDK error codes instead of string matching
Relying on
errorMessage.includes('fee too low')is fragile. If the Etherspot SDK exposes error codes (e.g.error.codeor custom error classes), switch to checking those for more reliable detection.• src/providers/EtherspotTransactionKitContextProvider.tsx (around line 210)
• Suggested update:- const isFeeTooLow = errorMessage.includes('fee too low'); + // TODO: replace substring match with SDK‐provided error code, e.g. error.code === 'FEE_TOO_LOW' + const isFeeTooLow = errorMessage.includes('fee too low');Please verify in the SDK documentation whether such codes or types exist and adjust accordingly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
example/package-lock.jsonis excluded by!**/package-lock.jsonpackage-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (5)
CHANGELOG.md(1 hunks)package.json(1 hunks)src/contexts/EtherspotTransactionKitContext.tsx(2 hunks)src/providers/EtherspotTransactionKitContextProvider.tsx(5 hunks)src/types/EtherspotTransactionKit.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/contexts/EtherspotTransactionKitContext.tsx (1)
src/types/EtherspotTransactionKit.ts (2)
SendOptions(205-209)ISentBatches(53-55)
🔇 Additional comments (5)
package.json (1)
4-4: Appropriate semantic version bump.The patch version increment from "1.1.0" to "1.1.1" is correct for this backward-compatible feature addition.
src/contexts/EtherspotTransactionKitContext.tsx (2)
8-8: Correct import addition for SendOptions type.The import statement properly includes the new
SendOptionstype.
17-20: Well-implemented interface update maintaining backward compatibility.The
sendmethod signature update correctly adds the optionalSendOptionsparameter while preserving existing functionality. The parameter is properly typed and optional, ensuring backward compatibility.src/providers/EtherspotTransactionKitContextProvider.tsx (2)
156-164: Good implementation of backward-compatible API enhancement.The optional
optionsparameter with sensible defaults ensures existing code continues to work while enabling the new retry functionality when needed.
270-271: Clean integration of retry logic.Good abstraction of the retry logic into a separate function while maintaining the existing error handling flow.
| } | ||
| } | ||
|
|
||
| throw new Error('Failed to send after retries.'); |
There was a problem hiding this comment.
Misleading error message.
This error message will only be reached when retrying due to "fee too low" errors. Consider making it more specific.
- throw new Error('Failed to send after retries.');
+ throw new Error(`Failed to send after ${maxRetries} retries with fee adjustments.`);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| throw new Error('Failed to send after retries.'); | |
| throw new Error(`Failed to send after ${maxRetries} retries with fee adjustments.`); |
🤖 Prompt for AI Agents
In src/providers/EtherspotTransactionKitContextProvider.tsx at line 243, the
error message "Failed to send after retries." is too generic and misleading
because it only occurs after retries due to "fee too low" errors. Update the
error message to specifically mention that the failure is due to repeated "fee
too low" errors to provide clearer context.
Description
How Has This Been Tested?
Screenshots (if appropriate):
Types of changes
Summary by CodeRabbit
New Features
Chores