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

Ensure state is correct in both EVM and Scilla environments #2414

Open
wants to merge 1 commit 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
2 changes: 0 additions & 2 deletions config-example.toml
Original file line number Diff line number Diff line change
Expand Up @@ -58,5 +58,3 @@ consensus.minimum_stake = "10_000_000_000_000_000_000_000_000"
# Gas parameters
consensus.eth_block_gas_limit = 84000000
consensus.gas_price = "4_761_904_800_000"

consensus.genesis_fork = { at_height = 0, call_mode_1_sets_caller_to_parent_caller = false, failed_scilla_call_from_gas_exempt_caller_causes_revert = false, scilla_messages_can_call_evm_contracts = false, scilla_contract_creation_increments_account_balance = false, scilla_json_preserve_order = false }
2 changes: 0 additions & 2 deletions config-single-node.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,5 +28,3 @@ consensus.minimum_stake = "10_000_000_000_000_000_000_000_000"
consensus.eth_block_gas_limit = 84000000
consensus.gas_price = "4_761_904_800_000"
consensus.local_address = "host.docker.internal"

consensus.genesis_fork = { at_height = 0, call_mode_1_sets_caller_to_parent_caller = false, failed_scilla_call_from_gas_exempt_caller_causes_revert = false, scilla_messages_can_call_evm_contracts = false, scilla_contract_creation_increments_account_balance = true, scilla_json_preserve_order = true }
2 changes: 1 addition & 1 deletion z2/resources/chain-specs/zq2-protomainnet.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ consensus.scilla_call_gas_exempt_addrs = ["0x95347b860Bd49818AFAccCA8403C55C23e7
consensus.contract_upgrade_block_heights = { deposit_v3 = 5342400, deposit_v4 = 7966800 }

api_servers = [{ port = 4201, enabled_apis = [{ namespace = "eth", apis = ["blockNumber"] }] }, { port = 4202, enabled_apis = ["admin", "debug", "erigon", "eth", "net", "ots", "trace", "txpool", "web3", "zilliqa"] }]
consensus.genesis_fork = { at_height = 0, call_mode_1_sets_caller_to_parent_caller = false, failed_scilla_call_from_gas_exempt_caller_causes_revert = false, scilla_messages_can_call_evm_contracts = false, scilla_contract_creation_increments_account_balance = false, scilla_json_preserve_order = false }
consensus.genesis_fork = { at_height = 0, call_mode_1_sets_caller_to_parent_caller = false, failed_scilla_call_from_gas_exempt_caller_causes_revert = false, scilla_messages_can_call_evm_contracts = false, scilla_contract_creation_increments_account_balance = false, scilla_json_preserve_order = false, scilla_call_respects_evm_state_changes = false }
consensus.forks = [
{ at_height = 5342400, failed_scilla_call_from_gas_exempt_caller_causes_revert = true, call_mode_1_sets_caller_to_parent_caller = true },
{ at_height = 7685881, scilla_json_preserve_order = true },
Expand Down
3 changes: 2 additions & 1 deletion z2/resources/chain-specs/zq2-prototestnet.toml
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,10 @@ consensus.scilla_call_gas_exempt_addrs = ["0x60E6b5b1B8D3E373E1C04dC0b4f5624776b
consensus.contract_upgrade_block_heights = { deposit_v3 = 8406000, deposit_v4 = 10890000 }

api_servers = [{ port = 4201, enabled_apis = [{ namespace = "eth", apis = ["blockNumber"] }] }, { port = 4202, enabled_apis = ["admin", "debug", "erigon", "eth", "net", "ots", "trace", "txpool", "web3", "zilliqa"] }]
consensus.genesis_fork = { at_height = 0, call_mode_1_sets_caller_to_parent_caller = false, failed_scilla_call_from_gas_exempt_caller_causes_revert = false, scilla_messages_can_call_evm_contracts = false, scilla_contract_creation_increments_account_balance = false, scilla_json_preserve_order = false }
consensus.genesis_fork = { at_height = 0, call_mode_1_sets_caller_to_parent_caller = false, failed_scilla_call_from_gas_exempt_caller_causes_revert = false, scilla_messages_can_call_evm_contracts = false, scilla_contract_creation_increments_account_balance = false, scilla_json_preserve_order = false, scilla_call_respects_evm_state_changes = false }
consensus.forks = [
{ at_height = 8404000, failed_scilla_call_from_gas_exempt_caller_causes_revert = true, call_mode_1_sets_caller_to_parent_caller = true },
{ at_height = 10200000, scilla_messages_can_call_evm_contracts = true },
{ at_height = 11152000, scilla_contract_creation_increments_account_balance = true, scilla_json_preserve_order = true },
{ at_height = 12556800, scilla_call_respects_evm_state_changes = true },
]
5 changes: 4 additions & 1 deletion z2/src/chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,8 @@ impl Chain {
"failed_scilla_call_from_gas_exempt_caller_causes_revert": false,
"scilla_messages_can_call_evm_contracts": false,
"scilla_contract_creation_increments_account_balance": false,
"scilla_json_preserve_order": false
"scilla_json_preserve_order": false,
"scilla_call_respects_evm_state_changes": false,
})),
_ => None,
}
Expand All @@ -207,6 +208,8 @@ impl Chain {
json!({ "at_height": 10200000, "scilla_messages_can_call_evm_contracts": true }),
// estimated: 2025-02-12T12:08:37Z
json!({ "at_height": 11152000, "scilla_contract_creation_increments_account_balance": true, "scilla_json_preserve_order": true }),
// estimated: 2025-03-04T09:22:45Z
json!({ "at_height": 12556800, "scilla_call_respects_evm_state_changes": true }),
]),
Chain::Zq2ProtoMainnet => Some(vec![
// estimated: 2024-12-20T23:33:12Z
Expand Down
12 changes: 0 additions & 12 deletions zilliqa/benches/it.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,12 +68,6 @@ fn process_empty(c: &mut Criterion) {
consensus.minimum_stake = "1"
consensus.eth_block_gas_limit = 1000000000
consensus.gas_price = "1"
consensus.genesis_fork.at_height = 0
consensus.genesis_fork.failed_scilla_call_from_gas_exempt_caller_causes_revert = true
consensus.genesis_fork.call_mode_1_sets_caller_to_parent_caller = true
consensus.genesis_fork.scilla_messages_can_call_evm_contracts = true
consensus.genesis_fork.scilla_contract_creation_increments_account_balance = true
consensus.genesis_fork.scilla_json_preserve_order = true
consensus.genesis_accounts = [
[
"0x0000000000000000000000000000000000000000",
Expand Down Expand Up @@ -194,12 +188,6 @@ fn consensus(
consensus.minimum_stake = "1"
consensus.eth_block_gas_limit = 84000000
consensus.gas_price = "1"
consensus.genesis_fork.at_height = 0
consensus.genesis_fork.failed_scilla_call_from_gas_exempt_caller_causes_revert = true
consensus.genesis_fork.call_mode_1_sets_caller_to_parent_caller = true
consensus.genesis_fork.scilla_messages_can_call_evm_contracts = true
consensus.genesis_fork.scilla_contract_creation_increments_account_balance = true
consensus.genesis_fork.scilla_json_preserve_order = true
"#,
)
.unwrap();
Expand Down
21 changes: 20 additions & 1 deletion zilliqa/src/cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -497,6 +497,7 @@ pub struct Fork {
pub scilla_messages_can_call_evm_contracts: bool,
pub scilla_contract_creation_increments_account_balance: bool,
pub scilla_json_preserve_order: bool,
pub scilla_call_respects_evm_state_changes: bool,
}

#[derive(Copy, Clone, Debug, Serialize, Deserialize)]
Expand Down Expand Up @@ -524,8 +525,14 @@ pub struct ForkDelta {
/// the contract balance will be sum of the existing balance and the amount sent in the deployment transaction.
/// If false, the contract balance will be the amount sent in the deployment transaction.
pub scilla_contract_creation_increments_account_balance: Option<bool>,
// If true then the compiled code is using serde_json with feature "preserve_order"
/// If true, JSON maps that are passed to Scilla will be in their original order. If false, the entries will be
/// sorted by their keys.
pub scilla_json_preserve_order: Option<bool>,
/// If true, interop calls to the `scilla_call` precompile will correctly see state changes already made by the EVM
/// before that point in the transaction's execution. Also both Scilla and EVM will be able to update the same
/// accounts without state changes being lost. If false, state changes can be lost if Scilla and EVM attempt to
/// update the same account. This can sometimes lead to mined transactions which don't increase the caller's nonce.
pub scilla_call_respects_evm_state_changes: Option<bool>,
}

impl Fork {
Expand All @@ -547,6 +554,9 @@ impl Fork {
scilla_json_preserve_order: delta
.scilla_json_preserve_order
.unwrap_or(self.scilla_json_preserve_order),
scilla_call_respects_evm_state_changes: delta
.scilla_call_respects_evm_state_changes
.unwrap_or(self.scilla_call_respects_evm_state_changes),
}
}
}
Expand Down Expand Up @@ -620,6 +630,7 @@ pub fn genesis_fork_default() -> Fork {
scilla_messages_can_call_evm_contracts: true,
scilla_contract_creation_increments_account_balance: true,
scilla_json_preserve_order: true,
scilla_call_respects_evm_state_changes: true,
}
}

Expand Down Expand Up @@ -700,6 +711,7 @@ mod tests {
scilla_messages_can_call_evm_contracts: None,
scilla_contract_creation_increments_account_balance: Some(false),
scilla_json_preserve_order: None,
scilla_call_respects_evm_state_changes: None,
}],
..Default::default()
};
Expand Down Expand Up @@ -728,6 +740,7 @@ mod tests {
scilla_messages_can_call_evm_contracts: Some(true),
scilla_contract_creation_increments_account_balance: None,
scilla_json_preserve_order: Some(true),
scilla_call_respects_evm_state_changes: None,
},
ForkDelta {
at_height: 20,
Expand All @@ -736,6 +749,7 @@ mod tests {
scilla_messages_can_call_evm_contracts: Some(false),
scilla_contract_creation_increments_account_balance: Some(true),
scilla_json_preserve_order: Some(true),
scilla_call_respects_evm_state_changes: None,
},
],
..Default::default()
Expand Down Expand Up @@ -778,6 +792,7 @@ mod tests {
scilla_messages_can_call_evm_contracts: None,
scilla_contract_creation_increments_account_balance: None,
scilla_json_preserve_order: None,
scilla_call_respects_evm_state_changes: None,
},
ForkDelta {
at_height: 10,
Expand All @@ -786,6 +801,7 @@ mod tests {
scilla_messages_can_call_evm_contracts: None,
scilla_contract_creation_increments_account_balance: None,
scilla_json_preserve_order: None,
scilla_call_respects_evm_state_changes: None,
},
],
..Default::default()
Expand Down Expand Up @@ -819,6 +835,7 @@ mod tests {
scilla_messages_can_call_evm_contracts: true,
scilla_contract_creation_increments_account_balance: true,
scilla_json_preserve_order: true,
scilla_call_respects_evm_state_changes: true,
},
forks: vec![],
..Default::default()
Expand All @@ -840,6 +857,7 @@ mod tests {
scilla_messages_can_call_evm_contracts: None,
scilla_contract_creation_increments_account_balance: None,
scilla_json_preserve_order: None,
scilla_call_respects_evm_state_changes: None,
},
ForkDelta {
at_height: 20,
Expand All @@ -848,6 +866,7 @@ mod tests {
scilla_messages_can_call_evm_contracts: None,
scilla_contract_creation_increments_account_balance: None,
scilla_json_preserve_order: None,
scilla_call_respects_evm_state_changes: None,
},
],
..Default::default()
Expand Down
74 changes: 66 additions & 8 deletions zilliqa/src/exec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use revm::{
AccountInfo, BlockEnv, Bytecode, Env, ExecutionResult, HaltReason, HandlerCfg, Output,
ResultAndState, SpecId, TxEnv, B256, KECCAK_EMPTY,
},
Database, DatabaseRef, Evm, GetInspector, Inspector,
Database, DatabaseRef, Evm, GetInspector, Inspector, JournaledState,
};
use serde::{Deserialize, Serialize};
use serde_json::{json, Value};
Expand Down Expand Up @@ -759,6 +759,9 @@ impl State {
continue;
}

