add lock to IBC Map (#3101)

* add lock

* pointer => index

* add note

* fix ut

* ut rerun

* rm debug

---------

Co-authored-by: ylsGit <[email protected]>
Co-authored-by: KamiD <[email protected]>

libs/cosmos-sdk/x/capability/keeper/keeper.go

@@ -3,6 +3,7 @@ package keeper
 import (
 	"fmt"
 	"strings"
+	"sync"
 
 	tmtypes "github.com/okex/exchain/libs/tendermint/types"
 
@@ -39,7 +40,7 @@ type (
 		cdc           *codec.Codec
 		storeKey      sdk.StoreKey
 		memKey        sdk.StoreKey
-		capMap        map[uint64]*types.Capability
+		capMap        *sync.Map
 		scopedModules map[string]struct{}
 		sealed        bool
 	}
@@ -54,7 +55,7 @@ type (
 		cdc      *codec.Codec
 		storeKey sdk.StoreKey
 		memKey   sdk.StoreKey
-		capMap   map[uint64]*types.Capability
+		capMap   *sync.Map
 		module   string
 	}
 )
@@ -66,7 +67,7 @@ func NewKeeper(cdc *codec.CodecProxy, storeKey, memKey sdk.StoreKey) *Keeper {
 		cdc:           cdc.GetCdc(),
 		storeKey:      storeKey,
 		memKey:        memKey,
-		capMap:        make(map[uint64]*types.Capability),
+		capMap:        &sync.Map{},
 		scopedModules: make(map[string]struct{}),
 		sealed:        false,
 	}
@@ -200,7 +201,7 @@ func (k Keeper) InitializeCapability(ctx sdk.Context, index uint64, owners types
 		memStore.Set(types.RevCapabilityKey(owner.Module, owner.Name), sdk.Uint64ToBigEndian(index))
 
 		// Set the mapping from index from index to in-memory capability in the go map
-		k.capMap[index] = cap
+		k.capMap.Store(index, cap)
 	}
 
 }
