From 30f9862b4b7e239d165256b569aabaa98601bc59 Mon Sep 17 00:00:00 2001 From: dauTT Date: Fri, 25 Sep 2020 10:31:31 +0200 Subject: [PATCH] Remove zero coins from ParseCoins and ParseDecCoins (#7348) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Remove zero coins from ParseCoins and ParseDecCoins * Implement requested changes * Update types/coin_test.go * Revert * Align the behaviour of NewCoins and ParseCoins (respectively NewDecCoins and ParseDecCoins) * Amend comments * Update types/dec_coin.go Co-authored-by: Alessio Treglia Co-authored-by: Alessio Treglia Co-authored-by: SaReN Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com> --- go.sum | 2 -- types/coin.go | 37 +++++++++++++++++++----------------- types/coin_test.go | 5 ++++- types/dec_coin.go | 43 +++++++++++++++++++++++------------------- types/dec_coin_test.go | 10 +++++++++- 5 files changed, 57 insertions(+), 40 deletions(-) diff --git a/go.sum b/go.sum index 104d26245d..7e1283aa33 100644 --- a/go.sum +++ b/go.sum @@ -488,8 +488,6 @@ github.com/prometheus/common v0.7.0/go.mod h1:DjGbpBbp5NYNiECxcL/VnbXCCaQpKd3tt2 github.com/prometheus/common v0.9.1/go.mod h1:yhUN8i9wzaXS3w1O07YhxHEBxD+W35wd8bs7vj7HSQ4= github.com/prometheus/common v0.10.0 h1:RyRA7RzGXQZiW+tGMr7sxa85G1z0yOpM1qq5c8lNawc= github.com/prometheus/common v0.10.0/go.mod h1:Tlit/dnDKsSWFlCLTWaA1cyBgKHSMdTB80sz/V91rCo= -github.com/prometheus/common v0.13.0 h1:vJlpe9wPgDRM1Z+7Wj3zUUjY1nr6/1jNKyl7llliccg= -github.com/prometheus/common v0.13.0/go.mod h1:U+gB1OBLb1lF3O42bTCL+FK18tX9Oar16Clt/msog/s= github.com/prometheus/common v0.14.0 h1:RHRyE8UocrbjU+6UvRzwi6HjiDfxrrBU91TtbKzkGp4= github.com/prometheus/common v0.14.0/go.mod h1:U+gB1OBLb1lF3O42bTCL+FK18tX9Oar16Clt/msog/s= github.com/prometheus/procfs v0.0.0-20181005140218-185b4288413d/go.mod h1:c3At6R/oaqEKCNdg8wHV1ftS6bRYblBhIjjI8uT2IGk= diff --git a/types/coin.go b/types/coin.go index 00ba996271..a2d7364a0f 100644 --- a/types/coin.go +++ b/types/coin.go @@ -135,16 +135,10 @@ func (coin Coin) IsNegative() bool { // Coins is a set of Coin, one per currency type Coins []Coin -// NewCoins constructs a new coin set. +// NewCoins constructs a new coin set. The provided coins will be sanitized by removing +// zero coins and sorting the coin set. A panic will occur if the coin set is not valid. func NewCoins(coins ...Coin) Coins { - // remove zeroes - newCoins := removeZeroCoins(Coins(coins)) - if len(newCoins) == 0 { - return Coins{} - } - - newCoins.Sort() - + newCoins := sanitizeCoins(coins) if err := newCoins.Validate(); err != nil { panic(fmt.Errorf("invalid coin set %s: %w", newCoins, err)) } @@ -152,6 +146,15 @@ func NewCoins(coins ...Coin) Coins { return newCoins } +func sanitizeCoins(coins []Coin) Coins { + newCoins := removeZeroCoins(coins) + if len(newCoins) == 0 { + return Coins{} + } + + return newCoins.Sort() +} + type coinsJSON Coins // MarshalJSON implements a custom JSON marshaller for the Coins type to allow @@ -644,8 +647,11 @@ func ParseCoin(coinStr string) (coin Coin, err error) { return NewCoin(denomStr, amount), nil } -// ParseCoins will parse out a list of coins separated by commas. If nothing is provided, it returns -// nil Coins. If the coins aren't valid they return an error. Returned coins are sorted. +// ParseCoins will parse out a list of coins separated by commas. If the parsing is successuful, +// the provided coins will be sanitized by removing zero coins and sorting the coin set. Lastly +// a validation of the coin set is executed. If the check passes, ParseCoins will return the sanitized coins. +// Otherwise it will return an error. +// If an empty string is provided to ParseCoins, it returns nil Coins. // Expected format: "{amount0}{denomination},...,{amountN}{denominationN}" func ParseCoins(coinsStr string) (Coins, error) { coinsStr = strings.TrimSpace(coinsStr) @@ -664,13 +670,10 @@ func ParseCoins(coinsStr string) (Coins, error) { coins[i] = coin } - // sort coins for determinism - coins.Sort() - - // validate coins before returning - if err := coins.Validate(); err != nil { + newCoins := sanitizeCoins(coins) + if err := newCoins.Validate(); err != nil { return nil, err } - return coins, nil + return newCoins, nil } diff --git a/types/coin_test.go b/types/coin_test.go index 1fc965bd01..fb4b9c4f1f 100644 --- a/types/coin_test.go +++ b/types/coin_test.go @@ -612,7 +612,7 @@ func TestCoinsLTE(t *testing.T) { assert.True(t, Coins{}.IsAllLTE(Coins{{testDenom1, one}})) } -func TestParse(t *testing.T) { +func TestParseCoins(t *testing.T) { one := OneInt() cases := []struct { @@ -621,7 +621,10 @@ func TestParse(t *testing.T) { expected Coins // if valid is true, make sure this is returned }{ {"", true, nil}, + {"0stake", true, Coins{}}, // remove zero coins + {"0stake,1foo,99bar", true, Coins{{"bar", NewInt(99)}, {"foo", one}}}, // remove zero coins {"1foo", true, Coins{{"foo", one}}}, + {"10btc,1atom,20btc", false, nil}, {"10bar", true, Coins{{"bar", NewInt(10)}}}, {"99bar,1foo", true, Coins{{"bar", NewInt(99)}, {"foo", one}}}, {"98 bar , 1 foo ", true, Coins{{"bar", NewInt(98)}, {"foo", one}}}, diff --git a/types/dec_coin.go b/types/dec_coin.go index 89dc716037..6e400d7a05 100644 --- a/types/dec_coin.go +++ b/types/dec_coin.go @@ -158,16 +158,10 @@ func (coin DecCoin) IsValid() bool { type DecCoins []DecCoin // NewDecCoins constructs a new coin set with with decimal values -// from DecCoins. +// from DecCoins. The provided coins will be sanitized by removing +// zero coins and sorting the coin set. A panic will occur if the coin set is not valid. func NewDecCoins(decCoins ...DecCoin) DecCoins { - // remove zeroes - newDecCoins := removeZeroDecCoins(DecCoins(decCoins)) - if len(newDecCoins) == 0 { - return DecCoins{} - } - - newDecCoins.Sort() - + newDecCoins := sanitizeDecCoins(decCoins) if err := newDecCoins.Validate(); err != nil { panic(fmt.Errorf("invalid coin set %s: %w", newDecCoins, err)) } @@ -175,6 +169,16 @@ func NewDecCoins(decCoins ...DecCoin) DecCoins { return newDecCoins } +func sanitizeDecCoins(decCoins []DecCoin) DecCoins { + // remove zeroes + newDecCoins := removeZeroDecCoins(decCoins) + if len(newDecCoins) == 0 { + return DecCoins{} + } + + return newDecCoins.Sort() +} + // NewDecCoinsFromCoins constructs a new coin set with decimal values // from regular Coins. func NewDecCoinsFromCoins(coins ...Coin) DecCoins { @@ -630,8 +634,12 @@ func ParseDecCoin(coinStr string) (coin DecCoin, err error) { return NewDecCoinFromDec(denomStr, amount), nil } -// ParseDecCoins will parse out a list of decimal coins separated by commas. If nothing is provided, -// it returns nil DecCoins. If the coins aren't valid they return an error. Returned coins are sorted. +// ParseDecCoins will parse out a list of decimal coins separated by commas. If the parsing is successuful, +// the provided coins will be sanitized by removing zero coins and sorting the coin set. Lastly +// a validation of the coin set is executed. If the check passes, ParseDecCoins will return the sanitized coins. +// Otherwise it will return an error. +// If an empty string is provided to ParseDecCoins, it returns nil Coins. +// Expected format: "{amount0}{denomination},...,{amountN}{denominationN}" func ParseDecCoins(coinsStr string) (DecCoins, error) { coinsStr = strings.TrimSpace(coinsStr) if len(coinsStr) == 0 { @@ -639,23 +647,20 @@ func ParseDecCoins(coinsStr string) (DecCoins, error) { } coinStrs := strings.Split(coinsStr, ",") - coins := make(DecCoins, len(coinStrs)) + decCoins := make(DecCoins, len(coinStrs)) for i, coinStr := range coinStrs { coin, err := ParseDecCoin(coinStr) if err != nil { return nil, err } - coins[i] = coin + decCoins[i] = coin } - // sort coins for determinism - coins.Sort() - - // validate coins before returning - if err := coins.Validate(); err != nil { + newDecCoins := sanitizeDecCoins(decCoins) + if err := newDecCoins.Validate(); err != nil { return nil, err } - return coins, nil + return newDecCoins, nil } diff --git a/types/dec_coin_test.go b/types/dec_coin_test.go index 821d297566..6eb89e4a73 100644 --- a/types/dec_coin_test.go +++ b/types/dec_coin_test.go @@ -348,7 +348,8 @@ func TestParseDecCoins(t *testing.T) { {"", nil, false}, {"4stake", nil, true}, {"5.5atom,4stake", nil, true}, - {"0.0stake", nil, true}, + {"0.0stake", DecCoins{}, false}, // remove zero coins + {"10.0btc,1.0atom,20.0btc", nil, true}, { "0.004STAKE", DecCoins{NewDecCoinFromDec("STAKE", NewDecWithPrec(4000000000000000, Precision))}, @@ -367,6 +368,13 @@ func TestParseDecCoins(t *testing.T) { }, false, }, + {"0.0stake,0.004stake,5.04atom", // remove zero coins + DecCoins{ + NewDecCoinFromDec("atom", NewDecWithPrec(5040000000000000000, Precision)), + NewDecCoinFromDec("stake", NewDecWithPrec(4000000000000000, Precision)), + }, + false, + }, } for i, tc := range testCases {