fix: only send client updates when necessary and when msg is properly…

… constructed

Previously in the relayer we were attempting to send a `MsgUpdateClient` when `needsClientUpdate` was true, as determined in the `shouldUpdateClientNow` method, without considering if the message was properly constructed in `assembleMsgUpdateClient`. This adds a check in `trackAndSendMessages` to ensure we only attempt to send the msg when a client update is necessary AND the `MsgUpdateClient` was properly constructed.

relayer/processor/message_processor.go

@@ -100,7 +100,7 @@ func (mp *messageProcessor) processMessages(
 	var needsClientUpdate bool
 
 	// Localhost IBC does not permit client updates
-	if src.clientState.ClientID != ibcexported.LocalhostClientID && dst.clientState.ClientID != ibcexported.LocalhostConnectionID {
+	if !isLocalhostClient(src.clientState.ClientID, dst.clientState.ClientID) {
 		var err error
 		needsClientUpdate, err = mp.shouldUpdateClientNow(ctx, src, dst)
 		if err != nil {
@@ -117,13 +117,22 @@ func (mp *messageProcessor) processMessages(
 	return mp.trackAndSendMessages(ctx, src, dst, needsClientUpdate)
 }
 
+func isLocalhostClient(srcClientID, dstClientID string) bool {
+	if srcClientID == ibcexported.LocalhostClientID && dstClientID == ibcexported.LocalhostConnectionID {
+		return true
+	}
+
+	return false
+}
+
 // shouldUpdateClientNow determines if an update client message should be sent
 // even if there are no messages to be sent now. It will not be attempted if
 // there has not been enough blocks since the last client update attempt.
 // Otherwise, it will be attempted if either 2/3 of the trusting period
 // or the configured client update threshold duration has passed.
 func (mp *messageProcessor) shouldUpdateClientNow(ctx context.Context, src, dst *pathEndRuntime) (bool, error) {
 	var consensusHeightTime time.Time
+
 	if dst.clientState.ConsensusTime.IsZero() {
 		h, err := src.chainProvider.QueryIBCHeader(ctx, int64(dst.clientState.ConsensusHeight.RevisionHeight))
 		if err != nil {
@@ -246,6 +255,7 @@ func (mp *messageProcessor) assembleMsgUpdateClient(ctx context.Context, src, ds
 	clientID := dst.info.ClientID
 	clientConsensusHeight := dst.clientState.ConsensusHeight
 	trustedConsensusHeight := dst.clientTrustedState.ClientState.ConsensusHeight
+
 	var trustedNextValidatorsHash []byte
 	if dst.clientTrustedState.IBCHeader != nil {
 		trustedNextValidatorsHash = dst.clientTrustedState.IBCHeader.NextValidatorsHash()
@@ -260,11 +270,13 @@ func (mp *messageProcessor) assembleMsgUpdateClient(ctx context.Context, src, ds
 			return fmt.Errorf("observed client trusted height: %d does not equal latest client state height: %d",
 				trustedConsensusHeight.RevisionHeight, clientConsensusHeight.RevisionHeight)
 		}
+
 		header, err := src.chainProvider.QueryIBCHeader(ctx, int64(clientConsensusHeight.RevisionHeight+1))
 		if err != nil {
 			return fmt.Errorf("error getting IBC header at height: %d for chain_id: %s, %w",
 				clientConsensusHeight.RevisionHeight+1, src.info.ChainID, err)
 		}
+
 		mp.log.Debug("Had to query for client trusted IBC header",
 			zap.String("path_name", src.info.PathName),
 			zap.String("chain_id", src.info.ChainID),
@@ -273,10 +285,12 @@ func (mp *messageProcessor) assembleMsgUpdateClient(ctx context.Context, src, ds
 			zap.Uint64("height", clientConsensusHeight.RevisionHeight+1),
 			zap.Uint64("latest_height", src.latestBlock.Height),
 		)
+
 		dst.clientTrustedState = provider.ClientTrustedState{
 			ClientState: dst.clientState,
 			IBCHeader:   header,
 		}
+
 		trustedConsensusHeight = clientConsensusHeight
 		trustedNextValidatorsHash = header.NextValidatorsHash()
 	}
@@ -346,7 +360,7 @@ func (mp *messageProcessor) trackAndSendMessages(
 		return nil
 	}
 
-	if needsClientUpdate {
+	if needsClientUpdate && mp.msgUpdateClient != nil {
 		go mp.sendClientUpdate(ctx, src, dst)
 		return nil
 	}
@@ -420,7 +434,10 @@ func (mp *messageProcessor) sendBatchMessages(
 	} else {
 		// messages are batch with appended MsgUpdateClient
 		msgs = make([]provider.RelayerMessage, 1+len(batch))
-		msgs[0] = mp.msgUpdateClient
+		if mp.msgUpdateClient != nil {
+			msgs[0] = mp.msgUpdateClient
+		}
+
 		for i, t := range batch {
 			msgs[i+1] = t.assembledMsg()
 			fields = append(fields, zap.Object(fmt.Sprintf("msg_%d", i), t))