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

test(transfer): last chain in forwarding packet is ICS20 v1 #6622

70 changes: 70 additions & 0 deletions e2e/tests/transfer/forwarding_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,16 @@ package transfer
import (
"context"
"testing"
"time"

"github.com/strangelove-ventures/interchaintest/v8/ibc"
testifysuite "github.com/stretchr/testify/suite"

"github.com/cosmos/ibc-go/e2e/testsuite"
"github.com/cosmos/ibc-go/e2e/testsuite/query"
"github.com/cosmos/ibc-go/e2e/testvalues"
transfertypes "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types"
clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types"
)

func TestTransferForwardingTestSuite(t *testing.T) {
Expand Down Expand Up @@ -95,3 +99,69 @@ func (s *TransferForwardingTestSuite) TestThreeChainSetup() {
s.Require().Equal(expected, actualBalance.Int64())
})
}

func (s *TransferForwardingTestSuite) TestForwarding_WithLastChainBeingICS20v1_Succeeds() {
ctx := context.TODO()
t := s.T()

relayer, chains := s.GetRelayer(), s.GetAllChains()

chainA, chainB, chainC := chains[0], chains[1], chains[2]

channelAtoB := s.GetChainAChannel()

// Creating a new path between chain B and chain C with a ICS20-v1 channel
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 went a bit back and forth on how to do this and in the end I thought the cleanest way was to just create a new path for this test because the alternative would be to move this to a separate TestSuite which I didn't like very much.

opts := s.TransferChannelOptions()
opts.Version = transfertypes.V1
channelBtoC, _ := s.CreateNewPath(ctx, chainB, chainC, ibc.DefaultClientOpts(), opts)
s.Require().Equal(transfertypes.V1, channelBtoC.Version, "the channel version is not ics20-1")

chainAWallet := s.CreateUserOnChainA(ctx, testvalues.StartingTokenAmount)
chainAAddress := chainAWallet.FormattedAddress()
chainADenom := chainA.Config().Denom

chainCWallet := s.CreateUserOnChainC(ctx, testvalues.StartingTokenAmount)
chainCAddress := chainCWallet.FormattedAddress()

t.Run("IBC transfer from A to C with forwarding through B", func(t *testing.T) {
inFiveMinutes := time.Now().Add(5 * time.Minute).UnixNano()
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately no. I have not found a nice way to do this. The best option I had was a function that queries the latest header and gets the current block time from that. But since these are anyway running in real time in the background, I think just using current time will always work.

Two options I see to improve this:

  • Implement the query of latest block header or status from a node
  • Make a little helper function for getting a reasonable timeout (just putting this in a function, basically)

forwarding := transfertypes.NewForwarding(false, transfertypes.Hop{
PortId: channelBtoC.PortID,
ChannelId: channelBtoC.ChannelID,
})

msgTransfer := testsuite.GetMsgTransfer(
channelAtoB.PortID,
channelAtoB.ChannelID,
transfertypes.V2,
testvalues.DefaultTransferCoins(chainADenom),
chainAAddress,
chainCAddress,
clienttypes.ZeroHeight(),
uint64(inFiveMinutes),
"",
forwarding)
resp := s.BroadcastMessages(ctx, chainA, chainAWallet, msgTransfer)
s.AssertTxSuccess(resp)
})

t.Run("start relayer", func(t *testing.T) {
s.StartRelayer(relayer)
})

t.Run("packets are relayed from A to B to C", func(t *testing.T) {
chainCDenom := transfertypes.NewDenom(chainADenom,
transfertypes.NewTrace(channelBtoC.Counterparty.PortID, channelBtoC.Counterparty.ChannelID),
transfertypes.NewTrace(channelAtoB.Counterparty.PortID, channelAtoB.Counterparty.ChannelID),
)

s.AssertPacketRelayed(ctx, chainA, channelAtoB.PortID, channelAtoB.ChannelID, 1)
s.AssertPacketRelayed(ctx, chainB, channelBtoC.PortID, channelBtoC.ChannelID, 1)

actualBalance, err := query.Balance(ctx, chainC, chainCAddress, chainCDenom.IBCDenom())
s.Require().NoError(err)

expected := testvalues.IBCTransferAmount
s.Require().Equal(expected, actualBalance.Int64())
})
}
68 changes: 38 additions & 30 deletions e2e/testsuite/testsuite.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,52 +177,60 @@ func (s *E2ETestSuite) SetupTest() {
s.SetupPath(ibc.DefaultClientOpts(), defaultChannelOpts(s.GetAllChains()))
}

