[Feature]: Allow failing messages to emit error-events

[nicolaslara]

Summary

This issue is to discuss an idea that would allow us to give better feedback to users that interact with cosmwasm contracts (ref: CosmWasm/wasmd#1122)

When a message fails, only the anteEvents are provided in the response. This feature would allow modules to submit events of a certain type (error-events) that will be copied to the parent context when discarding a CacheContext.

Problem Definition

When a message fails only the ante handler events and the error message is returned to the user. We should be able to add context by emitting error-events. This is particularly useful for cosmwasm.

Wasmd allows contracts to initialte submessages and receive the reply. Since events are not part of concensus there are no determinism guarantees on them, which is why wasmd removes the events before sending the respons back to the caller contract. Error details and logs are also removed because of non-determinism concerns.

This makes it very difficult for contract to provide proper error information back to users. For example, if contract A calls contract B, and B fails in message validation (say, invalid sender). No error is provided back to contract A, and so that contract can only fail with a generic error: "Submessage X call failed". A specific case of this can be found in https://github.com/osmosis-labs/affiliate-swap.

Adding this feature would allow any module to provide context information to users in case of errors.

The tradeoff is having to ensure events are copied to the parent context when a CacheContext is discarded

Proposal

Give cache CacheContext a writeErrorEvents() (or similar) function that allows the caller to decide if those should be kept.

This would need to be properly handled in runTxs and and runMsgs in baseapp, but then each module can decide if they want to provide error-events for context or not.

[alexanderbez]

runMsgs operates on the invariant that *sdk.Result will be nil if any message fails, thus making it impossible to get the events. What is the concrete proposal here?

[nicolaslara]

My thought was that we would change the signature of runMsgs, runTxs, and CacheContext. It's a bit intrusive, but can't think if a cleaner way to do it (open to suggestions)

[alexanderbez]

Yeah, we would have to get rid of that non-nil Result invariant, which sort of breaks idiomatic Go.

[nicolaslara]

Yeah, we would have to get rid of that non-nil Result invariant, which sort of breaks idiomatic Go.

I didn't mean breaking the non-nil Result invariant. I meant something like:

result, errorEvents, err = app.runMsgs(runMsgCtx, msgs, mode)

Alternatively we could have a composite error type for this:

type ExecError struct {
    err          error
    events   []sdk.Event
    logs       []string
}

[alexanderbez]

Yeah we could explore something like that! Should be pretty concise. Want to open a PR?

[nicolaslara]

yeah, I can add a PR. Just wanted to validate the idea with the team first

[alexanderbez]

You have my blessing -- I'll review 🦀

[colin-axner]

Would it be possible to have a solution which allows the actor reading events to know that the events were emitted under the error situation, outside of implementing logic to deduce this information? Ideally, you'd be able to clearly distinguish between error events and success events emitted by the same transaction

[alexanderbez]

You could simply add another event/attribute, something like message.error=.... But again, you'll only get the error/events for the very first message that fails.