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.

Sign up for GitHub

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

Jump to bottom

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

Merged
merged 8 commits into from
Jun 21, 2024
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
7a2b235
test(transfer): last chain in forwarding being ICS20 v1
gjermundgaraba Jun 17, 2024
4e23aff
Merge branch 'feat/ics20-v2-path-forwarding' into gjermund/6557-add-t…
gjermundgaraba Jun 19, 2024
6d9c9ea
Merge branch 'feat/ics20-v2-path-forwarding' into gjermund/6557-add-t…
gjermundgaraba Jun 21, 2024
4326e6f
Finish TestForwarding_WithLastChainBeingICS20v1_Succeeds
gjermundgaraba Jun 21, 2024
d460d8c
Merge branch 'feat/ics20-v2-path-forwarding' into gjermund/6557-add-t…
gjermundgaraba Jun 21, 2024
7ff0760
Merge branch 'feat/ics20-v2-path-forwarding' into gjermund/6557-add-t…
bznein Jun 21, 2024
1f19d33
Update CreateNewPath signature
gjermundgaraba Jun 21, 2024
d118811
Fix PR review comments
gjermundgaraba Jun 21, 2024
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
70 changes: 70 additions & 0 deletions e2e/tests/transfer/forwarding_test.go
View file
Open in desktop
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.

[q] can we use here https://github.com/cosmos/ibc-go/blob/feat%2Fics20-v2-path-forwarding/testing/chain.go#L634 instead?

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())
})
}
74 changes: 44 additions & 30 deletions e2e/testsuite/testsuite.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -177,52 +177,66 @@ 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,
) (chainAChannel ibc.ChannelOutput, chainBChannel ibc.ChannelOutput) {
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