Let contract fully abort IBC receive in certain case

[alpe]

Placeholder

To be refined better by @ethanfrey

Background

In IBC-go:
OnRecvPacket returns an Ack object that can either indicate success or failure of the operation. State is only persisted for the success case.
Returning nil would persist state but not store the Acknowledgment.
The forth case is panic which would not persist state nor store an Ack.

	cacheCtx, writeFn = ctx.CacheContext()
	ack := cbs.OnRecvPacket(cacheCtx, msg.Packet, relayer)
	if ack == nil || ack.Success() {
		// write application state changes for asynchronous and successful acknowledgements
		writeFn()
	} else {
		// NOTE: The context returned by CacheContext() refers to a new EventManager, so it needs to explicitly set events to the original context.
		// Events should still be emitted from failed acks and asynchronous acks
		ctx.EventManager().EmitEvents(cacheCtx.EventManager().Events())
	}

	// Set packet acknowledgement only if the acknowledgement is not nil.
	// NOTE: IBC applications modules may call the WriteAcknowledgement asynchronously if the
	// acknowledgement is nil.
	if ack != nil {
		if err := k.ChannelKeeper.WriteAcknowledgement(ctx, cap, msg.Packet, ack); err != nil {
			return nil, err
		}
	}

in modules/core/keeper/msg_server.go

[ethanfrey]

Great summary of the choices.

Write now, we convert Result::Ok to persist state and return ack
And Result::Err to don't persist state and create an error ack to return.
Contract panic'ing is handled like Result::Err (which is usually correct)

I want another state which will abort the whole tx (including storing the ibc ack).
This would be something like Result::Err(SomeSpecialErrorType).

This is only used in special cases, like a time delay, where the packet fails now, but should be retried later when it would succeed. It should require a very explicit opting in (not just a panic), but some way to handle this.

As it should abort the whole tx and not be caught in some other ibc level, I would suggest the Go code panic in this case. My main question is what we use to trigger it?

My rough thought was any error that started ABORT IBC TX would trigger it. It is very specific opt-in, but matching on error strings is frowned upon. I'd love a better idea.

[alpe]

My main question is what we use to trigger it?

I think a dedicated error type would be the cleanest solution. Not sure how quick we can get this released.

[ethanfrey]

Let's move this to 0.32 and think this through more

[ethanfrey]

Okay, I made changes to CosmWasm in the above link.

The following approach would leave CosmWasm as is, require returning wasmd so that it doesn't catch/handle errors from a failed ibc_receive_packet (they should abort tx, so no ack is written). Not sure how this plays with the IBC stack, but it is possible for the contract to do state management itself. Albeit significantly more boilerplate for the contract and harder to test.

confio/cw-ibc-demo#20

[alpe]

After discussion with @webmaster128 and @ethanfrey, the plan is to panic in relay.go to revert all state when an error is returned by the vm.
This is a simple and not very inversive change. Although this may cause IBC packages to contracts that "panic" to rather timeout than return an error ack. We also discussed that this can be hard to debug.

Relevant discussions in the past: #697 (comment)
We are talking about scenario 4 here.