From ae82b0c088224139b0118db306359e1b45e4bd91 Mon Sep 17 00:00:00 2001 From: James Hinshelwood Date: Wed, 18 Dec 2024 11:45:42 +0000 Subject: [PATCH] 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 --- .../chain-specs/zq2-protomainnet.toml | 6 +- .../chain-specs/zq2-prototestnet.toml | 6 +- z2/resources/config.tera.toml | 2 + z2/src/chain.rs | 17 +++++ z2/src/chain/node.rs | 9 +++ z2/src/setup.rs | 3 +- zilliqa/src/cfg.rs | 67 +++++++++++++++++++ zilliqa/src/exec.rs | 4 +- zilliqa/src/precompiles/scilla.rs | 31 +++++---- zilliqa/src/state.rs | 5 +- zilliqa/tests/it/main.rs | 4 +- zilliqa/tests/it/persistence.rs | 55 +-------------- 12 files changed, 136 insertions(+), 73 deletions(-) diff --git a/z2/resources/chain-specs/zq2-protomainnet.toml b/z2/resources/chain-specs/zq2-protomainnet.toml index 3369496bb..8e3deeafb 100644 --- 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" -] \ No newline at end of file +] +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 index 5c43af9cd..4331a38d5 100644 --- 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" -] \ No newline at end of file +] +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 index f3416d288..fff82e15c 100644 --- 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 index 778d23a93..239a01fc7 100644 --- 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 { + 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 index a2b46ca9b..77f2e9b93 100644 --- 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::(f)?.to_string())) + .collect::>>()?, + ); 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 index 2a4bc208a..d22c8fd87 100644 --- 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 index b93631942..d702d8835 100644 --- 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", into = "Vec")] +pub struct Forks(Vec); + +impl TryFrom> for Forks { + type Error = anyhow::Error; + + fn try_from(mut forks: Vec) -> Result { + // 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 for Vec { + 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 index 77952a96f..ab09823f2 100644 --- 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 index cb96c6a48..5c6f0bb37 100644 --- a/zilliqa/src/precompiles/scilla.rs +++ b/zilliqa/src/precompiles/scilla.rs @@ -350,22 +350,9 @@ pub fn scilla_call_handle_register( // 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( 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 index 8dd5d63ef..e8b5201ef 100644 --- 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
, pub chain_id: ChainId, + pub forks: Forks, pub block_store: Arc, } @@ -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 index 51abde9d0..7dc951ea1 100644 --- 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 index bc3ef200d..b4230308a 100644 --- 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,