From 2e457c2ba25b35ea1e9975aff88764fd0d941652 Mon Sep 17 00:00:00 2001 From: Justin Tieri <37750742+jtieri@users.noreply.github.com> Date: Thu, 22 Feb 2024 15:27:26 -0600 Subject: [PATCH 1/4] 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 | 23 ++++++++++++++++++++--- 1 file changed, 20 insertions(+), 3 deletions(-) diff --git a/relayer/processor/message_processor.go b/relayer/processor/message_processor.go index 30084b5f5..d2bcd25f5 100644 --- a/relayer/processor/message_processor.go +++ b/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,6 +117,14 @@ 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. @@ -124,6 +132,7 @@ func (mp *messageProcessor) processMessages( // 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)) From 73785a33ed9407a70d46bfa1dd255816b8e3bffc Mon Sep 17 00:00:00 2001 From: Justin Tieri <37750742+jtieri@users.noreply.github.com> Date: Thu, 22 Feb 2024 15:45:24 -0600 Subject: [PATCH 2/4] fix: undo previous check in `sendBatchMessages` --- relayer/processor/message_processor.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/relayer/processor/message_processor.go b/relayer/processor/message_processor.go index d2bcd25f5..b13ffa7cc 100644 --- a/relayer/processor/message_processor.go +++ b/relayer/processor/message_processor.go @@ -434,9 +434,7 @@ func (mp *messageProcessor) sendBatchMessages( } else { // messages are batch with appended MsgUpdateClient msgs = make([]provider.RelayerMessage, 1+len(batch)) - if mp.msgUpdateClient != nil { - msgs[0] = mp.msgUpdateClient - } + msgs[0] = mp.msgUpdateClient for i, t := range batch { msgs[i+1] = t.assembledMsg() From 8b6e8380645d6f56080baa97f015edfefafac77a Mon Sep 17 00:00:00 2001 From: Justin Tieri <37750742+jtieri@users.noreply.github.com> Date: Thu, 22 Feb 2024 21:09:46 -0600 Subject: [PATCH 3/4] chore: update cometbft-client to v0.1.0 --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index 5aa84d765..96f326819 100644 --- a/go.mod +++ b/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 diff --git a/go.sum b/go.sum index f448e31c0..54d064148 100644 --- a/go.sum +++ b/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= From 42424bcbedba8fb918723ee267f30ba4300758e2 Mon Sep 17 00:00:00 2001 From: Justin Tieri <37750742+jtieri@users.noreply.github.com> Date: Thu, 22 Feb 2024 21:13:43 -0600 Subject: [PATCH 4/4] chore: update cometbft-client to v0.1.0 in e2e tests --- interchaintest/go.mod | 2 +- interchaintest/go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/interchaintest/go.mod b/interchaintest/go.mod index 4eb61edff..f1a003f6c 100644 --- a/interchaintest/go.mod +++ b/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 diff --git a/interchaintest/go.sum b/interchaintest/go.sum index 29f79753b..3dc30ae16 100644 --- a/interchaintest/go.sum +++ b/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=