Skip to content

Commit

Permalink
Add infallible From conversions for types to ScVal (#1338)
Browse files Browse the repository at this point in the history
### 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.

(cherry picked from commit 8fc9f53)
  • Loading branch information
leighmcculloch committed Oct 21, 2024
1 parent 68411e7 commit e8732eb
Show file tree
Hide file tree
Showing 7 changed files with 101 additions and 85 deletions.
37 changes: 19 additions & 18 deletions soroban-sdk/src/address.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
}

Expand All @@ -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()
}
}

Expand Down
19 changes: 11 additions & 8 deletions soroban-sdk/src/bytes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
}

Expand Down
21 changes: 12 additions & 9 deletions soroban-sdk/src/map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
}

Expand Down
21 changes: 12 additions & 9 deletions soroban-sdk/src/num.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
}

Expand Down
19 changes: 11 additions & 8 deletions soroban-sdk/src/string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
}

Expand Down
25 changes: 14 additions & 11 deletions soroban-sdk/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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())
}
}

Expand Down
44 changes: 22 additions & 22 deletions soroban-sdk/src/vec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
}
}

Expand Down

0 comments on commit e8732eb

Please sign in to comment.