Repository: stellar/rs-soroban-env
Pull request #1083 (2023-09-22 21:59:08)

Small update for the default budget limits.
### What

Updated the comment, reduced the memory a bit, and made the constants private (they shouldn't serve as a source of truth for the network config).

### Why

Documentation improevment

### Known limitations

N/A



Patch:
diff --git a/.github/workflows/check-bump-interface-version.yml b/.github/workflows/check-bump-interface-version.yml
index e2cb0adb..303edae8 100644
--- a/.github/workflows/check-bump-interface-version.yml
+++ b/.github/workflows/check-bump-interface-version.yml
@@ -22,4 +22,4 @@ jobs:
     - name: "check soroban-test-wasms/wasm-workspace/opt/*.wasm were modified"
       uses: brettcannon/check-for-changed-files@36aaa76474fc979db4e19e44c05e8e978759ac01
       with:
-        file-pattern: "soroban-test-wasms/wasm-workspace/opt/*.wasm"
+        file-pattern: "soroban-test-wasms/wasm-workspace/opt/curr/*.wasm"
diff --git a/soroban-env-common/env.json b/soroban-env-common/env.json
index 31936347..e48357ec 100644
--- a/soroban-env-common/env.json
+++ b/soroban-env-common/env.json
@@ -1474,7 +1474,7 @@
                         }
                     ],
                     "return": "Val",
-                    "docs": "Calls a function in another contract with arguments contained in vector `args`, returning either the result of the called function or an ScError if the called function failed."
+                    "docs": "Calls a function in another contract with arguments contained in vector `args`, returning either the result of the called function or an `Error` if the called function failed. The returned error is either a custom `ContractError` that the called contract returns explicitly, or an error with type `Context` and code `InvalidAction` in case of any other error in the called contract (such as a host function failure that caused a trap). `try_call` might trap in a few scenarios where the error can't be meaningfully recovered from, such as running out of budget."
                 }
             ]
         },
diff --git a/soroban-env-common/src/error.rs b/soroban-env-common/src/error.rs
index bb09e68b..d3d382cc 100644
--- a/soroban-env-common/src/error.rs
+++ b/soroban-env-common/src/error.rs
@@ -199,6 +199,20 @@ impl From<wasmi::core::TrapCode> for Error {
     }
 }
 
+#[cfg(feature = "wasmi")]
+impl From<wasmi::errors::FuncError> for Error {
+    fn from(err: wasmi::errors::FuncError) -> Self {
+        let ec = match err {
+            wasmi::errors::FuncError::ExportedFuncNotFound => ScErrorCode::MissingValue,
+            wasmi::errors::FuncError::MismatchingParameterType
+            | wasmi::errors::FuncError::MismatchingResultType => ScErrorCode::UnexpectedType,
+            wasmi::errors::FuncError::MismatchingParameterLen
+            | wasmi::errors::FuncError::MismatchingResultLen => ScErrorCode::UnexpectedSize,
+        };
+        return Error::from_type_and_code(ScErrorType::WasmVm, ec);
+    }
+}
+
 #[cfg(feature = "wasmi")]
 impl From<wasmi::Error> for Error {
     fn from(e: wasmi::Error) -> Self {
@@ -235,6 +249,18 @@ impl From<wasmi::Error> for Error {
                     return code.into();
                 }
             }
+            wasmi::Error::Func(e) => {
+                return e.into();
+            }
+            wasmi::Error::Global(e) => {
+                if matches!(e, wasmi::errors::GlobalError::TypeMismatch { .. }) {
+                    return Error::from_type_and_code(
+                        ScErrorType::WasmVm,
+                        ScErrorCode::UnexpectedType,
+                    );
+                }
+            }
+
             _ => (),
         }
 
