auto max in flight for delegated EOA #32
Conversation
WalkthroughThis change introduces a shared EOA authorization cache using the Changes
Sequence Diagram(s)sequenceDiagram
participant Admin as Admin API Client
participant Server as Engine Server
participant Store as EoaExecutorStore (Redis)
participant Worker as EoaExecutorWorker
Admin->>Server: POST /admin/executors/eoa/{eoa_chain}/reset
Server->>Store: schedule_manual_reset()
Store-->>Server: Set manual_reset_key in Redis
Server-->>Admin: 200 OK
Worker->>Store: is_manual_reset_scheduled()
Store-->>Worker: true/false
alt If manual reset scheduled
Worker->>Store: Reset nonces atomically (incl. manual_reset_key)
end
sequenceDiagram
participant Router as ExecutionRouter
participant Cache as EoaAuthorizationCache
participant Worker as EoaExecutorJobHandler
participant Account as DelegatedAccount
Router->>Cache: is_minimal_account(&DelegatedAccount)
alt Cache hit
Cache-->>Router: bool
else Cache miss
Cache->>Account: is_minimal_account()
Account-->>Cache: bool
Cache-->>Router: bool
end
Router->>Worker: Set max_inflight based on bool
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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: 0
🧹 Nitpick comments (1)
executors/src/eoa/store/submitted.rs (1)
105-137: Well-implemented backward compatibility handling.The code correctly handles both the old 3-part and new 4-part string formats, ensuring smooth migration. Setting
submitted_atto 0 for old transactions is a reasonable default.Consider adding a TODO with a specific date or version for removing the backward compatibility code to ensure it doesn't remain indefinitely.
- // this exists for backwards compatibility with old transactions - // remove after sufficient time for old transactions to be cleaned up + // TODO: Remove backward compatibility after 2025-03-01 or version 2.0.0 + // This handles old 3-part format (hash:id:queued_at) for transactions created before submitted_at was added
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (13)
core/src/chain.rs(1 hunks)executors/Cargo.toml(1 hunks)executors/src/eoa/authorization_cache.rs(1 hunks)executors/src/eoa/mod.rs(1 hunks)executors/src/eoa/store/atomic.rs(2 hunks)executors/src/eoa/store/mod.rs(8 hunks)executors/src/eoa/store/submitted.rs(4 hunks)executors/src/eoa/worker/confirm.rs(7 hunks)executors/src/eoa/worker/mod.rs(4 hunks)server/src/execution_router/mod.rs(4 hunks)server/src/http/routes/admin/eoa_diagnostics.rs(5 hunks)server/src/main.rs(3 hunks)server/src/queue/manager.rs(3 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
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.
📚 Learning: the eoa executor store uses comprehensive watch-based coordination where every redis state mutation ...
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.
Applied to files:
executors/src/eoa/store/mod.rsexecutors/src/eoa/store/atomic.rsserver/src/execution_router/mod.rsexecutors/src/eoa/worker/confirm.rs
📚 Learning: in the eoa executor system, aggressive lock acquisition is safe because every redis state mutation u...
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.
Applied to files:
executors/src/eoa/store/mod.rsexecutors/src/eoa/worker/mod.rsserver/src/execution_router/mod.rsexecutors/src/eoa/worker/confirm.rs
🧬 Code Graph Analysis (1)
executors/src/eoa/authorization_cache.rs (2)
eip7702-core/src/delegated_account.rs (1)
chain(97-99)server/src/queue/manager.rs (1)
new(46-239)
🔇 Additional comments (30)
executors/Cargo.toml (1)
27-27: LGTM! Clean dependency addition for async caching.The
mokadependency with thefuturefeature correctly supports the newEoaAuthorizationCacheimplementation requiring async cache operations.executors/src/eoa/mod.rs (1)
1-1: LGTM! Clean module addition for centralized authorization caching.The new
authorization_cachemodule appropriately encapsulates EOA authorization logic and enables sharing across components.core/src/chain.rs (1)
263-263: LGTM! Appropriate trait bound addition for caching support.Adding the
Cloneconstraint enables safe reuse of chain instances across the new authorization cache operations. The existingThirdwebChainimplementation already supports cloning, ensuring backward compatibility.executors/src/eoa/store/atomic.rs (1)
509-509: LGTM! Proper atomic cleanup of manual reset state.The addition of
manual_reset_keydeletion maintains atomicity during nonce resets, ensuring consistent cleanup alongside optimistic nonce, cached nonce, and recycled nonces. This correctly integrates with the new manual reset scheduling feature.Also applies to: 527-527
server/src/main.rs (2)
57-63: LGTM! Well-configured shared authorization cache.The cache configuration is appropriate:
- 1GB capacity provides sufficient space for authorization data
- 5-minute TTL balances freshness with performance
- 1-minute TTI keeps active entries cached
71-71: LGTM! Clean centralized cache sharing pattern.Passing the same
EoaAuthorizationCacheinstance to bothQueueManagerandExecutionRouterproperly centralizes authorization caching across components, eliminating duplication and ensuring consistency.Also applies to: 90-90
server/src/queue/manager.rs (2)
8-8: LGTM: Clean import additionThe import of
EoaAuthorizationCachefollows the established pattern and supports the centralization of authorization cache management.
52-52: LGTM: Proper cache parameter threadingThe
authorization_cacheparameter is correctly added to the constructor signature and properly passed to theEoaExecutorJobHandler. This supports the centralized cache management pattern.Also applies to: 216-216
executors/src/eoa/worker/mod.rs (3)
10-10: LGTM: Proper imports and field additionThe new imports for
DelegatedAccountandEoaAuthorizationCacheare correctly added, and theauthorization_cachefield inEoaExecutorJobHandlersupports the delegation-aware concurrency control.Also applies to: 22-22, 115-115
161-172: LGTM: Sound delegation check with appropriate fallbackThe delegation check logic is well-implemented:
- Creates
DelegatedAccountappropriately- Uses the authorization cache for efficient lookups
- Proper error handling with logging and safe fallback to
false- The fallback behavior (assuming non-minimal on error) is conservative and appropriate
181-185: LGTM: Clear concurrency control logicThe concurrency control implementation is straightforward and correct:
- Minimal accounts are limited to
max_inflight = 1for safer execution- Regular accounts use the configured
self.max_inflightvalue- The conditional logic is clear and well-structured
server/src/execution_router/mod.rs (2)
3-3: LGTM: Clean refactoring to encapsulated cache typeThe import additions and field type change properly integrate the new
EoaAuthorizationCacheabstraction, replacing the previous manual cache management.Also applies to: 24-24, 55-55
408-412: LGTM: Clean cache usage simplificationThe refactoring successfully simplifies the cache interaction:
- Replaces manual cache key construction with encapsulated method call
- Maintains proper error handling and conversion to
EngineError- Cleaner, more readable code
server/src/http/routes/admin/eoa_diagnostics.rs (3)
31-31: LGTM: Proper field addition for manual reset statusThe
manual_reset_scheduledfield addition toEoaStateResponseproperly exposes the manual reset scheduling status for diagnostic purposes.Also applies to: 166-166
150-154: LGTM: Consistent error handling patternThe manual reset status check follows the established pattern for store method calls with proper error handling and conversion to
ApiEngineError.
380-409: LGTM: Well-structured new endpointThe
schedule_manual_resetendpoint follows established patterns:
- Proper authentication and parameter validation
- Consistent error handling and response format
- Clean route registration
- Appropriate HTTP method (POST) for the operation
Also applies to: 457-460
executors/src/eoa/authorization_cache.rs (3)
8-12: LGTM: Well-designed cache key structureThe
AuthorizationCacheKeystruct is properly designed:
- Uses appropriate fields (EOA address and chain ID) for unique identification
- Correct trait implementations (
Hash,Eq,PartialEq) for cache key usage- Good encapsulation with private fields
14-22: LGTM: Clean cache wrapper designThe
EoaAuthorizationCachewrapper is well-designed:
- Clean encapsulation of
moka::future::CacheCloneimplementation enables efficient sharing across components- Constructor allows external cache configuration
24-38: LGTM: Solid cache-or-compute implementationThe
is_minimal_accountmethod is well-implemented:
- Proper cache-or-compute pattern with
try_get_with- Correct cache key construction from
DelegatedAccount- Generic design works with any
Chainimplementation- Appropriate error handling that extracts underlying errors
executors/src/eoa/worker/confirm.rs (4)
47-52: LGTM! Manual reset check is properly placed.The manual reset check is appropriately positioned at the beginning of the confirm flow, ensuring it executes before any other nonce-related operations. The implementation correctly uses the freshly fetched chain transaction count.
262-274: LGTM! Proper handling of newest transaction selection.The logic correctly identifies the newest transaction using the
submitted_attimestamp when multiple transactions exist for the same nonce. The code handles both single and multiple transaction cases appropriately.
343-345: LGTM! Correct timestamp handling in gas bump attempt.The
submitted_atfield is properly set to the current time usingEoaExecutorStore::now()for the gas bump attempt, while preserving the originalqueued_attimestamp from the transaction data.
373-383: LGTM! Appropriate fallback to noop transaction.The fallback logic correctly handles the edge case where transaction data cannot be found, sending a noop transaction to unstick the stalled nonce. The updated log message accurately describes the situation.
executors/src/eoa/store/mod.rs (5)
278-289: LGTM! Consistent key generation for manual reset.The
manual_reset_key_namemethod follows the established pattern for key generation in the store, properly handling namespacing and using a clear, descriptive key name.
350-360: LGTM! Proper timestamp initialization in conversion.The
submitted_atfield is correctly set to the current time when converting fromBorrowedTransactionDatatoSubmittedTransactionDehydrated, ensuring accurate submission time tracking.
738-745: LGTM! Simple and effective manual reset scheduling.The
schedule_manual_resetmethod correctly stores the current timestamp to signal that a manual reset has been scheduled, which will be detected and executed by the confirm worker.
778-782: LGTM! Helpful documentation for time utility.The added documentation clarifies that
now()provides the canonical time representation for the store, which is important for consistency across timestamp fields.
867-873: LGTM! Clean implementation for reset status check.The
is_manual_reset_scheduledmethod provides a simple and efficient way to check if a manual reset is pending by checking for the existence of the timestamp value.executors/src/eoa/store/submitted.rs (2)
89-97: LGTM! Proper addition of submission timestamp field.The
submitted_atfield is appropriately added to track the actual submission time of transactions, distinct from thequeued_attime.
160-168: LGTM! Consistent serialization format update.The
to_redis_string_with_noncemethod correctly includes thesubmitted_atfield in the serialization format, maintaining consistency with the deserialization logic.
2 participants
Summary by CodeRabbit
New Features
Improvements
Bug Fixes