Merge pull request #28 from planetmint/jmastr/fix-some-code-smells

Fix some code smells

app/export.go

@@ -2,7 +2,6 @@ package app
 
 import (
 	"encoding/json"
-	"fmt"
 	"log"
 
 	tmproto "github.com/cometbft/cometbft/proto/tendermint/types"
@@ -74,23 +73,11 @@ func (app *App) prepForZeroHeightGenesis(ctx sdk.Context, jailAllowedAddrs []str
 	/* Handle fee distribution state. */
 
 	// withdraw all validator commission
-	app.StakingKeeper.IterateValidators(ctx, func(_ int64, val stakingtypes.ValidatorI) (stop bool) {
-		_, _ = app.DistrKeeper.WithdrawValidatorCommission(ctx, val.GetOperator())
-		return false
-	})
+	app.withdrawAllValidatorCommission(ctx)
 
 	// withdraw all delegator rewards
 	dels := app.StakingKeeper.GetAllDelegations(ctx)
-	for _, delegation := range dels {
-		valAddr, err := sdk.ValAddressFromBech32(delegation.ValidatorAddress)
-		if err != nil {
-			panic(err)
-		}
-
-		delAddr := sdk.MustAccAddressFromBech32(delegation.DelegatorAddress)
-
-		_, _ = app.DistrKeeper.WithdrawDelegationRewards(ctx, delAddr, valAddr)
-	}
+	app.withdrawAllDelegatorRewards(ctx, dels)
 
 	// clear validator slash events
 	app.DistrKeeper.DeleteAllValidatorSlashEvents(ctx)
@@ -103,6 +90,54 @@ func (app *App) prepForZeroHeightGenesis(ctx sdk.Context, jailAllowedAddrs []str
 	ctx = ctx.WithBlockHeight(0)
 
 	// reinitialize all validators
+	app.reinitializeAllValidators(ctx)
+
+	// reinitialize all delegations
+	app.reinitializeAllDelegations(ctx, dels)
+
+	// reset context height
+	ctx = ctx.WithBlockHeight(height)
+
+	/* Handle staking state. */
+
+	// iterate through redelegations, reset creation height
+	app.iterateRedelegations(ctx)
+
+	// iterate through unbonding delegations, reset creation height
+	app.iterateUnbondingDelegations(ctx)
+
+	// Iterate through validators by power descending, reset bond heights, and
+	// update bond intra-tx counters.
+	app.iterateValidatorsPowerDescending(ctx, applyAllowedAddrs, allowedAddrsMap)
+
+	/* Handle slashing state. */
+
+	// reset start height on signing infos
+	app.resetStartHeight(ctx)
+}
+
+func (app *App) withdrawAllValidatorCommission(ctx sdk.Context) {
+	app.StakingKeeper.IterateValidators(ctx, func(_ int64, val stakingtypes.ValidatorI) (stop bool) {
+		_, _ = app.DistrKeeper.WithdrawValidatorCommission(ctx, val.GetOperator())
+		return false
+	})
+}
+
+func (app *App) withdrawAllDelegatorRewards(ctx sdk.Context, dels []stakingtypes.Delegation) error {
+	for _, delegation := range dels {
+		valAddr, err := sdk.ValAddressFromBech32(delegation.ValidatorAddress)
+		if err != nil {
+			return err
+		}
+
+		delAddr := sdk.MustAccAddressFromBech32(delegation.DelegatorAddress)
+
+		_, _ = app.DistrKeeper.WithdrawDelegationRewards(ctx, delAddr, valAddr)
+	}
+	return nil
+}
+
+func (app *App) reinitializeAllValidators(ctx sdk.Context) {
 	app.StakingKeeper.IterateValidators(ctx, func(_ int64, val stakingtypes.ValidatorI) (stop bool) {
 		// donate any unwithdrawn outstanding reward fraction tokens to the community pool
 		scraps := app.DistrKeeper.GetValidatorOutstandingRewardsCoins(ctx, val.GetOperator())
@@ -115,32 +150,30 @@ func (app *App) prepForZeroHeightGenesis(ctx sdk.Context, jailAllowedAddrs []str
 		}
 		return false
 	})
+}
 
-	// reinitialize all delegations
+func (app *App) reinitializeAllDelegations(ctx sdk.Context, dels []stakingtypes.Delegation) error {
 	for _, del := range dels {
 		valAddr, err := sdk.ValAddressFromBech32(del.ValidatorAddress)
 		if err != nil {
-			panic(err)
+			return err
 		}
 		delAddr := sdk.MustAccAddressFromBech32(del.DelegatorAddress)
 
 		if err := app.DistrKeeper.Hooks().BeforeDelegationCreated(ctx, delAddr, valAddr); err != nil {
 			// never called as BeforeDelegationCreated always returns nil
-			panic(fmt.Errorf("error while incrementing period: %w", err))
+			return err
 		}
 
 		if err := app.DistrKeeper.Hooks().AfterDelegationModified(ctx, delAddr, valAddr); err != nil {
 			// never called as AfterDelegationModified always returns nil
-			panic(fmt.Errorf("error while creating a new delegation period record: %w", err))
+			return err
 		}
 	}
+	return nil
+}
 
-	// reset context height
-	ctx = ctx.WithBlockHeight(height)
-
-	/* Handle staking state. */
-
-	// iterate through redelegations, reset creation height
+func (app *App) iterateRedelegations(ctx sdk.Context) {
 	app.StakingKeeper.IterateRedelegations(ctx, func(_ int64, red stakingtypes.Redelegation) (stop bool) {
 		for i := range red.Entries {
 			red.Entries[i].CreationHeight = 0
@@ -148,8 +181,9 @@ func (app *App) prepForZeroHeightGenesis(ctx sdk.Context, jailAllowedAddrs []str
 		app.StakingKeeper.SetRedelegation(ctx, red)
 		return false
 	})
+}
 
-	// iterate through unbonding delegations, reset creation height
+func (app *App) iterateUnbondingDelegations(ctx sdk.Context) {
 	app.StakingKeeper.IterateUnbondingDelegations(ctx, func(_ int64, ubd stakingtypes.UnbondingDelegation) (stop bool) {
 		for i := range ubd.Entries {
 			ubd.Entries[i].CreationHeight = 0
@@ -157,9 +191,9 @@ func (app *App) prepForZeroHeightGenesis(ctx sdk.Context, jailAllowedAddrs []str
 		app.StakingKeeper.SetUnbondingDelegation(ctx, ubd)
 		return false
 	})
+}
 
-	// Iterate through validators by power descending, reset bond heights, and
-	// update bond intra-tx counters.
+func (app *App) iterateValidatorsPowerDescending(ctx sdk.Context, applyAllowedAddrs bool, allowedAddrsMap map[string]bool) {
 	store := ctx.KVStore(app.GetKey(stakingtypes.StoreKey))
 	iter := sdk.KVStoreReversePrefixIterator(store, stakingtypes.ValidatorsKey)
 	counter := int16(0)
@@ -189,10 +223,9 @@ func (app *App) prepForZeroHeightGenesis(ctx sdk.Context, jailAllowedAddrs []str
 	if err != nil {
 		log.Fatal(err)
 	}
+}
 
-	/* Handle slashing state. */
-
-	// reset start height on signing infos
+func (app *App) resetStartHeight(ctx sdk.Context) {
 	app.SlashingKeeper.IterateValidatorSigningInfos(
 		ctx,
 		func(addr sdk.ConsAddress, info slashingtypes.ValidatorSigningInfo) (stop bool) {

app/simulation_test.go

@@ -39,6 +39,8 @@ import (
 	"planetmint-go/app"
 )
 
+const SIMULATION_SETUP_FAILED = "simulation setup failed"
+
 type storeKeysPrefixes struct {
 	A        storetypes.StoreKey
 	B        storetypes.StoreKey
@@ -76,7 +78,7 @@ func BenchmarkSimulation(b *testing.B) {
 		simcli.FlagVerboseValue,
 		simcli.FlagEnabledValue,
 	)
-	require.NoError(b, err, "simulation setup failed")
+	require.NoError(b, err, SIMULATION_SETUP_FAILED)
 
 	b.Cleanup(func() {
 		require.NoError(b, db.Close())
@@ -230,7 +232,7 @@ func TestAppImportExport(t *testing.T) {
 	if skip {
 		t.Skip("skipping application import/export simulation")
 	}
-	require.NoError(t, err, "simulation setup failed")
+	require.NoError(t, err, SIMULATION_SETUP_FAILED)
 
 	defer func() {
 		require.NoError(t, db.Close())
@@ -295,7 +297,7 @@ func TestAppImportExport(t *testing.T) {
 		simcli.FlagVerboseValue,
 		simcli.FlagEnabledValue,
 	)
-	require.NoError(t, err, "simulation setup failed")
+	require.NoError(t, err, SIMULATION_SETUP_FAILED)
 
 	defer func() {
 		require.NoError(t, newDB.Close())
@@ -384,7 +386,7 @@ func TestAppSimulationAfterImport(t *testing.T) {
 	if skip {
 		t.Skip("skipping application simulation after import")
 	}
-	require.NoError(t, err, "simulation setup failed")
+	require.NoError(t, err, SIMULATION_SETUP_FAILED)
 
 	defer func() {
 		require.NoError(t, db.Close())
@@ -455,7 +457,7 @@ func TestAppSimulationAfterImport(t *testing.T) {
 		simcli.FlagVerboseValue,
 		simcli.FlagEnabledValue,
 	)
-	require.NoError(t, err, "simulation setup failed")
+	require.NoError(t, err, SIMULATION_SETUP_FAILED)
 
 	defer func() {
 		require.NoError(t, newDB.Close())

cmd/planetmint-god/cmd/genaccounts.go

@@ -8,6 +8,7 @@ import (
 
 	"github.com/cosmos/cosmos-sdk/client"
 	"github.com/cosmos/cosmos-sdk/client/flags"
+	"github.com/cosmos/cosmos-sdk/codec"
 	"github.com/cosmos/cosmos-sdk/crypto/keyring"
 	"github.com/cosmos/cosmos-sdk/server"
 	sdk "github.com/cosmos/cosmos-sdk/types"
@@ -36,7 +37,20 @@ the address will be looked up in the local Keybase. The list of initial tokens m
 contain valid denominations. Accounts may optionally be supplied with vesting parameters.
 `,
 		Args: cobra.ExactArgs(2),
-		RunE: func(cmd *cobra.Command, args []string) error {
+		RunE: addGenesisAccountCmdFunc,
+	}
+
+	cmd.Flags().String(flags.FlagKeyringBackend, flags.DefaultKeyringBackend, "Select keyring's backend (os|file|kwallet|pass|test)")
+	cmd.Flags().String(flags.FlagHome, defaultNodeHome, "The application home directory")
+	cmd.Flags().String(flagVestingAmt, "", "amount of coins for vesting accounts")
+	cmd.Flags().Int64(flagVestingStart, 0, "schedule start time (unix epoch) for vesting accounts")
+	cmd.Flags().Int64(flagVestingEnd, 0, "schedule end time (unix epoch) for vesting accounts")
+	flags.AddQueryFlagsToCmd(cmd)
+
+	return cmd
+}
+
+func addGenesisAccountCmdFunc(cmd *cobra.Command, args []string) error {
 	clientCtx := client.GetClientContextFromCmd(cmd)
 	cdc := clientCtx.Codec
 
@@ -52,27 +66,10 @@ contain valid denominations. Accounts may optionally be supplied with vesting pa
 
 	addr, err := sdk.AccAddressFromBech32(args[0])
 	if err != nil {
-				inBuf := bufio.NewReader(cmd.InOrStdin())
-				keyringBackend, err := cmd.Flags().GetString(flags.FlagKeyringBackend)
+		addr, err = resolveAddressFromKeybase(cmd, args[0], clientCtx.HomeDir, cdc)
 		if err != nil {
 			return err
 		}
-
-				// attempt to lookup address from Keybase if no address was provided
-				kb, err := keyring.New(sdk.KeyringServiceName(), keyringBackend, clientCtx.HomeDir, inBuf, cdc)
-				if err != nil {
-					return err
-				}
-
-				info, err := kb.Key(args[0])
-				if err != nil {
-					return fmt.Errorf("failed to get address from Keybase: %w", err)
-				}
-
-				addr, err = info.GetAddress()
-				if err != nil {
-					return fmt.Errorf("failed to get address from Keybase: %w", err)
-				}
 	}
 
 	vestingStart, err := cmd.Flags().GetInt64(flagVestingStart)
@@ -93,36 +90,9 @@ contain valid denominations. Accounts may optionally be supplied with vesting pa
 		return fmt.Errorf("failed to parse vesting amount: %w", err)
 	}
 
-			// create concrete account type based on input parameters
-			var genAccount authtypes.GenesisAccount
-
-			balances := banktypes.Balance{Address: addr.String(), Coins: coins.Sort()}
-			baseAccount := authtypes.NewBaseAccount(addr, nil, 0, 0)
-
-			if !vestingAmt.IsZero() {
-				baseVestingAccount := authvesting.NewBaseVestingAccount(baseAccount, vestingAmt.Sort(), vestingEnd)
-
-				if (balances.Coins.IsZero() && !baseVestingAccount.OriginalVesting.IsZero()) ||
-					baseVestingAccount.OriginalVesting.IsAnyGT(balances.Coins) {
-					return errors.New("vesting amount cannot be greater than total amount")
-				}
-
-				switch {
-				case vestingStart != 0 && vestingEnd != 0:
-					genAccount = authvesting.NewContinuousVestingAccountRaw(baseVestingAccount, vestingStart)
-
-				case vestingEnd != 0:
-					genAccount = authvesting.NewDelayedVestingAccountRaw(baseVestingAccount)
-
-				default:
-					return errors.New("invalid vesting parameters; must supply start and end time or end time")
-				}
-			} else {
-				genAccount = baseAccount
-			}
-
-			if err := genAccount.Validate(); err != nil {
-				return fmt.Errorf("failed to validate new genesis account: %w", err)
+	genAccount, err := createGenesisAccount(addr, coins, vestingAmt, vestingStart, vestingEnd)
+	if err != nil {
+		return err
 	}
 
 	genFile := config.GenesisFile()
@@ -131,6 +101,76 @@ contain valid denominations. Accounts may optionally be supplied with vesting pa
 		return fmt.Errorf("failed to unmarshal genesis state: %w", err)
 	}
 
+	if err := updateAuthGenesisState(cdc, appState, addr, genAccount); err != nil {
+		return err
+	}
+
+	if err := updateBankGenesisState(cdc, appState, addr, coins); err != nil {
+		return err
+	}
+
+	appStateJSON, err := json.Marshal(appState)
+	if err != nil {
+		return fmt.Errorf("failed to marshal application genesis state: %w", err)
+	}
+
+	genDoc.AppState = appStateJSON
+	return genutil.ExportGenesisFile(genDoc, genFile)
+}
+
+func resolveAddressFromKeybase(cmd *cobra.Command, keyName string, homeDir string, cdc codec.Codec) (sdk.AccAddress, error) {
+	inBuf := bufio.NewReader(cmd.InOrStdin())
+	keyringBackend, err := cmd.Flags().GetString(flags.FlagKeyringBackend)
+	if err != nil {
+		return nil, err
+	}
+
+	kb, err := keyring.New(sdk.KeyringServiceName(), keyringBackend, homeDir, inBuf, cdc)
+	if err != nil {
+		return nil, err
+	}
+
+	info, err := kb.Key(keyName)
+	if err != nil {
+		return nil, fmt.Errorf("failed to get address from Keybase: %w", err)
+	}
+
+	addr, err := info.GetAddress()
+	if err != nil {
+		return nil, fmt.Errorf("failed to get address from Keybase: %w", err)
+	}
+
+	return addr, nil
+}
+
+func createGenesisAccount(addr sdk.AccAddress, coins sdk.Coins, vestingAmt sdk.Coins, vestingStart, vestingEnd int64) (authtypes.GenesisAccount, error) {
+	balances := banktypes.Balance{Address: addr.String(), Coins: coins.Sort()}
+	baseAccount := authtypes.NewBaseAccount(addr, nil, 0, 0)
+
+	if !vestingAmt.IsZero() {
+		baseVestingAccount := authvesting.NewBaseVestingAccount(baseAccount, vestingAmt.Sort(), vestingEnd)
+
+		if (balances.Coins.IsZero() && !baseVestingAccount.OriginalVesting.IsZero()) ||
+			baseVestingAccount.OriginalVesting.IsAnyGT(balances.Coins) {
+			return nil, errors.New("vesting amount cannot be greater than total amount")
+		}
+
+		switch {
+		case vestingStart != 0 && vestingEnd != 0:
+			return authvesting.NewContinuousVestingAccountRaw(baseVestingAccount, vestingStart), nil
+
+		case vestingEnd != 0:
+			return authvesting.NewDelayedVestingAccountRaw(baseVestingAccount), nil
+
+		default:
+			return nil, errors.New("invalid vesting parameters; must supply start and end time or end time")
+		}
+	}
+
+	return baseAccount, nil
+}
+
+func updateAuthGenesisState(cdc codec.Codec, appState map[string]json.RawMessage, addr sdk.AccAddress, genAccount authtypes.GenesisAccount) error {
 	authGenState := authtypes.GetGenesisStateFromAppState(cdc, appState)
 
 	accs, err := authtypes.UnpackAccounts(authGenState.Accounts)
@@ -160,7 +200,12 @@ contain valid denominations. Accounts may optionally be supplied with vesting pa
 
 	appState[authtypes.ModuleName] = authGenStateBz
 
+	return nil
+}
+
+func updateBankGenesisState(cdc codec.Codec, appState map[string]json.RawMessage, addr sdk.AccAddress, coins sdk.Coins) error {
 	bankGenState := banktypes.GetGenesisStateFromAppState(cdc, appState)
+	balances := banktypes.Balance{Address: addr.String(), Coins: coins.Sort()}
 	bankGenState.Balances = append(bankGenState.Balances, balances)
 	bankGenState.Balances = banktypes.SanitizeGenesisBalances(bankGenState.Balances)
 
@@ -171,22 +216,5 @@ contain valid denominations. Accounts may optionally be supplied with vesting pa
 
 	appState[banktypes.ModuleName] = bankGenStateBz
 
-			appStateJSON, err := json.Marshal(appState)
-			if err != nil {
-				return fmt.Errorf("failed to marshal application genesis state: %w", err)
-			}
-
-			genDoc.AppState = appStateJSON
-			return genutil.ExportGenesisFile(genDoc, genFile)
-		},
-	}
-
-	cmd.Flags().String(flags.FlagKeyringBackend, flags.DefaultKeyringBackend, "Select keyring's backend (os|file|kwallet|pass|test)")
-	cmd.Flags().String(flags.FlagHome, defaultNodeHome, "The application home directory")
-	cmd.Flags().String(flagVestingAmt, "", "amount of coins for vesting accounts")
-	cmd.Flags().Int64(flagVestingStart, 0, "schedule start time (unix epoch) for vesting accounts")
-	cmd.Flags().Int64(flagVestingEnd, 0, "schedule end time (unix epoch) for vesting accounts")
-	flags.AddQueryFlagsToCmd(cmd)
-
-	return cmd
+	return nil
 }

config.yml

@@ -1,10 +1,11 @@
+---
 version: 1
 accounts:
-- name: alice
+  - name: alice
     coins:
       - 20000token
       - 200000000stake
-- name: bob
+  - name: bob
     coins:
       - 10000token
       - 100000000stake
@@ -17,5 +18,5 @@ faucet:
     - 5token
     - 100000stake
 validators:
-- name: alice
+  - name: alice
     bonded: 100000000stake

sonar-project.properties

@@ -1,2 +1,2 @@
 sonar.projectKey=planetmint_planetmint-go_AYjnSLNdwwdSy162QoXI
-sonar.exclusions=docs/static/openapi.yml,x/**/*.pb.go
+sonar.exclusions=docs/static/openapi.yml,x/**/*.pb.go,x/**/*.pb.gw.go

x/asset/types/genesis_test.go

@@ -3,11 +3,12 @@ package types_test
 import (
 	"testing"
 
-	"github.com/stretchr/testify/require"
 	"planetmint-go/x/asset/types"
+
+	"github.com/stretchr/testify/require"
 )
 
-func TestGenesisState_Validate(t *testing.T) {
+func TestGenesisStateValidate(t *testing.T) {
 	tests := []struct {
 		desc     string
 		genState *types.GenesisState

x/asset/types/message_notarize_asset_test.go

@@ -3,12 +3,13 @@ package types
 import (
 	"testing"
 
+	"planetmint-go/testutil/sample"
+
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 	"github.com/stretchr/testify/require"
-	"planetmint-go/testutil/sample"
 )
 
-func TestMsgNotarizeAsset_ValidateBasic(t *testing.T) {
+func TestMsgNotarizeAssetValidateBasic(t *testing.T) {
 	tests := []struct {
 		name string
 		msg  MsgNotarizeAsset

x/machine/types/genesis_test.go

@@ -3,11 +3,12 @@ package types_test
 import (
 	"testing"
 
-	"github.com/stretchr/testify/require"
 	"planetmint-go/x/machine/types"
+
+	"github.com/stretchr/testify/require"
 )
 
-func TestGenesisState_Validate(t *testing.T) {
+func TestGenesisStateValidate(t *testing.T) {
 	tests := []struct {
 		desc     string
 		genState *types.GenesisState