Strip redundant EIP-7702 authorizations#104
Conversation
Add logic to filter out signed EIP-7702 authorizations that are already delegated to the target contract. Introduces a helper (filter_already_delegated_authorizations) which queries the account code, detects an existing delegation via EIP-7702 prefix/length, extracts the delegation target, and removes any matching authorizations from the list. This prevents RPC-level rejections on strict chains (e.g. Etherlink) caused by sending stale/redundant authorizations. The build path now uses the filtered list and only attaches an authorization_list if non-empty; provider lookup failures are logged and result in leaving the list unchanged.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
Disabled knowledge base sources:
WalkthroughAdds EIP-7702 delegation-aware authorization filtering: when a transaction has a Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Builder as TransactionBuilder
participant RPC as OnchainRPC
participant Executor
Client->>Builder: build_typed_transaction(request with authorization_list)
alt request.to present
Builder->>RPC: getCode(request.to)
RPC-->>Builder: bytecode
Builder->>Builder: detect EIP-7702 delegation && derive delegatedTarget
Builder->>Builder: filter authorization_list removing entries with auth.address == delegatedTarget
end
Builder->>Executor: finalized transaction (authorization_list set only if filtered non-empty)
Executor-->>Client: tx ready / result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
executors/src/eoa/worker/transaction.rs (1)
271-275: Avoid hardcoded offset/length when extracting delegated target.Line 274 hardcodes
3..23; deriving bounds from the prefix constant makes this safer against future protocol-constant changes.Refactor sketch
- let delegated_to = Address::from_slice(&code[3..23]); + let prefix_len = EIP_7702_DELEGATION_PREFIX.len(); + let delegated_to = Address::from_slice(&code[prefix_len..prefix_len + 20]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@executors/src/eoa/worker/transaction.rs` around lines 271 - 275, The code extracts the delegated address using a hardcoded slice `code[3..23]`; instead compute start = EIP_7702_DELEGATION_PREFIX.len() and end = start + 20 (or use Address::LEN if available) and use `&code[start..end]` so the bounds follow the prefix constant; update the `delegated_to` assignment (the `delegated_to` variable and `code` usage) to use these computed bounds and ensure you still check `code.len() >= EIP_7702_DELEGATION_CODE_LENGTH` (or validate `code.len() >= end`) before slicing to avoid panics.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@executors/src/eoa/worker/transaction.rs`:
- Around line 260-264: The function filter_already_delegated_authorizations
currently checks EIP-7702 delegation bytecode at the `to` address, but the
delegation bytecode lives on the delegating/authority account (the
authorization's `from`/authorizer), so update the checks in
filter_already_delegated_authorizations (and the related checks at the other
occurrences noted) to inspect the bytecode/account state of the authorization's
authorizer (the `SignedAuthorization`'s from/authorizer address) instead of
`to`; specifically, replace uses of `to` when querying bytecode or delegation
status with the SignedAuthorization's source address (e.g., authorization.from
or authorization.authorizer) so redundant-authorizations are detected correctly
even when `to != from`.
---
Nitpick comments:
In `@executors/src/eoa/worker/transaction.rs`:
- Around line 271-275: The code extracts the delegated address using a hardcoded
slice `code[3..23]`; instead compute start = EIP_7702_DELEGATION_PREFIX.len()
and end = start + 20 (or use Address::LEN if available) and use
`&code[start..end]` so the bounds follow the prefix constant; update the
`delegated_to` assignment (the `delegated_to` variable and `code` usage) to use
these computed bounds and ensure you still check `code.len() >=
EIP_7702_DELEGATION_CODE_LENGTH` (or validate `code.len() >= end`) before
slicing to avoid panics.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3b7f6b90-f98e-4436-84a5-e2f5de629a62
📒 Files selected for processing (1)
executors/src/eoa/worker/transaction.rs
| async fn filter_already_delegated_authorizations( | ||
| &self, | ||
| authorization_list: &[SignedAuthorization], | ||
| to: Option<Address>, | ||
| ) -> Vec<SignedAuthorization> { |
There was a problem hiding this comment.
Check delegation on the authority (from) account, not to.
Line 269 checks bytecode at to, but EIP-7702 delegation bytecode is on the delegating EOA. This can miss redundant-authorizations when to != from, so strict-chain rejections can still happen. This also differs from eip7702-core/src/delegated_account.rs:30-73, which checks self.eoa_address.
Suggested fix
- async fn filter_already_delegated_authorizations(
- &self,
- authorization_list: &[SignedAuthorization],
- to: Option<Address>,
- ) -> Vec<SignedAuthorization> {
- // If we have a `to` address, check if it's already delegated to any of the
- // authorization targets. In the 7702 relayer flow, `to` is the user's smart
- // account and the authorization targets the delegation contract.
- if let Some(account_address) = to {
- match self.chain.provider().get_code_at(account_address).await {
+ async fn filter_already_delegated_authorizations(
+ &self,
+ authorization_list: &[SignedAuthorization],
+ authority: Address,
+ ) -> Vec<SignedAuthorization> {
+ let account_address = authority;
+ match self.chain.provider().get_code_at(account_address).await {
Ok(code) => {
if code.len() >= EIP_7702_DELEGATION_CODE_LENGTH
&& code.starts_with(&EIP_7702_DELEGATION_PREFIX)
{
let delegated_to = Address::from_slice(&code[3..23]);
@@
- }
- Err(e) => {
+ }
+ Err(e) => {
tracing::warn!(
account = ?account_address,
error = ?e,
"Failed to check delegation status, keeping all authorizations"
);
}
- }
}
-
authorization_list.to_vec()
}- let filtered = self
- .filter_already_delegated_authorizations(authorization_list, request.to)
+ let filtered = self
+ .filter_already_delegated_authorizations(authorization_list, request.from)
.await;Also applies to: 268-270, 363-364
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@executors/src/eoa/worker/transaction.rs` around lines 260 - 264, The function
filter_already_delegated_authorizations currently checks EIP-7702 delegation
bytecode at the `to` address, but the delegation bytecode lives on the
delegating/authority account (the authorization's `from`/authorizer), so update
the checks in filter_already_delegated_authorizations (and the related checks at
the other occurrences noted) to inspect the bytecode/account state of the
authorization's authorizer (the `SignedAuthorization`'s from/authorizer address)
instead of `to`; specifically, replace uses of `to` when querying bytecode or
delegation status with the SignedAuthorization's source address (e.g.,
authorization.from or authorization.authorizer) so redundant-authorizations are
detected correctly even when `to != from`.
|
Note: I know that we're not recovering the signer in the traditional way, but the |
Add logic to filter out signed EIP-7702 authorizations that are already delegated to the target contract. Introduces a helper (filter_already_delegated_authorizations) which queries the account code, detects an existing delegation via EIP-7702 prefix/length, extracts the delegation target, and removes any matching authorizations from the list. This prevents RPC-level rejections on strict chains (e.g. Etherlink) caused by sending stale/redundant authorizations. The build path now uses the filtered list and only attaches an authorization_list if non-empty; provider lookup failures are logged and result in leaving the list unchanged.
Summary by CodeRabbit