Spec of consensus/gossip protocol by milosevic · Pull Request #1039 · tendermint/tendermint

milosevic · 2017-12-29T21:17:33Z

No description provided.

melekes · 2017-12-29T23:01:01Z

docs/specification/new-spec/consensus.md

+
+### Heartbeat
+Heartbeat contains validator information (address and index),
+height, round and sequence number (?). It is signed by the private key of the validator. 


I was wondering what is the sequence field, but it's probably as it says sequence number :)

ebuchman

Great start! Thanks Zarko.

ebuchman · 2017-12-30T01:25:36Z

docs/specification/new-spec/consensus.md

+executed steps of the core consensus algorithm (`NewRoundStepMessage` and `CommitStepMessage`), and 
+also messages that inform peers what votes the process has seen (`HasVoteMessage`,
+`VoteSetMaj23Message` and `VoteSetBitsMessage`).
+


These opening paragraphs are wonderful <3

Though I might add a note about only proposal/vote being signed, and about how the gossip msgs are used to determine how we send data to other peers

ebuchman · 2017-12-30T01:27:03Z

docs/specification/new-spec/consensus.md

+next block in the blockchain should be.
+```
+type ProposalMessage struct {
+	Proposal *types.Proposal


let's drop all pointers and package names.

The goal with the spec is to lay out the data structures in a way that makes their encoding clear. We don't want the fact that it's a pointer to change the encoding, so it shouldn't be relevant (currently it does matter, but we plan to fix that!)

ebuchman · 2017-12-30T01:27:51Z

docs/specification/new-spec/consensus.md

+type Proposal struct {
+	Height           int64            
+	Round            int              
+	Timestamp        time.Time        


int64 (thats what its encoded as)

ebuchman · 2017-12-30T01:29:28Z

docs/specification/new-spec/consensus.md

+	BlockID          BlockID      
+	POLRound         int              
+	POLBlockID       BlockID          
+	Signature        crypto.Signature 


Let's drop the pkg name prefixes. This spec is defining the data structures, not referencing the code, and it all lives in one namespace. I defined Signature in blockchain.md

ebuchman · 2017-12-30T01:29:44Z

docs/specification/new-spec/consensus.md

+
+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.   


ebuchman · 2017-12-30T01:31:56Z

docs/specification/new-spec/consensus.md

+
+```
+type Heartbeat struct {
+	ValidatorAddress data.Bytes       


ebuchman · 2017-12-30T01:32:42Z

docs/specification/new-spec/consensus.md

+type NewRoundStepMessage struct {
+	Height                int64
+	Round                 int
+	Step                  cstypes.RoundStepType


lets be clear about the basic type here

What do you mean? Should we define RoundStepType?It's somehow implementation detail and can change, so I am not sure if it should be part of spec.

Let's just give a concrete type - int8

You mean we shouldn't use int type? Because we use it in Vote so I thought that's fine.

ebuchman · 2017-12-30T01:33:26Z

docs/specification/new-spec/consensus.md

+```
+type CommitStepMessage struct {
+	Height           int64
+	BlockPartsHeader types.PartSetHeader


lets make it BlockID ?

ebuchman · 2017-12-30T01:33:39Z

docs/specification/new-spec/consensus.md

+type CommitStepMessage struct {
+	Height           int64
+	BlockPartsHeader types.PartSetHeader
+	BlockParts       *cmn.BitArray


Mmm - we need to define BitArray and how it's encoded!

ebuchman · 2017-12-30T01:33:47Z

docs/specification/new-spec/consensus.md

+}
+```
+
+TODO: Maybe use BlockID instead of BlockPartsHeader for symmetry. 


milosevic · 2017-12-30T20:21:23Z

I have tried to address your comments. The encoding part needs to be improved (BitArray and Part), but it's probably easier for someone else to define it (it could be done in different PR). Please take a look if new version is fine so we can merge it @ebuchman @melekes .

codecov-io · 2017-12-30T20:43:59Z

Codecov Report

Merging #1039 into develop will increase coverage by 0.09%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           develop    #1039      +/-   ##
===========================================
+ Coverage    59.66%   59.75%   +0.09%     
===========================================
  Files          116      116              
  Lines        10667    10667              
===========================================
+ Hits          6364     6374      +10     
+ Misses        3725     3719       -6     
+ Partials       578      574       -4

adrianbrink · 2017-12-31T10:56:38Z

docs/specification/new-spec/consensus.md

+# Tendermint Consensus
+
+Tendermint consensus is a distributed protocol executed by validator processes to agree on
+the next block to be added to the Tendermint blockchain. The protocol proceeds in rounds, where 


... to be added to a blockchain.

Theoretically it could be any blockchain. For example, Polkdadot might use Tendermint consensus.

Also, we should be consistent with naming and always call it Tendermint Consensus.

adrianbrink · 2017-12-31T10:57:44Z

docs/specification/new-spec/consensus.md

+the `ProposalMessage`.
+The processes respond by voting for a block with `VoteMessage` (there are two kinds of vote messages, prevote 
+and precommit votes). Note that a proposal message is just a suggestion what the next block should be; a 
+validator might vote with a `VoteMessage` for a different block. If in some round, enough number 


... in some round enough processes vote for the same block, ...

adrianbrink · 2017-12-31T10:58:16Z

docs/specification/new-spec/consensus.md

+and precommit votes). Note that a proposal message is just a suggestion what the next block should be; a 
+validator might vote with a `VoteMessage` for a different block. If in some round, enough number 
+of processes vote for the same block, then this block is committed and later added to the blockchain. 
+`ProposalMessage` and `VoteMessage` are signed by the private key of the validator.


... by the private key of each process. We haven't explained validator before.

milosevic requested review from ebuchman and melekes as code owners December 29, 2017 21:17

melekes reviewed Dec 29, 2017

View reviewed changes

melekes approved these changes Dec 29, 2017

View reviewed changes

ebuchman reviewed Dec 30, 2017

View reviewed changes

Describe messages sent as part of consensus/gossip protocol

96e0e4a

milosevic force-pushed the add_consensus_spec branch from f11f979 to 96e0e4a Compare December 30, 2017 20:18

milosevic changed the title ~~WIP: Spec of consensus/gossip protocol~~ Spec of consensus/gossip protocol Dec 30, 2017

adrianbrink reviewed Dec 31, 2017

View reviewed changes

ebuchman merged commit 008de93 into develop Dec 31, 2017

ebuchman deleted the add_consensus_spec branch December 31, 2017 19:59

Conversation

milosevic commented Dec 29, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ebuchman left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

milosevic commented Dec 30, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov-io commented Dec 30, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

milosevic commented Dec 30, 2017 •

edited

Loading

codecov-io commented Dec 30, 2017 •

edited

Loading