Remove ConversionError from ScVal/Val conversions

soroban-env-common/src/convert.rs

@@ -371,16 +371,17 @@ impl<E: Env> TryFromVal<E, U256> for U256Val {
 impl<E: Env> TryFromVal<E, Val> for ScVal
 where
     ScValObject: TryFromVal<E, Object>,
+    <ScValObject as TryFromVal<E, Object>>::Error: Into<crate::Error>,
 {
-    type Error = ConversionError;
+    type Error = crate::Error;
 
-    fn try_from_val(env: &E, val: &Val) -> Result<Self, ConversionError> {
+    fn try_from_val(env: &E, val: &Val) -> Result<Self, crate::Error> {
         if let Ok(object) = Object::try_from(val) {
-            // Bug https://github.com/stellar/rs-soroban-env/issues/1076: it's
-            // not really great to be dropping the error from the other
-            // TryFromVal here, we should really switch to taking errors from E
-            // or returning Error or something.
-            let scvo: ScValObject = object.try_into_val(env).map_err(|_| ConversionError)?;
+            let scvo: ScValObject = object.try_into_val(env).map_err(|e| {
+                let e: <ScValObject as TryFromVal<E, Object>>::Error = e;
+                let e: crate::Error = e.into();
+                e
+            })?;
             return Ok(scvo.into());
         }
         let val = *val;
@@ -453,7 +454,7 @@ where
             | Tag::SmallCodeUpperBound
             | Tag::ObjectCodeLowerBound
             | Tag::ObjectCodeUpperBound
-            | Tag::Bad => Err(ConversionError),
+            | Tag::Bad => Err(ConversionError.into()),
         }
     }
 }
@@ -470,16 +471,21 @@ fn require_or_conversion_error(b: bool) -> Result<(), ConversionError> {
 #[cfg(feature = "std")]
 impl<E: Env> TryFromVal<E, ScVal> for Val
 where
-    Object: for<'a> TryFromVal<E, ScValObjRef<'a>, Error = ConversionError>,
+    Object: for<'a> TryFromVal<E, ScValObjRef<'a>>,
+    for<'a> <Object as TryFromVal<E, ScValObjRef<'a>>>::Error: Into<crate::Error>,
 {
-    type Error = ConversionError;
+    type Error = crate::Error;
     fn try_from_val(env: &E, val: &ScVal) -> Result<Val, Self::Error> {
         if !Val::can_represent_scval(val) {
-            return Err(ConversionError);
+            return Err(ConversionError.into());
         }
 
         if let Some(scvo) = ScValObjRef::classify(val) {
-            let obj = Object::try_from_val(env, &scvo)?;
+            let obj = Object::try_from_val(env, &scvo).map_err(|e| {
+                let e: <Object as TryFromVal<E, ScValObjRef>>::Error = e;
+                let e: crate::Error = e.into();
+                e
+            })?;
             return Ok(obj.into());
         }
 
@@ -527,9 +533,7 @@ where
             ScVal::Symbol(bytes) => {
                 // NB: Long symbols are objects and should have been
                 // handled before reaching this point.
-                SymbolSmall::try_from_bytes(bytes.as_slice())
-                    .map_err(|_| ConversionError {})?
-                    .into()
+                SymbolSmall::try_from_bytes(bytes.as_slice())?.into()
             }
 
             // These should all have been classified as ScValObjRef above, or are
@@ -542,7 +546,7 @@ where
             | ScVal::Address(_)
             | ScVal::LedgerKeyNonce(_)
             | ScVal::LedgerKeyContractInstance
-            | ScVal::ContractInstance(_) => return Err(ConversionError),
+            | ScVal::ContractInstance(_) => return Err(ConversionError.into()),
         })
     }
 }

soroban-env-common/src/object.rs

