Empty blocks are being created even though create_empty_blocks = false is set in 'config.toml'

[shravanshetty1]

Summary of Bug

Version

v.044

Steps to Reproduce

---

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

[tac0turtle]

this could be that the sdk is updating apphashes on every block. If there is a change in the app hash the next block needs to be created. Which modules do you have locally?

[shravanshetty1]

@marbar3778 The source code is here - https://github.com/shravanshetty1/blawgd.
The custom module I created does have some logic that runs every block but it should not be changing the app hash - https://github.com/shravanshetty1/blawgd/blob/main/backends/cosmos/pkg/modules/blawgd/keeper/block.go

[tac0turtle]

Some other people are stating the same issue. Some people are looking into IBC to see if this is effecting things.

[tac0turtle]

Just tested with main cosmos-sdk and see the same thing. Will triage this as a bug

[yihuang]

Just tested with main cosmos-sdk and see the same thing. Will triage this as a bug

There are many things happening in begin/end blockers, if it modify state there, it's cause constant changing of app hash.

[arkantos1482]

any update on this issue?
I am facing the same problem, using a cosmos sdk app (set up with Ignite CLI) without even one line of custom code.
Facing this problem. (App hash changes on each consesnus round, making tendermint to create empty blocks).

Do you think internal modules like IBC, slashing, mint etc causing this problem, if so
Is there a way to disable them to stop this behavior?

[TobiaszCudnik]

After investigating this issue (along with #12356) it seems that LastCommitHash changes, which triggers a new block. This happens because of BeginBlocker in modules like x/mint and x/gov doing writes to KV stores, although these ops seems to be related to creating new blocks.

One possible solution would be to introduce a flag eg BaseApp.PendingTxs, populated during CheckTx and unset in EndBlock. With PendingTxs false and create_empty_blocks false, BeginBlocker logic would be skipped thus delivering an unchanged hash to tendermint.

@tac0turtle @alexanderbez How feasible do you think the above would be?

Another way is to talk directly to tendermint to check the mempool during BeginBlocker, but this seems complicated (tendermint/tendermint#7893).

[alexanderbez]

My immediate thought is why push this onto the server, i.e. the app? If create_empty_blocks: true, then you can make the argument that CometBFT shouldn't even be calling BeginBlock to begin with, thus avoiding the need for the app to handle such cases.

[TobiaszCudnik]

My immediate thought is why push this onto the server, i.e. the app? If create_empty_blocks: true, then you can make the argument that CometBFT shouldn't even be calling BeginBlock to begin with, thus avoiding the need for the app to handle such cases.

It happens because of "proof blocks". Ive created PRs for sdk and tendermint which mark "proof blocks" in RequestBeginBlock, so the modules can avoid write ops for those.

This seems to fix the issue, but Im not sure about side effects. Please take a look at those drafts, thx.

[tac0turtle]

thanks for the pr, since its blocked on comet we may close the PR.

I like the approach you took, its a step in the right direction but i think the better fix is for comet to allow the application to control this. There was a proposal to allow this in endblock which would mean that the app can fire off a "proofBlock" and then stop blocks from being created. Secondly the other issue is this feature is a local config to node operators when it would actually be better as a consensus param or in endblock/finalizeblock.

[adizere]

Hi @TobiaszCudnik, you'd be welcome to move this to a discussion or issue on the Comet repo to follow-up on Marko's suggestions for alternative fixes. We'll be glad to help.

[tac0turtle]

For those coming to this issue, this requires a design change from the current design of inflation. We would need to move to epoch based inflation instead of every block. This is part of our 2024 roadmap and look forward to working on it

[alexanderbez]

Epochs make sense. But note, if an empty block exists and it's the epoch block, you'll have a new block...so it's still a slight edge case.

[tac0turtle]

closing this as we are working on adding epochs for the mint module and will allow empty blocks to not be created in a chain with the mint modul