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

fix: fix max gas validation (backport #673) #746

Merged
merged 3 commits into from
Oct 25, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
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
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.11
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.11
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.11
with:
path: ~/go/bin
Expand Down
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,8 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (x/token) [\#599](https://github.com/line/lbm-sdk/pull/599) fix the order of events
* (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
* (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
* (simapp) [\#679](https://github.com/line/lbm-sdk/pull/679) fix the bug not setting `iavl-cache-size` value of `app.toml`
* (x/foundation) [\#687](https://github.com/line/lbm-sdk/pull/687) fix bugs on aborting x/foundation proposals
* (global) [\#694](https://github.com/line/lbm-sdk/pull/694) replace deprecated functions since go 1.16 or 1.17
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 @@ -169,3 +169,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