CosmWasm / wasmd

Basic cosmos-sdk app with web assembly smart contracts
Other
369 stars 407 forks source link

SmartQuery errors are redacted #1160

Open apollo-sturdy opened 1 year ago

apollo-sturdy commented 1 year ago

Currently if you execute a contract that does a SmartQuery to another contract and that query fails, the error message is redacted and you instead only receive codespace: wasm, code: 9, which makes it very difficult to figure out why the query failed, or even which query failed if the contract execution is complicated (could be any query in any other contract that the original contract calls). We should figure out a way to forward these errors while avoiding non-determinism issues discussed in #759.

See below of more context:

          @ethanfrey Can we get some more insight on what kind of non-determinism this is trying to avoid? I have been spending the past several days trying to track down a smart contract bug, and it would have taken only minutes if the contract error from a SmartQuery were forwarded all the way to the transaction rather than being redacted to `codespace: wasm, code: 9`. If at all possible, I think fixing the propagation of simple SmartQuery errors would be the single most high impact fix for CosmWasm developer quality of life. I only did brief reading of the link in the first post here, but it seems that the non-determinism has to do with Interchain Accounts, so could regular smart contract query errors be passed on without issues? If not, could we at least make it so that the address of the queried contract is included or even the QueryMsg? Even this would speed up debugging by orders of magnitude.

Originally posted by @apollo-sturdy in https://github.com/CosmWasm/wasmd/issues/759#issuecomment-1386927341

apollo-sturdy commented 1 year ago

Please let me know if you have any ideas on how we can work around the non-determinism. As @webmaster128 mentioned here:

And when we know for sure the error is coming from a different CosmWasm contract instead of Cosmos SDK we can create an exception and pass on the error.

I'm happy to help with a PR, but as I'm not too familiar with the codebase I'd appreciate if you guys can help decide on a good course of action. I suppose something like creating a type for Errors instead of simply passing them as a string could be an option, but unsure what you would prefer.

webmaster128 commented 1 year ago

The error code 9 (ErrQueryFailed) is created here:

    env := types.NewEnv(ctx, contractAddr)
    queryResult, gasUsed, qErr := k.wasmVM.Query(codeInfo.CodeHash, env, req, prefixStore, cosmwasmAPI, querier, k.gasMeter(ctx), k.runtimeGasForContract(ctx), costJSONDeserialization)
    k.consumeRuntimeGas(ctx, gasUsed)
    if qErr != nil {
        return nil, sdkerrors.Wrap(types.ErrQueryFailed, qErr.Error())
    }

That means ANYTHING that can go wrong in api.Query and wasmVM.Query is stringified and ends up here. This includes

I doubt that we can guarantee determinism across all those components, especially with the Go standard library being out of our control.

I think we first need to differentiate between errors that are coming directly from the contract and those that are created in the host as a contract's output should be safe to use.

alpe commented 1 year ago

I can see that this is a big pain point in developing contracts. As @webmaster128 pointed out, there are several places that we can not control to be deterministic. Therefore redacting the errors was the only option to provide guarantees. Nevertheless it is possible to add more debug output to the logger when you run a local node. Does this help with your concrete problem already? With remote networks it is a different story as the logger output is not shared. On which network do you have the issues?

alpe commented 1 year ago

fyi: https://medium.com/cosmwasm/presenting-smart-tools-for-smart-contracts-and-blockchains-2ac380304b24 I don't have an idea when these tools will be available to the public but just linking the article here to show progress on the problem