Always charge for EVM precompile configuration reads

constants.testing.toml

@@ -35,8 +35,6 @@ EVM_BLOCK_PRUNING_THRESHOLD = 10000
 EVM_GAS_METERING_MODE = "Rollup"
 # After this evm block height, the max_fee check will be enforced.
 EVM_MAX_FEE_CHECK_HEIGHT = 0
-# Before this rollup height, custom EVM precompiles are disabled without reading their enabled set.
-ENABLE_EVM_CUSTOM_PRECOMPILES_AT = 0
 # How many rollup blocks to wait before terminating setup mode. While setup mode is enabled,
 # the rollup does not charge gas. This is useful when initializing the rollup with a bridged gas token.
 SETUP_MODE_TERMINATION_HEIGHT = 0 # Disable setup mode entirely by default.

constants.toml

@@ -62,8 +62,6 @@ EVM_BLOCK_PRUNING_THRESHOLD = 10000
 EVM_GAS_METERING_MODE = "Rollup"
 # After this evm block height, the max_fee check will be enforced.
 EVM_MAX_FEE_CHECK_HEIGHT = 0
-# Before this rollup height, custom EVM precompiles are disabled without reading their enabled set.
-ENABLE_EVM_CUSTOM_PRECOMPILES_AT = 0
 # How many rollup blocks to wait before terminating setup mode. While setup mode is enabled,
 # the rollup does not charge gas. This is useful when initializing the rollup with a bridged gas token.
 SETUP_MODE_TERMINATION_HEIGHT = 0 # Disable setup mode entirely by default.

crates/module-system/module-implementations/sov-evm/src/lib.rs

@@ -52,7 +52,6 @@ pub use revm::primitives::hardfork::SpecId;
 use serde::Serialize;
 use sov_address::{EthereumAddress, FromVmAddress};
 use sov_bank::Amount;
-use sov_modules_api::macros::config_value;
 use sov_modules_api::{
     err_detail, AccessoryStateMap, AccessoryStateValue, Context, CoreModuleError, DaSpec,
     ErrorContext, ErrorDetail, GenesisState, Module, ModuleId, ModuleInfo, Spec, StateMap,
@@ -305,16 +304,11 @@ where
         state: &mut Reader,
     ) -> Result<precompiles::SovPrecompileProvider<'a, S, P>, E>
     where
