Skip to content

Commit

Permalink
Revert "add precommit callback (#7)" (#10)
Browse files Browse the repository at this point in the history
This reverts commit 0e85c49.
  • Loading branch information
dydxwill authored and jonfung-dydx committed Mar 23, 2023
1 parent 237ba56 commit e8edf09
Show file tree
Hide file tree
Showing 9 changed files with 1 addition and 226 deletions.
4 changes: 0 additions & 4 deletions baseapp/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
50 changes: 1 addition & 49 deletions baseapp/abci_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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(&paramStore{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
Expand Down Expand Up @@ -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
})
Expand All @@ -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()
Expand Down
1 change: 0 additions & 1 deletion baseapp/baseapp.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
3 changes: 0 additions & 3 deletions baseapp/baseapp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
Expand Down
8 changes: 0 additions & 8 deletions baseapp/options.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
113 changes: 0 additions & 113 deletions testutil/mock/types_module_module.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 0 additions & 3 deletions types/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
24 changes: 0 additions & 24 deletions types/module/module.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -271,7 +265,6 @@ type Manager struct {
OrderBeginBlockers []string
OrderEndBlockers []string
OrderCommiters []string
OrderPrecommiters []string
OrderMigrations []string
}

Expand All @@ -290,7 +283,6 @@ func NewManager(modules ...AppModule) *Manager {
OrderExportGenesis: modulesStr,
OrderBeginBlockers: modulesStr,
OrderCommiters: modulesStr,
OrderPrecommiters: modulesStr,
OrderEndBlockers: modulesStr,
}
}
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)
Expand Down
21 changes: 0 additions & 21 deletions types/module/module_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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{}

Expand Down

0 comments on commit e8edf09

Please sign in to comment.