From de17de51c77c5ef1ccbc62bc699f8a3d3e9e37d1 Mon Sep 17 00:00:00 2001 From: srdtrk Date: Thu, 4 Apr 2024 14:00:29 +0700 Subject: [PATCH 1/3] test: added packet data backwards compatibility test --- modules/apps/transfer/keeper/relay_test.go | 84 ++++++++++++++++++++++ 1 file changed, 84 insertions(+) diff --git a/modules/apps/transfer/keeper/relay_test.go b/modules/apps/transfer/keeper/relay_test.go index 1eb02bba9bd..c888329e07e 100644 --- a/modules/apps/transfer/keeper/relay_test.go +++ b/modules/apps/transfer/keeper/relay_test.go @@ -913,3 +913,87 @@ func (suite *KeeperTestSuite) TestOnTimeoutPacketSetsTotalEscrowAmountForSourceI totalEscrowChainB = suite.chainB.GetSimApp().TransferKeeper.GetTotalEscrowForDenom(suite.chainB.GetContext(), coin.GetDenom()) suite.Require().Equal(sdkmath.ZeroInt(), totalEscrowChainB.Amount) } + +func (suite *KeeperTestSuite) TestPacketBackwardsCompatibility() { + // We are testing a scenario where a packet in the future has a new populated + // field called "new_field". And this packet is being sent to this module which + // doesn't have this field in the packet data. The module should be able to handle + // this packet without any issues. + + var packetData []byte + + testCases := []struct { + msg string + malleate func() + expPass bool + }{ + { + "success: new field", + func() { + jsonString := fmt.Sprintf(`{"denom":"denom","amount":"100","sender":"%s","receiver":"%s","memo":"memo","new_field":"value"}`, suite.chainB.SenderAccount.GetAddress().String(), suite.chainA.SenderAccount.GetAddress().String()) + packetData = []byte(jsonString) + }, + true, + }, + { + "success: no new field with memo", + func() { + jsonString := fmt.Sprintf(`{"denom":"denom","amount":"100","sender":"%s","receiver":"%s","memo":"memo","new_field":"value"}`, suite.chainB.SenderAccount.GetAddress().String(), suite.chainA.SenderAccount.GetAddress().String()) + packetData = []byte(jsonString) + }, + true, + }, + { + "success: no new field without memo", + func() { + jsonString := fmt.Sprintf(`{"denom":"denom","amount":"100","sender":"%s","receiver":"%s"}`, suite.chainB.SenderAccount.GetAddress().String(), suite.chainA.SenderAccount.GetAddress().String()) + packetData = []byte(jsonString) + }, + true, + }, + { + "failure: invalid packet data", + func() { + packetData = []byte("invalid packet data") + }, + false, + }, + { + "failure: missing field", + func() { + jsonString := fmt.Sprintf(`{"denom":"denom","amount":"100","receiver":"%s"}`, suite.chainA.SenderAccount.GetAddress().String()) + packetData = []byte(jsonString) + }, + false, + }, + } + + for _, tc := range testCases { + tc := tc + suite.Run(tc.msg, func() { + suite.SetupTest() // reset + packetData = nil + + path := ibctesting.NewTransferPath(suite.chainA, suite.chainB) + path.Setup() + + tc.malleate() + + timeoutHeight := suite.chainB.GetTimeoutHeight() + + seq, err := path.EndpointB.SendPacket(timeoutHeight, 0, packetData) + suite.Require().NoError(err) + + packet := channeltypes.NewPacket(packetData, seq, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, timeoutHeight, 0) + + // receive packet on chainA + err = path.RelayPacket(packet) + + if tc.expPass { + suite.Require().NoError(err) + } else { + suite.Require().Error(err) + } + }) + } +} From 4a5b037b128ed71aacd5d1c17f945f8c8216839c Mon Sep 17 00:00:00 2001 From: srdtrk Date: Thu, 4 Apr 2024 14:34:41 +0700 Subject: [PATCH 2/3] imp: fixed test case --- modules/apps/transfer/keeper/relay_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/apps/transfer/keeper/relay_test.go b/modules/apps/transfer/keeper/relay_test.go index c888329e07e..6ed068b521d 100644 --- a/modules/apps/transfer/keeper/relay_test.go +++ b/modules/apps/transfer/keeper/relay_test.go @@ -938,7 +938,7 @@ func (suite *KeeperTestSuite) TestPacketBackwardsCompatibility() { { "success: no new field with memo", func() { - jsonString := fmt.Sprintf(`{"denom":"denom","amount":"100","sender":"%s","receiver":"%s","memo":"memo","new_field":"value"}`, suite.chainB.SenderAccount.GetAddress().String(), suite.chainA.SenderAccount.GetAddress().String()) + jsonString := fmt.Sprintf(`{"denom":"denom","amount":"100","sender":"%s","receiver":"%s","memo":"memo"}`, suite.chainB.SenderAccount.GetAddress().String(), suite.chainA.SenderAccount.GetAddress().String()) packetData = []byte(jsonString) }, true, From 6257dfe7b7f4c2679a767a74aea6a0f901c44375 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Wed, 22 May 2024 12:02:13 +0200 Subject: [PATCH 3/3] imp: use expError --- modules/apps/transfer/keeper/relay_test.go | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/modules/apps/transfer/keeper/relay_test.go b/modules/apps/transfer/keeper/relay_test.go index 6ed068b521d..821a76a877c 100644 --- a/modules/apps/transfer/keeper/relay_test.go +++ b/modules/apps/transfer/keeper/relay_test.go @@ -13,6 +13,7 @@ import ( "github.com/cosmos/ibc-go/v8/modules/apps/transfer/types" clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types" + ibcerrors "github.com/cosmos/ibc-go/v8/modules/core/errors" ibctesting "github.com/cosmos/ibc-go/v8/testing" ) @@ -914,7 +915,7 @@ func (suite *KeeperTestSuite) TestOnTimeoutPacketSetsTotalEscrowAmountForSourceI suite.Require().Equal(sdkmath.ZeroInt(), totalEscrowChainB.Amount) } -func (suite *KeeperTestSuite) TestPacketBackwardsCompatibility() { +func (suite *KeeperTestSuite) TestPacketForwardsCompatibility() { // We are testing a scenario where a packet in the future has a new populated // field called "new_field". And this packet is being sent to this module which // doesn't have this field in the packet data. The module should be able to handle @@ -925,7 +926,7 @@ func (suite *KeeperTestSuite) TestPacketBackwardsCompatibility() { testCases := []struct { msg string malleate func() - expPass bool + expError error }{ { "success: new field", @@ -933,7 +934,7 @@ func (suite *KeeperTestSuite) TestPacketBackwardsCompatibility() { jsonString := fmt.Sprintf(`{"denom":"denom","amount":"100","sender":"%s","receiver":"%s","memo":"memo","new_field":"value"}`, suite.chainB.SenderAccount.GetAddress().String(), suite.chainA.SenderAccount.GetAddress().String()) packetData = []byte(jsonString) }, - true, + nil, }, { "success: no new field with memo", @@ -941,7 +942,7 @@ func (suite *KeeperTestSuite) TestPacketBackwardsCompatibility() { jsonString := fmt.Sprintf(`{"denom":"denom","amount":"100","sender":"%s","receiver":"%s","memo":"memo"}`, suite.chainB.SenderAccount.GetAddress().String(), suite.chainA.SenderAccount.GetAddress().String()) packetData = []byte(jsonString) }, - true, + nil, }, { "success: no new field without memo", @@ -949,22 +950,22 @@ func (suite *KeeperTestSuite) TestPacketBackwardsCompatibility() { jsonString := fmt.Sprintf(`{"denom":"denom","amount":"100","sender":"%s","receiver":"%s"}`, suite.chainB.SenderAccount.GetAddress().String(), suite.chainA.SenderAccount.GetAddress().String()) packetData = []byte(jsonString) }, - true, + nil, }, { "failure: invalid packet data", func() { packetData = []byte("invalid packet data") }, - false, + ibcerrors.ErrUnknownRequest, }, { "failure: missing field", func() { - jsonString := fmt.Sprintf(`{"denom":"denom","amount":"100","receiver":"%s"}`, suite.chainA.SenderAccount.GetAddress().String()) + jsonString := fmt.Sprintf(`{"amount":"100","sender":%s","receiver":"%s"}`, suite.chainB.SenderAccount.GetAddress().String(), suite.chainA.SenderAccount.GetAddress().String()) packetData = []byte(jsonString) }, - false, + ibcerrors.ErrUnknownRequest, }, } @@ -989,10 +990,11 @@ func (suite *KeeperTestSuite) TestPacketBackwardsCompatibility() { // receive packet on chainA err = path.RelayPacket(packet) - if tc.expPass { + expPass := tc.expError == nil + if expPass { suite.Require().NoError(err) } else { - suite.Require().Error(err) + suite.Require().ErrorContains(err, tc.expError.Error()) } }) }