Implement GenesisState.Sanitize() (#3397)

PENDING.md

@@ -15,11 +15,12 @@ BREAKING CHANGES
 
 FEATURES
 
-* Gaia REST API (`gaiacli advanced rest-server`)
+* Gaia REST API
 
 * Gaia CLI  (`gaiacli`)
 
 * Gaia
+  - [\#3397](https://github.com/cosmos/cosmos-sdk/pull/3397) Implement genesis file sanitization to avoid failures at chain init.
 
 * SDK
   * \#3270 [x/staking] limit number of ongoing unbonding delegations /redelegations per pair/trio
@@ -29,7 +30,7 @@ FEATURES
 
 IMPROVEMENTS
 
-* Gaia REST API (`gaiacli advanced rest-server`)
+* Gaia REST API
 
 * Gaia CLI  (`gaiacli`)
 
@@ -42,7 +43,7 @@ IMPROVEMENTS
 
 BUG FIXES
 
-* Gaia REST API (`gaiacli advanced rest-server`)
+* Gaia REST API
 
 * Gaia CLI  (`gaiacli`)
 

cmd/gaia/app/app.go

@@ -228,10 +228,7 @@ func (app *GaiaApp) EndBlocker(ctx sdk.Context, req abci.RequestEndBlock) abci.R
 
 // initialize store from a genesis state
 func (app *GaiaApp) initFromGenesisState(ctx sdk.Context, genesisState GenesisState) []abci.ValidatorUpdate {
-	// sort by account number to maintain consistency
-	sort.Slice(genesisState.Accounts, func(i, j int) bool {
-		return genesisState.Accounts[i].AccountNumber < genesisState.Accounts[j].AccountNumber
-	})
+	genesisState.Sanitize()
 
 	// load the accounts
 	for _, gacc := range genesisState.Accounts {
@@ -256,8 +253,7 @@ func (app *GaiaApp) initFromGenesisState(ctx sdk.Context, genesisState GenesisSt
 	mint.InitGenesis(ctx, app.mintKeeper, genesisState.MintData)
 
 	// validate genesis state
-	err = GaiaValidateGenesisState(genesisState)
-	if err != nil {
+	if err := GaiaValidateGenesisState(genesisState); err != nil {
 		panic(err) // TODO find a way to do this w/o panics
 	}
 

cmd/gaia/app/genesis.go

@@ -58,6 +58,17 @@ func NewGenesisState(accounts []GenesisAccount, authData auth.GenesisState,
 	}
 }
 
+// Sanitize sorts accounts and coin sets.
+func (gs GenesisState) Sanitize() {
+	sort.Slice(gs.Accounts, func(i, j int) bool {
+		return gs.Accounts[i].AccountNumber < gs.Accounts[j].AccountNumber
+	})
+
+	for _, acc := range gs.Accounts {
+		acc.Coins = acc.Coins.Sort()
+	}
+}
+
 // GenesisAccount defines an account initialized at genesis.
 type GenesisAccount struct {
 	Address       sdk.AccAddress `json:"address"`

cmd/gaia/app/genesis_test.go

@@ -5,12 +5,12 @@ import (
 	"testing"
 	"time"
 
-	"github.com/tendermint/tendermint/crypto/secp256k1"
-	tmtypes "github.com/tendermint/tendermint/types"
-
 	"github.com/stretchr/testify/require"
+
 	"github.com/tendermint/tendermint/crypto"
 	"github.com/tendermint/tendermint/crypto/ed25519"
+	"github.com/tendermint/tendermint/crypto/secp256k1"
+	tmtypes "github.com/tendermint/tendermint/types"
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/cosmos-sdk/x/auth"
@@ -142,3 +142,36 @@ func TestNewDefaultGenesisAccount(t *testing.T) {
 	require.Equal(t, sdk.NewInt(1000), acc.Coins.AmountOf("footoken"))
 	require.Equal(t, sdk.NewInt(150), acc.Coins.AmountOf(bondDenom))
 }
+
+func TestGenesisStateSanitize(t *testing.T) {
+	genesisState := makeGenesisState(t, nil)
+	require.Nil(t, GaiaValidateGenesisState(genesisState))
+
+	addr1 := sdk.AccAddress(ed25519.GenPrivKey().PubKey().Address())
+	authAcc1 := auth.NewBaseAccountWithAddress(addr1)
+	authAcc1.SetCoins(sdk.Coins{
+		sdk.NewInt64Coin("bcoin", 150),
+		sdk.NewInt64Coin("acoin", 150),
+	})
+	authAcc1.SetAccountNumber(1)
+	genAcc1 := NewGenesisAccount(&authAcc1)
+
+	addr2 := sdk.AccAddress(ed25519.GenPrivKey().PubKey().Address())
+	authAcc2 := auth.NewBaseAccountWithAddress(addr2)
+	authAcc2.SetCoins(sdk.Coins{
+		sdk.NewInt64Coin("acoin", 150),
+		sdk.NewInt64Coin("bcoin", 150),
+	})
+	genAcc2 := NewGenesisAccount(&authAcc2)
+
+	genesisState.Accounts = []GenesisAccount{genAcc1, genAcc2}
+	require.True(t, genesisState.Accounts[0].AccountNumber > genesisState.Accounts[1].AccountNumber)
+	require.Equal(t, genesisState.Accounts[0].Coins[0].Denom, "bcoin")
+	require.Equal(t, genesisState.Accounts[0].Coins[1].Denom, "acoin")
+	require.Equal(t, genesisState.Accounts[1].Address, addr2)
+	genesisState.Sanitize()
+	require.False(t, genesisState.Accounts[0].AccountNumber > genesisState.Accounts[1].AccountNumber)
+	require.Equal(t, genesisState.Accounts[1].Address, addr1)
+	require.Equal(t, genesisState.Accounts[1].Coins[0].Denom, "acoin")
+	require.Equal(t, genesisState.Accounts[1].Coins[1].Denom, "bcoin")
+}

cmd/gaia/cli_test/cli_test.go

@@ -909,10 +909,7 @@ func TestGaiadCollectGentxs(t *testing.T) {
 	f.UnsafeResetAll()
 
 	// Initialize keys
-	f.KeysDelete(keyFoo)
-	f.KeysDelete(keyBar)
 	f.KeysAdd(keyFoo)
-	f.KeysAdd(keyBar)
 
 	// Configure json output
 	f.CLIConfig("output", "json")
@@ -932,6 +929,42 @@ func TestGaiadCollectGentxs(t *testing.T) {
 	f.Cleanup(gentxDir)
 }
 
+func TestGaiadAddGenesisAccount(t *testing.T) {
+	t.Parallel()
+	f := NewFixtures(t)
+
+	// Reset testing path
+	f.UnsafeResetAll()
+
+	// Initialize keys
+	f.KeysDelete(keyFoo)
+	f.KeysDelete(keyBar)
+	f.KeysDelete(keyBaz)
+	f.KeysAdd(keyFoo)
+	f.KeysAdd(keyBar)
+	f.KeysAdd(keyBaz)
+
+	// Configure json output
+	f.CLIConfig("output", "json")
+
+	// Run init
+	f.GDInit(keyFoo)
+
+	// Add account to genesis.json
+	bazCoins := sdk.Coins{
+		sdk.NewInt64Coin("acoin", 1000000),
+		sdk.NewInt64Coin("bcoin", 1000000),
+	}
+
+	f.AddGenesisAccount(f.KeyAddress(keyFoo), startCoins)
+	f.AddGenesisAccount(f.KeyAddress(keyBar), bazCoins)
+	genesisState := f.GenesisState()
+	require.Equal(t, genesisState.Accounts[0].Address, f.KeyAddress(keyFoo))
+	require.Equal(t, genesisState.Accounts[1].Address, f.KeyAddress(keyBar))
+	require.True(t, genesisState.Accounts[0].Coins.IsEqual(startCoins))
+	require.True(t, genesisState.Accounts[1].Coins.IsEqual(bazCoins))
+}
+
 func TestSlashingGetParams(t *testing.T) {
 	t.Parallel()
 	f := InitFixtures(t)

cmd/gaia/cli_test/test_helpers.go

@@ -16,6 +16,7 @@ import (
 	"github.com/cosmos/cosmos-sdk/client/keys"
 	"github.com/cosmos/cosmos-sdk/client/tx"
 	"github.com/cosmos/cosmos-sdk/cmd/gaia/app"
+	appInit "github.com/cosmos/cosmos-sdk/cmd/gaia/init"
 	"github.com/cosmos/cosmos-sdk/codec"
 	"github.com/cosmos/cosmos-sdk/server"
 	"github.com/cosmos/cosmos-sdk/tests"
@@ -76,6 +77,22 @@ func NewFixtures(t *testing.T) *Fixtures {
 	}
 }
 
+// GenesisFile returns the path of the genesis file
+func (f Fixtures) GenesisFile() string {
+	return filepath.Join(f.GDHome, "config", "genesis.json")
+}
+
+// GenesisFile returns the application's genesis state
+func (f Fixtures) GenesisState() app.GenesisState {
+	cdc := codec.New()
+	genDoc, err := appInit.LoadGenesisDoc(cdc, f.GenesisFile())
+	require.NoError(f.T, err)
+
+	var appState app.GenesisState
+	require.NoError(f.T, cdc.UnmarshalJSON(genDoc.AppState, &appState))
+	return appState
+}
+
 // InitFixtures is called at the beginning of a test
 // and initializes a chain with 1 validator
 func InitFixtures(t *testing.T) (f *Fixtures) {

cmd/gaia/init/collect.go

@@ -44,7 +44,7 @@ func CollectGenTxsCmd(ctx *server.Context, cdc *codec.Codec) *cobra.Command {
 				return err
 			}
 
-			genDoc, err := loadGenesisDoc(cdc, config.GenesisFile())
+			genDoc, err := LoadGenesisDoc(cdc, config.GenesisFile())
 			if err != nil {
 				return err
 			}

cmd/gaia/init/genesis_accts.go

@@ -1,7 +1,6 @@
 package init
 
 import (
-	"encoding/json"
 	"fmt"
 
 	"github.com/spf13/cobra"
@@ -49,7 +48,7 @@ func AddGenesisAccountCmd(ctx *server.Context, cdc *codec.Codec) *cobra.Command
 			if !common.FileExists(genFile) {
 				return fmt.Errorf("%s does not exist, run `gaiad init` first", genFile)
 			}
-			genDoc, err := loadGenesisDoc(cdc, genFile)
+			genDoc, err := LoadGenesisDoc(cdc, genFile)
 			if err != nil {
 				return err
 			}
@@ -59,7 +58,12 @@ func AddGenesisAccountCmd(ctx *server.Context, cdc *codec.Codec) *cobra.Command
 				return err
 			}
 
-			appStateJSON, err := addGenesisAccount(cdc, appState, addr, coins)
+			appState, err = addGenesisAccount(cdc, appState, addr, coins)
+			if err != nil {
+				return err
+			}
+
+			appStateJSON, err := cdc.MarshalJSON(appState)
 			if err != nil {
 				return err
 			}
@@ -73,15 +77,15 @@ func AddGenesisAccountCmd(ctx *server.Context, cdc *codec.Codec) *cobra.Command
 	return cmd
 }
 
-func addGenesisAccount(cdc *codec.Codec, appState app.GenesisState, addr sdk.AccAddress, coins sdk.Coins) (json.RawMessage, error) {
+func addGenesisAccount(cdc *codec.Codec, appState app.GenesisState, addr sdk.AccAddress, coins sdk.Coins) (app.GenesisState, error) {
 	for _, stateAcc := range appState.Accounts {
 		if stateAcc.Address.Equals(addr) {
-			return nil, fmt.Errorf("the application state already contains account %v", addr)
+			return appState, fmt.Errorf("the application state already contains account %v", addr)
 		}
 	}
 
 	acc := auth.NewBaseAccountWithAddress(addr)
 	acc.Coins = coins
 	appState.Accounts = append(appState.Accounts, app.NewGenesisAccount(&acc))
-	return cdc.MarshalJSON(appState)
+	return appState, nil
 }

cmd/gaia/init/gentx.go

@@ -66,7 +66,7 @@ following delegation and commission default parameters:
 				return err
 			}
 
-			genDoc, err := loadGenesisDoc(cdc, config.GenesisFile())
+			genDoc, err := LoadGenesisDoc(cdc, config.GenesisFile())
 			if err != nil {
 				return err
 			}

cmd/gaia/init/testnet.go

@@ -295,7 +295,7 @@ func collectGenFiles(
 		nodeID, valPubKey := nodeIDs[i], valPubKeys[i]
 		initCfg := newInitConfig(chainID, gentxsDir, moniker, nodeID, valPubKey)
 
-		genDoc, err := loadGenesisDoc(cdc, config.GenesisFile())
+		genDoc, err := LoadGenesisDoc(cdc, config.GenesisFile())
 		if err != nil {
 			return err
 		}

cmd/gaia/init/utils.go

@@ -88,7 +88,8 @@ func InitializeNodeValidatorFiles(
 	return nodeID, valPubKey, nil
 }
 
-func loadGenesisDoc(cdc *amino.Codec, genFile string) (genDoc types.GenesisDoc, err error) {
+// LoadGenesisDoc reads and unmarshals GenesisDoc from the given file.
+func LoadGenesisDoc(cdc *amino.Codec, genFile string) (genDoc types.GenesisDoc, err error) {
 	genContents, err := ioutil.ReadFile(genFile)
 	if err != nil {
 		return genDoc, err