From ff70382d637ea39b1b0162be75ec7f435a256e27 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Thu, 18 Feb 2021 12:14:54 +0100 Subject: [PATCH 1/5] refactor connection handshake update messages --- relayer/connection.go | 83 ++++++++++++------------------------------- relayer/msgs.go | 60 ++++++++++++++++++++++++------- 2 files changed, 69 insertions(+), 74 deletions(-) diff --git a/relayer/connection.go b/relayer/connection.go index a7c3156f4..6016559d5 100644 --- a/relayer/connection.go +++ b/relayer/connection.go @@ -4,7 +4,6 @@ import ( "fmt" "time" - sdk "github.com/cosmos/cosmos-sdk/types" conntypes "github.com/cosmos/cosmos-sdk/x/ibc/core/03-connection/types" ) @@ -76,25 +75,7 @@ func (c *Chain) CreateOpenConnections(dst *Chain, maxRetries uint64, to time.Dur // file. The booleans return indicate if the message was successfully // executed and if this was the last handshake step. func ExecuteConnectionStep(src, dst *Chain) (success, last, modified bool, err error) { - // variables needed to determine the current handshake step - var ( - srcConn, dstConn *conntypes.QueryConnectionResponse - msgs []sdk.Msg - ) - - // NOTE: proof construction for handshake messages - // relies on delivery of the associated update message. - // Updating the light client again could result in - // failed handshakes since the proof height would - // rely on a consensus state that has not been committed - // to the chain. - srcUpdateMsg, err := src.UpdateClient(dst) - if err != nil { - return false, false, false, err - } - - dstUpdateMsg, err := dst.UpdateClient(src) - if err != nil { + if _, _, err := UpdateLightClients(src, dst); err != nil { return false, false, false, err } @@ -104,7 +85,7 @@ 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 == "" { - success, modified, err := InitializeConnection(src, dst, srcUpdateMsg, dstUpdateMsg) + success, modified, err := InitializeConnection(src, dst) if err != nil { return false, false, false, err } @@ -113,7 +94,7 @@ func ExecuteConnectionStep(src, dst *Chain) (success, last, modified bool, err e } // Query Connection data from src and dst - srcConn, dstConn, err = QueryConnectionPair(src, dst, int64(src.MustGetLatestLightHeight())-1, + srcConn, dstConn, err := QueryConnectionPair(src, dst, int64(src.MustGetLatestLightHeight())-1, int64(dst.MustGetLatestLightHeight()-1)) if err != nil { return false, false, false, err @@ -129,15 +110,11 @@ func ExecuteConnectionStep(src, dst *Chain) (success, last, modified bool, err e logConnectionStates(src, dst, srcConn, dstConn) } - openTry, err := src.ConnTry(dst) + msgs, err := src.ConnTry(dst) if err != nil { return false, false, false, err } - msgs = []sdk.Msg{ - srcUpdateMsg, - openTry, - } _, success, err = src.SendMsgs(msgs) if !success { return false, false, false, err @@ -152,15 +129,11 @@ func ExecuteConnectionStep(src, dst *Chain) (success, last, modified bool, err e logConnectionStates(src, dst, srcConn, dstConn) } - openAck, err := src.ConnAck(dst) + msgs, err := src.ConnAck(dst) if err != nil { return false, false, false, err } - msgs = []sdk.Msg{ - srcUpdateMsg, - openAck, - } _, success, err = src.SendMsgs(msgs) if !success { return false, false, false, err @@ -174,15 +147,11 @@ func ExecuteConnectionStep(src, dst *Chain) (success, last, modified bool, err e logConnectionStates(dst, src, dstConn, srcConn) } - openAck, err := dst.ConnAck(src) + msgs, err := dst.ConnAck(src) if err != nil { return false, false, false, err } - msgs = []sdk.Msg{ - dstUpdateMsg, - openAck, - } _, success, err = dst.SendMsgs(msgs) if !success { return false, false, false, err @@ -194,10 +163,11 @@ func ExecuteConnectionStep(src, dst *Chain) (success, last, modified bool, err e logConnectionStates(src, dst, srcConn, dstConn) } - msgs = []sdk.Msg{ - srcUpdateMsg, - src.ConnConfirm(dstConn), + msgs, err := src.ConnConfirm(dst) + if err != nil { + return false, false, false, err } + _, success, err = src.SendMsgs(msgs) if !success { return false, false, false, err @@ -211,16 +181,18 @@ func ExecuteConnectionStep(src, dst *Chain) (success, last, modified bool, err e logConnectionStates(dst, src, dstConn, srcConn) } - msgs = []sdk.Msg{ - dstUpdateMsg, - dst.ConnConfirm(srcConn), + msgs, err := dst.ConnConfirm(src) + if err != nil { + return false, false, false, err } - last = true + _, success, err = dst.SendMsgs(msgs) if !success { return false, false, false, err } + last = true + case srcConn.Connection.State == conntypes.OPEN && dstConn.Connection.State == conntypes.OPEN: last = true @@ -233,9 +205,7 @@ func ExecuteConnectionStep(src, dst *Chain) (success, last, modified bool, err e // The identifiers set in the PathEnd's are used to determine which connection ends need to be // initialized. The PathEnds are updated upon a successful transaction. // NOTE: This function may need to be called twice if neither connection exists. -func InitializeConnection( - src, dst *Chain, srcUpdateMsg, dstUpdateMsg sdk.Msg, -) (success, modified bool, err error) { +func InitializeConnection(src, dst *Chain) (success, modified bool, err error) { switch { // OpenInit on source @@ -249,10 +219,9 @@ func InitializeConnection( connectionID, found := FindMatchingConnection(src, dst) if !found { // construct OpenInit message to be submitted on source chain - - msgs := []sdk.Msg{ - srcUpdateMsg, - src.ConnInit(dst.PathEnd), + msgs, err := src.ConnInit(dst) + if err != nil { + return false, false, err } res, success, err := src.SendMsgs(msgs) @@ -281,15 +250,11 @@ func InitializeConnection( connectionID, found := FindMatchingConnection(src, dst) if !found { - openTry, err := src.ConnTry(dst) + msgs, err := src.ConnTry(dst) if err != nil { return false, false, err } - msgs := []sdk.Msg{ - srcUpdateMsg, - openTry, - } res, success, err := src.SendMsgs(msgs) if !success { return false, false, err @@ -316,15 +281,11 @@ func InitializeConnection( connectionID, found := FindMatchingConnection(dst, src) if !found { - openTry, err := dst.ConnTry(src) + msgs, err := dst.ConnTry(src) if err != nil { return false, false, err } - msgs := []sdk.Msg{ - dstUpdateMsg, - openTry, - } res, success, err := dst.SendMsgs(msgs) if !success { return false, false, err diff --git a/relayer/msgs.go b/relayer/msgs.go index 8043dc4aa..64d19e183 100644 --- a/relayer/msgs.go +++ b/relayer/msgs.go @@ -44,7 +44,8 @@ func (c *Chain) CreateClient( return msg } -// UpdateClient creates an sdk.Msg to update the client on src with data pulled from dst. +// UpdateClient creates an sdk.Msg to update the client on src with data pulled from dst +// at the request height.. func (c *Chain) UpdateClient(dst *Chain) (sdk.Msg, error) { header, err := dst.GetIBCUpdateHeader(c) if err != nil { @@ -66,22 +67,35 @@ func (c *Chain) UpdateClient(dst *Chain) (sdk.Msg, error) { } // ConnInit creates a MsgConnectionOpenInit -func (c *Chain) ConnInit(counterparty *PathEnd) sdk.Msg { +func (c *Chain) ConnInit(counterparty *Chain) ([]sdk.Msg, error) { + updateMsg, err := c.UpdateClient(counterparty) + if err != nil { + return nil, err + } + var version *conntypes.Version - return conntypes.NewMsgConnectionOpenInit( + msg := conntypes.NewMsgConnectionOpenInit( c.PathEnd.ClientID, - counterparty.ClientID, + counterparty.PathEnd.ClientID, defaultChainPrefix, version, defaultDelayPeriod, c.MustGetAddress(), // 'MustGetAddress' must be called directly before calling 'NewMsg...' ) + + return []sdk.Msg{updateMsg, msg}, nil + } // ConnTry creates a MsgConnectionOpenTry func (c *Chain) ConnTry( counterparty *Chain, -) (sdk.Msg, error) { +) ([]sdk.Msg, error) { + updateMsg, err := c.UpdateClient(counterparty) + if err != nil { + return nil, err + } + // NOTE: the proof height uses - 1 due to tendermint's delayed execution model clientState, clientStateProof, consensusStateProof, connStateProof, proofHeight, err := counterparty.GenerateConnHandshakeProof(counterparty.MustGetLatestLightHeight() - 1) @@ -109,13 +123,19 @@ func (c *Chain) ConnTry( if err := msg.ValidateBasic(); err != nil { return nil, err } - return msg, nil + + return []sdk.Msg{updateMsg, msg}, nil } // ConnAck creates a MsgConnectionOpenAck func (c *Chain) ConnAck( counterparty *Chain, -) (sdk.Msg, error) { +) ([]sdk.Msg, error) { + updateMsg, err := c.UpdateClient(counterparty) + if err != nil { + return nil, err + } + // NOTE: the proof height uses - 1 due to tendermint's delayed execution model clientState, clientStateProof, consensusStateProof, connStateProof, proofHeight, err := counterparty.GenerateConnHandshakeProof(counterparty.MustGetLatestLightHeight() - 1) @@ -123,7 +143,7 @@ func (c *Chain) ConnAck( return nil, err } - return conntypes.NewMsgConnectionOpenAck( + msg := conntypes.NewMsgConnectionOpenAck( c.PathEnd.ConnectionID, counterparty.PathEnd.ConnectionID, clientState, @@ -134,17 +154,31 @@ func (c *Chain) ConnAck( clientState.GetLatestHeight().(clienttypes.Height), conntypes.DefaultIBCVersion, c.MustGetAddress(), // 'MustGetAddress' must be called directly before calling 'NewMsg...' - ), nil + ) + + return []sdk.Msg{updateMsg, msg}, nil } // ConnConfirm creates a MsgConnectionOpenConfirm -func (c *Chain) ConnConfirm(counterpartyConnState *conntypes.QueryConnectionResponse) sdk.Msg { - return conntypes.NewMsgConnectionOpenConfirm( +func (c *Chain) ConnConfirm(counterparty *Chain) ([]sdk.Msg, error) { + updateMsg, err := c.UpdateClient(counterparty) + if err != nil { + return nil, err + } + + counterpartyConnState, err := counterparty.QueryConnection(int64(counterparty.MustGetLatestLightHeight()) - 1) + if err != nil { + return nil, err + } + + msg := conntypes.NewMsgConnectionOpenConfirm( c.PathEnd.ConnectionID, counterpartyConnState.Proof, counterpartyConnState.ProofHeight, c.MustGetAddress(), // 'MustGetAddress' must be called directly before calling 'NewMsg...' ) + + return []sdk.Msg{updateMsg, msg}, nil } // ChanInit creates a MsgChannelOpenInit @@ -162,7 +196,7 @@ func (c *Chain) ChanInit(counterparty *PathEnd) sdk.Msg { // ChanTry creates a MsgChannelOpenTry func (c *Chain) ChanTry( counterparty *Chain, -) (sdk.Msg, error) { +) (*chantypes.MsgChannelOpenTry, error) { // NOTE: the proof height uses - 1 due to tendermint's delayed execution model counterpartyChannelRes, err := counterparty.QueryChannel(int64(counterparty.MustGetLatestLightHeight()) - 1) if err != nil { @@ -188,7 +222,7 @@ func (c *Chain) ChanTry( // ChanAck creates a MsgChannelOpenAck func (c *Chain) ChanAck( counterparty *Chain, -) (sdk.Msg, error) { +) (*chantypes.MsgChannelOpenAck, error) { // NOTE: the proof height uses - 1 due to tendermint's delayed execution model counterpartyChannelRes, err := counterparty.QueryChannel(int64(counterparty.MustGetLatestLightHeight()) - 1) if err != nil { From dd8babae5955d1cf04e78633926a49365944498d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Thu, 18 Feb 2021 12:25:36 +0100 Subject: [PATCH 2/5] refactor channel update msg handling --- relayer/channel.go | 79 ++++++++++++---------------------------------- relayer/msgs.go | 60 +++++++++++++++++++++++++++-------- 2 files changed, 68 insertions(+), 71 deletions(-) diff --git a/relayer/channel.go b/relayer/channel.go index 6499d831a..c4eeb3c7a 100644 --- a/relayer/channel.go +++ b/relayer/channel.go @@ -4,7 +4,6 @@ import ( "fmt" "time" - sdk "github.com/cosmos/cosmos-sdk/types" chantypes "github.com/cosmos/cosmos-sdk/x/ibc/core/04-channel/types" ) @@ -79,26 +78,14 @@ func (c *Chain) CreateOpenChannels(dst *Chain, maxRetries uint64, to time.Durati // file. The booleans return indicate if the message was successfully // executed and if this was the last handshake step. func ExecuteChannelStep(src, dst *Chain) (success, last, modified bool, err error) { - // variables needed to determine the current handshake step - var ( - srcChan, dstChan *chantypes.QueryChannelResponse - msgs []sdk.Msg - ) - - srcUpdateMsg, err := src.UpdateClient(dst) - if err != nil { - return false, false, false, err - } - - dstUpdateMsg, err := dst.UpdateClient(src) - if err != nil { + if _, _, err := UpdateLightClients(src, dst); err != nil { return false, false, false, err } // 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 == "" { - success, modified, err := InitializeChannel(src, dst, srcUpdateMsg, dstUpdateMsg) + success, modified, err := InitializeChannel(src, dst) if err != nil { return false, false, false, err } @@ -107,7 +94,7 @@ func ExecuteChannelStep(src, dst *Chain) (success, last, modified bool, err erro } // Query Channel data from src and dst - srcChan, dstChan, err = QueryChannelPair(src, dst, int64(src.MustGetLatestLightHeight())-1, + srcChan, dstChan, err := QueryChannelPair(src, dst, int64(src.MustGetLatestLightHeight())-1, int64(dst.MustGetLatestLightHeight()-1)) if err != nil { return false, false, false, err @@ -123,16 +110,11 @@ func ExecuteChannelStep(src, dst *Chain) (success, last, modified bool, err erro logChannelStates(src, dst, srcChan, dstChan) } - openTry, err := src.ChanTry(dst) + msgs, err := src.ChanTry(dst) if err != nil { return false, false, false, err } - msgs = []sdk.Msg{ - srcUpdateMsg, - openTry, - } - _, success, err = src.SendMsgs(msgs) if !success { return false, false, false, err @@ -147,16 +129,11 @@ func ExecuteChannelStep(src, dst *Chain) (success, last, modified bool, err erro logChannelStates(src, dst, srcChan, dstChan) } - openAck, err := src.ChanAck(dst) + msgs, err := src.ChanAck(dst) if err != nil { return false, false, false, err } - msgs = []sdk.Msg{ - srcUpdateMsg, - openAck, - } - _, success, err = src.SendMsgs(msgs) if !success { return false, false, false, err @@ -170,16 +147,11 @@ func ExecuteChannelStep(src, dst *Chain) (success, last, modified bool, err erro logChannelStates(dst, src, dstChan, srcChan) } - openAck, err := dst.ChanAck(src) + msgs, err := dst.ChanAck(src) if err != nil { return false, false, false, err } - msgs = []sdk.Msg{ - dstUpdateMsg, - openAck, - } - _, success, err = dst.SendMsgs(msgs) if !success { return false, false, false, err @@ -191,10 +163,11 @@ func ExecuteChannelStep(src, dst *Chain) (success, last, modified bool, err erro logChannelStates(src, dst, srcChan, dstChan) } - msgs = []sdk.Msg{ - srcUpdateMsg, - src.ChanConfirm(dstChan), + msgs, err := src.ChanConfirm(dst) + if err != nil { + return false, false, false, err } + last = true _, success, err = src.SendMsgs(msgs) @@ -208,17 +181,18 @@ func ExecuteChannelStep(src, dst *Chain) (success, last, modified bool, err erro logChannelStates(dst, src, dstChan, srcChan) } - msgs = []sdk.Msg{ - dstUpdateMsg, - dst.ChanConfirm(srcChan), + msgs, err := dst.ChanConfirm(src) + if err != nil { + return false, false, false, err } - last = true _, success, err = dst.SendMsgs(msgs) if !success { return false, false, false, err } + last = true + case srcChan.Channel.State == chantypes.OPEN && dstChan.Channel.State == chantypes.OPEN: last = true @@ -231,9 +205,7 @@ func ExecuteChannelStep(src, dst *Chain) (success, last, modified bool, err erro // The identifiers set in the PathEnd's are used to determine which channel ends need to be // initialized. The PathEnds are updated upon a successful transaction. // NOTE: This function may need to be called twice if neither channel exists. -func InitializeChannel( - src, dst *Chain, srcUpdateMsg, dstUpdateMsg sdk.Msg, -) (success, modified bool, err error) { +func InitializeChannel(src, dst *Chain) (success, modified bool, err error) { switch { // OpenInit on source @@ -246,10 +218,9 @@ func InitializeChannel( channelID, found := FindMatchingChannel(src, dst) if !found { - // construct OpenInit message to be submitted on source chain - msgs := []sdk.Msg{ - srcUpdateMsg, - src.ChanInit(dst.PathEnd), + msgs, err := src.ChanInit(dst) + if err != nil { + return false, false, err } res, success, err := src.SendMsgs(msgs) @@ -279,15 +250,11 @@ func InitializeChannel( channelID, found := FindMatchingChannel(src, dst) if !found { // open try on source chain - openTry, err := src.ChanTry(dst) + msgs, err := src.ChanTry(dst) if err != nil { return false, false, err } - msgs := []sdk.Msg{ - srcUpdateMsg, - openTry, - } res, success, err := src.SendMsgs(msgs) if !success { return false, false, err @@ -315,15 +282,11 @@ func InitializeChannel( channelID, found := FindMatchingChannel(dst, src) if !found { // open try on destination chain - openTry, err := dst.ChanTry(src) + msgs, err := dst.ChanTry(src) if err != nil { return false, false, err } - msgs := []sdk.Msg{ - dstUpdateMsg, - openTry, - } res, success, err := dst.SendMsgs(msgs) if !success { return false, false, err diff --git a/relayer/msgs.go b/relayer/msgs.go index 64d19e183..cd2c61274 100644 --- a/relayer/msgs.go +++ b/relayer/msgs.go @@ -182,28 +182,40 @@ func (c *Chain) ConnConfirm(counterparty *Chain) ([]sdk.Msg, error) { } // ChanInit creates a MsgChannelOpenInit -func (c *Chain) ChanInit(counterparty *PathEnd) sdk.Msg { - return chantypes.NewMsgChannelOpenInit( +func (c *Chain) ChanInit(counterparty *Chain) ([]sdk.Msg, error) { + updateMsg, err := c.UpdateClient(counterparty) + if err != nil { + return nil, err + } + + msg := chantypes.NewMsgChannelOpenInit( c.PathEnd.PortID, c.PathEnd.Version, c.PathEnd.GetOrder(), []string{c.PathEnd.ConnectionID}, - counterparty.PortID, + counterparty.PathEnd.PortID, c.MustGetAddress(), // 'MustGetAddress' must be called directly before calling 'NewMsg...' ) + + return []sdk.Msg{updateMsg, msg}, nil } // ChanTry creates a MsgChannelOpenTry func (c *Chain) ChanTry( counterparty *Chain, -) (*chantypes.MsgChannelOpenTry, error) { +) ([]sdk.Msg, error) { + updateMsg, err := c.UpdateClient(counterparty) + if err != nil { + return nil, err + } + // NOTE: the proof height uses - 1 due to tendermint's delayed execution model counterpartyChannelRes, err := counterparty.QueryChannel(int64(counterparty.MustGetLatestLightHeight()) - 1) if err != nil { return nil, err } - return chantypes.NewMsgChannelOpenTry( + msg := chantypes.NewMsgChannelOpenTry( c.PathEnd.PortID, c.PathEnd.ChannelID, c.PathEnd.Version, @@ -216,20 +228,27 @@ func (c *Chain) ChanTry( counterpartyChannelRes.ProofHeight, c.MustGetAddress(), // 'MustGetAddress' must be called directly before calling 'NewMsg...' - ), nil + ) + + return []sdk.Msg{updateMsg, msg}, nil } // ChanAck creates a MsgChannelOpenAck func (c *Chain) ChanAck( counterparty *Chain, -) (*chantypes.MsgChannelOpenAck, error) { +) ([]sdk.Msg, error) { + updateMsg, err := c.UpdateClient(counterparty) + if err != nil { + return nil, err + } + // NOTE: the proof height uses - 1 due to tendermint's delayed execution model counterpartyChannelRes, err := counterparty.QueryChannel(int64(counterparty.MustGetLatestLightHeight()) - 1) if err != nil { return nil, err } - return chantypes.NewMsgChannelOpenAck( + msg := chantypes.NewMsgChannelOpenAck( c.PathEnd.PortID, c.PathEnd.ChannelID, counterparty.PathEnd.ChannelID, @@ -237,18 +256,33 @@ func (c *Chain) ChanAck( counterpartyChannelRes.Proof, counterpartyChannelRes.ProofHeight, c.MustGetAddress(), // 'MustGetAddress' must be called directly before calling 'NewMsg...' - ), nil + ) + + return []sdk.Msg{updateMsg, msg}, nil } // ChanConfirm creates a MsgChannelOpenConfirm -func (c *Chain) ChanConfirm(dstChanState *chantypes.QueryChannelResponse) sdk.Msg { - return chantypes.NewMsgChannelOpenConfirm( +func (c *Chain) ChanConfirm(counterparty *Chain) ([]sdk.Msg, error) { + updateMsg, err := c.UpdateClient(counterparty) + if err != nil { + return nil, err + } + + // NOTE: the proof height uses - 1 due to tendermint's delayed execution model + counterpartyChanState, err := counterparty.QueryChannel(int64(counterparty.MustGetLatestLightHeight()) - 1) + if err != nil { + return nil, err + } + + msg := chantypes.NewMsgChannelOpenConfirm( c.PathEnd.PortID, c.PathEnd.ChannelID, - dstChanState.Proof, - dstChanState.ProofHeight, + counterpartyChanState.Proof, + counterpartyChanState.ProofHeight, c.MustGetAddress(), // 'MustGetAddress' must be called directly before calling 'NewMsg...' ) + + return []sdk.Msg{updateMsg, msg}, nil } // ChanCloseInit creates a MsgChannelCloseInit From 494f3a6a3f736baff1fd30c3431f922484a99906 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Thu, 18 Feb 2021 14:35:16 +0100 Subject: [PATCH 3/5] fix chain id for timeout --- relayer/relayPackets.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/relayer/relayPackets.go b/relayer/relayPackets.go index 6b29b6654..ee7b40c66 100644 --- a/relayer/relayPackets.go +++ b/relayer/relayPackets.go @@ -76,7 +76,7 @@ func (rp *relayMsgTimeout) Msg(src, dst *Chain) (sdk.Msg, error) { if rp.dstRecvRes == nil { return nil, fmt.Errorf("timeout packet [%s]seq{%d} has no associated proofs", src.ChainID, rp.seq) } - version := clienttypes.ParseChainID(dst.PathEnd.ChainID) + version := clienttypes.ParseChainID(dst.ChainID) msg := chantypes.NewMsgTimeout( chantypes.NewPacket( rp.packetData, @@ -166,7 +166,7 @@ func (rp *relayMsgRecvPacket) Msg(src, dst *Chain) (sdk.Msg, error) { if rp.dstComRes == nil { return nil, fmt.Errorf("receive packet [%s]seq{%d} has no associated proofs", src.ChainID, rp.seq) } - version := clienttypes.ParseChainID(src.PathEnd.ChainID) + version := clienttypes.ParseChainID(src.ChainID) packet := chantypes.NewPacket( rp.packetData, rp.seq, @@ -211,7 +211,7 @@ func (rp *relayMsgPacketAck) Msg(src, dst *Chain) (sdk.Msg, error) { if rp.dstComRes == nil { return nil, fmt.Errorf("ack packet [%s]seq{%d} has no associated proofs", src.ChainID, rp.seq) } - version := clienttypes.ParseChainID(dst.PathEnd.ChainID) + version := clienttypes.ParseChainID(dst.ChainID) msg := chantypes.NewMsgAcknowledgement( chantypes.NewPacket( rp.packetData, From b070e400201f75fbe68a4532c07eec816628210d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Colin=20Axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Thu, 18 Feb 2021 14:44:02 +0100 Subject: [PATCH 4/5] fix build --- cmd/raw.go | 100 +++++++++++++---------------------------------------- 1 file changed, 24 insertions(+), 76 deletions(-) diff --git a/cmd/raw.go b/cmd/raw.go index 0d0306ca4..6e7e9cd83 100644 --- a/cmd/raw.go +++ b/cmd/raw.go @@ -152,7 +152,12 @@ $ %s tx raw conn-init ibc-0 ibc-1 ibczeroclient ibconeclient ibcconn1 ibcconn2`, return err } - return sendAndPrint([]sdk.Msg{chains[src].ConnInit(chains[dst].PathEnd)}, + msgs, err := chains[src].ConnInit(chains[dst]) + if err != nil { + return err + } + + return sendAndPrint(msgs, chains[src], cmd) }, } @@ -182,22 +187,12 @@ $ %s tx raw conn-try ibc-0 ibc-1 ibczeroclient ibconeclient ibcconn1 ibcconn2`, return err } - updateMsg, err := chains[src].UpdateClient(chains[dst]) - if err != nil { - return err - } - - openTry, err := chains[src].ConnTry(chains[dst]) + msgs, err := chains[src].ConnTry(chains[dst]) if err != nil { return err } - txs := []sdk.Msg{ - updateMsg, - openTry, - } - - return sendAndPrint(txs, chains[src], cmd) + return sendAndPrint(msgs, chains[src], cmd) }, } return cmd @@ -226,22 +221,12 @@ $ %s tx raw conn-ack ibc-0 ibc-1 ibconeclient ibczeroclient ibcconn1 ibcconn2`, return err } - updateMsg, err := chains[src].UpdateClient(chains[dst]) + msgs, err := chains[src].ConnAck(chains[dst]) if err != nil { return err } - openAck, err := chains[src].ConnAck(chains[dst]) - if err != nil { - return err - } - - txs := []sdk.Msg{ - updateMsg, - openAck, - } - - return sendAndPrint(txs, chains[src], cmd) + return sendAndPrint(msgs, chains[src], cmd) }, } return cmd @@ -270,24 +255,12 @@ $ %s tx raw conn-confirm ibc-0 ibc-1 ibczeroclient ibconeclient ibcconn1 ibcconn return err } - updateMsg, err := chains[src].UpdateClient(chains[dst]) + msgs, err := chains[src].ConnConfirm(chains[dst]) if err != nil { return err } - // NOTE: We query connection at height - 1 because of the way tendermint returns - // proofs the commit for height n is contained in the header of height n + 1 - dstState, err := chains[dst].QueryConnection(int64(chains[dst].MustGetLatestLightHeight()) - 1) - if err != nil { - return err - } - - txs := []sdk.Msg{ - updateMsg, - chains[src].ConnConfirm(dstState), - } - - return sendAndPrint(txs, chains[src], cmd) + return sendAndPrint(msgs, chains[src], cmd) }, } return cmd @@ -360,7 +333,12 @@ ibcconn1 ibcconn2 ibcchan1 ibcchan2 transfer transfer ordered`, appName)), return err } - return sendAndPrint([]sdk.Msg{chains[src].ChanInit(chains[dst].PathEnd)}, + msgs, err := chains[src].ChanInit(chains[dst]) + if err != nil { + return err + } + + return sendAndPrint(msgs, chains[src], cmd) }, } @@ -391,22 +369,12 @@ $ %s tx raw chan-try ibc-0 ibc-1 ibczeroclient ibcconn0 ibcchan1 ibcchan2 transf return err } - updateMsg, err := chains[src].UpdateClient(chains[dst]) + msgs, err := chains[src].ChanTry(chains[dst]) if err != nil { return err } - openTry, err := chains[src].ChanTry(chains[dst]) - if err != nil { - return err - } - - txs := []sdk.Msg{ - updateMsg, - openTry, - } - - return sendAndPrint(txs, chains[src], cmd) + return sendAndPrint(msgs, chains[src], cmd) }, } return cmd @@ -437,22 +405,12 @@ $ %s tx raw chan-ack ibc-0 ibc-1 ibczeroclient ibcchan1 ibcchan2 transfer transf return err } - updateMsg, err := chains[src].UpdateClient(chains[dst]) - if err != nil { - return err - } - - openAck, err := chains[src].ChanAck(chains[dst]) + msgs, err := chains[src].ChanAck(chains[dst]) if err != nil { return err } - txs := []sdk.Msg{ - updateMsg, - openAck, - } - - return sendAndPrint(txs, chains[src], cmd) + return sendAndPrint(msgs, chains[src], cmd) }, } return cmd @@ -482,22 +440,12 @@ $ %s tx raw chan-confirm ibc-0 ibc-1 ibczeroclient ibcchan1 ibcchan2 transfer tr return err } - updateMsg, err := chains[src].UpdateClient(chains[dst]) + msgs, err := chains[src].ChanConfirm(chains[dst]) if err != nil { return err } - dstChanState, err := chains[dst].QueryChannel(int64(chains[dst].MustGetLatestLightHeight()) - 1) - if err != nil { - return err - } - - txs := []sdk.Msg{ - updateMsg, - chains[src].ChanConfirm(dstChanState), - } - - return sendAndPrint(txs, chains[src], cmd) + return sendAndPrint(msgs, chains[src], cmd) }, } return cmd From 40c694962577254af81dbb8954accd1098296709 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?colin=20axn=C3=A9r?= <25233464+colin-axner@users.noreply.github.com> Date: Tue, 9 Mar 2021 15:55:46 +0100 Subject: [PATCH 5/5] refactor state based relaying (fixes update client bug on retries) (#435) * bump sdk version to v0.41.3 (#430) * bump sdk version * bump SDK to v0.41.3 * inital work for refactoring state based relaying * Modify relayPacketFromSequence * update tendermint client to not prune light blocks (#437) * Address comments and fix lint issues * Fix lint issues * Remove onRtyErr (lint issue) * typo fix (#438) * disable tm pruning (#441) * update release naming (#442) * Implement swagger docs and fix path validation (#434) * Add swagger setup * Add some routes docs and swagger ui * Add few more route docs * Add swagger docs for remaining routes * Fix golint issues * Fix unused lint issues * check chain-id in AddChain * add a light client database lock (#447) Add a lock to prevent multiple processes from attempting to access the light client database at the same time. This typically resulted in unnecessary errors or even panics * Close database connection even if second error triggers (#449) Co-authored-by: Mark * address comments Co-authored-by: akhilkumarpilli Co-authored-by: Afanti Co-authored-by: Akhil Kumar P <36399231+akhilkumarpilli@users.noreply.github.com> Co-authored-by: Mark | Microtick <409166+mark-microtick@users.noreply.github.com> Co-authored-by: Mark --- relayer/msgs.go | 193 ++++++++++++++++++++++++++++++++++++++ relayer/naive-strategy.go | 110 +++++++++------------- 2 files changed, 239 insertions(+), 64 deletions(-) diff --git a/relayer/msgs.go b/relayer/msgs.go index cd2c61274..a6a7297bd 100644 --- a/relayer/msgs.go +++ b/relayer/msgs.go @@ -1,6 +1,9 @@ package relayer import ( + "fmt" + + retry "github.com/avast/retry-go" sdk "github.com/cosmos/cosmos-sdk/types" transfertypes "github.com/cosmos/cosmos-sdk/x/ibc/applications/transfer/types" clienttypes "github.com/cosmos/cosmos-sdk/x/ibc/core/02-client/types" @@ -320,3 +323,193 @@ func (c *Chain) MsgTransfer(dst *PathEnd, amount sdk.Coin, dstAddr string, timeoutTimestamp, ) } + +// MsgRelayRecvPacket constructs the MsgRecvPacket which is to be sent to the receiving chain. +// The counterparty represents the sending chain where the packet commitment would be stored. +func (c *Chain) MsgRelayRecvPacket(counterparty *Chain, packet *relayMsgRecvPacket) (msgs []sdk.Msg, err error) { + var comRes *chantypes.QueryPacketCommitmentResponse + if err = retry.Do(func() (err error) { + // NOTE: the proof height uses - 1 due to tendermint's delayed execution model + comRes, err = counterparty.QueryPacketCommitment(int64(counterparty.MustGetLatestLightHeight())-1, packet.seq) + if err != nil { + return err + } + + if comRes.Proof == nil || comRes.Commitment == nil { + return fmt.Errorf("recv packet commitment query seq(%d) is nil", packet.seq) + } + + return nil + }, rtyAtt, rtyDel, rtyErr, retry.OnRetry(func(n uint, _ error) { + // clear messages + msgs = []sdk.Msg{} + + // OnRetry we want to update the light clients and then debug log + updateMsg, err := c.UpdateClient(counterparty) + if err != nil { + return + } + + msgs = append(msgs, updateMsg) + + if counterparty.debug { + counterparty.Log(fmt.Sprintf("- [%s]@{%d} - try(%d/%d) query packet commitment: %s", + counterparty.ChainID, counterparty.MustGetLatestLightHeight()-1, n+1, rtyAttNum, err)) + } + + })); err != nil { + counterparty.Error(err) + return + } + + if comRes == nil { + return nil, fmt.Errorf("receive packet [%s]seq{%d} has no associated proofs", c.ChainID, packet.seq) + } + + version := clienttypes.ParseChainID(c.ChainID) + msg := chantypes.NewMsgRecvPacket( + chantypes.NewPacket( + packet.packetData, + packet.seq, + counterparty.PathEnd.PortID, + counterparty.PathEnd.ChannelID, + c.PathEnd.PortID, + c.PathEnd.ChannelID, + clienttypes.NewHeight(version, packet.timeout), + packet.timeoutStamp, + ), + comRes.Proof, + comRes.ProofHeight, + c.MustGetAddress(), + ) + + return append(msgs, msg), nil +} + +// MsgRelayAcknowledgement constructs the MsgAcknowledgement which is to be sent to the sending chain. +// The counterparty represents the receiving chain where the acknowledgement would be stored. +func (c *Chain) MsgRelayAcknowledgement(counterparty *Chain, packet *relayMsgPacketAck) (msgs []sdk.Msg, err error) { + var ackRes *chantypes.QueryPacketAcknowledgementResponse + if err = retry.Do(func() (err error) { + // NOTE: the proof height uses - 1 due to tendermint's delayed execution model + ackRes, err = counterparty.QueryPacketAcknowledgement(int64(counterparty.MustGetLatestLightHeight())-1, packet.seq) + if err != nil { + return err + } + + if ackRes.Proof == nil || ackRes.Acknowledgement == nil { + return fmt.Errorf("ack packet acknowledgement query seq(%d) is nil", packet.seq) + } + + return nil + }, rtyAtt, rtyDel, rtyErr, retry.OnRetry(func(n uint, _ error) { + // clear messages + msgs = []sdk.Msg{} + + // OnRetry we want to update the light clients and then debug log + updateMsg, err := c.UpdateClient(counterparty) + if err != nil { + return + } + + msgs = append(msgs, updateMsg) + + if counterparty.debug { + counterparty.Log(fmt.Sprintf("- [%s]@{%d} - try(%d/%d) query packet acknowledgement: %s", + counterparty.ChainID, counterparty.MustGetLatestLightHeight()-1, n+1, rtyAttNum, err)) + } + + })); err != nil { + counterparty.Error(err) + return + } + + if ackRes == nil { + return nil, fmt.Errorf("ack packet [%s]seq{%d} has no associated proofs", counterparty.ChainID, packet.seq) + } + + version := clienttypes.ParseChainID(counterparty.ChainID) + msg := chantypes.NewMsgAcknowledgement( + chantypes.NewPacket( + packet.packetData, + packet.seq, + c.PathEnd.PortID, + c.PathEnd.ChannelID, + counterparty.PathEnd.PortID, + counterparty.PathEnd.ChannelID, + clienttypes.NewHeight(version, packet.timeout), + packet.timeoutStamp, + ), + packet.ack, + ackRes.Proof, + ackRes.ProofHeight, + c.MustGetAddress(), + ) + + return append(msgs, msg), nil +} + +// MsgRelayTimeout constructs the MsgTimeout which is to be sent to the sending chain. +// The counterparty represents the receiving chain where the receipts would have been +// stored. +func (c *Chain) MsgRelayTimeout(counterparty *Chain, packet *relayMsgTimeout) (msgs []sdk.Msg, err error) { + var recvRes *chantypes.QueryPacketReceiptResponse + if err = retry.Do(func() (err error) { + // NOTE: Timeouts currently only work with ORDERED channels for nwo + // NOTE: the proof height uses - 1 due to tendermint's delayed execution model + recvRes, err = counterparty.QueryPacketReceipt(int64(counterparty.MustGetLatestLightHeight())-1, packet.seq) + if err != nil { + return err + } + + if recvRes.Proof == nil { + return fmt.Errorf("timeout packet receipt proof seq(%d) is nil", packet.seq) + } + + return nil + }, rtyAtt, rtyDel, rtyErr, retry.OnRetry(func(n uint, _ error) { + // clear messages + msgs = []sdk.Msg{} + + // OnRetry we want to update the light clients and then debug log + updateMsg, err := c.UpdateClient(counterparty) + if err != nil { + return + } + + msgs = append(msgs, updateMsg) + + if counterparty.debug { + counterparty.Log(fmt.Sprintf("- [%s]@{%d} - try(%d/%d) query packet receipt: %s", + counterparty.ChainID, counterparty.MustGetLatestLightHeight()-1, n+1, rtyAttNum, err)) + } + + })); err != nil { + counterparty.Error(err) + return + } + + if recvRes == nil { + return nil, fmt.Errorf("timeout packet [%s]seq{%d} has no associated proofs", c.ChainID, packet.seq) + } + + version := clienttypes.ParseChainID(counterparty.ChainID) + msg := chantypes.NewMsgTimeout( + chantypes.NewPacket( + packet.packetData, + packet.seq, + c.PathEnd.PortID, + c.PathEnd.ChannelID, + counterparty.PathEnd.PortID, + counterparty.PathEnd.ChannelID, + clienttypes.NewHeight(version, packet.timeout), + packet.timeoutStamp, + ), + packet.seq, + recvRes.Proof, + recvRes.ProofHeight, + c.MustGetAddress(), + ) + + return append(msgs, msg), nil +} diff --git a/relayer/naive-strategy.go b/relayer/naive-strategy.go index 59372d5bb..b33106154 100644 --- a/relayer/naive-strategy.go +++ b/relayer/naive-strategy.go @@ -420,31 +420,23 @@ func (nrs *NaiveStrategy) RelayAcknowledgements(src, dst *Chain, sp *RelaySequen // add messages for sequences on src for _, seq := range sp.Src { // SRC wrote ack, so we query packet and send to DST - pkt, err := acknowledgementFromSequence(src, dst, seq) + relayAckMsgs, err := acknowledgementFromSequence(src, dst, seq) if err != nil { return err } - msg, err := pkt.Msg(dst, src) - if err != nil { - return err - } - msgs.Dst = append(msgs.Dst, msg) + msgs.Dst = append(msgs.Dst, relayAckMsgs...) } // add messages for sequences on dst for _, seq := range sp.Dst { // DST wrote ack, so we query packet and send to SRC - pkt, err := acknowledgementFromSequence(dst, src, seq) + relayAckMsgs, err := acknowledgementFromSequence(dst, src, seq) if err != nil { return err } - msg, err := pkt.Msg(src, dst) - if err != nil { - return err - } - msgs.Src = append(msgs.Src, msg) + msgs.Src = append(msgs.Src, relayAckMsgs...) } if !msgs.Ready() { @@ -492,56 +484,38 @@ func (nrs *NaiveStrategy) RelayPackets(src, dst *Chain, sp *RelaySequences) erro // add messages for sequences on src for _, seq := range sp.Src { // Query src for the sequence number to get type of packet - pkt, err := relayPacketFromSequence(src, dst, seq) + recvMsgs, timeoutMsgs, err := relayPacketFromSequence(src, dst, seq) if err != nil { return err } // depending on the type of message to be relayed, we need to // send to different chains - switch pkt.(type) { - case *relayMsgRecvPacket: - msg, err := pkt.Msg(dst, src) - if err != nil { - return err - } - msgs.Dst = append(msgs.Dst, msg) - case *relayMsgTimeout: - msg, err := pkt.Msg(src, dst) - if err != nil { - return err - } - msgs.Src = append(msgs.Src, msg) - default: - return fmt.Errorf("%T packet types not supported", pkt) + if recvMsgs != nil { + msgs.Dst = append(msgs.Dst, recvMsgs...) + } + + if timeoutMsgs != nil { + msgs.Src = append(msgs.Src, timeoutMsgs...) } } // add messages for sequences on dst for _, seq := range sp.Dst { // Query dst for the sequence number to get type of packet - pkt, err := relayPacketFromSequence(dst, src, seq) + recvMsgs, timeoutMsgs, err := relayPacketFromSequence(dst, src, seq) if err != nil { return err } // depending on the type of message to be relayed, we need to // send to different chains - switch pkt.(type) { - case *relayMsgRecvPacket: - msg, err := pkt.Msg(src, dst) - if err != nil { - return err - } - msgs.Src = append(msgs.Src, msg) - case *relayMsgTimeout: - msg, err := pkt.Msg(dst, src) - if err != nil { - return err - } - msgs.Dst = append(msgs.Dst, msg) - default: - return fmt.Errorf("%T packet types not supported", pkt) + if recvMsgs != nil { + msgs.Src = append(msgs.Src, recvMsgs...) + } + + if timeoutMsgs != nil { + msgs.Dst = append(msgs.Dst, timeoutMsgs...) } } @@ -583,54 +557,60 @@ func (nrs *NaiveStrategy) RelayPackets(src, dst *Chain, sp *RelaySequences) erro return nil } -// relayPacketFromSequence returns a sdk.Msg to relay a packet with a given seq on src -func relayPacketFromSequence(src, dst *Chain, seq uint64) (relayPacket, error) { +// relayPacketFromSequence relays a packet with a given seq on src +// and returns recvPacket msgs, timeoutPacketmsgs and error +func relayPacketFromSequence(src, dst *Chain, seq uint64) ([]sdk.Msg, []sdk.Msg, error) { txs, err := src.QueryTxs(src.MustGetLatestLightHeight(), 1, 1000, rcvPacketQuery(src.PathEnd.ChannelID, int(seq))) switch { case err != nil: - return nil, err + return nil, nil, err case len(txs) == 0: - return nil, fmt.Errorf("no transactions returned with query") + return nil, nil, fmt.Errorf("no transactions returned with query") case len(txs) > 1: - return nil, fmt.Errorf("more than one transaction returned with query") + return nil, nil, fmt.Errorf("more than one transaction returned with query") } rcvPackets, timeoutPackets, err := relayPacketsFromResultTx(src, dst, txs[0]) switch { case err != nil: - return nil, err + return nil, nil, err case len(rcvPackets) == 0 && len(timeoutPackets) == 0: - return nil, fmt.Errorf("no relay msgs created from query response") + return nil, nil, fmt.Errorf("no relay msgs created from query response") case len(rcvPackets)+len(timeoutPackets) > 1: - return nil, fmt.Errorf("more than one relay msg found in tx query") + return nil, nil, fmt.Errorf("more than one relay msg found in tx query") } if len(rcvPackets) == 1 { pkt := rcvPackets[0] if seq != pkt.Seq() { - return nil, fmt.Errorf("wrong sequence: expected(%d) got(%d)", seq, pkt.Seq()) + return nil, nil, fmt.Errorf("wrong sequence: expected(%d) got(%d)", seq, pkt.Seq()) } - if err = pkt.FetchCommitResponse(dst, src); err != nil { - return nil, err + + msgs, err := dst.MsgRelayRecvPacket(src, pkt.(*relayMsgRecvPacket)) + if err != nil { + return nil, nil, err } - return pkt, nil + return msgs, nil, nil } if len(timeoutPackets) == 1 { pkt := timeoutPackets[0] if seq != pkt.Seq() { - return nil, fmt.Errorf("wrong sequence: expected(%d) got(%d)", seq, pkt.Seq()) + return nil, nil, fmt.Errorf("wrong sequence: expected(%d) got(%d)", seq, pkt.Seq()) } - if err = pkt.FetchCommitResponse(src, dst); err != nil { - return nil, err + + msgs, err := src.MsgRelayTimeout(dst, pkt.(*relayMsgTimeout)) + if err != nil { + return nil, nil, err } - return pkt, nil + return nil, msgs, nil } - return nil, fmt.Errorf("should have errored before here") + return nil, nil, fmt.Errorf("should have errored before here") } -func acknowledgementFromSequence(src, dst *Chain, seq uint64) (relayPacket, error) { +// source is the sending chain, destination is the receiving chain +func acknowledgementFromSequence(src, dst *Chain, seq uint64) ([]sdk.Msg, error) { txs, err := src.QueryTxs(src.MustGetLatestLightHeight(), 1, 1000, ackPacketQuery(src.PathEnd.ChannelID, int(seq))) switch { case err != nil: @@ -655,10 +635,12 @@ func acknowledgementFromSequence(src, dst *Chain, seq uint64) (relayPacket, erro if seq != pkt.Seq() { return nil, fmt.Errorf("wrong sequence: expected(%d) got(%d)", seq, pkt.Seq()) } - if err = pkt.FetchCommitResponse(dst, src); err != nil { + + msgs, err := src.MsgRelayAcknowledgement(dst, pkt) + if err != nil { return nil, err } - return pkt, nil + return msgs, nil } // relayPacketsFromResultTx looks through the events in a *ctypes.ResultTx