Merge pull request #1135 from multiversx/debugger-breakpoints-2

VM - error handling refactoring #2
multiversx · Jun 28, 2023 · 3f1f16a · 3f1f16a
2 parents 801624b + e669728
commit 3f1f16a
Show file tree

Hide file tree

Showing 12 changed files with 78 additions and 71 deletions.
diff --git a/framework/scenario/src/whitebox/contract_obj_wrapper.rs b/framework/scenario/src/whitebox/contract_obj_wrapper.rs
@@ -717,25 +717,24 @@ 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);
         }
 
         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
@@ -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
@@ -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/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<dyn std::any::Any + std::marker::Send>,
     panic_message_flag: bool,
 ) -> TxPanic {
-    if let Some(panic_obj) = panic_any.downcast_ref::<TxPanic>() {
-        return panic_obj.clone();
-    }
-
     if let Some(panic_string) = panic_any.downcast_ref::<String>() {
         return interpret_panic_str_as_tx_result(panic_string.as_str(), panic_message_flag);
     }

diff --git 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
@@ -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,
@@ -40,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()
@@ -80,9 +85,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
@@ -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(())
         })
     }
 
@@ -35,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<EsdtInstanceMetadata, TxPanic> {
         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())
         })
     }
 
@@ -89,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")
 }
diff --git a/vm/src/tx_mock/tx_context.rs b/vm/src/tx_mock/tx_context.rs
@@ -4,28 +4,23 @@ 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},
     collections::HashMap,
     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<TxInput>,
     pub tx_cache: Rc<TxCache>,
     pub managed_types: RefCell<TxManagedTypes>,
-    pub lockable_static_buffer_cell: RefCell<LockableStaticBuffer>,
-    pub static_vars_cell: RefCell<TxStaticVars>,
     pub tx_result_cell: RefCell<TxResult>,
     pub b_rng: RefCell<BlockchainRng>,
-    pub printed_messages: RefCell<Vec<String>>,
 }
 
 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
@@ -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<String> {
-        self.0.printed_messages.borrow().clone()
-    }
-
-    /// Clears entire print history.
-    pub fn printed_messages_clear(&self) {
-        self.0.printed_messages.borrow_mut().clear();
-    }
 }
diff --git 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
@@ -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
@@ -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<TxManagedTypes>;
 
-    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)