From 18a27770c30db0692fdfb126da0a421a3c2f6b95 Mon Sep 17 00:00:00 2001 From: codchen Date: Mon, 19 Dec 2022 11:48:04 +0800 Subject: [PATCH 1/8] Fix Begin/EndBlock stuck at infinite loop in contracts --- x/dex/keeper/utils/wasm.go | 10 +++++++--- x/oracle/simulation/operations.go | 4 ++-- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/x/dex/keeper/utils/wasm.go b/x/dex/keeper/utils/wasm.go index dd776fb32b..79237528f8 100644 --- a/x/dex/keeper/utils/wasm.go +++ b/x/dex/keeper/utils/wasm.go @@ -39,12 +39,16 @@ func sudo(sdkCtx sdk.Context, k *keeper.Keeper, contractAddress []byte, wasmMsg // Measure the time it takes to execute the contract in WASM defer metrics.MeasureSudoExecutionDuration(time.Now(), msgType) // set up a tmp context to prevent race condition in reading gas consumed - tmpCtx := sdkCtx.WithGasMeter(sdk.NewInfiniteGasMeter()) + tmpCtx := sdkCtx.WithGasMeter(sdk.NewGasMeter(sdkCtx.GasMeter().Limit())) + initialGasLevel := sdkCtx.GasMeter().GasConsumedToLimit() // gas consumed so far + tmpCtx.GasMeter().ConsumeGas(initialGasLevel, "initialize temp") data, err := k.WasmKeeper.Sudo( tmpCtx, contractAddress, wasmMsg, ) - gasConsumed := tmpCtx.GasMeter().GasConsumed() - sdkCtx.GasMeter().ConsumeGas(gasConsumed, "sudo") + gasConsumed := tmpCtx.GasMeter().GasConsumed() - initialGasLevel + if gasConsumed > 0 { + sdkCtx.GasMeter().ConsumeGas(gasConsumed, "sudo") + } if hasErrInstantiatingWasmModuleDueToCPUFeature(err) { panic(utils.DecorateHardFailError(err)) } diff --git a/x/oracle/simulation/operations.go b/x/oracle/simulation/operations.go index 3988c4b164..2028e73625 100644 --- a/x/oracle/simulation/operations.go +++ b/x/oracle/simulation/operations.go @@ -65,7 +65,7 @@ func WeightedOperations( } // SimulateMsgAggregateExchangeRateVote generates a MsgAggregateExchangeRateVote with random values. -//nolint: funlen +// nolint: funlen func SimulateMsgAggregateExchangeRateVote(ak types.AccountKeeper, bk types.BankKeeper, k keeper.Keeper) simtypes.Operation { return func( r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, accs []simtypes.Account, chainID string, @@ -122,7 +122,7 @@ func SimulateMsgAggregateExchangeRateVote(ak types.AccountKeeper, bk types.BankK } // SimulateMsgDelegateFeedConsent generates a MsgDelegateFeedConsent with random values. -//nolint: funlen +// nolint: funlen func SimulateMsgDelegateFeedConsent(ak types.AccountKeeper, bk types.BankKeeper, k keeper.Keeper) simtypes.Operation { return func( r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, accs []simtypes.Account, chainID string, From 5844535245c115cb388b15fec0b02be615d75379 Mon Sep 17 00:00:00 2001 From: codchen Date: Mon, 19 Dec 2022 12:05:50 +0800 Subject: [PATCH 2/8] fix --- x/dex/keeper/utils/wasm.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/x/dex/keeper/utils/wasm.go b/x/dex/keeper/utils/wasm.go index 79237528f8..08eb9b5bf5 100644 --- a/x/dex/keeper/utils/wasm.go +++ b/x/dex/keeper/utils/wasm.go @@ -40,12 +40,10 @@ func sudo(sdkCtx sdk.Context, k *keeper.Keeper, contractAddress []byte, wasmMsg defer metrics.MeasureSudoExecutionDuration(time.Now(), msgType) // set up a tmp context to prevent race condition in reading gas consumed tmpCtx := sdkCtx.WithGasMeter(sdk.NewGasMeter(sdkCtx.GasMeter().Limit())) - initialGasLevel := sdkCtx.GasMeter().GasConsumedToLimit() // gas consumed so far - tmpCtx.GasMeter().ConsumeGas(initialGasLevel, "initialize temp") data, err := k.WasmKeeper.Sudo( tmpCtx, contractAddress, wasmMsg, ) - gasConsumed := tmpCtx.GasMeter().GasConsumed() - initialGasLevel + gasConsumed := tmpCtx.GasMeter().GasConsumed() if gasConsumed > 0 { sdkCtx.GasMeter().ConsumeGas(gasConsumed, "sudo") } From 08f3d3a04b1053dc7611bc21c54aaa0ab0f1e3ea Mon Sep 17 00:00:00 2001 From: codchen Date: Mon, 19 Dec 2022 14:28:04 +0800 Subject: [PATCH 3/8] catch out of gas panic --- x/dex/keeper/utils/wasm.go | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/x/dex/keeper/utils/wasm.go b/x/dex/keeper/utils/wasm.go index 08eb9b5bf5..5a5289d216 100644 --- a/x/dex/keeper/utils/wasm.go +++ b/x/dex/keeper/utils/wasm.go @@ -40,9 +40,7 @@ func sudo(sdkCtx sdk.Context, k *keeper.Keeper, contractAddress []byte, wasmMsg defer metrics.MeasureSudoExecutionDuration(time.Now(), msgType) // set up a tmp context to prevent race condition in reading gas consumed tmpCtx := sdkCtx.WithGasMeter(sdk.NewGasMeter(sdkCtx.GasMeter().Limit())) - data, err := k.WasmKeeper.Sudo( - tmpCtx, contractAddress, wasmMsg, - ) + data, err := sudoWithoutOutOfGasPanic(tmpCtx, k, contractAddress, wasmMsg) gasConsumed := tmpCtx.GasMeter().GasConsumed() if gasConsumed > 0 { sdkCtx.GasMeter().ConsumeGas(gasConsumed, "sudo") @@ -53,6 +51,18 @@ func sudo(sdkCtx sdk.Context, k *keeper.Keeper, contractAddress []byte, wasmMsg return data, gasConsumed, err } +func sudoWithoutOutOfGasPanic(ctx sdk.Context, k *keeper.Keeper, contractAddress []byte, wasmMsg []byte) ([]byte, error) { + defer func() { + if err := recover(); err != nil { + // only propagate panic if the error is out of gas + if _, ok := err.(sdk.ErrorOutOfGas); !ok { + panic(err) + } + } + }() + return k.WasmKeeper.Sudo(ctx, contractAddress, wasmMsg) +} + func hasErrInstantiatingWasmModuleDueToCPUFeature(err error) bool { if err == nil { return false From 41b75007f6221a4f23e3528e986f5d9285a95d89 Mon Sep 17 00:00:00 2001 From: codchen Date: Mon, 19 Dec 2022 14:46:46 +0800 Subject: [PATCH 4/8] fix test --- x/dex/keeper/abci/begin_block_new_block_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/x/dex/keeper/abci/begin_block_new_block_test.go b/x/dex/keeper/abci/begin_block_new_block_test.go index fe9ba72516..e444e29988 100644 --- a/x/dex/keeper/abci/begin_block_new_block_test.go +++ b/x/dex/keeper/abci/begin_block_new_block_test.go @@ -19,6 +19,7 @@ func TestHandleBBNewBlock(t *testing.T) { // is tested in module_test.go where an actual wasm file is deployed and invoked. wasmkeeper.TestingStakeParams.MinCommissionRate = sdk.NewDecWithPrec(5, 2) ctx, wasmkeepers := wasmkeeper.CreateTestInput(t, false, SupportedFeatures) + ctx = ctx.WithGasMeter(sdk.NewInfiniteGasMeter()) dexKeeper := keeper.Keeper{WasmKeeper: *wasmkeepers.WasmKeeper} wrapper := abci.KeeperWrapper{Keeper: &dexKeeper} wrapper.HandleBBNewBlock(ctx, TestContract, 1) From a3171afa01e8900b8b492e41dff9bea24c285c2c Mon Sep 17 00:00:00 2001 From: codchen Date: Mon, 19 Dec 2022 16:53:15 +0800 Subject: [PATCH 5/8] fix test --- x/dex/keeper/abci/begin_block_new_block_test.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/x/dex/keeper/abci/begin_block_new_block_test.go b/x/dex/keeper/abci/begin_block_new_block_test.go index e444e29988..91e635811f 100644 --- a/x/dex/keeper/abci/begin_block_new_block_test.go +++ b/x/dex/keeper/abci/begin_block_new_block_test.go @@ -7,6 +7,7 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" "github.com/sei-protocol/sei-chain/x/dex/keeper" "github.com/sei-protocol/sei-chain/x/dex/keeper/abci" + "github.com/sei-protocol/sei-chain/x/dex/types" ) const ( @@ -21,6 +22,10 @@ func TestHandleBBNewBlock(t *testing.T) { ctx, wasmkeepers := wasmkeeper.CreateTestInput(t, false, SupportedFeatures) ctx = ctx.WithGasMeter(sdk.NewInfiniteGasMeter()) dexKeeper := keeper.Keeper{WasmKeeper: *wasmkeepers.WasmKeeper} + dexKeeper.SetContract(ctx, &types.ContractInfoV2{ + ContractAddr: TestContract, + RentBalance: 100000000, + }) wrapper := abci.KeeperWrapper{Keeper: &dexKeeper} wrapper.HandleBBNewBlock(ctx, TestContract, 1) } From f4c2b3b12c7890b162fb097b6d149263e3a9d403 Mon Sep 17 00:00:00 2001 From: codchen Date: Mon, 19 Dec 2022 21:59:08 +0800 Subject: [PATCH 6/8] fix test --- x/dex/keeper/abci/begin_block_new_block_test.go | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/x/dex/keeper/abci/begin_block_new_block_test.go b/x/dex/keeper/abci/begin_block_new_block_test.go index 91e635811f..164b605d26 100644 --- a/x/dex/keeper/abci/begin_block_new_block_test.go +++ b/x/dex/keeper/abci/begin_block_new_block_test.go @@ -3,9 +3,8 @@ package abci_test import ( "testing" - wasmkeeper "github.com/CosmWasm/wasmd/x/wasm/keeper" sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/sei-protocol/sei-chain/x/dex/keeper" + keepertest "github.com/sei-protocol/sei-chain/testutil/keeper" "github.com/sei-protocol/sei-chain/x/dex/keeper/abci" "github.com/sei-protocol/sei-chain/x/dex/types" ) @@ -18,14 +17,12 @@ const ( func TestHandleBBNewBlock(t *testing.T) { // this test only ensures that HandleBBNewBlock doesn't crash. The actual logic // is tested in module_test.go where an actual wasm file is deployed and invoked. - wasmkeeper.TestingStakeParams.MinCommissionRate = sdk.NewDecWithPrec(5, 2) - ctx, wasmkeepers := wasmkeeper.CreateTestInput(t, false, SupportedFeatures) + keeper, ctx := keepertest.DexKeeper(t) ctx = ctx.WithGasMeter(sdk.NewInfiniteGasMeter()) - dexKeeper := keeper.Keeper{WasmKeeper: *wasmkeepers.WasmKeeper} - dexKeeper.SetContract(ctx, &types.ContractInfoV2{ + keeper.SetContract(ctx, &types.ContractInfoV2{ ContractAddr: TestContract, RentBalance: 100000000, }) - wrapper := abci.KeeperWrapper{Keeper: &dexKeeper} + wrapper := abci.KeeperWrapper{Keeper: keeper} wrapper.HandleBBNewBlock(ctx, TestContract, 1) } From 9e43686b0562873d5c70a0b2d9f9cd2ce8f6aab3 Mon Sep 17 00:00:00 2001 From: codchen Date: Tue, 20 Dec 2022 00:13:51 +0800 Subject: [PATCH 7/8] fix tests --- x/dex/keeper/abci/begin_block_new_block_test.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/x/dex/keeper/abci/begin_block_new_block_test.go b/x/dex/keeper/abci/begin_block_new_block_test.go index 164b605d26..fa6d1a02fe 100644 --- a/x/dex/keeper/abci/begin_block_new_block_test.go +++ b/x/dex/keeper/abci/begin_block_new_block_test.go @@ -1,12 +1,17 @@ package abci_test import ( + "context" "testing" + "time" sdk "github.com/cosmos/cosmos-sdk/types" keepertest "github.com/sei-protocol/sei-chain/testutil/keeper" + dexcache "github.com/sei-protocol/sei-chain/x/dex/cache" "github.com/sei-protocol/sei-chain/x/dex/keeper/abci" "github.com/sei-protocol/sei-chain/x/dex/types" + dexutils "github.com/sei-protocol/sei-chain/x/dex/utils" + tmproto "github.com/tendermint/tendermint/proto/tendermint/types" ) const ( @@ -17,12 +22,15 @@ const ( func TestHandleBBNewBlock(t *testing.T) { // this test only ensures that HandleBBNewBlock doesn't crash. The actual logic // is tested in module_test.go where an actual wasm file is deployed and invoked. - keeper, ctx := keepertest.DexKeeper(t) + testApp := keepertest.TestApp() + ctx := testApp.BaseApp.NewContext(false, tmproto.Header{Time: time.Now()}) + ctx = ctx.WithContext(context.WithValue(ctx.Context(), dexutils.DexMemStateContextKey, dexcache.NewMemState(testApp.GetKey(types.StoreKey)))) + keeper := testApp.DexKeeper ctx = ctx.WithGasMeter(sdk.NewInfiniteGasMeter()) keeper.SetContract(ctx, &types.ContractInfoV2{ ContractAddr: TestContract, RentBalance: 100000000, }) - wrapper := abci.KeeperWrapper{Keeper: keeper} + wrapper := abci.KeeperWrapper{Keeper: &keeper} wrapper.HandleBBNewBlock(ctx, TestContract, 1) } From d764ee64cefb85a73c52e07f7734015c11b28d12 Mon Sep 17 00:00:00 2001 From: codchen Date: Tue, 20 Dec 2022 00:19:58 +0800 Subject: [PATCH 8/8] comment --- x/dex/keeper/utils/wasm.go | 3 +++ 1 file changed, 3 insertions(+) diff --git a/x/dex/keeper/utils/wasm.go b/x/dex/keeper/utils/wasm.go index 5a5289d216..583c608e84 100644 --- a/x/dex/keeper/utils/wasm.go +++ b/x/dex/keeper/utils/wasm.go @@ -39,6 +39,9 @@ func sudo(sdkCtx sdk.Context, k *keeper.Keeper, contractAddress []byte, wasmMsg // Measure the time it takes to execute the contract in WASM defer metrics.MeasureSudoExecutionDuration(time.Now(), msgType) // set up a tmp context to prevent race condition in reading gas consumed + // Note that the limit will effectively serve as a soft limit since it's + // possible for the actual computation to go above the specified limit, but + // the associated contract would be charged corresponding rent. tmpCtx := sdkCtx.WithGasMeter(sdk.NewGasMeter(sdkCtx.GasMeter().Limit())) data, err := sudoWithoutOutOfGasPanic(tmpCtx, k, contractAddress, wasmMsg) gasConsumed := tmpCtx.GasMeter().GasConsumed()