-        Reader: StateReader<User, Error = E> + VersionReader,
+        Reader: StateReader<User, Error = E>,
     {
-        let activation_height: u64 = config_value!("ENABLE_EVM_CUSTOM_PRECOMPILES_AT");
-        // The enabled-set read is metered. Existing rollups that add their first custom precompile
-        // must preserve the old no-read execution path until a coordinated activation height.
-        let enabled_custom_precompiles = enabled_custom_precompiles_at_height(
-            state.rollup_height_to_access().get(),
-            activation_height,
-            || self.enabled_custom_precompile_addresses(state),
-        )?;
+        // Keep this read unconditional so changing the compiled precompile set does not change gas.
+        // Its cost depends only on the enabled set stored in state.
+        let enabled_custom_precompiles = self.enabled_custom_precompile_addresses(state)?;
         Ok(precompiles::SovPrecompileProvider::new(
             P::default(),
             context,
@@ -329,60 +323,13 @@ where
     where
         Reader: StateReader<User, Error = E>,
     {
-        // When the runtime has no compiled-in custom precompiles, the enabled set can never have
-        // any effect: a custom precompile is only ever activated if it is also present in
-        // `P::ADDRESSES` (see `SovPrecompileProvider::custom_precompile_enabled`). Skip the state
-        // read entirely in that case.
-        // This both avoids a tiny bit of extra gas usage for rollups not using custom precompiles,
-        // and provides backwards compatibility for existing pre-precompile rollups.
-        if P::ADDRESSES.is_empty() {
-            return Ok(BTreeSet::new());
-        }
         Ok(self
             .enabled_custom_precompiles
             .get(state)?
             .unwrap_or_default())
     }
 }
 
-fn enabled_custom_precompiles_at_height<E>(
-    rollup_height: u64,
-    activation_height: u64,
-    load_enabled: impl FnOnce() -> Result<BTreeSet<Address>, E>,
-) -> Result<BTreeSet<Address>, E> {
-    if rollup_height < activation_height {
-        return Ok(BTreeSet::new());
-    }
-    load_enabled()
-}
-
-#[cfg(test)]
-mod custom_precompile_activation_tests {
-    use std::convert::Infallible;
-
-    use super::enabled_custom_precompiles_at_height;
-
-    #[test]
-    fn enabled_set_is_not_loaded_before_activation() {
-        let enabled = enabled_custom_precompiles_at_height::<Infallible>(41, 42, || {
-            panic!("enabled precompile set must not be read before activation")
-        })
-        .unwrap();
-        assert!(enabled.is_empty());
-    }
-
-    #[test]
-    fn enabled_set_is_loaded_at_activation() {
-        let mut load_called = false;
-        enabled_custom_precompiles_at_height::<Infallible>(42, 42, || {
-            load_called = true;
-            Ok(Default::default())
-        })
-        .unwrap();
-        assert!(load_called);
-    }
-}
-
 pub(crate) fn to_rollup_address<S: Spec>(address: Address) -> S::Address
 where
     S::Address: FromVmAddress<EthereumAddress>,

crates/module-system/module-implementations/sov-evm/tests/integration/precompiles.rs

@@ -204,6 +204,32 @@ where
     tester
 }
 
+#[track_caller]
+fn execute_noop_call_and_get_gas<RT, Sp>(
+    runner: &mut TestRunner<RT, Sp>,
+    caller: &EvmAccount,
+    recipient: Address,
+) -> u64
+where
+    RT: Runtime<Sp> + EthereumAuthenticator<Sp> + MinimalGenesis<Sp>,
+    Sp: Spec<Da = MockDaSpec, CryptoSpec = MockZkvmCryptoSpec, Storage = TestStorage>,
+{
+    runner.execute_transaction(TransactionTestCase {
+        input: create_evm_tx::<RT, Sp>(0, caller, TxKind::Call(recipient), Bytes::new(), 100_000),
+        assert: Box::new(|ctx, _state| {
+            assert!(ctx.tx_receipt.is_successful());
+        }),
+    });
+
+    runner.query_visible_state(|state| {
+        Evm::<Sp>::default()
+            .receipt(0, state)
+            .expect("noop call should have an EVM receipt")
+            .0
+            .gas_used
+    })
+}
+
 #[track_caller]
 fn assert_precompile_result<RT, Sp>(
     runner: &mut TestRunner<RT, Sp>,
@@ -388,6 +414,65 @@ fn default_evm_keeps_eth_precompiles_and_custom_addresses_are_empty() {
     );
 }
 
+#[test]
+fn provider_read_gas_depends_on_stored_enabled_set_not_compiled_set() {
+    let PrecompileSetup {
+        mut runner,
+        caller,
+        balance_holder,
+        ..
+    } = default_runtime::setup_with_enabled_custom_precompiles(vec![]);
+    let no_compiled_gas =
+        execute_noop_call_and_get_gas(&mut runner, &caller, balance_holder.address());
+
+    let PrecompileSetup {
+        mut runner,
+        caller,
+        balance_holder,
+        ..
+    } = bank_runtime::setup_with_enabled_custom_precompiles(vec![]);
+    let one_compiled_gas =
+        execute_noop_call_and_get_gas(&mut runner, &caller, balance_holder.address());
+
+    let PrecompileSetup {
+        mut runner,
+        caller,
+        balance_holder,
+        ..
+    } = composite_runtime::setup_with_enabled_custom_precompiles(vec![]);
+    let two_compiled_gas =
+        execute_noop_call_and_get_gas(&mut runner, &caller, balance_holder.address());
+
+    assert_eq!(no_compiled_gas, one_compiled_gas);
+    assert_eq!(no_compiled_gas, two_compiled_gas);
+
+    let PrecompileSetup {
+        mut runner,
+        caller,
+        balance_holder,
+        ..
+    } = composite_runtime::setup_with_enabled_custom_precompiles(vec![
+        BANK_BALANCE_PRECOMPILE_ADDRESS,
+    ]);
+    let one_enabled_gas =
+        execute_noop_call_and_get_gas(&mut runner, &caller, balance_holder.address());
+
+    let PrecompileSetup {
+        mut runner,
+        caller,
+        balance_holder,
+        ..
+    } = composite_runtime::setup_with_enabled_custom_precompiles(vec![
+        BANK_BALANCE_PRECOMPILE_ADDRESS,
+        SEQUENCING_TIMESTAMP_PRECOMPILE_ADDRESS,
+    ]);
+    let two_enabled_gas =
+        execute_noop_call_and_get_gas(&mut runner, &caller, balance_holder.address());
+
+    assert!(one_enabled_gas > two_compiled_gas);
+    assert!(two_enabled_gas > one_enabled_gas);
+}
+
 #[test]
 fn bank_precompile_runtime_enables_only_bank_precompile() {
     let PrecompileSetup {