-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
feat: exceeding block gas meter don't fail #19318
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -842,6 +842,12 @@ func (app *BaseApp) runTx(mode execMode, txBytes []byte) (gInfo sdk.GasInfo, res | |
// fails. Hence, it's execution is deferred. | ||
consumeBlockGas := func() { | ||
if !blockGasConsumed { | ||
defer func() { | ||
// log and ignore, see: https://github.com/cosmos/cosmos-sdk/issues/19317 | ||
if err := recover(); err != nil { | ||
ctx.Logger().Error("block gas limit exceeded", "err", err) | ||
} | ||
}() | ||
blockGasConsumed = true | ||
ctx.BlockGasMeter().ConsumeGas( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sorry, can you please explain again how this could panic, thus making recovery needed? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It's related to unused gas refunding, if we include tx by user provided gas limit, they can fill block space with unused gas, the actually used gas is always lower than the block gas limit, we did some hack to include more txs, so we got exceeding block gas limit situation occasionally. |
||
ctx.GasMeter().GasConsumedToLimit(), "block gas meter", | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change potentially affects state.
Call sequence: