test: e2e test for timeout of ics20 packet #1992
Conversation
seantking
requested review from
colin-axner,
AdityaSripal,
crodriguezvega,
charleenfei,
damiannolan and
chatton
as code owners
seantking
changed the title
| } | ||
|
|
||
| t.Run("IBC transfer packet timesout", func(t *testing.T) { | ||
| tx, err := chainA.SendIBCTransfer(ctx, channelA.ChannelID, chainAWallet.KeyName, chainBWalletAmount, testvalues.ImmediatelyTimeout()) |
There was a problem hiding this comment.
@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.
There was a problem hiding this comment.
should we create an issue to investigate this?
There was a problem hiding this comment.
Agreed, I'd like to understand why this is the case?
There was a problem hiding this comment.
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
Codecov Report
@@ Coverage Diff @@
## main #1992 +/- ##
==========================================
+ Coverage 80.09% 80.10% +0.01%
==========================================
Files 167 167
Lines 11773 11771 -2
==========================================
Hits 9429 9429
+ Misses 1929 1927 -2
Partials 415 415
|
| } | ||
|
|
||
| t.Run("IBC transfer packet timesout", func(t *testing.T) { | ||
| tx, err := chainA.SendIBCTransfer(ctx, channelA.ChannelID, chainAWallet.KeyName, chainBWalletAmount, testvalues.ImmediatelyTimeout()) |
There was a problem hiding this comment.
Agreed, I'd like to understand why this is the case?
.github/workflows/e2e-manual.yaml
Outdated
| @@ -12,6 +12,7 @@ on: | |||
| options: | |||
| - TestTransferTestSuite | |||
| - TestFeeMiddlewareTestSuite | |||
| - TestTransferTestSuite | |||
There was a problem hiding this comment.
Should this be duplicated? I guess we can remove, must be from merging main.
| Amount: testvalues.IBCTransferAmount, | ||
| } | ||
|
|
||
| t.Run("IBC transfer packet timesout", func(t *testing.T) { |
There was a problem hiding this comment.
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()) | ||
| 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 |
There was a problem hiding this comment.
same difference I think
| time.Sleep(time.Nanosecond * 1) // want it to timeout immediately | |
| time.Sleep(time.Nanosecond) // want it to timeout immediately |
Description
closes: #1975
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/) or specification (x/<module>/spec/)godoccomments.Unreleasedsection inCHANGELOG.mdFiles changedin the Github PR explorerCodecov Reportin the comment section below once CI passes