From 3e9d208300db3f99ee7ff248b96384077b47adf6 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Tue, 28 Jul 2020 10:49:02 +0200 Subject: [PATCH 1/7] Clean up recursive query tests --- x/wasm/internal/keeper/recurse_test.go | 95 +++++++++++--------------- 1 file changed, 38 insertions(+), 57 deletions(-) diff --git a/x/wasm/internal/keeper/recurse_test.go b/x/wasm/internal/keeper/recurse_test.go index fc9f55dcd..680ae407f 100644 --- a/x/wasm/internal/keeper/recurse_test.go +++ b/x/wasm/internal/keeper/recurse_test.go @@ -2,7 +2,6 @@ package keeper import ( "encoding/json" - abci "github.com/tendermint/tendermint/abci/types" "io/ioutil" "os" "testing" @@ -11,6 +10,7 @@ import ( "github.com/stretchr/testify/require" sdk "github.com/cosmos/cosmos-sdk/types" + abci "github.com/tendermint/tendermint/abci/types" ) type Recurse struct { @@ -34,6 +34,38 @@ type recurseResponse struct { Hashed []byte `json:"hashed"` } +func initRecurseContract(t *testing.T) (contract sdk.AccAddress, creator sdk.AccAddress, ctx sdk.Context, keeper Keeper, cleanup func()) { + // we do one basic setup before all test cases (which are read-only and don't change state) + tempDir, err := ioutil.TempDir("", "wasm") + require.NoError(t, err) + cleanup = func() { os.RemoveAll(tempDir) } + + ctx, keepers := CreateTestInput(t, false, tempDir, SupportedFeatures, nil, nil) + accKeeper, keeper := keepers.AccountKeeper, keepers.WasmKeeper + deposit := sdk.NewCoins(sdk.NewInt64Coin("denom", 100000)) + creator = createFakeFundedAccount(ctx, accKeeper, deposit.Add(deposit...)) + + // store the code + wasmCode, err := ioutil.ReadFile("./testdata/contract.wasm") + require.NoError(t, err) + codeID, err := keeper.Create(ctx, creator, wasmCode, "", "", nil) + require.NoError(t, err) + + // instantiate the contract + _, _, bob := keyPubAddr() + _, _, fred := keyPubAddr() + initMsg := InitMsg{ + Verifier: fred, + Beneficiary: bob, + } + initMsgBz, err := json.Marshal(initMsg) + require.NoError(t, err) + contractAddr, err := keeper.Instantiate(ctx, codeID, creator, nil, initMsgBz, "recursive contract", deposit) + require.NoError(t, err) + + return contractAddr, creator, ctx, keeper, cleanup +} + func TestGasCostOnQuery(t *testing.T) { const ( GasNoWork uint64 = InstanceCost + 2_756 @@ -87,33 +119,8 @@ func TestGasCostOnQuery(t *testing.T) { }, } - // we do one basic setup before all test cases (which are read-only and don't change state) - tempDir, err := ioutil.TempDir("", "wasm") - require.NoError(t, err) - defer os.RemoveAll(tempDir) - - ctx, keepers := CreateTestInput(t, false, tempDir, SupportedFeatures, nil, nil) - accKeeper, keeper := keepers.AccountKeeper, keepers.WasmKeeper - deposit := sdk.NewCoins(sdk.NewInt64Coin("denom", 100000)) - creator := createFakeFundedAccount(ctx, accKeeper, deposit.Add(deposit...)) - - // store the code - wasmCode, err := ioutil.ReadFile("./testdata/contract.wasm") - require.NoError(t, err) - codeID, err := keeper.Create(ctx, creator, wasmCode, "", "", nil) - require.NoError(t, err) - - // instantiate the contract - _, _, bob := keyPubAddr() - _, _, fred := keyPubAddr() - initMsg := InitMsg{ - Verifier: fred, - Beneficiary: bob, - } - initMsgBz, err := json.Marshal(initMsg) - require.NoError(t, err) - contractAddr, err := keeper.Instantiate(ctx, codeID, creator, nil, initMsgBz, "recursive contract", deposit) - require.NoError(t, err) + contractAddr, creator, ctx, keeper, cleanup := initRecurseContract(t) + defer cleanup() for name, tc := range cases { t.Run(name, func(t *testing.T) { @@ -149,8 +156,7 @@ func TestGasCostOnQuery(t *testing.T) { func TestGasOnExternalQuery(t *testing.T) { const ( - GasWork50 uint64 = InstanceCost + 8_464 - GasReturnHashed uint64 = 597 + GasWork50 uint64 = InstanceCost + 8_464 ) cases := map[string]struct { @@ -190,33 +196,8 @@ func TestGasOnExternalQuery(t *testing.T) { }, } - // we do one basic setup before all test cases (which are read-only and don't change state) - tempDir, err := ioutil.TempDir("", "wasm") - require.NoError(t, err) - defer os.RemoveAll(tempDir) - - ctx, keepers := CreateTestInput(t, false, tempDir, SupportedFeatures, nil, nil) - accKeeper, keeper := keepers.AccountKeeper, keepers.WasmKeeper - deposit := sdk.NewCoins(sdk.NewInt64Coin("denom", 100000)) - creator := createFakeFundedAccount(ctx, accKeeper, deposit.Add(deposit...)) - - // store the code - wasmCode, err := ioutil.ReadFile("./testdata/contract.wasm") - require.NoError(t, err) - codeID, err := keeper.Create(ctx, creator, wasmCode, "", "", nil) - require.NoError(t, err) - - // instantiate the contract - _, _, bob := keyPubAddr() - _, _, fred := keyPubAddr() - initMsg := InitMsg{ - Verifier: fred, - Beneficiary: bob, - } - initMsgBz, err := json.Marshal(initMsg) - require.NoError(t, err) - contractAddr, err := keeper.Instantiate(ctx, codeID, creator, nil, initMsgBz, "recursive contract", deposit) - require.NoError(t, err) + contractAddr, _, ctx, keeper, cleanup := initRecurseContract(t) + defer cleanup() for name, tc := range cases { t.Run(name, func(t *testing.T) { From 4729de2481f7a0288a272326421fa8b3090f7044 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Tue, 28 Jul 2020 11:09:11 +0200 Subject: [PATCH 2/7] Add deep recursion with cpu burn test; --- x/wasm/internal/keeper/recurse_test.go | 85 ++++++++++++++++++++++++++ 1 file changed, 85 insertions(+) diff --git a/x/wasm/internal/keeper/recurse_test.go b/x/wasm/internal/keeper/recurse_test.go index 680ae407f..1042c4f4d 100644 --- a/x/wasm/internal/keeper/recurse_test.go +++ b/x/wasm/internal/keeper/recurse_test.go @@ -224,3 +224,88 @@ func TestGasOnExternalQuery(t *testing.T) { }) } } + +func TestLimitRecursiveQueryGas(t *testing.T) { + // The point of this test from https://github.com/CosmWasm/cosmwasm/issues/456 + // Basically, if I burn 90% of gas in CPU loop, then query out (to my self) + // the sub-query will have all the original gas (minus the 40k instance charge) + // and can burn 90% and call a sub-contract again... + // This attack would allow us to use far more than the provided gas before + // eventually hitting an OutOfGas panic. + + const ( + // Note: about 100 SDK gas (10k wasmer gas) for each round of sha256 + GasWork2k uint64 = InstanceCost + 233_379 // we have 6x gas used in cpu than in the instance + // This is overhead for calling into a sub-contract + GasReturnHashed uint64 = 603 + ) + + cases := map[string]struct { + gasLimit uint64 + msg Recurse + expectedGas uint64 + expectOutOfGas bool + }{ + "no recursion, lots of work": { + gasLimit: 4_000_000, + msg: Recurse{ + Depth: 0, + Work: 2000, + }, + expectedGas: GasWork2k, + }, + "recursion 5, lots of work": { + gasLimit: 4_000_000, + msg: Recurse{ + Depth: 5, + Work: 2000, + }, + expectedGas: GasWork2k + 5*(GasWork2k+GasReturnHashed), + }, + // this is where we expect an error... + // it has enough gas to run 4 times and die on the 5th + // however, if we don't charge the cpu gas before sub-dispatching, we can recurse over 20 times + // TODO: figure out how to asset how deep it went + "deep recursion, should die on 5th level": { + gasLimit: 1_200_000, + msg: Recurse{ + Depth: 5, + Work: 2000, + }, + expectOutOfGas: true, + }, + } + + contractAddr, _, ctx, keeper, cleanup := initRecurseContract(t) + defer cleanup() + + for name, tc := range cases { + t.Run(name, func(t *testing.T) { + // external limit has no effect (we get a panic if this is enforced) + keeper.queryGasLimit = 1000 + + // make sure we set a limit before calling + ctx = ctx.WithGasMeter(sdk.NewGasMeter(tc.gasLimit)) + require.Equal(t, uint64(0), ctx.GasMeter().GasConsumed()) + + // prepare the query + recurse := tc.msg + recurse.Contract = contractAddr + msg := buildQuery(t, recurse) + + // if we expect out of gas, make sure this panics + if tc.expectOutOfGas { + require.Panics(t, func() { + _, _ = keeper.QuerySmart(ctx, contractAddr, msg) + }) + return + } + + // otherwise, we expect a successful call + _, err := keeper.QuerySmart(ctx, contractAddr, msg) + require.NoError(t, err) + assert.Equal(t, tc.expectedGas, ctx.GasMeter().GasConsumed()) + + }) + } +} From 851a7a47a7684e593577853e50f845c69cdb992c Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Tue, 28 Jul 2020 13:32:48 +0200 Subject: [PATCH 3/7] Use global singleton counter, check call depth --- x/wasm/internal/keeper/keeper.go | 4 ++++ x/wasm/internal/keeper/recurse_test.go | 12 +++++++++--- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/x/wasm/internal/keeper/keeper.go b/x/wasm/internal/keeper/keeper.go index d9c150af7..35e202572 100644 --- a/x/wasm/internal/keeper/keeper.go +++ b/x/wasm/internal/keeper/keeper.go @@ -42,6 +42,9 @@ const InstanceCost uint64 = 40_000 // CompileCost is how much SDK gas we charge *per byte* for compiling WASM code. const CompileCost uint64 = 2 +// TODO: remove this - this is just for one test +var TimesQueryCalled int = 0 + // Keeper will have a reference to Wasmer with it's own data directory. type Keeper struct { storeKey sdk.StoreKey @@ -398,6 +401,7 @@ func (k Keeper) GetContractHistory(ctx sdk.Context, contractAddr sdk.AccAddress) // QuerySmart queries the smart contract itself. func (k Keeper) QuerySmart(ctx sdk.Context, contractAddr sdk.AccAddress, req []byte) ([]byte, error) { ctx.GasMeter().ConsumeGas(InstanceCost, "Loading CosmWasm module: query") + TimesQueryCalled++ codeInfo, prefixStore, err := k.contractInstance(ctx, contractAddr) if err != nil { diff --git a/x/wasm/internal/keeper/recurse_test.go b/x/wasm/internal/keeper/recurse_test.go index 1042c4f4d..3f4e86159 100644 --- a/x/wasm/internal/keeper/recurse_test.go +++ b/x/wasm/internal/keeper/recurse_test.go @@ -243,6 +243,7 @@ func TestLimitRecursiveQueryGas(t *testing.T) { cases := map[string]struct { gasLimit uint64 msg Recurse + expectCalls int expectedGas uint64 expectOutOfGas bool }{ @@ -252,6 +253,7 @@ func TestLimitRecursiveQueryGas(t *testing.T) { Depth: 0, Work: 2000, }, + expectCalls: 1, expectedGas: GasWork2k, }, "recursion 5, lots of work": { @@ -260,6 +262,7 @@ func TestLimitRecursiveQueryGas(t *testing.T) { Depth: 5, Work: 2000, }, + expectCalls: 6, expectedGas: GasWork2k + 5*(GasWork2k+GasReturnHashed), }, // this is where we expect an error... @@ -269,9 +272,10 @@ func TestLimitRecursiveQueryGas(t *testing.T) { "deep recursion, should die on 5th level": { gasLimit: 1_200_000, msg: Recurse{ - Depth: 5, + Depth: 50, Work: 2000, }, + expectCalls: 6, expectOutOfGas: true, }, } @@ -281,8 +285,8 @@ func TestLimitRecursiveQueryGas(t *testing.T) { for name, tc := range cases { t.Run(name, func(t *testing.T) { - // external limit has no effect (we get a panic if this is enforced) - keeper.queryGasLimit = 1000 + // reset the counter before test + TimesQueryCalled = 0 // make sure we set a limit before calling ctx = ctx.WithGasMeter(sdk.NewGasMeter(tc.gasLimit)) @@ -298,6 +302,7 @@ func TestLimitRecursiveQueryGas(t *testing.T) { require.Panics(t, func() { _, _ = keeper.QuerySmart(ctx, contractAddr, msg) }) + assert.Equal(t, tc.expectCalls, TimesQueryCalled) return } @@ -306,6 +311,7 @@ func TestLimitRecursiveQueryGas(t *testing.T) { require.NoError(t, err) assert.Equal(t, tc.expectedGas, ctx.GasMeter().GasConsumed()) + assert.Equal(t, tc.expectCalls, TimesQueryCalled) }) } } From 9734dfb3c99afc8544d3f294cd4e1d36205ca438 Mon Sep 17 00:00:00 2001 From: Alex Peters Date: Tue, 28 Jul 2020 14:03:49 +0200 Subject: [PATCH 4/7] Move counting singleton to test code --- x/wasm/internal/keeper/keeper.go | 4 --- x/wasm/internal/keeper/recurse_test.go | 44 +++++++++++++++++--------- 2 files changed, 29 insertions(+), 19 deletions(-) diff --git a/x/wasm/internal/keeper/keeper.go b/x/wasm/internal/keeper/keeper.go index 35e202572..d9c150af7 100644 --- a/x/wasm/internal/keeper/keeper.go +++ b/x/wasm/internal/keeper/keeper.go @@ -42,9 +42,6 @@ const InstanceCost uint64 = 40_000 // CompileCost is how much SDK gas we charge *per byte* for compiling WASM code. const CompileCost uint64 = 2 -// TODO: remove this - this is just for one test -var TimesQueryCalled int = 0 - // Keeper will have a reference to Wasmer with it's own data directory. type Keeper struct { storeKey sdk.StoreKey @@ -401,7 +398,6 @@ func (k Keeper) GetContractHistory(ctx sdk.Context, contractAddr sdk.AccAddress) // QuerySmart queries the smart contract itself. func (k Keeper) QuerySmart(ctx sdk.Context, contractAddr sdk.AccAddress, req []byte) ([]byte, error) { ctx.GasMeter().ConsumeGas(InstanceCost, "Loading CosmWasm module: query") - TimesQueryCalled++ codeInfo, prefixStore, err := k.contractInstance(ctx, contractAddr) if err != nil { diff --git a/x/wasm/internal/keeper/recurse_test.go b/x/wasm/internal/keeper/recurse_test.go index 3f4e86159..5067c467a 100644 --- a/x/wasm/internal/keeper/recurse_test.go +++ b/x/wasm/internal/keeper/recurse_test.go @@ -6,6 +6,7 @@ import ( "os" "testing" + wasmTypes "github.com/CosmWasm/go-cosmwasm/types" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -34,14 +35,27 @@ type recurseResponse struct { Hashed []byte `json:"hashed"` } + +// number os wasm queries called from a contract +var totalWasmQueryCounter int + func initRecurseContract(t *testing.T) (contract sdk.AccAddress, creator sdk.AccAddress, ctx sdk.Context, keeper Keeper, cleanup func()) { // we do one basic setup before all test cases (which are read-only and don't change state) tempDir, err := ioutil.TempDir("", "wasm") require.NoError(t, err) cleanup = func() { os.RemoveAll(tempDir) } - ctx, keepers := CreateTestInput(t, false, tempDir, SupportedFeatures, nil, nil) + var realWasmQuerier func(ctx sdk.Context, request *wasmTypes.WasmQuery) ([]byte, error) + countingQuerier := &QueryPlugins{ + Wasm: func(ctx sdk.Context, request *wasmTypes.WasmQuery) ([]byte, error) { + totalWasmQueryCounter++ + return realWasmQuerier(ctx, request) + }, + } + ctx, keepers := CreateTestInput(t, false, tempDir, SupportedFeatures, nil, countingQuerier) accKeeper, keeper := keepers.AccountKeeper, keepers.WasmKeeper + realWasmQuerier = WasmQuerier(&keeper) + deposit := sdk.NewCoins(sdk.NewInt64Coin("denom", 100000)) creator = createFakeFundedAccount(ctx, accKeeper, deposit.Add(deposit...)) @@ -241,11 +255,11 @@ func TestLimitRecursiveQueryGas(t *testing.T) { ) cases := map[string]struct { - gasLimit uint64 - msg Recurse - expectCalls int - expectedGas uint64 - expectOutOfGas bool + gasLimit uint64 + msg Recurse + expectQueriesFromContract int + expectedGas uint64 + expectOutOfGas bool }{ "no recursion, lots of work": { gasLimit: 4_000_000, @@ -253,8 +267,8 @@ func TestLimitRecursiveQueryGas(t *testing.T) { Depth: 0, Work: 2000, }, - expectCalls: 1, - expectedGas: GasWork2k, + expectQueriesFromContract: 0, + expectedGas: GasWork2k, }, "recursion 5, lots of work": { gasLimit: 4_000_000, @@ -262,8 +276,8 @@ func TestLimitRecursiveQueryGas(t *testing.T) { Depth: 5, Work: 2000, }, - expectCalls: 6, - expectedGas: GasWork2k + 5*(GasWork2k+GasReturnHashed), + expectQueriesFromContract: 5, + expectedGas: GasWork2k + 5*(GasWork2k+GasReturnHashed), }, // this is where we expect an error... // it has enough gas to run 4 times and die on the 5th @@ -275,8 +289,8 @@ func TestLimitRecursiveQueryGas(t *testing.T) { Depth: 50, Work: 2000, }, - expectCalls: 6, - expectOutOfGas: true, + expectQueriesFromContract: 5, + expectOutOfGas: true, }, } @@ -286,7 +300,7 @@ func TestLimitRecursiveQueryGas(t *testing.T) { for name, tc := range cases { t.Run(name, func(t *testing.T) { // reset the counter before test - TimesQueryCalled = 0 + totalWasmQueryCounter = 0 // make sure we set a limit before calling ctx = ctx.WithGasMeter(sdk.NewGasMeter(tc.gasLimit)) @@ -302,7 +316,7 @@ func TestLimitRecursiveQueryGas(t *testing.T) { require.Panics(t, func() { _, _ = keeper.QuerySmart(ctx, contractAddr, msg) }) - assert.Equal(t, tc.expectCalls, TimesQueryCalled) + assert.Equal(t, tc.expectQueriesFromContract, totalWasmQueryCounter) return } @@ -311,7 +325,7 @@ func TestLimitRecursiveQueryGas(t *testing.T) { require.NoError(t, err) assert.Equal(t, tc.expectedGas, ctx.GasMeter().GasConsumed()) - assert.Equal(t, tc.expectCalls, TimesQueryCalled) + assert.Equal(t, tc.expectQueriesFromContract, totalWasmQueryCounter) }) } } From b97c87212c79c781b88e88d0d77ff9a15ebe84e2 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Wed, 29 Jul 2020 21:06:38 +0200 Subject: [PATCH 5/7] Update go-cosmwasm and Use new gas limits --- go.mod | 2 +- go.sum | 2 ++ x/wasm/internal/keeper/query_plugins.go | 19 +++++++++++++------ x/wasm/internal/keeper/recurse_test.go | 3 ++- 4 files changed, 18 insertions(+), 8 deletions(-) diff --git a/go.mod b/go.mod index 92b829fc7..e09db0357 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,7 @@ module github.com/CosmWasm/wasmd go 1.13 require ( - github.com/CosmWasm/go-cosmwasm v0.10.0-alpha2 + github.com/CosmWasm/go-cosmwasm v0.10.0-alpha3 github.com/cosmos/cosmos-sdk v0.39.1-0.20200727135228-9d00f712e334 github.com/golang/mock v1.4.3 // indirect github.com/google/gofuzz v1.0.0 diff --git a/go.sum b/go.sum index 1d8cc7b1b..84f9ede19 100644 --- a/go.sum +++ b/go.sum @@ -11,6 +11,8 @@ github.com/ChainSafe/go-schnorrkel v0.0.0-20200405005733-88cbf1b4c40d h1:nalkkPQ github.com/ChainSafe/go-schnorrkel v0.0.0-20200405005733-88cbf1b4c40d/go.mod h1:URdX5+vg25ts3aCh8H5IFZybJYKWhJHYMTnf+ULtoC4= github.com/CosmWasm/go-cosmwasm v0.10.0-alpha2 h1:k069wQF0f24w3A52mjR3AK6AfkuvQ5d0Mdu/rpB5nEk= github.com/CosmWasm/go-cosmwasm v0.10.0-alpha2/go.mod h1:gAFCwllx97ejI+m9SqJQrmd2SBW7HA0fOjvWWJjM2uc= +github.com/CosmWasm/go-cosmwasm v0.10.0-alpha3 h1:+LJxN6oHNdHDoB9JT8v6Zh373MvBbFkKoikSnh4GGWM= +github.com/CosmWasm/go-cosmwasm v0.10.0-alpha3/go.mod h1:gAFCwllx97ejI+m9SqJQrmd2SBW7HA0fOjvWWJjM2uc= github.com/Knetic/govaluate v3.0.1-0.20171022003610-9aa49832a739+incompatible/go.mod h1:r7JcOSlj0wfOMncg0iLm8Leh48TZaKVeNIfJntJ2wa0= github.com/OneOfOne/xxhash v1.2.2/go.mod h1:HSdplMjZKSmBqAxg5vPj2TmRDmfkzw+cTzAElWljhcU= github.com/Shopify/sarama v1.19.0/go.mod h1:FVkBWblsNy7DGZRfXLU0O9RCGt5g3g3yEuWXgklEdEo= diff --git a/x/wasm/internal/keeper/query_plugins.go b/x/wasm/internal/keeper/query_plugins.go index e20e872b9..aae222ceb 100644 --- a/x/wasm/internal/keeper/query_plugins.go +++ b/x/wasm/internal/keeper/query_plugins.go @@ -2,6 +2,7 @@ package keeper import ( "encoding/json" + "fmt" wasmTypes "github.com/CosmWasm/go-cosmwasm/types" sdk "github.com/cosmos/cosmos-sdk/types" @@ -17,20 +18,26 @@ type QueryHandler struct { var _ wasmTypes.Querier = QueryHandler{} -func (q QueryHandler) Query(request wasmTypes.QueryRequest) ([]byte, error) { +func (q QueryHandler) Query(request wasmTypes.QueryRequest, gasLimit uint64) (res []byte, err error) { + sdkGas := gasLimit / GasMultiplier + fmt.Printf("Sdk gas %d, wasmer gas: %d\n", sdkGas, gasLimit) + subctx := q.Ctx.WithGasMeter(sdk.NewGasMeter(gasLimit / GasMultiplier)) + res, err = nil, wasmTypes.Unknown{} if request.Bank != nil { - return q.Plugins.Bank(q.Ctx, request.Bank) + res, err = q.Plugins.Bank(subctx, request.Bank) } if request.Custom != nil { - return q.Plugins.Custom(q.Ctx, request.Custom) + res, err = q.Plugins.Custom(subctx, request.Custom) } if request.Staking != nil { - return q.Plugins.Staking(q.Ctx, request.Staking) + res, err = q.Plugins.Staking(subctx, request.Staking) } if request.Wasm != nil { - return q.Plugins.Wasm(q.Ctx, request.Wasm) + res, err = q.Plugins.Wasm(subctx, request.Wasm) } - return nil, wasmTypes.Unknown{} + fmt.Printf("charged: %d\n", subctx.GasMeter().GasConsumed()) + q.Ctx.GasMeter().ConsumeGas(subctx.GasMeter().GasConsumed(), "contract sub-query") + return res, err } func (q QueryHandler) GasConsumed() uint64 { diff --git a/x/wasm/internal/keeper/recurse_test.go b/x/wasm/internal/keeper/recurse_test.go index 5067c467a..a8b569854 100644 --- a/x/wasm/internal/keeper/recurse_test.go +++ b/x/wasm/internal/keeper/recurse_test.go @@ -314,7 +314,8 @@ func TestLimitRecursiveQueryGas(t *testing.T) { // if we expect out of gas, make sure this panics if tc.expectOutOfGas { require.Panics(t, func() { - _, _ = keeper.QuerySmart(ctx, contractAddr, msg) + _, err := keeper.QuerySmart(ctx, contractAddr, msg) + t.Logf("Got error not panic: %#v", err) }) assert.Equal(t, tc.expectQueriesFromContract, totalWasmQueryCounter) return From ea5ae47ffdcb87d433d794b338b7df0d63f5de23 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Wed, 29 Jul 2020 22:08:34 +0200 Subject: [PATCH 6/7] Properly charge gas on sub-queries, tests pass --- go.mod | 2 +- go.sum | 2 ++ x/wasm/internal/keeper/query_plugins.go | 28 +++++++++++++------------ x/wasm/internal/keeper/recurse_test.go | 8 +++---- 4 files changed, 22 insertions(+), 18 deletions(-) diff --git a/go.mod b/go.mod index e09db0357..d53fab192 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,7 @@ module github.com/CosmWasm/wasmd go 1.13 require ( - github.com/CosmWasm/go-cosmwasm v0.10.0-alpha3 + github.com/CosmWasm/go-cosmwasm v0.10.0-alpha3.0.20200729195610-18b861d95ddb github.com/cosmos/cosmos-sdk v0.39.1-0.20200727135228-9d00f712e334 github.com/golang/mock v1.4.3 // indirect github.com/google/gofuzz v1.0.0 diff --git a/go.sum b/go.sum index 84f9ede19..6cc566f28 100644 --- a/go.sum +++ b/go.sum @@ -13,6 +13,8 @@ github.com/CosmWasm/go-cosmwasm v0.10.0-alpha2 h1:k069wQF0f24w3A52mjR3AK6AfkuvQ5 github.com/CosmWasm/go-cosmwasm v0.10.0-alpha2/go.mod h1:gAFCwllx97ejI+m9SqJQrmd2SBW7HA0fOjvWWJjM2uc= github.com/CosmWasm/go-cosmwasm v0.10.0-alpha3 h1:+LJxN6oHNdHDoB9JT8v6Zh373MvBbFkKoikSnh4GGWM= github.com/CosmWasm/go-cosmwasm v0.10.0-alpha3/go.mod h1:gAFCwllx97ejI+m9SqJQrmd2SBW7HA0fOjvWWJjM2uc= +github.com/CosmWasm/go-cosmwasm v0.10.0-alpha3.0.20200729195610-18b861d95ddb h1:JHeFb9Dle3c3RvXkxZbo6YURaS3aQf+7zdUn5P3TP04= +github.com/CosmWasm/go-cosmwasm v0.10.0-alpha3.0.20200729195610-18b861d95ddb/go.mod h1:gAFCwllx97ejI+m9SqJQrmd2SBW7HA0fOjvWWJjM2uc= github.com/Knetic/govaluate v3.0.1-0.20171022003610-9aa49832a739+incompatible/go.mod h1:r7JcOSlj0wfOMncg0iLm8Leh48TZaKVeNIfJntJ2wa0= github.com/OneOfOne/xxhash v1.2.2/go.mod h1:HSdplMjZKSmBqAxg5vPj2TmRDmfkzw+cTzAElWljhcU= github.com/Shopify/sarama v1.19.0/go.mod h1:FVkBWblsNy7DGZRfXLU0O9RCGt5g3g3yEuWXgklEdEo= diff --git a/x/wasm/internal/keeper/query_plugins.go b/x/wasm/internal/keeper/query_plugins.go index aae222ceb..bc76ca154 100644 --- a/x/wasm/internal/keeper/query_plugins.go +++ b/x/wasm/internal/keeper/query_plugins.go @@ -2,8 +2,6 @@ package keeper import ( "encoding/json" - "fmt" - wasmTypes "github.com/CosmWasm/go-cosmwasm/types" sdk "github.com/cosmos/cosmos-sdk/types" sdkerrors "github.com/cosmos/cosmos-sdk/types/errors" @@ -18,26 +16,30 @@ type QueryHandler struct { var _ wasmTypes.Querier = QueryHandler{} -func (q QueryHandler) Query(request wasmTypes.QueryRequest, gasLimit uint64) (res []byte, err error) { +func (q QueryHandler) Query(request wasmTypes.QueryRequest, gasLimit uint64) ([]byte, error) { + // set a limit for a subctx sdkGas := gasLimit / GasMultiplier - fmt.Printf("Sdk gas %d, wasmer gas: %d\n", sdkGas, gasLimit) - subctx := q.Ctx.WithGasMeter(sdk.NewGasMeter(gasLimit / GasMultiplier)) - res, err = nil, wasmTypes.Unknown{} + subctx := q.Ctx.WithGasMeter(sdk.NewGasMeter(sdkGas)) + + // make sure we charge the higher level context even on panic + defer func() { + q.Ctx.GasMeter().ConsumeGas(subctx.GasMeter().GasConsumed(), "contract sub-query") + }() + + // do the query if request.Bank != nil { - res, err = q.Plugins.Bank(subctx, request.Bank) + return q.Plugins.Bank(subctx, request.Bank) } if request.Custom != nil { - res, err = q.Plugins.Custom(subctx, request.Custom) + return q.Plugins.Custom(subctx, request.Custom) } if request.Staking != nil { - res, err = q.Plugins.Staking(subctx, request.Staking) + return q.Plugins.Staking(subctx, request.Staking) } if request.Wasm != nil { - res, err = q.Plugins.Wasm(subctx, request.Wasm) + return q.Plugins.Wasm(subctx, request.Wasm) } - fmt.Printf("charged: %d\n", subctx.GasMeter().GasConsumed()) - q.Ctx.GasMeter().ConsumeGas(subctx.GasMeter().GasConsumed(), "contract sub-query") - return res, err + return nil, wasmTypes.Unknown{} } func (q QueryHandler) GasConsumed() uint64 { diff --git a/x/wasm/internal/keeper/recurse_test.go b/x/wasm/internal/keeper/recurse_test.go index a8b569854..6d8884bc7 100644 --- a/x/wasm/internal/keeper/recurse_test.go +++ b/x/wasm/internal/keeper/recurse_test.go @@ -35,7 +35,6 @@ type recurseResponse struct { Hashed []byte `json:"hashed"` } - // number os wasm queries called from a contract var totalWasmQueryCounter int @@ -228,7 +227,8 @@ func TestGasOnExternalQuery(t *testing.T) { if tc.expectPanic { require.Panics(t, func() { // this should run out of gas - _, _ = NewQuerier(keeper)(ctx, path, req) + _, err := NewQuerier(keeper)(ctx, path, req) + t.Logf("%v", err) }) } else { // otherwise, make sure we get a good success @@ -280,7 +280,7 @@ func TestLimitRecursiveQueryGas(t *testing.T) { expectedGas: GasWork2k + 5*(GasWork2k+GasReturnHashed), }, // this is where we expect an error... - // it has enough gas to run 4 times and die on the 5th + // it has enough gas to run 4 times and die on the 5th (4th time dispatching to sub-contract) // however, if we don't charge the cpu gas before sub-dispatching, we can recurse over 20 times // TODO: figure out how to asset how deep it went "deep recursion, should die on 5th level": { @@ -289,7 +289,7 @@ func TestLimitRecursiveQueryGas(t *testing.T) { Depth: 50, Work: 2000, }, - expectQueriesFromContract: 5, + expectQueriesFromContract: 4, expectOutOfGas: true, }, } From d60095816f73f84452864acb3e55e62f42f157b4 Mon Sep 17 00:00:00 2001 From: Ethan Frey Date: Wed, 29 Jul 2020 22:37:00 +0200 Subject: [PATCH 7/7] Update go-cosmwasm to tagged release, not random commit --- go.mod | 2 +- go.sum | 8 ++------ 2 files changed, 3 insertions(+), 7 deletions(-) diff --git a/go.mod b/go.mod index d53fab192..415f49438 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,7 @@ module github.com/CosmWasm/wasmd go 1.13 require ( - github.com/CosmWasm/go-cosmwasm v0.10.0-alpha3.0.20200729195610-18b861d95ddb + github.com/CosmWasm/go-cosmwasm v0.10.0-alpha4 github.com/cosmos/cosmos-sdk v0.39.1-0.20200727135228-9d00f712e334 github.com/golang/mock v1.4.3 // indirect github.com/google/gofuzz v1.0.0 diff --git a/go.sum b/go.sum index 6cc566f28..3d7073067 100644 --- a/go.sum +++ b/go.sum @@ -9,12 +9,8 @@ github.com/BurntSushi/toml v0.3.1 h1:WXkYYl6Yr3qBf1K79EBnL4mak0OimBfB0XUf9Vl28OQ github.com/BurntSushi/toml v0.3.1/go.mod h1:xHWCNGjB5oqiDr8zfno3MHue2Ht5sIBksp03qcyfWMU= github.com/ChainSafe/go-schnorrkel v0.0.0-20200405005733-88cbf1b4c40d h1:nalkkPQcITbvhmL4+C4cKA87NW0tfm3Kl9VXRoPywFg= github.com/ChainSafe/go-schnorrkel v0.0.0-20200405005733-88cbf1b4c40d/go.mod h1:URdX5+vg25ts3aCh8H5IFZybJYKWhJHYMTnf+ULtoC4= -github.com/CosmWasm/go-cosmwasm v0.10.0-alpha2 h1:k069wQF0f24w3A52mjR3AK6AfkuvQ5d0Mdu/rpB5nEk= -github.com/CosmWasm/go-cosmwasm v0.10.0-alpha2/go.mod h1:gAFCwllx97ejI+m9SqJQrmd2SBW7HA0fOjvWWJjM2uc= -github.com/CosmWasm/go-cosmwasm v0.10.0-alpha3 h1:+LJxN6oHNdHDoB9JT8v6Zh373MvBbFkKoikSnh4GGWM= -github.com/CosmWasm/go-cosmwasm v0.10.0-alpha3/go.mod h1:gAFCwllx97ejI+m9SqJQrmd2SBW7HA0fOjvWWJjM2uc= -github.com/CosmWasm/go-cosmwasm v0.10.0-alpha3.0.20200729195610-18b861d95ddb h1:JHeFb9Dle3c3RvXkxZbo6YURaS3aQf+7zdUn5P3TP04= -github.com/CosmWasm/go-cosmwasm v0.10.0-alpha3.0.20200729195610-18b861d95ddb/go.mod h1:gAFCwllx97ejI+m9SqJQrmd2SBW7HA0fOjvWWJjM2uc= +github.com/CosmWasm/go-cosmwasm v0.10.0-alpha4 h1:1a3j/vdhnyYvUV+67Hg3GU87M9wn1jR6bReXDwy+TZQ= +github.com/CosmWasm/go-cosmwasm v0.10.0-alpha4/go.mod h1:gAFCwllx97ejI+m9SqJQrmd2SBW7HA0fOjvWWJjM2uc= github.com/Knetic/govaluate v3.0.1-0.20171022003610-9aa49832a739+incompatible/go.mod h1:r7JcOSlj0wfOMncg0iLm8Leh48TZaKVeNIfJntJ2wa0= github.com/OneOfOne/xxhash v1.2.2/go.mod h1:HSdplMjZKSmBqAxg5vPj2TmRDmfkzw+cTzAElWljhcU= github.com/Shopify/sarama v1.19.0/go.mod h1:FVkBWblsNy7DGZRfXLU0O9RCGt5g3g3yEuWXgklEdEo=