Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(statemachine)!: use path within IBC store without escaping characters in solomachine #4429

Merged
merged 5 commits into from
Aug 24, 2023
Merged

fix(statemachine)!: use path within IBC store without escaping characters in solomachine #4429

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
8a59dbb
use path in IBC store without escaping characters for solo machine
Aug 22, 2023
f946251
comment
Aug 22, 2023
de738a8
Merge branch 'main' into carlos/4128-wrong-url-encoded-path-used-in-s…
Aug 24, 2023
7424f5c
remove character unescaping in GetKey
Aug 24, 2023
6a74acb
Merge branch 'main' into carlos/4128-wrong-url-encoded-path-used-in-s…
Aug 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions modules/core/23-commitment/types/merkle.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a deprecated notice to these functions instead of removing them completely because I think removing them is API breaking and if I remove them in this PR the backport to v7.3.x will be more difficult. I will remove them in a follow up PR once this one gets merged.

// 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 @@ -93,6 +99,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 Down
2 changes: 1 addition & 1 deletion modules/core/exported/commitment.go
View file
Open in desktop
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
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we remove this function, I am not sure if this interface makes much sense anymore...

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed

Empty() bool
}

Expand Down
22 changes: 18 additions & 4 deletions modules/light-clients/06-solomachine/client_state.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,15 +128,20 @@ func (cs *ClientState) VerifyMembership(
return errorsmod.Wrapf(ibcerrors.ErrInvalidType, "expected %T, got %T", commitmenttypes.MerklePath{}, path)
}

if merklePath.Empty() {
return errorsmod.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())
}

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
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: value,
}

Expand Down Expand Up @@ -178,11 +183,20 @@ func (cs *ClientState) VerifyNonMembership(
return errorsmod.Wrapf(ibcerrors.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())
}

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
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
72 changes: 59 additions & 13 deletions modules/light-clients/06-solomachine/client_state_test.go
View file
Open in desktop
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) // 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) // 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) // 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) // 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) // 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) // 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) // 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 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) // 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) // 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) // 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) // 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
4 changes: 2 additions & 2 deletions modules/light-clients/07-tendermint/upgrade.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,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 errorsmod.Wrapf(err, "client state proof failed. Path: %s", upgradeClientPath.Pretty())
return errorsmod.Wrapf(err, "client state proof failed. Path: %s", upgradeClientPath.GetKeyPath())
}

// Verify consensus state proof
Expand All @@ -94,7 +94,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 errorsmod.Wrapf(err, "consensus state proof failed. Path: %s", upgradeConsStatePath.Pretty())
return errorsmod.Wrapf(err, "consensus state proof failed. Path: %s", upgradeConsStatePath.GetKeyPath())
}

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