Skip to content

Commit

Permalink
fix: fix max gas validation (#673)
Browse files Browse the repository at this point in the history
* Run the validation on checkTx

* Set consensus params

* Update CHANGELOG.md

* Update workflows

* Add tests to cover ReCheckTx of validateMaxGas

* Add tests to cover consensus parameter updates
  • Loading branch information
0Tech authored Sep 21, 2022
1 parent e19f863 commit 30cd552
Show file tree
Hide file tree
Showing 7 changed files with 73 additions and 4 deletions.
15 changes: 15 additions & 0 deletions .github/workflows/release-sims.yml
Original file line number Diff line number Diff line change
Expand Up @@ -21,13 +21,19 @@ jobs:
if: "!contains(github.event.head_commit.message, 'skip-sims')"
steps:
- uses: actions/checkout@v3
- uses: actions/setup-go@v3.3.0
with:
go-version: 1.18
- run: |
make build
install-runsim:
runs-on: ubuntu-latest
needs: build
steps:
- uses: actions/setup-go@v3.3.0
with:
go-version: 1.18
- name: install runsim
run: |
export GO111MODULE="on" && go install github.com/cosmos/tools/cmd/runsim@v1.0.0
Expand All @@ -41,6 +47,9 @@ jobs:
needs: [build, install-runsim]
steps:
- uses: actions/checkout@v3
- uses: actions/setup-go@v3.3.0
with:
go-version: 1.18
- uses: actions/cache@v3.0.8
with:
path: ~/go/bin
Expand All @@ -58,6 +67,9 @@ jobs:
needs: [build, install-runsim, test-sim-multi-seed-long-part1]
steps:
- uses: actions/checkout@v3
- uses: actions/setup-go@v3.3.0
with:
go-version: 1.18
- uses: actions/cache@v3.0.8
with:
path: ~/go/bin
Expand All @@ -75,6 +87,9 @@ jobs:
needs: [build, install-runsim, test-sim-multi-seed-long-part2]
steps:
- uses: actions/checkout@v3
- uses: actions/setup-go@v3.3.0
with:
go-version: 1.18
- uses: actions/cache@v3.0.8
with:
path: ~/go/bin
Expand Down
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (x/wasm) [\#640](https://github.com/line/lbm-sdk/pull/640) remove legacy codes of wasm
* (amino) [\#635](https://github.com/line/lbm-sdk/pull/635) change some minor things that haven't been fixed in #549
* (store) [\#666](https://github.com/line/lbm-sdk/pull/666) change default `iavl-cache-size` and description
* (x/auth) [\#673](https://github.com/line/lbm-sdk/pull/673) fix max gas validation

### Breaking Changes
* (proto) [\#564](https://github.com/line/lbm-sdk/pull/564) change gRPC path to original cosmos path
Expand Down
4 changes: 2 additions & 2 deletions baseapp/abci.go
Original file line number Diff line number Diff line change
Expand Up @@ -182,7 +182,6 @@ func (app *BaseApp) BeginBlock(req abci.RequestBeginBlock) (res abci.ResponseBeg

app.deliverState.ctx = app.deliverState.ctx.
WithVoteInfos(app.voteInfos).
WithConsensusParams(app.GetConsensusParams(app.deliverState.ctx)).
WithBlockGasMeter(gasMeter).
WithHeaderHash(req.Hash).
WithConsensusParams(app.GetConsensusParams(app.deliverState.ctx))
Expand All @@ -192,7 +191,8 @@ func (app *BaseApp) BeginBlock(req abci.RequestBeginBlock) (res abci.ResponseBeg
if app.checkState != nil {
app.checkState.ctx = app.checkState.ctx.
WithBlockGasMeter(gasMeter).
WithHeaderHash(req.Hash)
WithHeaderHash(req.Hash).
WithConsensusParams(app.GetConsensusParams(app.deliverState.ctx))
}

if app.beginBlocker != nil {
Expand Down
36 changes: 36 additions & 0 deletions baseapp/abci_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -160,3 +160,39 @@ func TestBaseAppCreateQueryContext(t *testing.T) {
})
}
}

// Test and ensure that consensus params has been updated.
// See:
// - https://github.com/line/lbm-sdk/pull/673
func TestBaseAppBeginBlockConsensusParams(t *testing.T) {
t.Parallel()

logger := defaultLogger()
db := dbm.NewMemDB()
name := t.Name()
app := NewBaseApp(name, logger, db, nil)
app.SetParamStore(&paramStore{db: dbm.NewMemDB()})
app.InitChain(abci.RequestInitChain{
ConsensusParams: &abci.ConsensusParams{
Block: &abci.BlockParams{
MaxGas: -1,
},
},
})
app.init()

// set block params
app.BeginBlock(abci.RequestBeginBlock{Header: ocproto.Header{Height: 1}})
ctx := app.deliverState.ctx
maxGas := int64(123456789)
app.paramStore.Set(ctx, ParamStoreKeyBlockParams,
&abci.BlockParams{
MaxGas: maxGas,
})
app.Commit()

// confirm consensus params updated into the context
app.BeginBlock(abci.RequestBeginBlock{Header: ocproto.Header{Height: 2}})
newCtx := app.getContextForTx(app.checkState, []byte{})
require.Equal(t, maxGas, newCtx.ConsensusParams().Block.MaxGas)
}
1 change: 1 addition & 0 deletions baseapp/baseapp.go
Original file line number Diff line number Diff line change
Expand Up @@ -551,6 +551,7 @@ func (app *BaseApp) getRunContextForTx(txBytes []byte, simulate bool) sdk.Contex

func (app *BaseApp) getContextForTx(s *state, txBytes []byte) sdk.Context {
ctx := s.ctx.WithTxBytes(txBytes)
ctx = ctx.WithConsensusParams(app.GetConsensusParams(ctx))
return ctx
}

Expand Down
3 changes: 1 addition & 2 deletions x/auth/ante/setup.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,7 @@ func SetGasMeter(simulate bool, ctx sdk.Context, gasLimit uint64) sdk.Context {
}

func validateGasWanted(ctx sdk.Context) error {
// validate gasWanted only when checkTx
if !ctx.IsCheckTx() || ctx.IsReCheckTx() {
if !ctx.IsCheckTx() {
return nil
}

Expand Down
17 changes: 17 additions & 0 deletions x/auth/ante/setup_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package ante_test

import (
abci "github.com/line/ostracon/abci/types"

cryptotypes "github.com/line/lbm-sdk/crypto/types"
"github.com/line/lbm-sdk/testutil/testdata"
sdk "github.com/line/lbm-sdk/types"
Expand Down Expand Up @@ -41,6 +43,21 @@ func (suite *AnteTestSuite) TestSetup() {

// Context GasMeter Limit should be set after SetUpContextDecorator runs
suite.Require().Equal(gasLimit, newCtx.GasMeter().Limit(), "GasMeter not set correctly")

// Set MaxGas lower than the tx's gasWanted
consensusParams := &abci.ConsensusParams{
Block: &abci.BlockParams{
MaxGas: int64(gasLimit) - 1,
},
}
suite.ctx = suite.ctx.WithConsensusParams(consensusParams)

// for both of CheckTx and ReCheckTx
for _, isRecheck := range []bool{false, true} {
suite.ctx = suite.ctx.WithIsReCheckTx(isRecheck)
_, err = antehandler(suite.ctx, tx, false)
suite.Require().Error(err)
}
}

func (suite *AnteTestSuite) TestRecoverPanic() {
Expand Down

0 comments on commit 30cd552

Please sign in to comment.