From b2ec5116265967d480d713e00c9efda2301e8b71 Mon Sep 17 00:00:00 2001 From: Andrei Marinica Date: Sun, 25 Jun 2023 23:43:01 +0300 Subject: [PATCH 1/4] vm - remove panics - subtract egld --- .../src/whitebox/contract_obj_wrapper.rs | 4 +++- vm/src/tx_execution/exec_call.rs | 10 ++++++++-- vm/src/tx_execution/exec_general_tx.rs | 17 +++++++++++++---- vm/src/tx_mock/tx_cache_balance_util.rs | 12 +++++++----- 4 files changed, 31 insertions(+), 12 deletions(-) diff --git a/framework/scenario/src/whitebox/contract_obj_wrapper.rs b/framework/scenario/src/whitebox/contract_obj_wrapper.rs index e9d75c0f1d..c0d2cdf0f5 100644 --- a/framework/scenario/src/whitebox/contract_obj_wrapper.rs +++ b/framework/scenario/src/whitebox/contract_obj_wrapper.rs @@ -717,7 +717,9 @@ impl BlockchainStateWrapper { let rust_zero = num_bigint::BigUint::zero(); if egld_payment > &rust_zero { - tx_cache.subtract_egld_balance(caller, egld_payment); + if let Err(err) = tx_cache.subtract_egld_balance(caller, egld_payment) { + return TxResult::from_panic_obj(&err); + } tx_cache.increase_egld_balance(sc_address, egld_payment); } diff --git a/vm/src/tx_execution/exec_call.rs b/vm/src/tx_execution/exec_call.rs index 07bede4188..65dbc9012b 100644 --- a/vm/src/tx_execution/exec_call.rs +++ b/vm/src/tx_execution/exec_call.rs @@ -57,7 +57,10 @@ pub fn execute_async_call_and_callback( } else { let state_rc = Rc::new(state); let tx_cache = TxCache::new(state_rc.clone()); - tx_cache.subtract_egld_balance(&async_data.from, &async_data.call_value); + if let Err(err) = tx_cache.subtract_egld_balance(&async_data.from, &async_data.call_value) { + let state = Rc::try_unwrap(state_rc).unwrap(); + return (TxResult::from_panic_obj(&err), TxResult::empty(), state); + } tx_cache.insert_account(AccountData { address: async_data.to.clone(), nonce: 0, @@ -136,7 +139,10 @@ pub fn execute_promise_call_and_callback( } else { let state_rc = Rc::new(state); let tx_cache = TxCache::new(state_rc.clone()); - tx_cache.subtract_egld_balance(address, &promise.call.call_value); + if let Err(err) = tx_cache.subtract_egld_balance(address, &promise.call.call_value) { + let state = Rc::try_unwrap(state_rc).unwrap(); + return (TxResult::from_panic_obj(&err), TxResult::empty(), state); + } tx_cache.insert_account(AccountData { address: promise.call.to.clone(), nonce: 0, diff --git a/vm/src/tx_execution/exec_general_tx.rs b/vm/src/tx_execution/exec_general_tx.rs index d33651ecf7..5d5a2aa98e 100644 --- a/vm/src/tx_execution/exec_general_tx.rs +++ b/vm/src/tx_execution/exec_general_tx.rs @@ -10,10 +10,12 @@ use super::execute_tx_context; pub fn default_execution(tx_input: TxInput, tx_cache: TxCache) -> (TxResult, BlockchainUpdate) { let mut tx_context = TxContext::new(tx_input, tx_cache); - tx_context.tx_cache.subtract_egld_balance( + if let Err(err) = tx_context.tx_cache.subtract_egld_balance( &tx_context.tx_input_box.from, &tx_context.tx_input_box.egld_value, - ); + ) { + return (TxResult::from_panic_obj(&err), BlockchainUpdate::empty()); + } tx_context.tx_cache.increase_egld_balance( &tx_context.tx_input_box.to, &tx_context.tx_input_box.egld_value, @@ -80,9 +82,16 @@ pub fn deploy_contract( let tx_context = TxContext::new(tx_input, tx_cache); let tx_input_ref = &*tx_context.tx_input_box; - tx_context + if let Err(err) = tx_context .tx_cache - .subtract_egld_balance(&tx_input_ref.from, &tx_input_ref.egld_value); + .subtract_egld_balance(&tx_input_ref.from, &tx_input_ref.egld_value) + { + return ( + TxResult::from_panic_obj(&err), + Address::zero(), + BlockchainUpdate::empty(), + ); + } tx_context.create_new_contract(&new_address, contract_path, tx_input_ref.from.clone()); tx_context .tx_cache diff --git a/vm/src/tx_mock/tx_cache_balance_util.rs b/vm/src/tx_mock/tx_cache_balance_util.rs index 500a95e1bf..339d9414cf 100644 --- a/vm/src/tx_mock/tx_cache_balance_util.rs +++ b/vm/src/tx_mock/tx_cache_balance_util.rs @@ -6,15 +6,17 @@ use crate::{tx_mock::TxPanic, world_mock::EsdtInstanceMetadata}; use super::TxCache; impl TxCache { - pub fn subtract_egld_balance(&self, address: &Address, call_value: &BigUint) { + pub fn subtract_egld_balance( + &self, + address: &Address, + call_value: &BigUint, + ) -> Result<(), TxPanic> { self.with_account_mut(address, |account| { if call_value > &account.egld_balance { - std::panic::panic_any(TxPanic { - status: 10, - message: "failed transfer (insufficient funds)".to_string(), - }); + return Err(TxPanic::vm_error("failed transfer (insufficient funds)")); } account.egld_balance -= call_value; + Ok(()) }) } From 92682940991c9c507d46aeece9d8aa600b25e204 Mon Sep 17 00:00:00 2001 From: Andrei Marinica Date: Mon, 26 Jun 2023 00:33:51 +0300 Subject: [PATCH 2/4] vm - remove panics - subtract esdt --- .../src/whitebox/contract_obj_wrapper.rs | 11 +++----- .../esdt_nft/esdt_local_burn.rs | 6 ++++- .../esdt_nft/esdt_nft_burn_mock.rs | 6 ++++- vm/src/tx_execution/exec_general_tx.rs | 5 +++- vm/src/tx_mock/tx_cache_balance_util.rs | 25 ++++++++----------- 5 files changed, 29 insertions(+), 24 deletions(-) diff --git a/framework/scenario/src/whitebox/contract_obj_wrapper.rs b/framework/scenario/src/whitebox/contract_obj_wrapper.rs index c0d2cdf0f5..5ba1a3e442 100644 --- a/framework/scenario/src/whitebox/contract_obj_wrapper.rs +++ b/framework/scenario/src/whitebox/contract_obj_wrapper.rs @@ -725,19 +725,16 @@ impl BlockchainStateWrapper { for esdt in &esdt_payments { if esdt.value > rust_zero { - let metadata = tx_cache.subtract_esdt_balance( + let transfer_result = tx_cache.transfer_esdt_balance( caller, - &esdt.token_identifier, - esdt.nonce, - &esdt.value, - ); - tx_cache.increase_esdt_balance( sc_address, &esdt.token_identifier, esdt.nonce, &esdt.value, - metadata, ); + if let Err(err) = transfer_result { + return TxResult::from_panic_obj(&err); + } } } diff --git a/vm/src/tx_execution/builtin_function_mocks/esdt_nft/esdt_local_burn.rs b/vm/src/tx_execution/builtin_function_mocks/esdt_nft/esdt_local_burn.rs index a0a89dfd8d..3ec6fd4872 100644 --- a/vm/src/tx_execution/builtin_function_mocks/esdt_nft/esdt_local_burn.rs +++ b/vm/src/tx_execution/builtin_function_mocks/esdt_nft/esdt_local_burn.rs @@ -21,7 +21,11 @@ impl BuiltinFunction for ESDTLocalBurn { let token_identifier = tx_input.args[0].clone(); let value = BigUint::from_bytes_be(tx_input.args[1].as_slice()); - tx_cache.subtract_esdt_balance(&tx_input.to, &token_identifier, 0, &value); + let subtract_result = + tx_cache.subtract_esdt_balance(&tx_input.to, &token_identifier, 0, &value); + if let Err(err) = subtract_result { + return (TxResult::from_panic_obj(&err), BlockchainUpdate::empty()); + } let esdt_nft_create_log = TxLog { address: tx_input.from, diff --git a/vm/src/tx_execution/builtin_function_mocks/esdt_nft/esdt_nft_burn_mock.rs b/vm/src/tx_execution/builtin_function_mocks/esdt_nft/esdt_nft_burn_mock.rs index cc8f3f8c77..31b865799f 100644 --- a/vm/src/tx_execution/builtin_function_mocks/esdt_nft/esdt_nft_burn_mock.rs +++ b/vm/src/tx_execution/builtin_function_mocks/esdt_nft/esdt_nft_burn_mock.rs @@ -25,7 +25,11 @@ impl BuiltinFunction for ESDTNftBurn { let nonce = u64::top_decode(tx_input.args[1].as_slice()).unwrap(); let value = BigUint::from_bytes_be(tx_input.args[2].as_slice()); - tx_cache.subtract_esdt_balance(&tx_input.to, &token_identifier, nonce, &value); + let subtract_result = + tx_cache.subtract_esdt_balance(&tx_input.to, &token_identifier, nonce, &value); + if let Err(err) = subtract_result { + return (TxResult::from_panic_obj(&err), BlockchainUpdate::empty()); + } let esdt_nft_create_log = TxLog { address: tx_input.from, diff --git a/vm/src/tx_execution/exec_general_tx.rs b/vm/src/tx_execution/exec_general_tx.rs index 5d5a2aa98e..0370ef98db 100644 --- a/vm/src/tx_execution/exec_general_tx.rs +++ b/vm/src/tx_execution/exec_general_tx.rs @@ -42,13 +42,16 @@ pub fn default_execution(tx_input: TxInput, tx_cache: TxCache) -> (TxResult, Blo // TODO: temporary, will convert to explicit builtin function first for esdt_transfer in tx_context.tx_input_box.esdt_values.iter() { - tx_context.tx_cache.transfer_esdt_balance( + let transfer_result = tx_context.tx_cache.transfer_esdt_balance( &tx_context.tx_input_box.from, &tx_context.tx_input_box.to, &esdt_transfer.token_identifier, esdt_transfer.nonce, &esdt_transfer.value, ); + if let Err(err) = transfer_result { + return (TxResult::from_panic_obj(&err), BlockchainUpdate::empty()); + } } let mut tx_result = if !tx_context.tx_input_box.to.is_smart_contract_address() diff --git a/vm/src/tx_mock/tx_cache_balance_util.rs b/vm/src/tx_mock/tx_cache_balance_util.rs index 339d9414cf..1f564e9886 100644 --- a/vm/src/tx_mock/tx_cache_balance_util.rs +++ b/vm/src/tx_mock/tx_cache_balance_util.rs @@ -37,32 +37,32 @@ impl TxCache { }); } - #[allow(clippy::redundant_closure)] // clippy is wrong here, `.unwrap_or_else(panic_insufficient_funds)` won't compile pub fn subtract_esdt_balance( &self, address: &Address, esdt_token_identifier: &[u8], nonce: u64, value: &BigUint, - ) -> EsdtInstanceMetadata { + ) -> Result { self.with_account_mut(address, |account| { let esdt_data_map = &mut account.esdt; let esdt_data = esdt_data_map .get_mut_by_identifier(esdt_token_identifier) - .unwrap_or_else(|| panic_insufficient_funds()); + .ok_or_else(err_insufficient_funds)?; let esdt_instances = &mut esdt_data.instances; let esdt_instance = esdt_instances .get_mut_by_nonce(nonce) - .unwrap_or_else(|| panic_insufficient_funds()); + .ok_or_else(err_insufficient_funds)?; + let esdt_balance = &mut esdt_instance.balance; if &*esdt_balance < value { - panic_insufficient_funds(); + return Err(err_insufficient_funds()); } *esdt_balance -= value; - esdt_instance.metadata.clone() + Ok(esdt_instance.metadata.clone()) }) } @@ -91,16 +91,13 @@ impl TxCache { esdt_token_identifier: &[u8], nonce: u64, value: &BigUint, - ) { - let metadata = self.subtract_esdt_balance(from, esdt_token_identifier, nonce, value); - + ) -> Result<(), TxPanic> { + let metadata = self.subtract_esdt_balance(from, esdt_token_identifier, nonce, value)?; self.increase_esdt_balance(to, esdt_token_identifier, nonce, value, metadata); + Ok(()) } } -fn panic_insufficient_funds() -> ! { - std::panic::panic_any(TxPanic { - status: 10, - message: "insufficient funds".to_string(), - }); +fn err_insufficient_funds() -> TxPanic { + TxPanic::vm_error("insufficient funds") } From d2f3a41094e8805a415c717d5ccab2e8a0f0367b Mon Sep 17 00:00:00 2001 From: Andrei Marinica Date: Mon, 26 Jun 2023 00:51:17 +0300 Subject: [PATCH 3/4] vm - remove panics - signal error --- vm/src/tx_execution/catch_tx_panic.rs | 4 ---- vm/src/vm_hooks/vh_debugger_stack.rs | 11 ++++++++++- vm/src/vm_hooks/vh_managed_types_cell.rs | 4 ++++ vm/src/vm_hooks/vh_source.rs | 9 ++------- 4 files changed, 16 insertions(+), 12 deletions(-) diff --git a/vm/src/tx_execution/catch_tx_panic.rs b/vm/src/tx_execution/catch_tx_panic.rs index c3624af9c7..460634d4f1 100644 --- a/vm/src/tx_execution/catch_tx_panic.rs +++ b/vm/src/tx_execution/catch_tx_panic.rs @@ -33,10 +33,6 @@ fn interpret_panic_as_tx_panic( panic_any: Box, panic_message_flag: bool, ) -> TxPanic { - if let Some(panic_obj) = panic_any.downcast_ref::() { - return panic_obj.clone(); - } - if let Some(panic_string) = panic_any.downcast_ref::() { return interpret_panic_str_as_tx_result(panic_string.as_str(), panic_message_flag); } diff --git a/vm/src/vm_hooks/vh_debugger_stack.rs b/vm/src/vm_hooks/vh_debugger_stack.rs index d1d48b250d..4092d44e92 100644 --- a/vm/src/vm_hooks/vh_debugger_stack.rs +++ b/vm/src/vm_hooks/vh_debugger_stack.rs @@ -13,7 +13,7 @@ use crate::{ tx_execution::{deploy_contract, execute_builtin_function_or_default}, tx_mock::{ async_call_tx_input, AsyncCallTxData, BlockchainUpdate, TxCache, TxContext, TxFunctionName, - TxInput, TxManagedTypes, TxResult, + TxInput, TxManagedTypes, TxPanic, TxResult, }, world_mock::{AccountData, BlockInfo, STORAGE_RESERVED_PREFIX}, }; @@ -46,6 +46,15 @@ impl VMHooksHandlerSource for TxContextWrapper { self.0.m_types_borrow_mut() } + fn halt_with_error(&self, status: u64, message: &str) -> ! { + *self.0.result_borrow_mut() = TxResult::from_panic_obj(&TxPanic::new(status, message)); + let breakpoint = match status { + 4 => BreakpointValue::SignalError, + _ => BreakpointValue::ExecutionFailed, + }; + std::panic::panic_any(breakpoint); + } + fn input_ref(&self) -> &TxInput { self.0.input_ref() } diff --git a/vm/src/vm_hooks/vh_managed_types_cell.rs b/vm/src/vm_hooks/vh_managed_types_cell.rs index 05eb5e2f0e..ee7284058c 100644 --- a/vm/src/vm_hooks/vh_managed_types_cell.rs +++ b/vm/src/vm_hooks/vh_managed_types_cell.rs @@ -29,6 +29,10 @@ impl VMHooksHandlerSource for TxManagedTypesCell { self.0.borrow_mut() } + fn halt_with_error(&self, status: u64, message: &str) -> ! { + panic!("VM error occured, status: {status}, message: {message}") + } + fn input_ref(&self) -> &TxInput { panic!("cannot access tx inputs in the StaticApi") } diff --git a/vm/src/vm_hooks/vh_source.rs b/vm/src/vm_hooks/vh_source.rs index 53f99059ac..c7172ae214 100644 --- a/vm/src/vm_hooks/vh_source.rs +++ b/vm/src/vm_hooks/vh_source.rs @@ -6,7 +6,7 @@ use std::{ use multiversx_sc::types::{Address, CodeMetadata, H256}; use crate::{ - tx_mock::{TxFunctionName, TxInput, TxLog, TxManagedTypes, TxPanic, TxResult}, + tx_mock::{TxFunctionName, TxInput, TxLog, TxManagedTypes, TxResult}, world_mock::{AccountData, BlockInfo}, }; @@ -16,12 +16,7 @@ pub trait VMHooksHandlerSource: Debug { fn m_types_borrow_mut(&self) -> RefMut; - fn halt_with_error(&self, status: u64, message: &str) -> ! { - std::panic::panic_any(TxPanic { - status, - message: message.to_string(), - }) - } + fn halt_with_error(&self, status: u64, message: &str) -> !; fn vm_error(&self, message: &str) -> ! { self.halt_with_error(10, message) From e669728408c4f6a600f0fa836209cceac68d7b2d Mon Sep 17 00:00:00 2001 From: Andrei Marinica Date: Mon, 26 Jun 2023 10:11:08 +0300 Subject: [PATCH 4/4] remove static state from TxContext --- vm/src/tx_mock/tx_context.rs | 15 ++------------- vm/src/tx_mock/tx_context_ref.rs | 10 ---------- 2 files changed, 2 insertions(+), 23 deletions(-) diff --git a/vm/src/tx_mock/tx_context.rs b/vm/src/tx_mock/tx_context.rs index 10a899b7e2..75290b7a94 100644 --- a/vm/src/tx_mock/tx_context.rs +++ b/vm/src/tx_mock/tx_context.rs @@ -4,7 +4,7 @@ use crate::{ }; use alloc::vec::Vec; use core::cell::RefCell; -use multiversx_sc::types::{heap::Address, LockableStaticBuffer}; +use multiversx_sc::types::heap::Address; use num_traits::Zero; use std::{ cell::{Ref, RefMut}, @@ -12,20 +12,15 @@ use std::{ rc::Rc, }; -use super::{ - BlockchainRng, BlockchainUpdate, TxCache, TxInput, TxManagedTypes, TxResult, TxStaticVars, -}; +use super::{BlockchainRng, BlockchainUpdate, TxCache, TxInput, TxManagedTypes, TxResult}; #[derive(Debug)] pub struct TxContext { pub tx_input_box: Box, pub tx_cache: Rc, pub managed_types: RefCell, - pub lockable_static_buffer_cell: RefCell, - pub static_vars_cell: RefCell, pub tx_result_cell: RefCell, pub b_rng: RefCell, - pub printed_messages: RefCell>, } impl TxContext { @@ -35,11 +30,8 @@ impl TxContext { tx_input_box: Box::new(tx_input), tx_cache: Rc::new(tx_cache), managed_types: RefCell::new(TxManagedTypes::new()), - lockable_static_buffer_cell: RefCell::new(LockableStaticBuffer::new()), - static_vars_cell: RefCell::new(TxStaticVars::default()), tx_result_cell: RefCell::new(TxResult::empty()), b_rng, - printed_messages: RefCell::new(Vec::new()), } } @@ -70,11 +62,8 @@ impl TxContext { tx_input_box: Box::new(tx_input), tx_cache: Rc::new(tx_cache), managed_types: RefCell::new(TxManagedTypes::new()), - lockable_static_buffer_cell: RefCell::new(LockableStaticBuffer::new()), - static_vars_cell: RefCell::new(TxStaticVars::default()), tx_result_cell: RefCell::new(TxResult::empty()), b_rng, - printed_messages: RefCell::new(Vec::new()), } } diff --git a/vm/src/tx_mock/tx_context_ref.rs b/vm/src/tx_mock/tx_context_ref.rs index a4a8d73637..2b7c7f3024 100644 --- a/vm/src/tx_mock/tx_context_ref.rs +++ b/vm/src/tx_mock/tx_context_ref.rs @@ -65,14 +65,4 @@ impl TxContextRef { .tx_result_cell .replace(TxResult::from_panic_obj(&tx_panic)); } - - /// Will yield a copy of all messages printed on this context. - pub fn printed_messages(&self) -> Vec { - self.0.printed_messages.borrow().clone() - } - - /// Clears entire print history. - pub fn printed_messages_clear(&self) { - self.0.printed_messages.borrow_mut().clear(); - } }