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

* 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
Finschia · Oct 25, 2022 · 84ddf95 · 84ddf95
1 parent 1726293
commit 84ddf95
Show file tree

Hide file tree

Showing 7 changed files with 74 additions and 5 deletions.
diff --git a/.github/workflows/release-sims.yml b/.github/workflows/release-sims.yml
@@ -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
@@ -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
@@ -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
@@ -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

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -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

diff --git a/baseapp/abci.go b/baseapp/abci.go
@@ -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))
@@ -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 {

diff --git a/baseapp/abci_test.go b/baseapp/abci_test.go
@@ -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)
+}
diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go
@@ -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
 }
 

diff --git a/x/auth/ante/setup.go b/x/auth/ante/setup.go
@@ -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
 	}
 

diff --git a/x/auth/ante/setup_test.go b/x/auth/ante/setup_test.go
@@ -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"
@@ -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() {