@@ -1,7 +1,6 @@
 use crate::xdr::{Duration, ScVal, TimePoint};
 use crate::{
-    impl_val_wrapper_base, num, val::ValConvert, Compare, ConversionError, Convert, Env, Tag,
-    TryFromVal, Val,
+    impl_val_wrapper_base, num, val::ValConvert, Compare, Convert, Env, Tag, TryFromVal, Val,
 };
 use core::{cmp::Ordering, fmt::Debug};
 
@@ -73,12 +72,12 @@ impl<'a, E> TryFromVal<E, ScValObjRef<'a>> for Object
 where
     E: Env + Convert<ScValObjRef<'a>, Object>,
 {
-    type Error = ConversionError;
+    type Error = crate::Error;
 
     fn try_from_val(env: &E, v: &ScValObjRef<'a>) -> Result<Self, Self::Error> {
         match env.convert(*v) {
             Ok(obj) => Ok(obj),
-            Err(_) => Err(ConversionError),
+            Err(e) => Err(e.into()),
         }
     }
 }

soroban-env-host/observations/test__bytes__bytes_xdr_roundtrip.json

@@ -37,11 +37,11 @@
   "  35 call obj_cmp(Map(obj#21), Map(obj#25))": "",
   "  36 ret obj_cmp -> Ok(0)": "cpu:557187",
   "  37 call deserialize_from_bytes(Bytes(obj#27))": "cpu:557627, mem:11250, objs:-/14@629f0649",
-  "  38 ret deserialize_from_bytes -> Err(Error(Value, UnexpectedType))": "cpu:618507, mem:11422",
+  "  38 ret deserialize_from_bytes -> Err(Error(Object, InvalidInput))": "cpu:618507, mem:11422",
   "  39 call deserialize_from_bytes(Bytes(obj#29))": "cpu:618947, mem:11486, objs:-/15@ede0653",
   "  40 ret deserialize_from_bytes -> Err(Error(Value, UnexpectedType))": "cpu:678371, mem:11510",
   "  41 call deserialize_from_bytes(Bytes(obj#31))": "cpu:678811, mem:11574, objs:-/16@e2db8e43",
-  "  42 ret deserialize_from_bytes -> Err(Error(Value, UnexpectedType))": "cpu:739941, mem:11770",
+  "  42 ret deserialize_from_bytes -> Err(Error(Object, InvalidInput))": "cpu:739941, mem:11770",
   "  43 call deserialize_from_bytes(Bytes(obj#33))": "cpu:740381, mem:11834, objs:-/17@a3c5f6ba",
   "  44 ret deserialize_from_bytes -> Err(Error(Value, UnexpectedType))": "cpu:800930, mem:11966",
   "  45 call deserialize_from_bytes(Bytes(obj#35))": "cpu:801370, mem:12030, objs:-/18@ec6a12bd",
@@ -57,7 +57,7 @@
   "  55 call deserialize_from_bytes(Bytes(obj#43))": "",
   "  56 ret deserialize_from_bytes -> Ok(Symbol())": "cpu:986698, mem:14164",
   "  57 call deserialize_from_bytes(Bytes(obj#45))": "cpu:987138, mem:14228, objs:-/23@2d3c8249",
-  "  58 ret deserialize_from_bytes -> Err(Error(Value, UnexpectedType))": "cpu:1046687, mem:14264",
+  "  58 ret deserialize_from_bytes -> Err(Error(Value, InvalidInput))": "cpu:1046687, mem:14264",
   "  59 call serialize_to_bytes(Error(Context, InternalError))": "",
   "  60 ret serialize_to_bytes -> Ok(Bytes(obj#47))": "cpu:1047878, mem:15090, objs:-/24@43b5a0dc",
   "  61 call deserialize_from_bytes(Bytes(obj#47))": "",

soroban-env-host/observations/test__hostile_opt__test_deep_stack_call_fails_when_serialization_depth_exceeded.json

