Fix Prefixes and Create Prefix Store for Clients (#5980)

* fix prefixes

* remove unnecessary prefixes

Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com>

x/ibc/02-client/client/utils/utils.go

@@ -44,7 +44,7 @@ func QueryClientState(
 ) (types.StateResponse, error) {
 	req := abci.RequestQuery{
 		Path:  "store/ibc/key",
-		Data:  ibctypes.KeyClientState(clientID),
+		Data:  prefixClientKey(clientID, ibctypes.KeyClientState()),
 		Prove: prove,
 	}
 
@@ -72,7 +72,7 @@ func QueryConsensusState(
 
 	req := abci.RequestQuery{
 		Path:  "store/ibc/key",
-		Data:  ibctypes.KeyConsensusState(clientID, height),
+		Data:  prefixClientKey(clientID, ibctypes.KeyConsensusState(height)),
 		Prove: prove,
 	}
 
@@ -155,3 +155,7 @@ func QueryNodeConsensusState(cliCtx context.CLIContext) (ibctmtypes.ConsensusSta
 
 	return state, height, nil
 }
+
+func prefixClientKey(clientID string, key []byte) []byte {
+	return append([]byte(fmt.Sprintf("clients/%s/", clientID)), key...)
+}

x/ibc/02-client/keeper/keeper.go

@@ -2,11 +2,13 @@ package keeper
 
 import (
 	"fmt"
+	"strings"
 
 	"github.com/tendermint/tendermint/libs/log"
 	tmtypes "github.com/tendermint/tendermint/types"
 
 	"github.com/cosmos/cosmos-sdk/codec"
+	"github.com/cosmos/cosmos-sdk/store/prefix"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/cosmos-sdk/x/ibc/02-client/exported"
 	"github.com/cosmos/cosmos-sdk/x/ibc/02-client/types"
@@ -40,8 +42,8 @@ func (k Keeper) Logger(ctx sdk.Context) log.Logger {
 
 // GetClientState gets a particular client from the store
 func (k Keeper) GetClientState(ctx sdk.Context, clientID string) (exported.ClientState, bool) {
-	store := ctx.KVStore(k.storeKey)
+	store := k.clientStore(ctx, clientID)
-	bz := store.Get(ibctypes.KeyClientState(clientID))
+	bz := store.Get(ibctypes.KeyClientState())
 	if bz == nil {
 		return nil, false
 	}
@@ -53,15 +55,15 @@ func (k Keeper) GetClientState(ctx sdk.Context, clientID string) (exported.Clien
 
 // SetClientState sets a particular Client to the store
 func (k Keeper) SetClientState(ctx sdk.Context, clientState exported.ClientState) {
-	store := ctx.KVStore(k.storeKey)
+	store := k.clientStore(ctx, clientState.GetID())
 	bz := k.cdc.MustMarshalBinaryLengthPrefixed(clientState)
-	store.Set(ibctypes.KeyClientState(clientState.GetID()), bz)
+	store.Set(ibctypes.KeyClientState(), bz)
 }
 
 // GetClientType gets the consensus type for a specific client
 func (k Keeper) GetClientType(ctx sdk.Context, clientID string) (exported.ClientType, bool) {
-	store := ctx.KVStore(k.storeKey)
+	store := k.clientStore(ctx, clientID)
-	bz := store.Get(ibctypes.KeyClientType(clientID))
+	bz := store.Get(ibctypes.KeyClientType())
 	if bz == nil {
 		return 0, false
 	}
@@ -71,14 +73,14 @@ func (k Keeper) GetClientType(ctx sdk.Context, clientID string) (exported.Client
 
 // SetClientType sets the specific client consensus type to the provable store
 func (k Keeper) SetClientType(ctx sdk.Context, clientID string, clientType exported.ClientType) {
-	store := ctx.KVStore(k.storeKey)
+	store := k.clientStore(ctx, clientID)
-	store.Set(ibctypes.KeyClientType(clientID), []byte{byte(clientType)})
+	store.Set(ibctypes.KeyClientType(), []byte{byte(clientType)})
 }
 
 // GetClientConsensusState gets the stored consensus state from a client at a given height.
 func (k Keeper) GetClientConsensusState(ctx sdk.Context, clientID string, height uint64) (exported.ConsensusState, bool) {
-	store := ctx.KVStore(k.storeKey)
+	store := k.clientStore(ctx, clientID)
-	bz := store.Get(ibctypes.KeyConsensusState(clientID, height))
+	bz := store.Get(ibctypes.KeyConsensusState(height))
 	if bz == nil {
 		return nil, false
 	}
@@ -91,16 +93,16 @@ func (k Keeper) GetClientConsensusState(ctx sdk.Context, clientID string, height
 // SetClientConsensusState sets a ConsensusState to a particular client at the given
 // height
 func (k Keeper) SetClientConsensusState(ctx sdk.Context, clientID string, height uint64, consensusState exported.ConsensusState) {
-	store := ctx.KVStore(k.storeKey)
+	store := k.clientStore(ctx, clientID)
 	bz := k.cdc.MustMarshalBinaryLengthPrefixed(consensusState)
-	store.Set(ibctypes.KeyConsensusState(clientID, height), bz)
+	store.Set(ibctypes.KeyConsensusState(height), bz)
 }
 
 // HasClientConsensusState returns if keeper has a ConsensusState for a particular
 // client at the given height
 func (k Keeper) HasClientConsensusState(ctx sdk.Context, clientID string, height uint64) bool {
-	store := ctx.KVStore(k.storeKey)
+	store := k.clientStore(ctx, clientID)
-	return store.Has(ibctypes.KeyConsensusState(clientID, height))
+	return store.Has(ibctypes.KeyConsensusState(height))
 }
 
 // GetLatestClientConsensusState gets the latest ConsensusState stored for a given client
@@ -148,10 +150,14 @@ func (k Keeper) GetSelfConsensusState(ctx sdk.Context, height uint64) (exported.
 // the iterator will close and stop.
 func (k Keeper) IterateClients(ctx sdk.Context, cb func(exported.ClientState) bool) {
 	store := ctx.KVStore(k.storeKey)
-	iterator := sdk.KVStorePrefixIterator(store, ibctypes.KeyClientPrefix)
+	iterator := sdk.KVStorePrefixIterator(store, ibctypes.KeyClientStorePrefix)
 
 	defer iterator.Close()
 	for ; iterator.Valid(); iterator.Next() {
+		keySplit := strings.Split(string(iterator.Key()), "/")
+		if keySplit[len(keySplit)-1] != "clientState" {
+			continue
+		}
 		var clientState exported.ClientState
 		k.cdc.MustUnmarshalBinaryLengthPrefixed(iterator.Value(), &clientState)
 
@@ -169,3 +175,12 @@ func (k Keeper) GetAllClients(ctx sdk.Context) (states []exported.ClientState) {
 	})
 	return states
 }
+
+// Returns isolated prefix store for each client so they can read/write in separate
+// namespace without being able to read/write other client's data
+func (k Keeper) clientStore(ctx sdk.Context, clientID string) sdk.KVStore {
+	// append here is safe, appends within a function won't cause
+	// weird side effects when its singlethreaded
+	clientPrefix := append([]byte("clients/"+clientID), '/')
+	return prefix.NewStore(ctx.KVStore(k.storeKey), clientPrefix)
+}

x/ibc/02-client/types/querier.go

@@ -48,7 +48,7 @@ func NewClientStateResponse(
 	return StateResponse{
 		ClientState: clientState,
 		Proof:       commitmenttypes.MerkleProof{Proof: proof},
-		ProofPath:   commitmenttypes.NewMerklePath(strings.Split(ibctypes.ClientStatePath(clientID), "/")),
+		ProofPath:   commitmenttypes.NewMerklePath(append([]string{clientID}, strings.Split(ibctypes.ClientStatePath(), "/")...)),
 		ProofHeight: uint64(height),
 	}
 }
@@ -69,7 +69,7 @@ func NewConsensusStateResponse(
 	return ConsensusStateResponse{
 		ConsensusState: cs,
 		Proof:          commitmenttypes.MerkleProof{Proof: proof},
-		ProofPath:      commitmenttypes.NewMerklePath(strings.Split(ibctypes.ConsensusStatePath(clientID, uint64(height)), "/")),
+		ProofPath:      commitmenttypes.NewMerklePath(append([]string{clientID}, strings.Split(ibctypes.ClientStatePath(), "/")...)),
 		ProofHeight:    uint64(height),
 	}
 }

x/ibc/03-connection/keeper/handshake_test.go

@@ -118,7 +118,7 @@ func (suite *KeeperTestSuite) TestConnOpenTry() {
 			connectionKey := ibctypes.KeyConnection(testConnectionIDA)
 			proofInit, proofHeight := queryProof(suite.chainA, connectionKey)
 
-			consensusKey := ibctypes.KeyConsensusState(testClientIDB, consensusHeight)
+			consensusKey := prefixedClientKey(testClientIDB, ibctypes.KeyConsensusState(consensusHeight))
 			proofConsensus, _ := queryProof(suite.chainA, consensusKey)
 
 			err := suite.chainB.App.IBCKeeper.ConnectionKeeper.ConnOpenTry(
@@ -209,7 +209,7 @@ func (suite *KeeperTestSuite) TestConnOpenAck() {
 			connectionKey := ibctypes.KeyConnection(testConnectionIDB)
 			proofTry, proofHeight := queryProof(suite.chainB, connectionKey)
 
-			consensusKey := ibctypes.KeyConsensusState(testClientIDA, consensusHeight)
+			consensusKey := prefixedClientKey(testClientIDA, ibctypes.KeyConsensusState(consensusHeight))
 			proofConsensus, _ := queryProof(suite.chainB, consensusKey)
 
 			err := suite.chainA.App.IBCKeeper.ConnectionKeeper.ConnOpenAck(

x/ibc/03-connection/keeper/keeper_test.go

@@ -316,3 +316,7 @@ func nextHeader(chain *TestChain) ibctmtypes.Header {
 	return ibctmtypes.CreateTestHeader(chain.Header.ChainID, chain.Header.Height+1,
 		time.Now(), chain.Vals, chain.Signers)
 }
+
+func prefixedClientKey(clientID string, key []byte) []byte {
+	return append([]byte("clients/"+clientID+"/"), key...)
+}

x/ibc/03-connection/keeper/verify_test.go

@@ -67,7 +67,7 @@ func (suite *KeeperTestSuite) TestVerifyClientConsensusState() {
 
 			// TODO: is this the right consensus height
 			consensusHeight := uint64(suite.chainA.Header.Height)
-			consensusKey := ibctypes.KeyConsensusState(testClientIDA, consensusHeight)
+			consensusKey := prefixedClientKey(testClientIDA, ibctypes.KeyConsensusState(consensusHeight))
 
 			// get proof that chainB stored chainA' consensus state
 			proof, proofHeight := queryProof(suite.chainB, consensusKey)

x/ibc/04-channel/keeper/keeper.go

@@ -139,7 +139,7 @@ func (k Keeper) GetPacketAcknowledgement(ctx sdk.Context, portID, channelID stri
 // and stop.
 func (k Keeper) IterateChannels(ctx sdk.Context, cb func(types.IdentifiedChannel) bool) {
 	store := ctx.KVStore(k.storeKey)
-	iterator := sdk.KVStorePrefixIterator(store, ibctypes.GetChannelPortsKeysPrefix(ibctypes.KeyChannelPrefix))
+	iterator := sdk.KVStorePrefixIterator(store, []byte(ibctypes.KeyChannelPrefix))
 
 	defer iterator.Close()
 	for ; iterator.Valid(); iterator.Next() {

x/ibc/07-tendermint/types/client_state.go

@@ -114,7 +114,8 @@ func (cs ClientState) VerifyClientConsensusState(
 	proof commitmentexported.Proof,
 	consensusState clientexported.ConsensusState,
 ) error {
-	path, err := commitmenttypes.ApplyPrefix(prefix, ibctypes.ConsensusStatePath(counterpartyClientIdentifier, consensusHeight))
+	clientPrefixedPath := "clients/" + counterpartyClientIdentifier + "/" + ibctypes.ConsensusStatePath(consensusHeight)
+	path, err := commitmenttypes.ApplyPrefix(prefix, clientPrefixedPath)
 	if err != nil {
 		return err
 	}

x/ibc/types/keys.go

@@ -21,22 +21,18 @@ const (
 
 // KVStore key prefixes for IBC
 var (
-	KeyClientPrefix            = []byte("clientState")
+	KeyClientStorePrefix = []byte("clients")
-	KeyClientTypePrefix        = []byte("clientType")
+	KeyConnectionPrefix  = []byte("connections")
-	KeyConsensusStatePrefix    = []byte("consensusState")
-	KeyClientConnectionsPrefix = []byte("clientConnections")
-	KeyConnectionPrefix        = []byte("connection")
 )
 
 // KVStore key prefixes for IBC
 const (
-	KeyChannelPrefix int = iota + 1
+	KeyChannelPrefix           = "channelEnds"
-	KeyChannelCapabilityPrefix
+	KeyChannelCapabilityPrefix = "capabilities"
-	KeyNextSeqSendPrefix
+	KeyNextSeqSendPrefix       = "seqSends"
-	KeyNextSeqRecvPrefix
+	KeyNextSeqRecvPrefix       = "seqRecvs"
-	KeyPacketCommitmentPrefix
+	KeyPacketCommitmentPrefix  = "commitments"
-	KeyPacketAckPrefix
+	KeyPacketAckPrefix         = "acks"
-	KeyPortsPrefix
 )
 
 // KeyPrefixBytes return the key prefix bytes from a URL string format
@@ -49,36 +45,36 @@ func KeyPrefixBytes(prefix int) []byte {
 
 // ClientStatePath takes an Identifier and returns a Path under which to store a
 // particular client state
-func ClientStatePath(clientID string) string {
+func ClientStatePath() string {
-	return fmt.Sprintf("clientState/%s", clientID)
+	return "clientState"
 }
 
 // ClientTypePath takes an Identifier and returns Path under which to store the
 // type of a particular client.
-func ClientTypePath(clientID string) string {
+func ClientTypePath() string {
-	return fmt.Sprintf("clientType/%s", clientID)
+	return "clientType"
 }
 
 // ConsensusStatePath takes an Identifier and returns a Path under which to
 // store the consensus state of a client.
-func ConsensusStatePath(clientID string, height uint64) string {
+func ConsensusStatePath(height uint64) string {
-	return fmt.Sprintf("consensusState/%s/%d", clientID, height)
+	return fmt.Sprintf("consensusState/%d", height)
 }
 
 // KeyClientState returns the store key for a particular client state
-func KeyClientState(clientID string) []byte {
+func KeyClientState() []byte {
-	return []byte(ClientStatePath(clientID))
+	return []byte(ClientStatePath())
 }
 
 // KeyClientType returns the store key for type of a particular client
-func KeyClientType(clientID string) []byte {
+func KeyClientType() []byte {
-	return []byte(ClientTypePath(clientID))
+	return []byte(ClientTypePath())
 }
 
 // KeyConsensusState returns the store key for the consensus state of a particular
 // client
-func KeyConsensusState(clientID string, height uint64) []byte {
+func KeyConsensusState(height uint64) []byte {
-	return []byte(ConsensusStatePath(clientID, height))
+	return []byte(ConsensusStatePath(height))
 }
 
 // ICS03
@@ -86,12 +82,12 @@ func KeyConsensusState(clientID string, height uint64) []byte {
 
 // ClientConnectionsPath defines a reverse mapping from clients to a set of connections
 func ClientConnectionsPath(clientID string) string {
-	return fmt.Sprintf("clientConnections/%s/", clientID)
+	return fmt.Sprintf("clients/%s/connections", clientID)
 }
 
 // ConnectionPath defines the path under which connection paths are stored
 func ConnectionPath(connectionID string) string {
-	return fmt.Sprintf("connection/%s", connectionID)
+	return fmt.Sprintf("connections/%s", connectionID)
 }
 
 // KeyClientConnections returns the store key for the connectios of a given client
@@ -114,33 +110,33 @@ func GetChannelPortsKeysPrefix(prefix int) []byte {
 
 // ChannelPath defines the path under which channels are stored
 func ChannelPath(portID, channelID string) string {
-	return fmt.Sprintf("%d/", KeyChannelPrefix) + channelPath(portID, channelID)
+	return fmt.Sprintf("%s/", KeyChannelPrefix) + channelPath(portID, channelID)
 }
 
 // ChannelCapabilityPath defines the path under which capability keys associated
 // with a channel are stored
 func ChannelCapabilityPath(portID, channelID string) string {
-	return fmt.Sprintf("%d/", KeyChannelCapabilityPrefix) + channelPath(portID, channelID) + "/key"
+	return fmt.Sprintf("%s/", KeyChannelCapabilityPrefix) + channelPath(portID, channelID) + "/key"
 }
 
 // NextSequenceSendPath defines the next send sequence counter store path
 func NextSequenceSendPath(portID, channelID string) string {
-	return fmt.Sprintf("%d/", KeyNextSeqSendPrefix) + channelPath(portID, channelID) + "/nextSequenceSend"
+	return fmt.Sprintf("%s/", KeyNextSeqSendPrefix) + channelPath(portID, channelID) + "/nextSequenceSend"
 }
 
 // NextSequenceRecvPath defines the next receive sequence counter store path
 func NextSequenceRecvPath(portID, channelID string) string {
-	return fmt.Sprintf("%d/", KeyNextSeqRecvPrefix) + channelPath(portID, channelID) + "/nextSequenceRecv"
+	return fmt.Sprintf("%s/", KeyNextSeqRecvPrefix) + channelPath(portID, channelID) + "/nextSequenceRecv"
 }
 
 // PacketCommitmentPath defines the commitments to packet data fields store path
 func PacketCommitmentPath(portID, channelID string, sequence uint64) string {
-	return fmt.Sprintf("%d/", KeyPacketCommitmentPrefix) + channelPath(portID, channelID) + fmt.Sprintf("/packets/%d", sequence)
+	return fmt.Sprintf("%s/", KeyPacketCommitmentPrefix) + channelPath(portID, channelID) + fmt.Sprintf("/packets/%d", sequence)
 }
 
 // PacketAcknowledgementPath defines the packet acknowledgement store path
 func PacketAcknowledgementPath(portID, channelID string, sequence uint64) string {
-	return fmt.Sprintf("%d/", KeyPacketAckPrefix) + channelPath(portID, channelID) + fmt.Sprintf("/acknowledgements/%d", sequence)
+	return fmt.Sprintf("%s/", KeyPacketAckPrefix) + channelPath(portID, channelID) + fmt.Sprintf("/acknowledgements/%d", sequence)
 }
 
 // KeyChannel returns the store key for a particular channel
@@ -198,7 +194,7 @@ func MustParseChannelPath(path string) (string, string) {
 
 // PortPath defines the path under which ports paths are stored
 func PortPath(portID string) string {
-	return fmt.Sprintf("%d/ports/%s", KeyPortsPrefix, portID)
+	return fmt.Sprintf("ports/%s", portID)
 }
 
 // KeyPort returns the store key for a particular port