Merge branch 'jonathan/5646-remove-internal-crisis' of github.com:cos…

…mos/cosmos-sdk into jonathan/5646-remove-internal-crisis

CHANGELOG.md

@@ -74,6 +74,7 @@ resulted in a panic when the tx execution mode was `CheckTx`.
 * (client) [\#5618](https://github.com/cosmos/cosmos-sdk/pull/5618) Fix crash on the client when the verifier is not set.
 * (x/distribution) [\#5620](https://github.com/cosmos/cosmos-sdk/pull/5620) Fix nil pointer deref in distribution tax/rewward validation helpers.
 * (genesis) [\#5086](https://github.com/cosmos/cosmos-sdk/issues/5086) Ensure `gentxs` are always an empty array instead of `nil`
+* (types) [\#5741](https://github.com/cosmos/cosmos-sdk/issues/5741) Prevent ChainAnteDecorators() from panicking when empty AnteDecorator slice is supplied.
 
 ### State Machine Breaking
 

simapp/test_helpers.go

@@ -116,6 +116,25 @@ func createIncrementalAccounts(accNum int) []sdk.AccAddress {
 	return addresses
 }
 
+// AddTestAddrsFromPubKeys adds the addresses into the SimApp providing only the public keys.
+func AddTestAddrsFromPubKeys(app *SimApp, ctx sdk.Context, pubKeys []crypto.PubKey, accAmt sdk.Int) {
+	initCoins := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), accAmt))
+
+	setTotalSupply(app, ctx, accAmt, len(pubKeys))
+
+	// fill all the addresses with some coins, set the loose pool tokens simultaneously
+	for _, pubKey := range pubKeys {
+		saveAccount(app, ctx, sdk.AccAddress(pubKey.Address()), initCoins)
+	}
+}
+
+// setTotalSupply provides the total supply based on accAmt * totalAccounts.
+func setTotalSupply(app *SimApp, ctx sdk.Context, accAmt sdk.Int, totalAccounts int) {
+	totalSupply := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), accAmt.MulRaw(int64(totalAccounts))))
+	prevSupply := app.SupplyKeeper.GetSupply(ctx)
+	app.SupplyKeeper.SetSupply(ctx, supply.NewSupply(prevSupply.GetTotal().Add(totalSupply...)))
+}
+
 // AddTestAddrs constructs and returns accNum amount of accounts with an
 // initial balance of accAmt in random order
 func AddTestAddrs(app *SimApp, ctx sdk.Context, accNum int, accAmt sdk.Int) []sdk.AccAddress {
@@ -132,23 +151,26 @@ func addTestAddrs(app *SimApp, ctx sdk.Context, accNum int, accAmt sdk.Int, stra
 	testAddrs := strategy(accNum)
 
 	initCoins := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), accAmt))
-	totalSupply := sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.BondDenom(ctx), accAmt.MulRaw(int64(len(testAddrs)))))
-	prevSupply := app.SupplyKeeper.GetSupply(ctx)
-	app.SupplyKeeper.SetSupply(ctx, supply.NewSupply(prevSupply.GetTotal().Add(totalSupply...)))
+	setTotalSupply(app, ctx, accAmt, accNum)
 
 	// fill all the addresses with some coins, set the loose pool tokens simultaneously
 	for _, addr := range testAddrs {
-		acc := app.AccountKeeper.NewAccountWithAddress(ctx, addr)
-		app.AccountKeeper.SetAccount(ctx, acc)
-
-		_, err := app.BankKeeper.AddCoins(ctx, addr, initCoins)
-		if err != nil {
-			panic(err)
-		}
+		saveAccount(app, ctx, addr, initCoins)
 	}
+
 	return testAddrs
 }
 
