diff --git a/CHANGELOG.md b/CHANGELOG.md index 7cb9a282b02..94f47163172 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,10 +47,11 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### Improvements * [\#4187](https://github.com/cosmos/ibc-go/pull/4187) Adds function 'WithICS4Wrapper' to keepers to allow to set the middleware after the keeper's creation. +* (light-clients/06-solomachine) [\#4429](https://github.com/cosmos/ibc-go/pull/4429) Remove IBC key from path of bytes signed by solomachine and not escape the path. ### Features -* [\#3796](https://github.com/cosmos/ibc-go/pull/3796) Adds support for json tx encoding for interchain accounts. +* (apps/27-interchain-accounts) [\#3796](https://github.com/cosmos/ibc-go/pull/3796) Adds support for json tx encoding for interchain accounts. * [\#4188](https://github.com/cosmos/ibc-go/pull/4188) Adds optional `PacketDataUnmarshaler` interface that allows a middleware to request the packet data to be unmarshaled by the base application. * [\#4199](https://github.com/cosmos/ibc-go/pull/4199) Adds optional `PacketDataProvider` interface for retrieving custom packet data stored on behalf of another application. * [\#4200](https://github.com/cosmos/ibc-go/pull/4200) Adds optional `PacketData` interface which application's packet data may implement. diff --git a/modules/core/23-commitment/types/merkle.go b/modules/core/23-commitment/types/merkle.go index 5d263ce1777..5e8904435a8 100644 --- a/modules/core/23-commitment/types/merkle.go +++ b/modules/core/23-commitment/types/merkle.go @@ -79,6 +79,12 @@ func NewMerklePath(keyPath ...string) MerklePath { // 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 { @@ -91,6 +97,12 @@ func (mp MerklePath) String() 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 { @@ -100,16 +112,11 @@ func (mp MerklePath) Pretty() string { } // 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)) } - key, err := url.PathUnescape(mp.KeyPath[i]) - if err != nil { - return nil, err - } - return []byte(key), nil + return []byte(mp.KeyPath[i]), nil } // Empty returns true if the path is empty diff --git a/modules/core/exported/commitment.go b/modules/core/exported/commitment.go index 904910e44e9..9a8b47f0151 100644 --- a/modules/core/exported/commitment.go +++ b/modules/core/exported/commitment.go @@ -28,7 +28,7 @@ type Prefix interface { // Path implements spec:CommitmentPath. // A path is the additional information provided to the verification function. type Path interface { - String() string + String() string // deprecated Empty() bool } diff --git a/modules/light-clients/06-solomachine/client_state.go b/modules/light-clients/06-solomachine/client_state.go index 24346407da7..d653455ac1e 100644 --- a/modules/light-clients/06-solomachine/client_state.go +++ b/modules/light-clients/06-solomachine/client_state.go @@ -125,15 +125,21 @@ func (cs *ClientState) VerifyMembership( return sdkerrors.Wrapf(sdkerrors.ErrInvalidType, "expected %T, got %T", commitmenttypes.MerklePath{}, path) } - if merklePath.Empty() { - return sdkerrors.Wrap(commitmenttypes.ErrInvalidProof, "path is empty") + if len(merklePath.GetKeyPath()) != 2 { + return sdkerrors.Wrapf(host.ErrInvalidPath, "path must be of length 2: %s", merklePath.GetKeyPath()) + } + + // 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 + key, err := merklePath.GetKey(1) + if err != nil { + return sdkerrors.Wrapf(host.ErrInvalidPath, "key not found at index 1: %v", err) } signBytes := &SignBytes{ Sequence: sequence, Timestamp: timestamp, Diversifier: cs.ConsensusState.Diversifier, - Path: []byte(merklePath.String()), + Path: key, Data: value, } @@ -175,11 +181,21 @@ func (cs *ClientState) VerifyNonMembership( return sdkerrors.Wrapf(sdkerrors.ErrInvalidType, "expected %T, got %T", commitmenttypes.MerklePath{}, path) } + if len(merklePath.GetKeyPath()) != 2 { + return sdkerrors.Wrapf(host.ErrInvalidPath, "path must be of length 2: %s", merklePath.GetKeyPath()) + } + + // 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 + key, err := merklePath.GetKey(1) + if err != nil { + return sdkerrors.Wrapf(host.ErrInvalidPath, "key not found at index 1: %v", err) + } + signBytes := &SignBytes{ Sequence: sequence, Timestamp: timestamp, Diversifier: cs.ConsensusState.Diversifier, - Path: []byte(merklePath.String()), + Path: key, Data: nil, } diff --git a/modules/light-clients/06-solomachine/client_state_test.go b/modules/light-clients/06-solomachine/client_state_test.go index 33888252e4e..418c8c6f9c8 100644 --- a/modules/light-clients/06-solomachine/client_state_test.go +++ b/modules/light-clients/06-solomachine/client_state_test.go @@ -176,11 +176,15 @@ func (suite *SoloMachineTestSuite) TestVerifyMembership() { suite.Require().NoError(err) path = suite.solomachine.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 + suite.Require().NoError(err) signBytes = solomachine.SignBytes{ Sequence: sm.GetHeight().GetRevisionHeight(), Timestamp: sm.Time, Diversifier: sm.Diversifier, - Path: []byte(path.String()), + Path: key, Data: clientStateBz, } @@ -208,11 +212,15 @@ func (suite *SoloMachineTestSuite) TestVerifyMembership() { suite.Require().NoError(err) path = sm.GetConsensusStatePath(counterpartyClientIdentifier, clienttypes.NewHeight(0, 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 + suite.Require().NoError(err) signBytes = solomachine.SignBytes{ Sequence: sm.Sequence, Timestamp: sm.Time, Diversifier: sm.Diversifier, - Path: []byte(path.String()), + Path: key, Data: consensusStateBz, } @@ -243,11 +251,15 @@ func (suite *SoloMachineTestSuite) TestVerifyMembership() { suite.Require().NoError(err) path = sm.GetConnectionStatePath(ibctesting.FirstConnectionID) + 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 + suite.Require().NoError(err) signBytes = solomachine.SignBytes{ Sequence: sm.Sequence, Timestamp: sm.Time, Diversifier: sm.Diversifier, - Path: []byte(path.String()), + Path: key, Data: connectionEndBz, } @@ -279,11 +291,15 @@ func (suite *SoloMachineTestSuite) TestVerifyMembership() { suite.Require().NoError(err) path = sm.GetChannelStatePath(ibctesting.MockPort, ibctesting.FirstChannelID) + 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 + suite.Require().NoError(err) signBytes = solomachine.SignBytes{ Sequence: sm.Sequence, Timestamp: sm.Time, Diversifier: sm.Diversifier, - Path: []byte(path.String()), + Path: key, Data: channelEndBz, } @@ -312,11 +328,15 @@ func (suite *SoloMachineTestSuite) TestVerifyMembership() { suite.Require().True(found) path = sm.GetNextSequenceRecvPath(ibctesting.MockPort, ibctesting.FirstChannelID) + 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 + suite.Require().NoError(err) signBytes = solomachine.SignBytes{ Sequence: sm.Sequence, Timestamp: sm.Time, Diversifier: sm.Diversifier, - Path: []byte(path.String()), + Path: key, Data: sdk.Uint64ToBigEndian(nextSeqRecv), } @@ -351,11 +371,15 @@ func (suite *SoloMachineTestSuite) TestVerifyMembership() { commitmentBz := channeltypes.CommitPacket(suite.chainA.Codec, packet) path = sm.GetPacketCommitmentPath(packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence()) + 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 + suite.Require().NoError(err) signBytes = solomachine.SignBytes{ Sequence: sm.Sequence, Timestamp: sm.Time, Diversifier: sm.Diversifier, - Path: []byte(path.String()), + Path: key, Data: commitmentBz, } @@ -378,11 +402,15 @@ func (suite *SoloMachineTestSuite) TestVerifyMembership() { "success: packet acknowledgement verification", func() { path = sm.GetPacketAcknowledgementPath(ibctesting.MockPort, ibctesting.FirstChannelID, 1) + merklePath, ok := path.(commitmenttypes.MerklePath) + suite.Require().True(ok) + 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 + suite.Require().NoError(err) signBytes = solomachine.SignBytes{ Sequence: sm.Sequence, Timestamp: sm.Time, Diversifier: sm.Diversifier, - Path: []byte(path.String()), + Path: key, Data: ibctesting.MockAcknowledgement, } @@ -405,11 +433,15 @@ func (suite *SoloMachineTestSuite) TestVerifyMembership() { "success: packet receipt verification", func() { 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 + suite.Require().NoError(err) signBytes = solomachine.SignBytes{ Sequence: sm.Sequence, Timestamp: sm.Time, Diversifier: sm.Diversifier, - Path: []byte(path.String()), + Path: key, Data: []byte{byte(1)}, // packet receipt is stored as a single byte } @@ -429,7 +461,7 @@ func (suite *SoloMachineTestSuite) TestVerifyMembership() { true, }, { - "invalid path type", + "invalid path type - empty", func() { path = ibcmock.KeyPath{} }, @@ -521,11 +553,15 @@ func (suite *SoloMachineTestSuite) TestVerifyMembership() { clientState = sm.ClientState() path = commitmenttypes.NewMerklePath("ibc", "solomachine") + 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 + suite.Require().NoError(err) signBytes = solomachine.SignBytes{ Sequence: sm.GetHeight().GetRevisionHeight(), Timestamp: sm.Time, Diversifier: sm.Diversifier, - Path: []byte(path.String()), + Path: key, Data: []byte("solomachine"), } @@ -570,11 +606,13 @@ 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) signBytesNilData := solomachine.SignBytes{ Sequence: sm.GetHeight().GetRevisionHeight(), Timestamp: sm.Time, Diversifier: sm.Diversifier, - Path: []byte(merklePath.String()), + Path: key, Data: nil, } @@ -582,7 +620,7 @@ func (suite *SoloMachineTestSuite) TestSignBytesMarshalling() { Sequence: sm.GetHeight().GetRevisionHeight(), Timestamp: sm.Time, Diversifier: sm.Diversifier, - Path: []byte(merklePath.String()), + Path: key, Data: []byte{}, } @@ -621,11 +659,15 @@ func (suite *SoloMachineTestSuite) TestVerifyNonMembership() { "success: packet receipt absence verification", func() { path = suite.solomachine.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 + suite.Require().NoError(err) signBytes = solomachine.SignBytes{ Sequence: sm.GetHeight().GetRevisionHeight(), Timestamp: sm.Time, Diversifier: sm.Diversifier, - Path: []byte(path.String()), + Path: key, Data: nil, } @@ -740,11 +782,15 @@ func (suite *SoloMachineTestSuite) TestVerifyNonMembership() { clientState = sm.ClientState() path = commitmenttypes.NewMerklePath("ibc", "solomachine") + 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 + suite.Require().NoError(err) signBytes = solomachine.SignBytes{ Sequence: sm.GetHeight().GetRevisionHeight(), Timestamp: sm.Time, Diversifier: sm.Diversifier, - Path: []byte(path.String()), + Path: key, Data: nil, } diff --git a/modules/light-clients/07-tendermint/upgrade.go b/modules/light-clients/07-tendermint/upgrade.go index 5e137dad0c5..3f482fa2fe3 100644 --- a/modules/light-clients/07-tendermint/upgrade.go +++ b/modules/light-clients/07-tendermint/upgrade.go @@ -80,7 +80,7 @@ func (cs ClientState) VerifyUpgradeAndUpdateState( // construct clientState Merkle path upgradeClientPath := constructUpgradeClientMerklePath(cs.UpgradePath, lastHeight) if err := merkleProofClient.VerifyMembership(cs.ProofSpecs, consState.GetRoot(), upgradeClientPath, bz); err != nil { - return sdkerrors.Wrapf(err, "client state proof failed. Path: %s", upgradeClientPath.Pretty()) + return sdkerrors.Wrapf(err, "client state proof failed. Path: %s", upgradeClientPath.GetKeyPath()) } // Verify consensus state proof @@ -91,7 +91,7 @@ func (cs ClientState) VerifyUpgradeAndUpdateState( // construct consensus state Merkle path upgradeConsStatePath := constructUpgradeConsStateMerklePath(cs.UpgradePath, lastHeight) if err := merkleProofConsState.VerifyMembership(cs.ProofSpecs, consState.GetRoot(), upgradeConsStatePath, bz); err != nil { - return sdkerrors.Wrapf(err, "consensus state proof failed. Path: %s", upgradeConsStatePath.Pretty()) + return sdkerrors.Wrapf(err, "consensus state proof failed. Path: %s", upgradeConsStatePath.GetKeyPath()) } // Construct new client state and consensus state diff --git a/testing/solomachine.go b/testing/solomachine.go index e63b04ab51b..06e4e371de4 100644 --- a/testing/solomachine.go +++ b/testing/solomachine.go @@ -509,11 +509,14 @@ 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) signBytes := &solomachine.SignBytes{ Sequence: solo.Sequence, Timestamp: solo.Time, Diversifier: solo.Diversifier, - Path: []byte(solo.GetClientStatePath(clientIDSolomachine).String()), + Path: key, Data: data, } @@ -526,11 +529,14 @@ 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) signBytes := &solomachine.SignBytes{ Sequence: solo.Sequence, Timestamp: solo.Time, Diversifier: solo.Diversifier, - Path: []byte(solo.GetConsensusStatePath(clientIDSolomachine, consensusHeight).String()), + Path: key, Data: data, } @@ -546,11 +552,14 @@ 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) signBytes := &solomachine.SignBytes{ Sequence: solo.Sequence, Timestamp: solo.Time, Diversifier: solo.Diversifier, - Path: []byte(solo.GetConnectionStatePath(connectionIDSolomachine).String()), + Path: key, Data: data, } @@ -566,11 +575,14 @@ 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) signBytes := &solomachine.SignBytes{ Sequence: solo.Sequence, Timestamp: solo.Time, Diversifier: solo.Diversifier, - Path: []byte(solo.GetChannelStatePath(portID, channelIDSolomachine).String()), + Path: key, Data: data, } @@ -586,11 +598,14 @@ 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) signBytes := &solomachine.SignBytes{ Sequence: solo.Sequence, Timestamp: solo.Time, Diversifier: solo.Diversifier, - Path: []byte(solo.GetChannelStatePath(portID, channelIDSolomachine).String()), + Path: key, Data: data, } @@ -601,11 +616,14 @@ 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) signBytes := &solomachine.SignBytes{ Sequence: solo.Sequence, Timestamp: solo.Time, Diversifier: solo.Diversifier, - Path: []byte(solo.GetPacketCommitmentPath(packet.GetSourcePort(), packet.GetSourceChannel(), packet.GetSequence()).String()), + Path: key, Data: commitment, } @@ -615,11 +633,15 @@ func (solo *Solomachine) GenerateCommitmentProof(packet channeltypes.Packet) []b // GenerateAcknowledgementProof generates an acknowledgement proof. 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) signBytes := &solomachine.SignBytes{ Sequence: solo.Sequence, Timestamp: solo.Time, Diversifier: solo.Diversifier, - Path: []byte(solo.GetPacketAcknowledgementPath(packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence()).String()), + Path: key, Data: channeltypes.CommitAcknowledgement(transferAck), } @@ -628,11 +650,14 @@ 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) signBytes := &solomachine.SignBytes{ Sequence: solo.Sequence, Timestamp: solo.Time, Diversifier: solo.Diversifier, - Path: []byte(solo.GetPacketReceiptPath(packet.GetDestPort(), packet.GetDestChannel(), packet.GetSequence()).String()), + Path: key, Data: nil, } return solo.GenerateProof(signBytes)