From cd29fd642a3c948c68355e5f14e55405242aed3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?colin=20axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Mon, 16 Nov 2020 10:30:57 +0100 Subject: [PATCH] Fix connection path validation and remove unused PathValidator function (#7930) Co-authored-by: Federico Kunze <31522760+fedekunze@users.noreply.github.com> Co-authored-by: mergify[bot] <37929162+mergify[bot]@users.noreply.github.com> --- x/ibc/core/03-connection/types/connection_test.go | 13 +++++++------ x/ibc/core/03-connection/types/genesis.go | 6 +++--- x/ibc/core/03-connection/types/genesis_test.go | 9 ++++----- x/ibc/core/24-host/validate.go | 11 ----------- x/ibc/core/24-host/validate_test.go | 7 ++++++- x/ibc/core/genesis_test.go | 13 ++++++------- 6 files changed, 26 insertions(+), 33 deletions(-) diff --git a/x/ibc/core/03-connection/types/connection_test.go b/x/ibc/core/03-connection/types/connection_test.go index ff2aa08d4a66..93891cbea12e 100644 --- a/x/ibc/core/03-connection/types/connection_test.go +++ b/x/ibc/core/03-connection/types/connection_test.go @@ -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) { diff --git a/x/ibc/core/03-connection/types/genesis.go b/x/ibc/core/03-connection/types/genesis.go index 9b155a368b05..58b21678c299 100644 --- a/x/ibc/core/03-connection/types/genesis.go +++ b/x/ibc/core/03-connection/types/genesis.go @@ -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) } } } diff --git a/x/ibc/core/03-connection/types/genesis_test.go b/x/ibc/core/03-connection/types/genesis_test.go index 8080f49d5c80..6290a98a9ff4 100644 --- a/x/ibc/core/03-connection/types/genesis_test.go +++ b/x/ibc/core/03-connection/types/genesis_test.go @@ -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" ) @@ -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, @@ -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, @@ -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, @@ -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, diff --git a/x/ibc/core/24-host/validate.go b/x/ibc/core/24-host/validate.go index bbfafbee75d2..d589223f58da 100644 --- a/x/ibc/core/24-host/validate.go +++ b/x/ibc/core/24-host/validate.go @@ -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) -} diff --git a/x/ibc/core/24-host/validate_test.go b/x/ibc/core/24-host/validate_test.go index 41dd5167d9a4..40987bd15714 100644 --- a/x/ibc/core/24-host/validate_test.go +++ b/x/ibc/core/24-host/validate_test.go @@ -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) diff --git a/x/ibc/core/genesis_test.go b/x/ibc/core/genesis_test.go index a14669faaa9b..d435cf85667a 100644 --- a/x/ibc/core/genesis_test.go +++ b/x/ibc/core/genesis_test.go @@ -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" @@ -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), @@ -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( @@ -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()), @@ -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}), }, ), }, @@ -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), @@ -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(