diff --git a/soroban-env-common/src/meta.rs b/soroban-env-common/src/meta.rs
index 1d506907..6a486ce7 100644
--- a/soroban-env-common/src/meta.rs
+++ b/soroban-env-common/src/meta.rs
@@ -58,7 +58,7 @@ soroban_env_macros::generate_env_meta_consts!(
 #[cfg(not(feature = "next"))]
 soroban_env_macros::generate_env_meta_consts!(
     ledger_protocol_version: 20,
-    pre_release_version: 60,
+    pre_release_version: 61,
 );
 
 pub const fn get_ledger_protocol_version(interface_version: u64) -> u32 {
diff --git a/soroban-env-host/src/budget.rs b/soroban-env-host/src/budget.rs
index 0bab914a..21592b68 100644
--- a/soroban-env-host/src/budget.rs
+++ b/soroban-env-host/src/budget.rs
@@ -15,9 +15,10 @@ use crate::{
 
 use wasmi::{errors, FuelCosts, ResourceLimiter};
 
-// These should match the default network config settings in core
-pub const DEFAULT_CPU_INSN_LIMIT: u64 = 100_000_000;
-pub const DEFAULT_MEM_BYTES_LIMIT: u64 = 100 * 1024 * 1024; // 100MB
+// These are some sane values, however the embedder should typically customize
+// these to match the network config.
+const DEFAULT_CPU_INSN_LIMIT: u64 = 100_000_000;
+const DEFAULT_MEM_BYTES_LIMIT: u64 = 40 * 1024 * 1024; // 40MB
 
 /// The number of bits to scale the linear term by. The linear coefficient has
 /// been scaled by this factor during parameter fitting to retain more significant
diff --git a/soroban-env-host/src/host.rs b/soroban-env-host/src/host.rs
index b97081dc..3f06cade 100644
--- a/soroban-env-host/src/host.rs
+++ b/soroban-env-host/src/host.rs
@@ -466,23 +466,20 @@ impl Host {
         use crate::native_contract::account_contract::ACCOUNT_CONTRACT_CHECK_AUTH_FN_NAME;
 
         let contract_id = self.hash_from_bytesobj_input("contract", contract)?;
-        let args = self.call_args_from_obj(args)?;
+        let args_vec = self.call_args_from_obj(args)?;
         let res = self.call_n_internal(
             &contract_id,
             ACCOUNT_CONTRACT_CHECK_AUTH_FN_NAME.try_into_val(self)?,
-            args.as_slice(),
+            args_vec.as_slice(),
             ContractReentryMode::Prohibited,
             true,
         );
         if let Err(e) = &res {
-            self.with_events_mut(|events| {
-                self.err_diagnostics(
-                    events,
-                    e.error,
-                    "check auth invocation for a custom account contract failed",
-                    &[],
-                )
-            })?;
+            self.error(
+                e.error,
+                "check auth invocation for a custom account contract failed",
+                &[contract.to_val(), args.to_val()],
+            );
         }
         res
     }
@@ -2424,14 +2421,11 @@ impl VmCallerEnv for Host {
             false,
         );
         if let Err(e) = &res {
-            self.with_events_mut(|events| {
-                self.err_diagnostics(
-                    events,
-                    e.error,
-                    "contract call failed",
-                    &[func.to_val(), args.to_val()],
-                )
-            })?;
+            self.error(
+                e.error,
+                "contract call failed",
+                &[func.to_val(), args.to_val()],
+            );
         }
         res
     }
@@ -2459,19 +2453,34 @@ impl VmCallerEnv for Host {
         match res {
             Ok(rv) => Ok(rv),
             Err(e) => {
-                self.with_events_mut(|events| {
-                    self.err_diagnostics(
-                        events,
-                        e.error,
-                        "contract try_call failed",
-                        &[func.to_val(), args.to_val()],
-                    )
-                })?;
+                self.error(
+                    e.error,
+                    "contract try_call failed",
+                    &[func.to_val(), args.to_val()],
+                );
                 // Only allow to gracefully handle the recoverable errors.
                 // Non-recoverable errors should still cause guest to panic and
                 // abort execution.
                 if e.is_recoverable() {
-                    Ok(e.error.to_val())
+                    // Pass contract errors through.
+                    if e.error.is_type(ScErrorType::Contract) {
+                        Ok(e.error.to_val())
+                    } else {
+                        // Narrow all the remaining host errors down to a single
+                        // error type. We don't want to expose the granular host
+                        // errors to the guest, consistently with how every
+                        // other host function works. This reduces the risk of
+                        // implementation being 'locked' into specific error
+                        // codes due to them being exposed to the guest and
+                        // hashed into blockchain.
+                        // The granular error codes are still observable with
+                        // diagnostic events.
+                        Ok(Error::from_type_and_code(
+                            ScErrorType::Context,
+                            ScErrorCode::InvalidAction,
+                        )
+                        .to_val())
+                    }
                 } else {
                     Err(e)
                 }
diff --git a/soroban-env-host/src/vm.rs b/soroban-env-host/src/vm.rs
index cbb95baf..34cf6c84 100644
--- a/soroban-env-host/src/vm.rs
+++ b/soroban-env-host/src/vm.rs
@@ -347,7 +347,14 @@ impl Vm {
             match e {
                 wasmi::Error::Trap(trap) => {
                     if let Some(code) = trap.trap_code() {
-                        return Err(code.into());
+                        let err = code.into();
+                        return Err(if host.is_debug()? {
+                            // With diagnostics on: log as much detail as we can from wasmi.
+                            let msg = format!("VM call trapped: {:?}", &code);
+                            host.error(err, &msg, &[func_sym.to_val(), err.to_val()])
+                        } else {
+                            err.into()
+                        });
                     }
                     if let Some(he) = trap.downcast::<HostError>() {
                         host.log_diagnostics(


Test patch:
diff --git a/soroban-env-host/src/test/budget_metering.rs b/soroban-env-host/src/test/budget_metering.rs
index 6ec649b0..b4547341 100644
--- a/soroban-env-host/src/test/budget_metering.rs
+++ b/soroban-env-host/src/test/budget_metering.rs
@@ -358,7 +358,7 @@ fn total_amount_charged_from_random_inputs() -> Result<(), HostError> {
     expect![[r#"
         =====================================================================================================================================================================
         Cpu limit: 100000000; used: 10115314
-        Mem limit: 104857600; used: 276044
+        Mem limit: 41943040; used: 276044
         =====================================================================================================================================================================
         CostType                 iterations     input          cpu_insns      mem_bytes      const_term_cpu      lin_term_cpu        const_term_mem      lin_term_mem        
         WasmInsnExec             246            None           1476           0              6                   0                   0                   0                   
diff --git a/soroban-env-host/src/test/invocation.rs b/soroban-env-host/src/test/invocation.rs
index 6e1ce5a4..7a98f762 100644
--- a/soroban-env-host/src/test/invocation.rs
+++ b/soroban-env-host/src/test/invocation.rs
@@ -103,9 +103,8 @@ fn invoke_cross_contract_with_err() -> Result<(), HostError> {
 
     // try_call
     let sv = host.try_call(id_obj, sym, args)?;
-    let code = (ScErrorType::Object, ScErrorCode::IndexBounds);
-    let exp_st: Error = code.into();
-    assert_eq!(sv.get_payload(), exp_st.to_val().get_payload());
+    let generic_host_error: Error = (ScErrorType::Context, ScErrorCode::InvalidAction).into();
+    assert_eq!(sv.get_payload(), generic_host_error.to_val().get_payload());
 
     let events = host.get_events()?.0;
     assert_eq!(events.len(), 5);
@@ -119,7 +118,10 @@ fn invoke_cross_contract_with_err() -> Result<(), HostError> {
 
     // call
     let res = host.call(id_obj, sym, args);
-    assert!(HostError::result_matches_err(res, code));
+    assert!(HostError::result_matches_err(
+        res,
+        (ScErrorType::Object, ScErrorCode::IndexBounds)
+    ));
 
     let events = host.get_events()?.0;
     assert_eq!(events.len(), 10);
@@ -161,12 +163,14 @@ fn invoke_cross_contract_indirect_err() -> Result<(), HostError> {
 
     // try call -- add will trap, and add_with will trap, but we will get an error
     let error = host.try_call(id0_obj, sym, args)?;
-    let code = (ScErrorType::WasmVm, ScErrorCode::InvalidAction);
-    let exp: Error = code.into();
-    assert_eq!(error.get_payload(), exp.to_val().get_payload());
+    let generic_host_error: Error = (ScErrorType::Context, ScErrorCode::InvalidAction).into();
+    assert_eq!(
+        error.get_payload(),
+        generic_host_error.to_val().get_payload()
+    );
 
     let events = host.get_events()?.0;
-    assert_eq!(events.len(), 2);
+    assert_eq!(events.len(), 3);
     let last_event = events.last().unwrap();
     // run `UPDATE_EXPECT=true cargo test` to update this.
     let expected = expect![[
@@ -177,10 +181,13 @@ fn invoke_cross_contract_indirect_err() -> Result<(), HostError> {
 
     // call
     let res = host.call(id0_obj, sym, args);
-    assert!(HostError::result_matches_err(res, code));
+    assert!(HostError::result_matches_err(
+        res,
+        (ScErrorType::WasmVm, ScErrorCode::InvalidAction)
+    ));
 
     let events = host.get_events()?.0;
-    assert_eq!(events.len(), 4);
+    assert_eq!(events.len(), 6);
     let last_event = events.last().unwrap();
     // run `UPDATE_EXPECT=true cargo test` to update this.
     let expected = expect![[