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

connection and channel identifiers reuse #386

Merged
merged 8 commits into from
Feb 1, 2021
Merged

connection and channel identifiers reuse #386

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
fabe1c7
connection and channel identifiers reuse
akhilkumarpilli Jan 21, 2021
f2a424b
Address PR comments
akhilkumarpilli Jan 22, 2021
2e0705f
Merge branch 'master' into akhil/349-identifier-reuse
akhilkumarpilli Jan 22, 2021
1bda75c
Fix validation in FindMatchingChannel
akhilkumarpilli Jan 22, 2021
efce2bf
Address comments
akhilkumarpilli Jan 25, 2021
60d1fad
Add gaia reuse identifiers test
akhilkumarpilli Jan 28, 2021
33804f9
Update channel and connection identifier reuse conditions
akhilkumarpilli Jan 29, 2021
c919ebf
Add one more condition in channel/connection creation
akhilkumarpilli Jan 29, 2021
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
156 changes: 105 additions & 51 deletions relayer/channel-tx.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,6 @@ func ExecuteChannelStep(src, dst *Chain) (success, last, modified bool, err erro
// if either identifier is missing, an existing channel that matches the required fields
// is chosen or a new channel is created.
if src.PathEnd.ChannelID == "" || dst.PathEnd.ChannelID == "" {
// TODO: Query for existing identifier and fill config, if possible
success, modified, err := InitializeChannel(src, dst, srcUpdateHeader, dstUpdateHeader, sh)
if err != nil {
return false, false, false, err
Expand Down Expand Up @@ -221,6 +220,9 @@ func ExecuteChannelStep(src, dst *Chain) (success, last, modified bool, err erro
return false, false, false, err
}

case srcChan.Channel.State == chantypes.OPEN && dstChan.Channel.State == chantypes.OPEN:
last = true

}

return true, last, false, nil
Expand All @@ -240,22 +242,25 @@ func InitializeChannel(src, dst *Chain, srcUpdateHeader, dstUpdateHeader *tmclie
// TODO: log that we are attempting to create new channel ends
}

// cosntruct OpenInit message to be submitted on source chain
msgs := []sdk.Msg{
src.UpdateClient(dstUpdateHeader),
src.ChanInit(dst.PathEnd),
}
channelID, found := FindMatchingChannel(src, dst)
if !found {
// construct OpenInit message to be submitted on source chain
msgs := []sdk.Msg{
src.UpdateClient(dstUpdateHeader),
src.ChanInit(dst.PathEnd),
}

res, success, err := src.SendMsgs(msgs)
if !success {
return false, false, err
}
res, success, err := src.SendMsgs(msgs)
if !success {
return false, false, err
}

// update channel identifier in PathEnd
// use index 1, channel open init is the second message in the transaction
channelID, err := ParseChannelIDFromEvents(res.Logs[1].Events)
if err != nil {
return false, false, err
// update channel identifier in PathEnd
// use index 1, channel open init is the second message in the transaction
channelID, err = ParseChannelIDFromEvents(res.Logs[1].Events)
if err != nil {
return false, false, err
}
}
src.PathEnd.ChannelID = channelID

Expand All @@ -268,26 +273,29 @@ func InitializeChannel(src, dst *Chain, srcUpdateHeader, dstUpdateHeader *tmclie
// TODO: update logging
}

// open try on source chain
openTry, err := src.ChanTry(dst, dstUpdateHeader.GetHeight().GetRevisionHeight()-1)
if err != nil {
return false, false, err
}
channelID, found := FindMatchingChannel(src, dst)
if !found {
// open try on source chain
openTry, err := src.ChanTry(dst, dstUpdateHeader.GetHeight().GetRevisionHeight()-1)
if err != nil {
return false, false, err
}

msgs := []sdk.Msg{
src.UpdateClient(dstUpdateHeader),
openTry,
}
res, success, err := src.SendMsgs(msgs)
if !success {
return false, false, err
}
msgs := []sdk.Msg{
src.UpdateClient(dstUpdateHeader),
openTry,
}
res, success, err := src.SendMsgs(msgs)
if !success {
return false, false, err
}

// update channel identifier in PathEnd
// use index 1, channel open try is the second message in the transaction
channelID, err := ParseChannelIDFromEvents(res.Logs[1].Events)
if err != nil {
return false, false, err
// update channel identifier in PathEnd
// use index 1, channel open try is the second message in the transaction
channelID, err = ParseChannelIDFromEvents(res.Logs[1].Events)
if err != nil {
return false, false, err
}
}
src.PathEnd.ChannelID = channelID

Expand All @@ -300,26 +308,29 @@ func InitializeChannel(src, dst *Chain, srcUpdateHeader, dstUpdateHeader *tmclie
// TODO: update logging
}

// open try on destination chain
openTry, err := dst.ChanTry(src, srcUpdateHeader.GetHeight().GetRevisionHeight()-1)
if err != nil {
return false, false, err
}
channelID, found := FindMatchingChannel(dst, src)
if !found {
// open try on destination chain
openTry, err := dst.ChanTry(src, srcUpdateHeader.GetHeight().GetRevisionHeight()-1)
if err != nil {
return false, false, err
}

msgs := []sdk.Msg{
dst.UpdateClient(srcUpdateHeader),
openTry,
}
res, success, err := dst.SendMsgs(msgs)
if !success {
return false, false, err
}
msgs := []sdk.Msg{
dst.UpdateClient(srcUpdateHeader),
openTry,
}
res, success, err := dst.SendMsgs(msgs)
if !success {
return false, false, err
}

// update channel identifier in PathEnd
// use index 1, channel open try is the second message in the transaction
channelID, err := ParseChannelIDFromEvents(res.Logs[1].Events)
if err != nil {
return false, false, err
// update channel identifier in PathEnd
// use index 1, channel open try is the second message in the transaction
channelID, err = ParseChannelIDFromEvents(res.Logs[1].Events)
if err != nil {
return false, false, err
}
}
dst.PathEnd.ChannelID = channelID

Expand Down Expand Up @@ -453,3 +464,46 @@ func (c *Chain) CloseChannelStep(dst *Chain) (*RelayMsgs, error) {
}
return out, nil
}

// FindMatchingChannel will determine if there already exists a channel between source and counterparty
// that matches the parameters set in the relayer config.
func FindMatchingChannel(source, counterparty *Chain) (string, bool) {
colin-axner marked this conversation as resolved.
Show resolved Hide resolved
// TODO: add appropriate offset and limits, along with retries
channelsResp, err := source.QueryChannels(0, 1000)
if err != nil {
if source.debug {
source.Log(fmt.Sprintf("Error: querying channels on %s failed: %v", source.PathEnd.ChainID, err))
}
return "", false
}

for _, channel := range channelsResp.Channels {
if IsMatchingChannel(source, counterparty, channel) {
// unused channel found
return channel.ChannelId, true
}
}

return "", false
}

// IsMatchingChannel determines if given channel matches required conditions
func IsMatchingChannel(source, counterparty *Chain, channel *chantypes.IdentifiedChannel) bool {
return channel.Ordering == source.PathEnd.GetOrder() &&
IsConnectionFound(channel.ConnectionHops, source.PathEnd.ConnectionID) &&
channel.Version == source.PathEnd.Version &&
channel.PortId == source.PathEnd.PortID && channel.Counterparty.PortId == counterparty.PathEnd.PortID &&
(((channel.State == chantypes.INIT || channel.State == chantypes.TRYOPEN) && channel.Counterparty.ChannelId == "") ||
(channel.State == chantypes.OPEN && (counterparty.PathEnd.ChannelID == "" ||
channel.Counterparty.ChannelId == counterparty.PathEnd.ChannelID)))
}

// IsConnectionFound determines if given connectionId is present in channel connectionHops list
func IsConnectionFound(connectionHops []string, connectionID string) bool {
for _, id := range connectionHops {
if id == connectionID {
return true
}
}
return false
}
144 changes: 95 additions & 49 deletions relayer/connection-tx.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,6 @@ func ExecuteConnectionStep(src, dst *Chain) (success, last, modified bool, err e
// is chosen or a new connection is created.
// This will perform either an OpenInit or OpenTry step and return
if src.PathEnd.ConnectionID == "" || dst.PathEnd.ConnectionID == "" {
// TODO: Query for existing identifier and fill config, if possible
success, modified, err := InitializeConnection(src, dst, srcUpdateHeader, dstUpdateHeader, sh)
if err != nil {
return false, false, false, err
Expand Down Expand Up @@ -218,6 +217,9 @@ func ExecuteConnectionStep(src, dst *Chain) (success, last, modified bool, err e
return false, false, false, err
}

case srcConn.Connection.State == conntypes.OPEN && dstConn.Connection.State == conntypes.OPEN:
last = true

Comment on lines +220 to +222
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@colin-axner, Added this extra condition since both src connection and dst connection states are OPEN and going into infinite loop. Is this right way?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes it is! It fixes a bug I was running into previously as well. Great job, great intuition. I'm glad there are tests now.

I redesigned the handshakes with the assumption that we were beginning with empty identifiers and therefore overlooked the case when a user supplies fully set identifiers

}

return true, last, false, nil
Expand All @@ -237,22 +239,25 @@ func InitializeConnection(src, dst *Chain, srcUpdateHeader, dstUpdateHeader *tmc
// TODO: log that we are attempting to create new connection ends
}

// cosntruct OpenInit message to be submitted on source chain
msgs := []sdk.Msg{
src.UpdateClient(dstUpdateHeader),
src.ConnInit(dst.PathEnd),
}
connectionID, found := FindMatchingConnection(src, dst)
if !found {
// construct OpenInit message to be submitted on source chain
msgs := []sdk.Msg{
src.UpdateClient(dstUpdateHeader),
src.ConnInit(dst.PathEnd),
}

res, success, err := src.SendMsgs(msgs)
if !success {
return false, false, err
}
res, success, err := src.SendMsgs(msgs)
if !success {
return false, false, err
}

// update connection identifier in PathEnd
// use index 1, connection open init is the second message in the transaction
connectionID, err := ParseConnectionIDFromEvents(res.Logs[1].Events)
if err != nil {
return false, false, err
// update connection identifier in PathEnd
// use index 1, connection open init is the second message in the transaction
connectionID, err = ParseConnectionIDFromEvents(res.Logs[1].Events)
if err != nil {
return false, false, err
}
}
src.PathEnd.ConnectionID = connectionID

Expand All @@ -265,25 +270,28 @@ func InitializeConnection(src, dst *Chain, srcUpdateHeader, dstUpdateHeader *tmc
// TODO: update logging
}

openTry, err := src.ConnTry(dst, dstUpdateHeader.GetHeight().GetRevisionHeight()-1)
if err != nil {
return false, false, err
}
connectionID, found := FindMatchingConnection(src, dst)
if !found {
openTry, err := src.ConnTry(dst, dstUpdateHeader.GetHeight().GetRevisionHeight()-1)
if err != nil {
return false, false, err
}

msgs := []sdk.Msg{
src.UpdateClient(dstUpdateHeader),
openTry,
}
res, success, err := src.SendMsgs(msgs)
if !success {
return false, false, err
}
msgs := []sdk.Msg{
src.UpdateClient(dstUpdateHeader),
openTry,
}
res, success, err := src.SendMsgs(msgs)
if !success {
return false, false, err
}

// update connection identifier in PathEnd
// use index 1, connection open try is the second message in the transaction
connectionID, err := ParseConnectionIDFromEvents(res.Logs[1].Events)
if err != nil {
return false, false, err
// update connection identifier in PathEnd
// use index 1, connection open try is the second message in the transaction
connectionID, err = ParseConnectionIDFromEvents(res.Logs[1].Events)
if err != nil {
return false, false, err
}
}
src.PathEnd.ConnectionID = connectionID

Expand All @@ -296,25 +304,28 @@ func InitializeConnection(src, dst *Chain, srcUpdateHeader, dstUpdateHeader *tmc
// TODO: update logging
}

openTry, err := dst.ConnTry(src, srcUpdateHeader.GetHeight().GetRevisionHeight()-1)
if err != nil {
return false, false, err
}
connectionID, found := FindMatchingConnection(dst, src)
if !found {
openTry, err := dst.ConnTry(src, srcUpdateHeader.GetHeight().GetRevisionHeight()-1)
if err != nil {
return false, false, err
}

msgs := []sdk.Msg{
dst.UpdateClient(srcUpdateHeader),
openTry,
}
res, success, err := dst.SendMsgs(msgs)
if !success {
return false, false, err
}
msgs := []sdk.Msg{
dst.UpdateClient(srcUpdateHeader),
openTry,
}
res, success, err := dst.SendMsgs(msgs)
if !success {
return false, false, err
}

// update connection identifier in PathEnd
// use index 1, connection open try is the second message in the transaction
connectionID, err := ParseConnectionIDFromEvents(res.Logs[1].Events)
if err != nil {
return false, false, err
// update connection identifier in PathEnd
// use index 1, connection open try is the second message in the transaction
connectionID, err = ParseConnectionIDFromEvents(res.Logs[1].Events)
if err != nil {
return false, false, err
}
}
dst.PathEnd.ConnectionID = connectionID

Expand All @@ -324,3 +335,38 @@ func InitializeConnection(src, dst *Chain, srcUpdateHeader, dstUpdateHeader *tmc
return false, true, fmt.Errorf("connection ends already created")
}
}

// FindMatchingConnection will determine if there already exists a connection between source and counterparty
// that matches the parameters set in the relayer config.
func FindMatchingConnection(source, counterparty *Chain) (string, bool) {
// TODO: add appropriate offset and limits, along with retries
connectionsResp, err := source.QueryConnections(0, 1000)
if err != nil {
if source.debug {
source.Log(fmt.Sprintf("Error: querying connections on %s failed: %v", source.PathEnd.ChainID, err))
}
return "", false
}

for _, connection := range connectionsResp.Connections {
if IsMatchingConnection(source, counterparty, connection) {
// unused connection found
return connection.Id, true
}
}

return "", false
}

// IsMatchingConnection determines if given connection matches required conditions
func IsMatchingConnection(source, counterparty *Chain, connection *conntypes.IdentifiedConnection) bool {
// determines version we use is matching with given versions
_, isVersionMatched := conntypes.FindSupportedVersion(conntypes.DefaultIBCVersion, conntypes.ProtoVersionsToExported(connection.Versions))
return connection.ClientId == source.PathEnd.ClientID &&
connection.Counterparty.ClientId == counterparty.PathEnd.ClientID &&
isVersionMatched && connection.DelayPeriod == defaultDelayPeriod &&
connection.Counterparty.Prefix.String() == defaultChainPrefix.String() &&
(((connection.State == conntypes.INIT || connection.State == conntypes.TRYOPEN) && connection.Counterparty.ConnectionId == "") ||
(connection.State == conntypes.OPEN && (counterparty.PathEnd.ConnectionID == "" ||
connection.Counterparty.ConnectionId == counterparty.PathEnd.ConnectionID)))
}
Loading