removed reseting of gas meter each time we enter a query

[reuvenpo]

This PR removes a line that was reseting the gas meter for each query, which meant that recursive queries would not report any gas usage, since it would be charged on a new meter, which the caller has no access to.
We now observe that gas is charged for external queries.
I expect some tests will fail after this first commit. Let;s see if they do. Are there tests that run such contract-to-contract queries?

[ethanfrey]

@reuvenpo thank you for bringing this forward.

I am working on CosmWasm/cosmwasm#494 to add such test potential to our standard test contract (your recursion idea plus a variable gas burning - sha256 hash loop). That is on 0.10.0 and we need to update go-cosmwasm CosmWasm/wasmvm#123 before we can use it.

I saw this last night after you pointed the bug. We need a different entry point for the external smart queries than the internal keeper. OR we do... but it is there at the external query handler that we need to set the gas limit, not in the keeper.

I built the contract to demo this issue. Unless you are really blocked by this, I would like to update everything to 0.10.0-alpha and get the test contract here with failing test cases before we start fixing bugs. Best way to ensure we squash them and there are no regressions

[codecov]

Codecov Report

Merging #216 into master will decrease coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #216      +/-   ##
==========================================
- Coverage   72.02%   72.01%   -0.02%     
==========================================
  Files          27       27              
  Lines        2624     2623       -1     
==========================================
- Hits         1890     1889       -1     
  Misses        622      622              
  Partials      112      112              

Impacted Files	Coverage Δ
x/wasm/internal/keeper/keeper.go	91.53% <ø> (-0.03%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 04cfcdc...d5b472f. Read the comment docs.

[assafmo]

I don't think there's a need for different entry points. I guess up until now queries didn't charge any gas and no one cared?

[reuvenpo]

We are not blocked by this. These last few PRs have been backports of changes that we have made in our repo, back to yours. We're fixing these bugs on our side, so we don't even need to pull in the full set of updates from your side yet.

[assafmo]

This needs a bit of fixing. Copying the TG messages in here as well.

@ethansf Sorry for my confusion!
I believe QuerySmart should receive an additional isExternalQuery bool param.
If it's false then do ctx = ctx.WithGasMeter(sdk.NewGasMeter(k.queryGasLimit))
If it's true, it means the used gas from this query should be charged from the ctx of the caller
If it's false, we should use .WithGasMeter in order to prevent query spam/dos
And queryGasLimit should be taken from .wasmd/config/config.toml

[ethanfrey]

I just merged #218 that expos ees 0.10 cosmwasn here and also the new gas metering. And the recursive query for testing.

I will look at this soon, tests first

[ethanfrey]

Thanks for finding the issue and pushing a patch.

I fixed this in #226 while still enforcing the limit on external queries. And adding a number of tests to ensure it works.

[assafmo]

Cool 👍