From d669816a1b904ddcfcdf8160600b2cd0763801c3 Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Mon, 11 Dec 2017 11:36:04 -0600 Subject: [PATCH 1/3] send absent validators in BeginBlock Refs #668 --- consensus/replay.go | 2 +- state/execution.go | 32 ++++++++++++++++++++++++-------- 2 files changed, 25 insertions(+), 9 deletions(-) diff --git a/consensus/replay.go b/consensus/replay.go index 168404af0..ac9b9910a 100644 --- a/consensus/replay.go +++ b/consensus/replay.go @@ -324,7 +324,7 @@ func (h *Handshaker) replayBlocks(proxyApp proxy.AppConns, appBlockHeight, store for i := appBlockHeight + 1; i <= finalBlock; i++ { h.logger.Info("Applying block", "height", i) block := h.store.LoadBlock(i) - appHash, err = sm.ExecCommitBlock(proxyApp.Consensus(), block, h.logger) + appHash, err = sm.ExecCommitBlock(proxyApp.Consensus(), block, h.logger, h.state.LastValidators) if err != nil { return nil, err } diff --git a/state/execution.go b/state/execution.go index a5eabed27..57315432b 100644 --- a/state/execution.go +++ b/state/execution.go @@ -26,7 +26,7 @@ func (s *State) ValExecBlock(txEventPublisher types.TxEventPublisher, proxyAppCo } // Execute the block txs - abciResponses, err := execBlockOnProxyApp(txEventPublisher, proxyAppConn, block, s.logger) + abciResponses, err := execBlockOnProxyApp(txEventPublisher, proxyAppConn, block, s.logger, s.LastValidators) if err != nil { // There was some error in proxyApp // TODO Report error and wait for proxyApp to be available. @@ -39,7 +39,7 @@ func (s *State) ValExecBlock(txEventPublisher types.TxEventPublisher, proxyAppCo // Executes block's transactions on proxyAppConn. // Returns a list of transaction results and updates to the validator set // TODO: Generate a bitmap or otherwise store tx validity in state. -func execBlockOnProxyApp(txEventPublisher types.TxEventPublisher, proxyAppConn proxy.AppConnConsensus, block *types.Block, logger log.Logger) (*ABCIResponses, error) { +func execBlockOnProxyApp(txEventPublisher types.TxEventPublisher, proxyAppConn proxy.AppConnConsensus, block *types.Block, logger log.Logger, lastValidators *types.ValidatorSet) (*ABCIResponses, error) { var validTxs, invalidTxs = 0, 0 txIndex := 0 @@ -76,12 +76,28 @@ func execBlockOnProxyApp(txEventPublisher types.TxEventPublisher, proxyAppConn p } proxyAppConn.SetResponseCallback(proxyCb) + // determine validators who did not sign last block + absentVals := make([]int32, 0) + for valA, _ := range lastValidators.Validators { + found := false + for _, voteB := range block.LastCommit.Precommits { + valB := voteB.ValidatorIndex + if valA == valB { + found = true + break + } + } + if !found { + absentVals = append(absentVals, int32(valA)) + } + } + // Begin block _, err := proxyAppConn.BeginBlockSync(abci.RequestBeginBlock{ - block.Hash(), - types.TM2PB.Header(block.Header), - nil, - nil, + Hash: block.Hash(), + Header: types.TM2PB.Header(block.Header), + AbsentValidators: absentVals, + ByzantineValidators: nil, }) if err != nil { logger.Error("Error in proxyAppConn.BeginBlock", "err", err) @@ -275,8 +291,8 @@ func (s *State) CommitStateUpdateMempool(proxyAppConn proxy.AppConnConsensus, bl // ExecCommitBlock executes and commits a block on the proxyApp without validating or mutating the state. // It returns the application root hash (result of abci.Commit). -func ExecCommitBlock(appConnConsensus proxy.AppConnConsensus, block *types.Block, logger log.Logger) ([]byte, error) { - _, err := execBlockOnProxyApp(types.NopEventBus{}, appConnConsensus, block, logger) +func ExecCommitBlock(appConnConsensus proxy.AppConnConsensus, block *types.Block, logger log.Logger, lastValidators *types.ValidatorSet) ([]byte, error) { + _, err := execBlockOnProxyApp(types.NopEventBus{}, appConnConsensus, block, logger, lastValidators) if err != nil { logger.Error("Error executing block on proxy app", "height", block.Height, "err", err) return nil, err From 808b8309420c223b8019a65b41e02e3e16395767 Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Mon, 11 Dec 2017 12:26:19 -0600 Subject: [PATCH 2/3] add a unit test Refs #668 --- state/execution_test.go | 82 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 80 insertions(+), 2 deletions(-) diff --git a/state/execution_test.go b/state/execution_test.go index b946a75a3..f67a8d4de 100644 --- a/state/execution_test.go +++ b/state/execution_test.go @@ -3,9 +3,11 @@ package state import ( "testing" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/tendermint/abci/example/dummy" + abci "github.com/tendermint/abci/types" crypto "github.com/tendermint/go-crypto" "github.com/tendermint/tendermint/proxy" "github.com/tendermint/tendermint/types" @@ -30,16 +32,53 @@ func TestApplyBlock(t *testing.T) { state := state() state.SetLogger(log.TestingLogger()) - // make block block := makeBlock(1, state) err = state.ApplyBlock(types.NopEventBus{}, proxyApp.Consensus(), block, block.MakePartSet(testPartSize).Header(), types.MockMempool{}) - require.Nil(t, err) // TODO check state and mempool } +// TestBeginBlockAbsentValidators ensures we send absent validators list. +func TestBeginBlockAbsentValidators(t *testing.T) { + app := &testApp{} + cc := proxy.NewLocalClientCreator(app) + proxyApp := proxy.NewAppConns(cc, nil) + err := proxyApp.Start() + require.Nil(t, err) + defer proxyApp.Stop() + + state := state() + state.SetLogger(log.TestingLogger()) + + // there were 2 validators + val1PrivKey := crypto.GenPrivKeyEd25519() + val2PrivKey := crypto.GenPrivKeyEd25519() + lastValidators := types.NewValidatorSet([]*types.Validator{ + types.NewValidator(val1PrivKey.PubKey(), 10), + types.NewValidator(val2PrivKey.PubKey(), 5), + }) + + // but last commit contains only the first validator + prevHash := state.LastBlockID.Hash + prevParts := types.PartSetHeader{} + prevBlockID := types.BlockID{prevHash, prevParts} + lastCommit := &types.Commit{BlockID: prevBlockID, Precommits: []*types.Vote{ + {ValidatorIndex: 0}, + }} + + valHash := state.Validators.Hash() + block, _ := types.MakeBlock(2, chainID, makeTxs(2), lastCommit, + prevBlockID, valHash, state.AppHash, testPartSize) + + _, err = ExecCommitBlock(proxyApp.Consensus(), block, log.TestingLogger(), lastValidators) + require.Nil(t, err) + + // -> app must receive an index of the absent validator + assert.Equal(t, []int32{1}, app.AbsentValidators) +} + //---------------------------------------------------------------------------- // make some bogus txs @@ -72,3 +111,42 @@ func makeBlock(height int64, state *State) *types.Block { state.AppHash, testPartSize) return block } + +//---------------------------------------------------------------------------- + +var _ abci.Application = (*testApp)(nil) + +type testApp struct { + abci.BaseApplication + + AbsentValidators []int32 +} + +func NewDummyApplication() *testApp { + return &testApp{} +} + +func (app *testApp) Info(req abci.RequestInfo) (resInfo abci.ResponseInfo) { + return abci.ResponseInfo{} +} + +func (app *testApp) BeginBlock(req abci.RequestBeginBlock) abci.ResponseBeginBlock { + app.AbsentValidators = req.AbsentValidators + return abci.ResponseBeginBlock{} +} + +func (app *testApp) DeliverTx(tx []byte) abci.ResponseDeliverTx { + return abci.ResponseDeliverTx{Tags: []*abci.KVPair{}} +} + +func (app *testApp) CheckTx(tx []byte) abci.ResponseCheckTx { + return abci.ResponseCheckTx{} +} + +func (app *testApp) Commit() abci.ResponseCommit { + return abci.ResponseCommit{} +} + +func (app *testApp) Query(reqQuery abci.RequestQuery) (resQuery abci.ResponseQuery) { + return +} From 7f649ccf2371b2e833a086cb070cdee677a19b9d Mon Sep 17 00:00:00 2001 From: Anton Kaliaev Date: Fri, 15 Dec 2017 12:12:45 -0600 Subject: [PATCH 3/3] fixes from Frey's review --- state/execution.go | 16 ++++------------ state/execution_test.go | 34 +++++++++++++++++++++++----------- 2 files changed, 27 insertions(+), 23 deletions(-) diff --git a/state/execution.go b/state/execution.go index 57315432b..7ba4ac3e9 100644 --- a/state/execution.go +++ b/state/execution.go @@ -76,19 +76,11 @@ func execBlockOnProxyApp(txEventPublisher types.TxEventPublisher, proxyAppConn p } proxyAppConn.SetResponseCallback(proxyCb) - // determine validators who did not sign last block + // determine which validators did not sign last block absentVals := make([]int32, 0) - for valA, _ := range lastValidators.Validators { - found := false - for _, voteB := range block.LastCommit.Precommits { - valB := voteB.ValidatorIndex - if valA == valB { - found = true - break - } - } - if !found { - absentVals = append(absentVals, int32(valA)) + for valI, vote := range block.LastCommit.Precommits { + if vote == nil { + absentVals = append(absentVals, int32(valI)) } } diff --git a/state/execution_test.go b/state/execution_test.go index f67a8d4de..ac5c72f26 100644 --- a/state/execution_test.go +++ b/state/execution_test.go @@ -2,6 +2,7 @@ package state import ( "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -60,23 +61,34 @@ func TestBeginBlockAbsentValidators(t *testing.T) { types.NewValidator(val2PrivKey.PubKey(), 5), }) - // but last commit contains only the first validator prevHash := state.LastBlockID.Hash prevParts := types.PartSetHeader{} prevBlockID := types.BlockID{prevHash, prevParts} - lastCommit := &types.Commit{BlockID: prevBlockID, Precommits: []*types.Vote{ - {ValidatorIndex: 0}, - }} - valHash := state.Validators.Hash() - block, _ := types.MakeBlock(2, chainID, makeTxs(2), lastCommit, - prevBlockID, valHash, state.AppHash, testPartSize) + now := time.Now().UTC() + testCases := []struct { + desc string + lastCommitPrecommits []*types.Vote + expectedAbsentValidators []int32 + }{ + {"none absent", []*types.Vote{{ValidatorIndex: 0, Timestamp: now}, {ValidatorIndex: 1, Timestamp: now}}, []int32{}}, + {"one absent", []*types.Vote{{ValidatorIndex: 0, Timestamp: now}, nil}, []int32{1}}, + {"multiple absent", []*types.Vote{nil, nil}, []int32{0, 1}}, + } - _, err = ExecCommitBlock(proxyApp.Consensus(), block, log.TestingLogger(), lastValidators) - require.Nil(t, err) + for _, tc := range testCases { + lastCommit := &types.Commit{BlockID: prevBlockID, Precommits: tc.lastCommitPrecommits} - // -> app must receive an index of the absent validator - assert.Equal(t, []int32{1}, app.AbsentValidators) + valHash := state.Validators.Hash() + block, _ := types.MakeBlock(2, chainID, makeTxs(2), state.LastBlockTotalTx, lastCommit, + prevBlockID, valHash, state.AppHash, testPartSize) + + _, err = ExecCommitBlock(proxyApp.Consensus(), block, log.TestingLogger(), lastValidators) + require.Nil(t, err, tc.desc) + + // -> app must receive an index of the absent validator + assert.Equal(t, tc.expectedAbsentValidators, app.AbsentValidators, tc.desc) + } } //----------------------------------------------------------------------------