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

Reject proposal if max block gas is exceeded #674

Merged
merged 3 commits into from
Mar 31, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 29 additions & 0 deletions app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -943,6 +943,13 @@ func (app *App) ClearOptimisticProcessingInfo() {
}

func (app *App) ProcessProposalHandler(ctx sdk.Context, req *abci.RequestProcessProposal) (*abci.ResponseProcessProposal, error) {
// TODO: this check decodes transactions which is redone in subsequent processing. We might be able to optimize performance
// by recording the decoding results and avoid decoding again later on.
if !app.checkTotalBlockGasWanted(ctx, req.Txs) {
return &abci.ResponseProcessProposal{
Status: abci.ResponseProcessProposal_REJECT,
}, nil
}
if app.optimisticProcessingInfo == nil {
completionSignal := make(chan struct{}, 1)
optimisticProcessingInfo := &OptimisticProcessingInfo{
Expand Down Expand Up @@ -1468,6 +1475,28 @@ func (app *App) RegisterTendermintService(clientCtx client.Context) {
tmservice.RegisterTendermintService(app.BaseApp.GRPCQueryRouter(), clientCtx, app.interfaceRegistry)
}

func (app *App) checkTotalBlockGasWanted(ctx sdk.Context, txs [][]byte) bool {
totalGasWanted := uint64(0)
for _, tx := range txs {
decoded, err := app.txDecoder(tx)
Copy link
Contributor

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

Copy link
Collaborator Author

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

if err != nil {
// such tx will not be processed and thus won't consume gas. Skipping
continue
}
feeTx, ok := decoded.(sdk.FeeTx)
if !ok {
// such tx will not be processed and thus won't consume gas. Skipping
continue
}
totalGasWanted += feeTx.GetGas()
if totalGasWanted > uint64(ctx.ConsensusParams().Block.MaxGas) {
// early return
return false
}
}
return true
}

// GetMaccPerms returns a copy of the module account permissions
func GetMaccPerms() map[string][]string {
dupMaccPerms := make(map[string][]string)
Expand Down
29 changes: 29 additions & 0 deletions app/app_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ import (
oracletypes "github.com/sei-protocol/sei-chain/x/oracle/types"
"github.com/stretchr/testify/require"
abci "github.com/tendermint/tendermint/abci/types"
"github.com/tendermint/tendermint/proto/tendermint/types"
)

func TestEmptyBlockIdempotency(t *testing.T) {
Expand Down Expand Up @@ -267,3 +268,31 @@ func TestProcessOracleAndOtherTxsSuccess(t *testing.T) {

require.Equal(t, 2, len(txResults))
}

func TestInvalidProposalWithExcessiveGasWanted(t *testing.T) {
tm := time.Now().UTC()
valPub := secp256k1.GenPrivKey().PubKey()

testWrapper := app.NewTestWrapper(t, tm, valPub)
ap := testWrapper.App
ctx := testWrapper.Ctx.WithConsensusParams(&types.ConsensusParams{
Block: &types.BlockParams{MaxGas: 10},
})
emptyTxBuilder := app.MakeEncodingConfig().TxConfig.NewTxBuilder()
txEncoder := app.MakeEncodingConfig().TxConfig.TxEncoder()
emptyTxBuilder.SetGasLimit(10)
emptyTx, _ := txEncoder(emptyTxBuilder.GetTx())
goodProposal := abci.RequestProcessProposal{
Txs: [][]byte{emptyTx},
}
res, err := ap.ProcessProposalHandler(ctx, &goodProposal)
require.Nil(t, err)
require.Equal(t, abci.ResponseProcessProposal_ACCEPT, res.Status)

badProposal := abci.RequestProcessProposal{
Txs: [][]byte{emptyTx, emptyTx},
}
res, err = ap.ProcessProposalHandler(ctx, &badProposal)
require.Nil(t, err)
require.Equal(t, abci.ResponseProcessProposal_REJECT, res.Status)
}