// SetupPath creates a path between the chains using the provided client and channel options.
// SetupPath creates paths between the all chains using the provided client and channel options.
func (s *E2ETestSuite) SetupPath(clientOpts ibc.CreateClientOptions, channelOpts ibc.CreateChannelOptions) {
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 was tempted to rename this to SetupPaths so the method below could be named SetupPath. Would like feedback if we agree on that (and the addition of the method below, obviously) and I can do that in a separate PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

If it creates more than one path, I am absolutely in agreement on renaming :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it used to create only one path always, and still does whenever there are only two chains. But it now creates paths such that all chains are connected in a line A->B->C->etc.

s.T().Logf("Setting up path for: %s", s.T().Name())
r := s.relayer

ctx := context.TODO()
allChains := s.GetAllChains()
for i := 0; i < len(allChains)-1; i++ {
chainA, chainB := allChains[i], allChains[i+1]
pathName := s.generatePathName()
s.T().Logf("establishing path between %s and %s on path %s", chainA.Config().ChainID, chainB.Config().ChainID, pathName)
_, _ = s.CreateNewPath(ctx, chainA, chainB, clientOpts, channelOpts)
}
}

func (s *E2ETestSuite) CreateNewPath(ctx context.Context, chainA ibc.Chain, chainB ibc.Chain, clientOpts ibc.CreateClientOptions, channelOpts ibc.CreateChannelOptions) (ibc.ChannelOutput, ibc.ChannelOutput) {
Copy link
Contributor

Choose a reason for hiding this comment

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

[nit] when returning more than one value of the same type, I like using named return values

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That would make it clearer, agreed! Will do that :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done. But I also ended up putting back the type for chainA because the signature got so long that I line split it.

gjermundgaraba marked this conversation as resolved.
Show resolved Hide resolved
r := s.relayer

err := r.GeneratePath(ctx, s.GetRelayerExecReporter(), chainA.Config().ChainID, chainB.Config().ChainID, pathName)
s.Require().NoError(err)
pathName := s.generatePathName()
s.T().Logf("establishing path between %s and %s on path %s", chainA.Config().ChainID, chainB.Config().ChainID, pathName)

// Create new clients
err = r.CreateClients(ctx, s.GetRelayerExecReporter(), pathName, clientOpts)
s.Require().NoError(err)
err = test.WaitForBlocks(ctx, 1, chainA, chainB)
s.Require().NoError(err)
err := r.GeneratePath(ctx, s.GetRelayerExecReporter(), chainA.Config().ChainID, chainB.Config().ChainID, pathName)
s.Require().NoError(err)

err = r.CreateConnections(ctx, s.GetRelayerExecReporter(), pathName)
s.Require().NoError(err)
err = test.WaitForBlocks(ctx, 1, chainA, chainB)
s.Require().NoError(err)
// Create new clients
err = r.CreateClients(ctx, s.GetRelayerExecReporter(), pathName, clientOpts)
s.Require().NoError(err)
err = test.WaitForBlocks(ctx, 1, chainA, chainB)
s.Require().NoError(err)

err = r.CreateChannel(ctx, s.GetRelayerExecReporter(), pathName, channelOpts)
s.Require().NoError(err)
err = test.WaitForBlocks(ctx, 1, chainA, chainB)
s.Require().NoError(err)
err = r.CreateConnections(ctx, s.GetRelayerExecReporter(), pathName)
s.Require().NoError(err)
err = test.WaitForBlocks(ctx, 1, chainA, chainB)
s.Require().NoError(err)

channelsA, err := r.GetChannels(ctx, s.GetRelayerExecReporter(), chainA.Config().ChainID)
s.Require().NoError(err)
err = r.CreateChannel(ctx, s.GetRelayerExecReporter(), pathName, channelOpts)
s.Require().NoError(err)
err = test.WaitForBlocks(ctx, 1, chainA, chainB)
s.Require().NoError(err)

channelsB, err := r.GetChannels(ctx, s.GetRelayerExecReporter(), chainB.Config().ChainID)
s.Require().NoError(err)
channelsA, err := r.GetChannels(ctx, s.GetRelayerExecReporter(), chainA.Config().ChainID)
s.Require().NoError(err)

if s.channels[s.T().Name()] == nil {
s.channels[s.T().Name()] = make(map[ibc.Chain][]ibc.ChannelOutput)
}
channelsB, err := r.GetChannels(ctx, s.GetRelayerExecReporter(), chainB.Config().ChainID)
s.Require().NoError(err)

// keep track of channels associated with a given chain for access within the tests.
s.channels[s.T().Name()][chainA] = channelsA
s.channels[s.T().Name()][chainB] = channelsB
s.testPaths[s.T().Name()] = append(s.testPaths[s.T().Name()], pathName)
if s.channels[s.T().Name()] == nil {
s.channels[s.T().Name()] = make(map[ibc.Chain][]ibc.ChannelOutput)
}

// keep track of channels associated with a given chain for access within the tests.
s.channels[s.T().Name()][chainA] = channelsA
s.channels[s.T().Name()][chainB] = channelsB

s.testPaths[s.T().Name()] = append(s.testPaths[s.T().Name()], pathName)

return channelsA[len(channelsA)-1], channelsB[len(channelsB)-1]
}

// GetChainAChannel returns the ibc.ChannelOutput for the current test.
Expand Down
Loading