Can we replace LastBlockID in Header with LastBlockHash ?

[ebuchman]

Is there some reason we need the BlockID in the Header? From what I understand, we only need it in the proposal to support the SWIFT like gossiping. I don't immediately see why we would need the next block to include a commitment to the way the last block was gossiped.

@jaekwon @milosevic

[ebuchman]

Also what's the reason for the hash of the header to be a merkle tree of its components ? Is that really worth it when we could simply amino hash the header as is ? Ideally the binary encoding of the header is effectively a protobuf encoding - that would be the simplest thing

[milosevic]

Regarding BlockID, seems reasonable what you are saying. I guess that the role of this field is just to link block at height h with a block at height h-1. For this purpose, hash should do the job, so BlockID is not needed.

Regarding the second question, when we need to prove something (for example with light client or IBC) do we need header to serve as a root hash, or it is sufficient having several root hashes for different aspects of the system. My understanding is that it's not really needed as we prove that header is valid by checking LastCommit, and then we validated various root hashes (validator set, app hash, etc).

[ebuchman]

header is valid by checking LastCommit

right, which means we need to hash the header to check the signature. I'm just proposing we make "hash the header" as simple as possible by making it a simple sha256 of the protobuf encoding!

[milosevic]

Understood this. Seems like sha256 of the protobuf encoding is fine.

[ebuchman]

Ok, so the reason the votes need to sign the BlockID is so that peers can securely download a block using the block-part mechanism just from seeing a commit. This also means the consensus reactor can help catch up old peers by gossiping block-parts (unlike the blockchain reactor sending whole blocks).

Then since the votes sign the BlockID, you need to know the BlockID to verify a commit. Currently the BlockID is in the commit votes and the header, but since we plan to remove it from the commit votes (#1648 ), we'd have to store it somewhere else in the block. It could be in the Commit structure, or else in the Header - I guess it might as well stay in the header.

This also allows us to use the block-part mechanism to securely catchup peers in the consensus reactor - once they verify a commit, they can securely download block parts from different peers because they know the whole BlockID.

If we wanted to get rid of BlockID, we'd probably have to do something like elevate the use of Proposals - you'd need to have the proposal and the commit to download blocks using the block-part mechanism. This is probably not worth the complexity in the short term, but interesting to consider.

[milosevic]

Ok, so it seems we have several different concerns here.

1. Currently in the block Header, we have the LastBlockID field as a reference to a previous block. The field is of type BlockId and contains hash of the header and the PartSetHeader. Hash of the header is a merkle tree of its components (

   tendermint/types/block.go

   Line 242 in 503de8c

   func (h *Header) Hash() cmn.HexBytes {

   ). If the role of this field is just to provide a cryptographic pointer to a previous block, then having LastBlockHash instead of LastBlockID seems sufficient, i.e., there is no need of having PartSetHeader of the previous Block as part of this field as it is useful only in SWIFT gossiping of the Block. Furthermore, it is not clear what is the benefit of computing hash of the header this way; it seems sufficient to compute it as a simple sha256 of the protobuf encoding of the header.

2. On the other hand, we have BlockID as part of Vote message. The reason why we have BlockID instead of simple block header hash is to enable SWIFT dissemination of the Block in case a process has seen Polka or Commit for some Block, but hasn't received block. Having PartSetHeader enables process to obtain Block faster by fetching parts in parallel from its peers. Note that at the point of Vote creation, process obtains the corresponding PartSetHeader from the ConsensusState, either
   from cs.LockedBlockParts.Header() or cs.ProposalBlockParts.Header().

3. As @ebuchman mentioned above, BlockID is useful also to securely catchup peers in the consensus reactor - once they verify a commit, they can securely download block parts from different peers because they know the whole BlockID. We achieve this by having BlockID as part of Commit structure.

To sum up, I think that we can replace LastBlockID in the block header with LastBlockHash, and that the hash of the header can be simple sha256 of the protobuf encoding of the header.

[ebuchman]

We still need the BlockID to verify commits though - if we remove it from the Header, it still has to be present in the block otherwise no one will be able to verify commits!

[milosevic]

The question was if we can replace LastBlockId with LastBlockHash. We do need to have BlockId in the block in case we want to support SWIFT dissemination. Could you please be more precise what do you mean by verifying commits?

[ebuchman]

Commits are a list of (signature, timestamp) (at least we're planning for that), and to verify the signature you need to know the BlockID (not just the block hash), so we have to store the BlockID somewhere in the block for for Commit to be verified - otherwise folks would need the entire block so they could compute the parts header to verify the commit!

[ebuchman]

We could replace BlockID with BlockHash in the header, but then we'd need to make sure the BlockID is still present in the block somehow so the votes can be verified. Since that's the case, it doesn't really feel worth it to remove it from the header...

As for the hash of the header, I think the argument is that if a light client wants to verify something in the header, it doesn't need the whole header (which could be a few hundred bytes) but just the field it's interested in and the Merkle proof. So using a Merkle tree here instead of a raw hash might save a few hundred bytes, but as soon as you want to verify more than one field in the header, those savings probably disappear ...

[milosevic]

We could replace BlockID with BlockHash in the header, but then we'd need to make sure the
BlockID is still present in the block somehow so the votes can be verified. Since that's the case, it
doesn't really feel worth it to remove it from the header...

The gain of removing BlockID from header is small in terms of bytes, but maybe it is cleaner like this.

[ebuchman]

So I think in conclusion we are not going to remove the BlockID, so nothing will change here. We should definitely better document why it's there and why we need it.

[xla]

Can we create a follow-up issue and close this?

[milosevic]

Here is the issue: https://github.com/tendermint/tendermint/issues/2373.