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

Jump to bottom

update WriteUpgradeTry to change flush status to FLUSHCOMPLETE if there are no packet commitments left #3964

Merged
merged 8 commits into from
Jun 28, 2023
Merged

update WriteUpgradeTry to change flush status to FLUSHCOMPLETE if there are no packet commitments left #3964

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
5fcafda
first draft writeupgradetry test
charleenfei Jun 26, 2023
5bffe98
add writeUpgradeTry test
charleenfei Jun 26, 2023
97cd02d
rm unnecessary fmt.Print
charleenfei Jun 26, 2023
b93bc67
pr comments
charleenfei Jun 28, 2023
3fea59e
lint
charleenfei Jun 28, 2023
c1cd83f
Merge branch '04-channel-upgrades' into charly/3928-writeupgradetry-f…
charleenfei Jun 28, 2023
5c35831
Merge branch '04-channel-upgrades' into charly/3928-writeupgradetry-f…
charleenfei Jun 28, 2023
788184b
assert on return arg
charleenfei Jun 28, 2023
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
5 changes: 4 additions & 1 deletion modules/core/04-channel/keeper/upgrade.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,9 +191,12 @@ func (k Keeper) WriteUpgradeTryChannel(ctx sdk.Context, portID, channelID string

previousState := channel.State
channel.State = types.TRYUPGRADE
// TODO: determine flush status
channel.FlushStatus = types.FLUSHING

if !k.hasInflightPackets(ctx, portID, channelID) {
channel.FlushStatus = types.FLUSHCOMPLETE
}

upgrade.Fields.Version = upgradeVersion

k.SetChannel(ctx, portID, channelID, channel)
Expand Down
65 changes: 65 additions & 0 deletions modules/core/04-channel/keeper/upgrade_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,66 @@ func (suite *KeeperTestSuite) TestChanUpgradeTry() {
}
}

func (suite *KeeperTestSuite) TestWriteUpgradeTry() {
charleenfei marked this conversation as resolved.
Show resolved Hide resolved
var (
path *ibctesting.Path
proposedUpgrade types.Upgrade
)

testCases := []struct {
name string
malleate func()
hasPacketCommitments bool
}{
{
"success with no packet commitments",
func() {},
false,
},
{
"success with packet commitments",
func() {
// manually set packet commitment
sequence, err := path.EndpointB.SendPacket(suite.chainB.GetTimeoutHeight(), 0, ibctesting.MockPacketData)
suite.Require().NoError(err)
suite.Require().Equal(uint64(1), sequence)
},
true,
},
}

for _, tc := range testCases {
tc := tc
suite.Run(tc.name, func() {
suite.SetupTest()

path = ibctesting.NewPath(suite.chainA, suite.chainB)
suite.coordinator.Setup(path)

path.EndpointB.ChannelConfig.ProposedUpgrade.Fields.Version = mock.UpgradeVersion
proposedUpgrade = path.EndpointB.GetProposedUpgrade()
Comment on lines +362 to +366
Copy link
Member

Choose a reason for hiding this comment

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

do we need to call path.EndpointA.ChanUpgradeInit() in order to setup expected state?

Possibly not as this is kind of being tested in isolation but might be nice to follow the flow

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I didn't call it as this is meant to be more of a unit test for this method (I was thinking similarly to the unit tests we have for ValidateBasic for example, and we do a test of the whole flow in the msg_server, but if you think it's better to simulate the whole flow here as well then I could call the whole flow!


tc.malleate()

upgradedChannelEnd, upgradeWithAppCallbackVersion := suite.chainB.GetSimApp().IBCKeeper.ChannelKeeper.WriteUpgradeTryChannel(suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, proposedUpgrade, proposedUpgrade.Fields.Version)

channel := path.EndpointB.GetChannel()
charleenfei marked this conversation as resolved.
Show resolved Hide resolved
suite.Require().Equal(upgradedChannelEnd, channel)

upgrade, found := suite.chainB.GetSimApp().IBCKeeper.ChannelKeeper.GetUpgrade(suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)
suite.Require().True(found)
suite.Require().Equal(types.TRYUPGRADE, channel.State)
suite.Require().Equal(upgradeWithAppCallbackVersion, upgrade)

if tc.hasPacketCommitments {
suite.Require().Equal(types.FLUSHING, channel.FlushStatus)
} else {
suite.Require().Equal(types.FLUSHCOMPLETE, channel.FlushStatus)
}
})
}
}

func (suite *KeeperTestSuite) TestChanUpgradeAck() {
var (
path *ibctesting.Path
Expand Down Expand Up @@ -459,6 +519,11 @@ func (suite *KeeperTestSuite) TestChanUpgradeAck() {
err := path.EndpointA.ChanUpgradeInit()
suite.Require().NoError(err)

// manually set packet commitment so that the chainB channel flush status is FLUSHING
sequence, err := path.EndpointB.SendPacket(suite.chainB.GetTimeoutHeight(), 0, ibctesting.MockPacketData)
suite.Require().NoError(err)
suite.Require().Equal(uint64(1), sequence)

err = path.EndpointB.ChanUpgradeTry()
suite.Require().NoError(err)

Expand Down