Pro 3206 #302
Pro 3206 #302
Conversation
WalkthroughThis update increments version numbers from 3.0.0 to 3.0.1 across configuration files and multiple packages. In addition, dependency version ranges have been updated accordingly. Several DTO files now use a new validation decorator, replacing the old Ethereum address check with a custom validator. A new custom validator implementation, Changes
Sequence Diagram(s)sequenceDiagram
participant UO as UserOperation
participant ES as EntryPointV8Service
participant P as Provider
participant C as Contract
UO->>ES: Request hash computation
alt eip7702Auth set and factory == "0x7702"
ES->>ES: Pack userOp and build tx
ES->>P: send(tx, stateOverrides)
P-->>ES: Return computed hash
else
ES->>C: getUserOpHash(packedUserOp, {})
C-->>ES: Return computed hash
end
ES->>UO: Return hash result
Suggested reviewers
Poem
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
packages/cli/src/cli.tsOops! Something went wrong! :( ESLint: 8.27.0 TypeError: prettier.resolveConfig.sync is not a function packages/api/src/utils/isValidFactory.tsOops! Something went wrong! :( ESLint: 8.27.0 TypeError: prettier.resolveConfig.sync is not a function packages/executor/src/services/MempoolService/reputation.tsOops! Something went wrong! :( ESLint: 8.27.0 TypeError: prettier.resolveConfig.sync is not a function
Tip ⚡💬 Agentic Chat (Pro Plan, General Availability)
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (13)
✅ Files skipped from review due to trivial changes (8)
🚧 Files skipped from review as they are similar to previous changes (5)
🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
packages/executor/src/services/EntryPointService/versions/0.0.8.ts (1)
74-94: EIP-7702 support added to user operation hash computationThis implementation adds support for the EIP-7702 standard by providing special handling for user operations that use EIP-7702 authentication. When a user operation contains an
eip7702Authproperty and has the factory set to "0x7702", the method uses state overrides to simulate the call, which is essential for computing the correct hash.The code avoids duplicating the
packUserOpcall by storing it in a variable first, which is a good optimization.Consider adding a brief comment explaining the EIP-7702 handling logic to make the code more maintainable, especially for developers who might not be familiar with this specific EIP implementation.
packages/executor/src/services/EntryPointService/utils/packedUserOpUtils.ts (1)
170-187: Consider adding validation for factory and factoryDataWhile there's validation that throws an error when paymaster is present but gas limits are missing, there's no similar validation for factory and factoryData. Consider adding a check to ensure both are properly provided when needed.
- const factory = hexRightPad(op.factory ?? "0x", 20); + let factory = "0x"; + if (op.factory != null) { + factory = hexRightPad(op.factory, 20); + if (op.factoryData == null) { + throw new Error("factory with no factoryData"); + } + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (18)
lerna.json(1 hunks)package.json(1 hunks)packages/api/package.json(2 hunks)packages/api/src/dto/EstimateUserOperation.dto.ts(2 hunks)packages/api/src/dto/SendUserOperation.dto.ts(2 hunks)packages/api/src/utils/index.ts(1 hunks)packages/api/src/utils/isValidFactory.ts(1 hunks)packages/cli/package.json(2 hunks)packages/db/package.json(2 hunks)packages/executor/package.json(2 hunks)packages/executor/src/services/EntryPointService/utils/packedUserOpUtils.ts(4 hunks)packages/executor/src/services/EntryPointService/versions/0.0.8.ts(1 hunks)packages/executor/src/services/MempoolService/reputation.ts(1 hunks)packages/monitoring/package.json(2 hunks)packages/node/package.json(2 hunks)packages/params/package.json(2 hunks)packages/types/package.json(1 hunks)packages/utils/package.json(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
packages/api/src/dto/SendUserOperation.dto.ts (1)
packages/api/src/utils/isValidFactory.ts (1)
IsValidFactory(4-26)
packages/api/src/dto/EstimateUserOperation.dto.ts (1)
packages/api/src/utils/isValidFactory.ts (1)
IsValidFactory(4-26)
packages/executor/src/services/EntryPointService/versions/0.0.8.ts (2)
packages/executor/src/services/EntryPointService/utils/packedUserOpUtils.ts (1)
packUserOp(170-201)packages/executor/src/services/EntryPointService/interfaces.ts (1)
StateOverrides(7-11)
🔇 Additional comments (31)
package.json (1)
4-4: Version Bump in Root package.json
The version number has been updated to "3.0.1", which aligns with the coordinated versioning effort across the project.lerna.json (1)
6-6: Consistent Version Update in Lerna Configuration
The version is correctly updated to "3.0.1", ensuring consistency with the root package.json and the overall project release.packages/api/src/utils/index.ts (1)
4-4: New Export forisValidFactoryAdded
Exporting theisValidFactorymodule broadens the utility API. Ensure that this new validator is comprehensively tested in DTOs where it replaces the old Ethereum address check.packages/db/package.json (2)
7-7: DB Package Version Updated
The DB module’s version has been updated to "3.0.1" to remain in sync with the overall project.
37-37: Dependency Update for@skandha/types
The dependency on@skandha/typeshas been updated to "^3.0.1", ensuring compatibility with other packages. Please verify that the new version does not introduce any unintended breaking changes.packages/monitoring/package.json (2)
7-7: Version increment looks good.Version has been correctly incremented from 3.0.0 to 3.0.1, which aligns with the version updates across the project.
35-35: Dependency version update is consistent.The dependency on
@skandha/typeshas been properly updated to match the new version (^3.0.1).packages/utils/package.json (2)
7-7: Version increment looks good.Version has been correctly incremented from 3.0.0 to 3.0.1, which aligns with the version updates across the project.
36-36: Dependency version update is consistent.The dependency on
@skandha/typeshas been properly updated to match the new version (^3.0.1).packages/executor/src/services/MempoolService/reputation.ts (1)
137-137:✅ Verification successful
Special handling for 7702 transactions implemented correctly.
The modified condition now excludes the factory address "0x7702" from the validation check, which aligns with the PR objective to implement compliance with 7702 transactions for ep8.
🏁 Script executed:
#!/bin/bash # Search for other mentions of the "0x7702" identifier in the codebase to ensure consistent handling rg -A 3 -B 3 "0x7702" --type tsLength of output: 2771
Special handling for 7702 transactions validated across the codebase.
The condition inpackages/executor/src/services/MempoolService/reputation.tsat line 137 correctly excludes the factory address"0x7702"from the validation check. Verification shows that this special handling is consistently applied in other relevant modules (such as inisValidFactory.ts,EntryPointService/versions/0.0.8.ts, andpackedUserOpUtils.ts), ensuring compliance with 7702 transactions for ep8.packages/params/package.json (2)
7-7: Version increment looks good.Version has been correctly incremented from 3.0.0 to 3.0.1, which aligns with the version updates across the project.
31-32: Dependency version updates are consistent.The dependencies on
@skandha/typesand@skandha/utilshave been properly updated to match the new versions (^3.0.1).packages/api/src/dto/SendUserOperation.dto.ts (2)
12-12: Updated import to include custom factory validator.The import statement now correctly includes the new
IsValidFactorydecorator from the utils module.
54-56: Factory property now validated using custom validator to support 0x7702 transactions.The validation has been changed from standard Ethereum address validation to a custom validator that accepts either a valid Ethereum address OR the special value "0x7702" for EP8 compliance.
packages/api/src/dto/EstimateUserOperation.dto.ts (2)
11-11: Updated import to include custom factory validator.The import statement now correctly imports both the existing
IsBigNumberand the newIsValidFactorydecorator.
32-34: Factory property now validated using custom validator to support 0x7702 transactions.Similar to the changes in SendUserOperation.dto.ts, the validation has been changed from standard Ethereum address validation to a custom validator that accepts either a valid Ethereum address OR the special value "0x7702" for EP8 compliance.
packages/api/package.json (2)
7-7: Version bumped to 3.0.1.The package version has been incremented to reflect the new feature addition.
37-40: Updated dependency versions to match package version.All internal @Skandha dependencies have been updated to use version 3.0.1.
packages/executor/package.json (2)
7-7: Version update from 3.0.0 to 3.0.1The package version has been incremented to 3.0.1, which reflects a minor version update.
38-41: Dependency versions synchronized with package versionAll @Skandha dependencies have been updated from ^3.0.0 to ^3.0.1, maintaining consistency across the project's packages.
packages/cli/package.json (2)
7-7: Version update from 3.0.0 to 3.0.1The package version has been incremented to 3.0.1, which is consistent with other package updates in this PR.
43-48: Dependency versions synchronized with package versionAll @Skandha dependencies have been updated from ^3.0.0 to ^3.0.1, maintaining version consistency across the project.
packages/node/package.json (2)
7-7: Version update from 3.0.0 to 3.0.1The package version has been incremented to 3.0.1, maintaining consistency with other packages in this PR.
59-65: Dependency versions synchronized with package versionAll @Skandha dependencies have been updated from ^3.0.0 to ^3.0.1, ensuring version compatibility across the project.
packages/executor/src/services/EntryPointService/utils/packedUserOpUtils.ts (7)
16-27: Good addition of a helper function for hex right paddingThe
hexRightPadfunction is well implemented with proper error handling for cases where the input exceeds the target length. This will help standardize the handling of factory addresses in user operations.
188-189: Improved handling of factory addresses with paddingUsing the new
hexRightPadfunction ensures the factory address is properly formatted to the required 20 bytes, which is important for consistent encoding of user operations.
192-193: Verify the conditional initCode generationThe logic for generating the initCode has been updated to incorporate factory data. Ensure this change aligns with the expected behavior for the 7702 transactions implementation.
70-71: Consistent use of nullish coalescingUsing nullish coalescing (
??) for default values is a good practice, ensuring empty hex strings when data is null or undefined.
248-307: Approved formatting improvements in encodeUserOpThe formatting changes in the
encodeUserOpfunction improve readability while maintaining the same functionality.
320-331: LGTM: getUserOpHash functionThe
getUserOpHashfunction formatting has been improved without changing the functionality, ensuring it still correctly computes the hash using the encoded operation, entry point, and chain ID.
221-227:❓ Verification inconclusive
Verify the special case handling for "0x7702" addresses
The code now handles a special case for factory addresses that start with "0x7702", which aligns with the PR objectives for implementing 7702 transactions. However, this string prefix matching approach could potentially match other addresses that coincidentally start with the same prefix.
Consider whether a more precise check against specific full addresses would be more appropriate:
🏁 Script executed:
#!/bin/bash # Check if there are any factory addresses in the codebase that start with "0x7702" but aren't exactly "0x7702" rg -i "0x7702[0-9a-f]+" --glob "*.{ts,js,json}" | grep -v "startsWith" | grep -v "\"0x7702\""Length of output: 91
Action Needed: Please verify the 0x7702 special case handling in factory addresses
The shell script did not produce output, which means there’s no immediate evidence that the string prefix check is catching unintended addresses. However, given the low-confidence inference from this script result, please perform a manual verification to ensure that no false positives (i.e. addresses starting with "0x7702" that aren’t meant to be remapped to exactly "0x7702") exist elsewhere in the codebase.
- Location to Verify:
packages/executor/src/services/EntryPointService/utils/packedUserOpUtils.ts(lines 221-227)- Verification Point: Confirm that the only intentional usage of "0x7702" is as handled in this snippet and that no other parts of the code unintentionally match "0x7702" as a prefix.
Please double-check the repository with additional context if necessary and update if you identify edge cases where a more precise full-address check might be required.
| }; | ||
| const stateOverrides: StateOverrides = { | ||
| [userOp.sender]: { | ||
| code: "0xef0100" + userOp.eip7702Auth.address.substring(2), |
There was a problem hiding this comment.
Should we consider moving this to a constants file?
| @@ -71,7 +71,27 @@ export class EntryPointV8Service implements IEntryPointService { | |||
| /** View functions */ | |||
|
|
|||
| async getUserOperationHash(userOp: UserOperation): Promise<string> { | |||
| return await this.contract.getUserOpHash(packUserOp(userOp)); | |||
| const packedUserOp = packUserOp(userOp); | |||
| if (userOp.eip7702Auth && userOp.factory === "0x7702") { | |||
There was a problem hiding this comment.
should we move 0x7702 to a constants file?
| @@ -134,7 +134,7 @@ export class MempoolReputationChecks { | |||
| } | |||
|
|
|||
| const factory = this.entryPointService.getFactory(entryPoint, userOp); | |||
| if (factory) { | |||
| if (factory && factory !== "0x7702") { | |||
| @@ -134,7 +134,7 @@ export class MempoolReputationChecks { | |||
| } | |||
|
|
|||
| const factory = this.entryPointService.getFactory(entryPoint, userOp); | |||
| if (factory) { | |||
| if (factory && factory !== "0x7702") { | |||
| if (accounts.includes(utils.getAddress(factory))) { | |||
| throw new RpcError( | |||
| `A Factory at ${factory} in this UserOperation is used as a sender entity in another UserOperation currently in mempool.`, | |||
There was a problem hiding this comment.
I understand this wasn't changed now. But you should consider rephrasing the error message here.
packages/types/package.json
Outdated
| @@ -4,7 +4,7 @@ | |||
| "publishConfig": { | |||
| "access": "public" | |||
| }, | |||
| "version": "3.0.0", | |||
| "version": "3.0.1", | |||
| "description": "The types of Etherspot bundler client", | |||
| "author": "Etherspot", | |||
| "homepage": "https://https://github.com/etherspot/skandha#readme", | |||
There was a problem hiding this comment.
Pls remove the double https:
Description
Types of changes
What types of changes does your code introduce?
Put an
xin the boxes that applyFurther comments (optional)
Summary by CodeRabbit
Chores
New Features
Refactor