From b6553be355b2b09af27fc94790958c54690182a4 Mon Sep 17 00:00:00 2001 From: Andrew Gouin Date: Fri, 23 Sep 2022 14:24:06 -0600 Subject: [PATCH] fix counterparty path filter (#1000) * fix counterparty path filter * Filter fix test * Add denylist test and add makefile and gh action * Slim test for non-self-hosted runner * Update ibctest to latest main * Make better assertion for denylist acks. Constants for allowlist/denylist. Validate filterRule in CLI * Use isolated prometheus registry per relayer instance instead of prometheus default registry * run path filter tests in parallel --- .github/workflows/ibctest.yml | 21 +++ Makefile | 3 + cmd/flags.go | 14 ++ cmd/paths.go | 48 ++++++ ibctest/path_filter_test.go | 306 ++++++++++++++++++++++++++++++++++ ibctest/relayer.go | 11 ++ relayer/path.go | 11 +- relayer/processor/metrics.go | 14 +- relayer/strategies.go | 4 +- 9 files changed, 419 insertions(+), 13 deletions(-) create mode 100644 ibctest/path_filter_test.go diff --git a/.github/workflows/ibctest.yml b/.github/workflows/ibctest.yml index 4d84662d5..d973952ea 100644 --- a/.github/workflows/ibctest.yml +++ b/.github/workflows/ibctest.yml @@ -70,3 +70,24 @@ jobs: - name: ibctest run: make ibctest-multiple + path-filter: + runs-on: ubuntu-latest + steps: + - name: Set up Go 1.18 + uses: actions/setup-go@v1 + with: + go-version: 1.18 + id: go + + - name: checkout relayer + uses: actions/checkout@v2 + + - uses: actions/cache@v1 + with: + path: ~/go/pkg/mod + key: ${{ runner.os }}-go-${{ hashFiles('**/go.sum') }} + restore-keys: | + ${{ runner.os }}-go- + + - name: ibctest + run: make ibctest-path-filter \ No newline at end of file diff --git a/Makefile b/Makefile index 1f39cc32e..8280e09ab 100644 --- a/Makefile +++ b/Makefile @@ -87,6 +87,9 @@ ibctest-legacy: ibctest-multiple: cd ibctest && go test -race -v -run TestRelayerMultiplePathsSingleProcess . +ibctest-path-filter: + cd ibctest && go test -race -v -run TestPathFilter . + coverage: @echo "viewing test coverage..." @go tool cover --html=coverage.out diff --git a/cmd/flags.go b/cmd/flags.go index a1ba4fae5..bd44e723c 100644 --- a/cmd/flags.go +++ b/cmd/flags.go @@ -45,6 +45,8 @@ const ( flagProcessor = "processor" flagInitialBlockHistory = "block-history" flagMemo = "memo" + flagFilterRule = "filter-rule" + flagFilterChannels = "filter-channels" ) const ( @@ -152,6 +154,18 @@ func fileFlag(v *viper.Viper, cmd *cobra.Command) *cobra.Command { return cmd } +func pathFilterFlags(v *viper.Viper, cmd *cobra.Command) *cobra.Command { + cmd.Flags().String(flagFilterRule, "", `filter rule ("allowlist", "denylist", or "" for no filtering)`) + if err := v.BindPFlag(flagFilterRule, cmd.Flags().Lookup(flagFilterRule)); err != nil { + panic(err) + } + cmd.Flags().String(flagFilterChannels, "", "channels from source chain perspective to filter") + if err := v.BindPFlag(flagFilterRule, cmd.Flags().Lookup(flagFilterRule)); err != nil { + panic(err) + } + return cmd +} + func timeoutFlag(v *viper.Viper, cmd *cobra.Command) *cobra.Command { cmd.Flags().StringP(flagTimeout, "t", "10s", "timeout between relayer runs") if err := v.BindPFlag(flagTimeout, cmd.Flags().Lookup(flagTimeout)); err != nil { diff --git a/cmd/paths.go b/cmd/paths.go index cad96d5a5..090ac919a 100644 --- a/cmd/paths.go +++ b/cmd/paths.go @@ -9,6 +9,7 @@ import ( "strings" "github.com/cosmos/relayer/v2/relayer" + "github.com/cosmos/relayer/v2/relayer/processor" "github.com/google/go-github/v43/github" "github.com/spf13/cobra" "gopkg.in/yaml.v3" @@ -30,6 +31,7 @@ This includes the client, connection, and channel ids from both the source and d pathsAddCmd(a), pathsAddDirCmd(a), pathsNewCmd(a), + pathsUpdateCmd(a), pathsFetchCmd(a), pathsDeleteCmd(a), ) @@ -260,6 +262,52 @@ $ %s pth n ibc-0 ibc-1 demo-path`, appName, appName)), return channelParameterFlags(a.Viper, cmd) } +func pathsUpdateCmd(a *appState) *cobra.Command { + cmd := &cobra.Command{ + Use: "update path_name", + Aliases: []string{"n"}, + Short: `Update a path such as the filter rule ("allowlist", "denylist", or "" for no filtering) and channels`, + Args: withUsage(cobra.ExactArgs(1)), + Example: strings.TrimSpace(fmt.Sprintf(` +$ %s paths update demo-path --filter-rule allowlist --filter-channels channel-0,channel-1 +$ %s paths update demo-path --filter-rule denylist --filter-channels channel-0,channel-1`, + appName, appName)), + RunE: func(cmd *cobra.Command, args []string) error { + name := args[0] + + filterRule, err := cmd.Flags().GetString(flagFilterRule) + if err != nil { + return err + } + if filterRule != "" && filterRule != processor.RuleAllowList && filterRule != processor.RuleDenyList { + return fmt.Errorf(`invalid filter rule : "%s". valid rules: ("", "%s", "%s")`, filterRule, processor.RuleAllowList, processor.RuleDenyList) + } + + filterChannels, err := cmd.Flags().GetString(flagFilterChannels) + if err != nil { + return err + } + + var channelList []string + + if filterChannels != "" { + channelList = strings.Split(filterChannels, ",") + } + + p := a.Config.Paths.MustGet(name) + + p.Filter = relayer.ChannelFilter{ + Rule: filterRule, + ChannelList: channelList, + } + + return a.OverwriteConfig(a.Config) + }, + } + cmd = pathFilterFlags(a.Viper, cmd) + return cmd +} + // pathsFetchCmd attempts to fetch the json files containing the path metadata, for each configured chain, from GitHub func pathsFetchCmd(a *appState) *cobra.Command { cmd := &cobra.Command{ diff --git a/ibctest/path_filter_test.go b/ibctest/path_filter_test.go new file mode 100644 index 000000000..a40bf577e --- /dev/null +++ b/ibctest/path_filter_test.go @@ -0,0 +1,306 @@ +package ibctest_test + +import ( + "context" + "fmt" + "testing" + + transfertypes "github.com/cosmos/ibc-go/v5/modules/apps/transfer/types" + relayeribctest "github.com/cosmos/relayer/v2/ibctest" + "github.com/cosmos/relayer/v2/relayer" + "github.com/cosmos/relayer/v2/relayer/processor" + ibctest "github.com/strangelove-ventures/ibctest/v5" + "github.com/strangelove-ventures/ibctest/v5/ibc" + "github.com/strangelove-ventures/ibctest/v5/test" + "github.com/strangelove-ventures/ibctest/v5/testreporter" + "github.com/stretchr/testify/require" + "go.uber.org/zap/zaptest" + "golang.org/x/sync/errgroup" +) + +// TestPathFilterAllow tests the channel allowlist +func TestPathFilterAllow(t *testing.T) { + t.Parallel() + ctx := context.Background() + + nv := 1 + nf := 0 + + // Chain Factory + cf := ibctest.NewBuiltinChainFactory(zaptest.NewLogger(t), []*ibctest.ChainSpec{ + {Name: "gaia", Version: "v7.0.3", NumValidators: &nv, NumFullNodes: &nf}, + {Name: "osmosis", Version: "v11.0.1", NumValidators: &nv, NumFullNodes: &nf}, + }) + + chains, err := cf.Chains(t.Name()) + require.NoError(t, err) + gaia, osmosis := chains[0], chains[1] + + // Relayer Factory to construct relayer + r := relayeribctest.NewRelayerFactory(relayeribctest.RelayerConfig{ + Processor: relayer.ProcessorEvents, + InitialBlockHistory: 100, + }).Build(t, nil, "") + + // Prep Interchain + const ibcPath = "gaia-osmosis" + ic := ibctest.NewInterchain(). + AddChain(gaia). + AddChain(osmosis). + AddRelayer(r, "relayer"). + AddLink(ibctest.InterchainLink{ + Chain1: gaia, + Chain2: osmosis, + Relayer: r, + Path: ibcPath, + }) + + // Reporter/logs + rep := testreporter.NewNopReporter() + eRep := rep.RelayerExecReporter(t) + + client, network := ibctest.DockerSetup(t) + + // Build interchain + require.NoError(t, ic.Build(ctx, eRep, ibctest.InterchainBuildOptions{ + TestName: t.Name(), + Client: client, + NetworkID: network, + + SkipPathCreation: false, + })) + + // Get Channel ID + gaiaChans, err := r.GetChannels(ctx, eRep, gaia.Config().ChainID) + require.NoError(t, err) + gaiaChannel := gaiaChans[0] + osmosisChannel := gaiaChans[0].Counterparty + + r.UpdatePath(ctx, eRep, ibcPath, ibc.ChannelFilter{ + Rule: processor.RuleAllowList, + ChannelList: []string{gaiaChannel.ChannelID}, + }) + + // Create and Fund User Wallets + fundAmount := int64(10_000_000) + users := ibctest.GetAndFundTestUsers(t, ctx, "default", int64(fundAmount), gaia, osmosis) + + gaiaUser, osmosisUser := users[0], users[1] + + r.StartRelayer(ctx, eRep, ibcPath) + + // Send Transaction + amountToSend := int64(1_000_000) + gaiaDstAddress := gaiaUser.Bech32Address(osmosis.Config().Bech32Prefix) + osmosisDstAddress := osmosisUser.Bech32Address(gaia.Config().Bech32Prefix) + + gaiaHeight, err := gaia.Height(ctx) + require.NoError(t, err) + + osmosisHeight, err := osmosis.Height(ctx) + require.NoError(t, err) + + var eg errgroup.Group + eg.Go(func() error { + tx, err := gaia.SendIBCTransfer(ctx, gaiaChannel.ChannelID, gaiaUser.KeyName, ibc.WalletAmount{ + Address: gaiaDstAddress, + Denom: gaia.Config().Denom, + Amount: amountToSend, + }, + nil, + ) + if err != nil { + return err + } + if err := tx.Validate(); err != nil { + return err + } + _, err = test.PollForAck(ctx, gaia, gaiaHeight, gaiaHeight+10, tx.Packet) + return err + }) + + eg.Go(func() error { + tx, err := osmosis.SendIBCTransfer(ctx, osmosisChannel.ChannelID, osmosisUser.KeyName, ibc.WalletAmount{ + Address: osmosisDstAddress, + Denom: osmosis.Config().Denom, + Amount: amountToSend, + }, + nil, + ) + if err != nil { + return err + } + if err := tx.Validate(); err != nil { + return err + } + _, err = test.PollForAck(ctx, osmosis, osmosisHeight, osmosisHeight+10, tx.Packet) + return err + }) + // Acks should exist + require.NoError(t, eg.Wait()) + + // Trace IBC Denom + gaiaDenomTrace := transfertypes.ParseDenomTrace(transfertypes.GetPrefixedDenom(osmosisChannel.PortID, osmosisChannel.ChannelID, gaia.Config().Denom)) + gaiaIbcDenom := gaiaDenomTrace.IBCDenom() + + osmosisDenomTrace := transfertypes.ParseDenomTrace(transfertypes.GetPrefixedDenom(gaiaChannel.PortID, gaiaChannel.ChannelID, osmosis.Config().Denom)) + osmosisIbcDenom := osmosisDenomTrace.IBCDenom() + + // Test destination wallets have increased funds + gaiaIBCBalance, err := osmosis.GetBalance(ctx, gaiaDstAddress, gaiaIbcDenom) + require.NoError(t, err) + require.Equal(t, amountToSend, gaiaIBCBalance) + + osmosisIBCBalance, err := gaia.GetBalance(ctx, osmosisDstAddress, osmosisIbcDenom) + require.NoError(t, err) + require.Equal(t, amountToSend, osmosisIBCBalance) +} + +// TestPathFilterDeny tests the channel denylist +func TestPathFilterDeny(t *testing.T) { + t.Parallel() + ctx := context.Background() + + nv := 1 + nf := 0 + + // Chain Factory + cf := ibctest.NewBuiltinChainFactory(zaptest.NewLogger(t), []*ibctest.ChainSpec{ + {Name: "gaia", Version: "v7.0.3", NumValidators: &nv, NumFullNodes: &nf}, + {Name: "osmosis", Version: "v11.0.1", NumValidators: &nv, NumFullNodes: &nf}, + }) + + chains, err := cf.Chains(t.Name()) + require.NoError(t, err) + gaia, osmosis := chains[0], chains[1] + + // Relayer Factory to construct relayer + r := relayeribctest.NewRelayerFactory(relayeribctest.RelayerConfig{ + Processor: relayer.ProcessorEvents, + InitialBlockHistory: 100, + }).Build(t, nil, "") + + // Prep Interchain + const ibcPath = "gaia-osmosis" + ic := ibctest.NewInterchain(). + AddChain(gaia). + AddChain(osmosis). + AddRelayer(r, "relayer"). + AddLink(ibctest.InterchainLink{ + Chain1: gaia, + Chain2: osmosis, + Relayer: r, + Path: ibcPath, + }) + + rep := testreporter.NewNopReporter() + eRep := rep.RelayerExecReporter(t) + + client, network := ibctest.DockerSetup(t) + + // Build interchain + require.NoError(t, ic.Build(ctx, eRep, ibctest.InterchainBuildOptions{ + TestName: t.Name(), + Client: client, + NetworkID: network, + + SkipPathCreation: false, + })) + + // Get Channel ID + gaiaChans, err := r.GetChannels(ctx, eRep, gaia.Config().ChainID) + require.NoError(t, err) + gaiaChannel := gaiaChans[0] + osmosisChannel := gaiaChans[0].Counterparty + + r.UpdatePath(ctx, eRep, ibcPath, ibc.ChannelFilter{ + Rule: processor.RuleDenyList, + ChannelList: []string{gaiaChannel.ChannelID}, + }) + + // Create and Fund User Wallets + fundAmount := int64(10_000_000) + users := ibctest.GetAndFundTestUsers(t, ctx, "default", int64(fundAmount), gaia, osmosis) + + gaiaUser, osmosisUser := users[0], users[1] + + r.StartRelayer(ctx, eRep, ibcPath) + + // Send Transaction + amountToSend := int64(1_000_000) + gaiaDstAddress := gaiaUser.Bech32Address(osmosis.Config().Bech32Prefix) + osmosisDstAddress := osmosisUser.Bech32Address(gaia.Config().Bech32Prefix) + + gaiaHeight, err := gaia.Height(ctx) + require.NoError(t, err) + + osmosisHeight, err := osmosis.Height(ctx) + require.NoError(t, err) + + var eg errgroup.Group + eg.Go(func() error { + tx, err := gaia.SendIBCTransfer(ctx, gaiaChannel.ChannelID, gaiaUser.KeyName, ibc.WalletAmount{ + Address: gaiaDstAddress, + Denom: gaia.Config().Denom, + Amount: amountToSend, + }, + nil, + ) + if err != nil { + return err + } + if err := tx.Validate(); err != nil { + return err + } + + // we want an error here + ack, err := test.PollForAck(ctx, gaia, gaiaHeight, gaiaHeight+10, tx.Packet) + if err == nil { + return fmt.Errorf("no error when error was expected when polling for ack: %+v", ack) + } + + return nil + }) + + eg.Go(func() error { + tx, err := osmosis.SendIBCTransfer(ctx, osmosisChannel.ChannelID, osmosisUser.KeyName, ibc.WalletAmount{ + Address: osmosisDstAddress, + Denom: osmosis.Config().Denom, + Amount: amountToSend, + }, + nil, + ) + if err != nil { + return err + } + if err := tx.Validate(); err != nil { + return err + } + + // we want an error here + ack, err := test.PollForAck(ctx, osmosis, osmosisHeight, osmosisHeight+10, tx.Packet) + if err == nil { + return fmt.Errorf("no error when error was expected when polling for ack: %+v", ack) + } + + return nil + }) + // Test that acks do not show up + require.NoError(t, eg.Wait()) + + // Trace IBC Denom + gaiaDenomTrace := transfertypes.ParseDenomTrace(transfertypes.GetPrefixedDenom(osmosisChannel.PortID, osmosisChannel.ChannelID, gaia.Config().Denom)) + gaiaIbcDenom := gaiaDenomTrace.IBCDenom() + + osmosisDenomTrace := transfertypes.ParseDenomTrace(transfertypes.GetPrefixedDenom(gaiaChannel.PortID, gaiaChannel.ChannelID, osmosis.Config().Denom)) + osmosisIbcDenom := osmosisDenomTrace.IBCDenom() + + // Test destination wallets do not have increased funds + gaiaIBCBalance, err := osmosis.GetBalance(ctx, gaiaDstAddress, gaiaIbcDenom) + require.NoError(t, err) + require.Equal(t, int64(0), gaiaIBCBalance) + + osmosisIBCBalance, err := gaia.GetBalance(ctx, osmosisDstAddress, osmosisIbcDenom) + require.NoError(t, err) + require.Equal(t, int64(0), osmosisIBCBalance) +} diff --git a/ibctest/relayer.go b/ibctest/relayer.go index 0223140c4..247838a74 100644 --- a/ibctest/relayer.go +++ b/ibctest/relayer.go @@ -110,6 +110,17 @@ func (r *Relayer) GeneratePath(ctx context.Context, _ ibc.RelayerExecReporter, s return nil } +func (r *Relayer) UpdatePath(ctx context.Context, _ ibc.RelayerExecReporter, pathName string, filter ibc.ChannelFilter) error { + res := r.sys().RunC(ctx, r.log(), "paths", "update", pathName, + "--filter-rule", filter.Rule, + "--filter-channels", strings.Join(filter.ChannelList, ","), + ) + if res.Err != nil { + return res.Err + } + return nil +} + func (r *Relayer) GetChannels(ctx context.Context, _ ibc.RelayerExecReporter, chainID string) ([]ibc.ChannelOutput, error) { res := r.sys().RunC(ctx, r.log(), "q", "channels", chainID) if res.Err != nil { diff --git a/relayer/path.go b/relayer/path.go index 7e2b5e757..f1d06b498 100644 --- a/relayer/path.go +++ b/relayer/path.go @@ -6,15 +6,14 @@ import ( clienttypes "github.com/cosmos/ibc-go/v3/modules/core/02-client/types" conntypes "github.com/cosmos/ibc-go/v3/modules/core/03-connection/types" + "github.com/cosmos/relayer/v2/relayer/processor" "golang.org/x/sync/errgroup" "gopkg.in/yaml.v3" ) const ( - check = "✔" - xIcon = "✘" - allowList = "allowlist" - denyList = "denylist" + check = "✔" + xIcon = "✘" ) // Paths represent connection paths between chains @@ -142,9 +141,9 @@ type IBCdata struct { // ValidateChannelFilterRule verifies that the configured ChannelFilter rule is set to an appropriate value. func (p *Path) ValidateChannelFilterRule() error { - if p.Filter.Rule != allowList && p.Filter.Rule != denyList && p.Filter.Rule != "" { + if p.Filter.Rule != processor.RuleAllowList && p.Filter.Rule != processor.RuleDenyList && p.Filter.Rule != "" { return fmt.Errorf("%s is not a valid channel filter rule, please "+ - "ensure your channel filter rule is `%s` or '%s'", p.Filter.Rule, allowList, denyList) + "ensure your channel filter rule is `%s` or '%s'", p.Filter.Rule, processor.RuleAllowList, processor.RuleDenyList) } return nil } diff --git a/relayer/processor/metrics.go b/relayer/processor/metrics.go index 0e267cff1..f5f4245e7 100644 --- a/relayer/processor/metrics.go +++ b/relayer/processor/metrics.go @@ -6,6 +6,7 @@ import ( ) type PrometheusMetrics struct { + Registry *prometheus.Registry PacketObservedCounter *prometheus.CounterVec PacketRelayedCounter *prometheus.CounterVec LatestHeightGauge *prometheus.GaugeVec @@ -37,24 +38,27 @@ func NewPrometheusMetrics() *PrometheusMetrics { packetLabels := []string{"path", "chain", "channel", "port", "type"} heightLabels := []string{"chain"} walletLabels := []string{"chain", "key", "denom"} + registry := prometheus.NewRegistry() + registerer := promauto.With(registry) return &PrometheusMetrics{ - PacketObservedCounter: promauto.NewCounterVec(prometheus.CounterOpts{ + Registry: registry, + PacketObservedCounter: registerer.NewCounterVec(prometheus.CounterOpts{ Name: "cosmos_relayer_observed_packets", Help: "The total number of observed packets", }, packetLabels), - PacketRelayedCounter: promauto.NewCounterVec(prometheus.CounterOpts{ + PacketRelayedCounter: registerer.NewCounterVec(prometheus.CounterOpts{ Name: "cosmos_relayer_relayed_packets", Help: "The total number of relayed packets", }, packetLabels), - LatestHeightGauge: promauto.NewGaugeVec(prometheus.GaugeOpts{ + LatestHeightGauge: registerer.NewGaugeVec(prometheus.GaugeOpts{ Name: "cosmos_relayer_chain_latest_height", Help: "The current height of the chain", }, heightLabels), - WalletBalance: promauto.NewGaugeVec(prometheus.GaugeOpts{ + WalletBalance: registerer.NewGaugeVec(prometheus.GaugeOpts{ Name: "cosmos_relayer_wallet_balance", Help: "The current balance for the relayer's wallet", }, walletLabels), - FeesSpent: promauto.NewGaugeVec(prometheus.GaugeOpts{ + FeesSpent: registerer.NewGaugeVec(prometheus.GaugeOpts{ Name: "cosmos_relayer_fees_spent", Help: "The amount of fees spent from the relayer's wallet", }, walletLabels), diff --git a/relayer/strategies.go b/relayer/strategies.go index 7036cada1..8f302df5c 100644 --- a/relayer/strategies.go +++ b/relayer/strategies.go @@ -286,7 +286,7 @@ func filterOpenChannels(channels []*types.IdentifiedChannel) map[string]*ActiveC // channels to relay on. func applyChannelFilterRule(filter ChannelFilter, channels []*types.IdentifiedChannel) []*types.IdentifiedChannel { switch filter.Rule { - case allowList: + case processor.RuleAllowList: var filteredChans []*types.IdentifiedChannel for _, c := range channels { if filter.InChannelList(c.ChannelId) { @@ -294,7 +294,7 @@ func applyChannelFilterRule(filter ChannelFilter, channels []*types.IdentifiedCh } } return filteredChans - case denyList: + case processor.RuleDenyList: var filteredChans []*types.IdentifiedChannel for _, c := range channels { if filter.InChannelList(c.ChannelId) {