From 2d638ccf4d2d68765b596594e14bfb1bc917c694 Mon Sep 17 00:00:00 2001 From: Carlos Rodriguez Date: Thu, 24 Aug 2023 21:32:23 +0200 Subject: [PATCH 1/4] remove pretty and string custom implementations for merkle path --- .../core/23-commitment/types/commitment.pb.go | 47 ++++++++++--------- modules/core/23-commitment/types/merkle.go | 38 --------------- .../core/23-commitment/types/merkle_test.go | 21 +-------- modules/core/exported/commitment.go | 1 - .../06-solomachine/client_state_test.go | 4 +- proto/ibc/core/commitment/v1/commitment.proto | 2 - testing/solomachine.go | 36 +++++++------- 7 files changed, 45 insertions(+), 104 deletions(-) diff --git a/modules/core/23-commitment/types/commitment.pb.go b/modules/core/23-commitment/types/commitment.pb.go index 06bb149a828..73267370f99 100644 --- a/modules/core/23-commitment/types/commitment.pb.go +++ b/modules/core/23-commitment/types/commitment.pb.go @@ -117,8 +117,9 @@ type MerklePath struct { KeyPath []string `protobuf:"bytes,1,rep,name=key_path,json=keyPath,proto3" json:"key_path,omitempty"` } -func (m *MerklePath) Reset() { *m = MerklePath{} } -func (*MerklePath) ProtoMessage() {} +func (m *MerklePath) Reset() { *m = MerklePath{} } +func (m *MerklePath) String() string { return proto.CompactTextString(m) } +func (*MerklePath) ProtoMessage() {} func (*MerklePath) Descriptor() ([]byte, []int) { return fileDescriptor_7921d88972a41469, []int{2} } @@ -217,27 +218,27 @@ func init() { } var fileDescriptor_7921d88972a41469 = []byte{ - // 316 bytes of a gzipped FileDescriptorProto - 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x4c, 0x91, 0x31, 0x4f, 0xc3, 0x30, - 0x10, 0x85, 0x13, 0x51, 0x15, 0xea, 0x76, 0xb2, 0x10, 0x82, 0x0a, 0x4c, 0xd5, 0x01, 0xba, 0xc4, - 0x56, 0xdb, 0x01, 0x54, 0x31, 0xc1, 0xc0, 0x84, 0x54, 0x65, 0x60, 0x60, 0x41, 0x89, 0x71, 0x13, - 0xab, 0x0d, 0x17, 0xc5, 0x6e, 0x44, 0xff, 0x01, 0x23, 0x23, 0x23, 0x3f, 0x87, 0xb1, 0x23, 0x23, - 0x6a, 0xff, 0x08, 0xb2, 0xdd, 0xa0, 0x6c, 0x77, 0x7a, 0x9f, 0x9f, 0xfc, 0xde, 0xa1, 0x4b, 0x19, - 0x73, 0xc6, 0xa1, 0x10, 0x8c, 0x43, 0x96, 0x49, 0x9d, 0x89, 0x57, 0xcd, 0xca, 0x61, 0x6d, 0xa3, - 0x79, 0x01, 0x1a, 0xf0, 0x91, 0x8c, 0x39, 0x35, 0x20, 0xad, 0x49, 0xe5, 0xb0, 0x7b, 0x98, 0x40, - 0x02, 0x16, 0x61, 0x66, 0x72, 0x74, 0xf7, 0x94, 0x83, 0xca, 0x40, 0x31, 0xc9, 0xd5, 0x68, 0x6c, - 0xfc, 0xf2, 0x02, 0x60, 0xa6, 0x9c, 0xda, 0xbf, 0x40, 0xe8, 0x41, 0x14, 0xf3, 0x85, 0x08, 0x01, - 0x34, 0xc6, 0xa8, 0x91, 0x46, 0x2a, 0x3d, 0xf6, 0x7b, 0xfe, 0xa0, 0x13, 0xda, 0x79, 0xd2, 0x78, - 0xff, 0x3a, 0xf7, 0xfa, 0x01, 0xea, 0x38, 0x6e, 0x5a, 0x88, 0x99, 0x7c, 0xc3, 0x67, 0x08, 0xcd, - 0xc5, 0xea, 0x39, 0xb7, 0xdb, 0x8e, 0x6f, 0xcd, 0xc5, 0xca, 0xc9, 0xfd, 0xa0, 0xb2, 0x9d, 0x46, - 0x3a, 0xc5, 0x27, 0xe8, 0xc0, 0xc2, 0x91, 0x36, 0xd6, 0x7b, 0x83, 0x56, 0xb8, 0x6f, 0xd0, 0x48, - 0xa7, 0x93, 0xc6, 0xa7, 0x71, 0xbf, 0x47, 0xed, 0xca, 0x1d, 0x60, 0x86, 0xaf, 0x51, 0xd3, 0x7d, - 0xd2, 0xd2, 0xed, 0x51, 0x8f, 0xba, 0x0c, 0xd4, 0x66, 0xa0, 0xe5, 0x90, 0xde, 0xfd, 0x07, 0xb7, - 0x2f, 0xc2, 0x1d, 0x7f, 0xfb, 0xf8, 0xbd, 0x21, 0xfe, 0x7a, 0x43, 0xfc, 0xdf, 0x0d, 0xf1, 0x3f, - 0xb6, 0xc4, 0x5b, 0x6f, 0x89, 0xf7, 0xb3, 0x25, 0xde, 0xd3, 0x4d, 0x22, 0x75, 0xba, 0x8c, 0x4d, - 0x65, 0xac, 0x6a, 0x24, 0xe6, 0x41, 0x02, 0xac, 0xbc, 0x62, 0x19, 0xbc, 0x2c, 0x17, 0x42, 0xb9, - 0xf6, 0x47, 0xe3, 0xa0, 0x76, 0x00, 0xbd, 0xca, 0x85, 0x8a, 0x9b, 0xb6, 0xad, 0xf1, 0x5f, 0x00, - 0x00, 0x00, 0xff, 0xff, 0x74, 0x07, 0xaf, 0xf3, 0xa4, 0x01, 0x00, 0x00, + // 312 bytes of a gzipped FileDescriptorProto + 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0x4c, 0x90, 0xb1, 0x4e, 0xeb, 0x30, + 0x14, 0x86, 0x13, 0xdd, 0xaa, 0x97, 0xba, 0x9d, 0x2c, 0x84, 0xa0, 0x02, 0x53, 0x65, 0xa0, 0x5d, + 0x6a, 0xab, 0xed, 0x00, 0x42, 0x4c, 0x30, 0x30, 0x21, 0x55, 0x19, 0x18, 0x58, 0x50, 0x62, 0xdc, + 0xc4, 0x6a, 0xc3, 0x89, 0x62, 0x37, 0xa2, 0x6f, 0xc0, 0xc8, 0x23, 0xf0, 0x38, 0x8c, 0x1d, 0x19, + 0x51, 0xf3, 0x22, 0xc8, 0x76, 0x83, 0xb2, 0x9d, 0xa3, 0xf3, 0xf9, 0xd7, 0xef, 0x0f, 0x0d, 0x65, + 0xcc, 0x19, 0x87, 0x42, 0x30, 0x0e, 0x59, 0x26, 0x75, 0x26, 0x5e, 0x35, 0x2b, 0x27, 0x8d, 0x8d, + 0xe6, 0x05, 0x68, 0xc0, 0x47, 0x32, 0xe6, 0xd4, 0x80, 0xb4, 0x71, 0x2a, 0x27, 0xfd, 0xc3, 0x04, + 0x12, 0xb0, 0x08, 0x33, 0x93, 0xa3, 0xfb, 0xa7, 0x1c, 0x54, 0x06, 0x8a, 0x49, 0xae, 0xa6, 0x33, + 0x93, 0x97, 0x17, 0x00, 0x0b, 0xe5, 0xae, 0xc1, 0x05, 0x42, 0x0f, 0xa2, 0x58, 0xae, 0x44, 0x08, + 0xa0, 0x31, 0x46, 0xad, 0x34, 0x52, 0xe9, 0xb1, 0x3f, 0xf0, 0x47, 0xbd, 0xd0, 0xce, 0xd7, 0xad, + 0xf7, 0xcf, 0x73, 0x2f, 0x18, 0xa3, 0x9e, 0xe3, 0xe6, 0x85, 0x58, 0xc8, 0x37, 0x7c, 0x86, 0xd0, + 0x52, 0x6c, 0x9e, 0x73, 0xbb, 0xed, 0xf9, 0xce, 0x52, 0x6c, 0xdc, 0x39, 0x18, 0xd6, 0xb1, 0xf3, + 0x48, 0xa7, 0xf8, 0x04, 0x1d, 0x58, 0x38, 0xd2, 0x26, 0xfa, 0xdf, 0xa8, 0x13, 0xfe, 0x37, 0x68, + 0xa4, 0xd3, 0xe0, 0x1e, 0x75, 0xeb, 0x5c, 0x80, 0x05, 0xbe, 0x42, 0x6d, 0x57, 0xcf, 0x72, 0xdd, + 0xe9, 0x80, 0xba, 0xf6, 0xd4, 0xb6, 0xa7, 0xe5, 0x84, 0xde, 0xfd, 0x7d, 0xd9, 0xbe, 0x08, 0xf7, + 0xfc, 0xed, 0xe3, 0xd7, 0x8e, 0xf8, 0xdb, 0x1d, 0xf1, 0x7f, 0x76, 0xc4, 0xff, 0xa8, 0x88, 0xb7, + 0xad, 0x88, 0xf7, 0x5d, 0x11, 0xef, 0xe9, 0x26, 0x91, 0x3a, 0x5d, 0xc7, 0x46, 0x16, 0xab, 0x5d, + 0xc4, 0x7c, 0x9c, 0x00, 0x2b, 0x2f, 0x59, 0x06, 0x2f, 0xeb, 0x95, 0x50, 0xce, 0xfb, 0x74, 0x36, + 0x6e, 0xa8, 0xd7, 0x9b, 0x5c, 0xa8, 0xb8, 0x6d, 0x3d, 0xcd, 0x7e, 0x03, 0x00, 0x00, 0xff, 0xff, + 0xea, 0xa4, 0xc2, 0x39, 0x9e, 0x01, 0x00, 0x00, } func (m *MerkleRoot) Marshal() (dAtA []byte, err error) { diff --git a/modules/core/23-commitment/types/merkle.go b/modules/core/23-commitment/types/merkle.go index 4367a27ccfe..e74c645b5e0 100644 --- a/modules/core/23-commitment/types/merkle.go +++ b/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)) diff --git a/modules/core/23-commitment/types/merkle_test.go b/modules/core/23-commitment/types/merkle_test.go index 5b80c7ce266..baf7fa91f1c 100644 --- a/modules/core/23-commitment/types/merkle_test.go +++ b/modules/core/23-commitment/types/merkle_test.go @@ -148,24 +148,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, "leng key path incorrect") } diff --git a/modules/core/exported/commitment.go b/modules/core/exported/commitment.go index 9a8b47f0151..7886b4d323a 100644 --- a/modules/core/exported/commitment.go +++ b/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 } diff --git a/modules/light-clients/06-solomachine/client_state_test.go b/modules/light-clients/06-solomachine/client_state_test.go index 418c8c6f9c8..eea09e3935e 100644 --- a/modules/light-clients/06-solomachine/client_state_test.go +++ b/modules/light-clients/06-solomachine/client_state_test.go @@ -605,8 +605,8 @@ 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 + merklePath := commitmenttypes.NewMerklePath("solomachine") + key, err := merklePath.GetKey(0) // use index 0 because there is no key for IBC store suite.Require().NoError(err) signBytesNilData := solomachine.SignBytes{ Sequence: sm.GetHeight().GetRevisionHeight(), diff --git a/proto/ibc/core/commitment/v1/commitment.proto b/proto/ibc/core/commitment/v1/commitment.proto index f44fb75379a..3fd3a2e036d 100644 --- a/proto/ibc/core/commitment/v1/commitment.proto +++ b/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; } diff --git a/testing/solomachine.go b/testing/solomachine.go index 8d542eae5fe..39d4f02d83e 100644 --- a/testing/solomachine.go +++ b/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,8 +516,8 @@ 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 + merklePath := commitmenttypes.NewMerklePath(host.FullClientStatePath(clientIDSolomachine)) + key, err := merklePath.GetKey(0) // use index 0 because there is no key for IBC store require.NoError(solo.t, err) signBytes := &solomachine.SignBytes{ Sequence: solo.Sequence, @@ -536,8 +536,8 @@ 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 + merklePath := commitmenttypes.NewMerklePath(host.FullConsensusStatePath(clientIDSolomachine, consensusHeight)) + key, err := merklePath.GetKey(0) // use index 0 because there is no key for IBC store require.NoError(solo.t, err) signBytes := &solomachine.SignBytes{ Sequence: solo.Sequence, @@ -559,8 +559,8 @@ 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 + merklePath := commitmenttypes.NewMerklePath(host.ConnectionPath(connectionIDSolomachine)) + key, err := merklePath.GetKey(0) // use index 0 because there is no key for IBC store require.NoError(solo.t, err) signBytes := &solomachine.SignBytes{ Sequence: solo.Sequence, @@ -582,8 +582,8 @@ 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 + merklePath := commitmenttypes.NewMerklePath(host.ChannelPath(portID, channelIDSolomachine)) + key, err := merklePath.GetKey(0) // use index 0 because there is no key for IBC store require.NoError(solo.t, err) signBytes := &solomachine.SignBytes{ Sequence: solo.Sequence, @@ -605,8 +605,8 @@ 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 + merklePath := commitmenttypes.NewMerklePath(host.ChannelPath(portID, channelIDSolomachine)) + key, err := merklePath.GetKey(0) // use index 0 because there is no key for IBC store require.NoError(solo.t, err) signBytes := &solomachine.SignBytes{ Sequence: solo.Sequence, @@ -623,8 +623,8 @@ 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 + merklePath := commitmenttypes.NewMerklePath(host.PacketCommitmentPath(packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence())) + key, err := merklePath.GetKey(0) // use index 0 because there is no key for IBC store require.NoError(solo.t, err) signBytes := &solomachine.SignBytes{ Sequence: solo.Sequence, @@ -641,8 +641,8 @@ 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 + merklePath := commitmenttypes.NewMerklePath(host.PacketAcknowledgementPath(packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence())) + key, err := merklePath.GetKey(0) // use index 0 because there is no key for IBC store require.NoError(solo.t, err) signBytes := &solomachine.SignBytes{ Sequence: solo.Sequence, @@ -657,8 +657,8 @@ 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 + merklePath := commitmenttypes.NewMerklePath(host.PacketReceiptPath(packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence())) + key, err := merklePath.GetKey(0) // use index 0 because there is no key for IBC store require.NoError(solo.t, err) signBytes := &solomachine.SignBytes{ Sequence: solo.Sequence, From 57150bdaee427d1d8d113f5d2f5fc3e1bf6b6512 Mon Sep 17 00:00:00 2001 From: Carlos Rodriguez Date: Mon, 28 Aug 2023 19:49:03 +0200 Subject: [PATCH 2/4] address review feedback --- .../06-solomachine/client_state_test.go | 16 +++---- testing/solomachine.go | 48 +++++++------------ 2 files changed, 23 insertions(+), 41 deletions(-) diff --git a/modules/light-clients/06-solomachine/client_state_test.go b/modules/light-clients/06-solomachine/client_state_test.go index eea09e3935e..ff3cb312352 100644 --- a/modules/light-clients/06-solomachine/client_state_test.go +++ b/modules/light-clients/06-solomachine/client_state_test.go @@ -175,7 +175,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 @@ -470,7 +470,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, @@ -605,14 +605,12 @@ func (suite *SoloMachineTestSuite) TestVerifyMembership() { func (suite *SoloMachineTestSuite) TestSignBytesMarshalling() { sm := suite.solomachine - merklePath := commitmenttypes.NewMerklePath("solomachine") - key, err := merklePath.GetKey(0) // use index 0 because there is no key for 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, } @@ -620,7 +618,7 @@ func (suite *SoloMachineTestSuite) TestSignBytesMarshalling() { Sequence: sm.GetHeight().GetRevisionHeight(), Timestamp: sm.Time, Diversifier: sm.Diversifier, - Path: key, + Path: path, Data: []byte{}, } @@ -658,7 +656,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 @@ -696,7 +694,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, diff --git a/testing/solomachine.go b/testing/solomachine.go index 39d4f02d83e..8355ad28de0 100644 --- a/testing/solomachine.go +++ b/testing/solomachine.go @@ -516,14 +516,12 @@ func (solo *Solomachine) GenerateClientStateProof(clientState exported.ClientSta data, err := clienttypes.MarshalClientState(solo.cdc, clientState) require.NoError(solo.t, err) - merklePath := commitmenttypes.NewMerklePath(host.FullClientStatePath(clientIDSolomachine)) - key, err := merklePath.GetKey(0) // use index 0 because there is no key for IBC store - require.NoError(solo.t, err) + path := []byte(host.FullClientStatePath(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 := commitmenttypes.NewMerklePath(host.FullConsensusStatePath(clientIDSolomachine, consensusHeight)) - key, err := merklePath.GetKey(0) // use index 0 because there is no key for IBC store - require.NoError(solo.t, err) + path := []byte(host.FullConsensusStatePath(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 := commitmenttypes.NewMerklePath(host.ConnectionPath(connectionIDSolomachine)) - key, err := merklePath.GetKey(0) // use index 0 because there is no key for IBC store - require.NoError(solo.t, err) + path := []byte(host.ConnectionPath(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 := commitmenttypes.NewMerklePath(host.ChannelPath(portID, channelIDSolomachine)) - key, err := merklePath.GetKey(0) // use index 0 because there is no key for IBC store - require.NoError(solo.t, err) + path := []byte(host.ChannelPath(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 := commitmenttypes.NewMerklePath(host.ChannelPath(portID, channelIDSolomachine)) - key, err := merklePath.GetKey(0) // use index 0 because there is no key for IBC store - require.NoError(solo.t, err) + path := []byte(host.ChannelPath(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 := commitmenttypes.NewMerklePath(host.PacketCommitmentPath(packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence())) - key, err := merklePath.GetKey(0) // use index 0 because there is no key for IBC store - require.NoError(solo.t, err) + path := []byte(host.PacketCommitmentPath(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 := commitmenttypes.NewMerklePath(host.PacketAcknowledgementPath(packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence())) - key, err := merklePath.GetKey(0) // use index 0 because there is no key for IBC store - require.NoError(solo.t, err) + path := []byte(host.PacketAcknowledgementPath(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 := commitmenttypes.NewMerklePath(host.PacketReceiptPath(packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence())) - key, err := merklePath.GetKey(0) // use index 0 because there is no key for IBC store - require.NoError(solo.t, err) + path := []byte(host.PacketReceiptPath(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) From bc03ca22afc9ac35754aa90239af659d6f089cbe Mon Sep 17 00:00:00 2001 From: Carlos Rodriguez Date: Tue, 19 Sep 2023 13:27:48 +0200 Subject: [PATCH 3/4] Update modules/core/23-commitment/types/merkle_test.go Co-authored-by: Damian Nolan --- modules/core/23-commitment/types/merkle_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/core/23-commitment/types/merkle_test.go b/modules/core/23-commitment/types/merkle_test.go index d871edf2bc5..74ea939abb9 100644 --- a/modules/core/23-commitment/types/merkle_test.go +++ b/modules/core/23-commitment/types/merkle_test.go @@ -150,5 +150,5 @@ func TestApplyPrefix(t *testing.T) { prefixedPath, err := types.ApplyPrefix(prefix, path) require.NoError(t, err, "valid prefix returns error") - require.Len(t, prefixedPath.GetKeyPath(), 2, "leng key path incorrect") + require.Len(t, prefixedPath.GetKeyPath(), 2, "unexpected key path length") } From 3c87b0e05a8848e291c5675740ba7e23baae07ed Mon Sep 17 00:00:00 2001 From: Carlos Rodriguez Date: Tue, 19 Sep 2023 13:37:06 +0200 Subject: [PATCH 4/4] review comment --- testing/solomachine.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/testing/solomachine.go b/testing/solomachine.go index 98f07ce9ac8..2293f285bec 100644 --- a/testing/solomachine.go +++ b/testing/solomachine.go @@ -516,7 +516,7 @@ func (solo *Solomachine) GenerateClientStateProof(clientState exported.ClientSta data, err := clienttypes.MarshalClientState(solo.cdc, clientState) require.NoError(solo.t, err) - path := []byte(host.FullClientStatePath(clientIDSolomachine)) + path := host.FullClientStateKey(clientIDSolomachine) signBytes := &solomachine.SignBytes{ Sequence: solo.Sequence, Timestamp: solo.Time, @@ -534,7 +534,7 @@ func (solo *Solomachine) GenerateConsensusStateProof(consensusState exported.Con data, err := clienttypes.MarshalConsensusState(solo.cdc, consensusState) require.NoError(solo.t, err) - path := []byte(host.FullConsensusStatePath(clientIDSolomachine, consensusHeight)) + path := host.FullConsensusStateKey(clientIDSolomachine, consensusHeight) signBytes := &solomachine.SignBytes{ Sequence: solo.Sequence, Timestamp: solo.Time, @@ -555,7 +555,7 @@ func (solo *Solomachine) GenerateConnOpenTryProof(counterpartyClientID, counterp data, err := solo.cdc.Marshal(&connection) require.NoError(solo.t, err) - path := []byte(host.ConnectionPath(connectionIDSolomachine)) + path := host.ConnectionKey(connectionIDSolomachine) signBytes := &solomachine.SignBytes{ Sequence: solo.Sequence, Timestamp: solo.Time, @@ -576,7 +576,7 @@ func (solo *Solomachine) GenerateChanOpenTryProof(portID, version, counterpartyC data, err := solo.cdc.Marshal(&channel) require.NoError(solo.t, err) - path := []byte(host.ChannelPath(portID, channelIDSolomachine)) + path := host.ChannelKey(portID, channelIDSolomachine) signBytes := &solomachine.SignBytes{ Sequence: solo.Sequence, Timestamp: solo.Time, @@ -597,7 +597,7 @@ func (solo *Solomachine) GenerateChanClosedProof(portID, version, counterpartyCh data, err := solo.cdc.Marshal(&channel) require.NoError(solo.t, err) - path := []byte(host.ChannelPath(portID, channelIDSolomachine)) + path := host.ChannelKey(portID, channelIDSolomachine) signBytes := &solomachine.SignBytes{ Sequence: solo.Sequence, Timestamp: solo.Time, @@ -613,7 +613,7 @@ func (solo *Solomachine) GenerateChanClosedProof(portID, version, counterpartyCh func (solo *Solomachine) GenerateCommitmentProof(packet channeltypes.Packet) []byte { commitment := channeltypes.CommitPacket(solo.cdc, packet) - path := []byte(host.PacketCommitmentPath(packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence())) + path := host.PacketCommitmentKey(packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence()) signBytes := &solomachine.SignBytes{ Sequence: solo.Sequence, Timestamp: solo.Time, @@ -629,7 +629,7 @@ func (solo *Solomachine) GenerateCommitmentProof(packet channeltypes.Packet) []b func (solo *Solomachine) GenerateAcknowledgementProof(packet channeltypes.Packet) []byte { transferAck := channeltypes.NewResultAcknowledgement([]byte{byte(1)}).Acknowledgement() - path := []byte(host.PacketAcknowledgementPath(packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence())) + path := host.PacketAcknowledgementKey(packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence()) signBytes := &solomachine.SignBytes{ Sequence: solo.Sequence, Timestamp: solo.Time, @@ -643,7 +643,7 @@ 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 { - path := []byte(host.PacketReceiptPath(packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence())) + path := host.PacketReceiptKey(packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence()) signBytes := &solomachine.SignBytes{ Sequence: solo.Sequence, Timestamp: solo.Time,