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

Feat: Delete upgrade information only rather than calling abort in TimeoutExecuted #5764

11 changes: 11 additions & 0 deletions modules/core/04-channel/keeper/handshake.go
Original file line number Diff line number Diff line change
Expand Up @@ -465,6 +465,17 @@ func (k Keeper) ChanCloseConfirm(
return err
}

// If the channel is closing during an upgrade, then we can delete all upgrade information.
if k.hasUpgrade(ctx, portID, channelID) {
k.deleteUpgradeInfo(ctx, portID, channelID)
k.Logger(ctx).Info(
"upgrade info deleted",
"port_id", portID,
"channel_id", channelID,
"upgrade_sequence", channel.UpgradeSequence,
)
}

k.Logger(ctx).Info("channel state updated", "port-id", portID, "channel-id", channelID, "previous-state", channel.State.String(), "new-state", types.CLOSED.String())

defer telemetry.IncrCounter(1, "ibc", "channel", "close-confirm")
Expand Down
31 changes: 30 additions & 1 deletion modules/core/04-channel/keeper/handshake_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -713,6 +713,28 @@ func (suite *KeeperTestSuite) TestChanCloseConfirm() {
err := path.EndpointA.SetChannelState(types.CLOSED)
suite.Require().NoError(err)
}, true},
{"success with upgrade info", func() {
path.Setup()
channelCap = suite.chainB.GetChannelCapability(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)

err := path.EndpointA.SetChannelState(types.CLOSED)
suite.Require().NoError(err)

// add mock upgrade info to simulate that the channel is closing during
// an upgrade and verify that the upgrade information is deleted
upgrade := types.Upgrade{
Fields: types.NewUpgradeFields(types.UNORDERED, []string{ibctesting.FirstConnectionID}, mock.UpgradeVersion),
Timeout: types.NewTimeout(clienttypes.ZeroHeight(), 1),
}

counterpartyUpgrade := types.Upgrade{
Fields: types.NewUpgradeFields(types.UNORDERED, []string{ibctesting.FirstConnectionID}, mock.UpgradeVersion),
Timeout: types.NewTimeout(clienttypes.ZeroHeight(), 1),
}

path.EndpointB.SetChannelUpgrade(upgrade)
path.EndpointB.SetChannelCounterpartyUpgrade(counterpartyUpgrade)
}, true},
{"channel doesn't exist", func() {
// any non-nil values work for connections
path.EndpointA.ChannelID = ibctesting.FirstChannelID
Expand Down Expand Up @@ -809,13 +831,20 @@ func (suite *KeeperTestSuite) TestChanCloseConfirm() {
channelKey := host.ChannelKey(path.EndpointA.ChannelConfig.PortID, ibctesting.FirstChannelID)
proof, proofHeight := suite.chainA.QueryProof(channelKey)

ctx := suite.chainB.GetContext()
err := suite.chainB.App.GetIBCKeeper().ChannelKeeper.ChanCloseConfirm(
suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID, ibctesting.FirstChannelID, channelCap,
ctx, path.EndpointB.ChannelConfig.PortID, ibctesting.FirstChannelID, channelCap,
proof, malleateHeight(proofHeight, heightDiff), counterpartyUpgradeSequence,
)

if tc.expPass {
suite.Require().NoError(err)

// if the channel closed during an upgrade, there should not be any upgrade information
_, found := suite.chainB.App.GetIBCKeeper().ChannelKeeper.GetUpgrade(ctx, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)
suite.Require().False(found)
_, found = suite.chainB.App.GetIBCKeeper().ChannelKeeper.GetCounterpartyUpgrade(ctx, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)
suite.Require().False(found)
} else {
suite.Require().Error(err)
}
Expand Down
11 changes: 8 additions & 3 deletions modules/core/04-channel/keeper/timeout.go
Original file line number Diff line number Diff line change
Expand Up @@ -174,10 +174,15 @@ func (k Keeper) TimeoutExecuted(

if channel.Ordering == types.ORDERED {
// NOTE: if the channel is ORDERED and a packet is timed out in FLUSHING state then
// the upgrade is aborted and the channel is set to CLOSED.
// all upgrade information is deleted and the channel is set to CLOSED.
if channel.State == types.FLUSHING {
// an error receipt is written to state and the channel is restored to OPEN
k.MustAbortUpgrade(ctx, packet.GetSourcePort(), packet.GetSourceChannel(), errorsmod.Wrap(types.ErrTimeoutElapsed, "packet timeout elapsed on ORDERED channel"))
k.deleteUpgradeInfo(ctx, packet.GetSourcePort(), packet.GetSourceChannel())
crodriguezvega marked this conversation as resolved.
Show resolved Hide resolved
k.Logger(ctx).Info(
"upgrade info deleted",
"port_id", packet.GetSourcePort(),
"channel_id", packet.GetSourceChannel(),
"upgrade_sequence", channel.UpgradeSequence,
)
}

channel.State = types.CLOSED
Expand Down
4 changes: 0 additions & 4 deletions modules/core/04-channel/keeper/timeout_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -506,10 +506,6 @@ func (suite *KeeperTestSuite) TestTimeoutExecuted() {
upgrade, found = suite.chainA.App.GetIBCKeeper().ChannelKeeper.GetCounterpartyUpgrade(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
suite.Require().False(found, "counterparty upgrade should not be present")
suite.Require().Equal(types.Upgrade{}, upgrade, "counterparty upgrade should be zero value")

errorReceipt, found := suite.chainA.App.GetIBCKeeper().ChannelKeeper.GetUpgradeErrorReceipt(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
suite.Require().True(found, "error receipt should be present")
suite.Require().Equal(channel.UpgradeSequence, errorReceipt.Sequence, "error receipt sequence should be equal to channel upgrade sequence")
},
nil,
},
Expand Down
Loading