ibc icon indicating copy to clipboard operation
ibc copied to clipboard

Allow `OnChanOpenTry` version negotiation to call `WriteOpenTryChannel` in a future block

Open michaelfig opened this issue 3 years ago • 4 comments
trafficstars

Summary

Allow OnChanOpenTry both to write the new state and publish the event in a future block.

Problem Definition

On the Agoric chain, we're using async messaging to implement smart contracts. This works fine with IBC and async acknowledgements, but the ibc-go version negotiation is unfortunately synchronous. We'd like to allow Agoric users to implement channel version negotiation in a smart contract, but the synchronous nature defeats that goal.

Benefits of sorting this out would be to reduce cross-chain synchronous coupling, which may be an issue for other chains with similar async requirements.

Disadvantage is the need to reassess the IBC standard to lift the requirement for synchronous behaviour.

Proposal

This is a strawman.

Without changing the API presented to synchronous OnChanOpenTry implementations, we can define a constant Error that indicates the need for postponed async operation.

  1. return "", types.MagicalAsyncError from OnChanOpenTry
  2. In modules/core/keeper/msg_server.go ChannelOpenTry, make this modification:
 	// Perform application logic callback
	version, err := cbs.OnChanOpenTry(ctx, msg.Channel.Ordering, msg.Channel.ConnectionHops, msg.PortId, channelID, cap, msg.Channel.Counterparty, msg.CounterpartyVersion)
        if err == types.MagicalAsyncError {
                // Bail out to have a future block write the state
                return &channeltypes.MsgChannelOpenTryResponse{}, nil
        }
  1. Call k.ChannelKeeper.WriteOpenTryChannel explicitly at a later time.

We would also need some way to write a failed open try into the state as well, to interrupt the handshake if no version negotiation can be successfully completed.


For Admin Use

  • [ ] Not duplicate issue
  • [ ] Appropriate labels applied
  • [ ] Appropriate contributors tagged/assigned

michaelfig avatar Mar 22 '22 20:03 michaelfig

Tagging @AdityaSripal for comment and any counterproposal.

Tagging @schnetzlerjoe as somebody negatively affected by the current synchronous situation.

michaelfig avatar Mar 22 '22 21:03 michaelfig

I anticipated something like this when proposing passive relayers. For contrast, here is the solution I used then: https://github.com/cosmos/ibc-go/blob/main/docs/architecture/adr-025-ibc-passive-channels.md#handling-channel-open-attempts

michaelfig avatar Mar 23 '22 20:03 michaelfig

cc: @mpoke

@schnetzlerjoe could you detail how the synchronous situation is affecting you?

AdityaSripal avatar May 03 '22 15:05 AdityaSripal

@schnetzlerjoe could you detail how the synchronous situation is affecting you?

Speaking on behalf of @schnetzlerjoe since I was working with him, lack of async version negotiation makes it impossible to implement a dIBC (async-only) Interchain Accounts host. We just avoided doing that, as implementing an ICA controller only creates outbound connections to hosts, thus the controller doesn't need to handle OnChanOpenTry.

What I'm concerned about is that ICA hosts (or some future protocol) will require version negotiation, and that will make it impossible to implement in dIBC. That will be a blocker for some IBC applications on Agoric, we just aren't feeling the pain yet of being blocked from creating an ICA host.

michaelfig avatar May 03 '22 16:05 michaelfig