-
Notifications
You must be signed in to change notification settings - Fork 9
Release v3.1.6 #200
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Release v3.1.6 #200
Conversation
WalkthroughThis update bumps the backend version from 3.1.5 to 3.1.6. The changelog now documents a fix for the paymaster estimation bug in the Changes
Sequence Diagram(s)sequenceDiagram
participant Client as Client Request
participant Route as paymasterRoutes
participant Provider as JsonRpcProvider
participant Wallet as Wallet (Signer)
participant Paymaster as Paymaster
Client->>Route: Request multi-token quote
Route->>Provider: Create provider (using bundlerUrl)
Route->>Wallet: Create signer (using privateKey & provider)
Route->>Paymaster: Call getQuotesMultiToken(..., signer, ...)
Paymaster-->>Route: Return paymaster data
Route-->>Client: Deliver response
Suggested reviewers
Poem
✨ 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:
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Pls push to prod once @nikhilkumar1612 approves as well,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
backend/CHANGELOG.md
(1 hunks)backend/package.json
(1 hunks)backend/src/paymaster/index.ts
(2 hunks)backend/src/routes/paymaster-routes.ts
(1 hunks)
🧰 Additional context used
🪛 markdownlint-cli2 (0.17.2)
backend/CHANGELOG.md
2-2: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Above
(MD022, blanks-around-headings)
2-2: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
3-3: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Above
(MD022, blanks-around-headings)
3-3: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
4-4: Lists should be surrounded by blank lines
null
(MD032, blanks-around-lists)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Cloudflare Pages
🔇 Additional comments (5)
backend/package.json (1)
3-3
: Version update looks appropriate for the bugfix.The version bump from 3.1.5 to 3.1.6 follows semantic versioning principles for a bugfix release.
backend/src/routes/paymaster-routes.ts (1)
192-194
: The signer integration is critical for the paymaster estimation fix.The addition of provider and signer creation before calling
getQuotesMultiToken
is a key part of the fix. The method now correctly receives a signer parameter, enabling it to generate the necessary paymaster data before estimation.backend/src/paymaster/index.ts (3)
553-554
: Method signature updated to include the signer parameter.The
getQuotesMultiToken
method has been updated to accept asigner
parameter of typeWallet
, which is essential for generating the paymaster data.
578-592
: Added timestamp generation for validation windows.These lines create the necessary timestamps and format them as hexadecimal strings, following the same pattern used in other methods in this class. This is required for signing the paymaster data.
593-594
:✅ Verification successful
Core fix: Generate paymasterAndData before estimation.
This is the critical part of the fix that addresses the bug described in the PR. By generating the
paymasterAndData
for the user operation before estimation, the system can now accurately estimate gas costs even when a user's wallet is undeployed or only contains token balances.Let's verify that the payload to the bundler now includes the correctly formatted paymasterAndData:
🏁 Script executed:
#!/bin/bash # Check if there are any instances where userOp.paymasterAndData is used without being initialized first rg -A 5 "send\('eth_estimateUserOperationGas'" --type tsLength of output: 3645
Verified: Multi-token paymaster now correctly pre-computes paymasterAndData for estimation
The updated code in
backend/src/paymaster/index.ts
(lines 593–594) now sets up the paymaster payload (usinggetPaymasterAndDataForMultiTokenPaymaster
) before the gas estimation process. Our verification via the search script confirms that no instance sends a payload with an uninitialized or misorderedpaymasterAndData
. This change ensures that gas estimation properly accounts for scenarios such as undeployed wallets or wallets holding only token balances.
- Verified that in the multi-token paymaster branch the paymasterAndData is generated early as intended.
- Confirmed that no branch is using an uninitialized paymasterAndData in calls to
eth_estimateUserOperationGas
.
## [3.1.6] - 2025-04-02 | ||
### Fixes | ||
- Fixed bug in paymaster estimation for multiTokenPaymaster getERC20Quotes API | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧹 Nitpick (assertive)
The changelog entry clearly documents the bug fix.
The changelog appropriately includes the version, date, and a concise description of the fixed issue. However, there are some markdown formatting issues in the document.
According to markdown best practices, headings should be surrounded by blank lines for better readability:
1
# Changelog
+
## [3.1.6] - 2025-04-02
+
### Fixes
+
- Fixed bug in paymaster estimation for multiTokenPaymaster getERC20Quotes API
📝 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.
## [3.1.6] - 2025-04-02 | |
### Fixes | |
- Fixed bug in paymaster estimation for multiTokenPaymaster getERC20Quotes API | |
# Changelog | |
## [3.1.6] - 2025-04-02 | |
### Fixes | |
- Fixed bug in paymaster estimation for multiTokenPaymaster getERC20Quotes API |
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
2-2: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Above
(MD022, blanks-around-headings)
2-2: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
3-3: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Above
(MD022, blanks-around-headings)
3-3: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below
(MD022, blanks-around-headings)
4-4: Lists should be surrounded by blank lines
null
(MD032, blanks-around-lists)
Description
Types of changes
What types of changes does your code introduce?
Further comments (optional)
Summary by CodeRabbit