Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support multiple packet data's in a single packet #7008

Open
3 tasks
colin-axner opened this issue Aug 1, 2024 · 1 comment
Open
3 tasks

Support multiple packet data's in a single packet #7008

colin-axner opened this issue Aug 1, 2024 · 1 comment
Labels
04-channel 05-port Issues concerns the 05-port submodule epic needs discussion Issues that need discussion before they can be worked on type: refactor Architecture, code or CI improvements that may or may not tackle technical debt.
Milestone

Comments

@colin-axner
Copy link
Contributor

Summary

Implement support for multiple packet data's to be delivered and executed within a single packet.

Problem Definition

IBC was initially designed with a single application per channel/packet. As creativity spurred, new ideas emerged to extend existing applications and demand grew for doing so in a backwards compatible fashion, so as not to lose the network effect of existing well established channels.

Two approaches to this were:

  • IBC middlewares
  • transfer memo

IBC middlewares allowed logic to be exected around the transfer (or other base applications) modules. Using "short-term" solutions, IBC middlewares could wrap the channel verison and acknowledgements with their own information. It could not submit its own packet data, which is why the transfer memo came into existence.

I think a number of issues have arisen:

  • lack of standardization for extending transfer (memo)
  • increased complexity (version/ack wrapping)
  • difficulty reasoning and wiring a middleware stack

Many of these issues are outlined in #6941

The port router refactor will aim to give ibc middlewares, which are actually standalone ibc apps, the same privledges as ibc apps. Primarily access to their own:

  • version
  • packet data
  • acknowledgement

In addition, there will be simple wiring to make this possible. This reduction in complexity already has many unspoken benefits.

Conceptually, multiple packet data's should allow for multiple application execution to occur with respect to a single packet. That is, a linear reduction in packets sent per actor at a given moment. If an actor wishes to do X actions at once, we can now reduce X actions into a single packet. The simple use case in my mind being transfer + incentivizing that transfer. ics29 + transfer is a single packet at the moment (with some extra "short-term" solutions, but it requries wrapping the transfer acknoweldgement and version which is a "hack" and is very problematic to maintain. The complexity reasoning here is quite high, so having a standard approach for this use, would be a great benefit!

In addition, when we modify the packet data api, we should:

  • support multiple encoding types per packet data
  • support declaration of the packet data type (version) in the packet data

These additions would allow us to remove channel handshakes and channel upgradability (reduction in complexity) as each packet data will indicate which application it is bound for, what version of the application it wishes to interact with and how it can be decoded.

Proposal

With the implementation of #6941, we should:

  • introduce a packet commitment structure to support multiple packet data's (spec requirement)
  • modify the Packet protobuf structure to support a multi-packetdata api

To keep things simple, the changes should be made with no inter-dependence between packet data's. That is, the packet data's should be executed in the same manner as if they were sent in different packets.

Inter-dependence introduces more complexity as it breaks from the systems current approach to packet processing. This will be addressed in #7003

As a starting point, I propose the following proto changes:

message PacketV2 {
  int64                     sequence            = 1;
  string                    source_port         = 2;
  string                    source_channel      = 3;
  string                    destination_port    = 4;
  string                    destination_channel = 5;
  repeated PacketData       data                = 6; // primary modification
  ibc.core.client.v1.Height timeout_height      = 7 [(gogoproto.nullable) = false];
  uint64                    timeout_timestamp   = 8;
}

message PacketData {
  string  portID  = 1; // or app_name, as you want
  Payload payload = 2;
}

message Payload {
  string type     = 1; // or version, as you want
  string encoding = 2;
  bytes  value    = 3;
}

Note: cross-compatibility is determined by the packet commitment structure, not by these protobuf types. While other ibc implementation may choose to use the same protobuf type, so long as they hash the packet commitment the same, these protobuf types are not necessary to be adopted and thus should be seen as part of the go api. The packet commitment structure is intentionally left to the spec to define.

There are two approaches to defining the application callback interface:

 OnRecvPacket(
    ctx context.Context,
    packetID types.PacketID, 
    payload api.Payload,  // api is the same as the existing exported directory, I choose this name as I believe longer term it'll be more relevant
    relayer sdk.AccAddress,
) api.RecvPacketResult // this type will be explained and outlined in a follow up issue

This approach gives an application access to the packetID and the payload for this application. (This requries core IBC storing full packets for async packets)

Applications can unmarshal their packet data's using only the payload information:

packetData, err := types.unmarshal(payload)

or

 OnRecvPacket(
    ctx context.Context,
    payload api.Payload, // payload for the application
    packet channeltypes.PacketV2 // full packet
    relayer sdk.AccAddress,
) api.RecvPacketResult

This approach gives an application access to the packetID and the entire packet.

Most applications should use the first interface. They won't need to iterate over the packet data's to find their own payload. Usage of the second interface might arise for special applications, such as callbacks, which is more of an integration module for contract developers. My first idea is to have two distinct interfaces and allow the application developer to decide which is most appropriate for their use case.

Backwards compatiblity

The unwrapped channel version should be set in the payload.type and payload.encoding should be left empty. In addition, the portID in the packetID should be overwritten with the app portID (as set in the packetData structure).

The mechanism for determining the packet commitment structure which supports multi packet data execution is a decision left for the specification.


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged/assigned
@colin-axner colin-axner added type: refactor Architecture, code or CI improvements that may or may not tackle technical debt. 04-channel 05-port Issues concerns the 05-port submodule labels Aug 1, 2024
@colin-axner colin-axner added the needs discussion Issues that need discussion before they can be worked on label Aug 1, 2024
@colin-axner
Copy link
Contributor Author

colin-axner commented Aug 12, 2024

From discussion with @nicolaslara: We should support the ability to indicate whether a packet data is being executed on source, destination, or both. This might be done via an enum, or by specifying the source port and destination port within the packet id (and if left empty indicates not intended for that side of the packet flow)

This would give first class support for one sided applications

@womensrights womensrights added this to the v10.0.0 milestone Aug 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
04-channel 05-port Issues concerns the 05-port submodule epic needs discussion Issues that need discussion before they can be worked on type: refactor Architecture, code or CI improvements that may or may not tackle technical debt.
Projects
Status: In progress 👷
Development

No branches or pull requests

3 participants