Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Feature]: Update block gas documentation #17320

Closed
Tracked by #139
colin-axner opened this issue Aug 8, 2023 · 0 comments · Fixed by #20128
Closed
Tracked by #139

[Feature]: Update block gas documentation #17320

colin-axner opened this issue Aug 8, 2023 · 0 comments · Fixed by #20128
Assignees
Labels
T:Docs Changes and features related to documentation. T:feature-request

Comments

@colin-axner
Copy link
Contributor

Summary

The block gas documentation needs to be updated, see here

Problem Definition

Older version of this doc:

When a new transaction is being processed via DeliverTx, the current value of BlockGasMeter is checked to see if it is above the limit. If it is, DeliverTx returns immediately. This can happen even with the first transaction in a block, as BeginBlock itself can consume gas. If not, the transaction is processed normally. At the end of DeliverTx, the gas tracked by ctx.BlockGasMeter() is increased by the amount consumed to process the transaction:

This seems to imply block gas is consumed on begin block (and maybe even end block). The reality is that a module could consume block gas through the context (but this could happen at any point)

The newer docs:

When a new transaction is being processed via FinalizeBlock, the current value of BlockGasMeter is checked to see if it is above the limit. If it is, the transaction fails and returned to the consensus engine as a failed transaction. This can happen even with the first transaction in a block, as FinalizeBlock itself can consume gas. If not, the transaction is processed normally. At the end of FinalizeBlock, the gas tracked by ctx.BlockGasMeter() is increased by the amount consumed to process the transaction:

The references to begin block got replaced with FinalizeBlock which also executes tx's so now it is quite confusing to read.

Proposal

Update the documentation to specify exactly when baseapp consumes block gas

@tac0turtle tac0turtle added the T:Docs Changes and features related to documentation. label Aug 8, 2023
@github-project-automation github-project-automation bot moved this to 👀 To Do in Cosmos-SDK Nov 17, 2023
@samricotta samricotta self-assigned this Apr 22, 2024
@samricotta samricotta moved this from 📋 Backlog to 🤸‍♂️ In Progress in Cosmos-SDK Apr 22, 2024
@github-project-automation github-project-automation bot moved this from 🤸‍♂️ In Progress to 🥳 Done in Cosmos-SDK Apr 22, 2024
@tac0turtle tac0turtle removed this from Cosmos-SDK Jul 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T:Docs Changes and features related to documentation. T:feature-request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants