x/bank: balance and metadata validation

CHANGELOG.md

@@ -38,10 +38,11 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ### Improvements
 
-* (x/ibc) [\#8404](https://github.com/cosmos/cosmos-sdk/pull/8404) Reorder IBC `ChanOpenAck` and `ChanOpenConfirm` handler execution to perform core handler first, followed by application callbacks. 
+* (x/ibc) [\#8404](https://github.com/cosmos/cosmos-sdk/pull/8404) Reorder IBC `ChanOpenAck` and `ChanOpenConfirm` handler execution to perform core handler first, followed by application callbacks.
 
 ### Bug Fixes
 
+* (x/bank) [\#8417](https://github.com/cosmos/cosmos-sdk/pull/8417) Validate balances and coin denom metadata on genesis
 * (server) [\#8399](https://github.com/cosmos/cosmos-sdk/pull/8399) fix gRPC-web flag default value
 
 ## [v0.40.1](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.40.1) - 2021-01-19

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.

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
+		}
+	}
+}

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)
+			}
+		})
+	}
+}

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
-		}
-	}
-}