Skip to content

Commit

Permalink
Fix connection path validation and remove unused PathValidator functi…
Browse files Browse the repository at this point in the history
…on (#7930)

Co-authored-by: Federico Kunze <[email protected]>
Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com>
  • Loading branch information
3 people authored Nov 16, 2020
1 parent 6b62d26 commit cd29fd6
Show file tree
Hide file tree
Showing 6 changed files with 26 additions and 33 deletions.
13 changes: 7 additions & 6 deletions x/ibc/core/03-connection/types/connection_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,12 +12,13 @@ import (
)

var (
chainID = "gaiamainnet"
connectionID = "connectionidone"
clientID = "clientidone"
connectionID2 = "connectionidtwo"
clientID2 = "clientidtwo"
clientHeight = clienttypes.NewHeight(0, 6)
chainID = "gaiamainnet"
connectionID = "connectionidone"
clientID = "clientidone"
connectionID2 = "connectionidtwo"
clientID2 = "clientidtwo"
invalidConnectionID = "(invalidConnectionID)"
clientHeight = clienttypes.NewHeight(0, 6)
)

func TestConnectionValidateBasic(t *testing.T) {
Expand Down
6 changes: 3 additions & 3 deletions x/ibc/core/03-connection/types/genesis.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,9 +45,9 @@ func (gs GenesisState) Validate() error {
if err := host.ClientIdentifierValidator(conPaths.ClientId); err != nil {
return fmt.Errorf("invalid client connection path %d: %w", i, err)
}
for _, path := range conPaths.Paths {
if err := host.PathValidator(path); err != nil {
return fmt.Errorf("invalid client connection path %d: %w", i, err)
for _, connectionID := range conPaths.Paths {
if err := host.ConnectionIdentifierValidator(connectionID); err != nil {
return fmt.Errorf("invalid client connection ID (%s) in connection paths %d: %w", connectionID, i, err)
}
}
}
Expand Down
9 changes: 4 additions & 5 deletions x/ibc/core/03-connection/types/genesis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ import (

"github.com/cosmos/cosmos-sdk/x/ibc/core/03-connection/types"
commitmenttypes "github.com/cosmos/cosmos-sdk/x/ibc/core/23-commitment/types"
host "github.com/cosmos/cosmos-sdk/x/ibc/core/24-host"
ibctesting "github.com/cosmos/cosmos-sdk/x/ibc/testing"
)

Expand All @@ -30,7 +29,7 @@ func TestValidateGenesis(t *testing.T) {
types.NewIdentifiedConnection(connectionID, types.NewConnectionEnd(types.INIT, clientID, types.Counterparty{clientID2, connectionID2, commitmenttypes.NewMerklePrefix([]byte("prefix"))}, []*types.Version{ibctesting.ConnectionVersion})),
},
[]types.ConnectionPaths{
{clientID, []string{host.ConnectionPath(connectionID)}},
{clientID, []string{connectionID}},
},
),
expPass: true,
Expand All @@ -42,7 +41,7 @@ func TestValidateGenesis(t *testing.T) {
types.NewIdentifiedConnection(connectionID, types.NewConnectionEnd(types.INIT, "(CLIENTIDONE)", types.Counterparty{clientID, connectionID, commitmenttypes.NewMerklePrefix([]byte("prefix"))}, []*types.Version{ibctesting.ConnectionVersion})),
},
[]types.ConnectionPaths{
{clientID, []string{host.ConnectionPath(connectionID)}},
{clientID, []string{connectionID}},
},
),
expPass: false,
Expand All @@ -54,7 +53,7 @@ func TestValidateGenesis(t *testing.T) {
types.NewIdentifiedConnection(connectionID, types.NewConnectionEnd(types.INIT, clientID, types.Counterparty{clientID2, connectionID2, commitmenttypes.NewMerklePrefix([]byte("prefix"))}, []*types.Version{ibctesting.ConnectionVersion})),
},
[]types.ConnectionPaths{
{"(CLIENTIDONE)", []string{host.ConnectionPath(connectionID)}},
{"(CLIENTIDONE)", []string{connectionID}},
},
),
expPass: false,
Expand All @@ -66,7 +65,7 @@ func TestValidateGenesis(t *testing.T) {
types.NewIdentifiedConnection(connectionID, types.NewConnectionEnd(types.INIT, clientID, types.Counterparty{clientID2, connectionID2, commitmenttypes.NewMerklePrefix([]byte("prefix"))}, []*types.Version{ibctesting.ConnectionVersion})),
},
[]types.ConnectionPaths{
{clientID, []string{connectionID}},
{clientID, []string{invalidConnectionID}},
},
),
expPass: false,
Expand Down
11 changes: 0 additions & 11 deletions x/ibc/core/24-host/validate.go
Original file line number Diff line number Diff line change
Expand Up @@ -112,14 +112,3 @@ func NewPathValidator(idValidator ValidateFn) ValidateFn {
return nil
}
}

