diff --git a/CHANGELOG.md b/CHANGELOG.md index de85d1dc787e..91ca28b567eb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,12 @@ Ref: https://keepachangelog.com/en/1.0.0/ # Changelog +## [Unreleased] + +### Bug Fixes + +* (x/bank) [\#8417](https://github.com/cosmos/cosmos-sdk/pull/8417) Validate balances and coin denom metadata on genesis + ## [v0.40.1](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.40.1) - 2021-01-19 ### Improvements diff --git a/x/bank/module.go b/x/bank/module.go index bc998634c12e..f271fa21975c 100644 --- a/x/bank/module.go +++ b/x/bank/module.go @@ -58,7 +58,7 @@ func (AppModuleBasic) ValidateGenesis(cdc codec.JSONMarshaler, _ client.TxEncodi return fmt.Errorf("failed to unmarshal %s genesis state: %w", types.ModuleName, err) } - return types.ValidateGenesis(data) + return data.Validate() } // RegisterRESTRoutes registers the REST routes for the bank module. diff --git a/x/bank/types/balance.go b/x/bank/types/balance.go new file mode 100644 index 000000000000..93517a1d925b --- /dev/null +++ b/x/bank/types/balance.go @@ -0,0 +1,93 @@ +package types + +import ( + "bytes" + "encoding/json" + fmt "fmt" + "sort" + + "github.com/cosmos/cosmos-sdk/codec" + sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/cosmos/cosmos-sdk/x/bank/exported" +) + +var _ exported.GenesisBalance = (*Balance)(nil) + +// GetAddress returns the account address of the Balance object. +func (b Balance) GetAddress() sdk.AccAddress { + addr, _ := sdk.AccAddressFromBech32(b.Address) + return addr +} + +// GetCoins returns the account coins of the Balance object. +func (b Balance) GetCoins() sdk.Coins { + return b.Coins +} + +// Validate checks for address and coins correctness. +func (b Balance) Validate() error { + _, err := sdk.AccAddressFromBech32(b.Address) + if err != nil { + return err + } + + if len(b.Coins) == 0 { + return fmt.Errorf("empty or nil coins for address %s", b.Address) + } + + seenDenoms := make(map[string]bool) + + // NOTE: we perform a custom validation since the coins.Validate function + // errors on zero balance coins + for _, coin := range b.Coins { + if seenDenoms[coin.Denom] { + return fmt.Errorf("duplicate denomination %s", coin.Denom) + } + + if err := sdk.ValidateDenom(coin.Denom); err != nil { + return err + } + + if coin.IsNegative() { + return fmt.Errorf("coin %s amount is cannot be negative", coin.Denom) + } + + seenDenoms[coin.Denom] = true + } + + // sort the coins post validation + b.Coins = b.Coins.Sort() + + return nil +} + +// SanitizeGenesisBalances sorts addresses and coin sets. +func SanitizeGenesisBalances(balances []Balance) []Balance { + sort.Slice(balances, func(i, j int) bool { + addr1, _ := sdk.AccAddressFromBech32(balances[i].Address) + addr2, _ := sdk.AccAddressFromBech32(balances[j].Address) + return bytes.Compare(addr1.Bytes(), addr2.Bytes()) < 0 + }) + + for _, balance := range balances { + balance.Coins = balance.Coins.Sort() + } + + return balances +} + +// GenesisBalancesIterator implements genesis account iteration. +type GenesisBalancesIterator struct{} + +// IterateGenesisBalances iterates over all the genesis balances found in +// appGenesis and invokes a callback on each genesis account. If any call +// returns true, iteration stops. +func (GenesisBalancesIterator) IterateGenesisBalances( + cdc codec.JSONMarshaler, appState map[string]json.RawMessage, cb func(exported.GenesisBalance) (stop bool), +) { + for _, balance := range GetGenesisStateFromAppState(cdc, appState).Balances { + if cb(balance) { + break + } + } +} diff --git a/x/bank/types/balance_test.go b/x/bank/types/balance_test.go new file mode 100644 index 000000000000..73e4d8eff387 --- /dev/null +++ b/x/bank/types/balance_test.go @@ -0,0 +1,80 @@ +package types + +import ( + "testing" + + "github.com/stretchr/testify/require" + + sdk "github.com/cosmos/cosmos-sdk/types" +) + +func TestBalanceValidate(t *testing.T) { + + testCases := []struct { + name string + balance Balance + expErr bool + }{ + { + "valid balance", + Balance{ + Address: "cosmos1yq8lgssgxlx9smjhes6ryjasmqmd3ts2559g0t", + Coins: sdk.Coins{sdk.NewInt64Coin("uatom", 1)}, + }, + false, + }, + {"empty balance", Balance{}, true}, + { + "nil balance coins", + Balance{ + Address: "cosmos1yq8lgssgxlx9smjhes6ryjasmqmd3ts2559g0t", + }, + true, + }, + { + "dup coins", + Balance{ + Address: "cosmos1yq8lgssgxlx9smjhes6ryjasmqmd3ts2559g0t", + Coins: sdk.Coins{ + sdk.NewInt64Coin("uatom", 1), + sdk.NewInt64Coin("uatom", 1), + }, + }, + true, + }, + { + "invalid coin denom", + Balance{ + Address: "cosmos1yq8lgssgxlx9smjhes6ryjasmqmd3ts2559g0t", + Coins: sdk.Coins{ + sdk.Coin{Denom: "", Amount: sdk.OneInt()}, + }, + }, + true, + }, + { + "negative coin", + Balance{ + Address: "cosmos1yq8lgssgxlx9smjhes6ryjasmqmd3ts2559g0t", + Coins: sdk.Coins{ + sdk.Coin{Denom: "uatom", Amount: sdk.NewInt(-1)}, + }, + }, + true, + }, + } + + for _, tc := range testCases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + + err := tc.balance.Validate() + + if tc.expErr { + require.Error(t, err) + } else { + require.NoError(t, err) + } + }) + } +} diff --git a/x/bank/types/genesis.go b/x/bank/types/genesis.go index ccf35be86231..4adc758f3bd9 100644 --- a/x/bank/types/genesis.go +++ b/x/bank/types/genesis.go @@ -1,51 +1,49 @@ package types import ( - "bytes" "encoding/json" - "sort" + "fmt" "github.com/cosmos/cosmos-sdk/codec" sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/cosmos/cosmos-sdk/x/bank/exported" ) -var _ exported.GenesisBalance = (*Balance)(nil) +// Validate performs basic validation of supply genesis data returning an +// error for any failed validation criteria. +func (gs GenesisState) Validate() error { + if err := gs.Params.Validate(); err != nil { + return err + } -// GetAddress returns the account address of the Balance object. -func (b Balance) GetAddress() sdk.AccAddress { - addr1, _ := sdk.AccAddressFromBech32(b.Address) - return addr1 -} + seenBalances := make(map[string]bool) + seenMetadatas := make(map[string]bool) -// GetAddress returns the account coins of the Balance object. -func (b Balance) GetCoins() sdk.Coins { - return b.Coins -} + for _, balance := range gs.Balances { + if seenBalances[balance.Address] { + return fmt.Errorf("duplicate balance for address %s", balance.Address) + } -// SanitizeGenesisAccounts sorts addresses and coin sets. -func SanitizeGenesisBalances(balances []Balance) []Balance { - sort.Slice(balances, func(i, j int) bool { - addr1, _ := sdk.AccAddressFromBech32(balances[i].Address) - addr2, _ := sdk.AccAddressFromBech32(balances[j].Address) - return bytes.Compare(addr1.Bytes(), addr2.Bytes()) < 0 - }) + if err := balance.Validate(); err != nil { + return err + } - for _, balance := range balances { - balance.Coins = balance.Coins.Sort() + seenBalances[balance.Address] = true } - return balances -} + for _, metadata := range gs.DenomMetadata { + if seenMetadatas[metadata.Base] { + return fmt.Errorf("duplicate client metadata for denom %s", metadata.Base) + } -// ValidateGenesis performs basic validation of supply genesis data returning an -// error for any failed validation criteria. -func ValidateGenesis(data GenesisState) error { - if err := data.Params.Validate(); err != nil { - return err + if err := metadata.Validate(); err != nil { + return err + } + + seenMetadatas[metadata.Base] = true } - return NewSupply(data.Supply).ValidateBasic() + // NOTE: this errors if supply for any given coin is zero + return NewSupply(gs.Supply).ValidateBasic() } // NewGenesisState creates a new genesis state. @@ -74,19 +72,3 @@ func GetGenesisStateFromAppState(cdc codec.JSONMarshaler, appState map[string]js return &genesisState } - -// GenesisAccountIterator implements genesis account iteration. -type GenesisBalancesIterator struct{} - -// IterateGenesisAccounts iterates over all the genesis accounts found in -// appGenesis and invokes a callback on each genesis account. If any call -// returns true, iteration stops. -func (GenesisBalancesIterator) IterateGenesisBalances( - cdc codec.JSONMarshaler, appState map[string]json.RawMessage, cb func(exported.GenesisBalance) (stop bool), -) { - for _, balance := range GetGenesisStateFromAppState(cdc, appState).Balances { - if cb(balance) { - break - } - } -} diff --git a/x/bank/types/genesis_test.go b/x/bank/types/genesis_test.go index 5f612aed726b..6751e9721f31 100644 --- a/x/bank/types/genesis_test.go +++ b/x/bank/types/genesis_test.go @@ -5,48 +5,147 @@ import ( "github.com/stretchr/testify/require" - "github.com/cosmos/cosmos-sdk/codec" + sdk "github.com/cosmos/cosmos-sdk/types" "github.com/cosmos/cosmos-sdk/x/bank/types" ) -func TestMarshalJSONMetaData(t *testing.T) { - cdc := codec.NewLegacyAmino() +func TestGenesisStateValidate(t *testing.T) { testCases := []struct { - name string - input []types.Metadata - strOutput string + name string + genesisState types.GenesisState + expErr bool }{ - {"nil metadata", nil, `null`}, - {"empty metadata", []types.Metadata{}, `[]`}, - {"non-empty coins", []types.Metadata{{ - Description: "The native staking token of the Cosmos Hub.", - DenomUnits: []*types.DenomUnit{ - {"uatom", uint32(0), []string{"microatom"}}, // The default exponent value 0 is omitted in the json - {"matom", uint32(3), []string{"milliatom"}}, - {"atom", uint32(6), nil}, + { + "valid genesisState", + types.GenesisState{ + Params: types.DefaultParams(), + Balances: []types.Balance{ + { + Address: "cosmos1yq8lgssgxlx9smjhes6ryjasmqmd3ts2559g0t", + Coins: sdk.Coins{sdk.NewInt64Coin("uatom", 1)}, + }, + }, + Supply: sdk.Coins{sdk.NewInt64Coin("uatom", 1)}, + DenomMetadata: []types.Metadata{ + { + Description: "The native staking token of the Cosmos Hub.", + DenomUnits: []*types.DenomUnit{ + {"uatom", uint32(0), []string{"microatom"}}, + {"matom", uint32(3), []string{"milliatom"}}, + {"atom", uint32(6), nil}, + }, + Base: "uatom", + Display: "atom", + }, + }, }, - Base: "uatom", - Display: "atom", + false, }, + {"empty genesisState", types.GenesisState{}, false}, + { + "invalid params ", + types.GenesisState{ + Params: types.Params{ + SendEnabled: []*types.SendEnabled{ + {"", true}, + }, + }, + }, + true, + }, + { + "dup balances", + types.GenesisState{ + Balances: []types.Balance{ + { + Address: "cosmos1yq8lgssgxlx9smjhes6ryjasmqmd3ts2559g0t", + Coins: sdk.Coins{sdk.NewInt64Coin("uatom", 1)}, + }, + { + Address: "cosmos1yq8lgssgxlx9smjhes6ryjasmqmd3ts2559g0t", + Coins: sdk.Coins{sdk.NewInt64Coin("uatom", 1)}, + }, + }, + }, + true, + }, + { + "invalid balance", + types.GenesisState{ + Balances: []types.Balance{ + { + Address: "cosmos1yq8lgssgxlx9smjhes6ryjasmqmd3ts2559g0t", + }, + }, + }, + true, + }, + { + "dup Metadata", + types.GenesisState{ + DenomMetadata: []types.Metadata{ + { + Description: "The native staking token of the Cosmos Hub.", + DenomUnits: []*types.DenomUnit{ + {"uatom", uint32(0), []string{"microatom"}}, + {"matom", uint32(3), []string{"milliatom"}}, + {"atom", uint32(6), nil}, + }, + Base: "uatom", + Display: "atom", + }, + { + Description: "The native staking token of the Cosmos Hub.", + DenomUnits: []*types.DenomUnit{ + {"uatom", uint32(0), []string{"microatom"}}, + {"matom", uint32(3), []string{"milliatom"}}, + {"atom", uint32(6), nil}, + }, + Base: "uatom", + Display: "atom", + }, + }, + }, + true, + }, + { + "invalid Metadata", + types.GenesisState{ + DenomMetadata: []types.Metadata{ + { + Description: "The native staking token of the Cosmos Hub.", + DenomUnits: []*types.DenomUnit{ + {"uatom", uint32(0), []string{"microatom"}}, + {"matom", uint32(3), []string{"milliatom"}}, + {"atom", uint32(6), nil}, + }, + Base: "", + Display: "", + }, + }, + }, + true, + }, + { + "invalid supply", + types.GenesisState{ + Supply: sdk.Coins{sdk.Coin{Denom: "", Amount: sdk.OneInt()}}, + }, + true, }, - `[{"description":"The native staking token of the Cosmos Hub.","denom_units":[{"denom":"uatom","aliases":["microatom"]},{"denom":"matom","exponent":3,"aliases":["milliatom"]},{"denom":"atom","exponent":6}],"base":"uatom","display":"atom"}]`}, } for _, tc := range testCases { tc := tc t.Run(tc.name, func(t *testing.T) { - bz, err := cdc.MarshalJSON(tc.input) - require.NoError(t, err) - require.Equal(t, tc.strOutput, string(bz)) - var newMetadata []types.Metadata - require.NoError(t, cdc.UnmarshalJSON(bz, &newMetadata)) + err := tc.genesisState.Validate() - if len(tc.input) == 0 { - require.Nil(t, newMetadata) + if tc.expErr { + require.Error(t, err) } else { - require.Equal(t, tc.input, newMetadata) + require.NoError(t, err) } }) } diff --git a/x/bank/types/metadata.go b/x/bank/types/metadata.go new file mode 100644 index 000000000000..5106964e994e --- /dev/null +++ b/x/bank/types/metadata.go @@ -0,0 +1,56 @@ +package types + +import ( + "fmt" + "strings" + + sdk "github.com/cosmos/cosmos-sdk/types" +) + +// Validate performs a basic validation of the coin metadata fields +func (m Metadata) Validate() error { + if err := sdk.ValidateDenom(m.Base); err != nil { + return fmt.Errorf("invalid metadata base denom: %w", err) + } + + if err := sdk.ValidateDenom(m.Display); err != nil { + return fmt.Errorf("invalid metadata display denom: %w", err) + } + + seenUnits := make(map[string]bool) + for _, denomUnit := range m.DenomUnits { + if seenUnits[denomUnit.Denom] { + return fmt.Errorf("duplicate denomination unit %s", denomUnit.Denom) + } + + if err := denomUnit.Validate(); err != nil { + return err + } + + seenUnits[denomUnit.Denom] = true + } + + return nil +} + +// Validate performs a basic validation of the denomination unit fields +func (du DenomUnit) Validate() error { + if err := sdk.ValidateDenom(du.Denom); err != nil { + return fmt.Errorf("invalid denom unit: %w", err) + } + + seenAliases := make(map[string]bool) + for _, alias := range du.Aliases { + if seenAliases[alias] { + return fmt.Errorf("duplicate denomination unit alias %s", alias) + } + + if strings.TrimSpace(alias) == "" { + return fmt.Errorf("alias for denom unit %s cannot be blank", du.Denom) + } + + seenAliases[alias] = true + } + + return nil +} diff --git a/x/bank/types/metadata_test.go b/x/bank/types/metadata_test.go new file mode 100644 index 000000000000..29f4d220f95f --- /dev/null +++ b/x/bank/types/metadata_test.go @@ -0,0 +1,155 @@ +package types_test + +import ( + "testing" + + "github.com/stretchr/testify/require" + + "github.com/cosmos/cosmos-sdk/codec" + "github.com/cosmos/cosmos-sdk/x/bank/types" +) + +func TestMetadataValidate(t *testing.T) { + testCases := []struct { + name string + metadata types.Metadata + expErr bool + }{ + { + "non-empty coins", + types.Metadata{ + Description: "The native staking token of the Cosmos Hub.", + DenomUnits: []*types.DenomUnit{ + {"uatom", uint32(0), []string{"microatom"}}, + {"matom", uint32(3), []string{"milliatom"}}, + {"atom", uint32(6), nil}, + }, + Base: "uatom", + Display: "atom", + }, + false, + }, + {"empty metadata", types.Metadata{}, true}, + { + "invalid base denom", + types.Metadata{ + Base: "", + }, + true, + }, + { + "invalid display denom", + types.Metadata{ + Base: "uatom", + Display: "", + }, + true, + }, + { + "duplicate denom unit", + types.Metadata{ + Description: "The native staking token of the Cosmos Hub.", + DenomUnits: []*types.DenomUnit{ + {"uatom", uint32(0), []string{"microatom"}}, + {"uatom", uint32(0), []string{"microatom"}}, + }, + Base: "uatom", + Display: "atom", + }, + true, + }, + { + "invalid denom unit", + types.Metadata{ + Description: "The native staking token of the Cosmos Hub.", + DenomUnits: []*types.DenomUnit{ + {"", uint32(0), []string{"microatom"}}, + }, + Base: "uatom", + Display: "atom", + }, + true, + }, + { + "invalid denom unit alias", + types.Metadata{ + Description: "The native staking token of the Cosmos Hub.", + DenomUnits: []*types.DenomUnit{ + {"uatom", uint32(0), []string{""}}, + }, + Base: "uatom", + Display: "atom", + }, + true, + }, + { + "duplicate denom unit alias", + types.Metadata{ + Description: "The native staking token of the Cosmos Hub.", + DenomUnits: []*types.DenomUnit{ + {"uatom", uint32(0), []string{"microatom", "microatom"}}, + }, + Base: "uatom", + Display: "atom", + }, + true, + }, + } + + for _, tc := range testCases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + + err := tc.metadata.Validate() + + if tc.expErr { + require.Error(t, err) + } else { + require.NoError(t, err) + } + }) + } +} + +func TestMarshalJSONMetaData(t *testing.T) { + cdc := codec.NewLegacyAmino() + + testCases := []struct { + name string + input []types.Metadata + strOutput string + }{ + {"nil metadata", nil, `null`}, + {"empty metadata", []types.Metadata{}, `[]`}, + {"non-empty coins", []types.Metadata{{ + Description: "The native staking token of the Cosmos Hub.", + DenomUnits: []*types.DenomUnit{ + {"uatom", uint32(0), []string{"microatom"}}, // The default exponent value 0 is omitted in the json + {"matom", uint32(3), []string{"milliatom"}}, + {"atom", uint32(6), nil}, + }, + Base: "uatom", + Display: "atom", + }, + }, + `[{"description":"The native staking token of the Cosmos Hub.","denom_units":[{"denom":"uatom","aliases":["microatom"]},{"denom":"matom","exponent":3,"aliases":["milliatom"]},{"denom":"atom","exponent":6}],"base":"uatom","display":"atom"}]`}, + } + + for _, tc := range testCases { + tc := tc + t.Run(tc.name, func(t *testing.T) { + bz, err := cdc.MarshalJSON(tc.input) + require.NoError(t, err) + require.Equal(t, tc.strOutput, string(bz)) + + var newMetadata []types.Metadata + require.NoError(t, cdc.UnmarshalJSON(bz, &newMetadata)) + + if len(tc.input) == 0 { + require.Nil(t, newMetadata) + } else { + require.Equal(t, tc.input, newMetadata) + } + }) + } +} diff --git a/x/bank/types/supply.go b/x/bank/types/supply.go index 558c852c2b83..c5c14b15bc2c 100644 --- a/x/bank/types/supply.go +++ b/x/bank/types/supply.go @@ -50,8 +50,8 @@ func (supply Supply) String() string { // ValidateBasic validates the Supply coins and returns error if invalid func (supply Supply) ValidateBasic() error { - if !supply.Total.IsValid() { - return fmt.Errorf("invalid total supply: %s", supply.Total.String()) + if err := supply.Total.Validate(); err != nil { + return fmt.Errorf("invalid total supply: %w", err) } return nil