Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

e2e: add client recovery proposal test #2130

Merged
merged 13 commits into from
Aug 30, 2022
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
110 changes: 110 additions & 0 deletions e2e/client_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
package e2e

import (
"context"
"testing"
"time"

"github.com/strangelove-ventures/ibctest/ibc"
"github.com/strangelove-ventures/ibctest/test"
"github.com/stretchr/testify/suite"

"github.com/cosmos/ibc-go/e2e/testsuite"
"github.com/cosmos/ibc-go/e2e/testvalues"
clienttypes "github.com/cosmos/ibc-go/v5/modules/core/02-client/types"
ibcexported "github.com/cosmos/ibc-go/v5/modules/core/exported"
ibctesting "github.com/cosmos/ibc-go/v5/testing"
)

func TestClientTestSuite(t *testing.T) {
suite.Run(t, new(ClientTestSuite))
}

type ClientTestSuite struct {
testsuite.E2ETestSuite
}

// Status queries the current status of the client
func (s *ClientTestSuite) Status(ctx context.Context, chain ibc.Chain, clientID string) (string, error) {
queryClient := s.GetChainGRCPClients(chain).ClientQueryClient
res, err := queryClient.ClientStatus(ctx, &clienttypes.QueryClientStatusRequest{
ClientId: clientID,
})
if err != nil {
return "", err
}

return res.Status, nil
}

func (s *ClientTestSuite) TestClientUpdateProposal_Succeeds() {
t := s.T()
ctx := context.TODO()

var (
relayer ibc.Relayer
subjectClientID string
substituteClientID string
badTrustingPeriod = time.Duration(time.Second)
)

t.Run("create substitute client with correct trusting period", func(t *testing.T) {
relayer, _ = s.SetupChainsRelayerAndChannel(ctx, transferChannelOptions())

// TODO: update when client identifier created is accessible
// currently assumes first client is 07-tendermint-0
substituteClientID = clienttypes.FormatClientIdentifier(ibcexported.Tendermint, 0)
})

chainA, chainB := s.GetChains()
chainAWallet := s.CreateUserOnChainA(ctx, testvalues.StartingTokenAmount)

t.Run("create subject client with bad trusting period", func(t *testing.T) {
createClientOptions := ibc.CreateClientOptions{
TrustingPeriod: badTrustingPeriod.String(),
}

s.SetupClients(ctx, relayer, createClientOptions)

// TODO: update when client identifier created is accessible
// currently assumes second client is 07-tendermint-1
subjectClientID = clienttypes.FormatClientIdentifier(ibcexported.Tendermint, 1)
})

time.Sleep(badTrustingPeriod)

s.Require().NoError(test.WaitForBlocks(ctx, 1, chainA, chainB), "failed to wait for blocks")

t.Run("check status of each client", func(t *testing.T) {
t.Run("substitute should be active", func(t *testing.T) {
status, err := s.Status(ctx, chainA, substituteClientID)
s.Require().NoError(err)
s.Require().Equal(ibcexported.Active.String(), status)
})

t.Run("subject should be expired", func(t *testing.T) {
status, err := s.Status(ctx, chainA, subjectClientID)
s.Require().NoError(err)
s.Require().Equal(ibcexported.Expired.String(), status)
})
})

t.Run("pass client update proposal", func(t *testing.T) {
proposal := clienttypes.NewClientUpdateProposal(ibctesting.Title, ibctesting.Description, subjectClientID, substituteClientID)
s.ExecuteGovProposal(ctx, chainA, chainAWallet, proposal)
})

t.Run("check status of each client", func(t *testing.T) {
t.Run("substitute should be active", func(t *testing.T) {
status, err := s.Status(ctx, chainA, substituteClientID)
s.Require().NoError(err)
s.Require().Equal(ibcexported.Active.String(), status)
})

t.Run("subject should be active", func(t *testing.T) {
status, err := s.Status(ctx, chainA, subjectClientID)
s.Require().NoError(err)
s.Require().Equal(ibcexported.Active.String(), status)
})
})
}
6 changes: 3 additions & 3 deletions e2e/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,11 @@ require (
github.com/cosmos/ibc-go/v5 v5.0.0-beta1
github.com/cosmos/interchain-accounts v0.3.1-0.20220816085955-393d8444c111
github.com/docker/docker v20.10.17+incompatible
github.com/strangelove-ventures/ibctest v0.0.0-20220824150312-9d02bdb119b0
github.com/strangelove-ventures/ibctest v0.0.0-20220824180329-f73a9f936fce
github.com/stretchr/testify v1.8.0
github.com/tendermint/tendermint v0.34.20
go.uber.org/zap v1.21.0
golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4
google.golang.org/grpc v1.48.0
)

