Add cosmwasm_std::Coins struct #1489
Conversation
| // the map having a different length from the vec means the vec must either | ||
| // 1) contain duplicate denoms, or 2) contain zero amounts | ||
| if map.len() != vec_len { | ||
| return Err(StdError::parse_err( | ||
| type_name::<Self>(), | ||
| "duplicate denoms or zero amount", | ||
| )); | ||
| } |
There was a problem hiding this comment.
Why not combine the duplicates and discard the zero amounts? It's more of a friendlier handling versus a throw.
There was a problem hiding this comment.
I'm also leaning towards a solution where Vec<Coin> -> Coins cannot fail. This can be achieved by just defining the semantics to be overriding old values. Vec<Coin> is typically some trusted value anyways.
There was a problem hiding this comment.
Vec<Coin>is typically some trusted value anyways
Wouldn't this be an argument for fail-able casting instead of against it? Since Vec<Coin> comes from trusted sources, it having dups or zero amounts usually indicates there are serious bugs in the source or in contract execution. In this case I think an error is the preferred behavior.
There was a problem hiding this comment.
If you put it that way, yes, it would be an argument for an assertion. Something is seriously wrong, an error case no one is prepared to handle. So it could be implemented as a panic.
Closes: #1377
@webmaster128 I accidentally closed #1487 when doing a force-push (I got confused with git rebasing and the commit history became very messed up) so here's another PR.