From 8fc9f5334dece34b6baab31261a7773295746235 Mon Sep 17 00:00:00 2001 From: Leigh McCulloch <351529+leighmcculloch@users.noreply.github.com> Date: Fri, 27 Sep 2024 07:40:11 +1000 Subject: [PATCH] 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. --- soroban-sdk/src/address.rs | 37 ++++++++++++++++---------------- soroban-sdk/src/bytes.rs | 19 +++++++++------- soroban-sdk/src/map.rs | 21 ++++++++++-------- soroban-sdk/src/num.rs | 21 ++++++++++-------- soroban-sdk/src/string.rs | 19 +++++++++------- soroban-sdk/src/symbol.rs | 25 ++++++++++++---------- soroban-sdk/src/vec.rs | 44 +++++++++++++++++++------------------- 7 files changed, 101 insertions(+), 85 deletions(-) diff --git a/soroban-sdk/src/address.rs b/soroban-sdk/src/address.rs index dfed7f87a..e735a6f23 100644 --- a/soroban-sdk/src/address.rs +++ b/soroban-sdk/src/address.rs @@ -124,18 +124,21 @@ impl TryFromVal for Val { } #[cfg(not(target_family = "wasm"))] -impl TryFrom<&Address> for ScVal { - type Error = ConversionError; - fn try_from(v: &Address) -> Result { - 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
for ScVal { - type Error = ConversionError; - fn try_from(v: Address) -> Result { - (&v).try_into() +impl From
for ScVal { + fn from(v: Address) -> Self { + (&v).into() } } @@ -152,21 +155,19 @@ impl TryFromVal for Address { } #[cfg(not(target_family = "wasm"))] -impl TryFrom<&Address> for ScAddress { - type Error = ConversionError; - fn try_from(v: &Address) -> Result { - 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
for ScAddress { - type Error = ConversionError; - fn try_from(v: Address) -> Result { - (&v).try_into() +impl From
for ScAddress { + fn from(v: Address) -> Self { + (&v).into() } } diff --git a/soroban-sdk/src/bytes.rs b/soroban-sdk/src/bytes.rs index 1605341bd..b654d66a9 100644 --- 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 { - 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 for ScVal { - type Error = ConversionError; - fn try_from(v: Bytes) -> Result { - (&v).try_into() +impl From for ScVal { + fn from(v: Bytes) -> Self { + (&v).into() } } diff --git a/soroban-sdk/src/map.rs b/soroban-sdk/src/map.rs index cf0f3877f..05582a77f 100644 --- a/soroban-sdk/src/map.rs +++ b/soroban-sdk/src/map.rs @@ -225,18 +225,21 @@ where } #[cfg(not(target_family = "wasm"))] -impl TryFrom<&Map> for ScVal { - type Error = ConversionError; - fn try_from(v: &Map) -> Result { - Ok(ScVal::try_from_val(&v.env, &v.obj.to_val())?) +impl From<&Map> for ScVal { + fn from(v: &Map) -> 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> for ScVal { - type Error = ConversionError; - fn try_from(v: Map) -> Result { - (&v).try_into() +impl From> for ScVal { + fn from(v: Map) -> Self { + (&v).into() } } @@ -244,7 +247,7 @@ impl TryFrom> for ScVal { impl TryFromVal> for ScVal { type Error = ConversionError; fn try_from_val(_e: &Env, v: &Map) -> Result { - v.try_into() + Ok(v.into()) } } diff --git a/soroban-sdk/src/num.rs b/soroban-sdk/src/num.rs index 1071943e6..d3f69cbde 100644 --- 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 { + 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 { - (&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 index 1c48ecb30..671624865 100644 --- 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 { - 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 for ScVal { - type Error = ConversionError; - fn try_from(v: String) -> Result { - (&v).try_into() +impl From for ScVal { + fn from(v: String) -> Self { + (&v).into() } } diff --git a/soroban-sdk/src/symbol.rs b/soroban-sdk/src/symbol.rs index 7f11fd3be..6389d382b 100644 --- a/soroban-sdk/src/symbol.rs +++ b/soroban-sdk/src/symbol.rs @@ -136,23 +136,26 @@ impl TryFromVal for Symbol { } #[cfg(not(target_family = "wasm"))] -impl TryFrom<&Symbol> for ScVal { - type Error = ConversionError; - fn try_from(v: &Symbol) -> Result { +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 for ScVal { - type Error = ConversionError; - fn try_from(v: Symbol) -> Result { - (&v).try_into() +impl From for ScVal { + fn from(v: Symbol) -> Self { + (&v).into() } } @@ -160,7 +163,7 @@ impl TryFrom for ScVal { impl TryFromVal for ScVal { type Error = ConversionError; fn try_from_val(_e: &Env, v: &Symbol) -> Result { - v.try_into() + Ok(v.into()) } } diff --git a/soroban-sdk/src/vec.rs b/soroban-sdk/src/vec.rs index 46f66bc9b..d7ae8eefb 100644 --- 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 TryFrom<&Vec> for ScVal { - type Error = ConversionError; - fn try_from(v: &Vec) -> Result { - Ok(ScVal::try_from_val(&v.env, &v.obj.to_val())?) +impl From<&Vec> for ScVal { + fn from(v: &Vec) -> 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<&Vec> for ScVec { - type Error = ConversionError; - fn try_from(v: &Vec) -> Result { - if let ScVal::Vec(Some(vec)) = ScVal::try_from(v)? { - Ok(vec) +impl From<&Vec> for ScVec { + fn from(v: &Vec) -> 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 TryFrom> for VecM { - type Error = ConversionError; - fn try_from(v: Vec) -> Result { - Ok(ScVec::try_from(v)?.0) +impl From> for VecM { + fn from(v: Vec) -> Self { + ScVec::from(v).0 } } #[cfg(not(target_family = "wasm"))] -impl TryFrom> for ScVal { - type Error = ConversionError; - fn try_from(v: Vec) -> Result { - (&v).try_into() +impl From> for ScVal { + fn from(v: Vec) -> Self { + (&v).into() } } #[cfg(not(target_family = "wasm"))] -impl TryFrom> for ScVec { - type Error = ConversionError; - fn try_from(v: Vec) -> Result { - (&v).try_into() +impl From> for ScVec { + fn from(v: Vec) -> Self { + (&v).into() } }