Add limiting wasm smart query decorator #668
Conversation
Codecov Report
@@ Coverage Diff @@
## master #668 +/- ##
=======================================
Coverage 60.10% 60.11%
=======================================
Files 48 48
Lines 5347 5363 +16
=======================================
+ Hits 3214 3224 +10
- Misses 1904 1910 +6
Partials 229 229
|
| // LimitedDepthWasmQuerier decorator to WasmQuerier to limit the depth of recursive calls | ||
| func LimitedDepthWasmQuerier(maxDepth uint32, next func(ctx sdk.Context, request *wasmvmtypes.WasmQuery) ([]byte, error)) func(ctx sdk.Context, request *wasmvmtypes.WasmQuery) ([]byte, error) { | ||
| return func(ctx sdk.Context, request *wasmvmtypes.WasmQuery) ([]byte, error) { | ||
| counter, ok := types.QueryDepthCounter(ctx) |
There was a problem hiding this comment.
ah, storing this in context is nice to handle this over multiple execution paths.
but what are the requirements / expected behaviour? can you document that first?
e.g. If there is a limit of 10 depths, and I dispatch 3 wasm messages and each make 4 queries, I do believe this will trigger the issue, even if not nested.
But... I am not really sure where these contexts will get unwrapped, so maybe this only will trigger when Contract 1 -> contract 2 -> ... Contract 11 and not multiple messages using the querier one after another.
| @@ -309,5 +312,6 @@ func DefaultWasmConfig() WasmConfig { | |||
| SmartQueryGasLimit: defaultQueryGasLimit, | |||
| MemoryCacheSize: defaultMemoryCacheSize, | |||
| ContractDebugMode: defaultContractDebugMode, | |||
| SmartQueryDepth: defaultSmartQueryDepth, | |||
There was a problem hiding this comment.
This can be overridden by genesis? Or per machine?
SmartQueryGasLimit: defaultQueryGasLimit,
MemoryCacheSize: defaultMemoryCacheSize,
ContractDebugMode: defaultContractDebugMode,
should not be consensus breaking, in that different nodes may have different values and they will handle external queries differently, have different performance profiles, etc but will return the same result for all transactions.
SmartQueryDepth will actually abort transactions and could cause consensus issues if different on different validator nodes.
Either we only allow setting it in the Go binary, or we set it in genesis (as x/params?). WasmConfig was read from a local toml file, right?
|
Not the right solution |
🚧 DRAFT - untested
Resolves #655