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: Removing restorechannel and adding SetUpgradeErrorReceipt (backport #5405) #5517

Merged
merged 1 commit into from
Jan 5, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
6 changes: 3 additions & 3 deletions modules/core/03-connection/keeper/verify_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -744,7 +744,7 @@ func (suite *KeeperTestSuite) TestVerifyUpgradeErrorReceipt() {
suite.coordinator.Setup(path)

upgradeError = channeltypes.NewUpgradeError(1, channeltypes.ErrInvalidChannel)
suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.SetUpgradeErrorReceipt(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, upgradeError.GetErrorReceipt())
suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.WriteErrorReceipt(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, upgradeError)

suite.chainA.Coordinator.CommitBlock(suite.chainA)
suite.Require().NoError(path.EndpointB.UpdateClient())
Expand Down Expand Up @@ -799,8 +799,8 @@ func (suite *KeeperTestSuite) TestVerifyUpgradeErrorReceiptAbsence() {
{
name: "verification fails when the key exists",
malleate: func() {
errorReceipt := channeltypes.NewUpgradeError(1, channeltypes.ErrInvalidChannel).GetErrorReceipt()
suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.SetUpgradeErrorReceipt(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, errorReceipt)
upgradeErr := channeltypes.NewUpgradeError(1, channeltypes.ErrInvalidChannel)
suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.WriteErrorReceipt(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, upgradeErr)
suite.chainA.Coordinator.CommitBlock(suite.chainA)
suite.Require().NoError(path.EndpointB.UpdateClient())
},
Expand Down
6 changes: 3 additions & 3 deletions modules/core/04-channel/keeper/export_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ func (k Keeper) CheckForUpgradeCompatibility(ctx sdk.Context, upgradeFields, cou
return k.checkForUpgradeCompatibility(ctx, upgradeFields, counterpartyUpgradeFields)
}

// WriteErrorReceipt is a wrapper around writeErrorReceipt to allow the function to be directly called in tests.
func (k Keeper) WriteErrorReceipt(ctx sdk.Context, portID, channelID string, upgradeError *types.UpgradeError) {
k.writeErrorReceipt(ctx, portID, channelID, upgradeError)
// SetUpgradeErrorReceipt is a wrapper around setUpgradeErrorReceipt to allow the function to be directly called in tests.
func (k Keeper) SetUpgradeErrorReceipt(ctx sdk.Context, portID, channelID string, errorReceipt types.ErrorReceipt) {
k.setUpgradeErrorReceipt(ctx, portID, channelID, errorReceipt)
}
2 changes: 1 addition & 1 deletion modules/core/04-channel/keeper/grpc_query_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1758,7 +1758,7 @@ func (suite *KeeperTestSuite) TestQueryUpgradeError() {
path := ibctesting.NewPath(suite.chainA, suite.chainB)
suite.coordinator.Setup(path)
upgradeErr = types.NewUpgradeError(uint64(1), fmt.Errorf("test error"))
suite.chainA.App.GetIBCKeeper().ChannelKeeper.SetUpgradeErrorReceipt(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, upgradeErr.GetErrorReceipt())
suite.chainA.App.GetIBCKeeper().ChannelKeeper.WriteErrorReceipt(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, upgradeErr)

req = &types.QueryUpgradeErrorRequest{
PortId: path.EndpointA.ChannelConfig.PortID,
Expand Down
4 changes: 2 additions & 2 deletions modules/core/04-channel/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -507,8 +507,8 @@ func (k Keeper) GetUpgradeErrorReceipt(ctx sdk.Context, portID, channelID string
return errorReceipt, true
}

// SetUpgradeErrorReceipt sets the provided error receipt in store using the port and channel identifiers.
func (k Keeper) SetUpgradeErrorReceipt(ctx sdk.Context, portID, channelID string, errorReceipt types.ErrorReceipt) {
// setUpgradeErrorReceipt sets the provided error receipt in store using the port and channel identifiers.
func (k Keeper) setUpgradeErrorReceipt(ctx sdk.Context, portID, channelID string, errorReceipt types.ErrorReceipt) {
store := ctx.KVStore(k.storeKey)
bz := k.cdc.MustMarshal(&errorReceipt)
store.Set(host.ChannelUpgradeErrorKey(portID, channelID), bz)
Expand Down
6 changes: 3 additions & 3 deletions modules/core/04-channel/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -479,12 +479,12 @@ func (suite *KeeperTestSuite) TestSetUpgradeErrorReceipt() {
suite.Require().False(found)
suite.Require().Empty(errorReceipt)

expErrorReceipt := types.NewUpgradeError(1, fmt.Errorf("testing")).GetErrorReceipt()
suite.chainA.App.GetIBCKeeper().ChannelKeeper.SetUpgradeErrorReceipt(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, expErrorReceipt)
expError := types.NewUpgradeError(1, fmt.Errorf("testing"))
suite.chainA.App.GetIBCKeeper().ChannelKeeper.WriteErrorReceipt(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, expError)

errorReceipt, found = suite.chainA.App.GetIBCKeeper().ChannelKeeper.GetUpgradeErrorReceipt(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
suite.Require().True(found)
suite.Require().Equal(expErrorReceipt, errorReceipt)
suite.Require().Equal(expError.GetErrorReceipt(), errorReceipt)
}

// TestDefaultSetParams tests the default params set are what is expected
Expand Down
20 changes: 11 additions & 9 deletions modules/core/04-channel/keeper/upgrade.go
Original file line number Diff line number Diff line change
Expand Up @@ -676,7 +676,8 @@ func (k Keeper) WriteUpgradeCancelChannel(ctx sdk.Context, portID, channelID str

previousState := channel.State

channel = k.restoreChannel(ctx, portID, channelID, sequence, channel, types.NewUpgradeError(sequence, types.ErrInvalidUpgrade))
channel = k.restoreChannel(ctx, portID, channelID, sequence, channel)
k.WriteErrorReceipt(ctx, portID, channelID, types.NewUpgradeError(sequence, types.ErrInvalidUpgrade))

k.Logger(ctx).Info("channel state updated", "port-id", portID, "channel-id", channelID, "previous-state", previousState, "new-state", types.OPEN.String())
EmitChannelUpgradeCancelEvent(ctx, portID, channelID, channel, upgrade)
Expand Down Expand Up @@ -794,7 +795,8 @@ func (k Keeper) WriteUpgradeTimeoutChannel(
panic(fmt.Errorf("could not find existing upgrade when cancelling channel upgrade, channelID: %s, portID: %s", channelID, portID))
}

channel = k.restoreChannel(ctx, portID, channelID, channel.UpgradeSequence, channel, types.NewUpgradeError(channel.UpgradeSequence, types.ErrUpgradeTimeout))
channel = k.restoreChannel(ctx, portID, channelID, channel.UpgradeSequence, channel)
k.WriteErrorReceipt(ctx, portID, channelID, types.NewUpgradeError(channel.UpgradeSequence, types.ErrUpgradeTimeout))

k.Logger(ctx).Info("channel state restored", "port-id", portID, "channel-id", channelID)

Expand Down Expand Up @@ -957,12 +959,14 @@ func (k Keeper) abortUpgrade(ctx sdk.Context, portID, channelID string, err erro

// the channel upgrade sequence has already been updated in ChannelUpgradeTry, so we can pass
// its updated value.
k.restoreChannel(ctx, portID, channelID, channel.UpgradeSequence, channel, upgradeError)
k.restoreChannel(ctx, portID, channelID, channel.UpgradeSequence, channel)
k.WriteErrorReceipt(ctx, portID, channelID, upgradeError)

return nil
}

// restoreChannel will restore the channel state to its pre-upgrade state so that upgrade is aborted.
func (k Keeper) restoreChannel(ctx sdk.Context, portID, channelID string, upgradeSequence uint64, channel types.Channel, err *types.UpgradeError) types.Channel {
func (k Keeper) restoreChannel(ctx sdk.Context, portID, channelID string, upgradeSequence uint64, channel types.Channel) types.Channel {
channel.State = types.OPEN
channel.UpgradeSequence = upgradeSequence

Expand All @@ -971,13 +975,11 @@ func (k Keeper) restoreChannel(ctx sdk.Context, portID, channelID string, upgrad
// delete state associated with upgrade which is no longer required.
k.deleteUpgradeInfo(ctx, portID, channelID)

k.SetUpgradeErrorReceipt(ctx, portID, channelID, err.GetErrorReceipt())

return channel
}

// writeErrorReceipt will write an error receipt from the provided UpgradeError.
func (k Keeper) writeErrorReceipt(ctx sdk.Context, portID, channelID string, upgradeError *types.UpgradeError) {
// WriteErrorReceipt will write an error receipt from the provided UpgradeError.
func (k Keeper) WriteErrorReceipt(ctx sdk.Context, portID, channelID string, upgradeError *types.UpgradeError) {
channel, found := k.GetChannel(ctx, portID, channelID)
if !found {
panic(errorsmod.Wrapf(types.ErrChannelNotFound, "port ID (%s) channel ID (%s)", portID, channelID))
Expand All @@ -990,6 +992,6 @@ func (k Keeper) writeErrorReceipt(ctx sdk.Context, portID, channelID string, upg
panic(errorsmod.Wrapf(types.ErrInvalidUpgradeSequence, "error receipt sequence (%d) must be greater than existing error receipt sequence (%d)", errorReceiptToWrite.Sequence, existingErrorReceipt.Sequence))
}

k.SetUpgradeErrorReceipt(ctx, portID, channelID, errorReceiptToWrite)
k.setUpgradeErrorReceipt(ctx, portID, channelID, errorReceiptToWrite)
EmitErrorReceiptEvent(ctx, portID, channelID, channel, upgradeError)
}
7 changes: 3 additions & 4 deletions modules/core/04-channel/keeper/upgrade_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -532,8 +532,7 @@ func (suite *KeeperTestSuite) TestChanUpgrade_CrossingHellos_UpgradeSucceeds_Aft
suite.Require().Error(err)
suite.assertUpgradeError(err, types.NewUpgradeError(4, types.ErrInvalidUpgradeSequence))

errorReceipt := err.(*types.UpgradeError).GetErrorReceipt()
suite.chainB.GetSimApp().IBCKeeper.ChannelKeeper.SetUpgradeErrorReceipt(suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, errorReceipt)
suite.chainB.GetSimApp().IBCKeeper.ChannelKeeper.WriteErrorReceipt(suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, err.(*types.UpgradeError))
suite.coordinator.CommitBlock(suite.chainB)
})

Expand Down Expand Up @@ -2784,7 +2783,7 @@ func (suite *KeeperTestSuite) TestWriteErrorReceipt() {
func() {
// write an error sequence with a lower sequence number
previousUpgradeError := types.NewUpgradeError(upgradeError.GetErrorReceipt().Sequence-1, types.ErrInvalidUpgrade)
suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.SetUpgradeErrorReceipt(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, previousUpgradeError.GetErrorReceipt())
suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.WriteErrorReceipt(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, previousUpgradeError)
},
nil,
},
Expand All @@ -2793,7 +2792,7 @@ func (suite *KeeperTestSuite) TestWriteErrorReceipt() {
func() {
// write an error sequence with a higher sequence number
previousUpgradeError := types.NewUpgradeError(upgradeError.GetErrorReceipt().Sequence+1, types.ErrInvalidUpgrade)
suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.SetUpgradeErrorReceipt(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, previousUpgradeError.GetErrorReceipt())
suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.WriteErrorReceipt(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, previousUpgradeError)
},
errorsmod.Wrap(types.ErrInvalidUpgradeSequence, "error receipt sequence (10) must be greater than existing error receipt sequence (11)"),
},
Expand Down
3 changes: 1 addition & 2 deletions modules/core/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -816,8 +816,7 @@ func (k Keeper) ChannelUpgradeTry(goCtx context.Context, msg *channeltypes.MsgCh
if err != nil {
ctx.Logger().Error("channel upgrade try failed", "error", errorsmod.Wrap(err, "channel upgrade try failed"))
if channeltypes.IsUpgradeError(err) {
k.ChannelKeeper.SetUpgradeErrorReceipt(ctx, msg.PortId, msg.ChannelId, err.(*channeltypes.UpgradeError).GetErrorReceipt())
keeper.EmitErrorReceiptEvent(ctx, msg.PortId, msg.ChannelId, channel, err)
k.ChannelKeeper.WriteErrorReceipt(ctx, msg.PortId, msg.ChannelId, err.(*channeltypes.UpgradeError))
// NOTE: a FAILURE result is returned to the client and an error receipt is written to state.
// This signals to the relayer to begin the cancel upgrade handshake subprotocol.
return &channeltypes.MsgChannelUpgradeTryResponse{Result: channeltypes.FAILURE}, nil
Expand Down
Loading