zoneconcierge: heartbeat IBC packet to keep relayer awake #195
Conversation
SebastianElvis
changed the title
There was a problem hiding this comment.
Awesome! You must have done a lot of investigation to make this PR done. Now I understand that the relayer will not relay headers if there are no pending IBC packets so we have to add some dummy messages in the channel periodically. This looks strange to me. If there are no IBC packets pending for a long time, wouldn't there be a lot of headers pending as well?
I would appreciate it if you could provide more context about why we need to put some TAO logic of ibc in zoneconcierge (i.e., setting up the channels and stuff). Can't we just create heartbeat messages and hand them over to ibc?
Which part of this PR is about TAO logic? I guess you mean the interfaces in You are right that we are not interested in injecting any logic into these interfaces since ZoneConcierge just needs to send heartbeat packets periodically to channels. In fact, this requires the ZoneConcierge to handle acknowledgements of these heartbeat messages, which require
That's right. The problem is that in order to use this functionality (specifically, function |
There was a problem hiding this comment.
Thanks for your explanation and the link. I think I did miss something. So basically, for any module that wants to send IBC packets needs to implement the IBCModule interface in order to set up channels and process Ack messages.
This PR LGTM! Only one question for the test. Do we need to add asserts about checking whether the headers are indeed updated in the clients?
| suite.True(found) | ||
|
|
||
| // Assert the gap between two sequence numbers | ||
| // Note that CommitBlock triggers 2 times of BeginBlock |
There was a problem hiding this comment.
Could you please explain why 2 times?
There was a problem hiding this comment.
Do we need to add asserts about checking whether the headers are indeed updated in the clients?
This is already tested by our forked IBC-Go and Babylon's test cases here. We could test this again here but it might be out of the scope of this test, which focuses on the heartbeat messages.
Could you please explain why 2 times?
I believe there is a bug (or maybe feature) in the ibctesting's CommitBlock function. It triggers BeginBlock twice rather than once. To make the test more clear without affecting the functionality, I moved the heartbeat logic into EndBlock. Now the sequence number gap is exactly the same as the number of heartbeat msgs sent.
There was a problem hiding this comment.
I see, maybe we can report this issue to the cosmos team 😄
Fixes BM-277
This PR implements the functionality of sending heartbeat IBC packets to the CZs upon each
BeginBlock. Such heartbeat packet is needed because the relayer relays headers and IBC packets only when there exists pending packets. Along the way, this PR also removes the restrictions on the port/version when establishing IBC channels, since ZoneConcierge is intended to serve any CZ without asking CZ to change its config.This PR also comes with a unit test that establishes an IBC channel between Babylon and CZ, triggers some heartbeat packets, and checks the consistency.