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.

Sign up for GitHub

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

refactor: ics29 use mock module for OnTimeoutPacket testing #928

Merged
merged 3 commits into from
Feb 21, 2022
Merged

refactor: ics29 use mock module for OnTimeoutPacket testing #928

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7588643
refactor: OnTimeoutPacket to use mock module
colin-axner Feb 16, 2022
f541e51
Merge branch 'ics29-fee-middleware' into colin/891-ontimeoutpacket
seantking Feb 17, 2022
7ac8763
Merge branch 'ics29-fee-middleware' into colin/891-ontimeoutpacket
colin-axner Feb 21, 2022
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
28 changes: 8 additions & 20 deletions modules/apps/29-fee/ibc_module_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ import (
host "github.com/cosmos/ibc-go/v3/modules/core/24-host"
ibctesting "github.com/cosmos/ibc-go/v3/testing"
ibcmock "github.com/cosmos/ibc-go/v3/testing/mock"
"github.com/cosmos/ibc-go/v3/testing/simapp"
)

var (
Expand Down Expand Up @@ -694,7 +693,7 @@ func (suite *FeeTestSuite) TestOnTimeoutPacket() {
func() {
suite.chainA.GetSimApp().IBCFeeKeeper.DeleteFeeEnabled(suite.chainA.GetContext(), suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID)

expectedBalance = originalBalance.Add(ibctesting.TestCoin) // timeout refund for ics20 transfer
expectedBalance = originalBalance
},
false,
},
Expand All @@ -705,7 +704,7 @@ func (suite *FeeTestSuite) TestOnTimeoutPacket() {
packetId := channeltypes.NewPacketId(suite.path.EndpointA.ChannelID, suite.path.EndpointA.ChannelConfig.PortID, suite.chainA.SenderAccount.GetSequence())
suite.chainA.GetSimApp().IBCFeeKeeper.DeleteFeesInEscrow(suite.chainA.GetContext(), packetId)

expectedBalance = originalBalance.Add(ibctesting.TestCoin) // timeout refund for ics20 transfer
expectedBalance = originalBalance
},
false,
},
Expand All @@ -716,8 +715,7 @@ func (suite *FeeTestSuite) TestOnTimeoutPacket() {

expectedBalance = originalBalance.
Add(identifiedFee.Fee.RecvFee[0]).
Add(identifiedFee.Fee.AckFee[0]).
Add(ibctesting.TestCoin) // timeout refund for ics20 transfer
Add(identifiedFee.Fee.AckFee[0])
Comment on lines -719 to +718
Copy link
Member

Choose a reason for hiding this comment

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

I don't understand how this was passing before. Seems like you're actually changing the test logic rather than just a refactor?

},
false,
},
Expand All @@ -726,27 +724,18 @@ func (suite *FeeTestSuite) TestOnTimeoutPacket() {
for _, tc := range testCases {
tc := tc
suite.Run(tc.name, func() {
suite.SetupTest()

// open incentivized channel
suite.SetupMockTest()
suite.coordinator.Setup(suite.path)

// set up coin & create ics20 packet
coin := ibctesting.TestCoin
packet := suite.CreateICS20Packet(coin)

// setup for ics20: fund chain A's escrow path so that tokens can be unescrowed upon timeout
escrow := transfertypes.GetEscrowAddress(suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID)
suite.Require().NoError(simapp.FundAccount(suite.chainA.GetSimApp(), suite.chainA.GetContext(), escrow, sdk.NewCoins(coin)))
Comment on lines -738 to -740
Copy link
Contributor Author

Choose a reason for hiding this comment

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

no longer needed. It was only there so the transfer OnTimeoutPacket callback wouldn't error

packet := suite.CreateMockPacket()

// set up module and callbacks
module, _, err := suite.chainA.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainA.GetContext(), ibctesting.TransferPort)
module, _, err := suite.chainA.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainA.GetContext(), ibctesting.MockFeePort)
suite.Require().NoError(err)

cbs, ok := suite.chainA.App.GetIBCKeeper().Router.GetRoute(module)
suite.Require().True(ok)

packetId := channeltypes.NewPacketId(suite.path.EndpointA.ChannelID, suite.path.EndpointA.ChannelConfig.PortID, suite.chainA.SenderAccount.GetSequence())
packetId := channeltypes.NewPacketId(packet.GetSourceChannel(), packet.GetSourcePort(), packet.GetSequence())

// must be explicitly changed
relayerAddr = suite.chainB.SenderAccount.GetAddress()
Expand All @@ -772,8 +761,7 @@ func (suite *FeeTestSuite) TestOnTimeoutPacket() {
// default to success case
expectedBalance = originalBalance.
Add(identifiedFee.Fee.RecvFee[0]).
Add(identifiedFee.Fee.AckFee[0]).
Add(coin) // timeout refund from ics20 transfer
Add(identifiedFee.Fee.AckFee[0])
Comment on lines -775 to +764
Copy link
Member

Choose a reason for hiding this comment

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

ditto


// malleate test case
tc.malleate()
Expand Down