Skip to content

Commit

Permalink
cli: convert coins to smallest unit registered (#7777)
Browse files Browse the repository at this point in the history
* cli: convert coins to smallest unit registered

fixes: #7623

- test order of decimal operations
- support both int and decimal coins, truncate when normalizing to
  base unit

* Update types/coin_test.go

* Update types/coin_test.go

Co-authored-by: Alessio Treglia <[email protected]>
Co-authored-by: Amaury <[email protected]>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
  • Loading branch information
4 people authored Nov 16, 2020
1 parent 0d0c969 commit 54201d1
Show file tree
Hide file tree
Showing 17 changed files with 225 additions and 53 deletions.
2 changes: 1 addition & 1 deletion client/tx/factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ func (f Factory) WithGas(gas uint64) Factory {

// WithFees returns a copy of the Factory with an updated fee.
func (f Factory) WithFees(fees string) Factory {
parsedFees, err := sdk.ParseCoins(fees)
parsedFees, err := sdk.ParseCoinsNormalized(fees)
if err != nil {
panic(err)
}
Expand Down
4 changes: 2 additions & 2 deletions simapp/simd/cmd/genaccounts.go
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ contain valid denominations. Accounts may optionally be supplied with vesting pa
addr = info.GetAddress()
}

coins, err := sdk.ParseCoins(args[1])
coins, err := sdk.ParseCoinsNormalized(args[1])
if err != nil {
return fmt.Errorf("failed to parse coins: %w", err)
}
Expand All @@ -76,7 +76,7 @@ contain valid denominations. Accounts may optionally be supplied with vesting pa
vestingEnd, _ := cmd.Flags().GetInt64(flagVestingEnd)
vestingAmtStr, _ := cmd.Flags().GetString(flagVestingAmt)

vestingAmt, err := sdk.ParseCoins(vestingAmtStr)
vestingAmt, err := sdk.ParseCoinsNormalized(vestingAmtStr)
if err != nil {
return fmt.Errorf("failed to parse vesting amount: %w", err)
}
Expand Down
39 changes: 12 additions & 27 deletions types/coin.go
Original file line number Diff line number Diff line change
Expand Up @@ -600,7 +600,7 @@ var (
// a letter, a number or a separator ('/').
reDnmString = `[a-zA-Z][a-zA-Z0-9/]{2,127}`
reAmt = `[[:digit:]]+`
reDecAmt = `[[:digit:]]*\.[[:digit:]]+`
reDecAmt = `[[:digit:]]+(?:\.[[:digit:]]+)?|\.[[:digit:]]+`
reSpc = `[[:space:]]*`
reDnm = returnReDnm
reCoin = returnReCoin
Expand Down Expand Up @@ -664,33 +664,18 @@ 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 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.
// ParseCoinsNormalized will parse out a list of coins separated by commas, and normalize them by converting to smallest
// unit. 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, ParseCoinsNormalized will return the
// sanitized coins.
// Otherwise it will return an error.
// If an empty string is provided to ParseCoins, it returns nil Coins.
// If an empty string is provided to ParseCoinsNormalized, it returns nil Coins.
// ParseCoinsNormalized supports decimal coins as inputs, and truncate them to int after converted to smallest unit.
// Expected format: "{amount0}{denomination},...,{amountN}{denominationN}"
func ParseCoins(coinsStr string) (Coins, error) {
coinsStr = strings.TrimSpace(coinsStr)
if len(coinsStr) == 0 {
return nil, nil
}

coinStrs := strings.Split(coinsStr, ",")
coins := make(Coins, len(coinStrs))
for i, coinStr := range coinStrs {
coin, err := ParseCoin(coinStr)
if err != nil {
return nil, err
}

coins[i] = coin
func ParseCoinsNormalized(coinStr string) (Coins, error) {
coins, err := ParseDecCoins(coinStr)
if err != nil {
return Coins{}, err
}

newCoins := sanitizeCoins(coins)
if err := newCoins.Validate(); err != nil {
return nil, err
}

return newCoins, nil
return NormalizeCoins(coins), nil
}
12 changes: 6 additions & 6 deletions types/coin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -660,18 +660,18 @@ func (s *coinTestSuite) TestParseCoins() {
{"98 bar , 1 foo ", true, sdk.Coins{{"bar", sdk.NewInt(98)}, {"foo", one}}},
{" 55\t \t bling\n", true, sdk.Coins{{"bling", sdk.NewInt(55)}}},
{"2foo, 97 bar", true, sdk.Coins{{"bar", sdk.NewInt(97)}, {"foo", sdk.NewInt(2)}}},
{"5 mycoin,", false, nil}, // no empty coins in a list
{"2 3foo, 97 bar", false, nil}, // 3foo is invalid coin name
{"11me coin, 12you coin", false, nil}, // no spaces in coin names
{"1.2btc", false, nil}, // amount must be integer
{"5foo:bar", false, nil}, // invalid separator
{"5 mycoin,", false, nil}, // no empty coins in a list
{"2 3foo, 97 bar", false, nil}, // 3foo is invalid coin name
{"11me coin, 12you coin", false, nil}, // no spaces in coin names
{"1.2btc", true, sdk.Coins{{"btc", sdk.NewInt(1)}}}, // amount can be decimal, will get truncated
{"5foo:bar", false, nil}, // invalid separator
{"10atom10", true, sdk.Coins{{"atom10", sdk.NewInt(10)}}},
{"200transfer/channelToA/uatom", true, sdk.Coins{{"transfer/channelToA/uatom", sdk.NewInt(200)}}},
{"50ibc/7F1D3FCF4AE79E1554D670D1AD949A9BA4E4A3C76C63093E17E446A46061A7A2", true, sdk.Coins{{"ibc/7F1D3FCF4AE79E1554D670D1AD949A9BA4E4A3C76C63093E17E446A46061A7A2", sdk.NewInt(50)}}},
}

for tcIndex, tc := range cases {
res, err := sdk.ParseCoins(tc.input)
res, err := sdk.ParseCoinsNormalized(tc.input)
if !tc.valid {
s.Require().Error(err, "%s: %#v. tc #%d", tc.input, res, tcIndex)
} else if s.Assert().Nil(err, "%s: %+v", tc.input, err) {
Expand Down
7 changes: 5 additions & 2 deletions types/dec_coin_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -352,8 +352,11 @@ func (s *decCoinTestSuite) TestParseDecCoins() {
expectedErr bool
}{
{"", nil, false},
{"4stake", nil, true},
{"5.5atom,4stake", nil, true},
{"4stake", sdk.DecCoins{sdk.NewDecCoinFromDec("stake", sdk.NewDecFromInt(sdk.NewInt(4)))}, false},
{"5.5atom,4stake", sdk.DecCoins{
sdk.NewDecCoinFromDec("atom", sdk.NewDecWithPrec(5500000000000000000, sdk.Precision)),
sdk.NewDecCoinFromDec("stake", sdk.NewDec(4)),
}, false},
{"0.0stake", sdk.DecCoins{}, false}, // remove zero coins
{"10.0btc,1.0atom,20.0btc", nil, true},
{
Expand Down
85 changes: 84 additions & 1 deletion types/denom.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,9 @@ import (
// multipliers (e.g. 1atom = 10^-6uatom).
var denomUnits = map[string]Dec{}

// baseDenom is the denom of smallest unit registered
var baseDenom string = ""

// RegisterDenom registers a denomination with a corresponding unit. If the
// denomination is already registered, an error will be returned.
func RegisterDenom(denom string, unit Dec) error {
Expand All @@ -20,6 +23,10 @@ func RegisterDenom(denom string, unit Dec) error {
}

denomUnits[denom] = unit

if baseDenom == "" || unit.LT(denomUnits[baseDenom]) {
baseDenom = denom
}
return nil
}

Expand All @@ -38,6 +45,14 @@ func GetDenomUnit(denom string) (Dec, bool) {
return unit, true
}

// GetBaseDenom returns the denom of smallest unit registered
func GetBaseDenom() (string, error) {
if baseDenom == "" {
return "", fmt.Errorf("no denom is registered")
}
return baseDenom, nil
}

// ConvertCoin attempts to convert a coin to a given denomination. If the given
// denomination is invalid or if neither denomination is registered, an error
// is returned.
Expand All @@ -60,5 +75,73 @@ func ConvertCoin(coin Coin, denom string) (Coin, error) {
return NewCoin(denom, coin.Amount), nil
}

return NewCoin(denom, coin.Amount.ToDec().Mul(srcUnit.Quo(dstUnit)).TruncateInt()), nil
return NewCoin(denom, coin.Amount.ToDec().Mul(srcUnit).Quo(dstUnit).TruncateInt()), nil
}

// ConvertDecCoin attempts to convert a decimal coin to a given denomination. If the given
// denomination is invalid or if neither denomination is registered, an error
// is returned.
func ConvertDecCoin(coin DecCoin, denom string) (DecCoin, error) {
if err := ValidateDenom(denom); err != nil {
return DecCoin{}, err
}

srcUnit, ok := GetDenomUnit(coin.Denom)
if !ok {
return DecCoin{}, fmt.Errorf("source denom not registered: %s", coin.Denom)
}

dstUnit, ok := GetDenomUnit(denom)
if !ok {
return DecCoin{}, fmt.Errorf("destination denom not registered: %s", denom)
}

if srcUnit.Equal(dstUnit) {
return NewDecCoinFromDec(denom, coin.Amount), nil
}

return NewDecCoinFromDec(denom, coin.Amount.Mul(srcUnit).Quo(dstUnit)), nil
}

// NormalizeCoin try to convert a coin to the smallest unit registered,
// returns original one if failed.
func NormalizeCoin(coin Coin) Coin {
base, err := GetBaseDenom()
if err != nil {
return coin
}
newCoin, err := ConvertCoin(coin, base)
if err != nil {
return coin
}
return newCoin
}

// NormalizeDecCoin try to convert a decimal coin to the smallest unit registered,
// returns original one if failed.
func NormalizeDecCoin(coin DecCoin) DecCoin {
base, err := GetBaseDenom()
if err != nil {
return coin
}
newCoin, err := ConvertDecCoin(coin, base)
if err != nil {
return coin
}
return newCoin
}

// NormalizeCoins normalize and truncate a list of decimal coins
func NormalizeCoins(coins []DecCoin) Coins {
if coins == nil {
return nil
}
result := make([]Coin, 0, len(coins))

for _, coin := range coins {
newCoin, _ := NormalizeDecCoin(coin).TruncateDecimal()
result = append(result, newCoin)
}

return result
}
101 changes: 101 additions & 0 deletions types/denom_internal_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ func (s *internalDenomTestSuite) TestRegisterDenom() {
s.Require().Equal(ZeroDec(), res)

// reset registration
baseDenom = ""
denomUnits = map[string]Dec{}
}

Expand All @@ -52,6 +53,21 @@ func (s *internalDenomTestSuite) TestConvertCoins() {
natomUnit := NewDecWithPrec(1, 9) // 10^-9 (nano)
s.Require().NoError(RegisterDenom(natom, natomUnit))

res, err := GetBaseDenom()
s.Require().NoError(err)
s.Require().Equal(res, natom)
s.Require().Equal(NormalizeCoin(NewCoin(uatom, NewInt(1))), NewCoin(natom, NewInt(1000)))
s.Require().Equal(NormalizeCoin(NewCoin(matom, NewInt(1))), NewCoin(natom, NewInt(1000000)))
s.Require().Equal(NormalizeCoin(NewCoin(atom, NewInt(1))), NewCoin(natom, NewInt(1000000000)))

coins, err := ParseCoinsNormalized("1atom,1matom,1uatom")
s.Require().NoError(err)
s.Require().Equal(coins, Coins{
Coin{natom, NewInt(1000000000)},
Coin{natom, NewInt(1000000)},
Coin{natom, NewInt(1000)},
})

testCases := []struct {
input Coin
denom string
Expand Down Expand Up @@ -87,5 +103,90 @@ func (s *internalDenomTestSuite) TestConvertCoins() {
}

// reset registration
baseDenom = ""
denomUnits = map[string]Dec{}
}

func (s *internalDenomTestSuite) TestConvertDecCoins() {
atomUnit := OneDec() // 1 (base denom unit)
s.Require().NoError(RegisterDenom(atom, atomUnit))

matomUnit := NewDecWithPrec(1, 3) // 10^-3 (milli)
s.Require().NoError(RegisterDenom(matom, matomUnit))

uatomUnit := NewDecWithPrec(1, 6) // 10^-6 (micro)
s.Require().NoError(RegisterDenom(uatom, uatomUnit))

natomUnit := NewDecWithPrec(1, 9) // 10^-9 (nano)
s.Require().NoError(RegisterDenom(natom, natomUnit))

res, err := GetBaseDenom()
s.Require().NoError(err)
s.Require().Equal(res, natom)
s.Require().Equal(NormalizeDecCoin(NewDecCoin(uatom, NewInt(1))), NewDecCoin(natom, NewInt(1000)))
s.Require().Equal(NormalizeDecCoin(NewDecCoin(matom, NewInt(1))), NewDecCoin(natom, NewInt(1000000)))
s.Require().Equal(NormalizeDecCoin(NewDecCoin(atom, NewInt(1))), NewDecCoin(natom, NewInt(1000000000)))

coins, err := ParseCoinsNormalized("0.1atom,0.1matom,0.1uatom")
s.Require().NoError(err)
s.Require().Equal(coins, Coins{
Coin{natom, NewInt(100000000)},
Coin{natom, NewInt(100000)},
Coin{natom, NewInt(100)},
})

testCases := []struct {
input DecCoin
denom string
result DecCoin
expErr bool
}{
{NewDecCoin("foo", ZeroInt()), atom, DecCoin{}, true},
{NewDecCoin(atom, ZeroInt()), "foo", DecCoin{}, true},
{NewDecCoin(atom, ZeroInt()), "FOO", DecCoin{}, true},

// 0.5atom
{NewDecCoinFromDec(atom, NewDecWithPrec(5, 1)), matom, NewDecCoin(matom, NewInt(500)), false}, // atom => matom
{NewDecCoinFromDec(atom, NewDecWithPrec(5, 1)), uatom, NewDecCoin(uatom, NewInt(500000)), false}, // atom => uatom
{NewDecCoinFromDec(atom, NewDecWithPrec(5, 1)), natom, NewDecCoin(natom, NewInt(500000000)), false}, // atom => natom

{NewDecCoin(uatom, NewInt(5000000)), matom, NewDecCoin(matom, NewInt(5000)), false}, // uatom => matom
{NewDecCoin(uatom, NewInt(5000000)), natom, NewDecCoin(natom, NewInt(5000000000)), false}, // uatom => natom
{NewDecCoin(uatom, NewInt(5000000)), atom, NewDecCoin(atom, NewInt(5)), false}, // uatom => atom

{NewDecCoin(matom, NewInt(5000)), natom, NewDecCoin(natom, NewInt(5000000000)), false}, // matom => natom
{NewDecCoin(matom, NewInt(5000)), uatom, NewDecCoin(uatom, NewInt(5000000)), false}, // matom => uatom
}

for i, tc := range testCases {
res, err := ConvertDecCoin(tc.input, tc.denom)
s.Require().Equal(
tc.expErr, err != nil,
"unexpected error; tc: #%d, input: %s, denom: %s", i+1, tc.input, tc.denom,
)
s.Require().Equal(
tc.result, res,
"invalid result; tc: #%d, input: %s, denom: %s", i+1, tc.input, tc.denom,
)
}

// reset registration
baseDenom = ""
denomUnits = map[string]Dec{}
}

func (s *internalDenomTestSuite) TestDecOperationOrder() {
dec, err := NewDecFromStr("11")
s.Require().NoError(err)
s.Require().NoError(RegisterDenom("unit1", dec))
dec, err = NewDecFromStr("100000011")
s.Require().NoError(RegisterDenom("unit2", dec))

coin, err := ConvertCoin(NewCoin("unit1", NewInt(100000011)), "unit2")
s.Require().NoError(err)
s.Require().Equal(coin, NewCoin("unit2", NewInt(11)))

// reset registration
baseDenom = ""
denomUnits = map[string]Dec{}
}
2 changes: 1 addition & 1 deletion types/rest/rest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ func TestBaseReq_Sanitize(t *testing.T) {

func TestBaseReq_ValidateBasic(t *testing.T) {
fromAddr := "cosmos1cq0sxam6x4l0sv9yz3a2vlqhdhvt2k6jtgcse0"
tenstakes, err := types.ParseCoins("10stake")
tenstakes, err := types.ParseCoinsNormalized("10stake")
require.NoError(t, err)
onestake, err := types.ParseDecCoins("1.0stake")
require.NoError(t, err)
Expand Down
2 changes: 1 addition & 1 deletion types/simulation/rand_util_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func TestRandStringOfLength(t *testing.T) {
}

func mustParseCoins(s string) sdk.Coins {
coins, err := sdk.ParseCoins(s)
coins, err := sdk.ParseCoinsNormalized(s)
if err != nil {
panic(err)
}
Expand Down
2 changes: 1 addition & 1 deletion x/auth/vesting/client/cli/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ timestamp.`,
return err
}

amount, err := sdk.ParseCoins(args[1])
amount, err := sdk.ParseCoinsNormalized(args[1])
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion x/bank/client/cli/tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ ignored as it is implied from [from_key_or_address].`,
return err
}

coins, err := sdk.ParseCoins(args[2])
coins, err := sdk.ParseCoinsNormalized(args[2])
if err != nil {
return err
}
Expand Down
2 changes: 1 addition & 1 deletion x/bank/client/testutil/cli_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ ignored as it is implied from [from_key_or_address].`,
return err
}

coins, err := sdk.ParseCoins(args[2])
coins, err := sdk.ParseCoinsNormalized(args[2])
if err != nil {
return err
}
Expand Down
Loading

0 comments on commit 54201d1

Please sign in to comment.