@@ -223,8 +224,6 @@ func (sk ScopedKeeper) NewCapability(ctx sdk.Context, name string) (*types.Capab
 	if _, ok := sk.GetCapability(ctx, name); ok {
 		return nil, sdkerrors.Wrapf(types.ErrCapabilityTaken, fmt.Sprintf("module: %s, name: %s", sk.module, name))
 	}
-
-	// create new capability with the current global index
 	index := types.IndexFromKey(store.Get(types.KeyIndex))
 	cap := types.NewCapability(index)
 
@@ -249,7 +248,7 @@ func (sk ScopedKeeper) NewCapability(ctx sdk.Context, name string) (*types.Capab
 	memStore.Set(types.RevCapabilityKey(sk.module, name), sdk.Uint64ToBigEndian(index))
 
 	// Set the mapping from index from index to in-memory capability in the go map
-	sk.capMap[index] = cap
+	sk.capMap.Store(index, cap)
 
 	logger(ctx).Info("created new capability", "module", sk.module, "name", name)
 
@@ -340,7 +339,7 @@ func (sk ScopedKeeper) ReleaseCapability(ctx sdk.Context, cap *types.Capability)
 		// remove capability owner set
 		prefixStore.Delete(indexKey)
 		// since no one owns capability, we can delete capability from map
-		delete(sk.capMap, cap.GetIndex())
+		sk.capMap.Delete(cap.GetIndex())
 	} else {
 		// update capability owner set
 		prefixStore.Set(indexKey, sk.cdc.MustMarshalBinaryBare(capOwners))
@@ -395,7 +394,6 @@ func (sk ScopedKeeper) GetCapability(ctx sdk.Context, name string) (*types.Capab
 	//}
 	//
 	//return cap, true
-
 	if strings.TrimSpace(name) == "" {
 		return nil, false
 	}
@@ -416,12 +414,12 @@ func (sk ScopedKeeper) GetCapability(ctx sdk.Context, name string) (*types.Capab
 		return nil, false
 	}
 
-	cap := sk.capMap[index]
-	if cap == nil {
+	cap, ok := sk.capMap.Load(index)
+	if !ok {
 		panic("capability found in memstore is missing from map")
 	}
 
-	return cap, true
+	return cap.(*types.Capability), true
 }
 
 // GetCapabilityName allows a module to retrieve the name under which it stored a given

libs/cosmos-sdk/x/capability/keeper/keeper_test.go

@@ -111,7 +111,7 @@ func (suite *KeeperTestSuite) TestAuthenticateCapability() {
 	suite.Require().NoError(err)
 	suite.Require().NotNil(cap1)
 
-	forgedCap := types.NewCapability(cap1.Index) // index should be the same index as the first capability
+	forgedCap := types.NewCapability(cap1.Index + 1) // index should be the same index as the first capability
 	suite.Require().False(sk1.AuthenticateCapability(suite.ctx, forgedCap, "transfer"))
 	suite.Require().False(sk2.AuthenticateCapability(suite.ctx, forgedCap, "transfer"))
 

libs/cosmos-sdk/x/capability/types/keys.go

@@ -2,6 +2,7 @@ package types
 
 import (
 	"fmt"
+
 	sdk "github.com/okex/exchain/libs/cosmos-sdk/types"
 )
 
@@ -37,8 +38,11 @@ func RevCapabilityKey(module, name string) []byte {
 
 // FwdCapabilityKey returns a forward lookup key for a given module and capability
 // reference.
+// NOTE, FwdCapabilityKey use the pointer address to build key
+// which means, when simulate and deliver  concurrently execute ,the capMap maybe override by simulate which will fail
+// to create the channel
 func FwdCapabilityKey(module string, cap *Capability) []byte {
-	return []byte(fmt.Sprintf("%s/fwd/%p", module, cap))
+	return []byte(fmt.Sprintf("%s/fwd/%d", module, cap.Index))
 }
 
 // IndexToKey returns bytes to be used as a key for a given capability index.

libs/cosmos-sdk/x/capability/types/keys_test.go

@@ -16,7 +16,7 @@ func TestRevCapabilityKey(t *testing.T) {
 
 func TestFwdCapabilityKey(t *testing.T) {
 	cap := types.NewCapability(23)
-	expected := []byte(fmt.Sprintf("bank/fwd/%p", cap))
+	expected := []byte(fmt.Sprintf("bank/fwd/%d", cap.Index))
 	require.Equal(t, expected, types.FwdCapabilityKey("bank", cap))
 }
 

libs/ibc-go/modules/core/04-channel/keeper/handshake_test.go

@@ -41,6 +41,8 @@ func (suite *KeeperTestSuite) TestChanOpenInit() {
 		}, true},
 		{"channel already exists", func() {
 			suite.coordinator.Setup(path)
+			// we refactor the `FwdCapabilityKey`,so we have to change the index
+			portCap.Index = 100
 		}, false},
 		{"connection doesn't exist", func() {
 			// any non-empty values
@@ -151,6 +153,22 @@ func (suite *KeeperTestSuite) TestChanOpenTry() {
 	)
 
 	testCases := []testCase{
+		{"connection does not support ORDERED channels", func() {
+			suite.coordinator.SetupConnections(path)
+			path.SetChannelOrdered()
+			path.EndpointA.ChanOpenInit()
+
+			// modify connA versions to only support UNORDERED channels
+			conn := path.EndpointA.GetConnection()
+
+			version := connectiontypes.NewVersion("1", []string{"ORDER_UNORDERED"})
+			conn.Versions = []*connectiontypes.Version{version}
+
+			suite.chainA.GetSimApp().GetIBCKeeper().ConnectionKeeper.SetConnection(
+				suite.chainA.GetContext(),
+				path.EndpointA.ConnectionID, conn,
+			)
+		}, false},
 		{"success", func() {
 			suite.coordinator.SetupConnections(path)
 			path.SetChannelOrdered()
@@ -216,24 +234,6 @@ func (suite *KeeperTestSuite) TestChanOpenTry() {
 			suite.chainB.CreatePortCapability(suite.chainB.GetSimApp().ScopedIBCMockKeeper, ibctesting.MockPort)
 			portCap = suite.chainB.GetPortCapability(ibctesting.MockPort)
 		}, false},
-		{"connection does not support ORDERED channels", func() {
-			suite.coordinator.SetupConnections(path)
-			path.SetChannelOrdered()
-			path.EndpointA.ChanOpenInit()
-
-			// modify connA versions to only support UNORDERED channels
-			conn := path.EndpointA.GetConnection()
-
-			version := connectiontypes.NewVersion("1", []string{"ORDER_UNORDERED"})
-			conn.Versions = []*connectiontypes.Version{version}
-
-			suite.chainA.GetSimApp().GetIBCKeeper().ConnectionKeeper.SetConnection(
-				suite.chainA.GetContext(),
-				path.EndpointA.ConnectionID, conn,
-			)
-			suite.chainA.CreatePortCapability(suite.chainA.GetSimApp().ScopedIBCMockKeeper, ibctesting.MockPort)
-			portCap = suite.chainA.GetPortCapability(ibctesting.MockPort)
-		}, false},
 	}
 
 	for _, tc := range testCases {

libs/ibc-go/modules/core/04-channel/keeper/packet_test.go

@@ -199,7 +199,7 @@ func (suite *KeeperTestSuite) TestSendPacket() {
 		{"channel capability not found", func() {
 			suite.coordinator.Setup(path)
 			packet = types.NewPacket(ibctesting.MockPacketData, 1, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, timeoutHeight, disabledTimeoutTimestamp)
-			channelCap = capabilitytypes.NewCapability(5)
+			channelCap = capabilitytypes.NewCapability(10)
 		}, false},
 	}