From cf70538073c834d3e50265266e4b9b113d934c17 Mon Sep 17 00:00:00 2001 From: Christoph Otter Date: Thu, 4 May 2023 15:35:51 +0200 Subject: [PATCH 1/6] Add FromStr implementation for Coin, closes #1575 --- packages/std/src/coin.rs | 58 ++++++++++++++++++++++++++++++++++++++-- 1 file changed, 56 insertions(+), 2 deletions(-) diff --git a/packages/std/src/coin.rs b/packages/std/src/coin.rs index 289911deef..cfaf748fda 100644 --- a/packages/std/src/coin.rs +++ b/packages/std/src/coin.rs @@ -1,8 +1,8 @@ use schemars::JsonSchema; use serde::{Deserialize, Serialize}; -use std::fmt; +use std::{fmt, str::FromStr}; -use crate::math::Uint128; +use crate::{math::Uint128, StdError}; #[derive(Serialize, Deserialize, Clone, Default, Debug, PartialEq, Eq, JsonSchema)] pub struct Coin { @@ -19,6 +19,25 @@ impl Coin { } } +impl FromStr for Coin { + type Err = StdError; + + fn from_str(s: &str) -> Result { + let pos = s + .find(|c: char| !c.is_ascii_digit()) + .ok_or_else(|| StdError::generic_err("Parsing Coin: missing denominator"))?; + let (amount, denom) = s.split_at(pos); + + match amount.parse::() { + Ok(amount) => Ok(Coin { + amount: amount.into(), + denom: denom.to_string(), + }), + Err(e) => Err(StdError::generic_err(format!("Parsing Coin: {}", e))), + } + } +} + impl fmt::Display for Coin { fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { // We use the formatting without a space between amount and denom, @@ -166,4 +185,39 @@ mod tests { // less than same type assert!(has_coins(&wallet, &coin(777, "ETH"))); } + + #[test] + fn parse_coin() { + let expected = Coin::new(123, "ucosm"); + assert_eq!(expected, "123ucosm".parse().unwrap()); + // leading zeroes should be ignored + assert_eq!(expected, "00123ucosm".parse().unwrap()); + // 0 amount parses correctly + assert_eq!(Coin::new(0, "ucosm"), "0ucosm".parse().unwrap()); + // ibc denom should work + let ibc_str = "11111ibc/27394FB092D2ECCD56123C74F36E4C1F926001CEADA9CA97EA622B25F41E5EB2"; + let ibc_coin = Coin::new( + 11111, + "ibc/27394FB092D2ECCD56123C74F36E4C1F926001CEADA9CA97EA622B25F41E5EB2", + ); + assert_eq!(ibc_coin, ibc_str.parse().unwrap()); + + // error cases + assert_eq!( + StdError::generic_err("Parsing Coin: missing denominator"), + Coin::from_str("123").unwrap_err() + ); + assert_eq!( + StdError::generic_err("Parsing Coin: cannot parse integer from empty string"), + Coin::from_str("ucosm").unwrap_err() + ); + assert_eq!( + StdError::generic_err("Parsing Coin: cannot parse integer from empty string"), + Coin::from_str("-123ucosm").unwrap_err() + ); + assert_eq!( + StdError::generic_err("Parsing Coin: number too large to fit in target type"), + Coin::from_str("340282366920938463463374607431768211456ucosm").unwrap_err() + ); + } } From eb341dc6106b04b4b7ad2c125ba7d23aae9b7efb Mon Sep 17 00:00:00 2001 From: Christoph Otter Date: Thu, 4 May 2023 16:12:11 +0200 Subject: [PATCH 2/6] Swap expected and actual value in assertions --- packages/std/src/coin.rs | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/packages/std/src/coin.rs b/packages/std/src/coin.rs index cfaf748fda..baabe24110 100644 --- a/packages/std/src/coin.rs +++ b/packages/std/src/coin.rs @@ -189,35 +189,35 @@ mod tests { #[test] fn parse_coin() { let expected = Coin::new(123, "ucosm"); - assert_eq!(expected, "123ucosm".parse().unwrap()); + assert_eq!("123ucosm".parse::().unwrap(), expected); // leading zeroes should be ignored - assert_eq!(expected, "00123ucosm".parse().unwrap()); + assert_eq!("00123ucosm".parse::().unwrap(), expected); // 0 amount parses correctly - assert_eq!(Coin::new(0, "ucosm"), "0ucosm".parse().unwrap()); + assert_eq!("0ucosm".parse::().unwrap(), Coin::new(0, "ucosm")); // ibc denom should work let ibc_str = "11111ibc/27394FB092D2ECCD56123C74F36E4C1F926001CEADA9CA97EA622B25F41E5EB2"; let ibc_coin = Coin::new( 11111, "ibc/27394FB092D2ECCD56123C74F36E4C1F926001CEADA9CA97EA622B25F41E5EB2", ); - assert_eq!(ibc_coin, ibc_str.parse().unwrap()); + assert_eq!(ibc_str.parse::().unwrap(), ibc_coin); // error cases assert_eq!( - StdError::generic_err("Parsing Coin: missing denominator"), - Coin::from_str("123").unwrap_err() + Coin::from_str("123").unwrap_err(), + StdError::generic_err("Parsing Coin: missing denominator") ); assert_eq!( - StdError::generic_err("Parsing Coin: cannot parse integer from empty string"), - Coin::from_str("ucosm").unwrap_err() + Coin::from_str("ucosm").unwrap_err(), + StdError::generic_err("Parsing Coin: cannot parse integer from empty string") ); assert_eq!( - StdError::generic_err("Parsing Coin: cannot parse integer from empty string"), - Coin::from_str("-123ucosm").unwrap_err() + Coin::from_str("-123ucosm").unwrap_err(), + StdError::generic_err("Parsing Coin: cannot parse integer from empty string") ); assert_eq!( - StdError::generic_err("Parsing Coin: number too large to fit in target type"), - Coin::from_str("340282366920938463463374607431768211456ucosm").unwrap_err() + Coin::from_str("340282366920938463463374607431768211456ucosm").unwrap_err(), + StdError::generic_err("Parsing Coin: number too large to fit in target type") ); } } From b81d90da82496e6b2d22b34e7ff40dee059484c5 Mon Sep 17 00:00:00 2001 From: Christoph Otter Date: Thu, 4 May 2023 17:17:09 +0200 Subject: [PATCH 3/6] Add special error type for Coin's FromStr --- packages/std/src/coin.rs | 33 ++++++++++++++-------------- packages/std/src/errors/mod.rs | 2 +- packages/std/src/errors/std_error.rs | 20 +++++++++++++++++ 3 files changed, 37 insertions(+), 18 deletions(-) diff --git a/packages/std/src/coin.rs b/packages/std/src/coin.rs index baabe24110..7df95c87bc 100644 --- a/packages/std/src/coin.rs +++ b/packages/std/src/coin.rs @@ -2,7 +2,7 @@ use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use std::{fmt, str::FromStr}; -use crate::{math::Uint128, StdError}; +use crate::{errors::CoinFromStrError, math::Uint128, StdError}; #[derive(Serialize, Deserialize, Clone, Default, Debug, PartialEq, Eq, JsonSchema)] pub struct Coin { @@ -20,21 +20,18 @@ impl Coin { } impl FromStr for Coin { - type Err = StdError; + type Err = CoinFromStrError; fn from_str(s: &str) -> Result { let pos = s .find(|c: char| !c.is_ascii_digit()) - .ok_or_else(|| StdError::generic_err("Parsing Coin: missing denominator"))?; + .ok_or(CoinFromStrError::MissingDenom)?; let (amount, denom) = s.split_at(pos); - match amount.parse::() { - Ok(amount) => Ok(Coin { - amount: amount.into(), - denom: denom.to_string(), - }), - Err(e) => Err(StdError::generic_err(format!("Parsing Coin: {}", e))), - } + Ok(Coin { + amount: amount.parse::()?.into(), + denom: denom.to_string(), + }) } } @@ -205,19 +202,21 @@ mod tests { // error cases assert_eq!( Coin::from_str("123").unwrap_err(), - StdError::generic_err("Parsing Coin: missing denominator") + CoinFromStrError::MissingDenom ); assert_eq!( - Coin::from_str("ucosm").unwrap_err(), - StdError::generic_err("Parsing Coin: cannot parse integer from empty string") + Coin::from_str("ucosm").unwrap_err().to_string(), + "Invalid amount: cannot parse integer from empty string" ); assert_eq!( - Coin::from_str("-123ucosm").unwrap_err(), - StdError::generic_err("Parsing Coin: cannot parse integer from empty string") + Coin::from_str("-123ucosm").unwrap_err().to_string(), + "Invalid amount: cannot parse integer from empty string" ); assert_eq!( - Coin::from_str("340282366920938463463374607431768211456ucosm").unwrap_err(), - StdError::generic_err("Parsing Coin: number too large to fit in target type") + Coin::from_str("340282366920938463463374607431768211456ucosm") + .unwrap_err() + .to_string(), + "Invalid amount: number too large to fit in target type" ); } } diff --git a/packages/std/src/errors/mod.rs b/packages/std/src/errors/mod.rs index 705382b732..5535479bdf 100644 --- a/packages/std/src/errors/mod.rs +++ b/packages/std/src/errors/mod.rs @@ -6,7 +6,7 @@ mod verification_error; pub use recover_pubkey_error::RecoverPubkeyError; pub use std_error::{ CheckedFromRatioError, CheckedMultiplyFractionError, CheckedMultiplyRatioError, - ConversionOverflowError, DivideByZeroError, OverflowError, OverflowOperation, + CoinFromStrError, ConversionOverflowError, DivideByZeroError, OverflowError, OverflowOperation, RoundUpOverflowError, StdError, StdResult, }; pub use system_error::SystemError; diff --git a/packages/std/src/errors/std_error.rs b/packages/std/src/errors/std_error.rs index 79b6a82f27..7093167590 100644 --- a/packages/std/src/errors/std_error.rs +++ b/packages/std/src/errors/std_error.rs @@ -590,6 +590,26 @@ pub enum CheckedFromRatioError { #[error("Round up operation failed because of overflow")] pub struct RoundUpOverflowError; +#[derive(Error, Debug, PartialEq, Eq)] +pub enum CoinFromStrError { + #[error("Missing denominator")] + MissingDenom, + #[error("Invalid amount: {0}")] + InvalidAmount(std::num::ParseIntError), +} + +impl From for CoinFromStrError { + fn from(value: std::num::ParseIntError) -> Self { + Self::InvalidAmount(value) + } +} + +impl From for StdError { + fn from(value: CoinFromStrError) -> Self { + Self::generic_err(format!("Parsing Coin: {}", value)) + } +} + #[cfg(test)] mod tests { use super::*; From 8297a7a8af519ea1faf2ba9dde177cc7b5d62eb9 Mon Sep 17 00:00:00 2001 From: Christoph Otter Date: Thu, 4 May 2023 17:26:32 +0200 Subject: [PATCH 4/6] Fix lint --- packages/std/src/coin.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/std/src/coin.rs b/packages/std/src/coin.rs index 7df95c87bc..9c8798a005 100644 --- a/packages/std/src/coin.rs +++ b/packages/std/src/coin.rs @@ -2,7 +2,7 @@ use schemars::JsonSchema; use serde::{Deserialize, Serialize}; use std::{fmt, str::FromStr}; -use crate::{errors::CoinFromStrError, math::Uint128, StdError}; +use crate::{errors::CoinFromStrError, math::Uint128}; #[derive(Serialize, Deserialize, Clone, Default, Debug, PartialEq, Eq, JsonSchema)] pub struct Coin { From bba7a9627d5176f35c0d517315a6133fd3250816 Mon Sep 17 00:00:00 2001 From: Christoph Otter Date: Fri, 5 May 2023 09:38:41 +0200 Subject: [PATCH 5/6] Add changelog entry --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a512a2112c..c71c5ecbc2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,8 +11,10 @@ and this project adheres to - cosmwasm-vm: Add `Cache::save_wasm_unchecked` to save Wasm blobs that have been checked before. This is useful for state-sync where we know the Wasm code was checked when it was first uploaded. ([#1635]) +- cosmwasm-std: Add `FromStr` impl for `Coin`. ([#1684]) [#1635]: https://github.com/CosmWasm/cosmwasm/pull/1635 +[#1684]: https://github.com/CosmWasm/cosmwasm/pull/1684 ### Changed From 66d026bcda634f7d81892670531e6d1d5047eb73 Mon Sep 17 00:00:00 2001 From: Christoph Otter Date: Mon, 15 May 2023 13:04:01 +0200 Subject: [PATCH 6/6] Check for missing amount when parsing Coin --- packages/std/src/coin.rs | 24 ++++++++++++++++++++---- packages/std/src/errors/std_error.rs | 2 ++ 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/packages/std/src/coin.rs b/packages/std/src/coin.rs index 9c8798a005..b88a6da3ee 100644 --- a/packages/std/src/coin.rs +++ b/packages/std/src/coin.rs @@ -28,6 +28,10 @@ impl FromStr for Coin { .ok_or(CoinFromStrError::MissingDenom)?; let (amount, denom) = s.split_at(pos); + if amount.is_empty() { + return Err(CoinFromStrError::MissingAmount); + } + Ok(Coin { amount: amount.parse::()?.into(), denom: denom.to_string(), @@ -205,12 +209,24 @@ mod tests { CoinFromStrError::MissingDenom ); assert_eq!( - Coin::from_str("ucosm").unwrap_err().to_string(), - "Invalid amount: cannot parse integer from empty string" + Coin::from_str("ucosm").unwrap_err(), // no amount + CoinFromStrError::MissingAmount + ); + assert_eq!( + Coin::from_str("-123ucosm").unwrap_err(), // negative amount + CoinFromStrError::MissingAmount + ); + assert_eq!( + Coin::from_str("").unwrap_err(), // empty input + CoinFromStrError::MissingDenom + ); + assert_eq!( + Coin::from_str(" 1ucosm").unwrap_err(), // unsupported whitespace + CoinFromStrError::MissingAmount ); assert_eq!( - Coin::from_str("-123ucosm").unwrap_err().to_string(), - "Invalid amount: cannot parse integer from empty string" + Coin::from_str("�1ucosm").unwrap_err(), // other broken data + CoinFromStrError::MissingAmount ); assert_eq!( Coin::from_str("340282366920938463463374607431768211456ucosm") diff --git a/packages/std/src/errors/std_error.rs b/packages/std/src/errors/std_error.rs index 7093167590..d90171c74c 100644 --- a/packages/std/src/errors/std_error.rs +++ b/packages/std/src/errors/std_error.rs @@ -594,6 +594,8 @@ pub struct RoundUpOverflowError; pub enum CoinFromStrError { #[error("Missing denominator")] MissingDenom, + #[error("Missing amount or non-digit characters in amount")] + MissingAmount, #[error("Invalid amount: {0}")] InvalidAmount(std::num::ParseIntError), }