Fail the whole EVM transaction if an internal Scilla call fails (#2016)

* Fix for failing scilla_call with whitelisted contracts * Update fork heights * Update fork heights again --------- Co-authored-by: Bartosz Zawistowski <[email protected]>
Zilliqa · Dec 18, 2024 · ae82b0c · ae82b0c
1 parent 1736ed5
commit ae82b0c
Show file tree

Hide file tree

Showing 12 changed files with 136 additions and 73 deletions.
diff --git a/z2/resources/chain-specs/zq2-protomainnet.toml b/z2/resources/chain-specs/zq2-protomainnet.toml
@@ -34,4 +34,8 @@ consensus.scilla_call_gas_exempt_addrs = [
   "0x9C3fE3f471d8380297e4fB222eFb313Ee94DFa0f",
   "0x20Dd5D5B5d4C72676514A0eA1052d0200003d69D",
   "0xbfDe2156aF75a29d36614bC1F8005DD816Bd9200"
-]
+]
+consensus.forks = [
+  { at_height = 0, failed_scilla_call_from_gas_exempt_caller_causes_revert = false },
+  { at_height = 5299000, failed_scilla_call_from_gas_exempt_caller_causes_revert = true },
+]
diff --git a/z2/resources/chain-specs/zq2-prototestnet.toml b/z2/resources/chain-specs/zq2-prototestnet.toml
@@ -28,4 +28,8 @@ consensus.scilla_call_gas_exempt_addrs = [
   "0x8895Aa1bEaC254E559A3F91e579CF4a67B70ce02",
   "0x453b11386FBd54bC532892c0217BBc316fc7b918",
   "0xaD581eC62eA08831c8FE2Cd7A1113473fE40A057"
-]
+]
+consensus.forks = [
+  { at_height = 0, failed_scilla_call_from_gas_exempt_caller_causes_revert = false },
+  { at_height = 8404000, failed_scilla_call_from_gas_exempt_caller_causes_revert = true },
+]
diff --git a/z2/resources/config.tera.toml b/z2/resources/config.tera.toml
@@ -39,3 +39,5 @@ enable_ots_indices = true
 file = "{{ checkpoint_file }}"
 hash = "{{ checkpoint_hash }}"
 {%- endif %}
+
+consensus.forks = {{ forks }}
diff --git a/z2/src/chain.rs b/z2/src/chain.rs
@@ -5,6 +5,7 @@ pub mod node;
 use anyhow::{anyhow, Result};
 use clap::ValueEnum;
 use colored::Colorize;
+use serde_json::{json, Value};
 use strum::EnumProperty;
 use strum_macros::{Display, EnumString};
 use zilliqa::cfg::ContractUpgradesBlockHeights;
@@ -167,6 +168,22 @@ impl Chain {
         }
     }
 
+    pub fn get_forks(&self) -> Vec<Value> {
+        match self {
+            Chain::Zq2ProtoTestnet => vec![
+                json!({ "at_height": 0, "failed_scilla_call_from_gas_exempt_caller_causes_revert": false }),
+                // estimated: 2024-12-18T14:57:53Z
+                json!({ "at_height": 8404000, "failed_scilla_call_from_gas_exempt_caller_causes_revert": true }),
+            ],
+            Chain::Zq2ProtoMainnet => vec![
+                json!({ "at_height": 0, "failed_scilla_call_from_gas_exempt_caller_causes_revert": false }),
+                // estimated: 2024-12-19T15:03:05Z
+                json!({ "at_height": 5299000, "failed_scilla_call_from_gas_exempt_caller_causes_revert": true }),
+            ],
+            _ => vec![],
+        }
+    }
+
     pub fn get_endpoint(&self) -> Result<&'static str> {
         let endpoint = self.get_str("endpoint");
 

