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

… constructed (#1407)

* 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.

* fix: undo previous check in `sendBatchMessages`

* chore: update cometbft-client to v0.1.0

* chore: update cometbft-client to v0.1.0 in e2e tests

go.mod

@@ -29,7 +29,7 @@ require (
 	github.com/prometheus/client_golang v1.17.0
 	github.com/spf13/cobra v1.8.0
 	github.com/spf13/viper v1.18.1
-	github.com/strangelove-ventures/cometbft-client v0.0.0-20240122193328-9503d3144af6
+	github.com/strangelove-ventures/cometbft-client v0.1.0
 	github.com/stretchr/testify v1.8.4
 	github.com/tyler-smith/go-bip39 v1.1.0
 	go.uber.org/multierr v1.10.0

go.sum

@@ -1056,8 +1056,8 @@ github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An
 github.com/spf13/viper v1.3.2/go.mod h1:ZiWeW+zYFKm7srdB9IoDzzZXaJaI5eL9QjNiN/DMA2s=
 github.com/spf13/viper v1.18.1 h1:rmuU42rScKWlhhJDyXZRKJQHXFX02chSVW1IvkPGiVM=
 github.com/spf13/viper v1.18.1/go.mod h1:EKmWIqdnk5lOcmR72yw6hS+8OPYcwD0jteitLMVB+yk=
-github.com/strangelove-ventures/cometbft-client v0.0.0-20240122193328-9503d3144af6 h1:+GBtxz0ZdS/UiGl9mK+g9P6k9MDpLxhw7reBIDyIm+Q=
-github.com/strangelove-ventures/cometbft-client v0.0.0-20240122193328-9503d3144af6/go.mod h1:QzThgjzvsGgUNVNpGPitmxOWMIhp6a0oqf80nCRNt/0=
+github.com/strangelove-ventures/cometbft-client v0.1.0 h1:fcA652QaaR0LDnyJOZVjZKtuyAawnVXaq/p1MWJSYD4=
+github.com/strangelove-ventures/cometbft-client v0.1.0/go.mod h1:QzThgjzvsGgUNVNpGPitmxOWMIhp6a0oqf80nCRNt/0=
 github.com/streadway/amqp v0.0.0-20190404075320-75d898a42a94/go.mod h1:AZpEONHx3DKn8O/DFsRAY58/XVQiIPMTMB1SddzLXVw=
 github.com/streadway/amqp v0.0.0-20190827072141-edfb9018d271/go.mod h1:AZpEONHx3DKn8O/DFsRAY58/XVQiIPMTMB1SddzLXVw=
 github.com/streadway/handy v0.0.0-20190108123426-d5acb3125c2a/go.mod h1:qNTQ5P5JnDBl6z3cMAg/SywNDC5ABu5ApDIw6lUbRmI=

interchaintest/go.mod

@@ -225,7 +225,7 @@ require (
 	github.com/spf13/cobra v1.8.0 // indirect
 	github.com/spf13/pflag v1.0.5 // indirect
 	github.com/spf13/viper v1.18.1 // indirect
-	github.com/strangelove-ventures/cometbft-client v0.0.0-20240122193328-9503d3144af6 // indirect
+	github.com/strangelove-ventures/cometbft-client v0.1.0 // indirect
 	github.com/subosito/gotenv v1.6.0 // indirect
 	github.com/supranational/blst v0.3.11 // indirect
 	github.com/syndtr/goleveldb v1.0.1-0.20220721030215-126854af5e6d // indirect

interchaintest/go.sum

@@ -1151,8 +1151,8 @@ github.com/spf13/pflag v1.0.5/go.mod h1:McXfInJRrz4CZXVZOBLb0bTZqETkiAhM9Iw0y3An
 github.com/spf13/viper v1.3.2/go.mod h1:ZiWeW+zYFKm7srdB9IoDzzZXaJaI5eL9QjNiN/DMA2s=
 github.com/spf13/viper v1.18.1 h1:rmuU42rScKWlhhJDyXZRKJQHXFX02chSVW1IvkPGiVM=
 github.com/spf13/viper v1.18.1/go.mod h1:EKmWIqdnk5lOcmR72yw6hS+8OPYcwD0jteitLMVB+yk=
-github.com/strangelove-ventures/cometbft-client v0.0.0-20240122193328-9503d3144af6 h1:+GBtxz0ZdS/UiGl9mK+g9P6k9MDpLxhw7reBIDyIm+Q=
-github.com/strangelove-ventures/cometbft-client v0.0.0-20240122193328-9503d3144af6/go.mod h1:QzThgjzvsGgUNVNpGPitmxOWMIhp6a0oqf80nCRNt/0=
+github.com/strangelove-ventures/cometbft-client v0.1.0 h1:fcA652QaaR0LDnyJOZVjZKtuyAawnVXaq/p1MWJSYD4=
+github.com/strangelove-ventures/cometbft-client v0.1.0/go.mod h1:QzThgjzvsGgUNVNpGPitmxOWMIhp6a0oqf80nCRNt/0=
 github.com/strangelove-ventures/interchaintest/v8 v8.0.1-0.20231114192524-e3719592933b h1:VDe2ofJ2xiiLwkJ6qhcF2gvg75gB4WVpXO8lFzhYQOU=
 github.com/strangelove-ventures/interchaintest/v8 v8.0.1-0.20231114192524-e3719592933b/go.mod h1:TbVaBTSa9Y7/Jj/JeqoH79fAcyQiHloz1zxXxKjtCFA=
 github.com/streadway/amqp v0.0.0-20190404075320-75d898a42a94/go.mod h1:AZpEONHx3DKn8O/DFsRAY58/XVQiIPMTMB1SddzLXVw=

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
 	}
@@ -421,6 +435,7 @@ func (mp *messageProcessor) sendBatchMessages(
 		// messages are batch with appended MsgUpdateClient
 		msgs = make([]provider.RelayerMessage, 1+len(batch))
 		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))