Duplicate coins allowed in ParseCoins

[colin-axner]

Summary

Duplicate coins are not checked for in ParseCoins, but they are in NewCoins

Problem Definition

It was not specified in the godoc if this is intentional so I thought I would open up an issue. There exist inconsistencies between using parse and new. Parsing coins calls parse coin which calls NewCoin which will panic on invalid coin denoms, but it doesn't call NewCoins which panics on dup denoms.

Proposal

Update ParseCoins to return an error on duplicate denoms (and perhaps remove zero coins as well?). Maybe there are reasons when want zero and dup coins to be parsed?

OR

Update godoc to specify more detailed reasoning on the inconsistencies between Parse and New validation and when these should be appropriately used or what functions they should be used in conjunction with (IsValid). I noticed this discrepancy when reviewing relayer code that could have easily just parsed the coins without doing validation checks and then passed this as an msg arg. It would be bad if an application made the validation check assumption with parsing.

---

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

[alexanderbez]

I'm not sure if there was rhyme or reason behind that, but probably it's just a bug. We need to be consistent above all else, so if Coins removes duplicates, then so should DecCoins (or vice versa).