From e8edf0920f868d51808d9bc939d546d6b242072d Mon Sep 17 00:00:00 2001 From: dydxwill <119354122+dydxwill@users.noreply.github.com> Date: Mon, 20 Mar 2023 11:26:16 -0400 Subject: [PATCH] Revert "add precommit callback (#7)" (#10) This reverts commit 0e85c497c44a2ace0c4adf0cc783a49454e43e3d. --- 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, 1 insertion(+), 226 deletions(-) diff --git a/baseapp/abci.go b/baseapp/abci.go index 5443cafcbcd48..a7b185b6dee27 100644 --- a/baseapp/abci.go +++ b/baseapp/abci.go @@ -485,10 +485,6 @@ func (app *BaseApp) Commit() abci.ResponseCommit { // Commit. Use the header from this latest block. app.setState(runTxModeCheck, 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 6a92c907b91d2..f4c1675de3794 100644 --- a/baseapp/abci_test.go +++ b/baseapp/abci_test.go @@ -625,33 +625,6 @@ 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 @@ -1387,6 +1360,7 @@ 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 }) @@ -1397,28 +1371,6 @@ 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 f66049c35f92c..b0cf381cc4c4e 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -71,7 +71,6 @@ 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 76cfa2421e813..ee8068bb045f8 100644 --- a/baseapp/baseapp_test.go +++ b/baseapp/baseapp_test.go @@ -391,9 +391,6 @@ 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 bd4ac7b4548da..ebbab425c395f 100644 --- a/baseapp/options.go +++ b/baseapp/options.go @@ -175,14 +175,6 @@ 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 7887248b621fe..ca77ba57f183d 100644 --- a/testutil/mock/types_module_module.go +++ b/testutil/mock/types_module_module.go @@ -992,116 +992,3 @@ 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 types1.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 types0.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 b866ac1ee1824..e5ae15455939c 100644 --- a/types/abci.go +++ b/types/abci.go @@ -23,9 +23,6 @@ type EndBlocker func(ctx Context, req abci.RequestEndBlock) (abci.ResponseEndBlo // 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 b584461faa944..19c8358ce3614 100644 --- a/types/module/module.go +++ b/types/module/module.go @@ -218,12 +218,6 @@ 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 @@ -271,7 +265,6 @@ type Manager struct { OrderBeginBlockers []string OrderEndBlockers []string OrderCommiters []string - OrderPrecommiters []string OrderMigrations []string } @@ -290,7 +283,6 @@ func NewManager(modules ...AppModule) *Manager { OrderExportGenesis: modulesStr, OrderBeginBlockers: modulesStr, OrderCommiters: modulesStr, - OrderPrecommiters: modulesStr, OrderEndBlockers: modulesStr, } } @@ -361,11 +353,6 @@ 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, @@ -743,17 +730,6 @@ 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 2a1e8d99f0eab..de1f2ec739d31 100644 --- a/types/module/module_test.go +++ b/types/module/module_test.go @@ -132,10 +132,6 @@ 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) { @@ -511,23 +507,6 @@ func TestManager_Commit(t *testing.T) { 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{}) -} - // MockCoreAppModule allows us to test functions like DefaultGenesis type MockCoreAppModule struct{}