I’ve rebased the PR agains develop, CI checks have now passed (previous failure was non-deterministic and unrelated to this PR, see #2227).

So the key part from #1909 (comment) that is missing here is We'd still need to make an empty proofBlock if the last block has transactions so we can get proofs for the new state resulting from the previous block.

In any case we should probably do this right by moving the control of block production into EndBlock. Of course that's a bit more involved, and should start with a spec to outline exactly how it should be done. ADR-036 has been reserved for this if you want to take a stab at it.

In the meantime, we should probably close this. Thanks.

This pull request only removes creating empty blocks when app hash changes, not when there are transactions, so I don’t see anything missing from the comment. Maybe there is an error in the documentation? It says that “[…] blocks will be created when there are new transactions or when the AppHash changes”. According to this, if there is a transaction that doesn’t change app hash, a block still will be created, so there should exist a mechanism that enforces block creation when there are transactions.

The problem is that if there are txs in block H, we need to create block H+1, even if block H+1 is empty, because block H+1 contains the AppHash for the result of the tx included in block H. If we don't create block H+1, a light client will not be able to verify the result of the tx included in block H.

The simplest way to achieve this "make a block after the block with txs" idea was to rely on the changing app hash, but that of course has the side effect that it will keep making blocks if the app hash keeps changing.

So we could consider a way to only make one empty block after a block with txs. That actually might not be very difficult, but would technically be a breaking change to the current semantics (though it's not clear the current semantics actually benefits anyone!)