diff --git a/z2/src/chain/node.rs b/z2/src/chain/node.rs
@@ -557,6 +557,15 @@ impl ChainNode {
         let toml_servers: toml::Value = serde_json::from_value(api_servers)?;
         ctx.insert("api_servers", &toml_servers.to_string());
         ctx.insert("enable_ots_indices", &enable_ots_indices);
+        ctx.insert(
+            "forks",
+            &self
+                .chain()?
+                .get_forks()
+                .into_iter()
+                .map(|f| Ok(serde_json::from_value::<toml::Value>(f)?.to_string()))
+                .collect::<Result<Vec<_>>>()?,
+        );
 
         if let Some(checkpoint_url) = self.chain.checkpoint_url() {
             if self.role == NodeRole::Validator {

diff --git a/z2/src/setup.rs b/z2/src/setup.rs
@@ -29,7 +29,7 @@ use zilliqa::{
         max_blocks_in_flight_default, minimum_time_left_for_empty_block_default,
         scilla_address_default, scilla_ext_libs_path_default, scilla_stdlib_dir_default,
         state_rpc_limit_default, total_native_token_supply_default, Amount, ConsensusConfig,
-        ContractUpgradesBlockHeights, GenesisDeposit,
+        ContractUpgradesBlockHeights, Forks, GenesisDeposit,
     },
     transaction::EvmGas,
 };
@@ -539,6 +539,7 @@ impl Setup {
                     total_native_token_supply: total_native_token_supply_default(),
                     scilla_call_gas_exempt_addrs: vec![],
                     contract_upgrade_block_heights: ContractUpgradesBlockHeights::default(),
+                    forks: Forks::default(),
                 },
                 block_request_limit: block_request_limit_default(),
                 max_blocks_in_flight: max_blocks_in_flight_default(),

diff --git a/zilliqa/src/cfg.rs b/zilliqa/src/cfg.rs
@@ -365,6 +365,10 @@ pub struct ConsensusConfig {
     /// The block heights at which we perform EIP-1967 contract upgrades
     #[serde(default)]
     pub contract_upgrade_block_heights: ContractUpgradesBlockHeights,
+    /// Forks in block execution logic. Each entry describes the difference in logic and the block height at which that
+    /// difference applies.
+    #[serde(default)]
+    pub forks: Forks,
 }
 
 impl Default for ConsensusConfig {
@@ -391,10 +395,73 @@ impl Default for ConsensusConfig {
             total_native_token_supply: total_native_token_supply_default(),
             scilla_call_gas_exempt_addrs: vec![],
             contract_upgrade_block_heights: ContractUpgradesBlockHeights::default(),
+            forks: Default::default(),
         }
     }
 }
 
+#[derive(Clone, Debug, Serialize, Deserialize)]
+#[serde(try_from = "Vec<Fork>", into = "Vec<Fork>")]
+pub struct Forks(Vec<Fork>);
+
+impl TryFrom<Vec<Fork>> for Forks {
+    type Error = anyhow::Error;
+
+    fn try_from(mut forks: Vec<Fork>) -> Result<Self, Self::Error> {
+        // Sort forks by height so we can binary search to find the current fork.
+        forks.sort_unstable_by_key(|f| f.at_height);
+
+        // Assert we have a fork that starts at the genesis block.
+        if forks.first().ok_or_else(|| anyhow!("no forks"))?.at_height != 0 {
+            return Err(anyhow!("first fork must start at height 0"));
+        }
+
+        Ok(Forks(forks))
+    }
+}
+
+impl From<Forks> for Vec<Fork> {
+    fn from(forks: Forks) -> Self {
+        forks.0
+    }
+}
+
+impl Default for Forks {
+    /// The default implementation of [Forks] returns a single fork at the genesis block, with the most up-to-date
+    /// execution logic.
+    fn default() -> Self {
+        vec![Fork {
+            at_height: 0,
+            failed_scilla_call_from_gas_exempt_caller_causes_revert: true,
+        }]
+        .try_into()
+        .unwrap()
+    }
+}
+
+impl Forks {
+    pub fn get(&self, height: u64) -> Fork {
+        // Binary search to find the fork at the specified height. If an entry was not found at exactly the specified
+        // height, the `Err` returned from `binary_search_by_key` will contain the index where an element with this
+        // height could be inserted. By subtracting one from this, we get the maximum entry with a height less than the
+        // searched height.
+        let index = self
+            .0
+            .binary_search_by_key(&height, |f| f.at_height)
+            .unwrap_or_else(|i| i - 1);
+        self.0[index]
+    }
+}
+
+#[derive(Copy, Clone, Debug, Serialize, Deserialize)]
+pub struct Fork {
+    pub at_height: u64,
+    /// If true, if a caller who is in the `scilla_call_gas_exempt_addrs` list makes a call to the `scilla_call`
+    /// precompile and the inner Scilla call fails, the entire transaction will revert. If false, the normal EVM
+    /// semantics apply where the caller can decide how to act based on the success of the inner call.
+    pub failed_scilla_call_from_gas_exempt_caller_causes_revert: bool,
+}
+
 #[derive(Debug, Clone, Serialize, Deserialize)]
 #[serde(deny_unknown_fields)]
 pub struct GenesisDeposit {

diff --git a/zilliqa/src/exec.rs b/zilliqa/src/exec.rs
@@ -30,7 +30,7 @@ use sha2::{Digest, Sha256};
 use tracing::{debug, info, trace, warn};
 
 use crate::{
-    cfg::{ScillaExtLibsPath, ScillaExtLibsPathInScilla, ScillaExtLibsPathInZq2},
+    cfg::{Fork, ScillaExtLibsPath, ScillaExtLibsPathInScilla, ScillaExtLibsPathInZq2},
     constants, contracts,
     crypto::{Hash, NodePublicKey},
     db::TrieStorage,
@@ -425,6 +425,7 @@ impl DatabaseRef for &State {
 /// The external context used by [Evm].
 pub struct ExternalContext<'a, I> {
     pub inspector: I,
+    pub fork: Fork,
     pub scilla_call_gas_exempt_addrs: &'a [Address],
     // This flag is only used for zq1 whitelisted contracts, and it's used to detect if the entire transaction should be marked as failed
     pub enforce_transaction_failure: bool,
@@ -516,6 +517,7 @@ impl State {
 
         let external_context = ExternalContext {
             inspector,
+            fork: self.forks.get(current_block.number),
             scilla_call_gas_exempt_addrs: &self.scilla_call_gas_exempt_addrs,
             enforce_transaction_failure: false,
         };

diff --git a/zilliqa/src/precompiles/scilla.rs b/zilliqa/src/precompiles/scilla.rs
@@ -350,22 +350,9 @@ pub fn scilla_call_handle_register<I: ScillaInspector>(
 
         // The behaviour is different for contracts having 21k gas and/or deployed with zq1
         // 1. If gas == 21k and gas_exempt -> allow it to run with gas_left()
-        // 2. if gas == 21k and NOT gas_exempt -> mark entire txn as failed (not only the current precompile)
+        // 2. if precompile failed and gas_exempt -> mark entire txn as failed (not only the current precompile)
         // 3. Otherwise, let it run with what it's given and let the caller decide
 
-        // Below is case 2nd
-        if gas.limit() == 21000 && !gas_exempt {
-            ctx.external.enforce_transaction_failure = true;
-            return Ok(FrameOrResult::new_call_result(
-                InterpreterResult {
-                    result: InstructionResult::OutOfGas,
-                    gas,
-                    output: Bytes::new(),
-                },
-                inputs.return_memory_offset.clone(),
-            ));
-        }
-
         let outcome = scilla_call_precompile(
             &inputs,
             gas.limit(),
@@ -400,6 +387,22 @@ pub fn scilla_call_handle_register<I: ScillaInspector>(
             Err(PrecompileErrors::Fatal { msg }) => return Err(EVMError::Precompile(msg)),
         }
 
+        if ctx
+            .external
+            .fork
+            .failed_scilla_call_from_gas_exempt_caller_causes_revert
+        {
+            // If precompile failed and this is whitelisted contract -> mark entire transaction as failed
+            match result.result {
+                InstructionResult::Return => {}
+                _ => {
+                    if gas_exempt {
+                        ctx.external.enforce_transaction_failure = true;
+                    }
+                }
+            }
+        }
+
         Ok(FrameOrResult::new_call_result(
             result,
             inputs.return_memory_offset.clone(),

diff --git a/zilliqa/src/state.rs b/zilliqa/src/state.rs
@@ -18,7 +18,7 @@ use tracing::debug;
 
 use crate::{
     block_store::BlockStore,
-    cfg::{Amount, NodeConfig, ScillaExtLibsPath},
+    cfg::{Amount, Forks, NodeConfig, ScillaExtLibsPath},
     contracts::{self, Contract},
     crypto::{self, Hash},
     db::TrieStorage,
@@ -53,6 +53,7 @@ pub struct State {
     pub gas_price: u128,
     pub scilla_call_gas_exempt_addrs: Vec<Address>,
     pub chain_id: ChainId,
+    pub forks: Forks,
     pub block_store: Arc<BlockStore>,
 }
 
@@ -72,6 +73,7 @@ impl State {
             gas_price: *consensus_config.gas_price,
             scilla_call_gas_exempt_addrs: consensus_config.scilla_call_gas_exempt_addrs.clone(),
             chain_id: ChainId::new(config.eth_chain_id),
+            forks: consensus_config.forks.clone(),
             block_store,
         }
     }
@@ -284,6 +286,7 @@ impl State {
             scilla_call_gas_exempt_addrs: self.scilla_call_gas_exempt_addrs.clone(),
             chain_id: self.chain_id,
             block_store: self.block_store.clone(),
+            forks: self.forks.clone(),
         }
     }
 

diff --git a/zilliqa/tests/it/main.rs b/zilliqa/tests/it/main.rs
@@ -71,7 +71,7 @@ use zilliqa::{
         minimum_time_left_for_empty_block_default, scilla_address_default,
         scilla_ext_libs_path_default, scilla_stdlib_dir_default, state_cache_size_default,
         state_rpc_limit_default, total_native_token_supply_default, Amount, ApiServer, Checkpoint,
-        ConsensusConfig, ContractUpgradesBlockHeights, GenesisDeposit, NodeConfig,
+        ConsensusConfig, ContractUpgradesBlockHeights, Forks, GenesisDeposit, NodeConfig,
     },
     crypto::{SecretKey, TransactionPublicKey},
     db,
@@ -353,6 +353,7 @@ impl Network {
                     Address::new(get_contract_address(secret_key_to_address(&genesis_key).0, 2).0),
                 ],
                 contract_upgrade_block_heights,
+                forks: Forks::default(),
             },
             api_servers: vec![ApiServer {
                 port: 4201,
@@ -491,6 +492,7 @@ impl Network {
                     get_contract_address(secret_key_to_address(&self.genesis_key).0, 2).0,
                 )],
                 contract_upgrade_block_heights,
+                forks: Forks::default(),
             },
             block_request_limit: block_request_limit_default(),
             max_blocks_in_flight: max_blocks_in_flight_default(),

diff --git a/zilliqa/tests/it/persistence.rs b/zilliqa/tests/it/persistence.rs
@@ -8,18 +8,8 @@ use primitive_types::H160;
 use rand::Rng;
 use tracing::*;
 use zilliqa::{
-    api,
-    cfg::{
-        allowed_timestamp_skew_default, block_request_batch_size_default,
-        block_request_limit_default, consensus_timeout_default, eth_chain_id_default,
-        failed_request_sleep_duration_default, max_blocks_in_flight_default,
-        max_rpc_response_size_default, minimum_time_left_for_empty_block_default,
-        scilla_address_default, scilla_ext_libs_path_default, scilla_stdlib_dir_default,
-        state_cache_size_default, state_rpc_limit_default, total_native_token_supply_default,
-        ApiServer, Checkpoint, ConsensusConfig, ContractUpgradesBlockHeights, NodeConfig,
-    },
+    cfg::Checkpoint,
     crypto::{Hash, SecretKey},
-    transaction::EvmGas,
 };
 
 use crate::{
@@ -92,50 +82,9 @@ async fn block_and_tx_data_persistence(mut network: Network) {
 
     // drop and re-create the node using the same datadir:
     drop(inner);
+    let config = node.inner.lock().unwrap().config.clone();
     #[allow(clippy::redundant_closure_call)]
     let dir = (|mut node: TestNode| node.dir.take())(node).unwrap(); // move dir out and drop the rest of node
-    let config = NodeConfig {
-        consensus: ConsensusConfig {
-            is_main: true,
-            genesis_accounts: Network::genesis_accounts(&network.genesis_key),
-            empty_block_timeout: Duration::from_millis(25),
-            local_address: "host.docker.internal".to_owned(),
-            rewards_per_hour: 204_000_000_000_000_000_000_000u128.into(),
-            blocks_per_hour: 3600 * 40,
-            minimum_stake: 32_000_000_000_000_000_000u128.into(),
-            eth_block_gas_limit: EvmGas(84000000),
-            gas_price: 4_761_904_800_000u128.into(),
-            consensus_timeout: consensus_timeout_default(),
-            genesis_deposits: Vec::new(),
-            main_shard_id: None,
-            minimum_time_left_for_empty_block: minimum_time_left_for_empty_block_default(),
-            scilla_address: scilla_address_default(),
-            blocks_per_epoch: 10,
-            epochs_per_checkpoint: 1,
-            scilla_stdlib_dir: scilla_stdlib_dir_default(),
-            scilla_ext_libs_path: scilla_ext_libs_path_default(),
-            total_native_token_supply: total_native_token_supply_default(),
-            scilla_call_gas_exempt_addrs: vec![],
-            contract_upgrade_block_heights: ContractUpgradesBlockHeights::default(),
-        },
-        allowed_timestamp_skew: allowed_timestamp_skew_default(),
-        data_dir: None,
-        state_cache_size: state_cache_size_default(),
-        load_checkpoint: None,
-        do_checkpoints: false,
-        api_servers: vec![ApiServer {
-            port: 4201,
-            enabled_apis: api::all_enabled(),
-        }],
-        eth_chain_id: eth_chain_id_default(),
-        block_request_limit: block_request_limit_default(),
-        max_blocks_in_flight: max_blocks_in_flight_default(),
-        block_request_batch_size: block_request_batch_size_default(),
-        state_rpc_limit: state_rpc_limit_default(),
-        failed_request_sleep_duration: failed_request_sleep_duration_default(),
-        enable_ots_indices: true,
-        max_rpc_response_size: max_rpc_response_size_default(),
-    };
     let mut rng = network.rng.lock().unwrap();
     let result = crate::node(
         config,