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.

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: added packet data forwards compatibility test #6089

Merged
merged 5 commits into from
May 22, 2024
Merged
Changes from 3 commits
Commits
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
84 changes: 84 additions & 0 deletions modules/apps/transfer/keeper/relay_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Comment on lines +919 to +922
Copy link
Contributor

Choose a reason for hiding this comment

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

this test will test is effectively testing compatibility of new fields in the packet data in version N and version N+1 right? If we were to run an E2E and add a new field to packet data and execute a MsgTransfer against an older version of ibc-go (pre-json marshalling) this could still blow up right?

Copy link
Contributor

Choose a reason for hiding this comment

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

actually I guess it just means that whichever version we are about to tag will be able to successfully process unknown fields, it doesn't necessarily mean it will be backwards compatible with a previous release right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes. Unless we backport this feature

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe this should be called a future proof test?


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,
colin-axner marked this conversation as resolved.
Show resolved Hide resolved
},
{
"success: no new field with memo",
func() {
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,
},
{
"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)
}
})
}
}
Loading