+// saveAccount saves the provided account into the simapp with balance based on initCoins.
+func saveAccount(app *SimApp, ctx sdk.Context, addr sdk.AccAddress, initCoins sdk.Coins) {
+	acc := app.AccountKeeper.NewAccountWithAddress(ctx, addr)
+	app.AccountKeeper.SetAccount(ctx, acc)
+	_, err := app.BankKeeper.AddCoins(ctx, addr, initCoins)
+	if err != nil {
+		panic(err)
+	}
+}
+
 // ConvertAddrsToValAddrs converts the provided addresses to ValAddress.
 func ConvertAddrsToValAddrs(addrs []sdk.AccAddress) []sdk.ValAddress {
 	valAddrs := make([]sdk.ValAddress, len(addrs))

types/handler.go

@@ -25,15 +25,15 @@ type AnteDecorator interface {
 // MUST set GasMeter with the FIRST AnteDecorator. Failing to do so will cause
 // transactions to be processed with an infinite gasmeter and open a DOS attack vector.
 // Use `ante.SetUpContextDecorator` or a custom Decorator with similar functionality.
+// Returns nil when no AnteDecorator are supplied.
 func ChainAnteDecorators(chain ...AnteDecorator) AnteHandler {
-	if (chain[len(chain)-1] != Terminator{}) {
-		chain = append(chain, Terminator{})
+	if len(chain) == 0 {
+		return nil
 	}
 
-	if len(chain) == 1 {
-		return func(ctx Context, tx Tx, simulate bool) (Context, error) {
-			return chain[0].AnteHandle(ctx, tx, simulate, nil)
-		}
+	// handle non-terminated decorators chain
+	if (chain[len(chain)-1] != Terminator{}) {
+		chain = append(chain, Terminator{})
 	}
 
 	return func(ctx Context, tx Tx, simulate bool) (Context, error) {

types/handler_test.go

@@ -0,0 +1,28 @@
+package types_test
+
+import (
+	"testing"
+
+	"github.com/golang/mock/gomock"
+	"github.com/stretchr/testify/require"
+
+	"github.com/cosmos/cosmos-sdk/tests/mocks"
+	sdk "github.com/cosmos/cosmos-sdk/types"
+)
+
+func TestChainAnteDecorators(t *testing.T) {
+	t.Parallel()
+	// test panic
+	require.Nil(t, sdk.ChainAnteDecorators([]sdk.AnteDecorator{}...))
+
+	ctx, tx := sdk.Context{}, sdk.Tx(nil)
+	mockCtrl := gomock.NewController(t)
+	mockAnteDecorator1 := mocks.NewMockAnteDecorator(mockCtrl)
+	mockAnteDecorator1.EXPECT().AnteHandle(gomock.Eq(ctx), gomock.Eq(tx), true, gomock.Any()).Times(1)
+	sdk.ChainAnteDecorators(mockAnteDecorator1)(ctx, tx, true)
+
+	mockAnteDecorator2 := mocks.NewMockAnteDecorator(mockCtrl)
+	mockAnteDecorator1.EXPECT().AnteHandle(gomock.Eq(ctx), gomock.Eq(tx), true, mockAnteDecorator2).Times(1)
+	mockAnteDecorator2.EXPECT().AnteHandle(gomock.Eq(ctx), gomock.Eq(tx), true, nil).Times(1)
+	sdk.ChainAnteDecorators(mockAnteDecorator1, mockAnteDecorator2)
+}

x/slashing/abci_test.go

@@ -4,32 +4,37 @@ import (
 	"testing"
 	"time"
 
-	"github.com/stretchr/testify/require"
-	abci "github.com/tendermint/tendermint/abci/types"
-
+	"github.com/cosmos/cosmos-sdk/simapp"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/cosmos-sdk/x/slashing"
 	slashingkeeper "github.com/cosmos/cosmos-sdk/x/slashing/keeper"
 	"github.com/cosmos/cosmos-sdk/x/staking"
+	"github.com/stretchr/testify/require"
+	abci "github.com/tendermint/tendermint/abci/types"
 )
 
 func TestBeginBlocker(t *testing.T) {
-	ctx, bk, sk, _, keeper := slashingkeeper.CreateTestInput(t, slashing.DefaultParams())
+	app := simapp.Setup(false)
+	ctx := app.BaseApp.NewContext(false, abci.Header{})
+
+	pks := simapp.CreateTestPubKeys(1)
+	simapp.AddTestAddrsFromPubKeys(app, ctx, pks, sdk.TokensFromConsensusPower(200))
+
 	power := int64(100)
 	amt := sdk.TokensFromConsensusPower(power)
-	addr, pk := slashingkeeper.Addrs[2], slashingkeeper.Pks[2]
+	addr, pk := sdk.ValAddress(pks[0].Address()), pks[0]
 
 	// bond the validator
-	res, err := staking.NewHandler(sk)(ctx, slashingkeeper.NewTestMsgCreateValidator(addr, pk, amt))
+	res, err := staking.NewHandler(app.StakingKeeper)(ctx, slashingkeeper.NewTestMsgCreateValidator(addr, pk, amt))
 	require.NoError(t, err)
 	require.NotNil(t, res)
 
-	staking.EndBlocker(ctx, sk)
+	staking.EndBlocker(ctx, app.StakingKeeper)
 	require.Equal(
-		t, bk.GetAllBalances(ctx, sdk.AccAddress(addr)),
-		sdk.NewCoins(sdk.NewCoin(sk.GetParams(ctx).BondDenom, slashingkeeper.InitTokens.Sub(amt))),
+		t, app.BankKeeper.GetAllBalances(ctx, sdk.AccAddress(addr)),
+		sdk.NewCoins(sdk.NewCoin(app.StakingKeeper.GetParams(ctx).BondDenom, slashingkeeper.InitTokens.Sub(amt))),
 	)
-	require.Equal(t, amt, sk.Validator(ctx, addr).GetBondedTokens())
+	require.Equal(t, amt, app.StakingKeeper.Validator(ctx, addr).GetBondedTokens())
 
 	val := abci.Validator{
 		Address: pk.Address(),
@@ -46,9 +51,9 @@ func TestBeginBlocker(t *testing.T) {
 		},
 	}
 
-	slashing.BeginBlocker(ctx, req, keeper)
+	slashing.BeginBlocker(ctx, req, app.SlashingKeeper)
 
-	info, found := keeper.GetValidatorSigningInfo(ctx, sdk.ConsAddress(pk.Address()))
+	info, found := app.SlashingKeeper.GetValidatorSigningInfo(ctx, sdk.ConsAddress(pk.Address()))
 	require.True(t, found)
 	require.Equal(t, ctx.BlockHeight(), info.StartHeight)
 	require.Equal(t, int64(1), info.IndexOffset)
@@ -58,7 +63,7 @@ func TestBeginBlocker(t *testing.T) {
 	height := int64(0)
 
 	// for 1000 blocks, mark the validator as having signed
-	for ; height < keeper.SignedBlocksWindow(ctx); height++ {
+	for ; height < app.SlashingKeeper.SignedBlocksWindow(ctx); height++ {
 		ctx = ctx.WithBlockHeight(height)
 		req = abci.RequestBeginBlock{
 			LastCommitInfo: abci.LastCommitInfo{
@@ -69,11 +74,11 @@ func TestBeginBlocker(t *testing.T) {
 			},
 		}
 
-		slashing.BeginBlocker(ctx, req, keeper)
+		slashing.BeginBlocker(ctx, req, app.SlashingKeeper)
 	}
 
 	// for 500 blocks, mark the validator as having not signed
-	for ; height < ((keeper.SignedBlocksWindow(ctx) * 2) - keeper.MinSignedPerWindow(ctx) + 1); height++ {
+	for ; height < ((app.SlashingKeeper.SignedBlocksWindow(ctx) * 2) - app.SlashingKeeper.MinSignedPerWindow(ctx) + 1); height++ {
 		ctx = ctx.WithBlockHeight(height)
 		req = abci.RequestBeginBlock{
 			LastCommitInfo: abci.LastCommitInfo{
@@ -84,14 +89,14 @@ func TestBeginBlocker(t *testing.T) {
 			},
 		}
 
-		slashing.BeginBlocker(ctx, req, keeper)
+		slashing.BeginBlocker(ctx, req, app.SlashingKeeper)
 	}
 
 	// end block
-	staking.EndBlocker(ctx, sk)
+	staking.EndBlocker(ctx, app.StakingKeeper)
 
 	// validator should be jailed
-	validator, found := sk.GetValidatorByConsAddr(ctx, sdk.GetConsAddress(pk))
+	validator, found := app.StakingKeeper.GetValidatorByConsAddr(ctx, sdk.GetConsAddress(pk))
 	require.True(t, found)
 	require.Equal(t, sdk.Unbonding, validator.GetStatus())
 }