// PathValidator takes in path string and validates with default identifier rules:
// path consists of `/`-separated valid identifiers,
// each identifier is between 1-64 characters and contains only alphanumeric and some allowed
// special characters (see IsValidID).
func PathValidator(path string) error {
f := NewPathValidator(func(path string) error {
return nil
})
return f(path)
}
7 changes: 6 additions & 1 deletion x/ibc/core/24-host/validate_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,12 @@ func TestPathValidator(t *testing.T) {
}

for _, tc := range testCases {
err := PathValidator(tc.id)
f := NewPathValidator(func(path string) error {
return nil
})

err := f(tc.id)

if tc.expPass {
seps := strings.Count(tc.id, "/")
require.Equal(t, 1, seps)
Expand Down
13 changes: 6 additions & 7 deletions x/ibc/core/genesis_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import (
connectiontypes "github.com/cosmos/cosmos-sdk/x/ibc/core/03-connection/types"
channeltypes "github.com/cosmos/cosmos-sdk/x/ibc/core/04-channel/types"
commitmenttypes "github.com/cosmos/cosmos-sdk/x/ibc/core/23-commitment/types"
host "github.com/cosmos/cosmos-sdk/x/ibc/core/24-host"
"github.com/cosmos/cosmos-sdk/x/ibc/core/exported"
"github.com/cosmos/cosmos-sdk/x/ibc/core/types"
ibctmtypes "github.com/cosmos/cosmos-sdk/x/ibc/light-clients/07-tendermint/types"
Expand Down Expand Up @@ -77,7 +76,7 @@ func (suite *IBCTestSuite) TestValidateGenesis() {
ClientGenesis: clienttypes.NewGenesisState(
[]clienttypes.IdentifiedClientState{
clienttypes.NewIdentifiedClientState(
clientID, ibctmtypes.NewClientState(suite.chainA.ChainID, ibctmtypes.DefaultTrustLevel, ibctesting.TrustingPeriod, ibctesting.UnbondingPeriod, ibctesting.MaxClockDrift, clientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false),
clientID, ibctmtypes.NewClientState(suite.chainA.ChainID, ibctmtypes.DefaultTrustLevel, ibctesting.TrustingPeriod, ibctesting.UnbondingPeriod, ibctesting.MaxClockDrift, clientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false),
),
clienttypes.NewIdentifiedClientState(
exported.Localhost, localhosttypes.NewClientState("chaindID", clientHeight),
Expand All @@ -104,7 +103,7 @@ func (suite *IBCTestSuite) TestValidateGenesis() {
connectiontypes.NewIdentifiedConnection(connectionID, connectiontypes.NewConnectionEnd(connectiontypes.INIT, clientID, connectiontypes.NewCounterparty(clientID2, connectionID2, commitmenttypes.NewMerklePrefix([]byte("prefix"))), []*connectiontypes.Version{ibctesting.ConnectionVersion})),
},
[]connectiontypes.ConnectionPaths{
connectiontypes.NewConnectionPaths(clientID, []string{host.ConnectionPath(connectionID)}),
connectiontypes.NewConnectionPaths(clientID, []string{connectionID}),
},
),
ChannelGenesis: channeltypes.NewGenesisState(
Expand Down Expand Up @@ -144,7 +143,7 @@ func (suite *IBCTestSuite) TestValidateGenesis() {
ClientGenesis: clienttypes.NewGenesisState(
[]clienttypes.IdentifiedClientState{
clienttypes.NewIdentifiedClientState(
clientID, ibctmtypes.NewClientState(suite.chainA.ChainID, ibctmtypes.DefaultTrustLevel, ibctesting.TrustingPeriod, ibctesting.UnbondingPeriod, ibctesting.MaxClockDrift, clientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false),
clientID, ibctmtypes.NewClientState(suite.chainA.ChainID, ibctmtypes.DefaultTrustLevel, ibctesting.TrustingPeriod, ibctesting.UnbondingPeriod, ibctesting.MaxClockDrift, clientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false),
),
clienttypes.NewIdentifiedClientState(
exported.Localhost, localhosttypes.NewClientState("(chaindID)", clienttypes.ZeroHeight()),
Expand All @@ -167,7 +166,7 @@ func (suite *IBCTestSuite) TestValidateGenesis() {
connectiontypes.NewIdentifiedConnection(connectionID, connectiontypes.NewConnectionEnd(connectiontypes.INIT, "(CLIENTIDONE)", connectiontypes.NewCounterparty(clientID, connectionID2, commitmenttypes.NewMerklePrefix([]byte("prefix"))), []*connectiontypes.Version{connectiontypes.NewVersion("1.1", nil)})),
},
[]connectiontypes.ConnectionPaths{
connectiontypes.NewConnectionPaths(clientID, []string{host.ConnectionPath(connectionID)}),
connectiontypes.NewConnectionPaths(clientID, []string{connectionID}),
},
),
},
Expand Down Expand Up @@ -216,7 +215,7 @@ func (suite *IBCTestSuite) TestInitGenesis() {
ClientGenesis: clienttypes.NewGenesisState(
[]clienttypes.IdentifiedClientState{
clienttypes.NewIdentifiedClientState(
clientID, ibctmtypes.NewClientState(suite.chainA.ChainID, ibctmtypes.DefaultTrustLevel, ibctesting.TrustingPeriod, ibctesting.UnbondingPeriod, ibctesting.MaxClockDrift, clientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false),
clientID, ibctmtypes.NewClientState(suite.chainA.ChainID, ibctmtypes.DefaultTrustLevel, ibctesting.TrustingPeriod, ibctesting.UnbondingPeriod, ibctesting.MaxClockDrift, clientHeight, commitmenttypes.GetSDKSpecs(), ibctesting.UpgradePath, false, false),
),
clienttypes.NewIdentifiedClientState(
exported.Localhost, localhosttypes.NewClientState("chaindID", clientHeight),
Expand All @@ -243,7 +242,7 @@ func (suite *IBCTestSuite) TestInitGenesis() {
connectiontypes.NewIdentifiedConnection(connectionID, connectiontypes.NewConnectionEnd(connectiontypes.INIT, clientID, connectiontypes.NewCounterparty(clientID2, connectionID2, commitmenttypes.NewMerklePrefix([]byte("prefix"))), []*connectiontypes.Version{ibctesting.ConnectionVersion})),
},
[]connectiontypes.ConnectionPaths{
connectiontypes.NewConnectionPaths(clientID, []string{host.ConnectionPath(connectionID)}),
connectiontypes.NewConnectionPaths(clientID, []string{connectionID}),
},
),
ChannelGenesis: channeltypes.NewGenesisState(
Expand Down

0 comments on commit cd29fd6

Please sign in to comment.