-
Notifications
You must be signed in to change notification settings - Fork 592
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: timeout transfer with grandpa light client #5018
Changes from 187 commits
01e8548
40337dd
90ed894
cf6fbfe
28d46c1
255905a
8bc0c68
b6e8a3a
2901cf8
581a27f
5a1a249
3607dd2
9298fe6
731dbbf
59ccad5
62ed4fb
341350a
f822b4f
bffb649
bf9d465
ed41863
de89920
644367f
20d4d7e
54f29ee
c4f8da4
2e17bb2
49c5e7d
4f0a711
479f470
2a7c242
fb9eed5
03d077b
16e1026
2913dbd
83ca9bb
fb745e8
1425ca2
17d255c
8f5354b
120d45b
81086e4
94f380d
12d74a9
bbec01f
090bc9f
1bc5845
aa8598d
70efe3a
95c9e29
c87c590
2d63b4a
60e565b
4f0198c
43106d5
540db8c
159343d
61db385
18a2185
5bff313
f6be1b8
64fe692
c7324c2
48cadc5
595b8b9
e9c0862
176a497
9a7b9b7
be3e2ff
b863dd3
b61ede0
86478ce
7032548
3f4061b
1c7e148
0ea6d08
551fb95
42ad3cf
8c9c717
32e4bd7
bd8e201
cd7b5d6
10ffafa
2ff2052
bc380a4
c9f3ba5
403af69
2a39e7f
bde2a2f
b54ac97
76bae0d
ab35cf4
bcdceab
e3790f5
19368c9
335286d
f02c619
fb594f6
5121172
02fe742
6f4102e
e655794
8a0eda1
40ba894
dd6d028
e2522b2
8c1ed9a
cb38b85
1b4ae46
4fa827a
6eb154c
a48b631
90027e4
90e345a
1d31c73
0abc469
95a1237
890ef9d
eea9a7d
433f1fa
ddb514d
56fb2af
1fa22f3
b0142e7
995a3bd
9655a7b
bdcc012
7fd07ad
16f7d6c
20c73fd
0e13d2e
fe602e0
9e15947
33993ba
30251d4
efedb95
147f0ca
243f36f
729cb09
1e8e25d
0be3f39
1ac7113
d7b99e8
103510a
c8e9a09
4d67e4c
3a3a998
d22682d
bc1f21a
8c485a1
ee88c74
e9c2be6
1c70944
86d55a2
d982d5d
d3f8709
33d1315
d0c05d5
0bee18b
bde614e
826df9e
c53bac7
825bf2b
716fcdc
f4ddaee
1cf7a2f
c24f8ed
a006cb2
4fe4530
2cb6729
e71b80f
edc525f
7b377ea
3b1cb9a
ff6f9a7
c983119
c0402b9
a344eb5
c5c6d78
4df51ff
90f57ae
769431c
56a1e6b
c842f39
554e53c
c8936c5
558c204
3d9ffb1
8ff31e1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -73,6 +73,7 @@ type GrandpaTestSuite struct { | |
// * send transfer over ibc | ||
func (s *GrandpaTestSuite) TestMsgTransfer_Succeeds_GrandpaContract() { | ||
ctx := context.Background() | ||
t := s.T() | ||
|
||
chainA, chainB := s.GetGrandpaTestChains() | ||
|
||
|
@@ -109,6 +110,15 @@ func (s *GrandpaTestSuite) TestMsgTransfer_Succeeds_GrandpaContract() { | |
fundAmount := int64(12_333_000_000_000) | ||
polkadotUser, cosmosUser := s.fundUsers(ctx, fundAmount, polkadotChain, cosmosChain) | ||
|
||
// TODO: this can be refactored to broadcast a MsgTransfer instead of CLI. | ||
// https://github.com/cosmos/ibc-go/issues/4963 | ||
amountToSend := int64(1_770_000) | ||
transfer := ibc.WalletAmount{ | ||
Address: polkadotUser.FormattedAddress(), | ||
Denom: cosmosChain.Config().Denom, | ||
Amount: math.NewInt(amountToSend), | ||
} | ||
|
||
pathName := s.GetPathName(0) | ||
|
||
err = r.GeneratePath(ctx, eRep, cosmosChain.Config().ChainID, polkadotChain.Config().ChainID, pathName) | ||
|
@@ -135,79 +145,110 @@ func (s *GrandpaTestSuite) TestMsgTransfer_Succeeds_GrandpaContract() { | |
// Start relayer | ||
s.Require().NoError(r.StartRelayer(ctx, eRep, pathName)) | ||
|
||
// TODO: this can be refactored to broadcast a MsgTransfer instead of CLI. | ||
// https://github.com/cosmos/ibc-go/issues/4963 | ||
// Send 1.77 stake from cosmosUser to parachainUser | ||
amountToSend := int64(1_770_000) | ||
transfer := ibc.WalletAmount{ | ||
Address: polkadotUser.FormattedAddress(), | ||
Denom: cosmosChain.Config().Denom, | ||
Amount: math.NewInt(amountToSend), | ||
} | ||
tx, err := cosmosChain.SendIBCTransfer(ctx, "channel-0", cosmosUser.KeyName(), transfer, ibc.TransferOptions{}) | ||
s.Require().NoError(err) | ||
s.Require().NoError(tx.Validate()) // test source wallet has decreased funds | ||
err = testutil.WaitForBlocks(ctx, 15, cosmosChain, polkadotChain) | ||
s.Require().NoError(err) | ||
|
||
// Verify tokens arrived on parachain user | ||
parachainUserStake, err := polkadotChain.GetIbcBalance(ctx, string(polkadotUser.Address()), 2) | ||
s.Require().NoError(err) | ||
s.Require().Equal(amountToSend, parachainUserStake.Amount.Int64(), "parachain user's stake amount not expected after first tx") | ||
|
||
// Send 1.16 stake from parachainUser to cosmosUser | ||
amountToReflect := int64(1_160_000) | ||
reflectTransfer := ibc.WalletAmount{ | ||
Address: cosmosUser.FormattedAddress(), | ||
Denom: "2", // stake | ||
Amount: math.NewInt(amountToReflect), | ||
} | ||
_, err = polkadotChain.SendIBCTransfer(ctx, "channel-0", polkadotUser.KeyName(), reflectTransfer, ibc.TransferOptions{}) | ||
s.Require().NoError(err) | ||
|
||
// Send 1.88 "UNIT" from Alice to cosmosUser | ||
amountUnits := math.NewInt(1_880_000_000_000) | ||
unitTransfer := ibc.WalletAmount{ | ||
Address: cosmosUser.FormattedAddress(), | ||
Denom: "1", // UNIT | ||
Amount: amountUnits, | ||
} | ||
_, err = polkadotChain.SendIBCTransfer(ctx, "channel-0", "alice", unitTransfer, ibc.TransferOptions{}) | ||
s.Require().NoError(err) | ||
|
||
// Wait for MsgRecvPacket on cosmos chain | ||
finalStakeBal := math.NewInt(fundAmount - amountToSend + amountToReflect) | ||
err = cosmos.PollForBalance(ctx, cosmosChain, 20, ibc.WalletAmount{ | ||
Address: cosmosUser.FormattedAddress(), | ||
Denom: cosmosChain.Config().Denom, | ||
Amount: finalStakeBal, | ||
t.Run("IBC transfer from Cosmos chain to Polkadot parachain times out", func(t *testing.T) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. did we want to add this as a completely separate test? Right now this is being added to TestMsgTransfer_Succeeds_GrandpaContract, right? This is maybe a little misleading. What do you think? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I did it as part of this test bc the setup takes so long for these tests (due to gov proposal needed to create a wasm client, etc). If you think it's worth it to add as a completely separate test I can do it, but tbh I really don't know if it's worth the setup time, as the functionality can easily be tested in the transfer flow. I could update the test name for clarity, as another option. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree with you that the test setup is pretty long winded and takes up quite a bit of time. But I think it would just be at the expense of one extra runner, right? General separation of concerns would have me leaning towards a separate test, even tho there's also probably some code duplication. I'd be fine with leaving it in this one too if there's consensus, but maybe we could rename the test if that were the case. Any thoughts? cc. @chatton @DimitrisJim @srdtrk There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd lean towards a separate test too, in most cases these tests will be running side by side anyway, so the time difference shouldn't be noticeable. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd prefer a separate but I'd be fine if done in separate PR so Charly can get that long awaited satisfaction from finally merging this son of a gun There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. alright! i can make a separate test, no worries. I'd rather do it now than revisit later haha, close it up and zip it up. |
||
// Stop relayer | ||
s.Require().NoError(r.StopRelayer(ctx, s.GetRelayerExecReporter())) | ||
|
||
tx, err := cosmosChain.SendIBCTransfer(ctx, "channel-0", cosmosUser.KeyName(), transfer, ibc.TransferOptions{Timeout: 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 | ||
|
||
// check that tokens are escrowed | ||
actualBalance, err := cosmosChain.GetBalance(ctx, cosmosUser.FormattedAddress(), cosmosChain.Config().Denom) | ||
s.Require().NoError(err) | ||
expected := fundAmount - amountToSend | ||
s.Require().Equal(expected, actualBalance.Int64()) | ||
|
||
// start relayer | ||
s.Require().NoError(r.StartRelayer(ctx, s.GetRelayerExecReporter(), s.GetPathName(0))) | ||
err = testutil.WaitForBlocks(ctx, 15, polkadotChain, cosmosChain) | ||
s.Require().NoError(err) | ||
|
||
// ensure that receiver on parachain did not receive any tokens | ||
receiverBalance, err := polkadotChain.GetIbcBalance(ctx, polkadotUser.FormattedAddress(), 2) | ||
s.Require().NoError(err) | ||
s.Require().Equal(int64(0), receiverBalance.Amount.Int64()) | ||
|
||
// check that tokens have been refunded to sender address | ||
senderBalance, err := cosmosChain.GetBalance(ctx, cosmosUser.FormattedAddress(), cosmosChain.Config().Denom) | ||
s.Require().NoError(err) | ||
s.Require().Equal(fundAmount, senderBalance.Int64()) | ||
}) | ||
s.Require().NoError(err) | ||
|
||
// Wait for a new update state | ||
err = testutil.WaitForBlocks(ctx, 5, cosmosChain, polkadotChain) | ||
s.Require().NoError(err) | ||
|
||
// Verify cosmos user's final "stake" balance | ||
cosmosUserStakeBal, err := cosmosChain.GetBalance(ctx, cosmosUser.FormattedAddress(), cosmosChain.Config().Denom) | ||
s.Require().NoError(err) | ||
s.Require().True(cosmosUserStakeBal.Equal(finalStakeBal)) | ||
|
||
// Verify cosmos user's final "unit" balance | ||
unitDenomTrace := transfertypes.ParseDenomTrace(transfertypes.GetPrefixedDenom("transfer", "channel-0", "UNIT")) | ||
cosmosUserUnitBal, err := cosmosChain.GetBalance(ctx, cosmosUser.FormattedAddress(), unitDenomTrace.IBCDenom()) | ||
s.Require().NoError(err) | ||
s.Require().True(cosmosUserUnitBal.Equal(amountUnits)) | ||
|
||
// Verify parachain user's final "unit" balance (will be less than expected due gas costs for stake tx) | ||
parachainUserUnits, err := polkadotChain.GetIbcBalance(ctx, string(polkadotUser.Address()), 1) | ||
s.Require().NoError(err) | ||
s.Require().True(parachainUserUnits.Amount.LTE(math.NewInt(fundAmount)), "parachain user's final unit amount not expected") | ||
t.Run("send successful IBC transfer from Cosmos to Polkadot parachain", func(t *testing.T) { | ||
// Send 1.77 stake from cosmosUser to parachainUser | ||
tx, err := cosmosChain.SendIBCTransfer(ctx, "channel-0", cosmosUser.KeyName(), transfer, ibc.TransferOptions{}) | ||
s.Require().NoError(tx.Validate(), "source ibc transfer tx is invalid") | ||
s.Require().NoError(err) | ||
// verify token balance for cosmos user has decreased | ||
balance, err := cosmosChain.GetBalance(ctx, cosmosUser.FormattedAddress(), cosmosChain.Config().Denom) | ||
s.Require().NoError(err) | ||
s.Require().Equal(balance, math.NewInt(fundAmount-amountToSend), "unexpected cosmos user balance after first tx") | ||
err = testutil.WaitForBlocks(ctx, 15, cosmosChain, polkadotChain) | ||
s.Require().NoError(err) | ||
|
||
// Verify tokens arrived on parachain user | ||
parachainUserStake, err := polkadotChain.GetIbcBalance(ctx, string(polkadotUser.Address()), 2) | ||
s.Require().NoError(err) | ||
s.Require().Equal(amountToSend, parachainUserStake.Amount.Int64(), "unexpected parachain user balance after first tx") | ||
}) | ||
|
||
// Verify parachain user's final "stake" balance | ||
parachainUserStake, err = polkadotChain.GetIbcBalance(ctx, string(polkadotUser.Address()), 2) | ||
s.Require().NoError(err) | ||
s.Require().True(parachainUserStake.Amount.Equal(math.NewInt(amountToSend-amountToReflect)), "parachain user's final stake amount not expected") | ||
t.Run("send two successful IBC transfers from Polkadot parachain to Cosmos, first with ibc denom, second with parachain denom", func(t *testing.T) { | ||
// Send 1.16 stake from parachainUser to cosmosUser | ||
amountToReflect := int64(1_160_000) | ||
reflectTransfer := ibc.WalletAmount{ | ||
Address: cosmosUser.FormattedAddress(), | ||
Denom: "2", // stake | ||
Amount: math.NewInt(amountToReflect), | ||
} | ||
_, err := polkadotChain.SendIBCTransfer(ctx, "channel-0", polkadotUser.KeyName(), reflectTransfer, ibc.TransferOptions{}) | ||
s.Require().NoError(err) | ||
|
||
// Send 1.88 "UNIT" from Alice to cosmosUser | ||
amountUnits := math.NewInt(1_880_000_000_000) | ||
unitTransfer := ibc.WalletAmount{ | ||
Address: cosmosUser.FormattedAddress(), | ||
Denom: "1", // UNIT | ||
Amount: amountUnits, | ||
} | ||
_, err = polkadotChain.SendIBCTransfer(ctx, "channel-0", "alice", unitTransfer, ibc.TransferOptions{}) | ||
s.Require().NoError(err) | ||
|
||
// Wait for MsgRecvPacket on cosmos chain | ||
finalStakeBal := math.NewInt(fundAmount - amountToSend + amountToReflect) | ||
err = cosmos.PollForBalance(ctx, cosmosChain, 20, ibc.WalletAmount{ | ||
Address: cosmosUser.FormattedAddress(), | ||
Denom: cosmosChain.Config().Denom, | ||
Amount: finalStakeBal, | ||
}) | ||
s.Require().NoError(err) | ||
|
||
// Wait for a new update state | ||
err = testutil.WaitForBlocks(ctx, 5, cosmosChain, polkadotChain) | ||
s.Require().NoError(err) | ||
|
||
// Verify cosmos user's final "stake" balance | ||
cosmosUserStakeBal, err := cosmosChain.GetBalance(ctx, cosmosUser.FormattedAddress(), cosmosChain.Config().Denom) | ||
s.Require().NoError(err) | ||
s.Require().True(cosmosUserStakeBal.Equal(finalStakeBal)) | ||
|
||
// Verify cosmos user's final "unit" balance | ||
unitDenomTrace := transfertypes.ParseDenomTrace(transfertypes.GetPrefixedDenom("transfer", "channel-0", "UNIT")) | ||
cosmosUserUnitBal, err := cosmosChain.GetBalance(ctx, cosmosUser.FormattedAddress(), unitDenomTrace.IBCDenom()) | ||
s.Require().NoError(err) | ||
s.Require().True(cosmosUserUnitBal.Equal(amountUnits)) | ||
|
||
// Verify parachain user's final "unit" balance (will be less than expected due gas costs for stake tx) | ||
parachainUserUnits, err := polkadotChain.GetIbcBalance(ctx, string(polkadotUser.Address()), 1) | ||
s.Require().NoError(err) | ||
s.Require().True(parachainUserUnits.Amount.LTE(math.NewInt(fundAmount)), "parachain user's final unit amount not expected") | ||
|
||
// Verify parachain user's final "stake" balance | ||
parachainUserStake, err := polkadotChain.GetIbcBalance(ctx, string(polkadotUser.Address()), 2) | ||
s.Require().NoError(err) | ||
s.Require().True(parachainUserStake.Amount.Equal(math.NewInt(amountToSend-amountToReflect)), "parachain user's final stake amount not expected") | ||
}) | ||
} | ||
|
||
// TestMsgMigrateContract_Success_GrandpaContract features | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is a bit weird that the tag is called timeout :P
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah it is a bit weird, maybe for now we can re-tag this image and give it a better name. Happy if this happened in a follow up though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yea, i think this should hopefully be fixed once we have a stable relayer image that's not running off just a branch...