fix(api)!: remove Pretty and String custom implementations of `Me…

…rklePath` (#4459)

## Description



Second part of #4128. First part was #4429.

closes: #4128 

### Commit Message / Changelog Entry

```bash
fix(api)!: remove `Pretty` and `String` custom implementations of `MerklePath`
```

see the [guidelines](https://github.com/cosmos/ibc-go/blob/main/docs/dev/pull-requests.md#commit-messages) for commit messages. (view raw markdown for examples)




---

Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.

- [x] Targeted PR against correct branch (see [CONTRIBUTING.md](https://github.com/cosmos/ibc-go/blob/main/docs/dev/pull-requests.md#pull-request-targeting)).
- [x] Linked to Github issue with discussion and accepted design OR link to spec that describes this work.
- [ ] Code follows the [module structure standards](https://github.com/cosmos/cosmos-sdk/blob/main/docs/docs/building-modules/11-structure.md) and [Go style guide](../docs/dev/go-style-guide.md).
- [ ] Wrote unit and integration [tests](https://github.com/cosmos/ibc-go/blob/main/testing/README.md#ibc-testing-package).
- [ ] Updated relevant documentation (`docs/`) or specification (`x/<module>/spec/`).
- [ ] Added relevant `godoc` [comments](https://blog.golang.org/godoc-documenting-go-code).
- [ ] Provide a [commit message](https://github.com/cosmos/ibc-go/blob/main/docs/dev/pull-requests.md#commit-messages) to be used for the changelog entry in the PR description for review.
- [x] Re-reviewed `Files changed` in the Github PR explorer.
- [ ] Review `Codecov Report` in the comment section below once CI passes.

modules/core/23-commitment/types/merkle.go

@@ -3,7 +3,6 @@ package types
 import (
 	"bytes"
 	"fmt"
-	"net/url"
 
 	"github.com/cosmos/gogoproto/proto"
 	ics23 "github.com/cosmos/ics23/go"
@@ -77,44 +76,7 @@ func NewMerklePath(keyPath ...string) MerklePath {
 	}
 }
 
-// String implements fmt.Stringer.
-// This represents the path in the same way the tendermint KeyPath will
-// represent a key path. The backslashes partition the key path into
-// the respective stores they belong to.
-//
-// Deprecated: This function makes assumptions about the way a merkle path
-// in a multistore should be represented as a string that are not standardized.
-// The decision on how to represent the merkle path as a string will be deferred
-// to upstream users of the type.
-// This function will be removed in a next release.
-func (mp MerklePath) String() string {
-	pathStr := ""
-	for _, k := range mp.KeyPath {
-		pathStr += "/" + url.PathEscape(k)
-	}
-	return pathStr
-}
-
-// Pretty returns the unescaped path of the URL string.
-// This function will unescape any backslash within a particular store key.
-// This makes the keypath more human-readable while removing information
-// about the exact partitions in the key path.
-//
-// Deprecated: This function makes assumptions about the way a merkle path
-// in a multistore should be represented as a string that are not standardized.
-// The decision on how to represent the merkle path as a string will be deferred
-// to upstream users of the type.
-// This function will be removed in a next release.
-func (mp MerklePath) Pretty() string {
-	path, err := url.PathUnescape(mp.String())
-	if err != nil {
-		panic(err)
-	}
-	return path
-}
-
 // GetKey will return a byte representation of the key
-// after URL escaping the key element
 func (mp MerklePath) GetKey(i uint64) ([]byte, error) {
 	if i >= uint64(len(mp.KeyPath)) {
 		return nil, fmt.Errorf("index out of range. %d (index) >= %d (len)", i, len(mp.KeyPath))

modules/core/23-commitment/types/merkle_test.go

@@ -150,24 +150,5 @@ func TestApplyPrefix(t *testing.T) {
 
 	prefixedPath, err := types.ApplyPrefix(prefix, path)
 	require.NoError(t, err, "valid prefix returns error")
-
-	require.Equal(t, "/storePrefixKey/"+pathStr, prefixedPath.Pretty(), "Prefixed path incorrect")
-	require.Equal(t, "/storePrefixKey/pathone%2Fpathtwo%2Fpaththree%2Fkey", prefixedPath.String(), "Prefixed escaped path incorrect")
-}
-
-func TestString(t *testing.T) {
-	path := types.NewMerklePath("rootKey", "storeKey", "path/to/leaf")
-
-	require.Equal(t, "/rootKey/storeKey/path%2Fto%2Fleaf", path.String(), "path String returns unxpected value")
-	require.Equal(t, "/rootKey/storeKey/path/to/leaf", path.Pretty(), "path's pretty string representation is incorrect")
-
-	onePath := types.NewMerklePath("path/to/leaf")
-
-	require.Equal(t, "/path%2Fto%2Fleaf", onePath.String(), "one element path does not have correct string representation")
-	require.Equal(t, "/path/to/leaf", onePath.Pretty(), "one element path has incorrect pretty string representation")
-
-	zeroPath := types.NewMerklePath()
-
-	require.Equal(t, "", zeroPath.String(), "zero element path does not have correct string representation")
-	require.Equal(t, "", zeroPath.Pretty(), "zero element path does not have correct pretty string representation")
+	require.Len(t, prefixedPath.GetKeyPath(), 2, "unexpected key path length")
 }

modules/core/exported/commitment.go

@@ -28,7 +28,6 @@ type Prefix interface {
 // Path implements spec:CommitmentPath.
 // A path is the additional information provided to the verification function.
 type Path interface {
-	String() string // deprecated
 	Empty() bool
 }
 

modules/light-clients/06-solomachine/client_state_test.go

@@ -179,7 +179,7 @@ func (suite *SoloMachineTestSuite) TestVerifyMembership() {
 					clientStateBz, err := suite.chainA.Codec.Marshal(clientState)
 					suite.Require().NoError(err)
 
-					path = suite.solomachine.GetClientStatePath(counterpartyClientIdentifier)
+					path = sm.GetClientStatePath(counterpartyClientIdentifier)
 					merklePath, ok := path.(commitmenttypes.MerklePath)
 					suite.Require().True(ok)
 					key, err := merklePath.GetKey(1) // in a multistore context: index 0 is the key for the IBC store in the multistore, index 1 is the key in the IBC store
@@ -474,7 +474,7 @@ func (suite *SoloMachineTestSuite) TestVerifyMembership() {
 			{
 				"malformed proof fails to unmarshal",
 				func() {
-					path = suite.solomachine.GetClientStatePath(counterpartyClientIdentifier)
+					path = sm.GetClientStatePath(counterpartyClientIdentifier)
 					proof = []byte("invalid proof")
 				},
 				false,
@@ -609,22 +609,20 @@ func (suite *SoloMachineTestSuite) TestVerifyMembership() {
 
 func (suite *SoloMachineTestSuite) TestSignBytesMarshalling() {
 	sm := suite.solomachine
-	merklePath := commitmenttypes.NewMerklePath("ibc", "solomachine")
-	key, err := merklePath.GetKey(1) // in a multistore context: index 0 is the key for the IBC store in the multistore, index 1 is the key in the IBC store
-	suite.Require().NoError(err)
+	path := []byte("solomachine")
 	signBytesNilData := solomachine.SignBytes{
 		Sequence:    sm.GetHeight().GetRevisionHeight(),
 		Timestamp:   sm.Time,
 		Diversifier: sm.Diversifier,
-		Path:        key,
+		Path:        path,
 		Data:        nil,
 	}
 
 	signBytesEmptyArray := solomachine.SignBytes{
 		Sequence:    sm.GetHeight().GetRevisionHeight(),
 		Timestamp:   sm.Time,
 		Diversifier: sm.Diversifier,
-		Path:        key,
+		Path:        path,
 		Data:        []byte{},
 	}
 
@@ -662,7 +660,7 @@ func (suite *SoloMachineTestSuite) TestVerifyNonMembership() {
 			{
 				"success: packet receipt absence verification",
 				func() {
-					path = suite.solomachine.GetPacketReceiptPath(ibctesting.MockPort, ibctesting.FirstChannelID, 1)
+					path = sm.GetPacketReceiptPath(ibctesting.MockPort, ibctesting.FirstChannelID, 1)
 					merklePath, ok := path.(commitmenttypes.MerklePath)
 					suite.Require().True(ok)
 					key, err := merklePath.GetKey(1) // in a multistore context: index 0 is the key for the IBC store in the multistore, index 1 is the key in the IBC store
@@ -700,7 +698,7 @@ func (suite *SoloMachineTestSuite) TestVerifyNonMembership() {
 			{
 				"malformed proof fails to unmarshal",
 				func() {
-					path = suite.solomachine.GetClientStatePath(counterpartyClientIdentifier)
+					path = sm.GetClientStatePath(counterpartyClientIdentifier)
 					proof = []byte("invalid proof")
 				},
 				false,

proto/ibc/core/commitment/v1/commitment.proto

@@ -26,8 +26,6 @@ message MerklePrefix {
 // arbitrary structured object (defined by a commitment type).
 // MerklePath is represented from root-to-leaf
 message MerklePath {
-  option (gogoproto.goproto_stringer) = false;
-
   repeated string key_path = 1;
 }
 

testing/solomachine.go

@@ -202,7 +202,7 @@ func (solo *Solomachine) CreateHeader(newDiversifier string) *solomachine.Header
 // CreateMisbehaviour constructs testing misbehaviour for the solo machine client
 // by signing over two different data bytes at the same sequence.
 func (solo *Solomachine) CreateMisbehaviour() *solomachine.Misbehaviour {
-	merklePath := solo.GetClientStatePath("counterparty")
+	merklePath := commitmenttypes.NewMerklePath(host.FullClientStatePath("counterparty"))
 	path, err := solo.cdc.Marshal(&merklePath)
 	require.NoError(solo.t, err)
 
@@ -231,7 +231,7 @@ func (solo *Solomachine) CreateMisbehaviour() *solomachine.Misbehaviour {
 	// misbehaviour signaturess can have different timestamps
 	solo.Time++
 
-	merklePath = solo.GetConsensusStatePath("counterparty", clienttypes.NewHeight(0, 1))
+	merklePath = commitmenttypes.NewMerklePath(host.FullConsensusStatePath("counterparty", clienttypes.NewHeight(0, 1)))
 	path, err = solo.cdc.Marshal(&merklePath)
 	require.NoError(solo.t, err)
 
@@ -516,14 +516,12 @@ func (solo *Solomachine) GenerateClientStateProof(clientState exported.ClientSta
 	data, err := clienttypes.MarshalClientState(solo.cdc, clientState)
 	require.NoError(solo.t, err)
 
-	merklePath := solo.GetClientStatePath(clientIDSolomachine)
-	key, err := merklePath.GetKey(1) // index 0 is the key for the IBC store in the multistore, index 1 is the key in the IBC store
-	require.NoError(solo.t, err)
+	path := host.FullClientStateKey(clientIDSolomachine)
 	signBytes := &solomachine.SignBytes{
 		Sequence:    solo.Sequence,
 		Timestamp:   solo.Time,
 		Diversifier: solo.Diversifier,
-		Path:        key,
+		Path:        path,
 		Data:        data,
 	}
 
@@ -536,14 +534,12 @@ func (solo *Solomachine) GenerateConsensusStateProof(consensusState exported.Con
 	data, err := clienttypes.MarshalConsensusState(solo.cdc, consensusState)
 	require.NoError(solo.t, err)
 
-	merklePath := solo.GetConsensusStatePath(clientIDSolomachine, consensusHeight)
-	key, err := merklePath.GetKey(1) // index 0 is the key for the IBC store in the multistore, index 1 is the key in the IBC store
-	require.NoError(solo.t, err)
+	path := host.FullConsensusStateKey(clientIDSolomachine, consensusHeight)
 	signBytes := &solomachine.SignBytes{
 		Sequence:    solo.Sequence,
 		Timestamp:   solo.Time,
 		Diversifier: solo.Diversifier,
-		Path:        key,
+		Path:        path,
 		Data:        data,
 	}
 
@@ -559,14 +555,12 @@ func (solo *Solomachine) GenerateConnOpenTryProof(counterpartyClientID, counterp
 	data, err := solo.cdc.Marshal(&connection)
 	require.NoError(solo.t, err)
 
-	merklePath := solo.GetConnectionStatePath(connectionIDSolomachine)
-	key, err := merklePath.GetKey(1) // index 0 is the key for the IBC store in the multistore, index 1 is the key in the IBC store
-	require.NoError(solo.t, err)
+	path := host.ConnectionKey(connectionIDSolomachine)
 	signBytes := &solomachine.SignBytes{
 		Sequence:    solo.Sequence,
 		Timestamp:   solo.Time,
 		Diversifier: solo.Diversifier,
-		Path:        key,
+		Path:        path,
 		Data:        data,
 	}
 
@@ -582,14 +576,12 @@ func (solo *Solomachine) GenerateChanOpenTryProof(portID, version, counterpartyC
 	data, err := solo.cdc.Marshal(&channel)
 	require.NoError(solo.t, err)
 
-	merklePath := solo.GetChannelStatePath(portID, channelIDSolomachine)
-	key, err := merklePath.GetKey(1) // index 0 is the key for the IBC store in the multistore, index 1 is the key in the IBC store
-	require.NoError(solo.t, err)
+	path := host.ChannelKey(portID, channelIDSolomachine)
 	signBytes := &solomachine.SignBytes{
 		Sequence:    solo.Sequence,
 		Timestamp:   solo.Time,
 		Diversifier: solo.Diversifier,
-		Path:        key,
+		Path:        path,
 		Data:        data,
 	}
 
@@ -605,14 +597,12 @@ func (solo *Solomachine) GenerateChanClosedProof(portID, version, counterpartyCh
 	data, err := solo.cdc.Marshal(&channel)
 	require.NoError(solo.t, err)
 
-	merklePath := solo.GetChannelStatePath(portID, channelIDSolomachine)
-	key, err := merklePath.GetKey(1) // index 0 is the key for the IBC store in the multistore, index 1 is the key in the IBC store
-	require.NoError(solo.t, err)
+	path := host.ChannelKey(portID, channelIDSolomachine)
 	signBytes := &solomachine.SignBytes{
 		Sequence:    solo.Sequence,
 		Timestamp:   solo.Time,
 		Diversifier: solo.Diversifier,
-		Path:        key,
+		Path:        path,
 		Data:        data,
 	}
 
@@ -623,14 +613,12 @@ func (solo *Solomachine) GenerateChanClosedProof(portID, version, counterpartyCh
 func (solo *Solomachine) GenerateCommitmentProof(packet channeltypes.Packet) []byte {
 	commitment := channeltypes.CommitPacket(solo.cdc, packet)
 
-	merklePath := solo.GetPacketCommitmentPath(packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence())
-	key, err := merklePath.GetKey(1) // index 0 is the key for the IBC store in the multistore, index 1 is the key in the IBC store
-	require.NoError(solo.t, err)
+	path := host.PacketCommitmentKey(packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence())
 	signBytes := &solomachine.SignBytes{
 		Sequence:    solo.Sequence,
 		Timestamp:   solo.Time,
 		Diversifier: solo.Diversifier,
-		Path:        key,
+		Path:        path,
 		Data:        commitment,
 	}
 
@@ -641,14 +629,12 @@ func (solo *Solomachine) GenerateCommitmentProof(packet channeltypes.Packet) []b
 func (solo *Solomachine) GenerateAcknowledgementProof(packet channeltypes.Packet) []byte {
 	transferAck := channeltypes.NewResultAcknowledgement([]byte{byte(1)}).Acknowledgement()
 
-	merklePath := solo.GetPacketAcknowledgementPath(packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence())
-	key, err := merklePath.GetKey(1) // index 0 is the key for the IBC store in the multistore, index 1 is the key in the IBC store
-	require.NoError(solo.t, err)
+	path := host.PacketAcknowledgementKey(packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence())
 	signBytes := &solomachine.SignBytes{
 		Sequence:    solo.Sequence,
 		Timestamp:   solo.Time,
 		Diversifier: solo.Diversifier,
-		Path:        key,
+		Path:        path,
 		Data:        channeltypes.CommitAcknowledgement(transferAck),
 	}
 
@@ -657,14 +643,12 @@ func (solo *Solomachine) GenerateAcknowledgementProof(packet channeltypes.Packet
 
 // GenerateReceiptAbsenceProof generates a receipt absence proof for the provided packet.
 func (solo *Solomachine) GenerateReceiptAbsenceProof(packet channeltypes.Packet) []byte {
-	merklePath := solo.GetPacketReceiptPath(packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence())
-	key, err := merklePath.GetKey(1) // index 0 is the key for the IBC store in the multistore, index 1 is the key in the IBC store
-	require.NoError(solo.t, err)
+	path := host.PacketReceiptKey(packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence())
 	signBytes := &solomachine.SignBytes{
 		Sequence:    solo.Sequence,
 		Timestamp:   solo.Time,
 		Diversifier: solo.Diversifier,
-		Path:        key,
+		Path:        path,
 		Data:        nil,
 	}
 	return solo.GenerateProof(signBytes)