-
Notifications
You must be signed in to change notification settings - Fork 7
Enhance transaction handling and EIP-7702 support #13
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
Conversation
- Added `call_gas_limit` to `UserOpBuilderConfig` for better gas management. - Introduced `AuthorizationSchema` and `SignedAuthorizationSchema` for EIP-7702 authorization handling. - Updated transaction structures to accommodate new gas limit and authorization features. - Refactored execution options to streamline transaction type data handling. - Improved error handling and logging in transaction processing. These changes aim to enhance the flexibility and efficiency of transaction execution, particularly for EIP-7702 compliant operations.
WalkthroughThe changes introduce new transaction type data structures and gas limit handling for Ethereum transactions, including EIP-7702 support. Transaction configuration and authorization schemas are refactored and relocated, with gas limit and transaction type data now part of the Changes
Sequence Diagram(s)Transaction Submission Flow with Gas Limit and Type DatasequenceDiagram
participant Client
participant Server (Router)
participant Executor
participant Bundler
participant Builder
Client->>Server (Router): Submit transaction (with gas_limit, transaction_type_data)
Server (Router)->>Executor: Create EoaTransactionRequest (using transaction.gas_limit, transaction_type_data)
Executor->>Bundler: Prepare job data (includes transactions with gas_limit)
Bundler->>Builder: Compute call_gas_limit from transactions
Builder->>Builder: Initialize UserOpBuilderConfig (with call_gas_limit)
Builder-->>Bundler: Built user operation
Bundler-->>Executor: Submit user operation
Executor-->>Server (Router): Respond with result
Server (Router)-->>Client: Return execution result
Transaction Type Data Handling in InnerTransactionsequenceDiagram
participant User
participant API
participant InnerTransaction
User->>API: Submit transaction (specifies type: EIP-7702, EIP-1559, or Legacy)
API->>InnerTransaction: Construct with gas_limit and transaction_type_data
InnerTransaction-->>API: Transaction with correct fields set
API-->>User: Confirmation/response
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. 🔧 Clippy (1.86.0)error: failed to get Caused by: Caused by: Caused by: Caused by: Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
✨ Finishing Touches
🪧 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 (
|
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: 0
🧹 Nitpick comments (2)
executors/src/external_bundler/send.rs (1)
418-432: LGTM: Custom gas limit calculation implemented correctlyThe logic correctly calculates the total gas limit by summing individual transaction gas limits when all transactions have gas limits specified. The fallback to
Nonewhen any transaction lacks a gas limit is appropriate.Consider using iterator methods for more concise code:
- let custom_call_gas_limit = { - let gas_limits: Vec<u64> = job_data.transactions.iter() - .filter_map(|tx| tx.gas_limit) - .collect(); - - if gas_limits.len() == job_data.transactions.len() { - // All transactions have gas limits specified, sum them up - let total_gas: u64 = gas_limits.iter().sum(); - Some(U256::from(total_gas)) - } else { - // Not all transactions have gas limits specified - None - } - }; + let custom_call_gas_limit = if job_data.transactions.iter().all(|tx| tx.gas_limit.is_some()) { + let total_gas: u64 = job_data.transactions.iter() + .filter_map(|tx| tx.gas_limit) + .sum(); + Some(U256::from(total_gas)) + } else { + None + };core/src/transaction.rs (1)
52-96: Consider adding field validation for gas pricing fields.The transaction type structs are well-designed, but consider adding validation to ensure gas pricing fields are within reasonable bounds (e.g., not zero, not exceeding u128::MAX / 10^9 for wei conversion safety).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (11)
aa-core/src/userop/builder.rs(3 hunks)core/src/defs.rs(1 hunks)core/src/execution_options/eoa.rs(1 hunks)core/src/execution_options/mod.rs(2 hunks)core/src/transaction.rs(2 hunks)executors/src/eoa/store.rs(3 hunks)executors/src/eoa/worker.rs(3 hunks)executors/src/external_bundler/send.rs(1 hunks)server/src/execution_router/mod.rs(1 hunks)server/src/http/dyn_contract.rs(1 hunks)server/src/http/routes/contract_read.rs(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
executors/src/eoa/store.rs (2)
Learnt from: d4mr
PR: thirdweb-dev/engine-core#5
File: executors/src/eoa/worker.rs:173-176
Timestamp: 2025-07-06T15:44:13.701Z
Learning: The EOA executor store uses comprehensive WATCH-based coordination where every Redis state mutation watches the lock key and validates ownership before proceeding. If the lock is lost during any operation, the transaction fails with LockLost error. This makes aggressive lock acquisition safe because only the actual lock holder can successfully perform state mutations, regardless of who claims the lock.
Learnt from: d4mr
PR: thirdweb-dev/engine-core#5
File: executors/src/eoa/worker.rs:173-176
Timestamp: 2025-07-06T15:44:13.701Z
Learning: In the EOA executor system, aggressive lock acquisition is safe because every Redis state mutation uses WATCH operations on the lock key. If the lock is lost during a transaction, the WATCH causes the transaction to fail and the worker exits gracefully. This provides coordination between workers even when using forceful lock takeover.
🔇 Additional comments (21)
server/src/http/routes/contract_read.rs (2)
25-25: LGTM: Import organization improvedThe grouping of
DisplayFromStr,PickFirst, andserde_asimports fromserde_withimproves code organization and readability.
148-148: LGTM: Correct attribute placementThe
#[serde_as]attribute placement on theReadOptionsstruct is correct and aligns with the serialization improvements.core/src/execution_options/mod.rs (2)
11-11: LGTM: Module reorderingMoving the
eoamodule beloweip7702aligns with the architectural changes in the PR and improves module organization.
35-35: LGTM: Default schema attribute addedAdding
defaultto theAutovariant's schema attribute correctly identifies it as the default execution option, improving API documentation.server/src/execution_router/mod.rs (2)
405-405: LGTM: Gas limit sourced from transactionThe change to source
gas_limitfromtransaction.gas_limitcorrectly reflects the architectural shift where transaction-specific data is now stored in the transaction struct rather than execution options.
409-409: LGTM: Transaction type data sourced from transactionThe change to source
transaction_type_datafromtransaction.transaction_type_dataaligns with the PR's objective of moving transaction type data into the transaction struct itself.executors/src/external_bundler/send.rs (1)
439-439: LGTM: Gas limit passed to builder configThe
custom_call_gas_limitis correctly passed to theUserOpBuilderConfig, enabling the user operation builder to use the calculated gas limit.server/src/http/dyn_contract.rs (1)
317-318: LGTM: Transaction fields properly initializedThe explicit initialization of
gas_limitandtransaction_type_datatoNonecorrectly aligns with the enhancedInnerTransactionstructure. For contract calls, these fields are not needed, soNoneis the appropriate default.aa-core/src/userop/builder.rs (3)
21-21: LGTM! Good addition of configurable gas limit.The optional
call_gas_limitfield enables flexible gas limit configuration while maintaining backward compatibility.
105-105: LGTM! Proper implementation of configurable gas limit.The implementation correctly uses the configured gas limit or falls back to zero, maintaining backward compatibility.
215-215: LGTM! Consistent implementation across builder versions.The V0_7 builder correctly mirrors the V0_6 implementation for the configurable gas limit feature.
executors/src/eoa/worker.rs (2)
13-13: LGTM! Import change supports unified transaction type handling.The change from
EoaTransactionTypeDatatoTransactionTypeDataaligns with the PR's goal of consolidating transaction type data structures.
1766-1808: LGTM! Pattern matching correctly updated for new enum.The match arms are properly updated to use
TransactionTypeDatavariants (Eip1559,Legacy,Eip7702) while preserving the original logic for handling gas fees and authorization lists.executors/src/eoa/store.rs (3)
8-8: LGTM!The import change correctly reflects the refactoring that moved transaction type data from execution options to the transaction module.
248-248: LGTM!The field type change is consistent with the new
TransactionTypeDataimport.
296-296: Ensure backward compatibility when renaming the Redis keyIt looks like the
eoa_tx_data:{transaction_id}key was renamed toeoa_executor:_tx_data:{transaction_id}without any migration or fallback in place:
- No remaining references to
eoa_tx_data:were found in the Rust code- No migration scripts or docs mention migrating old keys
- Any existing transaction data under the old key format will become inaccessible
Please add one of the following before rolling out this change in production:
- A migration script to copy or rename existing
eoa_tx_data:*keys- Fallback logic that attempts the old key if the new one is missing
And confirm that this covers all running environments.
core/src/defs.rs (2)
20-41: LGTM!The
AuthorizationSchemastruct is well-designed with proper documentation and schema annotations for EIP-7702 support.
43-83: LGTM!The
SignedAuthorizationSchemastruct correctly implements the signed authorization format with proper ECDSA signature components and backward-compatible field naming.core/src/transaction.rs (2)
23-38: LGTM!The additions to
InnerTransactionare well-designed:
- Optional
gas_limitallows flexibility for gas estimation- Flattened
transaction_type_dataprovides a clean API structure- Clear documentation about ERC4337 limitations
40-50: LGTM!The
TransactionTypeDataenum design with untagged serialization provides a clean API for different transaction types.core/src/execution_options/eoa.rs (1)
1-46: LGTM!The simplification of
EoaExecutionOptionsappropriately removes transaction type-specific data that has been moved to the transaction module, maintaining clean separation of concerns.
call_gas_limittoUserOpBuilderConfigfor better gas management.AuthorizationSchemaandSignedAuthorizationSchemafor EIP-7702 authorization handling.These changes aim to enhance the flexibility and efficiency of transaction execution, particularly for EIP-7702 compliant operations.
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Style