// We shouldn't mutate accounts that were from EVM.
assert!(!account.from_evm);

let mut storage = self.get_account_trie(address)?;

/// Recursively called internal function which assigns `value` at the correct key to `storage`.
Expand Down Expand Up @@ -1179,6 +1182,9 @@ pub fn zil_contract_address(sender: Address, nonce: u64) -> Address {
pub struct PendingState {
pub pre_state: State,
pub new_state: HashMap<Address, PendingAccount>,
// Read-only copy of the current cached EVM state. Only `Some` when this Scilla call is made by the `scilla_call`
// precompile.
pub evm_state: Option<JournaledState>,
}

/// Private helper function for `PendingState::load_account`. The only difference is that the fields of `PendingState`
Expand All @@ -1187,11 +1193,40 @@ pub struct PendingState {
fn load_account<'a>(
pre_state: &State,
new_state: &'a mut HashMap<Address, PendingAccount>,
evm_state: &Option<JournaledState>,
address: Address,
) -> Result<&'a mut PendingAccount> {
match new_state.entry(address) {
Entry::Occupied(entry) => Ok(entry.into_mut()),
Entry::Vacant(vac) => {
match (
new_state.entry(address),
evm_state.as_ref().and_then(|s| s.state.get(&address)),
) {
(Entry::Occupied(entry), _) => Ok(entry.into_mut()),
(Entry::Vacant(vac), Some(account)) => {
let account = Account {
nonce: account.info.nonce,
balance: account.info.balance.to(),
code: Code::Evm(if account.info.code_hash == KECCAK_EMPTY {
vec![]
} else {
account
.info
.code
.as_ref()
.ok_or_else(|| anyhow!("account should have code"))?
.original_bytes()
.to_vec()
}),
storage_root: B256::ZERO, // There's no need to set this, since Scilla cannot query EVM contracts' state.
};
let account = PendingAccount {
account,
storage: BTreeMap::new(),
from_evm: true,
touched: false,
};
Ok(vac.insert(account))
}
(Entry::Vacant(vac), None) => {
let account = pre_state.get_account(address)?;
Ok(vac.insert(account.into()))
}
Expand All @@ -1203,6 +1238,7 @@ impl PendingState {
PendingState {
pre_state: state,
new_state: HashMap::new(),
evm_state: None,
}
}

Expand All @@ -1225,7 +1261,12 @@ impl PendingState {
}

pub fn load_account(&mut self, address: Address) -> Result<&mut PendingAccount> {
load_account(&self.pre_state, &mut self.new_state, address)
load_account(
&self.pre_state,
&mut self.new_state,
&self.evm_state,
address,
)
}

pub fn load_var_info(&mut self, address: Address, variable: &str) -> Result<(&str, u8)> {
Expand All @@ -1248,7 +1289,12 @@ impl PendingState {
indices: &[Vec<u8>],
value: StorageValue,
) -> Result<()> {
let account = load_account(&self.pre_state, &mut self.new_state, address)?;
let account = load_account(
&self.pre_state,
&mut self.new_state,
&self.evm_state,
address,
)?;

let mut current = account
.storage
Expand Down Expand Up @@ -1283,7 +1329,12 @@ impl PendingState {
var_name: &str,
indices: &[Vec<u8>],
) -> Result<&mut Option<Bytes>> {
let account = load_account(&self.pre_state, &mut self.new_state, address)?;
let account = load_account(
&self.pre_state,
&mut self.new_state,
&self.evm_state,
address,
)?;

fn get_cached<'a>(
storage: &'a mut BTreeMap<String, StorageValue>,
Expand Down Expand Up @@ -1340,7 +1391,12 @@ impl PendingState {
var_name: &str,
indices: &[Vec<u8>],
) -> Result<BTreeMap<Vec<u8>, StorageValue>> {
let account = load_account(&self.pre_state, &mut self.new_state, address)?;
let account = load_account(
&self.pre_state,
&mut self.new_state,
&self.evm_state,
address,
)?;

// Even if we have something cached for this prefix, we don't know if it is a full representation of the map.
// It might have been the case that only a few subfields were cached. Therefore, we need to retrieve the full
Expand Down Expand Up @@ -1452,6 +1508,7 @@ pub struct PendingAccount {
pub account: Account,
/// Cached values of updated or deleted storage. Note that deletions can happen at any level of a map.
pub storage: BTreeMap<String, StorageValue>,
pub from_evm: bool,
pub touched: bool,
}

Expand Down Expand Up @@ -1492,6 +1549,7 @@ impl From<Account> for PendingAccount {
PendingAccount {
account,
storage: BTreeMap::new(),
from_evm: false,
touched: false,
}
}
Expand Down
31 changes: 27 additions & 4 deletions zilliqa/src/precompiles/scilla.rs
Original file line number Diff line number Diff line change
Expand Up @@ -584,9 +584,13 @@ fn scilla_call_precompile<I: ScillaInspector>(

let empty_state = PendingState::new(evmctx.db.pre_state.clone());
// Temporarily move the `PendingState` out of `evmctx`, replacing it with an empty state.
let state = std::mem::replace(&mut evmctx.db, empty_state);
let mut state = std::mem::replace(&mut evmctx.db, empty_state);
let depth = evmctx.journaled_state.depth;
if external_context.fork.scilla_call_respects_evm_state_changes {
state.evm_state = Some(evmctx.journaled_state.clone());
}
let scilla = evmctx.db.pre_state.scilla();
let Ok((result, state)) = scilla_call(
let Ok((result, mut state)) = scilla_call(
state,
scilla,
evmctx.env.tx.caller,
Expand All @@ -596,7 +600,7 @@ fn scilla_call_precompile<I: ScillaInspector>(
.call_mode_1_sets_caller_to_parent_caller
{
// Use the caller of the parent call-stack.
external_context.callers[evmctx.journaled_state.depth - 1]
external_context.callers[depth - 1]
} else {
// Use the original transaction signer.
evmctx.env.tx.caller
Expand All @@ -622,6 +626,7 @@ fn scilla_call_precompile<I: ScillaInspector>(
};
trace!(?result, "scilla_call complete");
if !&result.success {
evmctx.db = state;
if result.errors.values().any(|errs| {
errs.iter()
.any(|err| matches!(err, ScillaError::GasNotSufficient))
Expand All @@ -631,7 +636,25 @@ fn scilla_call_precompile<I: ScillaInspector>(
return err("scilla call failed");
}
}
// Move the new state back into `evmctx`.
state.new_state.retain(|address, account| {
if !account.touched {
return true;
}
if !account.from_evm {
return true;
}

// Apply changes made to EVM accounts back to the EVM `JournaledState`.
let before = evmctx.journaled_state.state.get_mut(address).unwrap();

// The only thing that Scilla is able to update is the balance.
if before.info.balance.to::<u128>() != account.account.balance {
before.info.balance = account.account.balance.try_into().unwrap();
before.mark_touch();
}

false
});
evmctx.db = state;

for log in result.logs {
Expand Down
Loading
Loading