Draft broadcast channel abstraction #912
Conversation
There was a problem hiding this comment.
Great work @angbrav , it's nice and clean! I think we could also need to define a callback interface similar to the one in ICS26. That way, application modules can interact with the core broadcast channel through callbacks
spec/core/ics-004-channel-and-packet-semantics/README-pubsub.md
Outdated
Show resolved
Hide resolved
| We pass the address of the `relayer` that signed and submitted the packet to enable a module to optionally provide some rewards. This provides a foundation for fee payment, but can be used for other techniques as well (like calculating a leaderboard). | ||
|
|
||
| ```typescript | ||
| function deliverPacket( |
There was a problem hiding this comment.
Why call this deliverPacket?
There was a problem hiding this comment.
I was just matching the standard API of broadcast primitives. In the literature, a broadcast service allows a process to send an application message m using a call broadcast(m). Messages are delivered using a notification deliver(m). We could use something else, for instance recvBroadcastPacket
There was a problem hiding this comment.
Hmm I guess its a question of whether we stick with broadcast terminology or IBC terminology. I do prefer standardizing on IBC terminology
Shouldn't we add it to ICS26 directly? |
There was a problem hiding this comment.
Thank your for starting this spec, @angbrav! I left a couple of comments and I have some questions:
- Since we don't have a 4-way handshake, doesn't seem like it will be possible to do versions negotiation? Would that be a problem at some point?
- If the broadcaster closes the channel, would the idea be that an event will be emitted as result and the relayer can pick that up and then close the channel on the subscribers as well?
- If a broadcaster opens a channel and subscribers subscribe to it and receive packet, what happens when a new subscriber joins later on? If I understand it correctly, the subscriber will initialize the next sequence recv to 1, but the broadcaster might already have sent several packets and is using a sequence number > 1...
| order: ChannelOrder, | ||
| portIdentifier: Identifier, | ||
| version: string): CapabilityKey { | ||
| abortTransactionUnless(order !== ORDERED_ALLOW_TIMEOUT) |
There was a problem hiding this comment.
From reading the spec I infer that it will not be possible to time out packets? If that's the case, would it be possible to just use the regular ordered channel? Or maybe have a new broadcast channel type?
There was a problem hiding this comment.
The thing here is that the broadcaster does not expect any other chain to receive it, so timeouts do not apply: it could actually open it and start broadcasting packets before any other chain subscribes.
A broadcast channel can be ordered or unordered, so we could not just use the regular ordered channel or a new type.
spec/core/ics-004-channel-and-packet-semantics/README-pubsub.md
Outdated
Show resolved
Hide resolved
| )) | ||
|
|
||
| switch channel.order { | ||
| case ORDERED: |
There was a problem hiding this comment.
Should it be ordered allow timeout?
| case ORDERED: | |
| case ORDERED_ALLOW_TIMEOUT: |
There was a problem hiding this comment.
I can see where the confusion comes from. I thought it is simpler to allow bcast channels to be either ORDERED or UNORDERED, as those are simpler terms than ORDERED_ALLOW_TIMEOUT. Nevertheless, I understand that semantically anORDERED bcast channel is closer to an ORDERED_ALLOW_TIMEOUT normal channel.
If it is up to me, I will leave as it is, but if you and @AdityaSripal think differently, I can change it.
There was a problem hiding this comment.
Think the binary choice of ORDERED and UNORDERED makes more sense. Since timeouts aren't even relevant here.
But maybe we can treat both ORDERED types the same way
There was a problem hiding this comment.
Ok, I think I am confused by the fact that in broadcastChanOpen the order is checked to be ORDERED_ALLOW_TIMEOUT and the channel end in the subscriber will also be of order ORDERED_ALLOW_TIMEOUT, right? But then when checking the channel order in deliverPacket I see ORDERED and UNORDERED... If I understand it correctly now, is the idea that the channel can be created as ORDERED or UNORDERED?
Co-authored-by: Carlos Rodriguez <carlos@interchain.io>
Thanks for the questions!
I do not think it is a problem. This is a one-to-many communication primitive, I do not think there can be a negotiation. In this case, the broadcaster should just pick a version.
I could do that. What do you think @AdityaSripal?
This is a good question. I was thinking that the relayer should relay all packets with sequence number >= 1 |
Yes but add it in a separate document since not all modules will have to implement this interface
Yes I think we should have a simple closing handshake
This is a good question. In the unordered case we don't need to worry about this. In the ORDERED case, I could see it working in a couple ways. Either we enforce all the sequences to be sent. Or we can initialize the subscriber with a starting sequence |
Yes, I agree with the second approach; I think it makes more sense. Another question: will it be possible to have incentivized broadcast channels? If so, how will that work? |
Yes, this would be done on the subscriber side. This is out of scope for this spec |
WIP
Rendered
Broadcast channel abstraction.