diff --git a/CHANGELOG_PENDING.md b/CHANGELOG_PENDING.md index 77940da8a..842322e19 100644 --- a/CHANGELOG_PENDING.md +++ b/CHANGELOG_PENDING.md @@ -7,6 +7,8 @@ - CLI/RPC/Config - Apps + - [abci] \#9468 Introduce `FinalizeBlock` which condenses `BeginBlock`, `DeliverTx` and `EndBlock` + into a single method call (@cmwaters) - P2P Protocol diff --git a/proto/tendermint/abci/types.proto b/proto/tendermint/abci/types.proto index dceb26bbd..2462ad19b 100644 --- a/proto/tendermint/abci/types.proto +++ b/proto/tendermint/abci/types.proto @@ -377,7 +377,7 @@ message ExtendedCommitInfo { } // Event allows application developers to attach additional information to -// ResponseBeginBlock, ResponseEndBlock, ResponseCheckTx and ResponseDeliverTx. +// ResponseFinalizeBlock and ResponseCheckTx. // Later, transactions may be queried using these events. message Event { string type = 1; diff --git a/state/execution.go b/state/execution.go index aef814894..8ef696975 100644 --- a/state/execution.go +++ b/state/execution.go @@ -198,16 +198,32 @@ func (blockExec *BlockExecutor) ApplyBlock( return state, ErrInvalidBlock(err) } + commitInfo := buildLastCommitInfo(block, blockExec.store, state.InitialHeight) + startTime := time.Now().UnixNano() - abciResponse, err := execBlockOnProxyApp( - blockExec.logger, blockExec.proxyApp, block, blockExec.store, state.InitialHeight, - ) + abciResponse, err := blockExec.proxyApp.FinalizeBlock(context.TODO(), &abci.RequestFinalizeBlock{ + Hash: block.Hash(), + NextValidatorsHash: block.NextValidatorsHash, + ProposerAddress: block.ProposerAddress, + Height: block.Height, + DecidedLastCommit: commitInfo, + Misbehavior: block.Evidence.Evidence.ToABCI(), + Txs: block.Txs.ToSliceOfBytes(), + }) endTime := time.Now().UnixNano() blockExec.metrics.BlockProcessingTime.Observe(float64(endTime-startTime) / 1000000) if err != nil { - return state, ErrProxyAppConn(err) + blockExec.logger.Error("error in proxyAppConn.FinalizeBlock", "err", err) + return state, err } + // Assert that the application correctly returned tx results for each of the transactions provided in the block + if len(block.Data.Txs) != len(abciResponse.TxResults) { + return state, fmt.Errorf("expected tx results length to match size of transactions in block. Expected %d, got %d", len(block.Data.Txs), len(abciResponse.TxResults)) + } + + blockExec.logger.Info("executed block", "height", block.Height, "agreed_app_data", abciResponse.AgreedAppData) + fail.Fail() // XXX // Save the results before we commit. @@ -327,40 +343,6 @@ func (blockExec *BlockExecutor) Commit( //--------------------------------------------------------- // Helper functions for executing blocks and updating state -// Executes block's transactions on proxyAppConn. -// Returns a list of transaction results and updates to the validator set -func execBlockOnProxyApp( - logger log.Logger, - proxyAppConn proxy.AppConnConsensus, - block *types.Block, - store Store, - initialHeight int64, -) (*abci.ResponseFinalizeBlock, error) { - commitInfo := buildLastCommitInfo(block, store, initialHeight) - - resp, err := proxyAppConn.FinalizeBlock(context.TODO(), &abci.RequestFinalizeBlock{ - Hash: block.Hash(), - NextValidatorsHash: block.NextValidatorsHash, - ProposerAddress: block.ProposerAddress, - Height: block.Height, - DecidedLastCommit: commitInfo, - Misbehavior: block.Evidence.Evidence.ToABCI(), - Txs: block.Txs.ToSliceOfBytes(), - }) - if err != nil { - logger.Error("error in proxyAppConn.FinalizeBlock", "err", err) - return nil, err - } - - // Assert that the application correctly returned tx results for each of the transactions provided in the block - if len(block.Data.Txs) != len(resp.TxResults) { - return nil, fmt.Errorf("expected tx results length to match size of transactions in block. Expected %d, got %d", len(block.Data.Txs), len(resp.TxResults)) - } - - logger.Info("executed block", "height", block.Height, "agreed_app_data", resp.AgreedAppData) - return resp, nil -} - func buildLastCommitInfo(block *types.Block, store Store, initialHeight int64) abci.CommitInfo { if block.Height == initialHeight { // there is no last commit for the initial height. @@ -586,12 +568,29 @@ func ExecCommitBlock( store Store, initialHeight int64, ) ([]byte, error) { - resp, err := execBlockOnProxyApp(logger, appConnConsensus, block, store, initialHeight) + commitInfo := buildLastCommitInfo(block, store, initialHeight) + + resp, err := appConnConsensus.FinalizeBlock(context.TODO(), &abci.RequestFinalizeBlock{ + Hash: block.Hash(), + NextValidatorsHash: block.NextValidatorsHash, + ProposerAddress: block.ProposerAddress, + Height: block.Height, + DecidedLastCommit: commitInfo, + Misbehavior: block.Evidence.Evidence.ToABCI(), + Txs: block.Txs.ToSliceOfBytes(), + }) if err != nil { - logger.Error("failed executing block on proxy app", "height", block.Height, "err", err) + logger.Error("error in proxyAppConn.FinalizeBlock", "err", err) return nil, err } + // Assert that the application correctly returned tx results for each of the transactions provided in the block + if len(block.Data.Txs) != len(resp.TxResults) { + return nil, fmt.Errorf("expected tx results length to match size of transactions in block. Expected %d, got %d", len(block.Data.Txs), len(resp.TxResults)) + } + + logger.Info("executed block", "height", block.Height, "agreed_app_data", resp.AgreedAppData) + // Commit block, get hash back _, err = appConnConsensus.Commit(context.TODO()) if err != nil { diff --git a/state/execution_test.go b/state/execution_test.go index 78830fcc7..3f5c40554 100644 --- a/state/execution_test.go +++ b/state/execution_test.go @@ -77,6 +77,83 @@ func TestApplyBlock(t *testing.T) { assert.EqualValues(t, 1, state.Version.Consensus.App, "App version wasn't updated") } +// TestFinalizeBlockDecidedLastCommit ensures we correctly send the +// DecidedLastCommit to the application. The test ensures that the +// DecidedLastCommit properly reflects which validators signed the preceding +// block. +func TestFinalizeBlockDecidedLastCommit(t *testing.T) { + app := &testApp{} + cc := proxy.NewLocalClientCreator(app) + proxyApp := proxy.NewAppConns(cc, proxy.NopMetrics()) + err := proxyApp.Start() + require.NoError(t, err) + defer proxyApp.Stop() //nolint:errcheck // ignore for tests + + state, stateDB, privVals := makeState(7, 1) + stateStore := sm.NewStore(stateDB, sm.StoreOptions{ + DiscardABCIResponses: false, + }) + absentSig := types.NewCommitSigAbsent() + + testCases := []struct { + name string + absentCommitSigs map[int]bool + }{ + {"none absent", map[int]bool{}}, + {"one absent", map[int]bool{1: true}}, + {"multiple absent", map[int]bool{1: true, 3: true}}, + } + + for _, tc := range testCases { + t.Run(tc.name, func(t *testing.T) { + blockStore := store.NewBlockStore(dbm.NewMemDB()) + evpool := &mocks.EvidencePool{} + evpool.On("PendingEvidence", mock.Anything).Return([]types.Evidence{}, 0) + evpool.On("Update", mock.Anything, mock.Anything).Return() + evpool.On("CheckEvidence", mock.Anything).Return(nil) + mp := &mpmocks.Mempool{} + mp.On("Lock").Return() + mp.On("Unlock").Return() + mp.On("FlushAppConn", mock.Anything).Return(nil) + mp.On("Update", + mock.Anything, + mock.Anything, + mock.Anything, + mock.Anything, + mock.Anything, + mock.Anything, + mock.Anything).Return(nil) + + eventBus := types.NewEventBus() + require.NoError(t, eventBus.Start()) + + blockExec := sm.NewBlockExecutor(stateStore, log.NewNopLogger(), proxyApp.Consensus(), mp, evpool, blockStore) + state, _, lastCommit, err := makeAndCommitGoodBlock(state, 1, new(types.Commit), state.NextValidators.Validators[0].Address, blockExec, privVals, nil) + require.NoError(t, err) + + for idx, isAbsent := range tc.absentCommitSigs { + if isAbsent { + lastCommit.Signatures[idx] = absentSig + } + } + + // block for height 2 + block := makeBlock(state, 2, lastCommit) + bps, err := block.MakePartSet(testPartSize) + require.NoError(t, err) + blockID := types.BlockID{Hash: block.Hash(), PartSetHeader: bps.Header()} + _, err = blockExec.ApplyBlock(state, blockID, block) + require.NoError(t, err) + + // -> app receives a list of validators with a bool indicating if they signed + for i, v := range app.CommitVotes { + _, absent := tc.absentCommitSigs[i] + assert.Equal(t, !absent, v.SignedLastBlock) + } + }) + } +} + // TestFinalizeBlockValidators ensures we send absent validators list. func TestFinalizeBlockValidators(t *testing.T) { app := &testApp{} diff --git a/state/store.go b/state/store.go index f2da03ec3..b6096adf8 100644 --- a/state/store.go +++ b/state/store.go @@ -406,13 +406,7 @@ func (store dbStore) LoadFinalizeBlockResponse(height int64) (*abci.ResponseFina // The state store contains the old format. Migrate to // the new ResponseFinalizeBlock format. Note that the // new struct expects the AgreedAppData which we don't have. - resp = &abci.ResponseFinalizeBlock{ - TxResults: legacyResp.DeliverTxs, - ValidatorUpdates: legacyResp.EndBlock.ValidatorUpdates, - ConsensusParamUpdates: legacyResp.EndBlock.ConsensusParamUpdates, - Events: append(legacyResp.BeginBlock.Events, legacyResp.EndBlock.Events...), - // NOTE: AgreedAppData is missing in the response - } + return responseFinalizeBlockFromLegacy(legacyResp), nil } // TODO: ensure that buf is completely read. @@ -456,15 +450,7 @@ func (store dbStore) LoadLastFinalizeBlockResponse(height int64) (*abci.Response if info.LegacyAbciResponses == nil { panic("state store contains last abci response but it is empty") } - legacyResp := info.LegacyAbciResponses - return &abci.ResponseFinalizeBlock{ - TxResults: legacyResp.DeliverTxs, - ValidatorUpdates: legacyResp.EndBlock.ValidatorUpdates, - ConsensusParamUpdates: legacyResp.EndBlock.ConsensusParamUpdates, - Events: append(legacyResp.BeginBlock.Events, legacyResp.EndBlock.Events...), - // NOTE: AgreedAppData is missing in the response but will - // be caught and filled in consensus/replay.go - }, nil + return responseFinalizeBlockFromLegacy(info.LegacyAbciResponses), nil } return info.ResponseFinalizeBlock, nil @@ -706,3 +692,16 @@ func min(a int64, b int64) int64 { } return b } + +// responseFinalizeBlockFromLegacy is a convenience function that takes the old abci responses and morphs +// it to the finalize block response. Note that the agreed app data is missing +func responseFinalizeBlockFromLegacy(legacyResp *tmstate.LegacyABCIResponses) *abci.ResponseFinalizeBlock { + return &abci.ResponseFinalizeBlock{ + TxResults: legacyResp.DeliverTxs, + ValidatorUpdates: legacyResp.EndBlock.ValidatorUpdates, + ConsensusParamUpdates: legacyResp.EndBlock.ConsensusParamUpdates, + Events: append(legacyResp.BeginBlock.Events, legacyResp.EndBlock.Events...), + // NOTE: AgreedAppData is missing in the response but will + // be caught and filled in consensus/replay.go + } +} diff --git a/state/store_test.go b/state/store_test.go index 4d7847046..773ec4346 100644 --- a/state/store_test.go +++ b/state/store_test.go @@ -15,6 +15,7 @@ import ( "github.com/tendermint/tendermint/crypto/ed25519" "github.com/tendermint/tendermint/internal/test" tmrand "github.com/tendermint/tendermint/libs/rand" + tmstate "github.com/tendermint/tendermint/proto/tendermint/state" sm "github.com/tendermint/tendermint/state" "github.com/tendermint/tendermint/types" ) @@ -297,3 +298,48 @@ func TestLastFinalizeBlockResponses(t *testing.T) { }) } + +func TestFinalizeBlockRecoveryUsingLegacyABCIResponses(t *testing.T) { + var ( + height int64 = 10 + lastABCIResponseKey = []byte("lastABCIResponseKey") + memDB = dbm.NewMemDB() + cp = types.DefaultConsensusParams().ToProto() + legacyResp = tmstate.ABCIResponsesInfo{ + LegacyAbciResponses: &tmstate.LegacyABCIResponses{ + BeginBlock: &tmstate.ResponseBeginBlock{ + Events: []abci.Event{{ + Type: "begin_block", + Attributes: []abci.EventAttribute{{ + Key: "key", + Value: "value", + }}, + }}, + }, + DeliverTxs: []*abci.ExecTxResult{{ + Events: []abci.Event{{ + Type: "tx", + Attributes: []abci.EventAttribute{{ + Key: "key", + Value: "value", + }}, + }}, + }}, + EndBlock: &tmstate.ResponseEndBlock{ + ConsensusParamUpdates: &cp, + }, + }, + Height: height, + } + ) + bz, err := legacyResp.Marshal() + require.NoError(t, err) + // should keep this in parity with state/store.go + require.NoError(t, memDB.Set(lastABCIResponseKey, bz)) + stateStore := sm.NewStore(memDB, sm.StoreOptions{DiscardABCIResponses: false}) + resp, err := stateStore.LoadLastFinalizeBlockResponse(height) + require.NoError(t, err) + require.Equal(t, resp.ConsensusParamUpdates, &cp) + require.Equal(t, resp.Events, legacyResp.LegacyAbciResponses.BeginBlock.Events) + require.Equal(t, resp.TxResults[0], legacyResp.LegacyAbciResponses.DeliverTxs[0]) +}