-
Notifications
You must be signed in to change notification settings - Fork 1
Create new executor for 7702 transactions #6
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
Create new executor for 7702 transactions #6
Conversation
Co-authored-by: joaquim.verges <[email protected]>
Co-authored-by: joaquim.verges <[email protected]>
WalkthroughA new Changes
Sequence Diagram(s)EIP-7702 Transaction Send FlowsequenceDiagram
participant JobQueue as Durable Job Queue
participant SendHandler as Eip7702SendHandler
participant ChainService
participant EoaSigner
participant Bundler
participant ConfirmQueue as Confirm Job Queue
participant WebhookQueue
JobQueue->>SendHandler: Dequeue EIP-7702 send job
SendHandler->>ChainService: Get chain instance
SendHandler->>ChainService: Check minimal account delegation (on-chain bytecode)
SendHandler->>EoaSigner: Sign wrapped calls (and optional authorization)
SendHandler->>Bundler: Call tw_execute RPC with data, signature, authorization
Bundler-->>SendHandler: Return transaction ID
SendHandler->>ConfirmQueue: Enqueue confirmation job
SendHandler->>WebhookQueue: Trigger webhook(s) (success/failure/nack)
EIP-7702 Transaction Confirmation FlowsequenceDiagram
participant ConfirmQueue as Durable Job Queue
participant ConfirmHandler as Eip7702ConfirmationHandler
participant ChainService
participant Bundler
participant Blockchain
participant TransactionRegistry
participant WebhookQueue
ConfirmQueue->>ConfirmHandler: Dequeue EIP-7702 confirm job
ConfirmHandler->>ChainService: Get chain instance
ConfirmHandler->>Bundler: Query for transaction hash via JSON-RPC
alt Hash not ready/pending
ConfirmHandler->>ConfirmQueue: Nack and retry later
ConfirmHandler->>WebhookQueue: Trigger nack webhook
else Hash ready
ConfirmHandler->>Blockchain: Get transaction receipt
alt Receipt missing
ConfirmHandler->>ConfirmQueue: Nack and retry later
ConfirmHandler->>WebhookQueue: Trigger nack webhook
else Receipt found
alt Transaction failed
ConfirmHandler->>TransactionRegistry: Update as failed
ConfirmHandler->>WebhookQueue: Trigger failure webhook
else Transaction succeeded
ConfirmHandler->>TransactionRegistry: Update as confirmed
ConfirmHandler->>WebhookQueue: Trigger success webhook
end
end
end
Possibly related PRs
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: 3
🧹 Nitpick comments (1)
executors/src/eip7702_executor/send.rs (1)
35-35
: Consider making the minimal account address configurable.The hardcoded address reduces flexibility. Consider accepting this as a configuration parameter or environment variable to support different deployments or testing environments.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
executors/src/eip7702_executor/confirm.rs
(1 hunks)executors/src/eip7702_executor/mod.rs
(1 hunks)executors/src/eip7702_executor/send.rs
(1 hunks)executors/src/lib.rs
(1 hunks)
🔇 Additional comments (5)
executors/src/eip7702_executor/mod.rs (1)
1-2
: LGTM!Standard module structure exposing the send and confirm submodules.
executors/src/lib.rs (1)
2-2
: LGTM!The new EIP-7702 executor module is properly exposed alongside existing modules.
executors/src/eip7702_executor/confirm.rs (1)
145-245
: Well-structured confirmation handler implementation.The process method has excellent error handling with appropriate nack/retry logic for transient failures. The distinction between permanent failures (fail) and temporary issues (nack with retry) is properly implemented.
executors/src/eip7702_executor/send.rs (2)
382-441
: Correct implementation of EIP-7702 delegation check.The function properly validates the EIP-7702 delegation prefix and extracts the target address. The byte range extraction (3..23) correctly extracts 20 bytes for the address.
485-502
: Verify y_parity calculation for modern signatures.The code assumes
v
is either 27 or 28 (legacy format), but modern EIP-2718 transactions use 0 or 1 for v values. This could lead to incorrect y_parity values.Consider updating the logic to handle both formats:
- y_parity: v != 27, + y_parity: match v { + 0 | 27 => false, + 1 | 28 => true, + _ => return Err(EngineError::ValidationError { + message: format!("Invalid v value: {}", v), + }), + },
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
♻️ Duplicate comments (2)
executors/src/eip7702_executor/send.rs (2)
400-400
: Fix error module path.The error module path is incorrect and should reference the engine_core module.
Apply this fix:
- kind: crate::error::RpcErrorKind::Unknown, + kind: engine_core::error::RpcErrorKind::Unknown,
468-482
: Remove unused code and fix error module path.The
request
JSON object is created but never used, and the error module path is incorrect.Apply this fix:
- let request = json!({ - "jsonrpc": "2.0", - "method": "tw_execute", - "params": params, - "id": 1 - }); - let response: Value = bundler_client .inner .request("tw_execute", params) .await .map_err(|e| EngineError::RpcError { message: format!("Bundler call failed: {}", e), - kind: crate::error::RpcErrorKind::Unknown, + kind: engine_core::error::RpcErrorKind::Unknown, })?;
🧹 Nitpick comments (1)
executors/src/eip7702_executor/send.rs (1)
35-35
: Consider making the minimal account implementation address configurable.The minimal account implementation address is hardcoded as a constant. Consider making this configurable per chain or through configuration to support different implementations across different networks.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
core/src/signer.rs
(3 hunks)executors/src/eip7702_executor/send.rs
(1 hunks)
🔇 Additional comments (8)
core/src/signer.rs (3)
4-8
: LGTM: New imports are correctly added for EIP-7702 support.The new imports (
U256
,FixedBytes
,Authorization
,serde_json::json
) are appropriately added to support the EIP-7702 authorization signing functionality.
182-190
: LGTM: Well-defined trait method for EIP-7702 authorization signing.The
sign_authorization
method signature is well-designed with appropriate parameters for EIP-7702 authorization (chain ID, address, nonce) and follows the async pattern used by other trait methods.
303-350
: LGTM: Consistent implementation of authorization signing.The implementation correctly handles both Vault and IAW signing credentials, follows the established error handling patterns, and includes appropriate logging. The method properly delegates to the respective client implementations.
executors/src/eip7702_executor/send.rs (5)
40-49
: LGTM: Well-structured job data with comprehensive metadata.The job data structure includes all necessary fields for EIP-7702 transaction processing, including optional webhook options and RPC credentials.
79-105
: LGTM: Comprehensive error handling with descriptive error types.The error enum covers all relevant failure scenarios for EIP-7702 transactions with clear error messages and proper serialization support.
179-301
: LGTM: Well-implemented transaction processing logic.The process method correctly handles the EIP-7702 transaction flow: chain setup, wrapped calls creation, signing, delegation checks, authorization, and bundler calls. The logic is well-structured and includes proper error handling.
389-448
: LGTM: Correct EIP-7702 delegation check implementation.The bytecode checking logic correctly implements EIP-7702 delegation verification by checking the prefix and extracting the target address from the expected byte positions.
352-387
: LGTM: Proper typed data structure for EIP-7702.The typed data creation follows the correct EIP-712 format with appropriate domain, types, and message structure for minimal account wrapped calls.
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: 3
🧹 Nitpick comments (1)
executors/src/eip7702_executor/send.rs (1)
210-210
: Consider clarifying the use of default address for missing targets.Using
Address::default()
(zero address) whentx.to
is None might be intentional, but it's worth documenting this behavior since transactions to the zero address have special semantics.Consider adding a comment to clarify this behavior:
target: tx.to.unwrap_or_default(), // Use zero address for contract creation
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
core/src/rpc_clients/bundler.rs
(2 hunks)core/src/signer.rs
(6 hunks)executors/src/eip7702_executor/confirm.rs
(1 hunks)executors/src/eip7702_executor/send.rs
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- core/src/signer.rs
- executors/src/eip7702_executor/confirm.rs
🔇 Additional comments (5)
core/src/rpc_clients/bundler.rs (2)
2-3
: LGTM! Import additions align with EIP-7702 requirements.The new imports for
Authorization
,TxHash
, andValue
are appropriate for the EIP-7702 transaction methods being added.Also applies to: 8-8
120-142
: LGTM! Well-structured RPC methods for EIP-7702 transactions.The
tw_execute
andtw_get_transaction_hash
methods follow the established patterns in the bundler client and provide the necessary RPC interface for EIP-7702 transaction handling. The parameter types and return types are appropriate for the bundler integration.executors/src/eip7702_executor/send.rs (3)
462-489
: LGTM! EIP-7702 delegation check logic is correct.The implementation correctly:
- Checks for the EIP-7702 delegation prefix
0xef0100
- Extracts the target address from bytes 3-23
- Compares with the minimal account implementation address
- Includes comprehensive logging for debugging
The logic aligns with the EIP-7702 specification for delegation checking.
177-310
: LGTM! Well-structured EIP-7702 transaction processing logic.The process method correctly implements the EIP-7702 transaction flow:
- Chain instance retrieval with proper error handling
- Wrapped calls creation with random UID
- Typed data signing for wrapped calls
- EIP-7702 delegation checking
- Authorization signing when needed
- Bundler RPC call execution
The error handling is comprehensive and the flow matches the EIP-7702 specification.
312-390
: LGTM! Proper webhook and transaction registry integration.The success, nack, and fail hooks are well-implemented with:
- Transaction registry updates for state tracking
- Confirmation job queueing for transaction status monitoring
- Webhook notifications for all outcomes
- Comprehensive error logging
The integration with the durable execution system is correct.
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
🧹 Nitpick comments (1)
server/src/queue/manager.rs (1)
83-88
: Consider separate configuration for EIP-7702 queuesThe EIP-7702 queues are reusing the same worker configurations as the external bundler queues. This might not be optimal if EIP-7702 transactions have different performance characteristics or load patterns.
Consider adding dedicated configuration options for EIP-7702 queues to allow independent tuning:
eip7702_send_workers
eip7702_confirm_workers
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
core/src/execution_options/eip7702.rs
(1 hunks)core/src/execution_options/mod.rs
(3 hunks)server/src/execution_router/mod.rs
(4 hunks)server/src/main.rs
(2 hunks)server/src/queue/manager.rs
(12 hunks)
✅ Files skipped from review due to trivial changes (1)
- core/src/execution_options/eip7702.rs
🧰 Additional context used
🧬 Code Graph Analysis (1)
server/src/execution_router/mod.rs (4)
executors/src/external_bundler/send.rs (1)
webhook_options
(62-64)executors/src/external_bundler/confirm.rs (1)
webhook_options
(337-339)executors/src/webhook/envelope.rs (3)
webhook_options
(78-78)transaction_id
(82-82)transaction_id
(86-88)core/src/execution_options/mod.rs (1)
transaction_id
(142-144)
🔇 Additional comments (6)
server/src/main.rs (2)
55-62
: LGTM! EOA signer parameter addition is appropriate.The addition of
eoa_signer.clone()
as the fifth parameter toQueueManager::new
is consistent with the EIP-7702 support requirements, as these transactions need EOA signing capabilities.
80-81
: Queue additions follow established patterns.The new EIP-7702 send and confirm queues are properly integrated into the
ExecutionRouter
, maintaining consistency with existing queue patterns.server/src/execution_router/mod.rs (4)
11-11
: Import additions are well-structured.The new EIP-7702 imports follow the established pattern for executor modules, with proper separation of execution options, handlers, and job data types.
Also applies to: 16-19
41-42
: Struct fields properly integrated.The new queue fields maintain type consistency with existing queues and use the appropriate handler types with the
ThirdwebChainService
generic parameter.
228-251
: Match arm implementation is consistent.The EIP-7702 execution path follows the established pattern from the ERC4337 implementation, properly forwarding all required parameters and constructing the response.
308-351
: Implementation looks correct, verify nonce handling design.The
execute_eip7702
method follows the established pattern fromexecute_external_bundler
. The key difference is settingnonce: None
rather than using a pregenerated nonce like in ERC4337.Please confirm this design choice aligns with EIP-7702 transaction requirements where nonce generation is delegated to the executor.
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
♻️ Duplicate comments (2)
executors/src/eip7702_executor/send.rs (2)
37-37
: Remove hardcoded minimal account implementation addressThis hardcoded address was flagged in previous reviews and should be configurable rather than fixed. The address should be passed as a parameter through the handler's configuration or constructor.
440-440
: Replace expect() with proper error handlingThe use of
expect()
onserde_json::from_value()
can cause panics and was flagged in previous reviews. This should be replaced with proper error handling.- let eip712_types: alloy::dyn_abi::eip712::Eip712Types = - serde_json::from_value(types_json).expect("Failed to parse EIP712 types"); + let eip712_types: alloy::dyn_abi::eip712::Eip712Types = + serde_json::from_value(types_json).map_err(|e| { + tracing::error!(error = ?e, "Failed to parse EIP712 types"); + e + })?;The function signature will need to be updated to return a
Result
type.
🧹 Nitpick comments (2)
executors/src/eip7702_executor/send.rs (2)
215-221
: Consider optimizing random UID generationCreating a new RNG instance for each job might be inefficient. Consider using a thread-local RNG or a more efficient random generation method.
- uid: { - let mut rng = rand::rng(); - let mut bytes = [0u8; 32]; - rng.fill(&mut bytes); - FixedBytes::from(bytes) - }, + uid: { + use rand::RngCore; + let mut bytes = [0u8; 32]; + rand::thread_rng().fill_bytes(&mut bytes); + FixedBytes::from(bytes) + },
475-493
: Verify EIP-7702 delegation parsing logicThe bytecode parsing logic looks correct, but the comment mentions JavaScript equivalent that doesn't align with the actual implementation. Consider clarifying the comment.
- // Extract the target address from bytes 3-23 (20 bytes for address) - // EIP-7702 format: 0xef0100 + 20 bytes address - // JS equivalent: code.slice(8, 48) extracts 40 hex chars = 20 bytes - // In hex string: "0xef0100" + address, so address starts at position 8 - // In byte array: [0xef, 0x01, 0x00, address_bytes...] - // The address starts at byte 3 and is 20 bytes long (bytes 3-22) + // Extract the target address from bytes 3-23 (20 bytes for address) + // EIP-7702 format: 0xef0100 + 20 bytes address + // In byte array: [0xef, 0x01, 0x00, address_bytes...] + // The address starts at byte 3 and is 20 bytes long (bytes 3-22 inclusive)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
core/src/execution_options/mod.rs
(3 hunks)executors/src/eip7702_executor/send.rs
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- core/src/execution_options/mod.rs
🔇 Additional comments (2)
executors/src/eip7702_executor/send.rs (2)
286-303
: Approve the bundler interaction logicThe bundler call implementation with proper error handling and transaction ID retrieval looks correct. The error mapping and logging provide good observability.
320-363
: Approve the transaction lifecycle managementThe success handler properly manages the transaction registry, enqueues confirmation jobs, and handles webhook notifications with appropriate error logging.
Add a new EIP-7702 executor to support EIP-7702 compliant transactions.
Summary by CodeRabbit
New Features
Chores