Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 0 additions & 7 deletions eip7702-core/src/constants.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,3 @@
use alloy::primitives::{Address, address};

// The minimal account implementation address used for EIP-7702 delegation
// pub const MINIMAL_ACCOUNT_IMPLEMENTATION_ADDRESS: Address =
// address!("0xD6999651Fc0964B9c6B444307a0ab20534a66560");
// NOTE!: do not hardcode. If needed later, use tw_getDelegationContract

/// EIP-7702 delegation prefix bytes
pub const EIP_7702_DELEGATION_PREFIX: [u8; 3] = [0xef, 0x01, 0x00];

Expand Down
39 changes: 21 additions & 18 deletions eip7702-core/src/delegated_account.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ impl<C: Chain> DelegatedAccount<C> {
}

/// Check if the EOA has EIP-7702 delegation to the minimal account implementation
pub async fn is_minimal_account(&self) -> Result<bool, EngineError> {
pub async fn is_minimal_account(&self, delegation_contract: Option<Address>) -> Result<bool, EngineError> {
// Get the bytecode at the EOA address using eth_getCode
let code = self
.chain
Expand Down Expand Up @@ -60,25 +60,28 @@ impl<C: Chain> DelegatedAccount<C> {
// Extract the target address from bytes 3-23 (20 bytes for address)
// EIP-7702 format: 0xef0100 + 20 bytes address

// NOTE!: skip the actual delegated target address check for now
// extremely unlikely that an EOA being used with engine is delegated to a non-minimal account
// Potential source for fringe edge cases, please verify delegated target address if debugging 7702 execution issues

// let target_bytes = &code[3..23];
// let target_address = Address::from_slice(target_bytes);

// // Compare with the minimal account implementation address
// let is_delegated = target_address == MINIMAL_ACCOUNT_IMPLEMENTATION_ADDRESS;
let target_bytes = &code[3..23];
let target_address = Address::from_slice(target_bytes);

// Compare with the minimal account implementation address
let is_delegated = match delegation_contract {
Some(delegation_contract) => {
target_address == delegation_contract
}
None => {
true
}
};
Comment on lines +67 to +74
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

🧩 Analysis chain

Reconsider the None case returning true by default.

When delegation_contract is None, the method returns true (assumes delegation exists), which seems semantically incorrect. This means "I don't know which contract to check against, so I'll assume it's delegated."

Additionally, the authorization cache (shown in relevant snippets) uses Address::ZERO as a fallback when delegation_contract is None, which could lead to cache key collisions—different delegation contracts would share the same cache entry.

Consider either:

  • Returning an error when delegation_contract is None and delegation verification is actually needed
  • Making delegation_contract a required parameter instead of Option<Address>
  • Documenting clearly why None means "assume delegated"

Run the following script to understand how None is used across the codebase:


🏁 Script executed:

#!/bin/bash
# Description: Find all call sites passing None to is_minimal_account

# Search for calls with None argument
rg -n -B 5 -A 3 '\.is_minimal_account\(\s*None\s*\)' --type rust

# Also check cache usage with Address::ZERO
rg -n -B 3 -A 3 'Address::ZERO' --type rust -g '*authorization_cache*'

Length of output: 672


🏁 Script executed:

#!/bin/bash
# 1. Find definition of is_minimal_account
rg -n 'fn is_minimal_account' -C3 --type rust

# 2. Show lines 60-100 in delegated_account.rs to inspect None handling
sed -n '60,100p' eip7702-core/src/delegated_account.rs

# 3. Show lines 1-100 in authorization_cache.rs for call-site context
sed -n '1,100p' executors/src/eoa/authorization_cache.rs

Length of output: 3615


Reconsider default behavior when delegation_contract is None.

  • In DelegatedAccount::is_minimal_account (eip7702-core/src/delegated_account.rs:67–74), returning true on None unconditionally bypasses the delegation check—any call without a contract will always succeed. Instead, either require a valid delegation_contract or return an error when it’s missing.
  • In EoaAuthorizationCache (executors/src/eoa/authorization_cache.rs:35–38), using unwrap_or(Address::ZERO) for the cache key collapses all None cases into the same entry, risking collisions. Consider encoding the Option directly in the key or using a distinct sentinel for “no contract.”
🤖 Prompt for AI Agents
In eip7702-core/src/delegated_account.rs around lines 67–74 and
executors/src/eoa/authorization_cache.rs around lines 35–38, the current logic
treats a missing delegation_contract as a successful delegation and collapses
all None cases into Address::ZERO for cache keys; update
DelegatedAccount::is_minimal_account to not unconditionally return true when
delegation_contract is None — either require Some and return false (or propagate
an Err) when it’s missing and adjust the function signature/call sites
accordingly so callers handle the missing contract explicitly; in
EoaAuthorizationCache stop using unwrap_or(Address::ZERO) — change the cache key
to encode the Option (e.g., use Option<Address> or a (bool, Address) tuple) or
choose a distinct sentinel value and document it to avoid collisions so each
None vs Some(contract) is cached separately.


// tracing::debug!(
// eoa_address = ?self.eoa_address,
// target_address = ?target_address,
// minimal_account_address = ?MINIMAL_ACCOUNT_IMPLEMENTATION_ADDRESS,
// has_delegation = is_delegated,
// "EIP-7702 delegation check result"
// );
tracing::debug!(
eoa_address = ?self.eoa_address,
target_address = ?target_address,
minimal_account_address = ?delegation_contract,
has_delegation = is_delegated,
"EIP-7702 delegation check result"
);

Ok(true)
Ok(is_delegated)
}

/// Get the EOA address
Expand Down
2 changes: 1 addition & 1 deletion eip7702-core/src/transaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ impl<C: Chain> MinimalAccountTransaction<C> {
credentials: &SigningCredential,
delegation_contract: Address,
) -> Result<Self, EngineError> {
if self.account.is_minimal_account().await? {
if self.account.is_minimal_account(Some(delegation_contract)).await? {
return Ok(self);
}

Expand Down
11 changes: 6 additions & 5 deletions eip7702-core/tests/integration_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -657,6 +657,7 @@ impl TestSetup {
async fn test_eip7702_integration() -> Result<(), Box<dyn std::error::Error>> {
// Set up test environment
let mut setup = TestSetup::new().await?;
let delegation_contract = setup.delegation_contract.expect("Delegation contract should be set");

// Step 1: Fetch and set bytecode from Base Sepolia
setup.fetch_and_set_bytecode().await?;
Expand All @@ -666,11 +667,11 @@ async fn test_eip7702_integration() -> Result<(), Box<dyn std::error::Error>> {

// Step 3: Test is_minimal_account - all should be false initially
assert!(
!developer_account.is_minimal_account().await?,
!developer_account.is_minimal_account(Some(delegation_contract)).await?,
"Developer should not be minimal account initially"
);
assert!(
!user_account.is_minimal_account().await?,
!user_account.is_minimal_account(Some(delegation_contract)).await?,
"User should not be minimal account initially"
);
println!("✓ All accounts are not minimal accounts initially");
Expand Down Expand Up @@ -731,7 +732,7 @@ async fn test_eip7702_integration() -> Result<(), Box<dyn std::error::Error>> {
);

assert!(
developer_account.is_minimal_account().await?,
developer_account.is_minimal_account(Some(delegation_contract)).await?,
"Developer should be minimal account after minting"
);

Expand All @@ -751,14 +752,14 @@ async fn test_eip7702_integration() -> Result<(), Box<dyn std::error::Error>> {
.await?;

assert!(
user_account.is_minimal_account().await?,
user_account.is_minimal_account(Some(delegation_contract)).await?,
"User (session key granter) should be minimal account after delegation"
);
println!("✓ User (session key granter) is now a minimal account (delegated by executor)");

// Step 9: Developer is already delegated via add_authorization_if_needed in owner_transaction
assert!(
developer_account.is_minimal_account().await?,
developer_account.is_minimal_account(Some(delegation_contract)).await?,
"Developer (session key grantee) should already be minimal account from earlier delegation"
);
println!("✓ Developer (session key grantee) was already delegated in previous step");
Expand Down
5 changes: 4 additions & 1 deletion executors/src/eoa/authorization_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use moka::future::Cache;
pub struct AuthorizationCacheKey {
eoa_address: Address,
chain_id: u64,
delegation_contract: Address,
}

#[derive(Clone)]
Expand All @@ -24,14 +25,16 @@ impl EoaAuthorizationCache {
pub async fn is_minimal_account<C: Chain>(
&self,
delegated_account: &DelegatedAccount<C>,
delegation_contract: Option<Address>,
) -> Result<bool, EngineError> {
self.inner
.try_get_with(
AuthorizationCacheKey {
eoa_address: delegated_account.eoa_address,
chain_id: delegated_account.chain.chain_id(),
delegation_contract: delegation_contract.unwrap_or(Address::ZERO),
},
delegated_account.is_minimal_account(),
delegated_account.is_minimal_account(delegation_contract),
)
.await
.map_err(|e| e.deref().clone())
Expand Down
2 changes: 1 addition & 1 deletion executors/src/eoa/worker/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ where
// if there's an error checking 7702 delegation here, we'll just assume it's not a minimal account for the purposes of max in flight
let is_minimal_account = self
.authorization_cache
.is_minimal_account(&delegated_account)
.is_minimal_account(&delegated_account, None)
.await
.inspect_err(|e| {
tracing::error!(error = ?e, "Error checking 7702 delegation");
Expand Down
2 changes: 1 addition & 1 deletion server/src/execution_router/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,7 @@ impl ExecutionRouter {
let delegated_account = DelegatedAccount::new(eoa_execution_options.from, chain);
let is_minimal_account = self
.authorization_cache
.is_minimal_account(&delegated_account)
.is_minimal_account(&delegated_account, None)
.await
.map_err(|e| EngineError::InternalError {
message: format!("Failed to check 7702 delegation: {e:?}"),
Expand Down