refactor: use isAuthority bool in favour of passing signer and author…

…ity to 04-channel
cosmos · Nov 22, 2023 · 37a2e4b · 37a2e4b
1 parent 11e1aaa
commit 37a2e4b
Show file tree

Hide file tree

Showing 3 changed files with 13 additions and 17 deletions.
diff --git 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
@@ -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
@@ -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")
 	}