@@ -283,8 +283,8 @@
   " 281 call vec_new_from_linear_memory(U32(1048552), U32(1))": "cpu:6600690, mem:6604252, objs:104@fd4da56f/133@cc2496ee, stk:2@ffc81085",
   " 282 ret vec_new_from_linear_memory -> Ok(Vec(obj#267))": "cpu:6602530, mem:6604340, objs:104@fd4da56f/134@72bdaced",
   " 283 call serialize_to_bytes(Vec(obj#267))": "cpu:6603026, mem:6604364, objs:105@5f8527e8/134@72bdaced, stk:2@f4217cde",
-  " 284 ret serialize_to_bytes -> Err(Error(Value, UnexpectedType))": "cpu:6663217, mem:6612364",
-  " 285 pop VM:5cf712f4:sym#63 -> Err(Error(Value, UnexpectedType))": " vm:1114112@24decc95/7@ae192e85",
+  " 284 ret serialize_to_bytes -> Err(Error(Context, ExceededLimit))": "cpu:6663217, mem:6612364",
+  " 285 pop VM:5cf712f4:sym#63 -> Err(Error(Context, ExceededLimit))": " vm:1114112@24decc95/7@ae192e85",
   " 286 ret require_auth_for_args -> Err(Error(Auth, InvalidAction))": " objs:2@82a2e941/134@72bdaced, vm:-/-, store:-/3@6a8cf640, stk:1@7dce9137, auth:1@c6a8e2e4/1@7c4ee7fb",
   " 287 pop VM:5cf712f4:call -> Err(Error(Auth, InvalidAction))": " vm:1114112@a6f378b8/7@ae192e85",
   " 288 ret call -> Err(Error(Auth, InvalidAction))": " objs:-/134@72bdaced, vm:-/-, stk:-, auth:-/-",

soroban-env-host/observations/test__hostile_opt__test_deep_stack_call_fails_xdr_deserialization_exceeding_host_object_depth.json

@@ -7221,8 +7221,8 @@
   "15653 call vec_get(Vec(obj#2979), U32(1))": "",
   "15654 ret vec_get -> Ok(Bytes(obj#2977))": "cpu:267629107",
   "15655 call deserialize_from_bytes(Bytes(obj#2977))": "cpu:267629603, mem:266295038, objs:5@688bccae/2385@d7c8d6dd, stk:100@6a4dd6a7",
-  "15656 ret deserialize_from_bytes -> Err(Error(Value, UnexpectedType))": "cpu:267779041, mem:266301050",
-  "15657 pop VM:5cf712f4:sym#4767 -> Err(Error(Value, UnexpectedType))": " vm:1114112@939ed128/7@ae192e85",
+  "15656 ret deserialize_from_bytes -> Err(Error(Context, ExceededLimit))": "cpu:267779041, mem:266301050",
+  "15657 pop VM:5cf712f4:sym#4767 -> Err(Error(Context, ExceededLimit))": " vm:1114112@939ed128/7@ae192e85",
   "15658 ret require_auth_for_args -> Err(Error(Auth, InvalidAction))": " objs:6@e1a15142/2385@d7c8d6dd, vm:-/-, stk:99@53f386d5, auth:99@f0706ecf/1@6847e66e",
   "15659 pop VM:5cf712f4:sym#4749 -> Err(Error(Auth, InvalidAction))": " vm:1114112@e86028c5/7@ae192e85",
   "1566 ret symbol_new_from_slice -> Ok(Symbol(obj#1765))": "cpu:134722781, mem:132928323, objs:-/883@55c5d37e",

soroban-env-host/src/host/conversion.rs

