adr-025-commit.md

ADR 025 Commit

Context

Currently the Commit structure contains a lot of potentially redundant or unnecessary data. It contains a list of precommits from every validator, where the precommit includes the whole Vote structure. Thus each of the commit height, round, type, and blockID are repeated for every validator, and could be deduplicated, leading to very significant savings in block size.

type Commit struct {
    BlockID    BlockID `json:"block_id"`
    Precommits []*Vote `json:"precommits"`
}

type Vote struct {
    ValidatorAddress Address   `json:"validator_address"`
    ValidatorIndex   int       `json:"validator_index"`
    Height           int64     `json:"height"`
    Round            int       `json:"round"`
    Timestamp        time.Time `json:"timestamp"`
    Type             byte      `json:"type"`
    BlockID          BlockID   `json:"block_id"`
    Signature        []byte    `json:"signature"`
}

The original tracking issue for this is #1648. We have discussed replacing the Vote type in Commit with a new CommitSig type, which includes at minimum the vote signature. The Vote type will continue to be used in the consensus reactor and elsewhere.

A primary question is what should be included in the CommitSig beyond the signature. One current constraint is that we must include a timestamp, since this is how we calculuate BFT time, though we may be able to change this in the future.

Other concerns here include:

• Validator Address #3596 - Should the CommitSig include the validator address? It is very convenient to do so, but likely not necessary. This was also discussed in #2226.
• Absent Votes #3591 - How to represent absent votes? Currently they are just present as nil in the Precommits list, which is actually problematic for serialization
• Other BlockIDs #3485 - How to represent votes for nil and for other block IDs? We currently allow votes for nil and votes for alternative block ids, but just ignore them

Decision

Deduplicate the fields and introduce CommitSig:

type Commit struct {
    Height  int64
    Round   int
    BlockID    BlockID      `json:"block_id"`
    Precommits []CommitSig `json:"precommits"`
}

type CommitSig struct {
    BlockID  BlockIDFlag
    ValidatorAddress Address
    Timestamp time.Time
    Signature []byte
}


// indicate which BlockID the signature is for
type BlockIDFlag int

const (
	BlockIDFlagAbsent BlockIDFlag = iota // vote is not included in the Commit.Precommits
	BlockIDFlagCommit                    // voted for the Commit.BlockID
	BlockIDFlagNil                       // voted for nil
)

Re the concerns outlined in the context:

Timestamp: Leave the timestamp for now. Removing it and switching to proposer based time will take more analysis and work, and will be left for a future breaking change. In the meantime, the concerns with the current approach to BFT time can be mitigated.

ValidatorAddress: we include it in the CommitSig for now. While this does increase the block size unecessarily (20-bytes per validator), it has some ergonomic and debugging advantages:

• Commit contains everything necessary to reconstruct []Vote, and doesn't depend on additional access to a ValidatorSet
• Lite clients can check if they know the validators in a commit without re-downloading the validator set
• Easy to see directly in a commit which validators signed what without having to fetch the validator set

If and when we change the CommitSig again, for instance to remove the timestamp, we can reconsider whether the ValidatorAddress should be removed.

Absent Votes: we include absent votes explicitly with no Signature or Timestamp but with the ValidatorAddress. This should resolve the serialization issues and make it easy to see which validator's votes failed to be included.

Other BlockIDs: We use a single byte to indicate which blockID a CommitSig is for. The only options are: - Absent - no vote received from the this validator, so no signature - Nil - validator voted Nil - meaning they did not see a polka in time - Commit - validator voted for this block

Note this means we don't allow votes for any other blockIDs. If a signature is included in a commit, it is either for nil or the correct blockID. According to the Tendermint protocol and assumptions, there is no way for a correct validator to precommit for a conflicting blockID in the same round an actual commit was created. This was the consensus from #3485

We may want to consider supporting other blockIDs later, as a way to capture evidence that might be helpful. We should clarify if/when/how doing so would actually help first. To implement it, we could change the Commit.BlockID field to a slice, where the first entry is the correct block ID and the other entries are other BlockIDs that validators precommited before. The BlockIDFlag enum can be extended to represent these additional block IDs on a per block basis.

Status

Implemented

Consequences

Positive

Removing the Type/Height/Round/Index and the BlockID saves roughly 80 bytes per precommit. It varies because some integers are varint. The BlockID contains two 32-byte hashes an integer, and the Height is 8-bytes.

For a chain with 100 validators, that's up to 8kB in savings per block!

Negative

• Large breaking change to the block and commit structure
• Requires differentiating in code between the Vote and CommitSig objects, which may add some complexity (votes need to be reconstructed to be verified and gossiped)

Neutral

• Commit.Precommits no longer contains nil values