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

chore: propagate IBC authority to 04-channel keeper to be used within upgrade cancellation #5093

Merged
merged 4 commits into from
Nov 22, 2023
Merged

chore: propagate IBC authority to 04-channel keeper to be used within upgrade cancellation #5093

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
9e1c847
chore: add authority to function params, test flow for checking msg s…
charleenfei Nov 13, 2023
f787dcf
linter
charleenfei Nov 13, 2023
11e1aaa
Merge branch '04-channel-upgrades' into charly/issue#4420-propagate-i…
damiannolan Nov 22, 2023
37a2e4b
refactor: use isAuthority bool in favour of passing signer and author…
damiannolan Nov 22, 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
11 changes: 2 additions & 9 deletions modules/core/04-channel/keeper/upgrade.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -545,7 +545,7 @@ func (k Keeper) WriteUpgradeOpenChannel(ctx sdk.Context, portID, channelID strin
}

// ChanUpgradeCancel is called by a module to cancel a channel upgrade that is in progress.
func (k Keeper) ChanUpgradeCancel(ctx sdk.Context, portID, channelID string, errorReceipt types.ErrorReceipt, errorReceiptProof []byte, proofHeight clienttypes.Height, signer string) error {
func (k Keeper) ChanUpgradeCancel(ctx sdk.Context, portID, channelID string, errorReceipt types.ErrorReceipt, errorReceiptProof []byte, proofHeight clienttypes.Height, signer, authority string) error {
channel, found := k.GetChannel(ctx, portID, channelID)
if !found {
return errorsmod.Wrapf(types.ErrChannelNotFound, "port ID (%s) channel ID (%s)", portID, channelID)
Expand All @@ -563,7 +563,7 @@ func (k Keeper) ChanUpgradeCancel(ctx sdk.Context, portID, channelID string, err
// if the msgSender is authorized to make and cancel upgrades AND the current channel has not already reached FLUSHCOMPLETE
// then we can restore immediately without any additional checks
// otherwise, we can only cancel if the counterparty wrote an error receipt during the upgrade handshake
if k.isAuthorizedUpgrader(signer) && channel.State != types.FLUSHCOMPLETE {
if signer == authority && channel.State != types.FLUSHCOMPLETE {
return nil
}

Expand Down Expand Up @@ -595,13 +595,6 @@ func (k Keeper) ChanUpgradeCancel(ctx sdk.Context, portID, channelID string, err
return nil
}

// isAuthorizedUpgrader checks if the sender is authorized to cancel the upgrade.
func (Keeper) isAuthorizedUpgrader(signer string) bool {
// TODO: the authority is only available on the core ibc keeper at the moment.
// return k.GetAuthority() == signer
return true
}

// WriteUpgradeCancelChannel writes a channel which has canceled the upgrade process.Auxiliary upgrade state is
// also deleted.
func (k Keeper) WriteUpgradeCancelChannel(ctx sdk.Context, portID, channelID string, errorReceipt types.ErrorReceipt) {
Expand Down
51 changes: 40 additions & 11 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 @@ -1215,8 +1215,11 @@ func (suite *KeeperTestSuite) TestChanUpgradeCancel() {
errorReceiptProof []byte
proofHeight clienttypes.Height
sender string
authority string
)

const invalidMessage = "different message"

tests := []struct {
name string
malleate func()
Expand All @@ -1236,6 +1239,23 @@ func (suite *KeeperTestSuite) TestChanUpgradeCancel() {
},
expError: nil,
},
{
name: "sender is authority, channel in flushing, upgrade can be cancelled even with invalid error receipt",
malleate: func() {
channel := path.EndpointA.GetChannel()
channel.State = types.FLUSHING
path.EndpointA.SetChannel(channel)

var ok bool
errorReceipt, ok = suite.chainB.GetSimApp().IBCKeeper.ChannelKeeper.GetUpgradeErrorReceipt(suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)
suite.Require().True(ok)

errorReceipt.Message = invalidMessage
suite.chainB.GetSimApp().IBCKeeper.ChannelKeeper.SetUpgradeErrorReceipt(suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, errorReceipt)
suite.coordinator.CommitBlock(suite.chainB)
},
expError: nil,
},
{
name: "channel not found",
malleate: func() {
Expand Down Expand Up @@ -1280,28 +1300,35 @@ func (suite *KeeperTestSuite) TestChanUpgradeCancel() {
expError: connectiontypes.ErrConnectionNotFound,
},
{
name: "error verification failed",
name: "sender is authority, channel is in flush complete, error verification failed",
malleate: func() {
var ok bool
errorReceipt, ok = suite.chainB.GetSimApp().IBCKeeper.ChannelKeeper.GetUpgradeErrorReceipt(suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)
suite.Require().True(ok)

errorReceipt.Message = "different message"
errorReceipt.Message = invalidMessage

suite.chainB.GetSimApp().IBCKeeper.ChannelKeeper.SetUpgradeErrorReceipt(suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, errorReceipt)

suite.coordinator.CommitBlock(suite.chainB)
},
expError: commitmenttypes.ErrInvalidProof,
},
// TODO: add test case for invalid sender once it is implemented.
// {
// name: "invalid sender",
// malleate: func() {
//
// },
// expError: nil,
// },
{
name: "sender is not authority, error verification failed",
malleate: func() {
authority = ibctesting.TestAccAddress

var ok bool
errorReceipt, ok = suite.chainB.GetSimApp().IBCKeeper.ChannelKeeper.GetUpgradeErrorReceipt(suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)
suite.Require().True(ok)

errorReceipt.Message = invalidMessage
suite.chainB.GetSimApp().IBCKeeper.ChannelKeeper.SetUpgradeErrorReceipt(suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, errorReceipt)
suite.coordinator.CommitBlock(suite.chainB)
},
expError: commitmenttypes.ErrInvalidProof,
},
}

for _, tc := range tests {
Expand Down Expand Up @@ -1347,9 +1374,11 @@ func (suite *KeeperTestSuite) TestChanUpgradeCancel() {
channel.State = types.FLUSHCOMPLETE
path.EndpointA.SetChannel(channel)

authority = sender
damiannolan marked this conversation as resolved.
Show resolved Hide resolved

tc.malleate()

err := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.ChanUpgradeCancel(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, errorReceipt, errorReceiptProof, proofHeight, sender)
err := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.ChanUpgradeCancel(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, errorReceipt, errorReceiptProof, proofHeight, sender, authority)

expPass := tc.expError == nil
if expPass {
Expand Down
3 changes: 2 additions & 1 deletion modules/core/keeper/msg_server.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1018,7 +1018,8 @@ func (k Keeper) ChannelUpgradeCancel(goCtx context.Context, msg *channeltypes.Ms
return nil, errorsmod.Wrapf(porttypes.ErrInvalidRoute, "route not found to module: %s", module)
}

if err := k.ChannelKeeper.ChanUpgradeCancel(ctx, msg.PortId, msg.ChannelId, msg.ErrorReceipt, msg.ProofErrorReceipt, msg.ProofHeight, msg.Signer); err != nil {
authority := k.GetAuthority()
if err := k.ChannelKeeper.ChanUpgradeCancel(ctx, msg.PortId, msg.ChannelId, msg.ErrorReceipt, msg.ProofErrorReceipt, msg.ProofHeight, msg.Signer, authority); err != nil {
ctx.Logger().Error("channel upgrade cancel failed", "port-id", msg.PortId, "error", err.Error())
return nil, errorsmod.Wrap(err, "channel upgrade cancel failed")
}
Expand Down
Loading