-
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
fix: check tx gas limit against block gas limit #16547
Changes from all commits
b29ea13
9816ad5
4ee99d9
b2bd73f
c83162f
e94dc4c
f804f1e
87153cd
20fc493
bbcdece
ac72869
cfb9caf
e8837b7
2540103
3432216
667765b
fde1547
5eb7cab
44f55f1
aa40b23
de4da6b
f8ffe1c
ff0fd01
46f9c77
5360e0f
4ea9b70
2dd1b26
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 |
---|---|---|
|
@@ -40,6 +40,14 @@ func (sud SetUpContextDecorator) AnteHandle(ctx sdk.Context, tx sdk.Tx, simulate | |
|
||
newCtx = SetGasMeter(simulate, ctx, gasTx.GetGas()) | ||
|
||
if cp := ctx.ConsensusParams(); cp.Block != nil { | ||
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. i dont think this fixes the issue at hand, the mempool could still put in an amount of txs that is larger the maxgas. 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. No, because txs are only inserted into the app's mempool if CheckTx passes. CheckTx would fail here so such a tx would never enter the app's mempool. However, we also need to update the SDK's default ABCI++ proposal handlers so we can handle an entire proposal. |
||
// If there exists a maximum block gas limit, we must ensure that the tx | ||
// does not exceed it. | ||
if cp.Block.MaxGas > 0 && gasTx.GetGas() > uint64(cp.Block.MaxGas) { | ||
return newCtx, errorsmod.Wrapf(sdkerrors.ErrInvalidGasLimit, "tx gas limit %d exceeds block max gas %d", gasTx.GetGas(), cp.Block.MaxGas) | ||
} | ||
} | ||
|
||
// Decorator will catch an OutOfGasPanic caused in the next antehandler | ||
// AnteHandlers must have their own defer/recover in order for the BaseApp | ||
// to know how much gas was used! This is because the GasMeter is created in | ||
|
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.
can we add a testcase that covers this.