@@ -320,9 +320,8 @@ impl Host {
         // and is done inside `from_host_obj`.
         let _span = tracy_span!("Val to ScVal");
         let scval = self.budget_cloned().with_limited_depth(|_| {
-            ScVal::try_from_val(self, &val).map_err(|cerr: crate::ConversionError| {
-                self.error(cerr.into(), "failed to convert host value to ScVal", &[])
-            })
+            ScVal::try_from_val(self, &val)
+                .map_err(|cerr| self.error(cerr, "failed to convert host value to ScVal", &[val]))
         })?;
         // This is a check of internal logical consistency: we came _from_ a Val
         // so the ScVal definitely should have been representable.
@@ -337,9 +336,7 @@ impl Host {
         // and is done inside `to_host_obj`.
         self.budget_cloned().with_limited_depth(|_| {
             v.try_into_val(self)
-                .map_err(|cerr: crate::ConversionError| {
-                    self.error(cerr.into(), "failed to convert ScVal to host value", &[])
-                })
+                .map_err(|cerr| self.error(cerr, "failed to convert ScVal to host value", &[]))
         })
     }
 
@@ -521,11 +518,11 @@ impl Host {
             | ScVal::I32(_)
             | ScVal::LedgerKeyNonce(_)
             | ScVal::ContractInstance(_)
-            | ScVal::LedgerKeyContractInstance => Err(err!(
-                self,
-                (ScErrorType::Value, ScErrorCode::InternalError),
+            | ScVal::LedgerKeyContractInstance => Err(self.err(
+                ScErrorType::Value,
+                ScErrorCode::InternalError,
                 "converting ScValObjRef on non-object ScVal type",
-                *val
+                &[],
             )),
         }
     }

soroban-env-host/src/host/data_helper.rs

@@ -95,11 +95,11 @@ impl Host {
         match &entry.data {
             LedgerEntryData::ContractData(e) => match &e.val {
                 ScVal::ContractInstance(instance) => instance.metered_clone(self),
-                other => Err(err!(
-                    self,
-                    (ScErrorType::Storage, ScErrorCode::InternalError),
+                _ => Err(self.err(
+                    ScErrorType::Storage,
+                    ScErrorCode::InternalError,
                     "ledger entry for contract instance does not contain contract instance",
-                    *other
+                    &[],
                 )),
             },
             _ => Err(self.err(

soroban-env-host/src/test/bytes.rs

@@ -121,11 +121,8 @@ fn bytes_xdr_roundtrip() -> Result<(), HostError> {
         let bo = host.add_host_object(ScBytes(bytes.try_into()?))?;
         let res = host.deserialize_from_bytes(bo);
         assert!(res.is_err());
-        assert!(res
-            .err()
-            .unwrap()
-            .error
-            .is_code(ScErrorCode::UnexpectedType));
+        let err = res.err().unwrap().error;
+        assert!(err.is_code(ScErrorCode::UnexpectedType) || err.is_code(ScErrorCode::InvalidInput));
         Ok(())
     };
     // u64
@@ -293,7 +290,10 @@ fn arbitrary_xdr_roundtrips() -> Result<(), HostError> {
                 successes += 1;
             }
             Err(err) => {
-                assert!(err.error.is_code(ScErrorCode::UnexpectedType));
+                assert!(
+                    err.error.is_code(ScErrorCode::UnexpectedType)
+                        || err.error.is_code(ScErrorCode::InvalidInput)
+                );
                 failures += 1;
             }
         }

soroban-env-host/src/test/depth_limit.rs

@@ -26,7 +26,7 @@ fn deep_scval_to_host_val() -> Result<(), HostError> {
     // NB: this error code is not great, it's a consequence of
     // bug https://github.com/stellar/rs-soroban-env/issues/1046
     // where the TryIntoVal impl in common eats HostErrors
-    let code = (ScErrorType::Value, ScErrorCode::UnexpectedType);
+    let code = (ScErrorType::Context, ScErrorCode::ExceededLimit);
     assert!(HostError::result_matches_err(res, code));
     Ok(())
 }
@@ -42,10 +42,7 @@ fn deep_host_val_to_scval() -> Result<(), HostError> {
         hv = host.vec_push_back(vv, hv.to_val())?;
     }
     let res = host.from_host_obj(hv);
-    // NB: this error code is not great, it's a consequence of
-    // bug https://github.com/stellar/rs-soroban-env/issues/1046
-    // where the TryIntoVal impl in common eats HostErrors
-    let code = (ScErrorType::Value, ScErrorCode::UnexpectedType);
+    let code = (ScErrorType::Context, ScErrorCode::ExceededLimit);
     assert!(HostError::result_matches_err(res, code));
     Ok(())
 }

soroban-env-host/src/test/event.rs

@@ -318,12 +318,9 @@ fn too_big_event_topic() -> Result<(), HostError> {
 
     let host = (*host).clone();
     let res = host.try_finish();
-    // This fails at converting from Bytes to ScBytes due to out-of-budget
-    // however, the `ScVal::try_from_val` converts it into a `ConversionError`
-    // which is misleading. We already have a tracking issue #1076.
     assert!(HostError::result_matches_err(
         res,
-        (ScErrorType::Value, ScErrorCode::UnexpectedType)
+        (ScErrorType::Budget, ScErrorCode::ExceededLimit)
     ));
     Ok(())
 }
@@ -341,12 +338,9 @@ fn too_big_event_data() -> Result<(), HostError> {
 
     let host = (*host).clone();
     let res = host.try_finish();
-    // This fails at converting from Bytes to ScBytes due to out-of-budget
-    // however, the `ScVal::try_from_val` converts it into a `ConversionError`
-    // which is misleading. We already have a tracking issue #1076.
     assert!(HostError::result_matches_err(
         res,
-        (ScErrorType::Value, ScErrorCode::UnexpectedType)
+        (ScErrorType::Budget, ScErrorCode::ExceededLimit)
     ));
     Ok(())
 }

soroban-env-host/src/test/map.rs

@@ -454,11 +454,9 @@ fn initialization_invalid() -> Result<(), HostError> {
 
     let map = host.map_new_from_slices(&keys, &vals.as_slice())?;
     let res = host.from_host_val(map.to_val());
-    // This is actually Budget::ExceededLimit, but ScVal::try_from_val converts
-    // he error to a ConversionError. We have an issue for it https://github.com/stellar/rs-soroban-env/issues/1046
     assert!(HostError::result_matches_err(
         res,
-        (ScErrorType::Value, ScErrorCode::UnexpectedType)
+        (ScErrorType::Budget, ScErrorCode::ExceededLimit)
     ));
 
     Ok(())

soroban-env-host/src/test/symbol.rs

@@ -180,10 +180,6 @@ fn invalid_symbol_conversions() {
         assert!(e.error.is_type(ScErrorType::Value));
         assert!(e.error.is_code(ScErrorCode::InvalidInput));
     };
-    let unexpected_type_err = |e: HostError| {
-        assert!(e.error.is_type(ScErrorType::Value));
-        assert!(e.error.is_code(ScErrorCode::UnexpectedType));
-    };
     for s in symbol_byte_strs {
         // Some of the test strings aren't valid UTF-8, but we should only interpret
         // them as bytes.
@@ -207,7 +203,7 @@ fn invalid_symbol_conversions() {
                 .unwrap()
                 .into(),
         );
-        unexpected_type_err(
+        invalid_input_err(
             host.to_host_val(&ScVal::Symbol(ScSymbol(s_str.try_into().unwrap())))
                 .err()
                 .unwrap(),

soroban-env-host/src/test/vec.rs

@@ -424,11 +424,9 @@ fn instantiate_oversized_vec_from_slice() -> Result<(), HostError> {
     let vals = vec![bytes_val; 5];
     let vec = host.vec_new_from_slice(&vals.as_slice())?;
     let res = host.from_host_val(vec.to_val());
-    // This is actually Budget::ExceededLimit, but ScVal::try_from_val converts
-    // he error to a ConversionError. We have an issue for it https://github.com/stellar/rs-soroban-env/issues/1046
     assert!(HostError::result_matches_err(
         res,
-        (ScErrorType::Value, ScErrorCode::UnexpectedType)
+        (ScErrorType::Budget, ScErrorCode::ExceededLimit)
     ));
     Ok(())
 }