fixes for ProposerAddress

- state.MakeBlock takes a proposerAddr
- validateBlock only checks that the ProposerAddress is in the validator
  set
- fix raceyness from bad proposer test:
  - use privValidator to get the proposer address (instead of racy
    state)
  - note we had to remove the test that checked the correct proposer was
    included for higher rounds because we don't have a good way to test
    this with multiple consensus states and not using the
    privValidator.Address while calling createProposalBlock was a hack!

blockchain/reactor_test.go

@@ -156,7 +156,7 @@ func makeTxs(height int64) (txs []types.Tx) {
 }
 
 func makeBlock(height int64, state sm.State) *types.Block {
-	block, _ := state.MakeBlock(height, makeTxs(height), new(types.Commit), nil)
+	block, _ := state.MakeBlock(height, makeTxs(height), new(types.Commit), nil, state.Validators.GetProposer().Address)
 	return block
 }
 

consensus/state.go

@@ -949,7 +949,8 @@ func (cs *ConsensusState) createProposalBlock() (block *types.Block, blockParts
 	// Mempool validated transactions
 	txs := cs.mempool.Reap(cs.state.ConsensusParams.BlockSize.MaxTxs)
 	evidence := cs.evpool.PendingEvidence()
-	block, parts := cs.state.MakeBlock(cs.Height, txs, commit, evidence)
+	proposerAddr := cs.privValidator.GetAddress()
+	block, parts := cs.state.MakeBlock(cs.Height, txs, commit, evidence, proposerAddr)
 	return block, parts
 }
 

consensus/state_test.go

@@ -108,10 +108,6 @@ func TestStateProposerSelection2(t *testing.T) {
 		if !bytes.Equal(prop.Address, correctProposer) {
 			panic(cmn.Fmt("expected RoundState.Validators.GetProposer() to be validator %d. Got %X", (i+2)%len(vss), prop.Address))
 		}
-		block, _ := cs1.createProposalBlock()
-		if !bytes.Equal(block.ProposerAddress, correctProposer) {
-			panic(cmn.Fmt("expected block.ProposerAddress to be validator %d. Got %X", (i+2)%len(vss), block.ProposerAddress))
-		}
 
 		rs := cs1.GetRoundState()
 		signAddVotes(cs1, types.VoteTypePrecommit, nil, rs.ProposalBlockParts.Header(), vss[1:]...)

state/execution_test.go

@@ -79,7 +79,7 @@ func TestBeginBlockValidators(t *testing.T) {
 		lastCommit := &types.Commit{BlockID: prevBlockID, Precommits: tc.lastCommitPrecommits}
 
 		// block for height 2
-		block, _ := state.MakeBlock(2, makeTxs(2), lastCommit, nil)
+		block, _ := state.MakeBlock(2, makeTxs(2), lastCommit, nil, state.Validators.GetProposer().Address)
 		_, err = ExecCommitBlock(proxyApp.Consensus(), block, log.TestingLogger(), state.Validators, stateDB)
 		require.Nil(t, err, tc.desc)
 
@@ -138,7 +138,7 @@ func TestBeginBlockByzantineValidators(t *testing.T) {
 	lastCommit := &types.Commit{BlockID: prevBlockID, Precommits: votes}
 	for _, tc := range testCases {
 
-		block, _ := state.MakeBlock(10, makeTxs(2), lastCommit, nil)
+		block, _ := state.MakeBlock(10, makeTxs(2), lastCommit, nil, state.Validators.GetProposer().Address)
 		block.Time = now
 		block.Evidence.Evidence = tc.evidence
 		_, err = ExecCommitBlock(proxyApp.Consensus(), block, log.TestingLogger(), state.Validators, stateDB)
@@ -269,7 +269,7 @@ func state(nVals, height int) (State, dbm.DB) {
 }
 
 func makeBlock(state State, height int64) *types.Block {
-	block, _ := state.MakeBlock(height, makeTxs(state.LastBlockHeight), new(types.Commit), nil)
+	block, _ := state.MakeBlock(height, makeTxs(state.LastBlockHeight), new(types.Commit), nil, state.Validators.GetProposer().Address)
 	return block
 }
 

state/state.go

@@ -99,12 +99,14 @@ func (state State) IsEmpty() bool {
 // Create a block from the latest state
 
 // MakeBlock builds a block from the current state with the given txs, commit,
-// and evidence.
+// and evidence. Note it also takes a proposerAddress because the state does not
+// track rounds, and hence doesn't know the correct proposer. TODO: alleviate this!
 func (state State) MakeBlock(
 	height int64,
 	txs []types.Tx,
 	commit *types.Commit,
 	evidence []types.Evidence,
+	proposerAddress []byte,
 ) (*types.Block, *types.PartSet) {
 
 	// Build base block with block data.
@@ -122,7 +124,9 @@ func (state State) MakeBlock(
 	block.AppHash = state.AppHash
 	block.LastResultsHash = state.LastResultsHash
 
-	block.ProposerAddress = state.Validators.GetProposer().Address
+	// NOTE: we can't use the state.Validators because we don't
+	// IncrementAccum for rounds there.
+	block.ProposerAddress = proposerAddress
 
 	return block, block.MakePartSet(state.ConsensusParams.BlockGossip.BlockPartSizeBytes)
 }

state/validation.go

@@ -5,6 +5,7 @@ import (
 	"errors"
 	"fmt"
 
+	"github.com/tendermint/go-crypto/tmhash"
 	dbm "github.com/tendermint/tendermint/libs/db"
 	"github.com/tendermint/tendermint/types"
 )
@@ -121,10 +122,13 @@ func validateBlock(stateDB dbm.DB, state State, block *types.Block) error {
 		}
 	}
 
-	if !bytes.Equal(block.ProposerAddress, state.Validators.GetProposer().Address) {
+	// NOTE: We can't actually verify it's the right proposer because we dont
+	// know what round the block was first proposed. So just check that it's
+	// a legit address and a known validator.
+	if len(block.ProposerAddress) != tmhash.Size ||
+		!state.Validators.HasAddress(block.ProposerAddress) {
 		return fmt.Errorf(
-			"Wrong Block.Header.ProposerAddress.  Expected %X, got %v",
-			state.Validators.GetProposer().Address,
+			"Block.Header.ProposerAddress, %X, is not a validator",
 			block.ProposerAddress,
 		)
 	}

state/validation_test.go

@@ -72,4 +72,7 @@ func TestValidateBlock(t *testing.T) {
 	block.ProposerAddress = ed25519.GenPrivKey().PubKey().Address()
 	err = blockExec.ValidateBlock(state, block)
 	require.Error(t, err)
+	block.ProposerAddress = []byte("wrong size")
+	err = blockExec.ValidateBlock(state, block)
+	require.Error(t, err)
 }