Simplify proposal msg (#2735)

* Align Proposal message with spec

* Update spec

CHANGELOG_PENDING.md

@@ -108,6 +108,7 @@ Friendly reminder, we have a [bug bounty program](https://hackerone.com/tendermi
 - [consensus] [\#2642](https://github.com/tendermint/tendermint/issues/2642) Initialized ValidRound and LockedRound to -1
 - [consensus] [\#1637](https://github.com/tendermint/tendermint/issues/1637) Limit the amount of evidence that can be included in a
   block
+- [consensus] [\#2646](https://github.com/tendermint/tendermint/issues/2646) Simplify Proposal message (align with spec)
 - [evidence] [\#2515](https://github.com/tendermint/tendermint/issues/2515) Fix db iter leak (@goolAdapter)
 - [libs/event] [\#2518](https://github.com/tendermint/tendermint/issues/2518) Fix event concurrency flaw (@goolAdapter)
 - [node] [\#2434](https://github.com/tendermint/tendermint/issues/2434) Make node respond to signal interrupts while sleeping for genesis time

consensus/byzantine_test.go

@@ -179,16 +179,16 @@ func byzantineDecideProposalFunc(t *testing.T, height int64, round int, cs *Cons
 
 	// Create a new proposal block from state/txs from the mempool.
 	block1, blockParts1 := cs.createProposalBlock()
-	polRound, polBlockID := cs.Votes.POLInfo()
-	proposal1 := types.NewProposal(height, round, blockParts1.Header(), polRound, polBlockID)
+	polRound, propBlockID := cs.ValidRound, types.BlockID{block1.Hash(), blockParts1.Header()}
+	proposal1 := types.NewProposal(height, round, polRound, propBlockID)
 	if err := cs.privValidator.SignProposal(cs.state.ChainID, proposal1); err != nil {
 		t.Error(err)
 	}
 
 	// Create a new proposal block from state/txs from the mempool.
 	block2, blockParts2 := cs.createProposalBlock()
-	polRound, polBlockID = cs.Votes.POLInfo()
-	proposal2 := types.NewProposal(height, round, blockParts2.Header(), polRound, polBlockID)
+	polRound, propBlockID = cs.ValidRound, types.BlockID{block2.Hash(), blockParts2.Header()}
+	proposal2 := types.NewProposal(height, round, polRound, propBlockID)
 	if err := cs.privValidator.SignProposal(cs.state.ChainID, proposal2); err != nil {
 		t.Error(err)
 	}

consensus/common_test.go

@@ -130,8 +130,8 @@ func decideProposal(cs1 *ConsensusState, vs *validatorStub, height int64, round
 	}
 
 	// Make proposal
-	polRound, polBlockID := cs1.Votes.POLInfo()
-	proposal = types.NewProposal(height, round, blockParts.Header(), polRound, polBlockID)
+	polRound, propBlockID := cs1.ValidRound, types.BlockID{block.Hash(), blockParts.Header()}
+	proposal = types.NewProposal(height, round, polRound, propBlockID)
 	if err := vs.SignProposal(cs1.state.ChainID, proposal); err != nil {
 		panic(err)
 	}

consensus/reactor.go

@@ -976,8 +976,8 @@ func (ps *PeerState) SetHasProposal(proposal *types.Proposal) {
 		return
 	}
 
-	ps.PRS.ProposalBlockPartsHeader = proposal.BlockPartsHeader
-	ps.PRS.ProposalBlockParts = cmn.NewBitArray(proposal.BlockPartsHeader.Total)
+	ps.PRS.ProposalBlockPartsHeader = proposal.BlockID.PartsHeader
+	ps.PRS.ProposalBlockParts = cmn.NewBitArray(proposal.BlockID.PartsHeader.Total)
 	ps.PRS.ProposalPOLRound = proposal.POLRound
 	ps.PRS.ProposalPOL = nil // Nil until ProposalPOLMessage received.
 }

consensus/replay.go

@@ -73,7 +73,7 @@ func (cs *ConsensusState) readReplayMessage(msg *TimedWALMessage, newStepCh chan
 		case *ProposalMessage:
 			p := msg.Proposal
 			cs.Logger.Info("Replay: Proposal", "height", p.Height, "round", p.Round, "header",
-				p.BlockPartsHeader, "pol", p.POLRound, "peer", peerID)
+				p.BlockID.PartsHeader, "pol", p.POLRound, "peer", peerID)
 		case *BlockPartMessage:
 			cs.Logger.Info("Replay: BlockPart", "height", msg.Height, "round", msg.Round, "peer", peerID)
 		case *VoteMessage:

consensus/replay_test.go

@@ -575,7 +575,7 @@ func readPieceFromWAL(msg *TimedWALMessage) interface{} {
 	case msgInfo:
 		switch msg := m.Msg.(type) {
 		case *ProposalMessage:
-			return &msg.Proposal.BlockPartsHeader
+			return &msg.Proposal.BlockID.PartsHeader
 		case *BlockPartMessage:
 			return msg.Part
 		case *VoteMessage:

consensus/state.go

@@ -901,9 +901,10 @@ func (cs *ConsensusState) defaultDecideProposal(height int64, round int) {
 	}
 
 	// Make proposal
-	polRound, polBlockID := cs.Votes.POLInfo()
-	proposal := types.NewProposal(height, round, blockParts.Header(), polRound, polBlockID)
+	propBlockId := types.BlockID{block.Hash(), blockParts.Header()}
+	proposal := types.NewProposal(height, round, cs.ValidRound, propBlockId)
 	if err := cs.privValidator.SignProposal(cs.state.ChainID, proposal); err == nil {
+
 		// send proposal and block parts on internal msg queue
 		cs.sendInternalMessage(msgInfo{&ProposalMessage{proposal}, ""})
 		for i := 0; i < blockParts.Total(); i++ {
@@ -1423,7 +1424,7 @@ func (cs *ConsensusState) defaultSetProposal(proposal *types.Proposal) error {
 	// This happens if we're already in cstypes.RoundStepCommit or if there is a valid block in the current round.
 	// TODO: We can check if Proposal is for a different block as this is a sign of misbehavior!
 	if cs.ProposalBlockParts == nil {
-		cs.ProposalBlockParts = types.NewPartSetFromHeader(proposal.BlockPartsHeader)
+		cs.ProposalBlockParts = types.NewPartSetFromHeader(proposal.BlockID.PartsHeader)
 	}
 	cs.Logger.Info("Received proposal", "proposal", proposal)
 	return nil

consensus/state_test.go

@@ -197,7 +197,9 @@ func TestStateBadProposal(t *testing.T) {
 	stateHash[0] = byte((stateHash[0] + 1) % 255)
 	propBlock.AppHash = stateHash
 	propBlockParts := propBlock.MakePartSet(partSize)
-	proposal := types.NewProposal(vs2.Height, round, propBlockParts.Header(), -1, types.BlockID{})
+	proposal := types.NewProposal(
+		vs2.Height, round, -1,
+		types.BlockID{propBlock.Hash(), propBlockParts.Header()})
 	if err := vs2.SignProposal(config.ChainID(), proposal); err != nil {
 		t.Fatal("failed to sign bad proposal", err)
 	}
@@ -811,6 +813,7 @@ func TestStateLockPOLSafety2(t *testing.T) {
 	_, propBlock0 := decideProposal(cs1, vss[0], height, round)
 	propBlockHash0 := propBlock0.Hash()
 	propBlockParts0 := propBlock0.MakePartSet(partSize)
+	propBlockID0 := types.BlockID{propBlockHash0, propBlockParts0.Header()}
 
 	// the others sign a polka but we don't see it
 	prevotes := signVotes(types.PrevoteType, propBlockHash0, propBlockParts0.Header(), vs2, vs3, vs4)
@@ -819,7 +822,6 @@ func TestStateLockPOLSafety2(t *testing.T) {
 	prop1, propBlock1 := decideProposal(cs1, vs2, vs2.Height, vs2.Round+1)
 	propBlockHash1 := propBlock1.Hash()
 	propBlockParts1 := propBlock1.MakePartSet(partSize)
-	propBlockID1 := types.BlockID{propBlockHash1, propBlockParts1.Header()}
 
 	incrementRound(vs2, vs3, vs4)
 
@@ -854,7 +856,7 @@ func TestStateLockPOLSafety2(t *testing.T) {
 
 	round = round + 1 // moving to the next round
 	// in round 2 we see the polkad block from round 0
-	newProp := types.NewProposal(height, round, propBlockParts0.Header(), 0, propBlockID1)
+	newProp := types.NewProposal(height, round, 0, propBlockID0)
 	if err := vs3.SignProposal(config.ChainID(), newProp); err != nil {
 		t.Fatal(err)
 	}
@@ -909,7 +911,7 @@ func TestProposeValidBlock(t *testing.T) {
 	ensurePrevote(voteCh, height, round)
 	validatePrevote(t, cs1, round, vss[0], propBlockHash)
 
-	// the others sign a polka but we don't see it
+	// the others sign a polka
 	signAddVotes(cs1, types.PrevoteType, propBlockHash, propBlock.MakePartSet(partSize).Header(), vs2, vs3, vs4)
 
 	ensurePrecommit(voteCh, height, round)
@@ -964,6 +966,8 @@ func TestProposeValidBlock(t *testing.T) {
 	rs = cs1.GetRoundState()
 	assert.True(t, bytes.Equal(rs.ProposalBlock.Hash(), propBlockHash))
 	assert.True(t, bytes.Equal(rs.ProposalBlock.Hash(), rs.ValidBlock.Hash()))
+	assert.True(t, rs.Proposal.POLRound == rs.ValidRound)
+	assert.True(t, bytes.Equal(rs.Proposal.BlockID.Hash, rs.ValidBlock.Hash()))
 }
 
 // What we want:

consensus/types/round_state_test.go

@@ -63,11 +63,8 @@ func BenchmarkRoundStateDeepCopy(b *testing.B) {
 	// Random Proposal
 	proposal := &types.Proposal{
 		Timestamp: tmtime.Now(),
-		BlockPartsHeader: types.PartSetHeader{
-			Hash: cmn.RandBytes(20),
-		},
-		POLBlockID: blockID,
-		Signature:  sig,
+		BlockID:   blockID,
+		Signature: sig,
 	}
 	// Random HeightVoteSet
 	// TODO: hvs :=

docs/spec/reactors/consensus/consensus.md

@@ -47,25 +47,21 @@ type ProposalMessage struct {
 ### Proposal
 
 Proposal contains height and round for which this proposal is made, BlockID as a unique identifier
-of proposed block, timestamp, and two fields (POLRound and POLBlockID) that are needed for
-termination of the consensus. The message is signed by the validator private key.
+of proposed block, timestamp, and POLRound (a so-called Proof-of-Lock (POL) round) that is needed for
+termination of the consensus. If POLRound >= 0, then BlockID corresponds to the block that 
+is locked in POLRound. The message is signed by the validator private key.
 
 ```go
 type Proposal struct {
     Height           int64
     Round            int
-    Timestamp        Time
-    BlockID          BlockID
     POLRound         int
-    POLBlockID       BlockID
+    BlockID          BlockID
+    Timestamp        Time
     Signature        Signature
 }
 ```
 
-NOTE: In the current version of the Tendermint, the consensus value in proposal is represented with
-PartSetHeader, and with BlockID in vote message. It should be aligned as suggested in this spec as
-BlockID contains PartSetHeader.
-
 ## VoteMessage
 
 VoteMessage is sent to vote for some block (or to inform others that a process does not vote in the

privval/priv_validator_test.go

@@ -140,8 +140,8 @@ func TestSignProposal(t *testing.T) {
 	require.Nil(t, err)
 	privVal := GenFilePV(tempFile.Name())
 
-	block1 := types.PartSetHeader{5, []byte{1, 2, 3}}
-	block2 := types.PartSetHeader{10, []byte{3, 2, 1}}
+	block1 := types.BlockID{[]byte{1, 2, 3}, types.PartSetHeader{5, []byte{1, 2, 3}}}
+	block2 := types.BlockID{[]byte{3, 2, 1}, types.PartSetHeader{10, []byte{3, 2, 1}}}
 	height, round := int64(10), 1
 
 	// sign a proposal for first time
@@ -179,7 +179,7 @@ func TestDifferByTimestamp(t *testing.T) {
 	require.Nil(t, err)
 	privVal := GenFilePV(tempFile.Name())
 
-	block1 := types.PartSetHeader{5, []byte{1, 2, 3}}
+	block1 := types.BlockID{[]byte{1, 2, 3}, types.PartSetHeader{5, []byte{1, 2, 3}}}
 	height, round := int64(10), 1
 	chainID := "mychainid"
 
@@ -241,11 +241,11 @@ func newVote(addr types.Address, idx int, height int64, round int, typ byte, blo
 	}
 }
 
-func newProposal(height int64, round int, partsHeader types.PartSetHeader) *types.Proposal {
+func newProposal(height int64, round int, blockID types.BlockID) *types.Proposal {
 	return &types.Proposal{
-		Height:           height,
-		Round:            round,
-		BlockPartsHeader: partsHeader,
-		Timestamp:        tmtime.Now(),
+		Height:    height,
+		Round:     round,
+		BlockID:   blockID,
+		Timestamp: tmtime.Now(),
 	}
 }

types/canonical.go

@@ -23,14 +23,13 @@ type CanonicalPartSetHeader struct {
 }
 
 type CanonicalProposal struct {
-	Type             SignedMsgType // type alias for byte
-	Height           int64         `binary:"fixed64"`
-	Round            int64         `binary:"fixed64"`
-	POLRound         int64         `binary:"fixed64"`
-	Timestamp        time.Time
-	BlockPartsHeader CanonicalPartSetHeader
-	POLBlockID       CanonicalBlockID
-	ChainID          string
+	Type      SignedMsgType // type alias for byte
+	Height    int64         `binary:"fixed64"`
+	Round     int64         `binary:"fixed64"`
+	POLRound  int64         `binary:"fixed64"`
+	BlockID   CanonicalBlockID
+	Timestamp time.Time
+	ChainID   string
 }
 
 type CanonicalVote struct {
@@ -71,14 +70,13 @@ func CanonicalizePartSetHeader(psh PartSetHeader) CanonicalPartSetHeader {
 
 func CanonicalizeProposal(chainID string, proposal *Proposal) CanonicalProposal {
 	return CanonicalProposal{
-		Type:             ProposalType,
-		Height:           proposal.Height,
-		Round:            int64(proposal.Round), // cast int->int64 to make amino encode it fixed64 (does not work for int)
-		POLRound:         int64(proposal.POLRound),
-		Timestamp:        proposal.Timestamp,
-		BlockPartsHeader: CanonicalizePartSetHeader(proposal.BlockPartsHeader),
-		POLBlockID:       CanonicalizeBlockID(proposal.POLBlockID),
-		ChainID:          chainID,
+		Type:      ProposalType,
+		Height:    proposal.Height,
+		Round:     int64(proposal.Round), // cast int->int64 to make amino encode it fixed64 (does not work for int)
+		POLRound:  int64(proposal.POLRound),
+		BlockID:   CanonicalizeBlockID(proposal.BlockID),
+		Timestamp: proposal.Timestamp,
+		ChainID:   chainID,
 	}
 }
 

types/proposal.go

@@ -15,43 +15,41 @@ var (
 )
 
 // Proposal defines a block proposal for the consensus.
-// It refers to the block only by its PartSetHeader.
+// It refers to the block by BlockID field.
 // It must be signed by the correct proposer for the given Height/Round
 // to be considered valid. It may depend on votes from a previous round,
-// a so-called Proof-of-Lock (POL) round, as noted in the POLRound and POLBlockID.
+// a so-called Proof-of-Lock (POL) round, as noted in the POLRound.
+// If POLRound >= 0, then BlockID corresponds to the block that is locked in POLRound.
 type Proposal struct {
-	Type             SignedMsgType
-	Height           int64         `json:"height"`
-	Round            int           `json:"round"`
-	POLRound         int           `json:"pol_round"` // -1 if null.
-	Timestamp        time.Time     `json:"timestamp"`
-	BlockPartsHeader PartSetHeader `json:"block_parts_header"`
-	POLBlockID       BlockID       `json:"pol_block_id"` // zero if null.
-	Signature        []byte        `json:"signature"`
+	Type      SignedMsgType
+	Height    int64     `json:"height"`
+	Round     int       `json:"round"`
+	POLRound  int       `json:"pol_round"` // -1 if null.
+	BlockID   BlockID   `json:"block_id"`
+	Timestamp time.Time `json:"timestamp"`
+	Signature []byte    `json:"signature"`
 }
 
 // NewProposal returns a new Proposal.
 // If there is no POLRound, polRound should be -1.
-func NewProposal(height int64, round int, blockPartsHeader PartSetHeader, polRound int, polBlockID BlockID) *Proposal {
+func NewProposal(height int64, round int, polRound int, blockID BlockID) *Proposal {
 	return &Proposal{
-		Type:             ProposalType,
-		Height:           height,
-		Round:            round,
-		POLRound:         polRound,
-		Timestamp:        tmtime.Now(),
-		BlockPartsHeader: blockPartsHeader,
-		POLBlockID:       polBlockID,
+		Type:      ProposalType,
+		Height:    height,
+		Round:     round,
+		BlockID:   blockID,
+		POLRound:  polRound,
+		Timestamp: tmtime.Now(),
 	}
 }
 
 // String returns a string representation of the Proposal.
 func (p *Proposal) String() string {
-	return fmt.Sprintf("Proposal{%v/%v %v (%v,%v) %X @ %s}",
+	return fmt.Sprintf("Proposal{%v/%v (%v, %v) %X @ %s}",
 		p.Height,
 		p.Round,
-		p.BlockPartsHeader,
+		p.BlockID,
 		p.POLRound,
-		p.POLBlockID,
 		cmn.Fingerprint(p.Signature),
 		CanonicalTime(p.Timestamp))
 }

types/proposal_test.go

@@ -15,11 +15,11 @@ func init() {
 		panic(err)
 	}
 	testProposal = &Proposal{
-		Height:           12345,
-		Round:            23456,
-		BlockPartsHeader: PartSetHeader{111, []byte("blockparts")},
-		POLRound:         -1,
-		Timestamp:        stamp,
+		Height:    12345,
+		Round:     23456,
+		BlockID:   BlockID{[]byte{1, 2, 3}, PartSetHeader{111, []byte("blockparts")}},
+		POLRound:  -1,
+		Timestamp: stamp,
 	}
 }
 
@@ -34,7 +34,7 @@ func TestProposalSignable(t *testing.T) {
 
 func TestProposalString(t *testing.T) {
 	str := testProposal.String()
-	expected := `Proposal{12345/23456 111:626C6F636B70 (-1,:0:000000000000) 000000000000 @ 2018-02-11T07:09:22.765Z}`
+	expected := `Proposal{12345/23456 (010203:111:626C6F636B70, -1) 000000000000 @ 2018-02-11T07:09:22.765Z}`
 	if str != expected {
 		t.Errorf("Got unexpected string for Proposal. Expected:\n%v\nGot:\n%v", expected, str)
 	}
@@ -44,7 +44,9 @@ func TestProposalVerifySignature(t *testing.T) {
 	privVal := NewMockPV()
 	pubKey := privVal.GetPubKey()
 
-	prop := NewProposal(4, 2, PartSetHeader{777, []byte("proper")}, 2, BlockID{})
+	prop := NewProposal(
+		4, 2, 2,
+		BlockID{[]byte{1, 2, 3}, PartSetHeader{777, []byte("proper")}})
 	signBytes := prop.SignBytes("test_chain_id")
 
 	// sign it