nit

Author: pgherveou

substrate/frame/revive/Cargo.toml

@@ -70,6 +70,7 @@ alloy-consensus = { workspace = true, default-features = true }
 array-bytes = { workspace = true, default-features = true }
 assert_matches = { workspace = true }
 itertools = { workspace = true }
+k256 = { workspace = true, features = ["alloc", "ecdsa"] }
 pretty_assertions = { workspace = true }
 secp256k1 = { workspace = true, features = ["recovery"] }
 serde_json = { workspace = true }

substrate/frame/revive/src/evm/call.rs

@@ -72,9 +72,6 @@ impl GenericTransaction {
 		let is_dry_run = matches!(mode, CreateCallMode::DryRun);
 		let base_fee = <Pallet<T>>::evm_base_fee();
 
-		// EIP-7702: Store the authorization list for later processing
-		let authorization_list = self.authorization_list.clone();
-
 		// We would like to allow for transactions without a chain id to be executed through pallet
 		// revive. These are called unprotected transactions and they are transactions that predate
 		// EIP-155 which do not include a Chain ID. These transactions are still useful today in
@@ -187,20 +184,8 @@ impl GenericTransaction {
 
 				crate::Call::eth_substrate_call::<T> { call: Box::new(call), transaction_encoded }
 					.into()
-			} else if self.authorization_list.is_empty() {
-				crate::Call::eth_call::<T> {
-					dest,
-					value,
-					weight_limit: Zero::zero(),
-					eth_gas_limit: gas,
-					data,
-					transaction_encoded,
-					effective_gas_price,
-					encoded_len,
-				}
-				.into()
 			} else {
-				crate::Call::eth_call_with_authorization_list::<T> {
+				crate::Call::eth_call::<T> {
 					dest,
 					value,
 					weight_limit: Zero::zero(),

substrate/frame/revive/src/evm/eip7702.rs

@@ -185,6 +185,7 @@ fn recover_authority(auth: &AuthorizationListEntry) -> Result<H160, ()> {
 /// - `address`: Target address to delegate to
 /// - `nonce`: Nonce for the authorization
 #[cfg(any(test, feature = "runtime-benchmarks"))]
+#[allow(dead_code)] // Used by benchmarks; tests use Signer::sign_authorization wrapper
 pub fn sign_authorization(
 	signing_key: &k256::ecdsa::SigningKey,
 	chain_id: U256,

substrate/frame/revive/src/exec.rs

@@ -664,6 +664,12 @@ struct Frame<T: Config> {
 	/// The delegate call info of the currently executing frame which was spawned by
 	/// `delegate_call`.
 	delegate: Option<DelegateInfo<T>>,
+	/// The address where the code (and immutable data) originates from.
+	///
+	/// For regular contracts, this equals the contract's own address.
+	/// For delegated accounts (EIP-7702), this is the delegation target's address.
+	/// For explicit delegate_call, this is the callee's address.
+	code_address: H160,
 	/// The output of the last executed call frame.
 	last_frame_output: ExecReturnValue,
 	/// The set of contracts that were created during this call stack.
@@ -1054,111 +1060,134 @@ where
 		input_data: &[u8],
 		exec_config: &ExecConfig<T>,
 	) -> Result<Option<(Frame<T>, ExecutableOrPrecompile<T, E, Self>)>, ExecError> {
-		let (account_id, contract_info, executable, delegate, entry_point) = match frame_args {
-			FrameArgs::Call { dest, cached_info, delegated_call } => {
-				let address = T::AddressMapper::to_address(&dest);
-				let precompile = <AllPrecompiles<T>>::get(address.as_fixed_bytes());
-
-				// which contract info to load is unaffected by the fact if this
-				// is a delegate call or not
-				let contract = match (cached_info, &precompile) {
-					(Some(info), _) => CachedContract::Cached(info),
-					(None, None) =>
-						if let Some(info) = AccountInfo::<T>::load_contract(&address) {
-							CachedContract::Cached(info)
-						} else {
-							CachedContract::None
+		let (account_id, contract_info, executable, delegate, code_address, entry_point) =
+			match frame_args {
+				FrameArgs::Call { dest, cached_info, delegated_call } => {
+					let address = T::AddressMapper::to_address(&dest);
+					let precompile = <AllPrecompiles<T>>::get(address.as_fixed_bytes());
+
+					// which contract info to load is unaffected by the fact if this
+					// is a delegate call or not
+					let contract = match (cached_info, &precompile) {
+						(Some(info), _) => CachedContract::Cached(info),
+						(None, None) => {
+							if let Some(info) = AccountInfo::<T>::load_contract(&address) {
+								CachedContract::Cached(info)
+							} else {
+								CachedContract::None
+							}
 						},
-					(None, Some(precompile)) if precompile.has_contract_info() => {
-						log::trace!(target: LOG_TARGET, "found precompile for address {address:?}");
-						if let Some(info) = AccountInfo::<T>::load_contract(&address) {
-							CachedContract::Cached(info)
-						} else {
-							let info = ContractInfo::new(&address, 0u32.into(), H256::zero())?;
-							CachedContract::Cached(info)
-						}
-					},
-					(None, Some(_)) => CachedContract::None,
-				};
+						(None, Some(precompile)) if precompile.has_contract_info() => {
+							log::trace!(target: LOG_TARGET, "found precompile for address {address:?}");
+							if let Some(info) = AccountInfo::<T>::load_contract(&address) {
+								CachedContract::Cached(info)
+							} else {
+								let info = ContractInfo::new(&address, 0u32.into(), H256::zero())?;
+								CachedContract::Cached(info)
+							}
+						},
+						(None, Some(_)) => CachedContract::None,
+					};
 
-				let delegated_call = delegated_call.or_else(|| {
-					exec_config.mock_handler.as_ref().and_then(|mock_handler| {
-						mock_handler.mock_delegated_caller(address, input_data)
-					})
-				});
-				// in case of delegate the executable is not the one at `address`
-				let executable = if let Some(delegated_call) = &delegated_call {
-					if let Some(precompile) =
-						<AllPrecompiles<T>>::get(delegated_call.callee.as_fixed_bytes())
-					{
-						ExecutableOrPrecompile::Precompile {
-							instance: precompile,
-							_phantom: Default::default(),
+					let delegated_call = delegated_call.or_else(|| {
+						exec_config.mock_handler.as_ref().and_then(|mock_handler| {
+							mock_handler.mock_delegated_caller(address, input_data)
+						})
+					});
+					// in case of delegate the executable is not the one at `address`
+					// code_address tracks where the code (and immutable data) comes from
+					let (executable, code_address) = if let Some(delegated_call) = &delegated_call {
+						if let Some(precompile) =
+							<AllPrecompiles<T>>::get(delegated_call.callee.as_fixed_bytes())
+						{
+							(
+								ExecutableOrPrecompile::Precompile {
+									instance: precompile,
+									_phantom: Default::default(),
+								},
+								delegated_call.callee,
+							)
+						} else {
+							let Some(info) =
+								AccountInfo::<T>::load_contract(&delegated_call.callee)
+							else {
+								return Ok(None);
+							};
+							let executable = E::from_storage(info.code_hash, meter)?;
+							(ExecutableOrPrecompile::Executable(executable), delegated_call.callee)
 						}
 					} else {
-						let Some(info) = AccountInfo::<T>::load_contract(&delegated_call.callee)
-						else {
-							return Ok(None);
-						};
-						let executable = E::from_storage(info.code_hash, meter)?;
-						ExecutableOrPrecompile::Executable(executable)
-					}
-				} else {
-					if let Some(precompile) = precompile {
-						ExecutableOrPrecompile::Precompile {
-							instance: precompile,
-							_phantom: Default::default(),
+						// For regular calls, check if the address is delegated (EIP-7702)
+						// If so, use the delegation target as the code source
+						let code_addr =
+							AccountInfo::<T>::get_delegation_target(&address).unwrap_or(address);
+						if let Some(precompile) = precompile {
+							(
+								ExecutableOrPrecompile::Precompile {
+									instance: precompile,
+									_phantom: Default::default(),
+								},
+								code_addr,
+							)
+						} else {
+							let Some(info) = AccountInfo::<T>::load_contract(&address) else {
+								return Ok(None);
+							};
+							let executable = E::from_storage(info.code_hash, meter)?;
+							(ExecutableOrPrecompile::Executable(executable), code_addr)
 						}
-					} else {
-						let Some(info) = AccountInfo::<T>::load_contract(&address) else {
-							return Ok(None);
-						};
-						let executable = E::from_storage(info.code_hash, meter)?;
-						ExecutableOrPrecompile::Executable(executable)
-					}
-				};
+					};
 
-				(dest, contract, executable, delegated_call, ExportedFunction::Call)
-			},
-			FrameArgs::Instantiate { sender, executable, salt, input_data } => {
-				let deployer = T::AddressMapper::to_address(&sender);
-				let account_nonce = <System<T>>::account_nonce(&sender);
-				let address = if let Some(salt) = salt {
-					address::create2(&deployer, executable.code(), input_data, salt)
-				} else {
-					use sp_runtime::Saturating;
-					address::create1(
-						&deployer,
-						// the Nonce from the origin has been incremented pre-dispatch, so we
-						// need to subtract 1 to get the nonce at the time of the call.
-						if origin_is_caller {
-							account_nonce.saturating_sub(1u32.into()).saturated_into()
-						} else {
-							account_nonce.saturated_into()
-						},
+					(
+						dest,
+						contract,
+						executable,
+						delegated_call,
+						code_address,
+						ExportedFunction::Call,
 					)
-				};
-				let contract = ContractInfo::new(
-					&address,
-					<System<T>>::account_nonce(&sender),
-					*executable.code_hash(),
-				)?;
-				(
-					T::AddressMapper::to_fallback_account_id(&address),
-					CachedContract::Cached(contract),
-					ExecutableOrPrecompile::Executable(executable),
-					None,
-					ExportedFunction::Constructor,
-				)
-			},
-		};
+				},
+				FrameArgs::Instantiate { sender, executable, salt, input_data } => {
+					let deployer = T::AddressMapper::to_address(&sender);
+					let account_nonce = <System<T>>::account_nonce(&sender);
+					let address = if let Some(salt) = salt {
+						address::create2(&deployer, executable.code(), input_data, salt)
+					} else {
+						use sp_runtime::Saturating;
+						address::create1(
+							&deployer,
+							// the Nonce from the origin has been incremented pre-dispatch, so we
+							// need to subtract 1 to get the nonce at the time of the call.
+							if origin_is_caller {
+								account_nonce.saturating_sub(1u32.into()).saturated_into()
+							} else {
+								account_nonce.saturated_into()
+							},
+						)
+					};
+					let contract = ContractInfo::new(
+						&address,
+						<System<T>>::account_nonce(&sender),
+						*executable.code_hash(),
+					)?;
+					(
+						T::AddressMapper::to_fallback_account_id(&address),
+						CachedContract::Cached(contract),
+						ExecutableOrPrecompile::Executable(executable),
+						None,
+						address, // For instantiate, code_address is the new contract's address
+						ExportedFunction::Constructor,
+					)
+				},
+			};
 
 		let frame = Frame {
 			delegate,
 			value_transferred,
 			contract_info,
 			account_id,
 			entry_point,
+			code_address,
 			frame_meter: meter.new_nested(call_resources)?,
 			allows_reentry: true,
 			read_only,
@@ -1938,13 +1967,9 @@ where
 			return Err(Error::<T>::InvalidImmutableAccess.into());
 		}
 
-		// Immutable is read from contract code being executed
-		let address = self
-			.top_frame()
-			.delegate
-			.as_ref()
-			.map(|d| d.callee)
-			.unwrap_or(T::AddressMapper::to_address(self.account_id()));
+		// Immutable data is read from the address where the code originates.
+		// This handles regular contracts, delegated accounts (EIP-7702), and delegate_call.
+		let address = self.top_frame().code_address;
 		Ok(<ImmutableDataOf<T>>::get(address).ok_or_else(|| Error::<T>::InvalidImmutableAccess)?)
 	}
 

substrate/frame/revive/src/lib.rs

@@ -1345,32 +1345,38 @@ pub mod pallet {
 			transaction_encoded: Vec<u8>,
 			effective_gas_price: U256,
 			encoded_len: u32,
-			authorization_list: Vec<evm::SignedAuthorizationListEntry>,
+			authorization_list: Vec<evm::AuthorizationListEntry>,
 		) -> DispatchResultWithPostInfo {
 			let signer = Self::ensure_eth_signed(origin)?;
 			let origin = OriginFor::<T>::signed(signer.clone());
 
 			Self::ensure_non_contract_if_signed(&origin)?;
 
+			// Process EIP-7702 authorizations with weight metering
 			let (eth_gas_limit, weight_limit) = if !authorization_list.is_empty() {
 				let chain_id = U256::from(T::ChainId::get());
-				let (new_accounts, existing_accounts) =
-					evm::eip7702::process_authorizations::<T>(&authorization_list, chain_id);
 
+				// Calculate worst-case weight for authorization processing
 				let auth_count = authorization_list.len() as u64;
-				let single_auth_weight = T::WeightInfo::process_single_authorization();
-				let auth_weight = single_auth_weight
+				let auth_weight = T::WeightInfo::validate_authorization()
 					.saturating_mul(auth_count)
-					.saturating_add(T::WeightInfo::apply_delegations_existing(
-						existing_accounts as u32,
-					))
-					.saturating_add(T::WeightInfo::apply_delegations_new(new_accounts as u32));
-
+					.saturating_add(T::WeightInfo::apply_delegation(1).saturating_mul(auth_count));
+
+				// Create a weight meter and process authorizations
+				let mut meter = frame_support::weights::WeightMeter::with_limit(auth_weight);
+				evm::eip7702::process_authorizations::<T>(
+					&authorization_list,
+					chain_id,
+					&mut meter,
+				)?;
+
+				// Adjust gas/weight limits by consumed weight
+				let consumed = meter.consumed();
 				let gas_scale: u64 = T::GasScale::get().into();
-				let auth_gas = auth_weight.ref_time().saturating_div(gas_scale);
+				let auth_gas = consumed.ref_time().saturating_div(gas_scale);
 
 				let adjusted_gas_limit = eth_gas_limit.saturating_sub(U256::from(auth_gas));
-				let adjusted_weight_limit = weight_limit.saturating_sub(auth_weight);
+				let adjusted_weight_limit = weight_limit.saturating_sub(consumed);
 
 				(adjusted_gas_limit, adjusted_weight_limit)
 			} else {
@@ -1386,7 +1392,7 @@ pub mod pallet {
 				transaction_encoded: transaction_encoded.clone(),
 				effective_gas_price,
 				encoded_len,
-				authorization_list,
+				authorization_list: authorization_list.clone(),
 			}
 			.into();
 			let info = T::FeeInfo::dispatch_info(&call);

substrate/frame/revive/src/metering/storage.rs

@@ -409,11 +409,12 @@ where
 								self.total_deposit = self.total_deposit.saturating_sub(&amount);
 								last.state = ContractState::Terminated;
 							},
-							(ContractState::Terminated, ContractState::Terminated) =>
+							(ContractState::Terminated, ContractState::Terminated) => {
 								debug_assert!(
 									false,
 									"We never emit two terminates for the same contract."
-								),
+								)
+							},
 						}
 						continue;
 					}

substrate/frame/revive/src/storage.rs

@@ -293,7 +293,6 @@ impl<T: Config> AccountInfo<T> {
 					updated.code_hash = target_code_hash;
 					updated
 				} else {
-					// Not currently delegated, create new ContractInfo
 					let account_id = T::AddressMapper::to_account_id(address);
 					let nonce = frame_system::Pallet::<T>::account_nonce(&account_id);
 					ContractInfo::<T>::new_for_delegation(address, nonce, target_code_hash)?