From 0e85c497c44a2ace0c4adf0cc783a49454e43e3d Mon Sep 17 00:00:00 2001 From: dydxwill <119354122+dydxwill@users.noreply.github.com> Date: Thu, 23 Feb 2023 10:37:28 -0500 Subject: [PATCH] add precommit callback (#7) --- baseapp/abci.go | 4 + baseapp/abci_test.go | 50 +++++++++++- baseapp/baseapp.go | 1 + baseapp/baseapp_test.go | 3 + baseapp/options.go | 8 ++ testutil/mock/types_module_module.go | 113 +++++++++++++++++++++++++++ types/abci.go | 3 + types/module/module.go | 24 ++++++ types/module/module_test.go | 21 +++++ 9 files changed, 226 insertions(+), 1 deletion(-) diff --git a/baseapp/abci.go b/baseapp/abci.go index d65735279ec9..a9c374a35381 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -442,6 +442,10 @@ func (app *BaseApp) Commit() abci.ResponseCommit { app.setState(runTxPrepareProposal, header) app.setState(runTxProcessProposal, header) + if app.precommiter != nil { + app.precommiter(app.deliverState.ctx) + } + // empty/reset the deliver state app.deliverState = nil diff --git a/baseapp/abci_test.go b/baseapp/abci_test.go index 7d0ffbf750d4..4daaa64dbdda 100644 --- a/baseapp/abci_test.go +++ b/baseapp/abci_test.go @@ -618,6 +618,33 @@ func TestBaseApp_Commit(t *testing.T) { require.Equal(t, true, wasCommiterCalled) } +func TestBaseApp_Precommit(t *testing.T) { + db := dbm.NewMemDB() + name := t.Name() + logger := defaultLogger() + + cp := &tmproto.ConsensusParams{ + Block: &tmproto.BlockParams{ + MaxGas: 5000000, + }, + } + + app := baseapp.NewBaseApp(name, logger, db, nil) + app.SetParamStore(¶mStore{db: dbm.NewMemDB()}) + app.InitChain(abci.RequestInitChain{ + ConsensusParams: cp, + }) + + wasPrecommiterCalled := false + app.SetPrecommiter(func(ctx sdk.Context) { + wasPrecommiterCalled = true + }) + app.Seal() + + app.Commit() + require.Equal(t, true, wasPrecommiterCalled) +} + func TestABCI_CheckTx(t *testing.T) { // This ante handler reads the key and checks that the value matches the // current counter. This ensures changes to the KVStore persist across @@ -1353,7 +1380,6 @@ func TestCommiterCalledWithCheckState(t *testing.T) { wasCommiterCalled := false app.SetCommiter(func(ctx sdk.Context) { - // Make sure context is for next block require.Equal(t, true, ctx.IsCheckTx()) wasCommiterCalled = true }) @@ -1364,6 +1390,28 @@ func TestCommiterCalledWithCheckState(t *testing.T) { require.Equal(t, true, wasCommiterCalled) } +// Verifies that the Precommiter is called with the deliverState. +func TestPrecommiterCalledWithDeliverState(t *testing.T) { + t.Parallel() + + logger := defaultLogger() + db := dbm.NewMemDB() + name := t.Name() + app := baseapp.NewBaseApp(name, logger, db, nil) + + wasPrecommiterCalled := false + app.SetPrecommiter(func(ctx sdk.Context) { + require.Equal(t, false, ctx.IsCheckTx()) + require.Equal(t, false, ctx.IsReCheckTx()) + wasPrecommiterCalled = true + }) + + app.BeginBlock(abci.RequestBeginBlock{Header: tmproto.Header{Height: 1}}) + app.Commit() + + require.Equal(t, true, wasPrecommiterCalled) +} + func TestABCI_Proposal_HappyPath(t *testing.T) { anteKey := []byte("ante-key") pool := mempool.NewSenderNonceMempool() diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 6d1f6cbda94a..3d70eccb3727 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -70,6 +70,7 @@ type BaseApp struct { //nolint: maligned prepareProposal sdk.PrepareProposalHandler // the handler which runs on ABCI PrepareProposal endBlocker sdk.EndBlocker // logic to run after all txs, and to determine valset changes commiter sdk.Commiter // logic to run during commit + precommiter sdk.Precommiter // logic to run during commit using the deliverState addrPeerFilter sdk.PeerFilter // filter peers by address and port idPeerFilter sdk.PeerFilter // filter peers by node ID fauxMerkleMode bool // if true, IAVL MountStores uses MountStoresDB for simulation speed. diff --git a/baseapp/baseapp_test.go b/baseapp/baseapp_test.go index 2f6a5ca1ba07..ad09ce61fdd6 100644 --- a/baseapp/baseapp_test.go +++ b/baseapp/baseapp_test.go @@ -393,6 +393,9 @@ func TestBaseAppOptionSeal(t *testing.T) { require.Panics(t, func() { suite.baseApp.SetCommiter(nil) }) + require.Panics(t, func() { + suite.baseApp.SetPrecommiter(nil) + }) require.Panics(t, func() { suite.baseApp.SetAnteHandler(nil) }) diff --git a/baseapp/options.go b/baseapp/options.go index d60130011cc6..dc0298513657 100644 --- a/baseapp/options.go +++ b/baseapp/options.go @@ -164,6 +164,14 @@ func (app *BaseApp) SetCommiter(commiter sdk.Commiter) { app.commiter = commiter } +func (app *BaseApp) SetPrecommiter(precommiter sdk.Precommiter) { + if app.sealed { + panic("SetPrecommiter() on sealed BaseApp") + } + + app.precommiter = precommiter +} + func (app *BaseApp) SetAnteHandler(ah sdk.AnteHandler) { if app.sealed { panic("SetAnteHandler() on sealed BaseApp") diff --git a/testutil/mock/types_module_module.go b/testutil/mock/types_module_module.go index ef3e0e3c0166..2261757989ec 100644 --- a/testutil/mock/types_module_module.go +++ b/testutil/mock/types_module_module.go @@ -992,3 +992,116 @@ func (mr *MockCommitAppModuleMockRecorder) RegisterLegacyAminoCodec(arg0 interfa mr.mock.ctrl.T.Helper() return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RegisterLegacyAminoCodec", reflect.TypeOf((*MockCommitAppModule)(nil).RegisterLegacyAminoCodec), arg0) } + +// MockPrecommitAppModule is a mock of PrecommitAppModule interface. +type MockPrecommitAppModule struct { + ctrl *gomock.Controller + recorder *MockPrecommitAppModuleMockRecorder +} + +// MockPrecommitAppModuleMockRecorder is the mock recorder for MockPrecommitAppModule. +type MockPrecommitAppModuleMockRecorder struct { + mock *MockPrecommitAppModule +} + +// NewMockPrecommitAppModule creates a new mock instance. +func NewMockPrecommitAppModule(ctrl *gomock.Controller) *MockPrecommitAppModule { + mock := &MockPrecommitAppModule{ctrl: ctrl} + mock.recorder = &MockPrecommitAppModuleMockRecorder{mock} + return mock +} + +// EXPECT returns an object that allows the caller to indicate expected use. +func (m *MockPrecommitAppModule) EXPECT() *MockPrecommitAppModuleMockRecorder { + return m.recorder +} + +// GetQueryCmd mocks base method. +func (m *MockPrecommitAppModule) GetQueryCmd() *cobra.Command { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetQueryCmd") + ret0, _ := ret[0].(*cobra.Command) + return ret0 +} + +// GetQueryCmd indicates an expected call of GetQueryCmd. +func (mr *MockPrecommitAppModuleMockRecorder) GetQueryCmd() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetQueryCmd", reflect.TypeOf((*MockPrecommitAppModule)(nil).GetQueryCmd)) +} + +// GetTxCmd mocks base method. +func (m *MockPrecommitAppModule) GetTxCmd() *cobra.Command { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "GetTxCmd") + ret0, _ := ret[0].(*cobra.Command) + return ret0 +} + +// GetTxCmd indicates an expected call of GetTxCmd. +func (mr *MockPrecommitAppModuleMockRecorder) GetTxCmd() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "GetTxCmd", reflect.TypeOf((*MockPrecommitAppModule)(nil).GetTxCmd)) +} + +// Name mocks base method. +func (m *MockPrecommitAppModule) Name() string { + m.ctrl.T.Helper() + ret := m.ctrl.Call(m, "Name") + ret0, _ := ret[0].(string) + return ret0 +} + +// Name indicates an expected call of Name. +func (mr *MockPrecommitAppModuleMockRecorder) Name() *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Name", reflect.TypeOf((*MockPrecommitAppModule)(nil).Name)) +} + +// Precommit mocks base method. +func (m *MockPrecommitAppModule) Precommit(arg0 types0.Context) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "Precommit", arg0) +} + +// Precommit indicates an expected call of Precommit. +func (mr *MockPrecommitAppModuleMockRecorder) Precommit(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "Precommit", reflect.TypeOf((*MockPrecommitAppModule)(nil).Precommit), arg0) +} + +// RegisterGRPCGatewayRoutes mocks base method. +func (m *MockPrecommitAppModule) RegisterGRPCGatewayRoutes(arg0 client.Context, arg1 *runtime.ServeMux) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "RegisterGRPCGatewayRoutes", arg0, arg1) +} + +// RegisterGRPCGatewayRoutes indicates an expected call of RegisterGRPCGatewayRoutes. +func (mr *MockPrecommitAppModuleMockRecorder) RegisterGRPCGatewayRoutes(arg0, arg1 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RegisterGRPCGatewayRoutes", reflect.TypeOf((*MockPrecommitAppModule)(nil).RegisterGRPCGatewayRoutes), arg0, arg1) +} + +// RegisterInterfaces mocks base method. +func (m *MockPrecommitAppModule) RegisterInterfaces(arg0 types.InterfaceRegistry) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "RegisterInterfaces", arg0) +} + +// RegisterInterfaces indicates an expected call of RegisterInterfaces. +func (mr *MockPrecommitAppModuleMockRecorder) RegisterInterfaces(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RegisterInterfaces", reflect.TypeOf((*MockPrecommitAppModule)(nil).RegisterInterfaces), arg0) +} + +// RegisterLegacyAminoCodec mocks base method. +func (m *MockPrecommitAppModule) RegisterLegacyAminoCodec(arg0 *codec.LegacyAmino) { + m.ctrl.T.Helper() + m.ctrl.Call(m, "RegisterLegacyAminoCodec", arg0) +} + +// RegisterLegacyAminoCodec indicates an expected call of RegisterLegacyAminoCodec. +func (mr *MockPrecommitAppModuleMockRecorder) RegisterLegacyAminoCodec(arg0 interface{}) *gomock.Call { + mr.mock.ctrl.T.Helper() + return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "RegisterLegacyAminoCodec", reflect.TypeOf((*MockPrecommitAppModule)(nil).RegisterLegacyAminoCodec), arg0) +} diff --git a/types/abci.go b/types/abci.go index 2816a1627d52..0dcadd916cac 100644 --- a/types/abci.go +++ b/types/abci.go @@ -23,6 +23,9 @@ type EndBlocker func(ctx Context, req abci.RequestEndBlock) abci.ResponseEndBloc // branched for the new block. type Commiter func(ctx Context) +// Precommiter runs code during commit before the `deliverState` is reset. +type Precommiter func(ctx Context) + // PeerFilter responds to p2p filtering queries from Tendermint type PeerFilter func(info string) abci.ResponseQuery diff --git a/types/module/module.go b/types/module/module.go index 3daf509b4878..1601f7fb7662 100644 --- a/types/module/module.go +++ b/types/module/module.go @@ -211,6 +211,12 @@ type CommitAppModule interface { Commit(sdk.Context) } +// PreommitAppModule is an extension interface that contains information about the AppModule and Precommit. +type PrecommitAppModule interface { + AppModule + Precommit(sdk.Context) +} + // GenesisOnlyAppModule is an AppModule that only has import/export functionality type GenesisOnlyAppModule struct { AppModuleGenesis @@ -258,6 +264,7 @@ type Manager struct { OrderBeginBlockers []string OrderEndBlockers []string OrderCommiters []string + OrderPrecommiters []string OrderMigrations []string } @@ -276,6 +283,7 @@ func NewManager(modules ...AppModule) *Manager { OrderExportGenesis: modulesStr, OrderBeginBlockers: modulesStr, OrderCommiters: modulesStr, + OrderPrecommiters: modulesStr, OrderEndBlockers: modulesStr, } } @@ -322,6 +330,11 @@ func (m *Manager) SetOrderCommiters(moduleNames ...string) { m.OrderCommiters = moduleNames } +// SetOrderPrecommiters sets the order of set precommiter calls +func (m *Manager) SetOrderPrecommiters(moduleNames ...string) { + m.OrderPrecommiters = moduleNames +} + // SetOrderEndBlockers sets the order of set end-blocker calls func (m *Manager) SetOrderEndBlockers(moduleNames ...string) { m.assertNoForgottenModules("SetOrderEndBlockers", moduleNames) @@ -623,6 +636,17 @@ func (m *Manager) Commit(ctx sdk.Context) { } } +// Precommit performs precommit functionality for all modules. +func (m *Manager) Precommit(ctx sdk.Context) { + for _, moduleName := range m.OrderPrecommiters { + module, ok := m.Modules[moduleName].(PrecommitAppModule) + if !ok { + continue + } + module.Precommit(ctx) + } +} + // GetVersionMap gets consensus version from all modules func (m *Manager) GetVersionMap() VersionMap { vermap := make(VersionMap) diff --git a/types/module/module_test.go b/types/module/module_test.go index 205dfddfd354..f3c7b2135a28 100644 --- a/types/module/module_test.go +++ b/types/module/module_test.go @@ -105,6 +105,10 @@ func TestManagerOrderSetters(t *testing.T) { require.Equal(t, []string{"module1", "module2"}, mm.OrderCommiters) mm.SetOrderCommiters("module2", "module1") require.Equal(t, []string{"module2", "module1"}, mm.OrderCommiters) + + require.Equal(t, []string{"module1", "module2"}, mm.OrderPrecommiters) + mm.SetOrderPrecommiters("module2", "module1") + require.Equal(t, []string{"module2", "module1"}, mm.OrderPrecommiters) } func TestManager_RegisterInvariants(t *testing.T) { @@ -272,3 +276,20 @@ func TestManager_Commit(t *testing.T) { mockAppModule2.EXPECT().Commit(gomock.Any()).Times(1) mm.Commit(sdk.Context{}) } + +func TestManager_Precommit(t *testing.T) { + mockCtrl := gomock.NewController(t) + t.Cleanup(mockCtrl.Finish) + + mockAppModule1 := mock.NewMockPrecommitAppModule(mockCtrl) + mockAppModule2 := mock.NewMockPrecommitAppModule(mockCtrl) + mockAppModule1.EXPECT().Name().Times(2).Return("module1") + mockAppModule2.EXPECT().Name().Times(2).Return("module2") + mm := module.NewManager(mockAppModule1, mockAppModule2) + require.NotNil(t, mm) + require.Equal(t, 2, len(mm.Modules)) + + mockAppModule1.EXPECT().Precommit(gomock.Any()).Times(1) + mockAppModule2.EXPECT().Precommit(gomock.Any()).Times(1) + mm.Precommit(sdk.Context{}) +}