Add infallible From conversions for types to ScVal (#1338)

### What Add infallible From conversions for types to ScVal, replacing the existing fallible TryFrom conversions. ### Why When I look through tests I see lots of `try_into().unwrap()` when converting types to their ScVal values. This is unnecessary, we know that the conversions will succeed because the host won't give the SDK types that aren't valid and couldn't be converted. It's just a result of the way the Rust types are setup we can't guarantee the type is always convertible, but in practice they are. Note that this is _not_ really a breaking change, because in the Rust core lib there is a blanket impl for all From conversions to also provide a TryFrom conversion. The only behaviour change is that if there were cases that would fail conversion previously a panic will occur instead of an error when using a TryFrom to do the conversion. In all cases there is no expected failure because the Env guarantees valid host types to be created and we're converting host types to ScVal types.
stellar · Sep 26, 2024 · 8fc9f53 · 8fc9f53
1 parent 9507fc4
commit 8fc9f53
Show file tree

Hide file tree

Showing 7 changed files with 101 additions and 85 deletions.
diff --git a/soroban-sdk/src/address.rs b/soroban-sdk/src/address.rs
@@ -124,18 +124,21 @@ impl TryFromVal<Env, &Address> for Val {
 }
 
 #[cfg(not(target_family = "wasm"))]
-impl TryFrom<&Address> for ScVal {
-    type Error = ConversionError;
-    fn try_from(v: &Address) -> Result<Self, ConversionError> {
-        Ok(ScVal::try_from_val(&v.env, &v.obj.to_val())?)
+impl From<&Address> for ScVal {
+    fn from(v: &Address) -> Self {
+        // This conversion occurs only in test utilities, and theoretically all
+        // values should convert to an ScVal because the Env won't let the host
+        // type to exist otherwise, unwrapping. Even if there are edge cases
+        // that don't, this is a trade off for a better test developer
+        // experience.
+        ScVal::try_from_val(&v.env, &v.obj.to_val()).unwrap()
     }
 }
 
 #[cfg(not(target_family = "wasm"))]
-impl TryFrom<Address> for ScVal {
-    type Error = ConversionError;
-    fn try_from(v: Address) -> Result<Self, ConversionError> {
-        (&v).try_into()
+impl From<Address> for ScVal {
+    fn from(v: Address) -> Self {
+        (&v).into()
     }
 }
 
@@ -152,21 +155,19 @@ impl TryFromVal<Env, ScVal> for Address {
 }
 
 #[cfg(not(target_family = "wasm"))]
-impl TryFrom<&Address> for ScAddress {
-    type Error = ConversionError;
-    fn try_from(v: &Address) -> Result<Self, Self::Error> {
-        match ScVal::try_from_val(&v.env, &v.obj.to_val())? {
-            ScVal::Address(a) => Ok(a),
-            _ => Err(ConversionError),
+impl From<&Address> for ScAddress {
+    fn from(v: &Address) -> Self {
+        match ScVal::try_from_val(&v.env, &v.obj.to_val()).unwrap() {
+            ScVal::Address(a) => a,
+            _ => panic!("expected ScVal::Address"),
         }
     }
 }
 
 #[cfg(not(target_family = "wasm"))]
-impl TryFrom<Address> for ScAddress {
-    type Error = ConversionError;
-    fn try_from(v: Address) -> Result<Self, Self::Error> {
-        (&v).try_into()
+impl From<Address> for ScAddress {
+    fn from(v: Address) -> Self {
+        (&v).into()
     }
 }
 

diff --git a/soroban-sdk/src/bytes.rs b/soroban-sdk/src/bytes.rs
@@ -232,18 +232,21 @@ impl From<&Bytes> for Bytes {
 }
 
 #[cfg(not(target_family = "wasm"))]
-impl TryFrom<&Bytes> for ScVal {
-    type Error = ConversionError;
-    fn try_from(v: &Bytes) -> Result<Self, ConversionError> {
-        Ok(ScVal::try_from_val(&v.env, &v.obj.to_val())?)
+impl From<&Bytes> for ScVal {
+    fn from(v: &Bytes) -> Self {
+        // This conversion occurs only in test utilities, and theoretically all
+        // values should convert to an ScVal because the Env won't let the host
+        // type to exist otherwise, unwrapping. Even if there are edge cases
+        // that don't, this is a trade off for a better test developer
+        // experience.
+        ScVal::try_from_val(&v.env, &v.obj.to_val()).unwrap()
     }
 }
 
 #[cfg(not(target_family = "wasm"))]
-impl TryFrom<Bytes> for ScVal {
-    type Error = ConversionError;
-    fn try_from(v: Bytes) -> Result<Self, ConversionError> {
-        (&v).try_into()
+impl From<Bytes> for ScVal {
+    fn from(v: Bytes) -> Self {
+        (&v).into()
     }
 }
 

diff --git a/soroban-sdk/src/map.rs b/soroban-sdk/src/map.rs
@@ -225,26 +225,29 @@ where
 }
 
 #[cfg(not(target_family = "wasm"))]
-impl<K, V> TryFrom<&Map<K, V>> for ScVal {
-    type Error = ConversionError;
-    fn try_from(v: &Map<K, V>) -> Result<Self, ConversionError> {
-        Ok(ScVal::try_from_val(&v.env, &v.obj.to_val())?)
+impl<K, V> From<&Map<K, V>> for ScVal {
+    fn from(v: &Map<K, V>) -> Self {
+        // This conversion occurs only in test utilities, and theoretically all
+        // values should convert to an ScVal because the Env won't let the host
+        // type to exist otherwise, unwrapping. Even if there are edge cases
+        // that don't, this is a trade off for a better test developer
+        // experience.
+        ScVal::try_from_val(&v.env, &v.obj.to_val()).unwrap()
     }
 }
 
 #[cfg(not(target_family = "wasm"))]
-impl<K, V> TryFrom<Map<K, V>> for ScVal {
-    type Error = ConversionError;
-    fn try_from(v: Map<K, V>) -> Result<Self, ConversionError> {
-        (&v).try_into()
+impl<K, V> From<Map<K, V>> for ScVal {
+    fn from(v: Map<K, V>) -> Self {
+        (&v).into()
     }
 }
 
 #[cfg(not(target_family = "wasm"))]
 impl<K, V> TryFromVal<Env, Map<K, V>> for ScVal {
     type Error = ConversionError;
     fn try_from_val(_e: &Env, v: &Map<K, V>) -> Result<Self, ConversionError> {
-        v.try_into()
+        Ok(v.into())
     }
 }
 

diff --git a/soroban-sdk/src/num.rs b/soroban-sdk/src/num.rs
@@ -89,22 +89,25 @@ macro_rules! impl_num_wrapping_val_type {
         }
 
         #[cfg(not(target_family = "wasm"))]
-        impl TryFrom<&$wrapper> for ScVal {
-            type Error = ConversionError;
-            fn try_from(v: &$wrapper) -> Result<Self, ConversionError> {
+        impl From<&$wrapper> for ScVal {
+            fn from(v: &$wrapper) -> Self {
+                // This conversion occurs only in test utilities, and theoretically all
+                // values should convert to an ScVal because the Env won't let the host
+                // type to exist otherwise, unwrapping. Even if there are edge cases
+                // that don't, this is a trade off for a better test developer
+                // experience.
                 if let Ok(ss) = <$small>::try_from(v.val) {
-                    ScVal::try_from(ss)
+                    ScVal::try_from(ss).unwrap()
                 } else {
-                    Ok(ScVal::try_from_val(&v.env, &v.to_val())?)
+                    ScVal::try_from_val(&v.env, &v.to_val()).unwrap()
                 }
             }
         }
 
         #[cfg(not(target_family = "wasm"))]
-        impl TryFrom<$wrapper> for ScVal {
-            type Error = ConversionError;
-            fn try_from(v: $wrapper) -> Result<Self, ConversionError> {
-                (&v).try_into()
+        impl From<$wrapper> for ScVal {
+            fn from(v: $wrapper) -> Self {
+                (&v).into()
             }
         }
 

diff --git a/soroban-sdk/src/string.rs b/soroban-sdk/src/string.rs
@@ -137,18 +137,21 @@ impl From<&String> for String {
 }
 
 #[cfg(not(target_family = "wasm"))]
-impl TryFrom<&String> for ScVal {
-    type Error = ConversionError;
-    fn try_from(v: &String) -> Result<Self, ConversionError> {
-        Ok(ScVal::try_from_val(&v.env, &v.obj.to_val())?)
+impl From<&String> for ScVal {
+    fn from(v: &String) -> Self {
+        // This conversion occurs only in test utilities, and theoretically all
+        // values should convert to an ScVal because the Env won't let the host
+        // type to exist otherwise, unwrapping. Even if there are edge cases
+        // that don't, this is a trade off for a better test developer
+        // experience.
+        ScVal::try_from_val(&v.env, &v.obj.to_val()).unwrap()
     }
 }
 
 #[cfg(not(target_family = "wasm"))]
-impl TryFrom<String> for ScVal {
-    type Error = ConversionError;
-    fn try_from(v: String) -> Result<Self, ConversionError> {
-        (&v).try_into()
+impl From<String> for ScVal {
+    fn from(v: String) -> Self {
+        (&v).into()
     }
 }
 

diff --git a/soroban-sdk/src/symbol.rs b/soroban-sdk/src/symbol.rs
@@ -136,31 +136,34 @@ impl TryFromVal<Env, &str> for Symbol {
 }
 
 #[cfg(not(target_family = "wasm"))]
-impl TryFrom<&Symbol> for ScVal {
-    type Error = ConversionError;
-    fn try_from(v: &Symbol) -> Result<Self, ConversionError> {
+impl From<&Symbol> for ScVal {
+    fn from(v: &Symbol) -> Self {
+        // This conversion occurs only in test utilities, and theoretically all
+        // values should convert to an ScVal because the Env won't let the host
+        // type to exist otherwise, unwrapping. Even if there are edge cases
+        // that don't, this is a trade off for a better test developer
+        // experience.
         if let Ok(ss) = SymbolSmall::try_from(v.val) {
-            Ok(ScVal::try_from(ss)?)
+            ScVal::try_from(ss).unwrap()
         } else {
-            let e: Env = v.env.clone().try_into()?;
-            Ok(ScVal::try_from_val(&e, &v.to_val())?)
+            let e: Env = v.env.clone().try_into().unwrap();
+            ScVal::try_from_val(&e, &v.to_val()).unwrap()
         }
     }
 }
 
 #[cfg(not(target_family = "wasm"))]
-impl TryFrom<Symbol> for ScVal {
-    type Error = ConversionError;
-    fn try_from(v: Symbol) -> Result<Self, ConversionError> {
-        (&v).try_into()
+impl From<Symbol> for ScVal {
+    fn from(v: Symbol) -> Self {
+        (&v).into()
     }
 }
 
 #[cfg(not(target_family = "wasm"))]
 impl TryFromVal<Env, Symbol> for ScVal {
     type Error = ConversionError;
     fn try_from_val(_e: &Env, v: &Symbol) -> Result<Self, ConversionError> {
-        v.try_into()
+        Ok(v.into())
     }
 }
 

diff --git a/soroban-sdk/src/vec.rs b/soroban-sdk/src/vec.rs
@@ -231,46 +231,46 @@ where
 use super::xdr::{ScVal, ScVec, VecM};
 
 #[cfg(not(target_family = "wasm"))]
-impl<T> TryFrom<&Vec<T>> for ScVal {
-    type Error = ConversionError;
-    fn try_from(v: &Vec<T>) -> Result<Self, ConversionError> {
-        Ok(ScVal::try_from_val(&v.env, &v.obj.to_val())?)
+impl<T> From<&Vec<T>> for ScVal {
+    fn from(v: &Vec<T>) -> Self {
+        // This conversion occurs only in test utilities, and theoretically all
+        // values should convert to an ScVal because the Env won't let the host
+        // type to exist otherwise, unwrapping. Even if there are edge cases
+        // that don't, this is a trade off for a better test developer
+        // experience.
+        ScVal::try_from_val(&v.env, &v.obj.to_val()).unwrap()
     }
 }
 
 #[cfg(not(target_family = "wasm"))]
-impl<T> TryFrom<&Vec<T>> for ScVec {
-    type Error = ConversionError;
-    fn try_from(v: &Vec<T>) -> Result<Self, ConversionError> {
-        if let ScVal::Vec(Some(vec)) = ScVal::try_from(v)? {
-            Ok(vec)
+impl<T> From<&Vec<T>> for ScVec {
+    fn from(v: &Vec<T>) -> Self {
+        if let ScVal::Vec(Some(vec)) = ScVal::try_from(v).unwrap() {
+            vec
         } else {
-            Err(ConversionError)
+            panic!("expected ScVec")
         }
     }
 }
 
 #[cfg(not(target_family = "wasm"))]
-impl<T> TryFrom<Vec<T>> for VecM<ScVal> {
-    type Error = ConversionError;
-    fn try_from(v: Vec<T>) -> Result<Self, ConversionError> {
-        Ok(ScVec::try_from(v)?.0)
+impl<T> From<Vec<T>> for VecM<ScVal> {
+    fn from(v: Vec<T>) -> Self {
+        ScVec::from(v).0
     }
 }
 
 #[cfg(not(target_family = "wasm"))]
-impl<T> TryFrom<Vec<T>> for ScVal {
-    type Error = ConversionError;
-    fn try_from(v: Vec<T>) -> Result<Self, ConversionError> {
-        (&v).try_into()
+impl<T> From<Vec<T>> for ScVal {
+    fn from(v: Vec<T>) -> Self {
+        (&v).into()
     }
 }
 
 #[cfg(not(target_family = "wasm"))]
-impl<T> TryFrom<Vec<T>> for ScVec {
-    type Error = ConversionError;
-    fn try_from(v: Vec<T>) -> Result<Self, ConversionError> {
-        (&v).try_into()
+impl<T> From<Vec<T>> for ScVec {
+    fn from(v: Vec<T>) -> Self {
+        (&v).into()
     }
 }