Skip to content

Commit

Permalink
fix types.ChainAnteDecorators() panic
Browse files Browse the repository at this point in the history
ChainAnteDecorators() panics when no arguments are supplied.
This change its behaviour and the function now returns a nil
AnteHandler in case no AnteDecorator instances are supplied.

Closes: #5741
  • Loading branch information
Alessio Treglia committed Mar 3, 2020
1 parent 3349c97 commit 3d8dc52
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 2 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
10 changes: 8 additions & 2 deletions types/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,10 @@ 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 {
Expand All @@ -36,6 +37,11 @@ func ChainAnteDecorators(chain ...AnteDecorator) AnteHandler {
}
}

// 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) {
return chain[0].AnteHandle(ctx, tx, simulate, ChainAnteDecorators(chain[1:]...))
}
Expand Down
28 changes: 28 additions & 0 deletions types/handler_test.go
Original file line number Diff line number Diff line change
@@ -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, nil).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)
}

0 comments on commit 3d8dc52

Please sign in to comment.