QuarkChain / go-ethereum

Official Go implementation of the Ethereum protocol
https://geth.ethereum.org
GNU Lesser General Public License v3.0
6 stars 2 forks source link

processing committed block validates commit only #62

Closed qizhou closed 2 years ago

qizhou commented 2 years ago

When a peer sends a committed block to local, the block already contains sufficient signature to verify the validity. As a result, we do need to touch the LastCommit part to verify the block.

blockchaindevsh commented 2 years ago

I think other validations are still needed? Maybe we should add an additional parameter seal bool to ValidateBlock(state pbft.ChainState, block *types.FullBlock) ? For committed blocks , seal is true; otherwise false.

qizhou commented 2 years ago

I think other validations are still needed? Maybe we should add an additional parameter seal bool to ValidateBlock(state pbft.ChainState, block *types.FullBlock) ? For committed blocks , seal is true; otherwise false.

Yes, another way is to add a boolean of committed to ValidateBlock().

In fact, this part is equivalent to the downloader (where the downloader should run first), but its purpose is to catch up with the current consensus state (height) if the node is temporarily disconnected from the network without switching to the downloader. So the verification logic is the same as that of the downloader, i.e., verifyHeader with seal=True.

blockchaindevsh commented 2 years ago

The downloader fetches block contents in phases: fetchHeaders, fetchBodies, fetchReceipts, processHeaders. But in our case it is all in one, so we need to do the full validation? Otherwise the body may not match with header.

qizhou commented 2 years ago

The downloader fetches block contents in phases: fetchHeaders, fetchBodies, fetchReceipts, processHeaders. But in our case it is all in one, so we need to do the full validation? Otherwise the body may not match with header.

I get your point. Basically, we need validation besides header including body (tx hash in header and post-state hash). Let me check how to put it into the validate function.

blockchaindevsh commented 2 years ago

Yeah, a simple fix IMO is to add a committed bool parameter to the existing ValidateBlock.