Skip to content

Commit cfe8b93

Browse files
authored
fix: Observed Packet Metric (#1345)
* fix: restructure `ShouldRelayChannel` * fix test * check channel open + tests * remove open channel check * fix race - remove warn log * add mutex's * Update comment
1 parent 654ea9f commit cfe8b93

File tree

7 files changed

+308
-358
lines changed

7 files changed

+308
-358
lines changed

interchaintest/go.mod

+3-2
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,6 @@ require (
172172
github.com/manifoldco/promptui v0.9.0 // indirect
173173
github.com/mattn/go-colorable v0.1.13 // indirect
174174
github.com/mattn/go-isatty v0.0.20 // indirect
175-
github.com/matttproud/golang_protobuf_extensions v1.0.4 // indirect
176175
github.com/matttproud/golang_protobuf_extensions/v2 v2.0.0 // indirect
177176
github.com/mimoo/StrobeGo v0.0.0-20220103164710-9a04d6ca976b // indirect
178177
github.com/minio/highwayhash v1.0.2 // indirect
@@ -213,13 +212,15 @@ require (
213212
github.com/rogpeppe/go-internal v1.11.0 // indirect
214213
github.com/rs/cors v1.8.3 // indirect
215214
github.com/rs/zerolog v1.31.0 // indirect
215+
github.com/sagikazarmark/locafero v0.3.0 // indirect
216+
github.com/sagikazarmark/slog-shim v0.1.0 // indirect
216217
github.com/sasha-s/go-deadlock v0.3.1 // indirect
217218
github.com/sirupsen/logrus v1.9.3 // indirect
219+
github.com/sourcegraph/conc v0.3.0 // indirect
218220
github.com/spaolacci/murmur3 v1.1.0 // indirect
219221
github.com/spf13/afero v1.10.0 // indirect
220222
github.com/spf13/cast v1.5.1 // indirect
221223
github.com/spf13/cobra v1.8.0 // indirect
222-
github.com/spf13/jwalterweatherman v1.1.0 // indirect
223224
github.com/spf13/pflag v1.0.5 // indirect
224225
github.com/spf13/viper v1.17.0 // indirect
225226
github.com/strangelove-ventures/cometbft v0.37.3-0.20231004194858-c01e8d5bcac3 // indirect

interchaintest/go.sum

+37-172
Large diffs are not rendered by default.

relayer/processor/path_end.go

-24
Original file line numberDiff line numberDiff line change
@@ -67,27 +67,3 @@ func (pe PathEnd) shouldRelayChannelSingle(channelKey ChainChannelKey, listChann
6767
}
6868
return !allowList
6969
}
70-
71-
// if port ID is empty on allowlist channel, allow all ports
72-
// if port ID is non-empty on allowlist channel, allow only that specific port
73-
// if port ID is empty on blocklist channel, block all ports
74-
// if port ID is non-empty on blocklist channel, block only that specific port
75-
func (pe PathEnd) ShouldRelayChannel(channelKey ChainChannelKey) bool {
76-
if pe.Rule == RuleAllowList {
77-
for _, allowedChannel := range pe.FilterList {
78-
if pe.shouldRelayChannelSingle(channelKey, allowedChannel, true) {
79-
return true
80-
}
81-
}
82-
return false
83-
} else if pe.Rule == RuleDenyList {
84-
for _, blockedChannel := range pe.FilterList {
85-
if !pe.shouldRelayChannelSingle(channelKey, blockedChannel, false) {
86-
return false
87-
}
88-
}
89-
return true
90-
}
91-
// if neither allow list or block list are provided, all channels are okay
92-
return true
93-
}

relayer/processor/path_end_runtime.go

+47-2
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ type pathEndRuntime struct {
3030
clientTrustedState provider.ClientTrustedState
3131
connectionStateCache ConnectionStateCache
3232
channelStateCache ChannelStateCache
33+
channelStateCacheMu sync.RWMutex
3334
channelOrderCache map[string]chantypes.Order
3435
latestHeader provider.IBCHeader
3536
ibcHeaderCache IBCHeaderCache
@@ -107,7 +108,7 @@ func (pathEnd *pathEndRuntime) mergeMessageCache(messageCache IBCMessagesCache,
107108
clientICQMessages := make(ClientICQMessagesCache)
108109

109110
for ch, pmc := range messageCache.PacketFlow {
110-
if pathEnd.info.ShouldRelayChannel(ChainChannelKey{ChainID: pathEnd.info.ChainID, CounterpartyChainID: counterpartyChainID, ChannelKey: ch}) {
111+
if pathEnd.ShouldRelayChannel(ChainChannelKey{ChainID: pathEnd.info.ChainID, CounterpartyChainID: counterpartyChainID, ChannelKey: ch}) {
111112
if inSync && pathEnd.metrics != nil {
112113
for eventType, pCache := range pmc {
113114
pathEnd.metrics.AddPacketsObserved(pathEnd.info.PathName, pathEnd.info.ChainID, ch.ChannelID, ch.PortID, eventType, len(pCache))
@@ -403,7 +404,10 @@ func (pathEnd *pathEndRuntime) mergeCacheData(ctx context.Context, cancel func()
403404
}
404405

405406
pathEnd.connectionStateCache = d.ConnectionStateCache // Update latest connection open state for chain
406-
pathEnd.channelStateCache = d.ChannelStateCache // Update latest channel open state for chain
407+
408+
pathEnd.channelStateCacheMu.Lock()
409+
pathEnd.channelStateCache = d.ChannelStateCache // Update latest channel open state for chain
410+
pathEnd.channelStateCacheMu.Unlock()
407411

408412
pathEnd.mergeMessageCache(d.IBCMessagesCache, counterpartyChainID, pathEnd.inSync && counterpartyInSync) // Merge incoming packet IBC messages into the backlog
409413

@@ -874,3 +878,44 @@ func (pathEnd *pathEndRuntime) localhostSentinelProofChannel(
874878
Version: info.Version,
875879
}, nil
876880
}
881+
882+
// ShouldRelayChannel determines whether a chain channel (and optionally a port), should be relayed
883+
// by this path end.
884+
//
885+
// It first checks if the channel matches any rule in the path end's filter list. If the channel matches a channel
886+
// in an allowed list, it returns true. If the channel matches any blocked channel it returns false. Otherwise, it returns true.
887+
//
888+
// If no filter rule matches, it checks if the channel or its counterparty is present in the path end's
889+
// channel state cache. This cache only holds channels relevant to the client for this path end, ensuring
890+
// the channel belongs to a client connected to this path end.
891+
//
892+
// Note that this function only determines whether the channel should be relayed based on the path end's
893+
// configuration. It does not guarantee that the channel is actually relayable, as other checks
894+
// (e.g., expired client) may still be necessary.
895+
func (pathEnd *pathEndRuntime) ShouldRelayChannel(chainChannelKey ChainChannelKey) bool {
896+
pe := pathEnd.info
897+
if pe.Rule == RuleAllowList {
898+
for _, allowedChannel := range pe.FilterList {
899+
if pe.shouldRelayChannelSingle(chainChannelKey, allowedChannel, true) {
900+
return true
901+
}
902+
}
903+
return false
904+
} else if pe.Rule == RuleDenyList {
905+
for _, blockedChannel := range pe.FilterList {
906+
if !pe.shouldRelayChannelSingle(chainChannelKey, blockedChannel, false) {
907+
return false
908+
}
909+
}
910+
return true
911+
}
912+
913+
pathEnd.channelStateCacheMu.RLock()
914+
defer pathEnd.channelStateCacheMu.RUnlock()
915+
916+
// if no filter rule, check if the channel or counterparty channel is in the channelStateCache.
917+
// Because channelStateCache only holds channels relevant to the client, we can ensure that the
918+
// channel is built on top of a client for this pathEnd
919+
_, exists := pathEnd.channelStateCache[chainChannelKey.ChannelKey]
920+
return exists
921+
}

0 commit comments

Comments
 (0)