-
Notifications
You must be signed in to change notification settings - Fork 9
Release 17.03.2025 #194
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 17.03.2025 #194
Conversation
WalkthroughThe pull request updates the backend software version from 3.1.2 to 3.1.3. The changelog now includes a new entry documenting a fix for verifying Paymaster execution on undeployed wallets alongside an earlier cron job fix. Additionally, the Changes
Sequence Diagram(s)sequenceDiagram
participant Client as UserOp Initiator
participant PM as Paymaster.signV07
participant Pack as packPaymasterData
participant GP as getPaymasterData
Client->>PM: Call signV07(userOp, estimate)
alt Estimate flag true
PM->>PM: Set paymaster, verification, and post op gas limits
PM->>Pack: Generate paymasterAndData using updated gas limit
Pack-->>PM: Return paymasterAndData
PM->>GP: Asynchronously fetch paymasterData
GP-->>PM: Return paymasterData
else No estimation changes
PM->>PM: Process userOp normally
end
PM-->>Client: Return modified userOp with correct gas limits
Suggested reviewers
Poem
Tip ⚡🧪 Multi-step agentic review comment chat (experimental)
✨ 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.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
backend/CHANGELOG.md
(1 hunks)backend/package.json
(1 hunks)backend/src/paymaster/index.ts
(3 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 (4)
backend/src/paymaster/index.ts (3)
115-115
: Good: A defined constant for paymasterPostOpGasLimit is now used consistently.Adding a fixed constant for post-op gas limit (40000) ensures consistent gas limit usage throughout the function, preventing possible inconsistencies in gas limit values.
149-149
: Consistent use of paymasterPostOpGasLimit in paymasterAndData packing.Using the constant
paymasterPostOpGasLimit
here ensures consistency with the earlier code, replacing what was likely a hardcoded value previously.
163-163
: Consistent use of paymasterPostOpGasLimit in return value.Using the constant
paymasterPostOpGasLimit
in the return value ensures consistency with the rest of the function, replacing what was previously likely a hardcoded "0x1" value.backend/package.json (1)
3-3
: Version bump corresponds to the bug fix.The version update from 3.1.2 to 3.1.3 properly follows semantic versioning for a bug fix, matching the entry in the changelog.
userOp.paymaster = paymasterAddress; | ||
userOp.paymasterVerificationGasLimit = this.EP7_PVGL; | ||
userOp.paymasterPostOpGasLimit = paymasterPostOpGasLimit; | ||
const accountGasLimits = this.packUint(userOp.verificationGasLimit, userOp.callGasLimit) | ||
const gasFees = this.packUint(userOp.maxPriorityFeePerGas, userOp.maxFeePerGas); | ||
const packedUserOp = { | ||
sender: userOp.sender, | ||
nonce: userOp.nonce, | ||
initCode: userOp.initCode, | ||
callData: userOp.callData, | ||
accountGasLimits: accountGasLimits, | ||
preVerificationGas: userOp.preVerificationGas, | ||
gasFees: gasFees, | ||
paymasterAndData: this.packPaymasterData(paymasterAddress, this.EP7_PVGL, paymasterPostOpGasLimit), | ||
signature: userOp.signature | ||
} | ||
userOp.paymasterData = await this.getPaymasterData(packedUserOp, validUntil, validAfter, paymasterContract, signer); |
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.
Critical bug fix: Paymaster data is now correctly included before estimation.
This change fixes the core issue with the Verifying Paymaster by ensuring that when estimate
is true, the paymaster data is properly included in the user operation before it's sent to the bundler for estimation. Previously, this data was missing during estimation for undeployed wallets.
The key improvements:
- Setting
userOp.paymaster
to the paymaster address - Adding proper gas limits via
paymasterVerificationGasLimit
andpaymasterPostOpGasLimit
- Creating a properly formatted
packedUserOp
with all required fields - Fetching and applying
paymasterData
before estimation
This ensures the bundler can correctly estimate gas costs for operations involving undeployed wallets.
## [3.1.3] - 2025-03-13 | ||
### Fixes | ||
- Fixed Verifying Paymaster execution for undeployed wallets | ||
|
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)
Changelog properly documents the bug fix.
The changelog entry clearly describes the issue that was fixed: "Fixed Verifying Paymaster execution for undeployed wallets." This aligns with the code changes in the paymaster module.
There are some minor markdown formatting issues though - headings should be surrounded by blank lines according to markdown best practices.
# Changelog
+
## [3.1.3] - 2025-03-13
+
### Fixes
+
- Fixed Verifying Paymaster execution for undeployed wallets
📝 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.3] - 2025-03-13 | |
### Fixes | |
- Fixed Verifying Paymaster execution for undeployed wallets | |
# Changelog | |
## [3.1.3] - 2025-03-13 | |
### Fixes | |
- Fixed Verifying Paymaster execution for undeployed wallets |
🧰 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)
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.
LGTM
Description
Types of changes
What types of changes does your code introduce?
Further comments (optional)
Summary by CodeRabbit