zoneconcierge: repurpose heartbeat mechanism to a generic function #233
Conversation
There was a problem hiding this comment.
LGTM! Only left a minor question. Please help me recall why we want heartbeat in the first place and why we do not need it now. Did I miss something?
Instead, we will make the relayer to solely update light clients for the phase-1 integration, without using any IBC channel or packet. The phase-1 integration will only require a CZ to establish an IBC connection with Babylon.
Just wanted to understand more about the context. Does it mean we will have a modified relayer that only does CZ -> Babylon updates?
| @@ -62,9 +59,9 @@ func (k Keeper) SendHeartbeatIBCPacket(ctx sdk.Context, channel channeltypes.Ide | |||
| // send packet | |||
| if err := k.ics4Wrapper.SendPacket(ctx, channelCap, packet); err != nil { | |||
| // Failed/timeout packet should not make the system crash | |||
| k.Logger(ctx).Error(fmt.Sprintf("failed to send heartbeat IBC packet to channel %v port %s: %v", destinationChannel, destinationPort, err)) | |||
| k.Logger(ctx).Error(fmt.Sprintf("failed to send IBC packet %v to channel %v port %s: %v", packet, destinationChannel, destinationPort, err)) | |||
There was a problem hiding this comment.
Will this pollute the log if the packet is very big?
There was a problem hiding this comment.
Agree. Changed to the sequence number that uniquely identifies a packet.
| } else { | ||
| k.Logger(ctx).Info(fmt.Sprintf("successfully sent heartbeat IBC packet to channel %v port %s", destinationChannel, destinationPort)) | ||
| k.Logger(ctx).Info(fmt.Sprintf("successfully sent IBC packet %v to channel %v port %s", packet, destinationChannel, destinationPort)) |
Good question, this was a long story 🤣 As we know, the relayer is lazy in the sense that it updates the light clients only when there exists an IBC packet. Previously, the integration was assumed to work in a way that the partner CZs are responsible for maintaining the relayers and establishing IBC channels. Since we have no control on how they will use the relayer, we provide the heartbeat mechanism to ease their jobs. When we started the integration process in the previous two weeks, we however ended up establishing everything by ourselves, partially due to the Thanksgiving long weekend in the US. The relayer provides a command Then you may remember that in the last engineering meeting, we thought the heartbeat packet mechanism costs extra complexity and increases cost for maintaining a relayer. Thus we decided to remove the heartbeat mechanism and repurposed the code for sending IBC packets (in preparation of phase-2 integration).
Yep exactly. Currently the demo is using a very simple (and dirty) script for IBC stuff. We hope to make it more robust in the future 🤣 while true
do
echo "Relaying headers between Babylon and $1..."
rly --home data/relayer tx update-clients $1
sleep 10
done |
|
I see. It makes sense. Thanks for answering my questions! |
Fixes BM-341
This PR removes the heartbeat packet functionality, and repurposes the
SendHeartbeatIBCPacketfunction to a generic function for sending any IBC packets.Instead, we will make the relayer to solely update light clients for the phase-1 integration, without using any IBC channel or packet. The phase-1 integration will only require a CZ to establish an IBC connection with Babylon.