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

test: e2e test for timeout of ics20 packet #1992

Merged
merged 5 commits into from
Aug 16, 2022
Merged

test: e2e test for timeout of ics20 packet #1992

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
eafa496
test: e2e test for timeout of ics20 packet
seantking Aug 11, 2022
d7d515a
chore: adding option for manual testing
seantking Aug 11, 2022
2c37cab
Merge branch 'main' into sean/issue#1975-timeout-ibc-transfer
seantking Aug 15, 2022
38b0ac0
nits
seantking Aug 16, 2022
07dc49b
Merge branch 'main' into sean/issue#1975-timeout-ibc-transfer
seantking Aug 16, 2022
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
1 change: 1 addition & 0 deletions .github/workflows/e2e-manual.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ on:
options:
- TestTransferTestSuite
- TestFeeMiddlewareTestSuite
- TestTransferTestSuite
Copy link
Member

Choose a reason for hiding this comment

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

Should this be duplicated? I guess we can remove, must be from merging main.

chain-a-image:
description: 'The image to use for chain A'
required: true
Expand Down
48 changes: 48 additions & 0 deletions e2e/transfer_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"context"
"fmt"
"testing"
"time"

sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/strangelove-ventures/ibctest"
Expand Down Expand Up @@ -114,6 +115,53 @@ func (s *TransferTestSuite) TestMsgTransfer_Succeeds_Nonincentivized() {
})
}

func (s *TransferTestSuite) TestMsgTransfer_Timeout_Nonincentivized() {
t := s.T()
ctx := context.TODO()

relayer, channelA := s.SetupChainsRelayerAndChannel(ctx, transferChannelOptions())
chainA, chainB := s.GetChains()
chainAWallet := s.CreateUserOnChainA(ctx, testvalues.StartingTokenAmount)
chainBWallet := s.CreateUserOnChainB(ctx, testvalues.StartingTokenAmount)

chainBWalletAmount := ibc.WalletAmount{
Address: chainBWallet.Bech32Address(chainB.Config().Bech32Prefix), // destination address
Denom: chainA.Config().Denom,
Amount: testvalues.IBCTransferAmount,
}

t.Run("IBC transfer packet timesout", func(t *testing.T) {
Copy link
Member

Choose a reason for hiding this comment

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

nit: should we make "IBC transfer packet timeout" or "IBC transfer packet times out"

tx, err := chainA.SendIBCTransfer(ctx, channelA.ChannelID, chainAWallet.KeyName, chainBWalletAmount, testvalues.ImmediatelyTimeout())
Copy link
Contributor Author

@seantking seantking Aug 11, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

@colin-axner I tried to use your helper function for broadcasting the message instead of using the built-in library fn but for some reason the packet never timesout. Everything works as expected when using the built in transfer helper.

Not sure if this is a golang relayer issue or if the message doesn't get broadcast correctly.

Copy link
Contributor

Choose a reason for hiding this comment

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

should we create an issue to investigate this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Agreed, I'd like to understand why this is the case?

Copy link
Contributor

Choose a reason for hiding this comment

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

The issue is that the cli takes in a relative timeout. So the in this instance, 1 is being passed for the timeout timestamp which is added to the latest known consensus state timestamp (which passes for sending a packet). In the function I added, you have to pass the absolute timeout

s.Require().NoError(err)
s.Require().NoError(tx.Validate(), "source ibc transfer tx is invalid")
time.Sleep(time.Nanosecond * 1) // want it to timeout immediately
Copy link
Member

Choose a reason for hiding this comment

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

same difference I think

Suggested change
time.Sleep(time.Nanosecond * 1) // want it to timeout immediately
time.Sleep(time.Nanosecond) // want it to timeout immediately

})

t.Run("tokens are escrowed", func(t *testing.T) {
actualBalance, err := s.GetChainANativeBalance(ctx, chainAWallet)
s.Require().NoError(err)

expected := testvalues.StartingTokenAmount - testvalues.IBCTransferAmount
s.Require().Equal(expected, actualBalance)
})

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

t.Run("ensure escrowed tokens have been refunded to sender due to timeout", func(t *testing.T) {
// ensure destination address did not recieve any tokens
bal, err := s.GetChainBNativeBalance(ctx, chainBWallet)
s.Require().NoError(err)
s.Require().Equal(testvalues.StartingTokenAmount, bal)

// ensure that the sender address has been successfully refunded the full amount
bal, err = s.GetChainANativeBalance(ctx, chainAWallet)
s.Require().NoError(err)
s.Require().Equal(testvalues.StartingTokenAmount, bal)
})
}

// transferChannelOptions configures both of the chains to have non-incentivized transfer channels.
func transferChannelOptions() func(options *ibc.CreateChannelOptions) {
return func(opts *ibc.CreateChannelOptions) {
Expand Down