From a4bdc245f55ef161f99f8243a5f4941b3f5d988b Mon Sep 17 00:00:00 2001 From: Michael Zaikin Date: Wed, 31 Jul 2024 18:25:45 +0100 Subject: [PATCH] EVM/Kernel: make withdrawal counter revertable (via system account state extension) --- etherlink/CHANGES_KERNEL.md | 1 + .../evm_execution/src/fa_bridge/mod.rs | 16 ++-- .../evm_execution/src/fa_bridge/test_utils.rs | 25 +++++- .../evm_execution/src/fa_bridge/tests.rs | 19 ++-- .../src/fa_bridge/ticket_table.rs | 9 +- etherlink/kernel_evm/evm_execution/src/lib.rs | 1 + .../evm_execution/src/precompiles/mod.rs | 3 + .../src/precompiles/withdrawal.rs | 11 ++- .../kernel_evm/evm_execution/src/storage.rs | 58 ------------- .../evm_execution/src/withdrawal_counter.rs | 86 +++++++++++++++++++ 10 files changed, 136 insertions(+), 93 deletions(-) create mode 100644 etherlink/kernel_evm/evm_execution/src/withdrawal_counter.rs diff --git a/etherlink/CHANGES_KERNEL.md b/etherlink/CHANGES_KERNEL.md index ba63f1d2b319..774ddc24b668 100644 --- a/etherlink/CHANGES_KERNEL.md +++ b/etherlink/CHANGES_KERNEL.md @@ -64,6 +64,7 @@ repository, but is part of [`etherlink-mainnet-launch`][mainnet-branch] instead. (!13901) - Forced blueprints (in case of delayed inbox timeout) always use a timestamp equal or greater than the predecessor. (!13832) +- Withdrawal counter is now revertable. (!14389) ## Internal diff --git a/etherlink/kernel_evm/evm_execution/src/fa_bridge/mod.rs b/etherlink/kernel_evm/evm_execution/src/fa_bridge/mod.rs index 9d97d3f17c98..083030b87235 100644 --- a/etherlink/kernel_evm/evm_execution/src/fa_bridge/mod.rs +++ b/etherlink/kernel_evm/evm_execution/src/fa_bridge/mod.rs @@ -39,15 +39,15 @@ use host::runtime::Runtime; use primitive_types::{H160, U256}; use tezos_ethereum::block::BlockConstants; use tezos_evm_logging::{log, Level::Info}; -use ticket_table::{TicketTable, TICKET_TABLE_ACCOUNT}; +use ticket_table::TicketTable; use withdrawal::FaWithdrawal; use crate::{ account_storage::EthereumAccountStorage, handler::{CreateOutcome, EvmHandler, ExecutionOutcome}, - precompiles::{PrecompileBTreeMap, PrecompileOutcome}, - storage::withdraw_nonce, + precompiles::{PrecompileBTreeMap, PrecompileOutcome, SYSTEM_ACCOUNT_ADDRESS}, transaction::TransactionContext, + withdrawal_counter::WithdrawalCounter, EthereumError, }; @@ -219,7 +219,7 @@ fn inner_execute_deposit( deposit: &FaDeposit, ) -> Result { // Updating the ticket table in accordance with the ownership. - let mut system = handler.get_or_create_account(TICKET_TABLE_ACCOUNT)?; + let mut system = handler.get_or_create_account(SYSTEM_ACCOUNT_ADDRESS)?; if system.ticket_balance_add( handler.borrow_host(), @@ -251,7 +251,7 @@ fn inner_execute_withdrawal( withdrawal: &FaWithdrawal, ) -> Result { // Updating the ticket table in accordance with the ownership. - let mut system = handler.get_or_create_account(TICKET_TABLE_ACCOUNT)?; + let mut system = handler.get_or_create_account(SYSTEM_ACCOUNT_ADDRESS)?; if system.ticket_balance_remove( handler.borrow_host(), @@ -259,10 +259,8 @@ fn inner_execute_withdrawal( &withdrawal.ticket_owner, withdrawal.amount, )? { - // NOTE that the nonce will remain incremented even if the precompile call fails. - // That is fine, since we only care about its uniqueness and determinism. - let withdrawal_id = withdraw_nonce::get_and_increment(handler.borrow_host()) - .map_err(|e| EthereumError::WrappedError(Cow::from(format!("{:?}", e))))?; + let withdrawal_id = + system.withdrawal_counter_get_and_increment(handler.borrow_host())?; handler .add_log(withdrawal.event_log(withdrawal_id)) diff --git a/etherlink/kernel_evm/evm_execution/src/fa_bridge/test_utils.rs b/etherlink/kernel_evm/evm_execution/src/fa_bridge/test_utils.rs index 3cf044c4e2cf..edaeb0a52ce3 100644 --- a/etherlink/kernel_evm/evm_execution/src/fa_bridge/test_utils.rs +++ b/etherlink/kernel_evm/evm_execution/src/fa_bridge/test_utils.rs @@ -23,15 +23,16 @@ use tezos_smart_rollup_mock::MockHost; use crate::{ account_storage::{account_path, read_u256, EthereumAccountStorage}, handler::{EvmHandler, ExecutionOutcome}, - precompiles::{self, precompile_set}, + precompiles::{self, precompile_set, SYSTEM_ACCOUNT_ADDRESS}, run_transaction, utilities::keccak256_hash, + withdrawal_counter::WITHDRAWAL_COUNTER_PATH, }; use super::{ deposit::{ticket_hash, FaDeposit}, execute_fa_deposit, execute_fa_withdrawal, - ticket_table::{ticket_balance_path, TicketTable, TICKET_TABLE_ACCOUNT}, + ticket_table::{ticket_balance_path, TicketTable}, withdrawal::FaWithdrawal, }; @@ -178,7 +179,7 @@ pub fn ticket_balance_add( balance: U256, ) -> bool { let mut system = evm_account_storage - .get_or_create(host, &account_path(&TICKET_TABLE_ACCOUNT).unwrap()) + .get_or_create(host, &account_path(&SYSTEM_ACCOUNT_ADDRESS).unwrap()) .unwrap(); system .ticket_balance_add(host, ticket_hash, address, balance) @@ -193,7 +194,7 @@ pub fn ticket_balance_get( address: &H160, ) -> U256 { let system = evm_account_storage - .get(host, &account_path(&TICKET_TABLE_ACCOUNT).unwrap()) + .get(host, &account_path(&SYSTEM_ACCOUNT_ADDRESS).unwrap()) .unwrap() .unwrap(); @@ -203,6 +204,22 @@ pub fn ticket_balance_get( read_u256(host, &path, U256::zero()).unwrap() } +/// Get next withdrawal counter value +pub fn withdrawal_counter_next( + host: &impl Runtime, + evm_account_storage: &EthereumAccountStorage, +) -> Option { + let system = evm_account_storage + .get_or_create(host, &account_path(&SYSTEM_ACCOUNT_ADDRESS).unwrap()) + .unwrap(); + + let path = system.custom_path(&WITHDRAWAL_COUNTER_PATH).unwrap(); + match host.store_read_all(&path) { + Ok(bytes) => Some(U256::from_little_endian(&bytes)), + _ => None, + } +} + /// Provision TEZ balance for a specified account pub fn set_balance( host: &mut impl Runtime, diff --git a/etherlink/kernel_evm/evm_execution/src/fa_bridge/tests.rs b/etherlink/kernel_evm/evm_execution/src/fa_bridge/tests.rs index 2b165b334a38..4e0091ea8beb 100644 --- a/etherlink/kernel_evm/evm_execution/src/fa_bridge/tests.rs +++ b/etherlink/kernel_evm/evm_execution/src/fa_bridge/tests.rs @@ -14,10 +14,9 @@ use crate::{ convert_h160, convert_log, convert_u256, deploy_mock_wrapper, dummy_fa_deposit, dummy_fa_withdrawal, dummy_ticket, fa_bridge_precompile_call_withdraw, get_storage_flag, kernel_wrapper, run_fa_deposit, ticket_balance_add, - ticket_balance_get, token_wrapper, + ticket_balance_get, token_wrapper, withdrawal_counter_next, }, handler::ExtendedExitReason, - storage::withdraw_nonce, }; #[test] @@ -363,10 +362,10 @@ fn fa_withdrawal_executed_via_l2_proxy_contract() { assert_eq!(burn_event.sender, convert_h160(&withdrawal.sender)); assert_eq!(burn_event.amount, convert_u256(&withdrawal.amount)); - // Ensure withdrawal counter is updated + // Ensure withdrawal counter is incremented assert_eq!( - U256::one(), - withdraw_nonce::get_and_increment(&mut mock_runtime).unwrap() + Some(U256::one()), + withdrawal_counter_next(&mock_runtime, &evm_account_storage) ); } @@ -413,10 +412,10 @@ fn fa_withdrawal_fails_due_to_faulty_l2_proxy() { U256::MAX, )); - // Ensure withdrawal counter is updated + // Ensure withdrawal counter is reverted assert_eq!( - U256::one(), - withdraw_nonce::get_and_increment(&mut mock_runtime).unwrap() + None, + withdrawal_counter_next(&mock_runtime, &evm_account_storage) ); assert!( matches!(res.reason, ExtendedExitReason::Exit(ExitReason::Error(ExitError::Other(err))) if err.contains("Proxy contract does not exist")) @@ -447,8 +446,8 @@ fn fa_withdrawal_fails_due_to_insufficient_balance() { // Ensure withdrawal counter is not updated (returned before incrementing the nonce) assert_eq!( - U256::zero(), - withdraw_nonce::get_and_increment(&mut mock_runtime).unwrap() + None, + withdrawal_counter_next(&mock_runtime, &evm_account_storage) ); assert!( matches!(res.reason, ExtendedExitReason::Exit(ExitReason::Error(ExitError::Other(err))) if err.contains("Insufficient ticket balance")) diff --git a/etherlink/kernel_evm/evm_execution/src/fa_bridge/ticket_table.rs b/etherlink/kernel_evm/evm_execution/src/fa_bridge/ticket_table.rs index 9a0510f95ddb..86b6ff7af1bb 100644 --- a/etherlink/kernel_evm/evm_execution/src/fa_bridge/ticket_table.rs +++ b/etherlink/kernel_evm/evm_execution/src/fa_bridge/ticket_table.rs @@ -21,9 +21,6 @@ use crate::account_storage::{ /// Path where global ticket table is stored const TICKET_TABLE_PATH: RefPath = RefPath::assert_from(b"/ticket_table"); -/// Global ticket table belongs to the zero account (system) -pub const TICKET_TABLE_ACCOUNT: H160 = H160::zero(); - pub trait TicketTable { /// Increases ticket balance fn ticket_balance_add( @@ -95,7 +92,7 @@ mod tests { use tezos_smart_rollup_host::path::RefPath; use tezos_smart_rollup_mock::MockHost; - use crate::account_storage::read_u256; + use crate::{account_storage::read_u256, precompiles::SYSTEM_ACCOUNT_ADDRESS}; use super::*; @@ -103,7 +100,7 @@ mod tests { fn ticket_table_balance_add_succeeds() { let mut host = MockHost::default(); - let mut account = EthereumAccount::from_address(&H160([0u8; 20])).unwrap(); + let mut account = EthereumAccount::from_address(&SYSTEM_ACCOUNT_ADDRESS).unwrap(); let ticket_hash: H256 = H256([1u8; 32]); let address = H160([2u8; 20]); @@ -129,7 +126,7 @@ mod tests { fn ticket_table_balance_add_overflows() { let mut host = MockHost::default(); - let mut account = EthereumAccount::from_address(&H160([0u8; 20])).unwrap(); + let mut account = EthereumAccount::from_address(&SYSTEM_ACCOUNT_ADDRESS).unwrap(); let ticket_hash: H256 = H256([1u8; 32]); let address = H160([2u8; 20]); diff --git a/etherlink/kernel_evm/evm_execution/src/lib.rs b/etherlink/kernel_evm/evm_execution/src/lib.rs index 7b20f4f00fd8..ecce9cbb6523 100644 --- a/etherlink/kernel_evm/evm_execution/src/lib.rs +++ b/etherlink/kernel_evm/evm_execution/src/lib.rs @@ -34,6 +34,7 @@ pub mod trace; pub mod transaction; pub mod transaction_layer_data; pub mod utilities; +pub mod withdrawal_counter; pub use evm::Config; diff --git a/etherlink/kernel_evm/evm_execution/src/precompiles/mod.rs b/etherlink/kernel_evm/evm_execution/src/precompiles/mod.rs index 3c0b33bf8314..76a9a7f27927 100644 --- a/etherlink/kernel_evm/evm_execution/src/precompiles/mod.rs +++ b/etherlink/kernel_evm/evm_execution/src/precompiles/mod.rs @@ -42,6 +42,9 @@ pub const FA_BRIDGE_PRECOMPILE_ADDRESS: H160 = H160([ 0xff, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 2, ]); +// System (zero) account address, owns ticket table and withdrawal counter +pub const SYSTEM_ACCOUNT_ADDRESS: H160 = H160::zero(); + /// Outcome of executing a precompiled contract. Covers both successful /// return, stop and revert and additionally, it covers contract execution /// failures (malformed input etc.). This is encoded using the `ExitReason` diff --git a/etherlink/kernel_evm/evm_execution/src/precompiles/withdrawal.rs b/etherlink/kernel_evm/evm_execution/src/precompiles/withdrawal.rs index cd9fe46dcc66..0471996ac640 100644 --- a/etherlink/kernel_evm/evm_execution/src/precompiles/withdrawal.rs +++ b/etherlink/kernel_evm/evm_execution/src/precompiles/withdrawal.rs @@ -13,9 +13,9 @@ use crate::handler::RouterInterface; use crate::handler::Withdrawal; use crate::precompiles::tick_model; use crate::precompiles::PrecompileOutcome; -use crate::precompiles::WITHDRAWAL_ADDRESS; +use crate::precompiles::{SYSTEM_ACCOUNT_ADDRESS, WITHDRAWAL_ADDRESS}; use crate::read_ticketer; -use crate::storage::withdraw_nonce; +use crate::withdrawal_counter::WithdrawalCounter; use crate::{abi, fail_if_too_much, EthereumError}; use evm::{Context, ExitReason, ExitRevert, ExitSucceed, Transfer}; use host::runtime::Runtime; @@ -188,10 +188,9 @@ pub fn withdrawal_precompile( return Ok(revert_withdrawal()); }; - let withdrawal_id = withdraw_nonce::get_and_increment(handler.borrow_host()) - .map_err(|e| { - EthereumError::WrappedError(Cow::from(format!("{:?}", e))) - })?; + let mut system = handler.get_or_create_account(SYSTEM_ACCOUNT_ADDRESS)?; + let withdrawal_id = + system.withdrawal_counter_get_and_increment(handler.borrow_host())?; // We use the original amount in order not to lose additional information let withdrawal_event = diff --git a/etherlink/kernel_evm/evm_execution/src/storage.rs b/etherlink/kernel_evm/evm_execution/src/storage.rs index 047c139d60ee..9e78d7444476 100644 --- a/etherlink/kernel_evm/evm_execution/src/storage.rs +++ b/etherlink/kernel_evm/evm_execution/src/storage.rs @@ -235,61 +235,3 @@ pub mod blocks { } } } - -// API to interact with the withdraw nonce storage -pub mod withdraw_nonce { - use host::{path::RefPath, runtime::Runtime}; - use primitive_types::U256; - - use crate::account_storage::{read_u256, write_u256, AccountStorageError}; - - pub const WITHDRAW_NONCE_PATH: RefPath = - RefPath::assert_from(b"/evm/world_state/withdraw_nonce"); - - /// All errors that may happen as result of using this storage interface. - #[derive(thiserror::Error, Debug, PartialEq)] - pub enum WithdrawNonceStorageError { - #[error("Runtime error: {0:?}")] - RuntimeError(#[from] host::runtime::RuntimeError), - #[error("Nonce overflow")] - NonceOverflow, - #[error("Failed to read: {0:?}")] - StorageError(#[from] AccountStorageError), - } - - /// Returns current nonce from the storage (or 0 if it's not initialized) - /// and increments & store the new value (will fail in case of overflow). - pub fn get_and_increment( - host: &mut impl Runtime, - ) -> Result { - let old_value = read_u256(host, &WITHDRAW_NONCE_PATH, U256::zero())?; - let new_value = old_value - .checked_add(U256::one()) - .ok_or(WithdrawNonceStorageError::NonceOverflow)?; - - write_u256(host, &WITHDRAW_NONCE_PATH, new_value)?; - Ok(old_value) - } - - #[cfg(test)] - mod tests { - use host::runtime::{Runtime, ValueType}; - use primitive_types::U256; - use tezos_smart_rollup_mock::MockHost; - - use crate::storage::withdraw_nonce::WITHDRAW_NONCE_PATH; - - use super::get_and_increment; - - #[test] - fn withdraw_nonce_initializes_and_increments() { - let mut mock_host = MockHost::default(); - assert_eq!(U256::zero(), get_and_increment(&mut mock_host).unwrap()); - assert_eq!( - mock_host.store_has(&WITHDRAW_NONCE_PATH).unwrap(), - Some(ValueType::Value) - ); - assert_eq!(U256::one(), get_and_increment(&mut mock_host).unwrap()); - } - } -} diff --git a/etherlink/kernel_evm/evm_execution/src/withdrawal_counter.rs b/etherlink/kernel_evm/evm_execution/src/withdrawal_counter.rs new file mode 100644 index 000000000000..bacac2c6dd46 --- /dev/null +++ b/etherlink/kernel_evm/evm_execution/src/withdrawal_counter.rs @@ -0,0 +1,86 @@ +// SPDX-FileCopyrightText: 2024 TriliTech +// +// SPDX-License-Identifier: MIT + +//! Withdrawal counter tracks successful XTZ/FA withdrawals, +//! in other words - absolute outbox messages identifiers. +//! It is not related to actual outbox message IDs and indended for offchain +//! usage only (in particular for indexing purposes). +//! +//! Implemented as an EVM account state extension to enable +//! revertable updates (if withdrawal fails the counter won't increment). + +use primitive_types::U256; +use tezos_smart_rollup_host::{path::RefPath, runtime::Runtime}; + +use crate::account_storage::{ + read_u256, write_u256, AccountStorageError, EthereumAccount, +}; + +/// Path where withdrawal counter is stored (relative to account) +pub const WITHDRAWAL_COUNTER_PATH: RefPath = RefPath::assert_from(b"/withdrawal_counter"); + +pub trait WithdrawalCounter { + /// Returns current withdrawal ID from the storage (or 0 if it's not initialized) + /// and increments & store the new value (will fail in case of overflow). + fn withdrawal_counter_get_and_increment( + &mut self, + host: &mut impl Runtime, + ) -> Result; +} + +impl WithdrawalCounter for EthereumAccount { + fn withdrawal_counter_get_and_increment( + &mut self, + host: &mut impl Runtime, + ) -> Result { + let path = self.custom_path(&WITHDRAWAL_COUNTER_PATH)?; + let old_value = read_u256(host, &path, U256::zero())?; + let new_value = old_value + .checked_add(U256::one()) + .ok_or(AccountStorageError::NonceOverflow)?; + + write_u256(host, &path, new_value)?; + Ok(old_value) + } +} + +#[cfg(test)] +mod tests { + use primitive_types::U256; + use tezos_smart_rollup_host::path::RefPath; + use tezos_smart_rollup_mock::MockHost; + + use crate::{ + account_storage::{read_u256, EthereumAccount}, + precompiles::SYSTEM_ACCOUNT_ADDRESS, + }; + + use super::WithdrawalCounter; + + #[test] + fn withdrawal_counter_initializes_and_increments() { + let mut mock_host = MockHost::default(); + + let mut account = EthereumAccount::from_address(&SYSTEM_ACCOUNT_ADDRESS).unwrap(); + let path = b"/evm/world_state/eth_accounts/0000000000000000000000000000000000000000/withdrawal_counter"; + + let id = account + .withdrawal_counter_get_and_increment(&mut mock_host) + .unwrap(); + assert_eq!(U256::zero(), id); + + let next_id = + read_u256(&mock_host, &RefPath::assert_from(path), U256::zero()).unwrap(); + assert_eq!(U256::one(), next_id); + + let id = account + .withdrawal_counter_get_and_increment(&mut mock_host) + .unwrap(); + assert_eq!(U256::one(), id); + + let next_id = + read_u256(&mock_host, &RefPath::assert_from(path), U256::zero()).unwrap(); + assert_eq!(U256::from(2), next_id); + } +} -- GitLab