diff --git a/blocksync/reactor_test.go b/blocksync/reactor_test.go index cd6e640ab75..838bbd9970c 100644 --- a/blocksync/reactor_test.go +++ b/blocksync/reactor_test.go @@ -121,7 +121,8 @@ func newReactor( for blockHeight := int64(1); blockHeight <= maxBlockHeight; blockHeight++ { lastExtCommit := seenExtCommit.Clone() - thisBlock := state.MakeBlock(blockHeight, nil, lastExtCommit.ToCommit(), nil, state.Validators.Proposer.Address) + thisBlock, err := state.MakeBlock(blockHeight, nil, lastExtCommit.ToCommit(), nil, state.Validators.Proposer.Address) + require.NoError(t, err) thisParts, err := thisBlock.MakePartSet(types.BlockPartSizeBytes) require.NoError(t, err) diff --git a/consensus/replay_test.go b/consensus/replay_test.go index bcf74387f36..3de2af41c30 100644 --- a/consensus/replay_test.go +++ b/consensus/replay_test.go @@ -898,7 +898,10 @@ func makeBlocks(n int, state sm.State, privVals []types.PrivValidator) ([]*types if err != nil { return nil, err } - block := state.MakeBlock(height, test.MakeNTxs(height, 10), lastCommit, nil, state.LastValidators.Proposer.Address) + block, err := state.MakeBlock(height, test.MakeNTxs(height, 10), lastCommit, nil, state.LastValidators.Proposer.Address) + if err != nil { + return nil, err + } blocks[i] = block state.LastBlockID = blockID state.LastBlockHeight = height diff --git a/consensus/state_test.go b/consensus/state_test.go index dfd979d8597..93afc91d4cd 100644 --- a/consensus/state_test.go +++ b/consensus/state_test.go @@ -3205,13 +3205,14 @@ func findBlockSizeLimit(t *testing.T, height, maxBytes int64, cs *State, partSiz } softMaxDataBytes := int(types.MaxDataBytes(maxBytes, 0, 0)) for i := softMaxDataBytes; i < softMaxDataBytes*2; i++ { - propBlock := cs.state.MakeBlock( + propBlock, err := cs.state.MakeBlock( height, []types.Tx{[]byte("a=" + strings.Repeat("o", i-2))}, &types.Commit{}, nil, cs.privValidatorPubKey.Address(), ) + require.NoError(t, err) propBlockParts, err := propBlock.MakePartSet(partSize) require.NoError(t, err) diff --git a/evidence/pool_test.go b/evidence/pool_test.go index 1837f0395d3..227ed970bec 100644 --- a/evidence/pool_test.go +++ b/evidence/pool_test.go @@ -418,9 +418,12 @@ func initializeBlockStore(db dbm.DB, state sm.State, valAddr []byte) (*store.Blo for i := int64(1); i <= state.LastBlockHeight; i++ { lastCommit := makeExtCommit(i-1, valAddr) - block := state.MakeBlock(i, test.MakeNTxs(i, 1), lastCommit.ToCommit(), nil, state.Validators.Proposer.Address) - block.Header.Time = defaultEvidenceTime.Add(time.Duration(i) * time.Minute) - block.Header.Version = cmtversion.Consensus{Block: version.BlockProtocol, App: 1} + block, err := state.MakeBlock(i, test.MakeNTxs(i, 1), lastCommit.ToCommit(), nil, state.Validators.Proposer.Address) + if err != nil { + return nil, err + } + block.Time = defaultEvidenceTime.Add(time.Duration(i) * time.Minute) + block.Version = cmtversion.Consensus{Block: version.BlockProtocol, App: 1} partSet, err := block.MakePartSet(types.BlockPartSizeBytes) if err != nil { return nil, err diff --git a/state/execution.go b/state/execution.go index bec0ae38f6f..a9b461eaf2c 100644 --- a/state/execution.go +++ b/state/execution.go @@ -126,7 +126,10 @@ func (blockExec *BlockExecutor) CreateProposalBlock( txs := blockExec.mempool.ReapMaxBytesMaxGas(maxReapBytes, maxGas) commit := lastExtCommit.ToCommit() - block := state.MakeBlock(height, txs, commit, evidence, proposerAddr) + block, err := state.MakeBlock(height, txs, commit, evidence, proposerAddr) + if err != nil { + return nil, err + } rpp, err := blockExec.proxyApp.PrepareProposal( ctx, &abci.RequestPrepareProposal{ @@ -157,7 +160,7 @@ func (blockExec *BlockExecutor) CreateProposalBlock( return nil, err } - return state.MakeBlock(height, txl, commit, evidence, proposerAddr), nil + return state.MakeBlock(height, txl, commit, evidence, proposerAddr) } func (blockExec *BlockExecutor) ProcessProposal( diff --git a/state/execution_test.go b/state/execution_test.go index e6e21a24796..fcbe0ae6e0a 100644 --- a/state/execution_test.go +++ b/state/execution_test.go @@ -67,7 +67,8 @@ func TestApplyBlock(t *testing.T) { blockExec := sm.NewBlockExecutor(stateStore, log.TestingLogger(), proxyApp.Consensus(), mp, sm.EmptyEvidencePool{}, blockStore) - block := makeBlock(state, 1, new(types.Commit)) + block, err := makeBlock(state, 1, new(types.Commit)) + require.NoError(t, err) bps, err := block.MakePartSet(testPartSize) require.NoError(t, err) blockID := types.BlockID{Hash: block.Hash(), PartSetHeader: bps.Header()} @@ -141,7 +142,8 @@ func TestFinalizeBlockDecidedLastCommit(t *testing.T) { } // block for height 2 - block := makeBlock(state, 2, lastCommit.ToCommit()) + block, err := makeBlock(state, 2, lastCommit.ToCommit()) + require.NoError(t, err) bps, err := block.MakePartSet(testPartSize) require.NoError(t, err) blockID := types.BlockID{Hash: block.Hash(), PartSetHeader: bps.Header()} @@ -221,7 +223,8 @@ func TestFinalizeBlockValidators(t *testing.T) { } // block for height 2 - block := makeBlock(state, 2, lastCommit.ToCommit()) + block, err := makeBlock(state, 2, lastCommit.ToCommit()) + require.NoError(t, err) _, err = sm.ExecCommitBlock(proxyApp.Consensus(), block, log.TestingLogger(), stateStore, 1) require.NoError(t, err, tc.desc) @@ -348,7 +351,8 @@ func TestFinalizeBlockMisbehavior(t *testing.T) { blockExec := sm.NewBlockExecutor(stateStore, log.TestingLogger(), proxyApp.Consensus(), mp, evpool, blockStore) - block := makeBlock(state, 1, new(types.Commit)) + block, err := makeBlock(state, 1, new(types.Commit)) + require.NoError(t, err) block.Evidence = types.EvidenceData{Evidence: ev} block.Header.EvidenceHash = block.Evidence.Hash() bps, err := block.MakePartSet(testPartSize) @@ -395,7 +399,8 @@ func TestProcessProposal(t *testing.T) { blockStore, ) - block0 := makeBlock(state, height-1, new(types.Commit)) + block0, err := makeBlock(state, height-1, new(types.Commit)) + require.NoError(t, err) lastCommitSig := []types.CommitSig{} partSet, err := block0.MakePartSet(types.BlockPartSizeBytes) require.NoError(t, err) @@ -419,10 +424,11 @@ func TestProcessProposal(t *testing.T) { lastCommitSig = append(lastCommitSig, vote.CommitSig()) } - block1 := makeBlock(state, height, &types.Commit{ + block1, err := makeBlock(state, height, &types.Commit{ Height: height - 1, Signatures: lastCommitSig, }) + require.NoError(t, err) block1.Txs = txs @@ -653,7 +659,8 @@ func TestFinalizeBlockValidatorUpdates(t *testing.T) { ) require.NoError(t, err) - block := makeBlock(state, 1, new(types.Commit)) + block, err := makeBlock(state, 1, new(types.Commit)) + require.NoError(t, err) bps, err := block.MakePartSet(testPartSize) require.NoError(t, err) blockID := types.BlockID{Hash: block.Hash(), PartSetHeader: bps.Header()} @@ -721,7 +728,8 @@ func TestFinalizeBlockValidatorUpdatesResultingInEmptySet(t *testing.T) { blockStore, ) - block := makeBlock(state, 1, new(types.Commit)) + block, err := makeBlock(state, 1, new(types.Commit)) + require.NoError(t, err) bps, err := block.MakePartSet(testPartSize) require.NoError(t, err) blockID := types.BlockID{Hash: block.Hash(), PartSetHeader: bps.Header()} @@ -1066,26 +1074,26 @@ func TestCreateProposalAbsentVoteExtensions(t *testing.T) { }{ { name: "missing extension data on first required height", - height: 2, - extensionEnableHeight: 1, + height: 3, + extensionEnableHeight: 2, expectPanic: true, }, { name: "missing extension during before required height", - height: 2, - extensionEnableHeight: 2, + height: 3, + extensionEnableHeight: 3, expectPanic: false, }, { name: "missing extension data and not required", - height: 2, + height: 3, extensionEnableHeight: 0, expectPanic: false, }, { name: "missing extension data and required in two heights", - height: 2, - extensionEnableHeight: 3, + height: 3, + extensionEnableHeight: 4, expectPanic: false, }, } { @@ -1129,7 +1137,8 @@ func TestCreateProposalAbsentVoteExtensions(t *testing.T) { sm.EmptyEvidencePool{}, blockStore, ) - block := makeBlock(state, testCase.height, new(types.Commit)) + block, err := makeBlock(state, testCase.height, new(types.Commit)) + require.NoError(t, err) bps, err := block.MakePartSet(testPartSize) require.NoError(t, err) @@ -1139,7 +1148,8 @@ func TestCreateProposalAbsentVoteExtensions(t *testing.T) { stripSignatures(lastCommit) if testCase.expectPanic { require.Panics(t, func() { - blockExec.CreateProposalBlock(ctx, testCase.height, state, lastCommit, pa) //nolint:errcheck + _, err := blockExec.CreateProposalBlock(ctx, testCase.height, state, lastCommit, pa) + require.NoError(t, err) }) } else { _, err = blockExec.CreateProposalBlock(ctx, testCase.height, state, lastCommit, pa) diff --git a/state/helpers_test.go b/state/helpers_test.go index d592acb3982..e7b4d4d7695 100644 --- a/state/helpers_test.go +++ b/state/helpers_test.go @@ -56,7 +56,10 @@ func makeAndCommitGoodBlock( func makeAndApplyGoodBlock(state sm.State, height int64, lastCommit *types.Commit, proposerAddr []byte, blockExec *sm.BlockExecutor, evidence []types.Evidence, ) (sm.State, types.BlockID, error) { - block := state.MakeBlock(height, test.MakeNTxs(height, 10), lastCommit, evidence, proposerAddr) + block, err := state.MakeBlock(height, test.MakeNTxs(height, 10), lastCommit, evidence, proposerAddr) + if err != nil { + return state, types.BlockID{}, nil + } partSet, err := block.MakePartSet(types.BlockPartSizeBytes) if err != nil { return state, types.BlockID{}, err @@ -76,7 +79,7 @@ func makeAndApplyGoodBlock(state sm.State, height int64, lastCommit *types.Commi return state, blockID, nil } -func makeBlock(state sm.State, height int64, c *types.Commit) *types.Block { +func makeBlock(state sm.State, height int64, c *types.Commit) (*types.Block, error) { return state.MakeBlock( height, test.MakeNTxs(state.LastBlockHeight, 10), @@ -171,7 +174,10 @@ func makeHeaderPartsResponsesValPubKeyChange( state sm.State, pubkey crypto.PubKey, ) (types.Header, types.BlockID, *abci.ResponseFinalizeBlock) { - block := makeBlock(state, state.LastBlockHeight+1, new(types.Commit)) + block, err := makeBlock(state, state.LastBlockHeight+1, new(types.Commit)) + if err != nil { + return types.Header{}, types.BlockID{}, nil + } abciResponses := &abci.ResponseFinalizeBlock{} // If the pubkey is new, remove the old and add the new. _, val := state.NextValidators.GetByIndex(0) @@ -189,7 +195,10 @@ func makeHeaderPartsResponsesValPowerChange( state sm.State, power int64, ) (types.Header, types.BlockID, *abci.ResponseFinalizeBlock) { - block := makeBlock(state, state.LastBlockHeight+1, new(types.Commit)) + block, err := makeBlock(state, state.LastBlockHeight+1, new(types.Commit)) + if err != nil { + return types.Header{}, types.BlockID{}, nil + } abciResponses := &abci.ResponseFinalizeBlock{} // If the pubkey is new, remove the old and add the new. @@ -207,7 +216,10 @@ func makeHeaderPartsResponsesParams( state sm.State, params cmtproto.ConsensusParams, ) (types.Header, types.BlockID, *abci.ResponseFinalizeBlock) { - block := makeBlock(state, state.LastBlockHeight+1, new(types.Commit)) + block, err := makeBlock(state, state.LastBlockHeight+1, new(types.Commit)) + if err != nil { + return types.Header{}, types.BlockID{}, nil + } abciResponses := &abci.ResponseFinalizeBlock{ ConsensusParamUpdates: ¶ms, } diff --git a/state/state.go b/state/state.go index 8fa44bfd56d..20be5feb146 100644 --- a/state/state.go +++ b/state/state.go @@ -245,7 +245,7 @@ func (state State) MakeBlock( lastCommit *types.Commit, evidence []types.Evidence, proposerAddress []byte, -) *types.Block { +) (*types.Block, error) { // Build base block with block data. block := types.MakeBlock(height, txs, lastCommit, evidence) @@ -255,7 +255,11 @@ func (state State) MakeBlock( if height == state.InitialHeight { timestamp = state.LastBlockTime // genesis time } else { - timestamp = MedianTime(lastCommit, state.LastValidators) + ts, err := MedianTime(lastCommit, state.LastValidators) + if err != nil { + return nil, fmt.Errorf("error making block while calculating median time: %w", err) + } + timestamp = ts } // Fill rest of header with state data. @@ -267,14 +271,14 @@ func (state State) MakeBlock( proposerAddress, ) - return block + return block, nil } // MedianTime computes a median time for a given Commit (based on Timestamp field of votes messages) and the // corresponding validator set. The computed time is always between timestamps of // the votes sent by honest processes, i.e., a faulty processes can not arbitrarily increase or decrease the // computed value. -func MedianTime(commit *types.Commit, validators *types.ValidatorSet) time.Time { +func MedianTime(commit *types.Commit, validators *types.ValidatorSet) (time.Time, error) { weightedTimes := make([]*cmttime.WeightedTime, len(commit.Signatures)) totalVotingPower := int64(0) @@ -284,13 +288,15 @@ func MedianTime(commit *types.Commit, validators *types.ValidatorSet) time.Time } _, validator := validators.GetByAddress(commitSig.ValidatorAddress) // If there's no condition, TestValidateBlockCommit panics; not needed normally. - if validator != nil { - totalVotingPower += validator.VotingPower - weightedTimes[i] = cmttime.NewWeightedTime(commitSig.Timestamp, validator.VotingPower) + if validator == nil { + return time.Time{}, fmt.Errorf("commit validator not found in validator set: %X", + commitSig.ValidatorAddress) } + totalVotingPower += validator.VotingPower + weightedTimes[i] = cmttime.NewWeightedTime(commitSig.Timestamp, validator.VotingPower) } - return cmttime.WeightedMedian(weightedTimes, totalVotingPower) + return cmttime.WeightedMedian(weightedTimes, totalVotingPower), nil } //------------------------------------------------------------------------ diff --git a/state/state_test.go b/state/state_test.go index 8b5c149383b..d01d3cb3885 100644 --- a/state/state_test.go +++ b/state/state_test.go @@ -7,6 +7,7 @@ import ( "math/big" "os" "testing" + "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -105,7 +106,8 @@ func TestFinalizeBlockResponsesSaveLoad1(t *testing.T) { state.LastBlockHeight++ // Build mock responses. - block := makeBlock(state, 2, new(types.Commit)) + block, err := makeBlock(state, 2, new(types.Commit)) + require.NoError(t, err) abciResponses := new(abci.ResponseFinalizeBlock) dtxs := make([]*abci.ExecTxResult, 2) @@ -117,7 +119,9 @@ func TestFinalizeBlockResponsesSaveLoad1(t *testing.T) { types.TM2PB.NewValidatorUpdate(ed25519.GenPrivKey().PubKey(), 10, true), } - err := stateStore.SaveFinalizeBlockResponse(block.Height, abciResponses) + abciResponses.AppHash = make([]byte, 1) + + err = stateStore.SaveFinalizeBlockResponse(block.Height, abciResponses) require.NoError(t, err) loadedABCIResponses, err := stateStore.LoadFinalizeBlockResponse(block.Height) assert.NoError(err) @@ -455,7 +459,8 @@ func TestProposerPriorityDoesNotGetResetToZero(t *testing.T) { // NewValidatorSet calls IncrementProposerPriority but uses on a copy of val1 assert.EqualValues(t, 0, val1.ProposerPriority) - block := makeBlock(state, state.LastBlockHeight+1, new(types.Commit)) + block, err := makeBlock(state, state.LastBlockHeight+1, new(types.Commit)) + require.NoError(t, err) bps, err := block.MakePartSet(testPartSize) require.NoError(t, err) blockID := types.BlockID{Hash: block.Hash(), PartSetHeader: bps.Header()} @@ -573,7 +578,8 @@ func TestProposerPriorityProposerAlternates(t *testing.T) { // we only have one validator: assert.Equal(t, val1PubKey.Address(), state.Validators.Proposer.Address) - block := makeBlock(state, state.LastBlockHeight+1, new(types.Commit)) + block, err := makeBlock(state, state.LastBlockHeight+1, new(types.Commit)) + assert.NoError(t, err) bps, err := block.MakePartSet(testPartSize) require.NoError(t, err) blockID := types.BlockID{Hash: block.Hash(), PartSetHeader: bps.Header()} @@ -751,7 +757,8 @@ func TestLargeGenesisValidator(t *testing.T) { validatorUpdates, err := types.PB2TM.ValidatorUpdates(abciResponses.ValidatorUpdates) require.NoError(t, err) - block := makeBlock(oldState, oldState.LastBlockHeight+1, new(types.Commit)) + block, err := makeBlock(oldState, oldState.LastBlockHeight+1, new(types.Commit)) + require.NoError(t, err) bps, err := block.MakePartSet(testPartSize) require.NoError(t, err) blockID := types.BlockID{Hash: block.Hash(), PartSetHeader: bps.Header()} @@ -781,7 +788,8 @@ func TestLargeGenesisValidator(t *testing.T) { abciResponses := &abci.ResponseFinalizeBlock{ ValidatorUpdates: []abci.ValidatorUpdate{firstAddedVal}, } - block := makeBlock(oldState, oldState.LastBlockHeight+1, new(types.Commit)) + block, err := makeBlock(oldState, oldState.LastBlockHeight+1, new(types.Commit)) + require.NoError(t, err) bps, err := block.MakePartSet(testPartSize) require.NoError(t, err) @@ -797,7 +805,8 @@ func TestLargeGenesisValidator(t *testing.T) { validatorUpdates, err := types.PB2TM.ValidatorUpdates(abciResponses.ValidatorUpdates) require.NoError(t, err) - block := makeBlock(lastState, lastState.LastBlockHeight+1, new(types.Commit)) + block, err := makeBlock(lastState, lastState.LastBlockHeight+1, new(types.Commit)) + require.NoError(t, err) bps, err = block.MakePartSet(testPartSize) require.NoError(t, err) @@ -833,7 +842,8 @@ func TestLargeGenesisValidator(t *testing.T) { abciResponses := &abci.ResponseFinalizeBlock{ ValidatorUpdates: []abci.ValidatorUpdate{addedVal}, } - block := makeBlock(oldState, oldState.LastBlockHeight+1, new(types.Commit)) + block, err := makeBlock(oldState, oldState.LastBlockHeight+1, new(types.Commit)) + require.NoError(t, err) bps, err := block.MakePartSet(testPartSize) require.NoError(t, err) @@ -851,7 +861,7 @@ func TestLargeGenesisValidator(t *testing.T) { ValidatorUpdates: []abci.ValidatorUpdate{removeGenesisVal}, } - block = makeBlock(oldState, oldState.LastBlockHeight+1, new(types.Commit)) + block, err = makeBlock(oldState, oldState.LastBlockHeight+1, new(types.Commit)) require.NoError(t, err) bps, err = block.MakePartSet(testPartSize) @@ -874,7 +884,8 @@ func TestLargeGenesisValidator(t *testing.T) { abciResponses := &abci.ResponseFinalizeBlock{} validatorUpdates, err = types.PB2TM.ValidatorUpdates(abciResponses.ValidatorUpdates) require.NoError(t, err) - block = makeBlock(curState, curState.LastBlockHeight+1, new(types.Commit)) + block, err = makeBlock(curState, curState.LastBlockHeight+1, new(types.Commit)) + require.NoError(t, err) bps, err := block.MakePartSet(testPartSize) require.NoError(t, err) @@ -900,7 +911,8 @@ func TestLargeGenesisValidator(t *testing.T) { validatorUpdates, err := types.PB2TM.ValidatorUpdates(abciResponses.ValidatorUpdates) require.NoError(t, err) - block := makeBlock(updatedState, updatedState.LastBlockHeight+1, new(types.Commit)) + block, err := makeBlock(updatedState, updatedState.LastBlockHeight+1, new(types.Commit)) + require.NoError(t, err) bps, err := block.MakePartSet(testPartSize) require.NoError(t, err) @@ -1003,7 +1015,8 @@ func TestStateMakeBlock(t *testing.T) { proposerAddress := state.Validators.GetProposer().Address stateVersion := state.Version.Consensus - block := makeBlock(state, 2, new(types.Commit)) + block, err := makeBlock(state, 2, new(types.Commit)) + require.NoError(t, err) // test we set some fields assert.Equal(t, stateVersion, block.Version) @@ -1112,3 +1125,86 @@ func TestStateProto(t *testing.T) { } } } + +func TestMedianTime(t *testing.T) { + val1 := types.NewValidator(ed25519.GenPrivKey().PubKey(), 30, false) + val2 := types.NewValidator(ed25519.GenPrivKey().PubKey(), 30, false) + val3 := types.NewValidator(ed25519.GenPrivKey().PubKey(), 30, false) + + vals := types.NewValidatorSet([]*types.Validator{val1, val2, val3}) + + t.Run("all validators present", func(t *testing.T) { + now := time.Now() + commit := &types.Commit{ + Height: 1, + Signatures: []types.CommitSig{ + { + BlockIDFlag: types.BlockIDFlagCommit, + ValidatorAddress: val1.Address, + Timestamp: now, + }, + { + BlockIDFlag: types.BlockIDFlagCommit, + ValidatorAddress: val2.Address, + Timestamp: now.Add(1 * time.Minute), + }, + { + BlockIDFlag: types.BlockIDFlagCommit, + ValidatorAddress: val3.Address, + Timestamp: now.Add(2 * time.Minute), + }, + }, + } + + medianTime, err := sm.MedianTime(commit, vals) + require.NoError(t, err) + require.Equal(t, medianTime, now.Add(1*time.Minute)) + }) + + t.Run("validator not in validator set", func(t *testing.T) { + unknownVal := ed25519.GenPrivKey().PubKey().Address() + now := time.Now() + commit := &types.Commit{ + Height: 1, + Signatures: []types.CommitSig{ + { + BlockIDFlag: types.BlockIDFlagCommit, + ValidatorAddress: val1.Address, + Timestamp: now, + }, + { + BlockIDFlag: types.BlockIDFlagCommit, + ValidatorAddress: unknownVal, + Timestamp: now.Add(1 * time.Minute), + }, + }, + } + + _, err := sm.MedianTime(commit, vals) + require.Error(t, err) + require.Contains(t, err.Error(), "commit validator not found in validator set") + }) + + t.Run("not all validators present", func(t *testing.T) { + now := time.Now() + commit := &types.Commit{ + Height: 1, + Signatures: []types.CommitSig{ + { + BlockIDFlag: types.BlockIDFlagCommit, + ValidatorAddress: val1.Address, + Timestamp: now, + }, + { + BlockIDFlag: types.BlockIDFlagCommit, + ValidatorAddress: val2.Address, + Timestamp: now.Add(1 * time.Minute), + }, + }, + } + + medianTime, err := sm.MedianTime(commit, vals) + require.NoError(t, err) + require.Equal(t, medianTime, now) + }) +} diff --git a/state/validation.go b/state/validation.go index 287aec31aaa..84a8e79f88e 100644 --- a/state/validation.go +++ b/state/validation.go @@ -119,7 +119,11 @@ func validateBlock(state State, block *types.Block) error { state.LastBlockTime, ) } - medianTime := MedianTime(block.LastCommit, state.LastValidators) + + medianTime, err := MedianTime(block.LastCommit, state.LastValidators) + if err != nil { + return fmt.Errorf("error validating block while calculating median time: %w", err) + } if !block.Time.Equal(medianTime) { return fmt.Errorf("invalid block time. Expected %v, got %v", medianTime, diff --git a/state/validation_test.go b/state/validation_test.go index b4efcd75989..47e08e47951 100644 --- a/state/validation_test.go +++ b/state/validation_test.go @@ -99,9 +99,10 @@ func TestValidateBlockHeader(t *testing.T) { Invalid blocks don't pass */ for _, tc := range testCases { - block := makeBlock(state, height, lastCommit) + block, err := makeBlock(state, height, lastCommit) + require.NoError(t, err) tc.malleateBlock(block) - err := blockExec.ValidateBlock(state, block) + err = blockExec.ValidateBlock(state, block) require.Error(t, err, tc.name) } @@ -116,9 +117,10 @@ func TestValidateBlockHeader(t *testing.T) { } nextHeight := validationTestsStopHeight - block := makeBlock(state, nextHeight, lastCommit) + block, err := makeBlock(state, nextHeight, lastCommit) + require.NoError(t, err) state.InitialHeight = nextHeight + 1 - err := blockExec.ValidateBlock(state, block) + err = blockExec.ValidateBlock(state, block) require.Error(t, err, "expected an error when state is ahead of block") assert.Contains(t, err.Error(), "lower than initial height") } @@ -184,22 +186,18 @@ func TestValidateBlockCommit(t *testing.T) { BlockID: state.LastBlockID, Signatures: []types.CommitSig{wrongHeightVote.CommitSig()}, } - block := makeBlock(state, height, wrongHeightCommit) - err := blockExec.ValidateBlock(state, block) + block, err := makeBlock(state, height, wrongHeightCommit) + require.NoError(t, err) + err = blockExec.ValidateBlock(state, block) _, isErrInvalidCommitHeight := err.(types.ErrInvalidCommitHeight) require.True(t, isErrInvalidCommitHeight, "expected ErrInvalidCommitHeight at height %d but got: %v", height, err) /* #2589: test len(block.LastCommit.Signatures) == state.LastValidators.Size() */ - block = makeBlock(state, height, wrongSigsCommit) - err = blockExec.ValidateBlock(state, block) - _, isErrInvalidCommitSignatures := err.(types.ErrInvalidCommitSignatures) - require.True(t, isErrInvalidCommitSignatures, - "expected ErrInvalidCommitSignatures at height %d, but got: %v", - height, - err, - ) + _, err = makeBlock(state, height, wrongSigsCommit) + require.Error(t, err) + require.ErrorContains(t, err, "error making block") } /* @@ -326,9 +324,10 @@ func TestValidateBlockEvidence(t *testing.T) { evidence = append(evidence, newEv) currentBytes += int64(len(newEv.Bytes())) } - block := state.MakeBlock(height, test.MakeNTxs(height, 10), lastCommit, evidence, proposerAddr) + block, err := state.MakeBlock(height, test.MakeNTxs(height, 10), lastCommit, evidence, proposerAddr) + require.NoError(t, err) - err := blockExec.ValidateBlock(state, block) + err = blockExec.ValidateBlock(state, block) if assert.Error(t, err) { _, ok := err.(*types.ErrEvidenceOverflow) require.True(t, ok, "expected error to be of type ErrEvidenceOverflow at height %d but got %v", height, err) @@ -367,3 +366,148 @@ func TestValidateBlockEvidence(t *testing.T) { } } + +func TestValidateBlockTime(t *testing.T) { + proxyApp := newTestApp() + require.NoError(t, proxyApp.Start()) + defer proxyApp.Stop() //nolint:errcheck // ignore for tests + + state, stateDB, privVals := makeState(3, 1) + stateStore := sm.NewStore(stateDB, sm.StoreOptions{ + DiscardABCIResponses: false, + }) + 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).Return(nil) + + blockStore := store.NewBlockStore(dbm.NewMemDB()) + + blockExec := sm.NewBlockExecutor( + stateStore, + log.TestingLogger(), + proxyApp.Consensus(), + mp, + sm.EmptyEvidencePool{}, + blockStore, + ) + lastCommit := &types.Commit{} + var lastExtCommit *types.ExtendedCommit + + // Build up state for test + for height := int64(1); height < 3; height++ { + var err error + state, _, lastExtCommit, err = makeAndCommitGoodBlock( + state, height, lastCommit, state.Validators.GetProposer().Address, blockExec, privVals, nil) + require.NoError(t, err, "height %d", height) + lastCommit = lastExtCommit.ToCommit() + } + + t.Run("block time before last block time", func(t *testing.T) { + height := int64(3) + block, err := makeBlock(state, height, lastCommit) + require.NoError(t, err) + + // Set time to before last block time + block.Time = block.Time.Add(-time.Millisecond * 10) + err = blockExec.ValidateBlock(state, block) + + require.ErrorContains(t, err, "not greater than last block time") + }) + + t.Run("block time after last block time, different than median time", func(t *testing.T) { + height := int64(3) + block, err := makeBlock(state, height, lastCommit) + require.NoError(t, err) + // Set time to after the median time + block.Time = block.Time.Add(time.Second) + err = blockExec.ValidateBlock(state, block) + require.Error(t, err) + require.Contains(t, err.Error(), "invalid block time") + }) + + t.Run("block time after last block time, same as median time", func(t *testing.T) { + height := int64(3) + block, err := makeBlock(state, height, lastCommit) + require.NoError(t, err) + err = blockExec.ValidateBlock(state, block) + require.NoError(t, err) + }) +} + +func TestValidateBlockInvalidCommit(t *testing.T) { + proxyApp := newTestApp() + require.NoError(t, proxyApp.Start()) + defer proxyApp.Stop() //nolint:errcheck // ignore for tests + + state, stateDB, privVals := makeState(3, 1) + stateStore := sm.NewStore(stateDB, sm.StoreOptions{ + DiscardABCIResponses: false, + }) + 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).Return(nil) + + blockStore := store.NewBlockStore(dbm.NewMemDB()) + + blockExec := sm.NewBlockExecutor( + stateStore, + log.TestingLogger(), + proxyApp.Consensus(), + mp, + sm.EmptyEvidencePool{}, + blockStore, + ) + lastCommit := &types.Commit{} + var lastExtCommit *types.ExtendedCommit + + // Build up state for test + for height := int64(1); height < 3; height++ { + var err error + state, _, lastExtCommit, err = makeAndCommitGoodBlock( + state, height, lastCommit, state.Validators.GetProposer().Address, blockExec, privVals, nil) + require.NoError(t, err, "height %d", height) + lastCommit = lastExtCommit.ToCommit() + } + + t.Run("commit with unknown validator flagged as commit", func(t *testing.T) { + height := int64(3) + + // Create a commit where only unknown validators have BlockIDFlagCommit + unknownVal := ed25519.GenPrivKey() + now := time.Now() + + invalidCommit := &types.Commit{ + Height: height - 1, + Round: 0, + BlockID: state.LastBlockID, + Signatures: []types.CommitSig{ + { + BlockIDFlag: types.BlockIDFlagCommit, + ValidatorAddress: unknownVal.PubKey().Address(), + Timestamp: now, + Signature: []byte("dummy"), + }, + }, + } + + _, err := makeBlock(state, height, invalidCommit) + require.Error(t, err) + require.Contains(t, err.Error(), "commit validator not found in validator set") + }) +} diff --git a/store/store_test.go b/store/store_test.go index 63b6d5492ac..bcd5248ea7d 100644 --- a/store/store_test.go +++ b/store/store_test.go @@ -157,7 +157,8 @@ func TestBlockStoreSaveLoadBlock(t *testing.T) { // save a block big enough to have two block parts txs := []types.Tx{make([]byte, types.BlockPartSizeBytes)} // TX taking one block part alone - block := state.MakeBlock(bs.Height()+1, txs, new(types.Commit), nil, state.Validators.GetProposer().Address) + block, err := state.MakeBlock(bs.Height()+1, txs, new(types.Commit), nil, state.Validators.GetProposer().Address) + require.NoError(t, err) validPartSet, err := block.MakePartSet(types.BlockPartSizeBytes) require.NoError(t, err) require.GreaterOrEqual(t, validPartSet.Total(), uint32(2)) @@ -399,7 +400,8 @@ func TestSaveBlockWithExtendedCommitPanicOnAbsentExtension(t *testing.T) { state, bs, cleanup := makeStateAndBlockStore() defer cleanup() h := bs.Height() + 1 - block := state.MakeBlock(h, test.MakeNTxs(h, 10), new(types.Commit), nil, state.Validators.GetProposer().Address) + block, err := state.MakeBlock(h, test.MakeNTxs(h, 10), new(types.Commit), nil, state.Validators.GetProposer().Address) + require.NoError(t, err) seenCommit := makeTestExtCommit(block.Header.Height, cmttime.Now()) ps, err := block.MakePartSet(types.BlockPartSizeBytes) @@ -440,8 +442,9 @@ func TestLoadBlockExtendedCommit(t *testing.T) { state, bs, cleanup := makeStateAndBlockStore() defer cleanup() h := bs.Height() + 1 - block := state.MakeBlock(h, test.MakeNTxs(h, 10), new(types.Commit), nil, state.Validators.GetProposer().Address) - seenCommit := makeTestExtCommit(block.Header.Height, cmttime.Now()) + block, err := state.MakeBlock(h, test.MakeNTxs(h, 10), new(types.Commit), nil, state.Validators.GetProposer().Address) + require.NoError(t, err) + seenCommit := makeTestExtCommit(block.Height, cmttime.Now()) ps, err := block.MakePartSet(types.BlockPartSizeBytes) require.NoError(t, err) if testCase.saveExtended { @@ -470,7 +473,8 @@ func TestLoadBaseMeta(t *testing.T) { bs := NewBlockStore(dbm.NewMemDB()) for h := int64(1); h <= 10; h++ { - block := state.MakeBlock(h, test.MakeNTxs(h, 10), new(types.Commit), nil, state.Validators.GetProposer().Address) + block, err := state.MakeBlock(h, test.MakeNTxs(h, 10), new(types.Commit), nil, state.Validators.GetProposer().Address) + require.NoError(t, err) partSet, err := block.MakePartSet(types.BlockPartSizeBytes) require.NoError(t, err) seenCommit := makeTestExtCommit(h, cmttime.Now()) @@ -515,7 +519,8 @@ func TestLoadBlockPart(t *testing.T) { require.Contains(t, panicErr.Error(), "unmarshal to cmtproto.Part failed") // 3. A good block serialized and saved to the DB should be retrievable - block := state.MakeBlock(height, nil, new(types.Commit), nil, state.Validators.GetProposer().Address) + block, err := state.MakeBlock(height, nil, new(types.Commit), nil, state.Validators.GetProposer().Address) + require.NoError(t, err) partSet, err := block.MakePartSet(types.BlockPartSizeBytes) require.NoError(t, err) part1 := partSet.GetPart(0) @@ -560,7 +565,8 @@ func TestPruneBlocks(t *testing.T) { // make more than 1000 blocks, to test batch deletions for h := int64(1); h <= 1500; h++ { - block := state.MakeBlock(h, test.MakeNTxs(h, 10), new(types.Commit), nil, state.Validators.GetProposer().Address) + block, err := state.MakeBlock(h, test.MakeNTxs(h, 10), new(types.Commit), nil, state.Validators.GetProposer().Address) + require.NoError(t, err) partSet, err := block.MakePartSet(types.BlockPartSizeBytes) require.NoError(t, err) seenCommit := makeTestExtCommit(h, cmttime.Now()) @@ -689,7 +695,8 @@ func TestLoadBlockMetaByHash(t *testing.T) { require.NoError(t, err) bs := NewBlockStore(dbm.NewMemDB()) - b1 := state.MakeBlock(state.LastBlockHeight+1, test.MakeNTxs(state.LastBlockHeight+1, 10), new(types.Commit), nil, state.Validators.GetProposer().Address) + b1, err := state.MakeBlock(state.LastBlockHeight+1, test.MakeNTxs(state.LastBlockHeight+1, 10), new(types.Commit), nil, state.Validators.GetProposer().Address) + require.NoError(t, err) partSet, err := b1.MakePartSet(types.BlockPartSizeBytes) require.NoError(t, err) seenCommit := makeTestExtCommit(1, cmttime.Now()) @@ -705,7 +712,8 @@ func TestBlockFetchAtHeight(t *testing.T) { state, bs, cleanup := makeStateAndBlockStore() defer cleanup() require.Equal(t, bs.Height(), int64(0), "initially the height should be zero") - block := state.MakeBlock(bs.Height()+1, nil, new(types.Commit), nil, state.Validators.GetProposer().Address) + block, err := state.MakeBlock(bs.Height()+1, nil, new(types.Commit), nil, state.Validators.GetProposer().Address) + require.NoError(t, err) partSet, err := block.MakePartSet(types.BlockPartSizeBytes) require.NoError(t, err) diff --git a/tests.mk b/tests.mk index ec461eb69de..765d2bd3fdc 100644 --- a/tests.mk +++ b/tests.mk @@ -61,7 +61,7 @@ vagrant_test: ### go tests test: @echo "--> Running go test" - @go test -p 1 $(PACKAGES) -tags deadlock + @go test -p 1 $(PACKAGES) .PHONY: test test_race: @@ -71,5 +71,5 @@ test_race: test_deadlock: @echo "--> Running go test --deadlock" - @go test -p 1 -v $(PACKAGES) -tags deadlock + @go test -p 1 -v $(PACKAGES) -tags deadlock .PHONY: test_race diff --git a/types/validation.go b/types/validation.go index d15b35f7e50..af9940ac65b 100644 --- a/types/validation.go +++ b/types/validation.go @@ -1,6 +1,7 @@ package types import ( + "bytes" "errors" "fmt" @@ -244,6 +245,10 @@ func verifyCommitBatch( // them by index else we need to retrieve them by address if lookUpByIndex { val = vals.Validators[idx] + if !bytes.Equal(val.Address, commitSig.ValidatorAddress) { + return fmt.Errorf("validator address mismatch at index %d: expected %X, got %X", + idx, val.Address, commitSig.ValidatorAddress) + } } else { valIdx, val = vals.GetByAddress(commitSig.ValidatorAddress) @@ -352,6 +357,10 @@ func verifyCommitSingle( // them by index else we need to retrieve them by address if lookUpByIndex { val = vals.Validators[idx] + if !bytes.Equal(val.Address, commitSig.ValidatorAddress) { + return fmt.Errorf("validator address mismatch at index %d: expected %X, got %X", + idx, val.Address, commitSig.ValidatorAddress) + } } else { valIdx, val = vals.GetByAddress(commitSig.ValidatorAddress)