Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: remove proposer from ValidatorSet #56

Merged
merged 13 commits into from
Apr 16, 2020
Merged

fix: remove proposer from ValidatorSet #56

Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
0c104af
fix: remove proposer from ValidatorSet
Apr 13, 2020
c07da9d
fix: test failure
Apr 14, 2020
04019e9
fix: test failure
Apr 14, 2020
a45a5c7
fix: lint errors
Apr 14, 2020
1453882
fix: increase timeout
Apr 14, 2020
6bd7af7
fix: test failure
Apr 14, 2020
adbd3d6
Merge branch 'develop' of https://github.com/line/tendermint into fix…
Apr 14, 2020
dabb16e
fix: test failure
Apr 14, 2020
6babcbc
fix: test failure
Apr 14, 2020
8b57fe5
fix: lint error
Apr 14, 2020
0870d74
fix: revert timeout
Apr 16, 2020
eb21bfe
fix: apply comment
Apr 16, 2020
670a173
fix: apply comment
Apr 16, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion blockchain/v0/reactor_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -348,7 +348,8 @@ func makeTxs(height int64) (txs []types.Tx) {
func makeBlock(privVal types.PrivValidator, height int64, state sm.State, lastCommit *types.Commit) *types.Block {
message := state.MakeHashMessage(0)
proof, _ := privVal.GenerateVRFProof(message)
block, _ := state.MakeBlock(height, makeTxs(height), lastCommit, nil, state.Validators.GetProposer().Address, 0, proof)
block, _ := state.MakeBlock(height, makeTxs(height), lastCommit, nil,
types.SelectProposer(state.Validators, state.LastProofHash, height, 0).Address, 0, proof)
return block
}

Expand Down
3 changes: 2 additions & 1 deletion blockchain/v1/reactor_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -422,7 +422,8 @@ func makeTxs(height int64) (txs []types.Tx) {
func makeBlock(privVal types.PrivValidator, height int64, state sm.State, lastCommit *types.Commit) *types.Block {
message := state.MakeHashMessage(0)
proof, _ := privVal.GenerateVRFProof(message)
block, _ := state.MakeBlock(height, makeTxs(height), lastCommit, nil, state.Validators.GetProposer().Address, 0, proof)
block, _ := state.MakeBlock(height, makeTxs(height), lastCommit, nil,
types.SelectProposer(state.Validators, state.LastProofHash, height, 0).Address, 0, proof)
return block
}

Expand Down
2 changes: 1 addition & 1 deletion consensus/common_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ type cleanupFunc func()
// genesis, chain_id, priv_val
var config *cfg.Config // NOTE: must be reset for each _test.go file
var consensusReplayConfig *cfg.Config
var ensureTimeout = time.Millisecond * 100
var ensureTimeout = time.Millisecond * 1000
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you change this timeout value?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll revert it.


func ensureDir(dir string, mode os.FileMode) {
if err := tmos.EnsureDir(dir, mode); err != nil {
Expand Down
3 changes: 1 addition & 2 deletions consensus/replay.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -305,8 +305,7 @@ func (h *Handshaker) ReplayBlocks(
for i, val := range h.genDoc.Validators {
validators[i] = types.NewValidator(val.PubKey, val.Power)
}
roundHash := types.MakeRoundHash(h.genDoc.Hash(), 0, 0)
validatorSet := types.NewRandomValidatorSet(validators, roundHash)
validatorSet := types.NewValidatorSet(validators)
nextVals := types.TM2PB.ValidatorUpdates(validatorSet)
csParams := types.TM2PB.ConsensusParams(h.genDoc.ConsensusParams)
req := abci.RequestInitChain{
Expand Down
3 changes: 2 additions & 1 deletion consensus/replay_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -920,7 +920,8 @@ func makeBlock(state sm.State, lastBlock *types.Block, lastBlockMeta *types.Bloc

message := state.MakeHashMessage(0)
proof, _ := privVal.GenerateVRFProof(message)
return state.MakeBlock(height, []types.Tx{}, lastCommit, nil, state.Validators.GetProposer().Address, 0, proof)
return state.MakeBlock(height, []types.Tx{}, lastCommit, nil,
types.SelectProposer(state.Validators, state.LastProofHash, height, 0).Address, 0, proof)
}

type badApp struct {
Expand Down
18 changes: 5 additions & 13 deletions consensus/state.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -827,21 +827,13 @@ func (cs *State) enterNewRound(height int64, round int) {

logger.Info(fmt.Sprintf("enterNewRound(%v/%v). Current: %v/%v/%v", height, round, cs.Height, cs.Round, cs.Step))

// Increment validators if necessary
validators := cs.Validators
if cs.Round < round {
validators = validators.Copy()
validators.IncrementProposerPriority(round - cs.Round)
}

// Select the current height and round Proposer
validators.SelectProposerWithRound(cs.state.LastProofHash, height, round)
cs.Proposer = types.SelectProposer(cs.Validators, cs.state.LastProofHash, height, round)

// Setup new round
// we don't fire newStep for this step,
// but we fire an event, so update the round step first
cs.updateRoundStep(round, cstypes.RoundStepNewRound)
cs.Validators = validators
if round == 0 {
// We've already reset these upon new height,
// and meanwhile we might have received a proposal
Expand Down Expand Up @@ -934,21 +926,21 @@ func (cs *State) enterPropose(height int64, round int) {
if cs.isProposer(address) {
logger.Info("enterPropose: Our turn to propose",
"proposer",
cs.Validators.GetProposer().Address,
cs.Proposer.Address,
"privValidator",
cs.privValidator)
cs.decideProposal(height, round)
} else {
logger.Info("enterPropose: Not our turn to propose",
"proposer",
cs.Validators.GetProposer().Address,
cs.Proposer.Address,
"privValidator",
cs.privValidator)
}
}

func (cs *State) isProposer(address []byte) bool {
return bytes.Equal(cs.Validators.GetProposer().Address, address)
return bytes.Equal(cs.Proposer.Address, address)
}

func (cs *State) defaultDecideProposal(height int64, round int) {
Expand Down Expand Up @@ -1559,7 +1551,7 @@ func (cs *State) defaultSetProposal(proposal *types.Proposal) error {
}

// Verify signature
if !cs.Validators.GetProposer().PubKey.VerifyBytes(proposal.SignBytes(cs.state.ChainID), proposal.Signature) {
if !cs.Proposer.PubKey.VerifyBytes(proposal.SignBytes(cs.state.ChainID), proposal.Signature) {
return ErrInvalidProposalSignature
}

Expand Down
11 changes: 8 additions & 3 deletions consensus/state_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ func TestStateProposerSelection0(t *testing.T) {
ensureNewRound(newRoundCh, height, round)

// Commit a block and ensure proposer for the next height is correct.
prop := cs1.GetRoundState().Validators.GetProposer()
prop := cs1.GetRoundState().Proposer
address := cs1.privValidator.GetPubKey().Address()
if !bytes.Equal(prop.Address, address) {
t.Fatalf("expected proposer to be validator %d. Got %X", 0, prop.Address)
Expand All @@ -82,7 +82,7 @@ func TestStateProposerSelection0(t *testing.T) {
// Wait for new round so next validator is set.
ensureNewRound(newRoundCh, height+1, 0)

prop = cs1.GetRoundState().Validators.GetProposer()
prop = cs1.GetRoundState().Proposer
addr := vss[1].GetPubKey().Address()
if !bytes.Equal(prop.Address, addr) {
panic(fmt.Sprintf("expected proposer to be validator %d. Got %X", 1, prop.Address))
Expand All @@ -109,7 +109,7 @@ func TestStateProposerSelection2(t *testing.T) {

// everyone just votes nil. we get a new proposer each round
for i := 0; i < len(vss); i++ {
prop := cs1.GetRoundState().Validators.GetProposer()
prop := cs1.GetRoundState().Proposer
addr := vss[(i+round)%len(vss)].GetPubKey().Address()
correctProposer := addr
if !bytes.Equal(prop.Address, correctProposer) {
Expand Down Expand Up @@ -1033,6 +1033,9 @@ func TestValidateValidBlockOnCommit(t *testing.T) {
addr := cs1.privValidator.GetPubKey().Address()
voteCh := subscribeToVoter(cs1, addr)

// Set the proofHash value arbitrarily to ensure that the first vss is elected proposer.
cs1.state.LastProofHash = []byte{2}

// start round and wait for propose and prevote
startTestRound(cs1, cs1.Height, round)
ensureNewRound(newRoundCh, height, round)
Expand Down Expand Up @@ -1438,6 +1441,8 @@ func TestCommitFromPreviousRound(t *testing.T) {
validBlockCh := subscribe(cs1.eventBus, types.EventQueryValidBlock)
proposalCh := subscribe(cs1.eventBus, types.EventQueryCompleteProposal)

// Set the proofHash value arbitrarily to ensure that the first vss is elected proposer.
cs1.state.LastProofHash = []byte{2}
prop, propBlock := decideProposal(cs1, vs2, vs2.Height, vs2.Round)
propBlockHash := propBlock.Hash()
propBlockParts := propBlock.MakePartSet(partSize)
Expand Down
5 changes: 3 additions & 2 deletions consensus/types/round_state.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,7 @@ type RoundState struct {
// Subjective time when +2/3 precommits for Block at Round were found
CommitTime time.Time `json:"commit_time"`
Validators *types.ValidatorSet `json:"validators"`
Proposer *types.Validator `json:"proposer"`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It needs to output a string for the new field with StringIndented().

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's good point. Thank you for that.

Proposal *types.Proposal `json:"proposal"`
ProposalBlock *types.Block `json:"proposal_block"`
ProposalBlockParts *types.PartSet `json:"proposal_block_parts"`
Expand Down Expand Up @@ -111,7 +112,7 @@ func (rs *RoundState) RoundStateSimple() RoundStateSimple {
panic(err)
}

addr := rs.Validators.GetProposer().Address
addr := rs.Proposer.Address
idx, _ := rs.Validators.GetByAddress(addr)

return RoundStateSimple{
Expand All @@ -130,7 +131,7 @@ func (rs *RoundState) RoundStateSimple() RoundStateSimple {

// NewRoundEvent returns the RoundState with proposer information as an event.
func (rs *RoundState) NewRoundEvent() types.EventDataNewRound {
addr := rs.Validators.GetProposer().Address
addr := rs.Proposer.Address
idx, _ := rs.Validators.GetByAddress(addr)

return types.EventDataNewRound{
Expand Down
4 changes: 2 additions & 2 deletions evidence/pool_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ func initializeValidatorState(valAddr []byte, height int64) dbm.DB {
state := sm.State{
LastBlockHeight: 0,
LastBlockTime: tmtime.Now(),
Validators: types.NewRandomValidatorSet(vals, types.MakeRoundHash([]byte{}, 1, 0)),
NextValidators: types.NewRandomValidatorSet(vals, types.MakeRoundHash([]byte{}, 2, 0)),
Validators: types.NewValidatorSet(vals),
NextValidators: types.NewValidatorSet(vals),
LastHeightValidatorsChanged: 1,
ConsensusParams: types.ConsensusParams{
Evidence: types.EvidenceParams{
Expand Down
12 changes: 6 additions & 6 deletions state/execution_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ func TestApplyBlock(t *testing.T) {
blockExec := sm.NewBlockExecutor(stateDB, log.TestingLogger(), proxyApp.Consensus(),
mock.Mempool{}, sm.MockEvidencePool{})

block := makeBlockWithPrivVal(state, privVals[state.Validators.Proposer.Address.String()], 1)
block := makeBlockWithPrivVal(state, privVals[state.Validators.Validators[0].Address.String()], 1)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you tell me what your intention is in replacing Proposer with Validator[0] in this test case?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ValidatorSet.Proposer is not available any more. In this test, validatorSet has only one validator, so Validators[0] is the proposer in this case.
If we must get the proposer among several Validators, we should use SelectProposer() function instead of ValidatorSet.Proposer

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For tests with more than one Validator, I modified it to use SelectProposer().

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, the reason why Validator[0] can be swapped with Proposer is because there is only one validator in ValidatorSet.

blockID := types.BlockID{Hash: block.Hash(), PartsHeader: block.MakePartSet(testPartSize).Header()}

//nolint:ineffassign
Expand Down Expand Up @@ -89,10 +89,10 @@ func TestBeginBlockValidators(t *testing.T) {
lastCommit := types.NewCommit(1, 0, prevBlockID, tc.lastCommitSigs)

message := state.MakeHashMessage(0)
proof, _ := privVals[state.Validators.GetProposer().Address.String()].GenerateVRFProof(message)
proof, _ := privVals[state.Validators.Validators[0].Address.String()].GenerateVRFProof(message)

// block for height 2
block, _ := state.MakeBlock(2, makeTxs(2), lastCommit, nil, state.Validators.GetProposer().Address, 0, proof)
block, _ := state.MakeBlock(2, makeTxs(2), lastCommit, nil, state.Validators.Validators[0].Address, 0, proof)

_, err = sm.ExecCommitBlock(proxyApp.Consensus(), block, log.TestingLogger(), stateDB)
require.Nil(t, err, tc.desc)
Expand Down Expand Up @@ -160,8 +160,8 @@ func TestBeginBlockByzantineValidators(t *testing.T) {
lastCommit := types.NewCommit(9, 0, prevBlockID, commitSigs)
for _, tc := range testCases {
message := state.MakeHashMessage(0)
proof, _ := privVals[state.Validators.GetProposer().Address.String()].GenerateVRFProof(message)
block, _ := state.MakeBlock(10, makeTxs(2), lastCommit, nil, state.Validators.GetProposer().Address, 0, proof)
proof, _ := privVals[state.Validators.Validators[0].Address.String()].GenerateVRFProof(message)
block, _ := state.MakeBlock(10, makeTxs(2), lastCommit, nil, state.Validators.Validators[0].Address, 0, proof)
block.Time = now
block.Evidence.Evidence = tc.evidence
_, err = sm.ExecCommitBlock(proxyApp.Consensus(), block, log.TestingLogger(), stateDB)
Expand Down Expand Up @@ -351,7 +351,7 @@ func TestEndBlockValidatorUpdates(t *testing.T) {
)
require.NoError(t, err)

block := makeBlockWithPrivVal(state, privVals[state.Validators.Proposer.Address.String()], 1)
block := makeBlockWithPrivVal(state, privVals[state.Validators.Validators[0].Address.String()], 1)
blockID := types.BlockID{Hash: block.Hash(), PartsHeader: block.MakePartSet(testPartSize).Header()}

pubkey := ed25519.GenPrivKey().PubKey()
Expand Down
7 changes: 4 additions & 3 deletions state/helpers_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,8 +45,9 @@ func makeAndCommitGoodBlock(
privVals map[string]types.PrivValidator,
evidence []types.Evidence) (sm.State, types.BlockID, *types.Commit, error) {
// A good block passes
state, blockID, err := makeAndApplyGoodBlock(state, privVals[state.Validators.Proposer.Address.String()], height,
lastCommit, proposerAddr, blockExec, evidence)
state, blockID, err := makeAndApplyGoodBlock(state,
privVals[types.SelectProposer(state.Validators, state.LastProofHash, height, 0).Address.String()],
height, lastCommit, proposerAddr, blockExec, evidence)
if err != nil {
return state, types.BlockID{}, nil, err
}
Expand Down Expand Up @@ -151,7 +152,7 @@ func makeBlockWithPrivVal(state sm.State, privVal types.PrivValidator, height in
makeTxs(state.LastBlockHeight),
new(types.Commit),
nil,
state.Validators.GetProposer().Address,
privVal.GetPubKey().Address(),
0,
proof,
)
Expand Down
6 changes: 3 additions & 3 deletions state/state.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -240,8 +240,8 @@ func MakeGenesisState(genDoc *types.GenesisDoc) (State, error) {
for i, val := range genDoc.Validators {
validators[i] = types.NewValidator(val.PubKey, val.Power)
}
validatorSet = types.NewRandomValidatorSet(validators, types.MakeRoundHash(genDoc.Hash(), 1, 0))
nextValidatorSet = types.NewRandomValidatorSet(validators, types.MakeRoundHash(genDoc.Hash(), 2, 0))
validatorSet = types.NewValidatorSet(validators)
nextValidatorSet = types.NewValidatorSet(validators)
}

return State{
Expand All @@ -257,7 +257,7 @@ func MakeGenesisState(genDoc *types.GenesisDoc) (State, error) {

NextValidators: nextValidatorSet,
Validators: validatorSet,
LastValidators: types.NewRandomValidatorSet(nil, types.MakeRoundHash(genDoc.Hash(), 1, 0)),
LastValidators: types.NewValidatorSet(nil),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why don't you add Proposer to the State of the state module, such as the State of the consensus module?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can add LastProposer to State if we need it, but for now, we don't need it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, I understand. Thank you. :)

LastHeightValidatorsChanged: 1,

ConsensusParams: *genDoc.ConsensusParams,
Expand Down
Loading