Write ADRs for checking block encoding and message inclusion during consensus

[evan-forbes]

Summary

There are currently no checks for block encoding and message inclusion during consensus, and we will need those.

Details

There are two important checks that need to be performed during consensus that we are not currently doing. The first is to check that each SignedTransactionDataPayForMessage included in the block has a corresponding message in the same block. The second check that we have to implement is that the block data is actually encoded correctly. If either of these checks fail, then the block is not valid, and should treated as such by validators and full nodes.

Per usual, tendermint’s current deferred execution model makes implementing these checks more complicated than it would initially seem, and that’s why I’m suggesting we write two specific ADRs on how to implement each check as opposed to just writing two proposals.

Checking for bad encoding should be relatively simple, and is related to the current efforts to implement BadEncodingFraudProofs (#513 and #461) and removing the DAH from block #512. This ADR should touch on how full nodes/validators will reject blocks that are not encoded properly, along with how a celestia node would generate and propagate a BadEncodingFraudProof. The efforts to remove the data availability header from the block are also related, as part of validating the encoding of the block requires generating the DataAvailabilityHeader and comparing it to the DataHash.

Ensuring that each SignedTransactionDataPayForMessage included in the block has a corresponding message will likely be more complicated. This is mainly because the app will have to be made aware of the block data encoding scheme, and how and when it will communicate with celestia-core to indicate that a block is invalid. Implementing this is also relevant to the proposal to reap the optimal transactions for a block #454, in that both require the app to be made aware of the block data encoding scheme.

It might be worth waiting for ABCI++ (probably after devnet) before implementing or even writing these ADRs, as it will give us much more freedom in how the app and core communicate.

I think it definitely makes sense to at least wait until we have upgraded from upstream, because implementing either of these beforehand will likely make upgrading more cumbersome.

• Write ADR for including an encoding check during consensus
• Write ADR for including a message inclusion check during consensus

References and Context

related to celestiaorg/celestia-app#1006 and #461
blocked by #492

[evan-forbes]

note: The most recent refactor/update of core removed the DataAvailabilityHeader from the Block, which forces validators to regenerate the data hash from block data when performing basic validation on the block after it is propagated.

celestia-core/types/block.go

Lines 83 to 86 in c2df2c4

	// NOTE: b.Data.Txs may be nil, but b.Data.Hash() still works fine.
	if w, g := b.Data.Hash(), b.DataHash; !bytes.Equal(w, g) {
	return fmt.Errorf("wrong Header.DataHash. Expected %X, got %X", w, g)
	}

[evan-forbes]

During a synchronous discussion, we clarified the point that make an honest majority assumption is okay, but making an assumption that all proposers will be honest is not safe assumption.

Meaning that we will definitely have to have a check for message inclusion before a given validator votes.

[liamsi]

Meaning that we will definitely have to have a check for message inclusion before a given validator votes.

We need to verify if this is possible at all without abci++, if not I think we need this step: tendermint/tendermint#7091

Would be good if we continued joining the abci++ calls.

[evan-forbes]

This was completed across a few different PRs when switching to ABCI++ in the application.