-
Notifications
You must be signed in to change notification settings - Fork 7
Implement delegation contract handling in EIP-7702 executor and bundler client #41
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
Changes from 1 commit
33a1cb4
b6f91f5
b249621
80b47ef
2066a8b
2cca023
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,68 @@ | ||
use std::{ops::Deref, sync::Arc}; | ||
|
||
use alloy::primitives::Address; | ||
use engine_core::{ | ||
chain::Chain, | ||
error::{AlloyRpcErrorToEngineError, EngineError}, | ||
rpc_clients::TwGetDelegationContractResponse, | ||
}; | ||
use moka::future::Cache; | ||
|
||
/// Cache key for delegation contract - uses chain_id as the key since each chain has one delegation contract | ||
#[derive(Hash, Eq, PartialEq, Clone, Debug)] | ||
pub struct DelegationContractCacheKey { | ||
chain_id: u64, | ||
} | ||
|
||
/// Cache for delegation contract addresses to avoid repeated RPC calls | ||
#[derive(Clone)] | ||
pub struct DelegationContractCache { | ||
pub inner: moka::future::Cache<DelegationContractCacheKey, Address>, | ||
} | ||
|
||
impl DelegationContractCache { | ||
/// Create a new delegation contract cache with the provided moka cache | ||
pub fn new(cache: Cache<DelegationContractCacheKey, Address>) -> Self { | ||
Self { inner: cache } | ||
} | ||
|
||
/// Get the delegation contract address for a chain, fetching it if not cached | ||
pub async fn get_delegation_contract<C: Chain>( | ||
&self, | ||
chain: &C, | ||
) -> Result<Address, EngineError> { | ||
let cache_key = DelegationContractCacheKey { | ||
chain_id: chain.chain_id(), | ||
}; | ||
|
||
// Use try_get_with for SWR behavior - this will fetch if not cached or expired | ||
let result = self | ||
.inner | ||
.try_get_with(cache_key, async { | ||
tracing::debug!( | ||
chain_id = chain.chain_id(), | ||
"Fetching delegation contract from bundler" | ||
); | ||
|
||
let TwGetDelegationContractResponse { | ||
delegation_contract, | ||
} = chain | ||
.bundler_client() | ||
.tw_get_delegation_contract() | ||
.await | ||
.map_err(|e| e.to_engine_bundler_error(chain))?; | ||
|
||
tracing::debug!( | ||
chain_id = chain.chain_id(), | ||
delegation_contract = ?delegation_contract, | ||
"Successfully fetched and cached delegation contract" | ||
); | ||
|
||
Ok(delegation_contract) | ||
}) | ||
.await | ||
.map_err(|e: Arc<EngineError>| e.deref().clone())?; | ||
|
||
Ok(result) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1,3 @@ | ||
pub mod send; | ||
pub mod confirm; | ||
pub mod confirm; | ||
pub mod delegation_cache; |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -22,7 +22,11 @@ use twmq::{ | |||||||||||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
use crate::{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
metrics::{record_transaction_queued_to_sent, current_timestamp_ms, calculate_duration_seconds_from_twmq}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
eip7702_executor::delegation_cache::DelegationContractCache, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
metrics::{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
calculate_duration_seconds_from_twmq, current_timestamp_ms, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
record_transaction_queued_to_sent, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
}, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
transaction_registry::TransactionRegistry, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
webhook::{ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
WebhookJobHandler, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -138,6 +142,7 @@ where | |||||||||||||||||||||||||||||||||||||||||||||||||||||
pub webhook_queue: Arc<Queue<WebhookJobHandler>>, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
pub confirm_queue: Arc<Queue<Eip7702ConfirmationHandler<CS>>>, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
pub transaction_registry: Arc<TransactionRegistry>, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
pub delegation_contract_cache: DelegationContractCache, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
impl<CS> ExecutorStage for Eip7702SendHandler<CS> | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -216,8 +221,20 @@ where | |||||||||||||||||||||||||||||||||||||||||||||||||||||
None => account.owner_transaction(&job_data.transactions), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Get delegation contract from cache | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
let delegation_contract = self | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
.delegation_contract_cache | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
.get_delegation_contract(transactions.account().chain()) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
.await | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
.map_err(|e| Eip7702SendError::DelegationCheckFailed { inner_error: e }) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
.map_err_fail()?; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Make delegation-contract lookup retryable on transient errors Currently, a failure in get_delegation_contract() always fails the job. Align with the retry policy used later (add_authorization_if_needed) so network blips don’t cause permanent failures. - let delegation_contract = self
- .delegation_contract_cache
- .get_delegation_contract(transactions.account().chain())
- .await
- .map_err(|e| Eip7702SendError::DelegationCheckFailed { inner_error: e })
- .map_err_fail()?;
+ let delegation_contract = self
+ .delegation_contract_cache
+ .get_delegation_contract(transactions.account().chain())
+ .await
+ .map_err(|e| {
+ let mapped = Eip7702SendError::DelegationCheckFailed { inner_error: e.clone() };
+ if is_build_error_retryable(&e) {
+ mapped.nack_with_max_retries(
+ Some(Duration::from_secs(2)),
+ twmq::job::RequeuePosition::Last,
+ 3,
+ job.attempts(),
+ )
+ } else {
+ mapped.fail()
+ }
+ })?; Additionally, add a debug log to aid triage: + tracing::debug!(delegation_contract = ?delegation_contract, "resolved delegation contract"); 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
let transactions = transactions | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
.add_authorization_if_needed(self.eoa_signer.deref(), &job_data.signing_credential) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
.add_authorization_if_needed( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
self.eoa_signer.deref(), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
&job_data.signing_credential, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
delegation_contract, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
.await | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
.map_err(|e| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
let mapped_error = match e { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -281,10 +298,11 @@ where | |||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
tracing::debug!(transaction_id = ?transaction_id, "EIP-7702 transaction sent to bundler"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Record metrics: transaction queued to sent | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
let sent_timestamp = current_timestamp_ms(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
let queued_to_sent_duration = calculate_duration_seconds_from_twmq(job.job.created_at, sent_timestamp); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
record_transaction_queued_to_sent("eip7702", job_data.chain_id, queued_to_sent_duration); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
// Record metrics: transaction queued to sent | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
let sent_timestamp = current_timestamp_ms(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
let queued_to_sent_duration = | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
calculate_duration_seconds_from_twmq(job.job.created_at, sent_timestamp); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
record_transaction_queued_to_sent("eip7702", job_data.chain_id, queued_to_sent_duration); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||
let sender_details = match session_key_target_address { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
Some(target_address) => Eip7702Sender::SessionKey { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
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.
💡 Verification agent
🧩 Analysis chain
Prefer sending [] over () for zero-arg JSON-RPC calls
Some servers reject params: null. Using an empty array is safer and matches other methods here.
Apply this diff:
To verify consistency repo-wide (and catch any other zero-arg calls using ()), run:
🏁 Script executed:
Length of output: 305
Use an empty JSON array for zero-arg RPC calls
Some JSON-RPC servers reject
params: null
. To ensure compatibility and match the rest of our clients, always send[]
instead of()
when there are no parameters.Affected methods:
core/src/rpc_clients/bundler.rs
:tw_get_delegation_contract
core/src/rpc_clients/paymaster.rs
:thirdweb_getUserOperationGasPrice
Proposed diffs:
📝 Committable suggestion
🤖 Prompt for AI Agents