From 51c6b865d3e8ee5e8aea053c33d9635d92ae4766 Mon Sep 17 00:00:00 2001 From: akhilkumarpilli Date: Thu, 28 Jan 2021 11:14:46 +0530 Subject: [PATCH 1/7] WIP: Add path validation in config --- cmd/config.go | 43 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) diff --git a/cmd/config.go b/cmd/config.go index 69b3f3480..9693484f8 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -417,3 +417,46 @@ func overWriteConfig(cmd *cobra.Command, cfg *Config) error { } return err } + +// ValidatePath checks that a path is valid +func (c *Config) ValidatePath(p *relayer.Path) (err error) { + if err = p.Src.ValidateFull(); err != nil { + return err + } + if p.Src.Version == "" { + return fmt.Errorf("source must specify a version") + } + if err = p.Dst.ValidateFull(); err != nil { + return err + } + if _, err = p.GetStrategy(); err != nil { + return err + } + if p.Src.Order != p.Dst.Order { + return fmt.Errorf("both sides must have same order ('ORDERED' or 'UNORDERED'), got src(%s) and dst(%s)", + p.Src.Order, p.Dst.Order) + } + return nil +} + +// ValidatePathEnd allow empty identifiers and check specific config if user provides identifier +// returns error for invalid identifiers +func (c *Config) ValidatePathEnd(pe *relayer.PathEnd) error { + if err := pe.ValidateBasic(); err != nil { + return err + } + if err := pe.Vclient(); err != nil { + return err + } + if pe.ConnectionID != "" { + if err := pe.Vconn(); err != nil { + return err + } + } + if pe.ChannelID != "" { + if err := pe.Vchan(); err != nil { + return err + } + } + return nil +} From 19c5142067fe57bbdec3700c3431613a3f08ff92 Mon Sep 17 00:00:00 2001 From: akhilkumarpilli Date: Fri, 29 Jan 2021 17:00:41 +0530 Subject: [PATCH 2/7] Implement bottom up validation --- cmd/config.go | 102 +++++++++++++++++++++++++++++++++++++++++++----- cmd/paths.go | 29 ++++++++++++++ relayer/path.go | 1 - 3 files changed, 121 insertions(+), 11 deletions(-) diff --git a/cmd/config.go b/cmd/config.go index 9693484f8..8519e81b1 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -218,15 +218,17 @@ func cfgFilesAdd(dir string) (cfg *Config, err error) { } pthName := strings.Split(f.Name(), ".")[0] + if err = config.ValidatePath(p); err != nil { + fmt.Printf("%s: %s\n", pth, err.Error()) + continue + } if err = cfg.AddPath(pthName, p); err != nil { fmt.Printf("%s: %s\n", pth, err.Error()) continue } - // TODO: Do bottom up validation // For now, we assume that all chain files must have same filename as chain-id // this is to ensure non-chain files (global config) does not get parsed into chain struct. - // Future work should implement bottom-up validation. if c.ChainID != pthName { fmt.Printf("Skipping non chain file: %s\n", f.Name()) continue @@ -420,13 +422,13 @@ func overWriteConfig(cmd *cobra.Command, cfg *Config) error { // ValidatePath checks that a path is valid func (c *Config) ValidatePath(p *relayer.Path) (err error) { - if err = p.Src.ValidateFull(); err != nil { + if err = c.ValidatePathEnd(p.Src); err != nil { return err } if p.Src.Version == "" { return fmt.Errorf("source must specify a version") } - if err = p.Dst.ValidateFull(); err != nil { + if err = c.ValidatePathEnd(p.Dst); err != nil { return err } if _, err = p.GetStrategy(); err != nil { @@ -439,24 +441,104 @@ func (c *Config) ValidatePath(p *relayer.Path) (err error) { return nil } -// ValidatePathEnd allow empty identifiers and check specific config if user provides identifier -// returns error for invalid identifiers +// ValidatePathEnd allow validates provided pathend and returns error for invalid identifiers func (c *Config) ValidatePathEnd(pe *relayer.PathEnd) error { if err := pe.ValidateBasic(); err != nil { return err } - if err := pe.Vclient(); err != nil { - return err + if pe.ClientID != "" { + if err := c.ValidateClient(pe); err != nil { + return err + } } if pe.ConnectionID != "" { - if err := pe.Vconn(); err != nil { + if err := c.ValidateConnection(pe); err != nil { return err } } if pe.ChannelID != "" { - if err := pe.Vchan(); err != nil { + if err := c.ValidateChannel(pe); err != nil { return err } } return nil } + +// ValidateClient validates client id in provided pathend +func (c *Config) ValidateClient(pe *relayer.PathEnd) error { + if err := pe.Vclient(); err != nil { + return err + } + + chain, err := c.Chains.Get(pe.ChainID) + if err != nil { + return err + } + + // TODO: add appropriate offset and limits, along with retries + clients, err := chain.QueryClients(0, 1000) + if err != nil { + return err + } + + for _, clientState := range clients.ClientStates { + if clientState.ClientId == pe.ClientID { + return nil + } + } + return fmt.Errorf("No client exists with given client id: %s", pe.ClientID) +} + +// ValidateConnection validates connection id in provided pathend +func (c *Config) ValidateConnection(pe *relayer.PathEnd) error { + if err := pe.Vconn(); err != nil { + return err + } + + chain, err := c.Chains.Get(pe.ChainID) + if err != nil { + return err + } + + // TODO: add appropriate offset and limits, along with retries + connections, err := chain.QueryConnections(0, 1000) + if err != nil { + return err + } + + for _, connection := range connections.Connections { + if connection.ClientId == pe.ClientID && connection.Id == pe.ConnectionID { + return nil + } + } + return fmt.Errorf("No connection exists with given connection id: %s", pe.ConnectionID) +} + +// ValidateChannel validates channel id in provided pathend +func (c *Config) ValidateChannel(pe *relayer.PathEnd) error { + if err := pe.Vchan(); err != nil { + return err + } + + chain, err := c.Chains.Get(pe.ChainID) + if err != nil { + return err + } + + // TODO: add appropriate offset and limits, along with retries + channels, err := chain.QueryChannels(0, 1000) + if err != nil { + return err + } + + for _, channel := range channels.Channels { + if channel.ChannelId == pe.ChannelID { + for _, connection := range channel.ConnectionHops { + if connection == pe.ConnectionID { + return nil + } + } + } + } + return fmt.Errorf("No channel exists with given channel id: %s", pe.ChannelID) +} diff --git a/cmd/paths.go b/cmd/paths.go index c2a142e25..1231dff18 100644 --- a/cmd/paths.go +++ b/cmd/paths.go @@ -155,6 +155,9 @@ $ %s pth gen ibc-0 ibc-1 demo-path --unordered false --version ics20-2`, appName path.GenSrcConnID() path.GenSrcChanID() path.GenDstChanID() + if err = config.ValidatePath(path); err != nil { + return err + } if err = config.Paths.Add(pth, path); err != nil { return err } @@ -166,6 +169,9 @@ $ %s pth gen ibc-0 ibc-1 demo-path --unordered false --version ics20-2`, appName path.GenSrcConnID() path.GenSrcChanID() path.GenDstChanID() + if err = config.ValidatePath(path); err != nil { + return err + } if err = config.Paths.Add(pth, path); err != nil { return err } @@ -177,6 +183,9 @@ $ %s pth gen ibc-0 ibc-1 demo-path --unordered false --version ics20-2`, appName path.GenSrcConnID() path.GenSrcChanID() path.GenDstChanID() + if err = config.ValidatePath(path); err != nil { + return err + } if err = config.Paths.Add(pth, path); err != nil { return err } @@ -225,6 +234,9 @@ $ %s pth gen ibc-0 ibc-1 demo-path --unordered false --version ics20-2`, appName path.GenDstConnID() path.GenSrcChanID() path.GenDstChanID() + if err = config.ValidatePath(path); err != nil { + return err + } if err = config.Paths.Add(pth, path); err != nil { return err } @@ -236,6 +248,9 @@ $ %s pth gen ibc-0 ibc-1 demo-path --unordered false --version ics20-2`, appName path.GenDstConnID() path.GenSrcChanID() path.GenDstChanID() + if err = config.ValidatePath(path); err != nil { + return err + } if err = config.Paths.Add(pth, path); err != nil { return err } @@ -288,6 +303,9 @@ $ %s pth gen ibc-0 ibc-1 demo-path --unordered false --version ics20-2`, appName path.GenSrcChanID() path.GenDstChanID() } + if err = config.ValidatePath(path); err != nil { + return err + } if err = config.Paths.Add(pth, path); err != nil { return err } @@ -296,6 +314,9 @@ $ %s pth gen ibc-0 ibc-1 demo-path --unordered false --version ics20-2`, appName default: path.GenSrcChanID() path.GenDstChanID() + if err = config.ValidatePath(path); err != nil { + return err + } if err = config.Paths.Add(pth, path); err != nil { return err } @@ -495,6 +516,10 @@ func fileInputPathAdd(file, name string) (cfg *Config, err error) { return nil, err } + if err = config.ValidatePath(p); err != nil { + return nil, err + } + if err = config.Paths.Add(name, p); err != nil { return nil, err } @@ -629,6 +654,10 @@ func userInputPathAdd(src, dst, name string) (*Config, error) { return nil, err } + if err = config.ValidatePath(path); err != nil { + return nil, err + } + if err = config.Paths.Add(name, path); err != nil { return nil, err } diff --git a/relayer/path.go b/relayer/path.go index 5d793fc1c..131ad34b9 100644 --- a/relayer/path.go +++ b/relayer/path.go @@ -49,7 +49,6 @@ func (p Paths) MustGet(name string) *Path { // Add adds a path by its name func (p Paths) Add(name string, path *Path) error { - // TODO: Do bottomo up validation of path if _, found := p[name]; found { return fmt.Errorf("path with name %s already exists", name) } From c2748df9f0499071f8eb5aa2e95b34a0d0b5c1ab Mon Sep 17 00:00:00 2001 From: akhilkumarpilli Date: Sat, 30 Jan 2021 10:31:12 +0530 Subject: [PATCH 3/7] Fetch single identifier data directly in validation --- cmd/config.go | 51 +++++++++++++++++++++++++++------------------------ 1 file changed, 27 insertions(+), 24 deletions(-) diff --git a/cmd/config.go b/cmd/config.go index 8519e81b1..02c43d8cc 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -475,18 +475,17 @@ func (c *Config) ValidateClient(pe *relayer.PathEnd) error { return err } - // TODO: add appropriate offset and limits, along with retries - clients, err := chain.QueryClients(0, 1000) + height, err := chain.QueryLatestHeight() if err != nil { return err } - for _, clientState := range clients.ClientStates { - if clientState.ClientId == pe.ClientID { - return nil - } + _, err = chain.QueryClientState(height) + if err != nil { + return err } - return fmt.Errorf("No client exists with given client id: %s", pe.ClientID) + + return nil } // ValidateConnection validates connection id in provided pathend @@ -500,18 +499,21 @@ func (c *Config) ValidateConnection(pe *relayer.PathEnd) error { return err } - // TODO: add appropriate offset and limits, along with retries - connections, err := chain.QueryConnections(0, 1000) + height, err := chain.QueryLatestHeight() if err != nil { return err } - for _, connection := range connections.Connections { - if connection.ClientId == pe.ClientID && connection.Id == pe.ConnectionID { - return nil - } + connection, err := chain.QueryConnection(height) + if err != nil { + return err + } + + if connection.Connection.ClientId != pe.ClientID { + return fmt.Errorf("ClientID of connection: %s didn't match with provided ClientID", pe.ConnectionID) } - return fmt.Errorf("No connection exists with given connection id: %s", pe.ConnectionID) + + return nil } // ValidateChannel validates channel id in provided pathend @@ -525,20 +527,21 @@ func (c *Config) ValidateChannel(pe *relayer.PathEnd) error { return err } - // TODO: add appropriate offset and limits, along with retries - channels, err := chain.QueryChannels(0, 1000) + height, err := chain.QueryLatestHeight() if err != nil { return err } - for _, channel := range channels.Channels { - if channel.ChannelId == pe.ChannelID { - for _, connection := range channel.ConnectionHops { - if connection == pe.ConnectionID { - return nil - } - } + channel, err := chain.QueryChannel(height) + if err != nil { + return err + } + + for _, connection := range channel.Channel.ConnectionHops { + if connection == pe.ConnectionID { + return nil } } - return fmt.Errorf("No channel exists with given channel id: %s", pe.ChannelID) + + return fmt.Errorf("ConnectionID of channel: %s didn't match with provided ConnectionID", pe.ChannelID) } From f8bfb87b13a685ba40f6b730d2c533343f3486f7 Mon Sep 17 00:00:00 2001 From: akhilkumarpilli Date: Sat, 30 Jan 2021 12:45:55 +0530 Subject: [PATCH 4/7] Address PR comments --- cmd/config.go | 57 +++++++++++++++++---------------------------------- 1 file changed, 19 insertions(+), 38 deletions(-) diff --git a/cmd/config.go b/cmd/config.go index 02c43d8cc..76b59c6b8 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -441,23 +441,34 @@ func (c *Config) ValidatePath(p *relayer.Path) (err error) { return nil } -// ValidatePathEnd allow validates provided pathend and returns error for invalid identifiers +// ValidatePathEnd validates provided pathend and returns error for invalid identifiers func (c *Config) ValidatePathEnd(pe *relayer.PathEnd) error { if err := pe.ValidateBasic(); err != nil { return err } + + chain, err := c.Chains.Get(pe.ChainID) + if err != nil { + return err + } + + height, err := chain.QueryLatestHeight() + if err != nil { + return err + } + if pe.ClientID != "" { - if err := c.ValidateClient(pe); err != nil { + if err := c.ValidateClient(chain, height, pe); err != nil { return err } } if pe.ConnectionID != "" { - if err := c.ValidateConnection(pe); err != nil { + if err := c.ValidateConnection(chain, height, pe); err != nil { return err } } if pe.ChannelID != "" { - if err := c.ValidateChannel(pe); err != nil { + if err := c.ValidateChannel(chain, height, pe); err != nil { return err } } @@ -465,22 +476,12 @@ func (c *Config) ValidatePathEnd(pe *relayer.PathEnd) error { } // ValidateClient validates client id in provided pathend -func (c *Config) ValidateClient(pe *relayer.PathEnd) error { +func (c *Config) ValidateClient(chain *relayer.Chain, height int64, pe *relayer.PathEnd) error { if err := pe.Vclient(); err != nil { return err } - chain, err := c.Chains.Get(pe.ChainID) - if err != nil { - return err - } - - height, err := chain.QueryLatestHeight() - if err != nil { - return err - } - - _, err = chain.QueryClientState(height) + _, err := chain.QueryClientState(height) if err != nil { return err } @@ -489,21 +490,11 @@ func (c *Config) ValidateClient(pe *relayer.PathEnd) error { } // ValidateConnection validates connection id in provided pathend -func (c *Config) ValidateConnection(pe *relayer.PathEnd) error { +func (c *Config) ValidateConnection(chain *relayer.Chain, height int64, pe *relayer.PathEnd) error { if err := pe.Vconn(); err != nil { return err } - chain, err := c.Chains.Get(pe.ChainID) - if err != nil { - return err - } - - height, err := chain.QueryLatestHeight() - if err != nil { - return err - } - connection, err := chain.QueryConnection(height) if err != nil { return err @@ -517,21 +508,11 @@ func (c *Config) ValidateConnection(pe *relayer.PathEnd) error { } // ValidateChannel validates channel id in provided pathend -func (c *Config) ValidateChannel(pe *relayer.PathEnd) error { +func (c *Config) ValidateChannel(chain *relayer.Chain, height int64, pe *relayer.PathEnd) error { if err := pe.Vchan(); err != nil { return err } - chain, err := c.Chains.Get(pe.ChainID) - if err != nil { - return err - } - - height, err := chain.QueryLatestHeight() - if err != nil { - return err - } - channel, err := chain.QueryChannel(height) if err != nil { return err From cc643a792d8973ac250c361740b0e46dab3a05c3 Mon Sep 17 00:00:00 2001 From: akhilkumarpilli Date: Tue, 2 Feb 2021 19:15:53 +0530 Subject: [PATCH 5/7] Address new PR comments --- cmd/config.go | 29 ++++++++++++++++++----------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/cmd/config.go b/cmd/config.go index 76b59c6b8..18705934e 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -422,12 +422,12 @@ func overWriteConfig(cmd *cobra.Command, cfg *Config) error { // ValidatePath checks that a path is valid func (c *Config) ValidatePath(p *relayer.Path) (err error) { - if err = c.ValidatePathEnd(p.Src); err != nil { - return err - } if p.Src.Version == "" { return fmt.Errorf("source must specify a version") } + if err = c.ValidatePathEnd(p.Src); err != nil { + return err + } if err = c.ValidatePathEnd(p.Dst); err != nil { return err } @@ -461,17 +461,24 @@ func (c *Config) ValidatePathEnd(pe *relayer.PathEnd) error { if err := c.ValidateClient(chain, height, pe); err != nil { return err } - } - if pe.ConnectionID != "" { - if err := c.ValidateConnection(chain, height, pe); err != nil { - return err + + if pe.ConnectionID != "" { + if err := c.ValidateConnection(chain, height, pe); err != nil { + return err + } + + if pe.ChannelID != "" { + if err := c.ValidateChannel(chain, height, pe); err != nil { + return err + } + } } } - if pe.ChannelID != "" { - if err := c.ValidateChannel(chain, height, pe); err != nil { - return err - } + + if pe.ClientID == "" && pe.ConnectionID != "" { + return fmt.Errorf("ClientID is not conifgured for the connection: %s", pe.ConnectionID) } + return nil } From 28cce1d48d3c60691bef02bca3945d639987c85e Mon Sep 17 00:00:00 2001 From: akhilkumarpilli Date: Tue, 2 Feb 2021 19:18:34 +0530 Subject: [PATCH 6/7] Add channel and connection condition --- cmd/config.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/cmd/config.go b/cmd/config.go index 18705934e..2eabfa132 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -473,10 +473,14 @@ func (c *Config) ValidatePathEnd(pe *relayer.PathEnd) error { } } } + + if pe.ConnectionID == "" && pe.ChannelID != "" { + return fmt.Errorf("ConnectionID is not configured for the channel: %s", pe.ChannelID) + } } if pe.ClientID == "" && pe.ConnectionID != "" { - return fmt.Errorf("ClientID is not conifgured for the connection: %s", pe.ConnectionID) + return fmt.Errorf("ClientID is not configured for the connection: %s", pe.ConnectionID) } return nil From a618e03d1f8cf419fa16603f1f5dbbdffc614cf6 Mon Sep 17 00:00:00 2001 From: akhilkumarpilli Date: Tue, 2 Feb 2021 19:20:20 +0530 Subject: [PATCH 7/7] Fix lint issues --- cmd/config.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/cmd/config.go b/cmd/config.go index 8a7bdf500..43f6a9877 100644 --- a/cmd/config.go +++ b/cmd/config.go @@ -478,12 +478,12 @@ func (c *Config) ValidatePathEnd(pe *relayer.PathEnd) error { } if pe.ConnectionID == "" && pe.ChannelID != "" { - return fmt.Errorf("ConnectionID is not configured for the channel: %s", pe.ChannelID) + return fmt.Errorf("connectionID is not configured for the channel: %s", pe.ChannelID) } } if pe.ClientID == "" && pe.ConnectionID != "" { - return fmt.Errorf("ClientID is not configured for the connection: %s", pe.ConnectionID) + return fmt.Errorf("clientID is not configured for the connection: %s", pe.ConnectionID) } return nil @@ -515,7 +515,7 @@ func (c *Config) ValidateConnection(chain *relayer.Chain, height int64, pe *rela } if connection.Connection.ClientId != pe.ClientID { - return fmt.Errorf("ClientID of connection: %s didn't match with provided ClientID", pe.ConnectionID) + return fmt.Errorf("clientID of connection: %s didn't match with provided ClientID", pe.ConnectionID) } return nil @@ -538,5 +538,5 @@ func (c *Config) ValidateChannel(chain *relayer.Chain, height int64, pe *relayer } } - return fmt.Errorf("ConnectionID of channel: %s didn't match with provided ConnectionID", pe.ChannelID) + return fmt.Errorf("connectionID of channel: %s didn't match with provided ConnectionID", pe.ChannelID) }