-
Notifications
You must be signed in to change notification settings - Fork 814
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
Reject proposal if max block gas is exceeded #674
Conversation
func (app *App) checkTotalBlockGasWanted(ctx sdk.Context, txs [][]byte) bool { | ||
totalGasWanted := uint64(0) | ||
for _, tx := range txs { | ||
decoded, err := app.txDecoder(tx) |
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.
Nit: If we have perf concern, we can make decode run in parallel, and then use atomic integer for calculating totalGasWanted
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.
yeah good point. I'll add it in in a later PR if we find this to be a bottleneck
|
Describe your changes and provide context
Implementation for https://github.com/sei-protocol/sei-chain/pull/new/tony-chen-max-block-gas-check
We want honest validators (i.e. validators who do not use a maliciously modified binary) to reject block proposals whose transactions' GasWanted add up to something greater than max block gas. Since transactions in block proposals are sent as raw bytes, the only place to decode them would be on application-level during ProcessProposal. For context, ProcessProposal is called during
prevote
phase by validators, and if it returns a status ofREJECT
, validators will vote nil.Testing performed to validate your change
unit test
integration test in a loadtest cluster with a modified node to verify such proposal will indeed be rejected