Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions z2/resources/chain-specs/zq2-protomainnet.toml
Original file line number Diff line number Diff line change
Expand Up @@ -35,3 +35,7 @@ consensus.scilla_call_gas_exempt_addrs = [
"0x20Dd5D5B5d4C72676514A0eA1052d0200003d69D",
"0xbfDe2156aF75a29d36614bC1F8005DD816Bd9200"
]
consensus.forks = [
{ at_height = 0, failed_scilla_call_from_gas_exempt_caller_causes_revert = false },
{ at_height = 5352000, failed_scilla_call_from_gas_exempt_caller_causes_revert = true },
]
4 changes: 4 additions & 0 deletions z2/resources/chain-specs/zq2-prototestnet.toml
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,7 @@ consensus.scilla_call_gas_exempt_addrs = [
"0x453b11386FBd54bC532892c0217BBc316fc7b918",
"0xaD581eC62eA08831c8FE2Cd7A1113473fE40A057"
]
consensus.forks = [
{ at_height = 0, failed_scilla_call_from_gas_exempt_caller_causes_revert = false },
{ at_height = 8381000, failed_scilla_call_from_gas_exempt_caller_causes_revert = true },
]
2 changes: 2 additions & 0 deletions z2/resources/config.tera.toml
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,5 @@ enable_ots_indices = true
file = "{{ checkpoint_file }}"
hash = "{{ checkpoint_hash }}"
{%- endif %}

consensus.forks = {{ forks }}
17 changes: 17 additions & 0 deletions z2/src/chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -151,6 +152,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-17T12:06:47Z
json!({ "at_height": 8381000, "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-19T11:56:39Z
json!({ "at_height": 5352000, "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");

Expand Down
16 changes: 13 additions & 3 deletions z2/src/chain/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -547,10 +547,20 @@ impl ChainNode {
"whitelisted_evm_contract_addresses",
&whitelisted_evm_contract_addresses,
);
// convert json to toml formatting
let toml_servers: toml::Value = serde_json::from_value(api_servers)?;
ctx.insert("api_servers", &toml_servers.to_string());
ctx.insert(
"api_servers",
&serde_json::from_value::<toml::Value>(api_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 {
Expand Down
3 changes: 2 additions & 1 deletion z2/src/setup.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand Down Expand Up @@ -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(),
Expand Down
67 changes: 67 additions & 0 deletions zilliqa/src/cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -357,6 +357,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 {
Expand All @@ -383,10 +387,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 {
Expand Down
4 changes: 3 additions & 1 deletion zilliqa/src/exec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
};
Expand Down
31 changes: 17 additions & 14 deletions zilliqa/src/precompiles/scilla.rs
Original file line number Diff line number Diff line change
Expand Up @@ -345,22 +345,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(),
Expand Down Expand Up @@ -395,6 +382,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(),
Expand Down
5 changes: 4 additions & 1 deletion zilliqa/src/state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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>,
}

Expand All @@ -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,
}
}
Expand Down Expand Up @@ -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(),
}
}

Expand Down
4 changes: 3 additions & 1 deletion zilliqa/tests/it/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,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_cache_size_default, state_rpc_limit_default, total_native_token_supply_default,
Amount, ApiServer, Checkpoint, ConsensusConfig, ContractUpgradesBlockHeights,
Amount, ApiServer, Checkpoint, ConsensusConfig, ContractUpgradesBlockHeights, Forks,
GenesisDeposit, NodeConfig,
},
crypto::{SecretKey, TransactionPublicKey},
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -490,6 +491,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(),
Expand Down
54 changes: 2 additions & 52 deletions zilliqa/tests/it/persistence.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
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::{
Expand Down Expand Up @@ -92,49 +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,
};
let mut rng = network.rng.lock().unwrap();
let result = crate::node(
config,
Expand Down
Loading