Skip to content

Commit

Permalink
fix(statemachine)!: use key within IBC store without escaping charact…
Browse files Browse the repository at this point in the history 
…ers in solomachine (#4429)

(cherry picked from commit 98b0c99)

# Conflicts:
#	modules/light-clients/06-solomachine/client_state.go
#	modules/light-clients/07-tendermint/upgrade.go
  • Loading branch information
@crodriguezvega @mergify
crodriguezvega authored and mergify[bot] committed Aug 24, 2023
1 parent e3faf70 commit f164b9b
Show file tree
Hide file tree
Showing 6 changed files with 138 additions and 30 deletions.
18 changes: 13 additions & 5 deletions modules/core/23-commitment/types/merkle.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 {
Expand All @@ -105,11 +117,7 @@ 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
Expand Down
2 changes: 1 addition & 1 deletion modules/core/exported/commitment.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
25 changes: 23 additions & 2 deletions modules/light-clients/06-solomachine/client_state.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -125,15 +125,26 @@ func (cs *ClientState) VerifyMembership(
return sdkerrors.Wrapf(sdkerrors.ErrInvalidType, "expected %T, got %T", commitmenttypes.MerklePath{}, path)
}

<<<<<<< HEAD
if merklePath.Empty() {
return sdkerrors.Wrap(commitmenttypes.ErrInvalidProof, "path is empty")
=======
if len(merklePath.GetKeyPath()) != 2 {
return errorsmod.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 errorsmod.Wrapf(host.ErrInvalidPath, "key not found at index 1: %v", err)
>>>>>>> 98b0c992 (fix(statemachine)!: use key within IBC store without escaping characters in solomachine (#4429))
}

signBytes := &SignBytes{
Sequence: sequence,
Timestamp: timestamp,
Diversifier: cs.ConsensusState.Diversifier,
Path: []byte(merklePath.String()),
Path: key,
Data: value,
}

Expand Down Expand Up @@ -175,11 +186,21 @@ func (cs *ClientState) VerifyNonMembership(
return sdkerrors.Wrapf(sdkerrors.ErrInvalidType, "expected %T, got %T", commitmenttypes.MerklePath{}, path)
}

if len(merklePath.GetKeyPath()) != 2 {
return errorsmod.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 errorsmod.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,
}

Expand Down
74 changes: 60 additions & 14 deletions modules/light-clients/06-solomachine/client_state_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}

Expand Down Expand Up @@ -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,
}

Expand Down Expand Up @@ -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,
}

Expand Down Expand Up @@ -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,
}

Expand Down Expand Up @@ -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),
}

Expand Down Expand Up @@ -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,
}

Expand All @@ -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,
}

Expand All @@ -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
}

Expand All @@ -429,7 +461,7 @@ func (suite *SoloMachineTestSuite) TestVerifyMembership() {
true,
},
{
"invalid path type",
"invalid path type - empty",
func() {
path = ibcmock.KeyPath{}
},
Expand Down Expand Up @@ -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"),
}

Expand Down Expand Up @@ -570,19 +606,21 @@ 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,
}

signBytesEmptyArray := solomachine.SignBytes{
Sequence: sm.GetHeight().GetRevisionHeight(),
Timestamp: sm.Time,
Diversifier: sm.Diversifier,
Path: []byte(merklePath.String()),
Path: key,
Data: []byte{},
}

Expand Down Expand Up @@ -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,
}

Expand Down Expand Up @@ -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,
}

Expand Down
8 changes: 8 additions & 0 deletions modules/light-clients/07-tendermint/upgrade.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,11 @@ 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 {
<<<<<<< HEAD
return sdkerrors.Wrapf(err, "client state proof failed. Path: %s", upgradeClientPath.Pretty())
=======
return errorsmod.Wrapf(err, "client state proof failed. Path: %s", upgradeClientPath.GetKeyPath())
>>>>>>> 98b0c992 (fix(statemachine)!: use key within IBC store without escaping characters in solomachine (#4429))
}

// Verify consensus state proof
Expand All @@ -91,7 +95,11 @@ 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 {
<<<<<<< HEAD
return sdkerrors.Wrapf(err, "consensus state proof failed. Path: %s", upgradeConsStatePath.Pretty())
=======
return errorsmod.Wrapf(err, "consensus state proof failed. Path: %s", upgradeConsStatePath.GetKeyPath())
>>>>>>> 98b0c992 (fix(statemachine)!: use key within IBC store without escaping characters in solomachine (#4429))
}

// Construct new client state and consensus state
Expand Down
Loading

0 comments on commit f164b9b

Please sign in to comment.