From 90cbfb690aeb6dfd96e1dcc3c5531f9f2a8d556b Mon Sep 17 00:00:00 2001 From: Alessio Treglia Date: Fri, 25 Jan 2019 17:11:24 +0000 Subject: [PATCH 1/2] Implement genesis file sanitization Add GenesisState.Sanitize(). It sorts genesis accounts and coin sets to ensure genesis state passes validation. gaia app calls GenesisState.Sanitize() on initFromGenesisState() before processing the genesis state. Closes: #3390 --- PENDING.md | 7 +++--- cmd/gaia/app/app.go | 8 ++----- cmd/gaia/app/genesis.go | 11 +++++++++ cmd/gaia/app/genesis_test.go | 27 ++++++++++++++++++--- cmd/gaia/cli_test/cli_test.go | 39 ++++++++++++++++++++++++++++--- cmd/gaia/cli_test/test_helpers.go | 17 ++++++++++++++ cmd/gaia/init/collect.go | 2 +- cmd/gaia/init/genesis_accts.go | 16 ++++++++----- cmd/gaia/init/gentx.go | 2 +- cmd/gaia/init/testnet.go | 2 +- cmd/gaia/init/utils.go | 3 ++- 11 files changed, 109 insertions(+), 25 deletions(-) diff --git a/PENDING.md b/PENDING.md index 0a01c3eb6500..3f5a4146c81e 100644 --- a/PENDING.md +++ b/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`) diff --git a/cmd/gaia/app/app.go b/cmd/gaia/app/app.go index 8b5ce4e73380..e99d0b8088cf 100644 --- a/cmd/gaia/app/app.go +++ b/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 } diff --git a/cmd/gaia/app/genesis.go b/cmd/gaia/app/genesis.go index cfda1c6c488d..b0010323baf0 100644 --- a/cmd/gaia/app/genesis.go +++ b/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"` diff --git a/cmd/gaia/app/genesis_test.go b/cmd/gaia/app/genesis_test.go index 1383794a3b8a..5114d4bd69fa 100644 --- a/cmd/gaia/app/genesis_test.go +++ b/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,24 @@ 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)) + priv := ed25519.GenPrivKey() + addr := sdk.AccAddress(priv.PubKey().Address()) + authAcc := auth.NewBaseAccountWithAddress(addr) + authAcc.SetCoins(sdk.Coins{ + sdk.NewInt64Coin("bcoin", 150), + sdk.NewInt64Coin("acoin", 150), + }) + genAcc := NewGenesisAccount(&authAcc) + genesisState.Accounts = append(genesisState.Accounts, genAcc) + lastAccount := genesisState.Accounts[len(genesisState.Accounts)-1] + require.Equal(t, lastAccount.Coins[0].Denom, "bcoin") + require.Equal(t, lastAccount.Coins[1].Denom, "acoin") + genesisState.Sanitize() + lastAccount = genesisState.Accounts[len(genesisState.Accounts)-1] + require.Equal(t, lastAccount.Coins[0].Denom, "acoin") + require.Equal(t, lastAccount.Coins[1].Denom, "bcoin") +} diff --git a/cmd/gaia/cli_test/cli_test.go b/cmd/gaia/cli_test/cli_test.go index 2d6864ec99e2..1d1655602247 100644 --- a/cmd/gaia/cli_test/cli_test.go +++ b/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) diff --git a/cmd/gaia/cli_test/test_helpers.go b/cmd/gaia/cli_test/test_helpers.go index 7571985062a1..54f592f10bee 100644 --- a/cmd/gaia/cli_test/test_helpers.go +++ b/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) { diff --git a/cmd/gaia/init/collect.go b/cmd/gaia/init/collect.go index e630d494cc7d..d0842df2cbe0 100644 --- a/cmd/gaia/init/collect.go +++ b/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 } diff --git a/cmd/gaia/init/genesis_accts.go b/cmd/gaia/init/genesis_accts.go index bb9c4af0c8ce..8b580584f3b5 100644 --- a/cmd/gaia/init/genesis_accts.go +++ b/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 } diff --git a/cmd/gaia/init/gentx.go b/cmd/gaia/init/gentx.go index 25de93aab498..142a9801e039 100644 --- a/cmd/gaia/init/gentx.go +++ b/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 } diff --git a/cmd/gaia/init/testnet.go b/cmd/gaia/init/testnet.go index 111a58032697..c69384a2d659 100644 --- a/cmd/gaia/init/testnet.go +++ b/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 } diff --git a/cmd/gaia/init/utils.go b/cmd/gaia/init/utils.go index 149b4d10e1bb..4e2f2f26f27d 100644 --- a/cmd/gaia/init/utils.go +++ b/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 From 48db843d6d27517fb8c2284c55f417a8b9aa899c Mon Sep 17 00:00:00 2001 From: Alessio Treglia Date: Sat, 26 Jan 2019 02:13:11 +0000 Subject: [PATCH 2/2] Address @sunnya97 comment --- cmd/gaia/app/genesis_test.go | 36 ++++++++++++++++++++++++------------ 1 file changed, 24 insertions(+), 12 deletions(-) diff --git a/cmd/gaia/app/genesis_test.go b/cmd/gaia/app/genesis_test.go index 5114d4bd69fa..be0c9dba4cec 100644 --- a/cmd/gaia/app/genesis_test.go +++ b/cmd/gaia/app/genesis_test.go @@ -146,20 +146,32 @@ func TestNewDefaultGenesisAccount(t *testing.T) { func TestGenesisStateSanitize(t *testing.T) { genesisState := makeGenesisState(t, nil) require.Nil(t, GaiaValidateGenesisState(genesisState)) - priv := ed25519.GenPrivKey() - addr := sdk.AccAddress(priv.PubKey().Address()) - authAcc := auth.NewBaseAccountWithAddress(addr) - authAcc.SetCoins(sdk.Coins{ + + addr1 := sdk.AccAddress(ed25519.GenPrivKey().PubKey().Address()) + authAcc1 := auth.NewBaseAccountWithAddress(addr1) + authAcc1.SetCoins(sdk.Coins{ sdk.NewInt64Coin("bcoin", 150), sdk.NewInt64Coin("acoin", 150), }) - genAcc := NewGenesisAccount(&authAcc) - genesisState.Accounts = append(genesisState.Accounts, genAcc) - lastAccount := genesisState.Accounts[len(genesisState.Accounts)-1] - require.Equal(t, lastAccount.Coins[0].Denom, "bcoin") - require.Equal(t, lastAccount.Coins[1].Denom, "acoin") + 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() - lastAccount = genesisState.Accounts[len(genesisState.Accounts)-1] - require.Equal(t, lastAccount.Coins[0].Denom, "acoin") - require.Equal(t, lastAccount.Coins[1].Denom, "bcoin") + 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") }