github.com/badrootd/nibiru-cometbft@v0.37.5-0.20240307173500-2a75559eee9b/docs/architecture/adr-025-commit.md (about)

     1  # ADR 025 Commit
     2  
     3  ## Context
     4  
     5  Currently the `Commit` structure contains a lot of potentially redundant or unnecessary data.
     6  It contains a list of precommits from every validator, where the precommit
     7  includes the whole `Vote` structure. Thus each of the commit height, round,
     8  type, and blockID are repeated for every validator, and could be deduplicated,
     9  leading to very significant savings in block size.
    10  
    11  ```
    12  type Commit struct {
    13      BlockID    BlockID `json:"block_id"`
    14      Precommits []*Vote `json:"precommits"`
    15  }
    16  
    17  type Vote struct {
    18      ValidatorAddress Address   `json:"validator_address"`
    19      ValidatorIndex   int       `json:"validator_index"`
    20      Height           int64     `json:"height"`
    21      Round            int       `json:"round"`
    22      Timestamp        time.Time `json:"timestamp"`
    23      Type             byte      `json:"type"`
    24      BlockID          BlockID   `json:"block_id"`
    25      Signature        []byte    `json:"signature"`
    26  }
    27  ```
    28  
    29  The original tracking issue for this is [#1648](https://github.com/tendermint/tendermint/issues/1648).
    30  We have discussed replacing the `Vote` type in `Commit` with a new `CommitSig`
    31  type, which includes at minimum the vote signature. The `Vote` type will
    32  continue to be used in the consensus reactor and elsewhere.
    33  
    34  A primary question is what should be included in the `CommitSig` beyond the
    35  signature. One current constraint is that we must include a timestamp, since
    36  this is how we calculuate BFT time, though we may be able to change this [in the
    37  future](https://github.com/tendermint/tendermint/issues/2840).
    38  
    39  Other concerns here include:
    40  
    41  - Validator Address [#3596](https://github.com/tendermint/tendermint/issues/3596) -
    42      Should the CommitSig include the validator address? It is very convenient to
    43      do so, but likely not necessary. This was also discussed in [#2226](https://github.com/tendermint/tendermint/issues/2226).
    44  - Absent Votes [#3591](https://github.com/tendermint/tendermint/issues/3591) -
    45      How to represent absent votes? Currently they are just present as `nil` in the
    46      Precommits list, which is actually problematic for serialization
    47  - Other BlockIDs [#3485](https://github.com/tendermint/tendermint/issues/3485) -
    48      How to represent votes for nil and for other block IDs? We currently allow
    49      votes for nil and votes for alternative block ids, but just ignore them
    50  
    51  
    52  ## Decision
    53  
    54  Deduplicate the fields and introduce `CommitSig`:
    55  
    56  ```
    57  type Commit struct {
    58      Height  int64
    59      Round   int
    60      BlockID    BlockID      `json:"block_id"`
    61      Precommits []CommitSig `json:"precommits"`
    62  }
    63  
    64  type CommitSig struct {
    65      BlockID  BlockIDFlag
    66      ValidatorAddress Address
    67      Timestamp time.Time
    68      Signature []byte
    69  }
    70  
    71  
    72  // indicate which BlockID the signature is for
    73  type BlockIDFlag int
    74  
    75  const (
    76  	BlockIDFlagAbsent BlockIDFlag = iota // vote is not included in the Commit.Precommits
    77  	BlockIDFlagCommit                    // voted for the Commit.BlockID
    78  	BlockIDFlagNil                       // voted for nil
    79  )
    80  
    81  ```
    82  
    83  Re the concerns outlined in the context:
    84  
    85  **Timestamp**: Leave the timestamp for now. Removing it and switching to
    86  proposer based time will take more analysis and work, and will be left for a
    87  future breaking change. In the meantime, the concerns with the current approach to
    88  BFT time [can be
    89  mitigated](https://github.com/tendermint/tendermint/issues/2840#issuecomment-529122431).
    90  
    91  **ValidatorAddress**: we include it in the `CommitSig` for now. While this
    92  does increase the block size unecessarily (20-bytes per validator), it has some ergonomic and debugging advantages:
    93  
    94  - `Commit` contains everything necessary to reconstruct `[]Vote`, and doesn't depend on additional access to a `ValidatorSet`
    95  - Lite clients can check if they know the validators in a commit without
    96    re-downloading the validator set
    97  - Easy to see directly in a commit which validators signed what without having
    98    to fetch the validator set
    99  
   100  If and when we change the `CommitSig` again, for instance to remove the timestamp,
   101  we can reconsider whether the ValidatorAddress should be removed.
   102  
   103  **Absent Votes**: we include absent votes explicitly with no Signature or
   104  Timestamp but with the ValidatorAddress. This should resolve the serialization
   105  issues and make it easy to see which validator's votes failed to be included.
   106  
   107  **Other BlockIDs**: We use a single byte to indicate which blockID a `CommitSig`
   108  is for. The only options are:
   109      - `Absent` - no vote received from the this validator, so no signature
   110      - `Nil` - validator voted Nil - meaning they did not see a polka in time
   111      - `Commit` - validator voted for this block
   112  
   113  Note this means we don't allow votes for any other blockIDs. If a signature is
   114  included in a commit, it is either for nil or the correct blockID. According to
   115  the Tendermint protocol and assumptions, there is no way for a correct validator to
   116  precommit for a conflicting blockID in the same round an actual commit was
   117  created. This was the consensus from
   118  [#3485](https://github.com/tendermint/tendermint/issues/3485)
   119  
   120  We may want to consider supporting other blockIDs later, as a way to capture
   121  evidence that might be helpful. We should clarify if/when/how doing so would
   122  actually help first. To implement it, we could change the `Commit.BlockID`
   123  field to a slice, where the first entry is the correct block ID and the other
   124  entries are other BlockIDs that validators precommited before. The BlockIDFlag
   125  enum can be extended to represent these additional block IDs on a per block
   126  basis.
   127  
   128  ## Status
   129  
   130  Implemented
   131  
   132  ## Consequences
   133  
   134  ### Positive
   135  
   136  Removing the Type/Height/Round/Index and the BlockID saves roughly 80 bytes per precommit.
   137  It varies because some integers are varint. The BlockID contains two 32-byte hashes an integer,
   138  and the Height is 8-bytes.
   139  
   140  For a chain with 100 validators, that's up to 8kB in savings per block!
   141  
   142  
   143  ### Negative
   144  
   145  - Large breaking change to the block and commit structure
   146  - 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)
   147  
   148  ### Neutral
   149  
   150  - Commit.Precommits no longer contains nil values