Expand Down Expand Up @@ -159,7 +161,6 @@ require (
github.com/tendermint/btcd v0.1.1 // indirect
github.com/tendermint/crypto v0.0.0-20191022145703-50d29ede1e15 // indirect
github.com/tendermint/go-amino v0.16.0 // indirect
github.com/tendermint/tendermint v0.34.20 // indirect
github.com/tendermint/tm-db v0.6.7 // indirect
github.com/ulikunitz/xz v0.5.8 // indirect
github.com/vedhavyas/go-subkey v1.0.3 // indirect
Expand All @@ -169,7 +170,6 @@ require (
go.uber.org/atomic v1.9.0 // indirect
go.uber.org/multierr v1.8.0 // indirect
golang.org/x/crypto v0.0.0-20220525230936-793ad666bf5e // indirect
golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4 // indirect
golang.org/x/net v0.0.0-20220617184016-355a448f1bc9 // indirect
golang.org/x/oauth2 v0.0.0-20220411215720-9780585627b5 // indirect
golang.org/x/sync v0.0.0-20220513210516-0976fa681c29 // indirect
Expand Down
4 changes: 2 additions & 2 deletions e2e/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -1255,8 +1255,8 @@ github.com/status-im/keycard-go v0.0.0-20190316090335-8537d3370df4/go.mod h1:RZL
github.com/stbenjam/no-sprintf-host-port v0.1.1/go.mod h1:TLhvtIvONRzdmkFiio4O8LHsN9N74I+PhRquPsxpL0I=
github.com/strangelove-ventures/go-subkey v1.0.7 h1:cOP/Lajg3uxV/tvspu0m6+0Cu+DJgygkEAbx/s+f35I=
github.com/strangelove-ventures/go-subkey v1.0.7/go.mod h1:E34izOIEm+sZ1YmYawYRquqBQWeZBjVB4pF7bMuhc1c=
github.com/strangelove-ventures/ibctest v0.0.0-20220824150312-9d02bdb119b0 h1:IBb9ktwUfrLcBLCAZz2Yf9qMz98U/1Nc1HpQFhA92qQ=
github.com/strangelove-ventures/ibctest v0.0.0-20220824150312-9d02bdb119b0/go.mod h1:fwqMUdv+pE55HipI/b+7+8xbjrqePGYTEHoZOtWvTqQ=
github.com/strangelove-ventures/ibctest v0.0.0-20220824180329-f73a9f936fce h1:CwbsETcaS9sRbcRdiwfxlZTn3hrpEYW2vAvMTPJDa4M=
github.com/strangelove-ventures/ibctest v0.0.0-20220824180329-f73a9f936fce/go.mod h1:fwqMUdv+pE55HipI/b+7+8xbjrqePGYTEHoZOtWvTqQ=
github.com/streadway/amqp v0.0.0-20190404075320-75d898a42a94/go.mod h1:AZpEONHx3DKn8O/DFsRAY58/XVQiIPMTMB1SddzLXVw=
github.com/streadway/amqp v1.0.0/go.mod h1:AZpEONHx3DKn8O/DFsRAY58/XVQiIPMTMB1SddzLXVw=
github.com/streadway/handy v0.0.0-20200128134331-0f66f006fb2e/go.mod h1:qNTQ5P5JnDBl6z3cMAg/SywNDC5ABu5ApDIw6lUbRmI=
Expand Down
15 changes: 13 additions & 2 deletions e2e/testsuite/testsuite.go
Original file line number Diff line number Diff line change
Expand Up @@ -108,8 +108,7 @@ func (s *E2ETestSuite) SetupChainsRelayerAndChannel(ctx context.Context, channel

r := newCosmosRelayer(s.T(), testconfig.FromEnv(), s.logger, s.DockerClient, s.network)

pathName := fmt.Sprintf("%s-path", s.T().Name())
pathName = strings.ReplaceAll(pathName, "/", "-")
pathName := s.getPathName()

ic := ibctest.NewInterchain().
AddChain(chainA).
Expand Down Expand Up @@ -157,6 +156,18 @@ func (s *E2ETestSuite) SetupChainsRelayerAndChannel(ctx context.Context, channel
return r, chainAChannels[len(chainAChannels)-1]
}

// getPathName generates the path name using the test suites name
func (s *E2ETestSuite) getPathName() string {
pathName := fmt.Sprintf("%s-path", s.T().Name())
return strings.ReplaceAll(pathName, "/", "-")
}

// SetupClients creates clients on chainA and chainB using the provided create client options
func (s *E2ETestSuite) SetupClients(ctx context.Context, relayer ibc.Relayer, opts ibc.CreateClientOptions) {
err := relayer.CreateClients(ctx, s.GetRelayerExecReporter(), s.getPathName(), opts)
s.Require().NoError(err)
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is currently not working. My guess is it is no-oping because a client already exists for the given path name. I think we need to generate/add a path for every client/connection/channel (aka endpoint) stack. My vote would be to add the endpoint type and when you call NewEndpoint, it adds the ibctest path name

Copy link

@boojamya boojamya Aug 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Soo looks like you're right, that new client is not being created.
The easy solution here is to wire up the --override flag in ibctest for the relayers rly tx clients --override <path> command.

The client is not begin created b/c in the relayer config, the client already exists so it does not create a new client.
This is what the --override flag is used for; to create a new client even though the config is showing a client-id already exists. But, it is currently not working. Issue here: cosmos/relayer#952

In the meantime, one dirty solution that I think will work is to create another relayer instances with a different home folder path. Then create the new "substituteClient" with the bad trusting period that way.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generating a new path (with a new path name) worked


// GetChains returns two chains that can be used in a test. The pair returned
// is unique to the current test being run. Note: this function does not create containers.
func (s *E2ETestSuite) GetChains(chainOpts ...testconfig.ChainOptionConfiguration) (*cosmos.CosmosChain, *cosmos.CosmosChain) {
Expand Down