From 88e4acac6b5e8eeaa961ae7315eff0a482468f8b Mon Sep 17 00:00:00 2001
From: Aditya <adityasripal@gmail.com>
Date: Mon, 2 Nov 2020 14:32:32 +0000
Subject: [PATCH] Don't claim channel capability on crossing hellos (#7763)

* don't claim capability on crossing hellos

* clarify comment
---
 x/ibc/applications/transfer/keeper/keeper.go |  5 +++++
 x/ibc/applications/transfer/module.go        | 12 +++++++++---
 x/ibc/applications/transfer/module_test.go   | 12 ++++++------
 3 files changed, 20 insertions(+), 9 deletions(-)

diff --git a/x/ibc/applications/transfer/keeper/keeper.go b/x/ibc/applications/transfer/keeper/keeper.go
index c255606da0..e1149ef9be 100644
--- a/x/ibc/applications/transfer/keeper/keeper.go
+++ b/x/ibc/applications/transfer/keeper/keeper.go
@@ -159,6 +159,11 @@ func (k Keeper) IterateDenomTraces(ctx sdk.Context, cb func(denomTrace types.Den
 	}
 }
 
+// AuthenticateCapability wraps the scopedKeeper's AuthenticateCapability function
+func (k Keeper) AuthenticateCapability(ctx sdk.Context, cap *capabilitytypes.Capability, name string) bool {
+	return k.scopedKeeper.AuthenticateCapability(ctx, cap, name)
+}
+
 // ClaimCapability allows the transfer module that can claim a capability that IBC module
 // passes to it
 func (k Keeper) ClaimCapability(ctx sdk.Context, cap *capabilitytypes.Capability, name string) error {
diff --git a/x/ibc/applications/transfer/module.go b/x/ibc/applications/transfer/module.go
index 0f9b3faee2..d14637614b 100644
--- a/x/ibc/applications/transfer/module.go
+++ b/x/ibc/applications/transfer/module.go
@@ -245,9 +245,15 @@ func (am AppModule) OnChanOpenTry(
 		return sdkerrors.Wrapf(types.ErrInvalidVersion, "invalid counterparty version: got: %s, expected %s", counterpartyVersion, types.Version)
 	}
 
-	// Claim channel capability passed back by IBC module
-	if err := am.keeper.ClaimCapability(ctx, chanCap, host.ChannelCapabilityPath(portID, channelID)); err != nil {
-		return err
+	// Module may have already claimed capability in OnChanOpenInit in the case of crossing hellos
+	// (ie chainA and chainB both call ChanOpenInit before one of them calls ChanOpenTry)
+	// If module can already authenticate the capability then module already owns it so we don't need to claim
+	// Otherwise, module does not have channel capability and we must claim it from IBC
+	if !am.keeper.AuthenticateCapability(ctx, chanCap, host.ChannelCapabilityPath(portID, channelID)) {
+		// Only claim channel capability passed back by IBC module if we do not already own it
+		if err := am.keeper.ClaimCapability(ctx, chanCap, host.ChannelCapabilityPath(portID, channelID)); err != nil {
+			return err
+		}
 	}
 
 	return nil
diff --git a/x/ibc/applications/transfer/module_test.go b/x/ibc/applications/transfer/module_test.go
index 308c998672..7f3cec0bde 100644
--- a/x/ibc/applications/transfer/module_test.go
+++ b/x/ibc/applications/transfer/module_test.go
@@ -108,6 +108,12 @@ func (suite *TransferTestSuite) TestOnChanOpenTry() {
 		{
 			"success", func() {}, true,
 		},
+		{
+			"capability already claimed in INIT should pass", func() {
+				err := suite.chainA.App.ScopedTransferKeeper.ClaimCapability(suite.chainA.GetContext(), chanCap, host.ChannelCapabilityPath(testChannel.PortID, testChannel.ID))
+				suite.Require().NoError(err)
+			}, true,
+		},
 		{
 			"invalid order - ORDERED", func() {
 				channel.Ordering = channeltypes.ORDERED
@@ -128,12 +134,6 @@ func (suite *TransferTestSuite) TestOnChanOpenTry() {
 				counterpartyVersion = "version"
 			}, false,
 		},
-		{
-			"capability already claimed", func() {
-				err := suite.chainA.App.ScopedTransferKeeper.ClaimCapability(suite.chainA.GetContext(), chanCap, host.ChannelCapabilityPath(testChannel.PortID, testChannel.ID))
-				suite.Require().NoError(err)
-			}, false,
-		},
 	}
 
 	for _, tc := range testCases {