From e36572d01b741e3271c470371005c9e4e12d1f53 Mon Sep 17 00:00:00 2001 From: Facundo Medica <14063057+facundomedica@users.noreply.github.com> Date: Wed, 1 Feb 2023 11:20:39 -0300 Subject: [PATCH] feat: Implement BeginBlock EndBlock for Core API modules (#14819) Co-authored-by: Aleksandr Bezobchuk --- testutil/mock/types_mock_appmodule.go | 28 ++++++++++ types/module/mock_appmodule_test.go | 2 + types/module/module.go | 39 ++++++++----- types/module/module_test.go | 79 ++++++++++++++++++++++++--- 4 files changed, 124 insertions(+), 24 deletions(-) diff --git a/testutil/mock/types_mock_appmodule.go b/testutil/mock/types_mock_appmodule.go index ce0aff5b86a3..e19a27f1b20a 100644 --- a/testutil/mock/types_mock_appmodule.go +++ b/testutil/mock/types_mock_appmodule.go @@ -265,6 +265,20 @@ func (m *MockCoreAppModule) EXPECT() *MockCoreAppModuleMockRecorder { return m.recorder } +// BeginBlock mocks base method. +func (m *MockCoreAppModule) BeginBlock(arg0 context.Context) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "BeginBlock", arg0) + ret0, _ := ret[0].(error) + return ret0 +} + +// BeginBlock indicates an expected call of BeginBlock. +func (mr *MockCoreAppModuleMockRecorder) BeginBlock(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "BeginBlock", reflect.TypeOf((*MockCoreAppModule)(nil).BeginBlock), arg0) +} + // DefaultGenesis mocks base method. func (m *MockCoreAppModule) DefaultGenesis(arg0 appmodule.GenesisTarget) error { m.ctrl.T.Helper() @@ -279,6 +293,20 @@ func (mr *MockCoreAppModuleMockRecorder) DefaultGenesis(arg0 interface{}) *gomoc return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "DefaultGenesis", reflect.TypeOf((*MockCoreAppModule)(nil).DefaultGenesis), arg0) } +// EndBlock mocks base method. +func (m *MockCoreAppModule) EndBlock(arg0 context.Context) error { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "EndBlock", arg0) + ret0, _ := ret[0].(error) + return ret0 +} + +// EndBlock indicates an expected call of EndBlock. +func (mr *MockCoreAppModuleMockRecorder) EndBlock(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "EndBlock", reflect.TypeOf((*MockCoreAppModule)(nil).EndBlock), arg0) +} + // ExportGenesis mocks base method. func (m *MockCoreAppModule) ExportGenesis(arg0 context.Context, arg1 appmodule.GenesisTarget) error { m.ctrl.T.Helper() diff --git a/types/module/mock_appmodule_test.go b/types/module/mock_appmodule_test.go index 94d8c466b396..d53c6fae0cd5 100644 --- a/types/module/mock_appmodule_test.go +++ b/types/module/mock_appmodule_test.go @@ -23,4 +23,6 @@ type AppModuleWithAllExtensions interface { type CoreAppModule interface { appmodule.AppModule appmodule.HasGenesis + appmodule.HasBeginBlocker + appmodule.HasEndBlocker } diff --git a/types/module/module.go b/types/module/module.go index 2b9ab3eb8424..41d76fdc5d72 100644 --- a/types/module/module.go +++ b/types/module/module.go @@ -416,7 +416,7 @@ func (m *Manager) InitGenesis(ctx sdk.Context, cdc codec.JSONCodec, genesisData // a chain must initialize with a non-empty validator set if len(validatorUpdates) == 0 { - panic(fmt.Sprintf("validator set is empty after InitGenesis, please ensure at least one validator is initialized with a delegation greater than or equal to the DefaultPowerReduction (%d)", sdk.DefaultPowerReduction)) + return abci.ResponseInitChain{}, fmt.Errorf("validator set is empty after InitGenesis, please ensure at least one validator is initialized with a delegation greater than or equal to the DefaultPowerReduction (%d)", sdk.DefaultPowerReduction) } return abci.ResponseInitChain{ @@ -640,9 +640,13 @@ func (m *Manager) BeginBlock(ctx sdk.Context, req abci.RequestBeginBlock) (abci. ctx = ctx.WithEventManager(sdk.NewEventManager()) for _, moduleName := range m.OrderBeginBlockers { - module, ok := m.Modules[moduleName].(BeginBlockAppModule) - if ok { + if module, ok := m.Modules[moduleName].(BeginBlockAppModule); ok { module.BeginBlock(ctx, req) + } else if module, ok := m.Modules[moduleName].(appmodule.HasBeginBlocker); ok { + err := module.BeginBlock(ctx) + if err != nil { + return abci.ResponseBeginBlock{}, err + } } } @@ -659,20 +663,25 @@ func (m *Manager) EndBlock(ctx sdk.Context, req abci.RequestEndBlock) (abci.Resp validatorUpdates := []abci.ValidatorUpdate{} for _, moduleName := range m.OrderEndBlockers { - module, ok := m.Modules[moduleName].(EndBlockAppModule) - if !ok { - continue - } - moduleValUpdates := module.EndBlock(ctx, req) + if module, ok := m.Modules[moduleName].(EndBlockAppModule); ok { + moduleValUpdates := module.EndBlock(ctx, req) - // use these validator updates if provided, the module manager assumes - // only one module will update the validator set - if len(moduleValUpdates) > 0 { - if len(validatorUpdates) > 0 { - return abci.ResponseEndBlock{}, errors.New("validator EndBlock updates already set by a previous module") - } + // use these validator updates if provided, the module manager assumes + // only one module will update the validator set + if len(moduleValUpdates) > 0 { + if len(validatorUpdates) > 0 { + return abci.ResponseEndBlock{}, errors.New("validator EndBlock updates already set by a previous module") + } - validatorUpdates = moduleValUpdates + validatorUpdates = moduleValUpdates + } + } else if module, ok := m.Modules[moduleName].(appmodule.HasEndBlocker); ok { + err := module.EndBlock(ctx) + if err != nil { + return abci.ResponseEndBlock{}, err + } + } else { + continue } } diff --git a/types/module/module_test.go b/types/module/module_test.go index 7fa51989cc4a..1989d703451e 100644 --- a/types/module/module_test.go +++ b/types/module/module_test.go @@ -194,7 +194,8 @@ func TestManager_InitGenesis(t *testing.T) { // this should panic since the validator set is empty even after init genesis mockAppModule1.EXPECT().InitGenesis(gomock.Eq(ctx), gomock.Eq(cdc), gomock.Eq(genesisData["module1"])).Times(1).Return(nil) - require.Panics(t, func() { mm.InitGenesis(ctx, cdc, genesisData) }) + _, err := mm.InitGenesis(ctx, cdc, genesisData) + require.ErrorContains(t, err, "validator set is empty after InitGenesis") // test panic genesisData = map[string]json.RawMessage{ @@ -206,14 +207,15 @@ func TestManager_InitGenesis(t *testing.T) { // panic because more than one module returns validator set updates mockAppModule1.EXPECT().InitGenesis(gomock.Eq(ctx), gomock.Eq(cdc), gomock.Eq(genesisData["module1"])).Times(1).Return([]abci.ValidatorUpdate{{}}) mockAppModule2.EXPECT().InitGenesis(gomock.Eq(ctx), gomock.Eq(cdc), gomock.Eq(genesisData["module2"])).Times(1).Return([]abci.ValidatorUpdate{{}}) - _, err := mm.InitGenesis(ctx, cdc, genesisData) - require.Error(t, err) + _, err = mm.InitGenesis(ctx, cdc, genesisData) + require.ErrorContains(t, err, "validator InitGenesis updates already set by a previous module") // happy path mockAppModule1.EXPECT().InitGenesis(gomock.Eq(ctx), gomock.Eq(cdc), gomock.Eq(genesisData["module1"])).Times(1).Return([]abci.ValidatorUpdate{{}}) mockAppModule2.EXPECT().InitGenesis(gomock.Eq(ctx), gomock.Eq(cdc), gomock.Eq(genesisData["module2"])).Times(1).Return([]abci.ValidatorUpdate{}) mockAppModule3.EXPECT().InitGenesis(gomock.Eq(ctx), gomock.Any()).Times(1).Return(nil) - mm.InitGenesis(ctx, cdc, genesisData) + _, err = mm.InitGenesis(ctx, cdc, genesisData) + require.NoError(t, err) } func TestManager_ExportGenesis(t *testing.T) { @@ -279,7 +281,8 @@ func TestManager_BeginBlock(t *testing.T) { mockAppModule1.EXPECT().BeginBlock(gomock.Any(), gomock.Eq(req)).Times(1) mockAppModule2.EXPECT().BeginBlock(gomock.Any(), gomock.Eq(req)).Times(1) - mm.BeginBlock(sdk.Context{}, req) + _, err := mm.BeginBlock(sdk.Context{}, req) + require.NoError(t, err) } func TestManager_EndBlock(t *testing.T) { @@ -288,11 +291,13 @@ func TestManager_EndBlock(t *testing.T) { mockAppModule1 := mock.NewMockEndBlockAppModule(mockCtrl) mockAppModule2 := mock.NewMockEndBlockAppModule(mockCtrl) + mockAppModule3 := mock.NewMockAppModule(mockCtrl) mockAppModule1.EXPECT().Name().Times(2).Return("module1") mockAppModule2.EXPECT().Name().Times(2).Return("module2") - mm := module.NewManager(mockAppModule1, mockAppModule2) + mockAppModule3.EXPECT().Name().Times(2).Return("module3") + mm := module.NewManager(mockAppModule1, mockAppModule2, mockAppModule3) require.NotNil(t, mm) - require.Equal(t, 2, len(mm.Modules)) + require.Equal(t, 3, len(mm.Modules)) req := abci.RequestEndBlock{Height: 10} @@ -341,9 +346,11 @@ func TestCoreAPIManager_InitGenesis(t *testing.T) { // this should panic since the validator set is empty even after init genesis mockAppModule1.EXPECT().InitGenesis(gomock.Eq(ctx), gomock.Any()).Times(1).Return(nil) - require.Panics(t, func() { mm.InitGenesis(ctx, cdc, genesisData) }) + _, err := mm.InitGenesis(ctx, cdc, genesisData) + require.ErrorContains(t, err, "validator set is empty after InitGenesis, please ensure at least one validator is initialized with a delegation greater than or equal to the DefaultPowerReduction") - // TODO: add happy path test. We are not returning any validator updates + // TODO: add happy path test. We are not returning any validator updates, this will come with the services. + // REF: https://github.com/cosmos/cosmos-sdk/issues/14688 } func TestCoreAPIManager_ExportGenesis(t *testing.T) { @@ -421,6 +428,60 @@ func TestCoreAPIManagerOrderSetters(t *testing.T) { require.Equal(t, []string{"module2", "module1", "module3"}, mm.OrderEndBlockers) } +func TestCoreAPIManager_BeginBlock(t *testing.T) { + mockCtrl := gomock.NewController(t) + t.Cleanup(mockCtrl.Finish) + + mockAppModule1 := mock.NewMockCoreAppModule(mockCtrl) + mockAppModule2 := mock.NewMockCoreAppModule(mockCtrl) + mm := module.NewManagerFromMap(map[string]appmodule.AppModule{ + "module1": mockAppModule1, + "module2": mockAppModule2, + }) + require.NotNil(t, mm) + require.Equal(t, 2, len(mm.Modules)) + + req := abci.RequestBeginBlock{Hash: []byte("test")} + + mockAppModule1.EXPECT().BeginBlock(gomock.Any()).Times(1).Return(nil) + mockAppModule2.EXPECT().BeginBlock(gomock.Any()).Times(1).Return(nil) + _, err := mm.BeginBlock(sdk.Context{}, req) + require.NoError(t, err) + + // test panic + mockAppModule1.EXPECT().BeginBlock(gomock.Any()).Times(1).Return(errors.New("some error")) + _, err = mm.BeginBlock(sdk.Context{}, req) + require.EqualError(t, err, "some error") + +} + +func TestCoreAPIManager_EndBlock(t *testing.T) { + mockCtrl := gomock.NewController(t) + t.Cleanup(mockCtrl.Finish) + + mockAppModule1 := mock.NewMockCoreAppModule(mockCtrl) + mockAppModule2 := mock.NewMockCoreAppModule(mockCtrl) + mm := module.NewManagerFromMap(map[string]appmodule.AppModule{ + "module1": mockAppModule1, + "module2": mockAppModule2, + }) + require.NotNil(t, mm) + require.Equal(t, 2, len(mm.Modules)) + + req := abci.RequestEndBlock{Height: 10} + + mockAppModule1.EXPECT().EndBlock(gomock.Any()).Times(1).Return(nil) + mockAppModule2.EXPECT().EndBlock(gomock.Any()).Times(1).Return(nil) + res, err := mm.EndBlock(sdk.Context{}, req) + require.NoError(t, err) + require.Len(t, res.ValidatorUpdates, 0) + + // test panic + mockAppModule1.EXPECT().EndBlock(gomock.Any()).Times(1).Return(errors.New("some error")) + _, err = mm.EndBlock(sdk.Context{}, req) + require.EqualError(t, err, "some error") +} + // MockCoreAppModule allows us to test functions like DefaultGenesis type MockCoreAppModule struct{}