Add helpers to stringify coins

[ethanfrey]

I have this in some contracts to produce events. Maybe this would be a useful helper in cosmwasm_std?

#[inline]
fn coin_to_string(coin: &Coin) -> String {
    format!("{} {}", c.amount, c.denom)
}

fn coins_to_string(coins: &[Coin]) -> String {
    let strings: Vec<_> = coins
        .iter()
        .map(coin_to_string)
        .collect();
    strings.join(",")
}

[webmaster128]

The first one looks like a very good candidate for a Display implementation of Coin.

For the second one I'd rather wait for https://doc.rust-lang.org/std/slice/trait.Join.html, which can cover the boilerplate. The separator is mostly caller specific I think. For human readable outputs you'd usually prefer ", ".

[maurolacy]

The separator is mostly caller specific I think. For human readable outputs you'd susually prefer ", ".

Given that this is amount and denom, " " makes sense as a separator, i.e. 1 ATOM.

[webmaster128]

Given that this is amount and denom, " " makes sense as a separator, i.e. 1 ATOM.

Right. I'm referring to the element separator in coins_to_string

[webmaster128]

But even for a Coin I'm not sure anymore. They are written as 87154861534uatom without space in many other places in the Cosmos ecosystem. Do we want 87154861534 uatom for a CosmWasm Coin?

[maurolacy]

If that's a "standard", I guess sticking to it makes sense.

[maurolacy]

Creating a coin denom that starts with numbers (i.e 00coin) may confuse some parsers. I guess this is just for logging / display. So, no big deal.

[webmaster128]

Looking at https://github.com/cosmos/cosmos-sdk/blob/master/types/coin.go we seem to get "guarantee" that the denom does not start with a digit. Also note Expected format: "{amount}{denomination}" and Expected format: "{amount0}{denomination},...,{amountN}{denominationN}" for the parsers. I guess it helps to align with that.

[ethanfrey]

I agree with implementing Display for Coin. Ideally using something equivalent to format!("{} {}", c.amount, c.denom) (I had it without space until Mauro's PR comment - I agree it looks nice his way, but sticking with sdk standard is good.

[webmaster128]

I'm thinking about the second part again. We could add and maintain it here. This gives us the opportunity to optimize the implementation in a way that you don't want every contract developer to be aware of. Collecting into Vec<String> and using join created n+1 heap allocated strings. If we implement it via a loop utilizing Display, we only need a single String. See https://stackoverflow.com/questions/28333612/how-can-i-append-a-formatted-string-to-an-existing-string

[ethanfrey]

Good point with optimization.
But after updating the contracts, I realized that in practice we only ever have one coin, so coin.to_string() is plenty.

Happy if you want to add it, but it seems not so important as impl Display for Coin (which also gives a standard format)

[webmaster128]

An optimized version is here:

/// Creates a comma separated string representation of the Display representation of the coins.
///
/// E.g. "" for the empty list, "123ustake" for a single element list and "123ucosm,8910ustake" for two elements.
pub fn coins_to_string(coins: &[Coin]) -> String {
    let separators = coins.len().saturating_sub(1);
    let mut out = String::with_capacity(coins.len() * 20 + separators);
    let mut first = true;
    for coin in coins {
        if !first {
            out.push(',');
        } else {
            first = false;
        }
        write!(out, "{}", coin).unwrap(); // See https://stackoverflow.com/a/28333723/2013738 for why unwrap is okay
    }
    out
}

However, the smaller the API the better. We have to maintain this stuff basically forever. If we don't need it, let's not add it.