Skip to content

Commit

Permalink
ABCI 2.0: Check if it makes sense to cherry-pick something from (tm)#…
Browse files Browse the repository at this point in the history
…7768 (#7)

* [cherry-picked] Fixing handling of contexts in the ABCI++ rebased branch (#7768)

* Fixing context

* Removed logger change

* Fixing UTs

* Bump

* Fix build, make UTs pass

* Thread contexts to the concerned test cases

* Placate linter

* Make TestCheckSwitchToConsensusLastHeightZero less flaky

* bump

* Update blocksync/reactor_test.go
  • Loading branch information
sergio-mena committed Jan 11, 2023
1 parent 4255d5d commit 0b340da
Show file tree
Hide file tree
Showing 10 changed files with 124 additions and 46 deletions.
6 changes: 5 additions & 1 deletion blocksync/reactor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,11 @@ func TestCheckSwitchToConsensusLastHeightZero(t *testing.T) {
}
}

// -1 because of "-1" in IsCaughtUp
// -1 pool.height points to the _next_ height
// -1 because we measure height of block store
const maxDiff = 3
for _, r := range reactorPairs {
assert.GreaterOrEqual(t, r.reactor.store.Height(), maxBlockHeight-2)
assert.GreaterOrEqual(t, r.reactor.store.Height(), maxBlockHeight-maxDiff)
}
}
17 changes: 12 additions & 5 deletions consensus/byzantine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,9 @@ import (

// Byzantine node sends two different prevotes (nil and blockID) to the same validator
func TestByzantinePrevoteEquivocation(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()

const nValidators = 4
const byzantineNode = 0
const prevoteHeight = int64(2)
Expand Down Expand Up @@ -223,7 +226,7 @@ func TestByzantinePrevoteEquivocation(t *testing.T) {
proposerAddr := lazyProposer.privValidatorPubKey.Address()

block, err := lazyProposer.blockExec.CreateProposalBlock(
lazyProposer.Height, lazyProposer.state, extCommit, proposerAddr)
ctx, lazyProposer.Height, lazyProposer.state, extCommit, proposerAddr)
require.NoError(t, err)
blockParts, err := block.MakePartSet(types.BlockPartSizeBytes)
require.NoError(t, err)
Expand Down Expand Up @@ -312,6 +315,10 @@ func TestByzantinePrevoteEquivocation(t *testing.T) {
func TestByzantineConflictingProposalsWithPartition(t *testing.T) {
N := 4
logger := consensusLogger().With("test", "byzantine")

ctx, cancel := context.WithCancel(context.Background())
defer cancel()

app := newKVStore
css, cleanup := randConsensusNet(t, N, "consensus_byzantine_test", newMockTickerFunc(false), app)
defer cleanup()
Expand Down Expand Up @@ -347,7 +354,7 @@ func TestByzantineConflictingProposalsWithPartition(t *testing.T) {
css[i].privValidator.(types.MockPV).DisableChecks()
css[i].decideProposal = func(j int32) func(int64, int32) {
return func(height int64, round int32) {
byzantineDecideProposalFunc(t, height, round, css[j], switches[j])
byzantineDecideProposalFunc(ctx, t, height, round, css[j], switches[j])
}
}(int32(i))
// We are setting the prevote function to do nothing because the prevoting
Expand Down Expand Up @@ -466,12 +473,12 @@ func TestByzantineConflictingProposalsWithPartition(t *testing.T) {
//-------------------------------
// byzantine consensus functions

func byzantineDecideProposalFunc(t *testing.T, height int64, round int32, cs *State, sw *p2p.Switch) {
func byzantineDecideProposalFunc(ctx context.Context, t *testing.T, height int64, round int32, cs *State, sw *p2p.Switch) {
// byzantine user should create two proposals and try to split the vote.
// Avoid sending on internalMsgQueue and running consensus state.

// Create a new proposal block from state/txs from the mempool.
block1, err := cs.createProposalBlock()
block1, err := cs.createProposalBlock(ctx)
require.NoError(t, err)
blockParts1, err := block1.MakePartSet(types.BlockPartSizeBytes)
require.NoError(t, err)
Expand All @@ -488,7 +495,7 @@ func byzantineDecideProposalFunc(t *testing.T, height int64, round int32, cs *St
deliverTxsRange(t, cs, 0, 1)

// Create a new proposal block from state/txs from the mempool.
block2, err := cs.createProposalBlock()
block2, err := cs.createProposalBlock(ctx)
require.NoError(t, err)
blockParts2, err := block2.MakePartSet(types.BlockPartSizeBytes)
require.NoError(t, err)
Expand Down
9 changes: 5 additions & 4 deletions consensus/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -212,14 +212,15 @@ func startTestRound(cs *State, height int64, round int32) {

// Create proposal block from cs1 but sign it with vs.
func decideProposal(
ctx context.Context,
t *testing.T,
cs1 *State,
vs *validatorStub,
height int64,
round int32,
) (proposal *types.Proposal, block *types.Block) {
) (*types.Proposal, *types.Block) {
cs1.mtx.Lock()
block, err := cs1.createProposalBlock()
block, err := cs1.createProposalBlock(ctx)
require.NoError(t, err)
blockParts, err := block.MakePartSet(types.BlockPartSizeBytes)
require.NoError(t, err)
Expand All @@ -232,15 +233,15 @@ func decideProposal(

// Make proposal
polRound, propBlockID := validRound, types.BlockID{Hash: block.Hash(), PartSetHeader: blockParts.Header()}
proposal = types.NewProposal(height, round, polRound, propBlockID)
proposal := types.NewProposal(height, round, polRound, propBlockID)
p := proposal.ToProto()
if err := vs.SignProposal(chainID, p); err != nil {
panic(err)
}

proposal.Signature = p.Signature

return
return proposal, block
}

func addVotes(to *State, votes ...*types.Vote) {
Expand Down
5 changes: 4 additions & 1 deletion consensus/reactor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,9 @@ func TestSwitchToConsensusVoteExtensions(t *testing.T) {
},
} {
t.Run(testCase.name, func(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()

cs, vs := randState(1)
validator := vs[0]
validator.Height = testCase.storedHeight
Expand All @@ -364,7 +367,7 @@ func TestSwitchToConsensusVoteExtensions(t *testing.T) {
cs.state.LastValidators = cs.state.Validators.Copy()
cs.state.ConsensusParams.ABCI.VoteExtensionsEnableHeight = testCase.initialRequiredHeight

propBlock, err := cs.createProposalBlock()
propBlock, err := cs.createProposalBlock(ctx)
require.NoError(t, err)

// Consensus is preparing to do the next height after the stored height.
Expand Down
11 changes: 7 additions & 4 deletions consensus/replay_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -313,6 +313,9 @@ var modes = []uint{0, 1, 2, 3}

// This is actually not a test, it's for storing validator change tx data for testHandshakeReplay
func setupChainWithChangingValidators(t *testing.T, name string, nBlocks int) (*cfg.Config, []*types.Block, []*types.ExtendedCommit, sm.State) {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()

nPeers := 7
nVals := 4
css, genDoc, config, cleanup := randConsensusNetWithPeers(
Expand Down Expand Up @@ -358,7 +361,7 @@ func setupChainWithChangingValidators(t *testing.T, name string, nBlocks int) (*
newValidatorTx1 := kvstore.MakeValSetChangeTx(valPubKey1ABCI, testMinPower)
err = assertMempool(css[0].txNotifier).CheckTx(newValidatorTx1, nil, mempool.TxInfo{})
assert.NoError(t, err)
propBlock, err := css[0].createProposalBlock() // changeProposer(t, cs1, vs2)
propBlock, err := css[0].createProposalBlock(ctx) // changeProposer(t, cs1, vs2)
require.NoError(t, err)
propBlockParts, err := propBlock.MakePartSet(partSize)
require.NoError(t, err)
Expand Down Expand Up @@ -390,7 +393,7 @@ func setupChainWithChangingValidators(t *testing.T, name string, nBlocks int) (*
updateValidatorTx1 := kvstore.MakeValSetChangeTx(updatePubKey1ABCI, 25)
err = assertMempool(css[0].txNotifier).CheckTx(updateValidatorTx1, nil, mempool.TxInfo{})
assert.NoError(t, err)
propBlock, err = css[0].createProposalBlock() // changeProposer(t, cs1, vs2)
propBlock, err = css[0].createProposalBlock(ctx) // changeProposer(t, cs1, vs2)
require.NoError(t, err)
propBlockParts, err = propBlock.MakePartSet(partSize)
require.NoError(t, err)
Expand Down Expand Up @@ -429,7 +432,7 @@ func setupChainWithChangingValidators(t *testing.T, name string, nBlocks int) (*
newValidatorTx3 := kvstore.MakeValSetChangeTx(newVal3ABCI, testMinPower)
err = assertMempool(css[0].txNotifier).CheckTx(newValidatorTx3, nil, mempool.TxInfo{})
assert.NoError(t, err)
propBlock, err = css[0].createProposalBlock() // changeProposer(t, cs1, vs2)
propBlock, err = css[0].createProposalBlock(ctx) // changeProposer(t, cs1, vs2)
require.NoError(t, err)
propBlockParts, err = propBlock.MakePartSet(partSize)
require.NoError(t, err)
Expand Down Expand Up @@ -506,7 +509,7 @@ func setupChainWithChangingValidators(t *testing.T, name string, nBlocks int) (*
removeValidatorTx3 := kvstore.MakeValSetChangeTx(newVal3ABCI, 0)
err = assertMempool(css[0].txNotifier).CheckTx(removeValidatorTx3, nil, mempool.TxInfo{})
assert.NoError(t, err)
propBlock, err = css[0].createProposalBlock() // changeProposer(t, cs1, vs2)
propBlock, err = css[0].createProposalBlock(ctx) // changeProposer(t, cs1, vs2)
require.NoError(t, err)
propBlockParts, err = propBlock.MakePartSet(partSize)
require.NoError(t, err)
Expand Down
11 changes: 6 additions & 5 deletions consensus/state.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package consensus

import (
"bytes"
"context"
"errors"
"fmt"
"io"
Expand Down Expand Up @@ -1175,7 +1176,7 @@ func (cs *State) defaultDecideProposal(height int64, round int32) {
} else {
// Create a new proposal block from state/txs from the mempool.
var err error
block, err = cs.createProposalBlock()
block, err = cs.createProposalBlock(context.TODO())
if err != nil {
cs.Logger.Error("unable to create proposal block", "error", err)
return
Expand Down Expand Up @@ -1240,7 +1241,7 @@ func (cs *State) isProposalComplete() bool {
//
// NOTE: keep it side-effect free for clarity.
// CONTRACT: cs.privValidator is not nil.
func (cs *State) createProposalBlock() (*types.Block, error) {
func (cs *State) createProposalBlock(ctx context.Context) (*types.Block, error) {
if cs.privValidator == nil {
return nil, errors.New("entered createProposalBlock with privValidator being nil")
}
Expand Down Expand Up @@ -1269,7 +1270,7 @@ func (cs *State) createProposalBlock() (*types.Block, error) {

proposerAddr := cs.privValidatorPubKey.Address()

ret, err := cs.blockExec.CreateProposalBlock(cs.Height, cs.state, lastExtCommit, proposerAddr)
ret, err := cs.blockExec.CreateProposalBlock(ctx, cs.Height, cs.state, lastExtCommit, proposerAddr)
if err != nil {
panic(err)
}
Expand Down Expand Up @@ -2131,7 +2132,7 @@ func (cs *State) addVote(vote *types.Vote, peerID p2p.ID) (added bool, err error
return false, err
}

if err = cs.blockExec.VerifyVoteExtension(vote); err != nil {
if err = cs.blockExec.VerifyVoteExtension(context.TODO(), vote); err != nil {
return false, err
}
}
Expand Down Expand Up @@ -2313,7 +2314,7 @@ func (cs *State) signVote(
// if the signedMessage type is for a non-nil precommit, add
// VoteExtension
if extEnabled {
ext, err := cs.blockExec.ExtendVote(vote)
ext, err := cs.blockExec.ExtendVote(context.TODO(), vote)
if err != nil {
return nil, err
}
Expand Down
Loading

0 comments on commit 0b340da

Please sign in to comment.