-
Notifications
You must be signed in to change notification settings - Fork 624
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
fix: ics29: switch source with destintion for chan/port IDs #961
Changes from 8 commits
be8d931
1884d32
86bda95
2d867e0
113c892
53decd4
f67f139
36d0b6b
40ccecd
5bdcd17
599c574
a3d4ed3
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 |
---|---|---|
|
@@ -498,12 +498,12 @@ func (suite *FeeTestSuite) TestOnRecvPacket() { | |
tc := tc | ||
suite.Run(tc.name, func() { | ||
suite.SetupTest() | ||
// setup path2 (chainA -> chainC) first in order to have different channel IDs for chainA & chainB | ||
suite.coordinator.Setup(suite.path2) | ||
// setup path for chainA -> chainB | ||
suite.coordinator.Setup(suite.path) | ||
|
||
// set up a different channel to make sure that the test will error if the destination channel of the packet is not fee enabled | ||
suite.path.EndpointB.ChannelID = "channel-1" | ||
suite.chainB.GetSimApp().IBCFeeKeeper.SetFeeEnabled(suite.chainB.GetContext(), suite.path.EndpointB.ChannelConfig.PortID, suite.path.EndpointB.ChannelID) | ||
suite.chainB.GetSimApp().IBCFeeKeeper.DeleteFeeEnabled(suite.chainB.GetContext(), suite.path.EndpointB.ChannelConfig.PortID, "channel-0") | ||
|
||
packet := suite.CreateMockPacket() | ||
|
||
|
@@ -514,7 +514,7 @@ func (suite *FeeTestSuite) TestOnRecvPacket() { | |
cbs, ok := suite.chainB.App.GetIBCKeeper().Router.GetRoute(module) | ||
suite.Require().True(ok) | ||
|
||
suite.chainB.GetSimApp().IBCFeeKeeper.SetCounterpartyAddress(suite.chainB.GetContext(), suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String(), suite.path.EndpointB.ChannelID) | ||
suite.chainB.GetSimApp().IBCFeeKeeper.SetCounterpartyAddress(suite.chainB.GetContext(), suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String(), suite.path.EndpointA.ChannelID) | ||
|
||
// malleate test case | ||
tc.malleate() | ||
|
@@ -527,11 +527,18 @@ func (suite *FeeTestSuite) TestOnRecvPacket() { | |
|
||
case tc.forwardRelayer && result == nil: | ||
suite.Require().Equal(nil, result) | ||
packetId := channeltypes.NewPacketId(packet.GetDestChannel(), packet.GetDestPort(), packet.GetSequence()) | ||
|
||
// retrieve the forward relayer that was stored in `onRecvPacket` | ||
relayer, _ := suite.chainB.GetSimApp().IBCFeeKeeper.GetRelayerAddressForAsyncAck(suite.chainB.GetContext(), packetId) | ||
suite.Require().Equal(relayer, suite.chainA.SenderAccount.GetAddress().String()) | ||
|
||
case !tc.forwardRelayer: | ||
forwardAddr, _ := suite.chainB.GetSimApp().IBCFeeKeeper.GetCounterpartyAddress(suite.chainB.GetContext(), suite.chainA.SenderAccount.GetAddress().String(), suite.path.EndpointA.ChannelID) | ||
|
||
expectedAck := types.IncentivizedAcknowledgement{ | ||
Result: ibcmock.MockAcknowledgement.Acknowledgement(), | ||
ForwardRelayerAddress: "", | ||
ForwardRelayerAddress: forwardAddr, | ||
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. this confuses me a little, shouldn't this case have an empty forward relayer? 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. Yes it should you're right. I need to make a change to our calls to get/set CounterpartyAddress 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've updated this now so that anywhere we change DestChannel/Port to Source we'll get failing tests (for forward relayer or counterparty) |
||
UnderlyingAppSuccess: true, | ||
} | ||
suite.Require().Equal(expectedAck, result) | ||
|
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.
nit: want to call this
pathAToC
maybe? Don't feel too strongly but might be easier to reason about when revisitingThere 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.
updated 👍