From 1a745eccfb522725a3e0dd548c347aec14edf7b5 Mon Sep 17 00:00:00 2001 From: Alessio Treglia Date: Tue, 3 Mar 2020 16:57:33 +0000 Subject: [PATCH 1/3] fix types.ChainAnteDecorators() panic (#5742) 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 --- CHANGELOG.md | 1 + types/handler.go | 12 ++++++------ types/handler_test.go | 28 ++++++++++++++++++++++++++++ 3 files changed, 35 insertions(+), 6 deletions(-) create mode 100644 types/handler_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 71d574731d46..d9d17a438cce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -81,6 +81,7 @@ options `pruning-keep-recent`, `pruning-keep-every`, and `pruning-interval`. The dictate how many recent versions are kept on disk and the offset of what versions are kept after that respectively, and the latter defines the height interval in which versions are deleted in a batch. **Note, there are some client-facing API breaking changes with regard to IAVL, stores, and pruning settings.** +* (types) [\#5741](https://github.com/cosmos/cosmos-sdk/issues/5741) Prevent ChainAnteDecorators() from panicking when empty AnteDecorator slice is supplied. ## [v0.38.4] - 2020-05-21 diff --git a/types/handler.go b/types/handler.go index 6815230a861f..03d1f02d5806 100644 --- a/types/handler.go +++ b/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) { diff --git a/types/handler_test.go b/types/handler_test.go new file mode 100644 index 000000000000..4847a113137a --- /dev/null +++ b/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) +} From f1550456e556ce5872f47ebf284b0d8f535cef7b Mon Sep 17 00:00:00 2001 From: Alessio Treglia Date: Fri, 10 Jul 2020 07:25:43 +0100 Subject: [PATCH 2/3] fix tests --- Makefile | 1 + tests/mocks/account_retriever.go | 13 +++++++-- tests/mocks/types_handler.go | 49 ++++++++++++++++++++++++++++++++ 3 files changed, 61 insertions(+), 2 deletions(-) create mode 100644 tests/mocks/types_handler.go diff --git a/Makefile b/Makefile index 8d947640f75a..9d857bc8ca7c 100644 --- a/Makefile +++ b/Makefile @@ -35,6 +35,7 @@ update-swagger-docs: statik mocks: $(MOCKS_DIR) mockgen -source=x/auth/types/account_retriever.go -package mocks -destination tests/mocks/account_retriever.go + mockgen -source=types/handler.go -package mocks -destination tests/mocks/types_handler.go .PHONY: mocks $(MOCKS_DIR): diff --git a/tests/mocks/account_retriever.go b/tests/mocks/account_retriever.go index 9f8ee69282a2..a3086c77bdca 100644 --- a/tests/mocks/account_retriever.go +++ b/tests/mocks/account_retriever.go @@ -1,30 +1,38 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: x/auth/types/account_retriever.go + +// Package mocks is a generated GoMock package. package mocks import ( - reflect "reflect" - gomock "github.com/golang/mock/gomock" + reflect "reflect" ) +// MockNodeQuerier is a mock of NodeQuerier interface type MockNodeQuerier struct { ctrl *gomock.Controller recorder *MockNodeQuerierMockRecorder } +// MockNodeQuerierMockRecorder is the mock recorder for MockNodeQuerier type MockNodeQuerierMockRecorder struct { mock *MockNodeQuerier } +// NewMockNodeQuerier creates a new mock instance func NewMockNodeQuerier(ctrl *gomock.Controller) *MockNodeQuerier { mock := &MockNodeQuerier{ctrl: ctrl} mock.recorder = &MockNodeQuerierMockRecorder{mock} return mock } +// EXPECT returns an object that allows the caller to indicate expected use func (m *MockNodeQuerier) EXPECT() *MockNodeQuerierMockRecorder { return m.recorder } +// QueryWithData mocks base method func (m *MockNodeQuerier) QueryWithData(path string, data []byte) ([]byte, int64, error) { m.ctrl.T.Helper() ret := m.ctrl.Call(m, "QueryWithData", path, data) @@ -34,6 +42,7 @@ func (m *MockNodeQuerier) QueryWithData(path string, data []byte) ([]byte, int64 return ret0, ret1, ret2 } +// QueryWithData indicates an expected call of QueryWithData func (mr *MockNodeQuerierMockRecorder) QueryWithData(path, data interface{}) *gomock.Call { mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "QueryWithData", reflect.TypeOf((*MockNodeQuerier)(nil).QueryWithData), path, data) diff --git a/tests/mocks/types_handler.go b/tests/mocks/types_handler.go new file mode 100644 index 000000000000..da80614ae884 --- /dev/null +++ b/tests/mocks/types_handler.go @@ -0,0 +1,49 @@ +// Code generated by MockGen. DO NOT EDIT. +// Source: types/handler.go + +// Package mocks is a generated GoMock package. +package mocks + +import ( + types "github.com/cosmos/cosmos-sdk/types" + gomock "github.com/golang/mock/gomock" + reflect "reflect" +) + +// MockAnteDecorator is a mock of AnteDecorator interface +type MockAnteDecorator struct { + ctrl *gomock.Controller + recorder *MockAnteDecoratorMockRecorder +} + +// MockAnteDecoratorMockRecorder is the mock recorder for MockAnteDecorator +type MockAnteDecoratorMockRecorder struct { + mock *MockAnteDecorator +} + +// NewMockAnteDecorator creates a new mock instance +func NewMockAnteDecorator(ctrl *gomock.Controller) *MockAnteDecorator { + mock := &MockAnteDecorator{ctrl: ctrl} + mock.recorder = &MockAnteDecoratorMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use +func (m *MockAnteDecorator) EXPECT() *MockAnteDecoratorMockRecorder { + return m.recorder +} + +// AnteHandle mocks base method +func (m *MockAnteDecorator) AnteHandle(ctx types.Context, tx types.Tx, simulate bool, next types.AnteHandler) (types.Context, error) { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "AnteHandle", ctx, tx, simulate, next) + ret0, _ := ret[0].(types.Context) + ret1, _ := ret[1].(error) + return ret0, ret1 +} + +// AnteHandle indicates an expected call of AnteHandle +func (mr *MockAnteDecoratorMockRecorder) AnteHandle(ctx, tx, simulate, next interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "AnteHandle", reflect.TypeOf((*MockAnteDecorator)(nil).AnteHandle), ctx, tx, simulate, next) +} From fe18579a681efca75b4a95c83bb1261aba54ce86 Mon Sep 17 00:00:00 2001 From: Alessio Treglia Date: Fri, 10 Jul 2020 09:39:39 +0100 Subject: [PATCH 3/3] Update CHANGELOG.md Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com> --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d9d17a438cce..8eb96c4b4410 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -81,7 +81,7 @@ options `pruning-keep-recent`, `pruning-keep-every`, and `pruning-interval`. The dictate how many recent versions are kept on disk and the offset of what versions are kept after that respectively, and the latter defines the height interval in which versions are deleted in a batch. **Note, there are some client-facing API breaking changes with regard to IAVL, stores, and pruning settings.** -* (types) [\#5741](https://github.com/cosmos/cosmos-sdk/issues/5741) Prevent ChainAnteDecorators() from panicking when empty AnteDecorator slice is supplied. +* (types) [\#5741](https://github.com/cosmos/cosmos-sdk/issues/5741) Prevent `ChainAnteDecorators()` from panicking when empty `AnteDecorator` slice is supplied. ## [v0.38.4] - 2020-05-21