From 37a2e4b44fc81350460a93c71540f7c98df9b18c Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Wed, 22 Nov 2023 11:21:01 +0100 Subject: [PATCH] refactor: use isAuthority bool in favour of passing signer and authority to 04-channel --- modules/core/04-channel/keeper/upgrade.go | 4 ++-- .../core/04-channel/keeper/upgrade_test.go | 22 ++++++++----------- modules/core/keeper/msg_server.go | 4 ++-- 3 files changed, 13 insertions(+), 17 deletions(-) diff --git a/modules/core/04-channel/keeper/upgrade.go b/modules/core/04-channel/keeper/upgrade.go index 08c73f7b513..0b6e5a3f215 100644 --- a/modules/core/04-channel/keeper/upgrade.go +++ b/modules/core/04-channel/keeper/upgrade.go @@ -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, authority string) error { +func (k Keeper) ChanUpgradeCancel(ctx sdk.Context, portID, channelID string, errorReceipt types.ErrorReceipt, errorReceiptProof []byte, proofHeight clienttypes.Height, isAuthority bool) error { channel, found := k.GetChannel(ctx, portID, channelID) if !found { return errorsmod.Wrapf(types.ErrChannelNotFound, "port ID (%s) channel ID (%s)", portID, channelID) @@ -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 signer == authority && channel.State != types.FLUSHCOMPLETE { + if isAuthority && channel.State != types.FLUSHCOMPLETE { return nil } diff --git a/modules/core/04-channel/keeper/upgrade_test.go b/modules/core/04-channel/keeper/upgrade_test.go index cd83be869ac..7a76c23560f 100644 --- a/modules/core/04-channel/keeper/upgrade_test.go +++ b/modules/core/04-channel/keeper/upgrade_test.go @@ -1202,12 +1202,9 @@ func (suite *KeeperTestSuite) TestChanUpgradeCancel() { errorReceipt types.ErrorReceipt errorReceiptProof []byte proofHeight clienttypes.Height - sender string - authority string + isAuthority bool ) - const invalidMessage = "different message" - tests := []struct { name string malleate func() @@ -1238,9 +1235,11 @@ func (suite *KeeperTestSuite) TestChanUpgradeCancel() { 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 + errorReceipt.Message = ibctesting.InvalidID suite.chainB.GetSimApp().IBCKeeper.ChannelKeeper.SetUpgradeErrorReceipt(suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, errorReceipt) suite.coordinator.CommitBlock(suite.chainB) + + isAuthority = true }, expError: nil, }, @@ -1294,24 +1293,23 @@ func (suite *KeeperTestSuite) TestChanUpgradeCancel() { 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 + errorReceipt.Message = ibctesting.InvalidID suite.chainB.GetSimApp().IBCKeeper.ChannelKeeper.SetUpgradeErrorReceipt(suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, errorReceipt) - suite.coordinator.CommitBlock(suite.chainB) + + isAuthority = true }, expError: commitmenttypes.ErrInvalidProof, }, { 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 + errorReceipt.Message = ibctesting.InvalidID suite.chainB.GetSimApp().IBCKeeper.ChannelKeeper.SetUpgradeErrorReceipt(suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, errorReceipt) suite.coordinator.CommitBlock(suite.chainB) }, @@ -1362,11 +1360,9 @@ func (suite *KeeperTestSuite) TestChanUpgradeCancel() { channel.State = types.FLUSHCOMPLETE path.EndpointA.SetChannel(channel) - authority = sender - tc.malleate() - err := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.ChanUpgradeCancel(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, errorReceipt, errorReceiptProof, proofHeight, sender, authority) + err := suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.ChanUpgradeCancel(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, errorReceipt, errorReceiptProof, proofHeight, isAuthority) expPass := tc.expError == nil if expPass { diff --git a/modules/core/keeper/msg_server.go b/modules/core/keeper/msg_server.go index 9728a81d958..7aa3deadb22 100644 --- a/modules/core/keeper/msg_server.go +++ b/modules/core/keeper/msg_server.go @@ -1018,8 +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) } - authority := k.GetAuthority() - if err := k.ChannelKeeper.ChanUpgradeCancel(ctx, msg.PortId, msg.ChannelId, msg.ErrorReceipt, msg.ProofErrorReceipt, msg.ProofHeight, msg.Signer, authority); err != nil { + isAuthority := k.GetAuthority() == msg.Signer + if err := k.ChannelKeeper.ChanUpgradeCancel(ctx, msg.PortId, msg.ChannelId, msg.ErrorReceipt, msg.ProofErrorReceipt, msg.ProofHeight, isAuthority); 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") }