From 80e4f39802f9153351da02aad333417380d012b2 Mon Sep 17 00:00:00 2001 From: Zarko Milosevic Date: Wed, 16 Sep 2020 15:45:40 +0200 Subject: [PATCH 1/9] Add initial error handling logic in relayer main loop --- docs/spec/relayer/Packets.md | 109 +++++++++++++++++++++++++-------- docs/spec/relayer/Relayer.md | 114 ++++++++++++----------------------- 2 files changed, 122 insertions(+), 101 deletions(-) diff --git a/docs/spec/relayer/Packets.md b/docs/spec/relayer/Packets.md index 1de2ff1f79..48e90396e7 100644 --- a/docs/spec/relayer/Packets.md +++ b/docs/spec/relayer/Packets.md @@ -117,6 +117,13 @@ GetClientState(chain Chain, clientId Identifier, proofHeight Height) (ClientState, CommitmentProof) +// Returns consensus state with a commitment proof. +GetConsensusState(chain Chain, + clientId Identifier, + targetHeight Height, + proofHeight Height) (ConsensusState, CommitmentProof) + + // Returns packet commitment with a commitment proof. GetPacketCommitment(chain Chain, portId Identifier, @@ -147,48 +154,96 @@ GetCurrentTimestamp(chainB) uint64 For functions that return proof, if proof != nil, then the returned value is being verified. The value is being verified using the header's app hash that is provided by the corresponding light client. + +### Error handling + +Helper functions listed above assume querying (parts of the) application state using Tendermint RPC. For example, +GetChannel relies on QueryChannel. RPC calls can fail as: + +- no response is received within some timeout (TimeoutError) or +- malformed response is received (SerializationError). + +In both cases, error handling logic should be defined by the caller. For example, in the former case, the caller might +retry to send the same request to a same provider (full node), while in the latter case the request might be sent to +some other provider node. Although these kinds of errors could be due to network infrastructure issues, it is normally +simpler to blame the provider (assume implicitly network is always correct and reliable). Therefore, correct provider +always respond timely with a correct response, while in case of errors we consider the provider node faulty, and then +we replace it with a different node. + +```go +type Chain { + chainID string + clientID Identifier + peerList List> + provider Pair + lc LightClient +} +``` + We now show the pseudocode for one of those functions: ```go -GetChannel(chain Chain, +func GetChannel(chain Chain, portId Identifier, channelId Identifier, proofHeight Height) (ChannelEnd, CommitmentProof) { - // Query provable store exposed by the full node of chain. - // The path for the channel end is at channelEnds/ports/{portId}/channels/{channelId}". - // The membership proof returned is read at height proofHeight. - channel, proof = QueryChannel(chain, portId, channelId, proofHeight) - if proof == nil return { (nil, nil) } - - header = GetHeader(chain.lc, proofHeight) // get header for height proofHeight using light client of the given chain + while(true) { + // Query provable store exposed by the full node of chain. + // The path for the channel end is at channelEnds/ports/{portId}/channels/{channelId}". + // The membership proof returned is read at height proofHeight. + channel, proof, error = QueryChannel(chain.provider, portId, channelId, proofHeight) + if error != nil { + // elect a new provider from the peer list + if !ReplaceProvider(chain) { return (nil, nil) } // return if fail to elect new provider + } - // verify membership of the channel at path channelEnds/ports/{portId}/channels/{channelId} using - // the root hash header.AppHash - if verifyMembership(header.AppHash, proofHeight, proof, channelPath(portId, channelId), channel) { - return channel, proof - } else { return (nil, nil) } + header, error = GetHeader(chain.lc, proofHeight) // get header for height proofHeight using light client + if error != nil { return (nil, nil) } // return if light client can't provide header for the given height + + // verify membership of the channel at path channelEnds/ports/{portId}/channels/{channelId} using + // the root hash header.AppHash + if verifyMembership(header.AppHash, proofHeight, proof, channelPath(portId, channelId), channel) { + return channel, proof + } + + // membership check fails; therefore provider is faulty. Try to elect new provider + if !ReplaceProvider(chain) { return (nil, nil) } // if fails to elect new provider return + } + panic // should never reach this line +} + +// Simplified version of logic for electing new provider. In reality it will probably involve opening a connection to +// a newply elected provider and closing connection with an old provider. +func ReplaceProvider(chain Chain) boolean { + if chain.peerList.IsEmpty() return false + chain.provider = Head(chain.peerList) + chain.peerList = Tail(chain.peerList) + return true } ``` If LATEST_HEIGHT is passed as a parameter, the data should be read (and the corresponding proof created) at the most recent height. - ## Computing destination chain ```golang -func GetDestinationInfo(ev IBCEvent, chainA Chain) Chain { +func GetDestinationInfo(ev IBCEvent, chain Chain) Chain { switch ev.type { case SendPacketEvent: channel, proof = GetChannel(chain, ev.sourcePort, ev.sourceChannel, ev.Height) - if proof == nil return nil + if proof == nil { return nil } connectionId = channel.connectionHops[0] connection, proof = GetConnection(chain, connectionId, ev.Height) - if proof == nil return nil + if proof == nil { return nil } - clientState = GetClientState(chain, connection.clientIdentifier, ev.Height) - return getHostInfo(clientState.chainID) + clientState, proof = GetClientState(chain, connection.clientIdentifier, ev.Height) + if proof == nil { return nil } + + // We assume that the relayer maintains a map of known chainIDs and the corresponding chains. + // Note that in a normal case, relayer should be aware of chains between it relays packets + return getChain(clientState.chainID) ... } } @@ -209,25 +264,26 @@ func createPacketRecvDatagram(ev SendPacketEvent, chainA Chain, chainB Chain, in GetPacketCommitment(chainA, ev.sourcePort, ev.sourceChannel, ev.sequence, proofHeight) if packetCommitmentProof != nil { return nil } - if packetCommitment == null OR + if packetCommitment == nil OR packetCommitment != hash(concat(ev.data, ev.timeoutHeight, ev.timeoutTimestamp)) { return nil } // Stage 2 // Execute checks IBC handler on chainB will execute channel, proof = GetChannel(chainB, ev.destPort, ev.destChannel, LATEST_HEIGHT) - if proof != nil { return nil } + if proof == nil { return nil } - if channel == null OR + // TODO: not necessarily fatal error as optimistic packet send might be taking place + if channel == nil OR channel.state != OPEN OR ev.sourcePort != channel.counterpartyPortIdentifier OR ev.sourceChannel != channel.counterpartyChannelIdentifier { return nil } connectionId = channel.connectionHops[0] connection, proof = GetConnection(chainB, connectionId, LATEST_HEIGHT) - if proof != nil { return nil } + if proof == nil { return nil } - if connection == null OR connection.state != OPEN { return nil } + if connection == nil OR connection.state != OPEN { return nil } if ev.timeoutHeight != 0 AND GetConsensusHeight(chainB) >= ev.timeoutHeight { return nil } if ev.timeoutTimestamp != 0 AND GetCurrentTimestamp(chainB) >= ev.timeoutTimestamp { return nil } @@ -235,14 +291,15 @@ func createPacketRecvDatagram(ev SendPacketEvent, chainA Chain, chainB Chain, in // we now check if this packet is already received by the destination chain if (channel.ordering === ORDERED) { nextSequenceRecv, proof = GetNextSequenceRecv(chainB, ev.destPort, ev.destChannel, LATEST_HEIGHT) - if proof != nil { return nil } + if proof == nil { return nil } if ev.sequence != nextSequenceRecv { return nil } // packet has already been delivered by another relayer } else { + // TODO: Can be proof of absence also and we should be able to verify it. packetAcknowledgement, proof = GetPacketAcknowledgement(chainB, ev.destPort, ev.destChannel, ev.sequence, LATEST_HEIGHT) - if proof != nil { return nil } + if proof == nil { return nil } if packetAcknowledgement != nil { return nil } } diff --git a/docs/spec/relayer/Relayer.md b/docs/spec/relayer/Relayer.md index c8fc6e315a..835f9bb858 100644 --- a/docs/spec/relayer/Relayer.md +++ b/docs/spec/relayer/Relayer.md @@ -27,9 +27,10 @@ then m was sent by the module A to the module B. ## Data Types ```go -type ClientState struct { - Height Height - SignedHeader SignedHeader +type ConsensusState { + timestamp uint64 + validatorSet List> + commitmentRoot []byte } ``` @@ -49,7 +50,7 @@ We assume the existence of the following helper functions: // We assume that this function handles non-responsive full node error by switching to a different full node. queryClientConsensusState(chainA, targetHeight) (ClientState, MembershipProof) verifyClientStateProof(clientStateAonB, membershipProof, sh.appHash) boolean -pendingDatagrams(height, chainA, chainB) IBCDatagram[] +createDatagrams(height, chainA, chainB, installedHeight) IBCDatagram[] verifyProof(datagrams, sh.appHash) boolean createUpdateClientDatagrams(shs) IBCDatagram[] submit(datagrams) error @@ -60,8 +61,8 @@ The main relayer event loop is a pipeline of three stages. Assuming some IBC eve the relayer: 1. Updates (on `chainB`) the IBC client for `chainA` to a certain height `H` where `H >= h+1`. -2. Create IBC datagrams at height `H-1`. -3. Submit the datagrams from stage (2) to `chainB`. +2. Create IBC datagram at height `H-1`. +3. Submit the datagram from stage (2) to `chainB`. Note that an IBC event at height `h` corresponds to the modifications to the data store made as part of executing block at height `h`. The corresponding proof (that data is indeed written to the data store) can be verified using @@ -69,18 +70,27 @@ the data store root hash that is part of the header at height `h+1`. Once stage 1 finishes correctly, stage 2 should succeed assuming that `chainB` has not already processed the event. The interface between stage 1 and stage 2 is just the height `H`. Once stage 2 finishes correctly, stage 3 should -succeed. The interface between stage 2 and stage 3 is a set of datagrams. +succeed. The interface between stage 2 and stage 3 is an IBC datagram. -We assume that the corresponding light client is correctly installed on each chain. +We assume that the corresponding light client is correctly installed on each chain. ```golang -func handleEvent(ev, chainA) { +enum Error { + INIT, + TRYOPEN, + OPEN, +} +``` + +```golang +func handleEvent(ev, chainA) Error { // NOTE: we don't verify if event data are valid at this point. We trust full node we are connected to // until some verification fails. Otherwise, we can have Stage 2 (datagram creation being done first). // Stage 1. // Determine destination chain - chainB = GetDestinationInfo(ev, chainA) + chainB = GetDestinationInfo(ev, chainA) + if chainB == nil { return } // TODO: return correct error type // Stage 2. // Update on `chainB` the IBC client for `chainA` to height `>= targetHeight`. @@ -88,7 +98,7 @@ func handleEvent(ev, chainA) { // See the code for `updateIBCClient` below. installedHeight, error := updateIBCClient(chainB, chainA, targetHeight) if error != nil { - return error + return Error } // Stage 3. @@ -98,7 +108,8 @@ func handleEvent(ev, chainA) { // Stage 4. // Submit datagrams. if datagram != nil { - chainB.submit(datagram) + error = chainB.submit(datagram) + if error != nil { return Error } } } @@ -109,67 +120,27 @@ func handleEvent(ev, chainA) { // Postconditions: // - returns the installedHeight >= targetHeight // - return error if verification of client state fails -func updateIBCClient(dest, src, targetHeight) -> {installedHeight, error} { +func updateIBCClient(dest Chain, src Chain, targetHeight Height) -> (Height, Error) { - while (true) { - // Check if targetHeight exists already on destination chain. - // Query state of IBC client for `src` on chain `dest`. - clientState, membershipProof = dest.queryClientConsensusState(src, targetHeight) - // NOTE: What if a full node we are connected to send us stale (but correct) information regarding targetHeight? - - // Verify the result of the query - sh = dest.lc.get_header(membershipProof.Height + 1) - // NOTE: Headers we obtain from the light client are trusted. - if verifyClientStateProof(clientState, membershipProof, sh.appHash) { - break; - } - replaceFullNode(dst) - } + clientState, proof = GetClientState(dest, dest.clientId, LATEST_HEIGHT) + if proof == nil { return (nil, Error) } + // NOTE: What if a full node we are connected to send us stale (but correct) information regarding targetHeight? - // At this point we know that clientState is indeed part of the state on dest chain. - // Verify if installed header is equal to the header obtained the from the local client - // at the same height. - if !src.lc.get_header(clientState.Height) == clientState.SignedHeader.Header { - // We know at this point that conflicting header is installed at the dst chain. - // We need to create proof of fork and submit it to src chain and to dst chain so light client is frozen. - src.lc.createAndSubmitProofOfFork(dst, clientState) - return {nil, error} - } - - while (clientState.Height < targetHeight) { - // Installed height is smaller than the target height. + // if installed height is smaller than the targetHeight, we need to update client with targetHeight + while (clientState.latestHeight < targetHeight) { // Do an update to IBC client for `src` on `dest`. - shs = src.lc.get_minimal_set(clientState.Height, targetHeight) - // Blocking call. Wait until transaction is committed to the dest chain. - dest.submit(createUpdateClientDatagrams(shs)) + shs, error = src.lc.getMinimalSet(clientState.latestHeight, targetHeight) + if error != nil { return (nil, Error) } - while (true) { - // Check if targetHeight exists already on destination chain. - // Query state of IBC client for `src` on chain `dest`. - clientState, membershipProof = dest.queryClientConsensusState(src, targetHeight) - // NOTE: What if a full node we are connected to send us stale (but correct) information regarding targetHeight? - - // Verify the result of the query - sh = dest.lc.get_header(membershipProof.Height + 1) - // NOTE: Headers we obtain from the light client are trusted. - if verifyClientStateProof(clientState, membershipProof, sh.appHash) { - break; - } - replaceFullNode(dst) - } - - // At this point we know that clientState is indeed part of the state on dest chain. - // Verify if installed header is equal to the header obtained the from the local client - // at the same height. - if !src.lc.get_header(clientState.Height) == clientState.SignedHeader.Header { - // We know at this point that conflicting header is installed at the dst chain. - // We need to create proof of fork and submit it to src chain and to dst chain so light client is frozen. - src.lc.createAndSubmitProofOfFork(dst, clientState) - return {nil, error} - } + error = dest.submit(createUpdateClientDatagrams(shs)) + if error != nil { return (nil, Error) } + + clientState, proof = GetClientState(dest, dest.clientId, LATEST_HEIGHT) + if proof == nil { return (nil, Error) } } - - return {clientState.Height, nil} + + // NOTE: semantic check of the installed header is done using fork detection component + return { clientState.Height, nil } } ``` @@ -230,10 +201,3 @@ chain at height *h*. The trusted header is provided by the corresponding light c -- it transfers data between two chains: chainA and chainB. This implies that a -relayer has connections with full nodes from chainA and chainB in order to inspect their -state. We assume that blockchain applications that operates on top of chainA and chainB writes -relevant data into publicly available data store (for example IBC packets). -- in order to verify data written by the application to its store, a relayer needs -light client node for each connected chain. Light client will on its own establish connections -with multiple From 4f540fd42f65b8a6fae784b1c45e6927d97d9288 Mon Sep 17 00:00:00 2001 From: Zarko Milosevic Date: Wed, 16 Sep 2020 16:51:32 +0200 Subject: [PATCH 2/9] Add error handling for the createDatagram function --- docs/spec/relayer/Packets.md | 47 +++++++++++++++++++++--------------- docs/spec/relayer/Relayer.md | 33 +++++++++++++------------ 2 files changed, 45 insertions(+), 35 deletions(-) diff --git a/docs/spec/relayer/Packets.md b/docs/spec/relayer/Packets.md index 48e90396e7..86afc51d32 100644 --- a/docs/spec/relayer/Packets.md +++ b/docs/spec/relayer/Packets.md @@ -254,7 +254,10 @@ func GetDestinationInfo(ev IBCEvent, chain Chain) Chain { ### PacketRecv datagram creation ```golang -func createPacketRecvDatagram(ev SendPacketEvent, chainA Chain, chainB Chain, installedHeight Height) PacketRecv { +func createPacketRecvDatagram(ev SendPacketEvent, + chainA Chain, + chainB Chain, + installedHeight Height) (PacketRecv, Error) { // Stage 1 // Verify if packet is committed to chain A and it is still pending (commitment exists) @@ -262,46 +265,52 @@ func createPacketRecvDatagram(ev SendPacketEvent, chainA Chain, chainB Chain, in proofHeight = installedHeight - 1 packetCommitment, packetCommitmentProof = GetPacketCommitment(chainA, ev.sourcePort, ev.sourceChannel, ev.sequence, proofHeight) - if packetCommitmentProof != nil { return nil } + if packetCommitmentProof == nil { return (nil, Error.RETRY) } if packetCommitment == nil OR - packetCommitment != hash(concat(ev.data, ev.timeoutHeight, ev.timeoutTimestamp)) { return nil } + packetCommitment != hash(concat(ev.data, ev.timeoutHeight, ev.timeoutTimestamp)) { + // invalid event; replace provider + ReplaceProvider(chainA) + return (nil, Error.DROP) + } // Stage 2 // Execute checks IBC handler on chainB will execute channel, proof = GetChannel(chainB, ev.destPort, ev.destChannel, LATEST_HEIGHT) - if proof == nil { return nil } + if proof == nil { return (nil, Error.RETRY) } - // TODO: not necessarily fatal error as optimistic packet send might be taking place - if channel == nil OR - channel.state != OPEN OR - ev.sourcePort != channel.counterpartyPortIdentifier OR - ev.sourceChannel != channel.counterpartyChannelIdentifier { return nil } + if channel != nil AND + (channel.state == CLOSED OR + ev.sourcePort != channel.counterpartyPortIdentifier OR + ev.sourceChannel != channel.counterpartyChannelIdentifier) { (nil, Error.DROP) } + if channel == nil OR channel.state != OPEN { (nil, Error.RETRY) } + // TODO: Maybe we shouldn't even enter handle loop for packets if the corresponding channel is not open! + connectionId = channel.connectionHops[0] connection, proof = GetConnection(chainB, connectionId, LATEST_HEIGHT) - if proof == nil { return nil } + if proof == nil { return (nil, Error.RETRY) } - if connection == nil OR connection.state != OPEN { return nil } + if connection == nil OR connection.state != OPEN { return (nil, Error.RETRY) } - if ev.timeoutHeight != 0 AND GetConsensusHeight(chainB) >= ev.timeoutHeight { return nil } - if ev.timeoutTimestamp != 0 AND GetCurrentTimestamp(chainB) >= ev.timeoutTimestamp { return nil } + if ev.timeoutHeight != 0 AND GetConsensusHeight(chainB) >= ev.timeoutHeight { return (nil, Error.DROP) } + if ev.timeoutTimestamp != 0 AND GetCurrentTimestamp(chainB) >= ev.timeoutTimestamp { return (nil, Error.DROP) } // we now check if this packet is already received by the destination chain if (channel.ordering === ORDERED) { nextSequenceRecv, proof = GetNextSequenceRecv(chainB, ev.destPort, ev.destChannel, LATEST_HEIGHT) - if proof == nil { return nil } + if proof == nil { return (nil, Error.RETRY) } - if ev.sequence != nextSequenceRecv { return nil } // packet has already been delivered by another relayer + if ev.sequence != nextSequenceRecv { return (nil, Error.DROP) } // packet has already been delivered by another relayer } else { - // TODO: Can be proof of absence also and we should be able to verify it. + // Note that absence of ack (packetAcknowledgment == nil) is also proven also and we should be able to verify it. packetAcknowledgement, proof = GetPacketAcknowledgement(chainB, ev.destPort, ev.destChannel, ev.sequence, LATEST_HEIGHT) - if proof == nil { return nil } + if proof == nil { return (nil, Error.RETRY) } - if packetAcknowledgement != nil { return nil } + if packetAcknowledgement != nil { return (nil, Error.DROP) } // packet has already been delivered by another relayer } // Stage 3 @@ -317,7 +326,7 @@ func createPacketRecvDatagram(ev SendPacketEvent, chainA Chain, chainB Chain, in data: ev.data } - return PacketRecv { packet, packetCommitmentProof, proofHeight } + return (PacketRecv { packet, packetCommitmentProof, proofHeight }, nil) } ``` diff --git a/docs/spec/relayer/Relayer.md b/docs/spec/relayer/Relayer.md index 835f9bb858..db52f69dce 100644 --- a/docs/spec/relayer/Relayer.md +++ b/docs/spec/relayer/Relayer.md @@ -76,12 +76,16 @@ We assume that the corresponding light client is correctly installed on each cha ```golang enum Error { - INIT, - TRYOPEN, - OPEN, + RETRY, + DROP, } ``` +The semantic of the returned error is to signal to the caller whether there was an error in the event processing that +could be transient (for example not able to establish connection to a correct full node), in which case +the event can be reprocessed later, or event should be dropped as it is already been received by the destination +chain (due to activity of concurrent relayer). + ```golang func handleEvent(ev, chainA) Error { // NOTE: we don't verify if event data are valid at this point. We trust full node we are connected to @@ -90,27 +94,24 @@ func handleEvent(ev, chainA) Error { // Stage 1. // Determine destination chain chainB = GetDestinationInfo(ev, chainA) - if chainB == nil { return } // TODO: return correct error type + if chainB == nil { return Error.RETRY } // Stage 2. // Update on `chainB` the IBC client for `chainA` to height `>= targetHeight`. targetHeight = ev.height + 1 // See the code for `updateIBCClient` below. installedHeight, error := updateIBCClient(chainB, chainA, targetHeight) - if error != nil { - return Error - } + if error != nil { return Error.RETRY } // Stage 3. // Create the IBC datagrams including `ev` & verify them. - datagram = createDatagram(ev, chainA, chainB, installedHeight) + datagram, error = createDatagram(ev, chainA, chainB, installedHeight) + if error != nil { return Error.RETRY } // Stage 4. // Submit datagrams. - if datagram != nil { - error = chainB.submit(datagram) - if error != nil { return Error } - } + error = chainB.submit(datagram) + if error != nil { return Error.RETRY } } @@ -123,20 +124,20 @@ func handleEvent(ev, chainA) Error { func updateIBCClient(dest Chain, src Chain, targetHeight Height) -> (Height, Error) { clientState, proof = GetClientState(dest, dest.clientId, LATEST_HEIGHT) - if proof == nil { return (nil, Error) } + if proof == nil { return (nil, Error.RETRY) } // NOTE: What if a full node we are connected to send us stale (but correct) information regarding targetHeight? // if installed height is smaller than the targetHeight, we need to update client with targetHeight while (clientState.latestHeight < targetHeight) { // Do an update to IBC client for `src` on `dest`. shs, error = src.lc.getMinimalSet(clientState.latestHeight, targetHeight) - if error != nil { return (nil, Error) } + if error != nil { return (nil, Error.RETRY) } error = dest.submit(createUpdateClientDatagrams(shs)) - if error != nil { return (nil, Error) } + if error != nil { return (nil, Error.RETRY) } clientState, proof = GetClientState(dest, dest.clientId, LATEST_HEIGHT) - if proof == nil { return (nil, Error) } + if proof == nil { return (nil, Error.RETRY) } } // NOTE: semantic check of the installed header is done using fork detection component From ab21ffaad52a770c08796aae0352178c0784d0cf Mon Sep 17 00:00:00 2001 From: Zarko Milosevic Date: Thu, 17 Sep 2020 13:37:26 +0200 Subject: [PATCH 3/9] Add error handling and reorganise spec better --- docs/spec/relayer/Definitions.md | 267 +++++++++++++++++++++++++++++++ docs/spec/relayer/Packets.md | 241 +--------------------------- docs/spec/relayer/Relayer.md | 183 ++++++++++----------- 3 files changed, 358 insertions(+), 333 deletions(-) create mode 100644 docs/spec/relayer/Definitions.md diff --git a/docs/spec/relayer/Definitions.md b/docs/spec/relayer/Definitions.md new file mode 100644 index 0000000000..64625779ad --- /dev/null +++ b/docs/spec/relayer/Definitions.md @@ -0,0 +1,267 @@ +# Data structure and helper function definitions + +This document defines data types and helper functions used by the relayer logic. + +## Data Types + +### Chain + +Chain is a data structure that captures relayer's perspective of a given chain and contains all important +information that allows relayer to communicate with a chain. A provider is a Tendermint full node through +which a relayer read information about the given chain and submit transactions. A relayer maintains a list +of full nodes (*peerList*) as a current provider could be faulty, so it can be replaced by another full node. +For each chain a relayer is connected to, the relayer has a light client that provides the relayer +access to the trusted headers (used as part of data verification). + +```go +type Chain { + chainID string + clientID Identifier + peerList List> + provider Pair + lc LightClient +} +``` + +### Client state and consensus state + +```go +type ClientState { + chainID string + validatorSet List> + trustLevel Rational + trustingPeriod uint64 + unbondingPeriod uint64 + latestHeight Height + latestTimestamp uint64 + frozenHeight Maybe + upgradeCommitmentPrefix CommitmentPrefix + upgradeKey []byte + maxClockDrift uint64 + proofSpecs []ProofSpec +} +``` + +```go +type ConsensusState { + timestamp uint64 + validatorSet List> + commitmentRoot []byte +} +``` + +### Membership proof + +```go +type MembershipProof struct { + Height Height + Proof Proof +} +``` + +### Connection + +```go +type ConnectionEnd { + state ConnectionState + counterpartyConnectionIdentifier Identifier + counterpartyPrefix CommitmentPrefix + clientIdentifier Identifier + counterpartyClientIdentifier Identifier + version []string +} + +enum ConnectionState { + INIT, + TRYOPEN, + OPEN, +} +``` + +### Channel + +```go +type ChannelEnd { + state ChannelState + ordering ChannelOrder + counterpartyPortIdentifier Identifier + counterpartyChannelIdentifier Identifier + connectionHops [Identifier] + version string +} + +enum ChannelState { + INIT, + TRYOPEN, + OPEN, + CLOSED, +} + +enum ChannelOrder { + ORDERED, + UNORDERED, +} +``` + +```go +type Packet { + sequence uint64 + timeoutHeight Height + timeoutTimestamp uint64 + sourcePort Identifier + sourceChannel Identifier + destPort Identifier + destChannel Identifier + data []byte +} +``` + +```go +type PacketRecv { + packet Packet + proof CommitmentProof + proofHeight Height +} +``` + +## Helper functions + +We assume the existence of the following helper functions: + +```go +// Returns channel end with a commitment proof. +GetChannel(chain Chain, + portId Identifier, + channelId Identifier, + proofHeight Height) (ChannelEnd, CommitmentProof) + +// Returns connection end with a commitment proof. +GetConnection(chain Chain, + connectionId Identifier, + proofHeight Height) (ConnectionEnd, CommitmentProof) + + +// Returns client state with a commitment proof. +GetClientState(chain Chain, + clientId Identifier, + proofHeight Height) (ClientState, CommitmentProof) + +// Returns consensus state with a commitment proof. +GetConsensusState(chain Chain, + clientId Identifier, + targetHeight Height, + proofHeight Height) (ConsensusState, CommitmentProof) + + +// Returns packet commitment with a commitment proof. +GetPacketCommitment(chain Chain, + portId Identifier, + channelId Identifier, + sequence uint64, + proofHeight Height) (bytes, CommitmentProof) + +// Returns next recv sequence number with a commitment proof. +GetNextSequenceRecv(chain Chain, + portId Identifier, + channelId Identifier, + proofHeight Height) (uint64, CommitmentProof) + +// Returns packet acknowledgment with a commitment proof. +GetPacketAcknowledgement(chain Chain, + portId Identifier, + channelId Identifier, + sequence uint64, + proofHeight Height) (bytes, CommitmentProof) + +// Returns estimate of the consensus height on the given chain. +GetConsensusHeight(chain Chain) Height + +// Returns estimate of the current time on the given chain. +GetCurrentTimestamp(chainB) uint64 + +// Verify that the data is written at the given path using provided membership proof and the root hash. +VerifyMembership(rootHash []byte, + proofHeight Height, + proof MembershipProof, + path String, + data []byte) boolean + +// Create IBC datagram as part of processing event at chainA. +CreateDatagram(ev IBCEvent, + chainA Chain, + chainB Chain, + installedHeight Height) (IBCDatagram, Error) + +// Create UpdateClient datagrams from the list of signed headers +CreateUpdateClientDatagrams(shs []SignedHeader) IBCDatagram[] + +// Submit given datagram to a given chain +Submit(chain Chain, datagram IBCDatagram) Error + +// Return the correspondin chain for a given chainID +// We assume that the relayer maintains a map of known chainIDs and the corresponding chains. +GetChain(chainID String) Chain +``` + +For functions that return proof, if proof != nil, then the returned value is being verified. +The value is being verified using the header's app hash that is provided by the corresponding light client. + +### Error handling + +Helper functions listed above assume querying (parts of the) application state using Tendermint RPC. For example, +`GetChannel` relies on `QueryChannel`. RPC calls can fail as + +- no response is received within some timeout or +- malformed response is received. + +In both cases, error handling logic should be defined by the caller. For example, in the former case, the caller might +retry sending the same request to a same provider (full node), while in the latter case the request might be sent to +some other provider node. Although these kinds of errors could be due to network infrastructure issues, it is normally +simpler to blame the provider (assume implicitly network is always correct and reliable). Therefore, correct provider +always respond timely with a correct response, while in case of errors we consider the provider node faulty, and then +we replace it with a different node. + +We now show the pseudocode for one of those functions that contains simplified error handling logic: + +```go +func GetChannel(chain Chain, + portId Identifier, + channelId Identifier, + proofHeight Height) (ChannelEnd, CommitmentProof) { + + while(true) { + // Query provable store exposed by the full node of chain. + // The path for the channel end is at channelEnds/ports/{portId}/channels/{channelId}". + // The membership proof returned is read at height proofHeight. + channel, proof, error = QueryChannel(chain.provider, portId, channelId, proofHeight) + if error != nil { + // elect a new provider from the peer list + if !ReplaceProvider(chain) { return (nil, nil) } // return if fail to elect new provider + } + + header, error = GetHeader(chain.lc, proofHeight) // get header for height proofHeight using light client + if error != nil { return (nil, nil) } // return if light client can't provide header for the given height + + // verify membership of the channel at path channelEnds/ports/{portId}/channels/{channelId} using + // the root hash header.AppHash + if VerifyMembership(header.AppHash, proofHeight, proof, channelPath(portId, channelId), channel) { + return (channel, proof) + } + + // membership check fails; therefore provider is faulty. Try to elect new provider + if !ReplaceProvider(chain) { return (nil, nil) } // if fails to elect new provider return + } + panic // should never reach this line +} + +// Simplified version of logic for electing new provider. In reality it will probably involve opening a connection to +// a newply elected provider and closing connection with an old provider. +func ReplaceProvider(chain Chain) boolean { + if chain.peerList.IsEmpty() return false + chain.provider = Head(chain.peerList) + chain.peerList = Tail(chain.peerList) + return true +} +``` +If *LATEST_HEIGHT* is passed as a parameter, the data should be read (and the corresponding proof created) +at the most recent height. diff --git a/docs/spec/relayer/Packets.md b/docs/spec/relayer/Packets.md index 86afc51d32..2144a4d98d 100644 --- a/docs/spec/relayer/Packets.md +++ b/docs/spec/relayer/Packets.md @@ -2,28 +2,7 @@ This document specifies datagram creation logic for packets. It is used by the relayer. -## Data Types - -```go -type Packet { - sequence uint64 - timeoutHeight Height - timeoutTimestamp uint64 - sourcePort Identifier - sourceChannel Identifier - destPort Identifier - destChannel Identifier - data bytes -} -``` - -```go -type PacketRecv { - packet Packet - proof CommitmentProof - proofHeight Height -} -``` +## Packet related IBC events ```go type SendPacketEvent { @@ -39,225 +18,15 @@ type SendPacketEvent { } ``` -```go -type ChannelEnd { - state ChannelState - ordering ChannelOrder - counterpartyPortIdentifier Identifier - counterpartyChannelIdentifier Identifier - connectionHops [Identifier] - version string -} - -enum ChannelState { - INIT, - TRYOPEN, - OPEN, - CLOSED, -} - -enum ChannelOrder { - ORDERED, - UNORDERED, -} -``` - -```go -type ConnectionEnd { - state ConnectionState - counterpartyConnectionIdentifier Identifier - counterpartyPrefix CommitmentPrefix - clientIdentifier Identifier - counterpartyClientIdentifier Identifier - version []string -} - -enum ConnectionState { - INIT, - TRYOPEN, - OPEN, -} -``` - -```go -type ClientState { - chainID string - validatorSet List> - trustLevel Rational - trustingPeriod uint64 - unbondingPeriod uint64 - latestHeight Height - latestTimestamp uint64 - frozenHeight Maybe - upgradeCommitmentPrefix CommitmentPrefix - upgradeKey []byte - maxClockDrift uint64 - proofSpecs []ProofSpec -} -``` -## Helper functions - -We assume the existence of the following helper functions: - -```go -// Returns channel end with a commitment proof. -GetChannel(chain Chain, - portId Identifier, - channelId Identifier, - proofHeight Height) (ChannelEnd, CommitmentProof) - -// Returns connection end with a commitment proof. -GetConnection(chain Chain, - connectionId Identifier, - proofHeight Height) (ConnectionEnd, CommitmentProof) - - -// Returns client state with a commitment proof. -GetClientState(chain Chain, - clientId Identifier, - proofHeight Height) (ClientState, CommitmentProof) - -// Returns consensus state with a commitment proof. -GetConsensusState(chain Chain, - clientId Identifier, - targetHeight Height, - proofHeight Height) (ConsensusState, CommitmentProof) - - -// Returns packet commitment with a commitment proof. -GetPacketCommitment(chain Chain, - portId Identifier, - channelId Identifier, - sequence uint64, - proofHeight Height) (bytes, CommitmentProof) - -// Returns next recv sequence number with a commitment proof. -GetNextSequenceRecv(chain Chain, - portId Identifier, - channelId Identifier, - proofHeight Height) (uint64, CommitmentProof) - -// Returns packet acknowledgment with a commitment proof. -GetPacketAcknowledgement(chain Chain, - portId Identifier, - channelId Identifier, - sequence uint64, - proofHeight Height) (bytes, CommitmentProof) - -// Returns estimate of the consensus height on the given chain. -GetConsensusHeight(chain Chain) Height - -// Returns estimate of the current time on the given chain. -GetCurrentTimestamp(chainB) uint64 - -``` - -For functions that return proof, if proof != nil, then the returned value is being verified. -The value is being verified using the header's app hash that is provided by the corresponding light client. - -### Error handling - -Helper functions listed above assume querying (parts of the) application state using Tendermint RPC. For example, -GetChannel relies on QueryChannel. RPC calls can fail as: - -- no response is received within some timeout (TimeoutError) or -- malformed response is received (SerializationError). - -In both cases, error handling logic should be defined by the caller. For example, in the former case, the caller might -retry to send the same request to a same provider (full node), while in the latter case the request might be sent to -some other provider node. Although these kinds of errors could be due to network infrastructure issues, it is normally -simpler to blame the provider (assume implicitly network is always correct and reliable). Therefore, correct provider -always respond timely with a correct response, while in case of errors we consider the provider node faulty, and then -we replace it with a different node. - -```go -type Chain { - chainID string - clientID Identifier - peerList List> - provider Pair - lc LightClient -} -``` - -We now show the pseudocode for one of those functions: - -```go -func GetChannel(chain Chain, - portId Identifier, - channelId Identifier, - proofHeight Height) (ChannelEnd, CommitmentProof) { - - while(true) { - // Query provable store exposed by the full node of chain. - // The path for the channel end is at channelEnds/ports/{portId}/channels/{channelId}". - // The membership proof returned is read at height proofHeight. - channel, proof, error = QueryChannel(chain.provider, portId, channelId, proofHeight) - if error != nil { - // elect a new provider from the peer list - if !ReplaceProvider(chain) { return (nil, nil) } // return if fail to elect new provider - } - - header, error = GetHeader(chain.lc, proofHeight) // get header for height proofHeight using light client - if error != nil { return (nil, nil) } // return if light client can't provide header for the given height - - // verify membership of the channel at path channelEnds/ports/{portId}/channels/{channelId} using - // the root hash header.AppHash - if verifyMembership(header.AppHash, proofHeight, proof, channelPath(portId, channelId), channel) { - return channel, proof - } - - // membership check fails; therefore provider is faulty. Try to elect new provider - if !ReplaceProvider(chain) { return (nil, nil) } // if fails to elect new provider return - } - panic // should never reach this line -} - -// Simplified version of logic for electing new provider. In reality it will probably involve opening a connection to -// a newply elected provider and closing connection with an old provider. -func ReplaceProvider(chain Chain) boolean { - if chain.peerList.IsEmpty() return false - chain.provider = Head(chain.peerList) - chain.peerList = Tail(chain.peerList) - return true -} -``` -If LATEST_HEIGHT is passed as a parameter, the data should be read (and the corresponding proof created) -at the most recent height. - -## Computing destination chain - -```golang -func GetDestinationInfo(ev IBCEvent, chain Chain) Chain { - switch ev.type { - case SendPacketEvent: - channel, proof = GetChannel(chain, ev.sourcePort, ev.sourceChannel, ev.Height) - if proof == nil { return nil } - - connectionId = channel.connectionHops[0] - connection, proof = GetConnection(chain, connectionId, ev.Height) - if proof == nil { return nil } - - clientState, proof = GetClientState(chain, connection.clientIdentifier, ev.Height) - if proof == nil { return nil } - - // We assume that the relayer maintains a map of known chainIDs and the corresponding chains. - // Note that in a normal case, relayer should be aware of chains between it relays packets - return getChain(clientState.chainID) - ... - } -} -``` - ## Datagram creation logic ### PacketRecv datagram creation ```golang -func createPacketRecvDatagram(ev SendPacketEvent, - chainA Chain, - chainB Chain, - installedHeight Height) (PacketRecv, Error) { +func CreateDatagram(ev SendPacketEvent, + chainA Chain, + chainB Chain, + installedHeight Height) (PacketRecv, Error) { // Stage 1 // Verify if packet is committed to chain A and it is still pending (commitment exists) diff --git a/docs/spec/relayer/Relayer.md b/docs/spec/relayer/Relayer.md index db52f69dce..17d07c2250 100644 --- a/docs/spec/relayer/Relayer.md +++ b/docs/spec/relayer/Relayer.md @@ -24,76 +24,99 @@ eventually received by the module B. **[ICS18-Validity]**: If a module B receives an IBC datagram m from a module A, then m was sent by the module A to the module B. -## Data Types +## System model -```go -type ConsensusState { - timestamp uint64 - validatorSet List> - commitmentRoot []byte -} -``` +We assume that a correct relayer operates in the following model: -```go -type MembershipProof struct { - Height Height - Proof Proof -} -``` +### Connected chains -## Relayer algorithm +Relayer transfers data between two chains: chainA and chainB. For simplicity, we assume Tendermint chains. +Each chain operates under Tendermint security model: +- given a block b at height h committed at time `t = b.Header.Time`, `+2/3` of voting power behaves correctly +at least before `t + UNBONDING_PERIOD`, where `UNBONDING_PERIOD` is a system parameter (typically order of weeks). +Validators sets can be changed in every block, and we don't assume any constraint on the way validators are changed +(application specific logic). -We assume the existence of the following helper functions: - -```go -// returns ClientState for the targetHeight if it exists; otherwise returns ClientState at the latest height. -// We assume that this function handles non-responsive full node error by switching to a different full node. -queryClientConsensusState(chainA, targetHeight) (ClientState, MembershipProof) -verifyClientStateProof(clientStateAonB, membershipProof, sh.appHash) boolean -createDatagrams(height, chainA, chainB, installedHeight) IBCDatagram[] -verifyProof(datagrams, sh.appHash) boolean -createUpdateClientDatagrams(shs) IBCDatagram[] -submit(datagrams) error -replaceFullNode(chain) -``` +Furthermore, we assume that blockchain applications that operate on top of chainA and chainB writes +relevant data into Merkleised data store (for example IBC packets), and that parts of the store are publicly +available (so relayers can access it). + +In order to access IBC relevant data, a relayer needs to establish connections with full nodes (correct) from +both chains. Note that there is no constrain on number of faulty full nodes: we can only assume that a correct relayer +will eventually have access to a correct full node. + +### Data availability -The main relayer event loop is a pipeline of three stages. Assuming some IBC event at height `h` on `chainA`, +Note that data written to a store at height *h* as part of executing block *b* (`b.Height = h`) is effectively committed by +the next block (at height h+1). The reason is the fact that the data store root hash as an effect of executing block at +height h is part of the block header at height h+1. Therefore, data read at height h is available until time +`t = b.Header.Time + UNBONDING_PERIOD`, where `b.Header.Height = h+1`. After time *t* we cannot trust that data anymore. +Note that data present in the store are re-validated by each new block: data added/modified at block *h* are still +valid even if not altered after, as they are still "covered" by the root hash of the store. + +Therefore UNBONDING_PERIOD gives absolute time bound during which relayer needs to transfer data read at source chain +to the destination chain. As we will explain below, due to fork detection and accountability protocols, the effective +data availability period will be shorter than UNBONDING_PERIOD. + +### Data verification + +As connected chains in IBC do not blindly trust each other, data coming from the opposite chain must be verified at +the destination before being acted upon. Data verification in IBC is implemented by relying on the concept of light client. +Light client is a process that by relying on an initial trusted header (subjective initialisation), verifies and maintains +set of trusted headers. Note that a light client does not maintain full blockchain and does not execute (verify) application +transitions. It operates by relying on the Tendermint security model, and by applying header verification logic that operates +only on signed headers (header + corresponding commit). + +More details about light client assumptions and protocols can be found +[here](https://github.com/tendermint/spec/tree/master/rust-spec/lightclient). For the purpose of this document, we assume +that a relayer has access to the light client node that provides trusted headers. +Given a data d read at a given path at height h with a proof p, we assume existence of a function +`VerifyMembership(header.AppHash, h, proof, path, d)` that returns `true` if data was committed by the corresponding +chain at height *h*. The trusted header is provided by the corresponding light client. + +## Relayer algorithm + +The main relayer event loop is a pipeline of four stages. Assuming some IBC event at height `h` on `chainA`, the relayer: -1. Updates (on `chainB`) the IBC client for `chainA` to a certain height `H` where `H >= h+1`. -2. Create IBC datagram at height `H-1`. -3. Submit the datagram from stage (2) to `chainB`. +1. Determine destination chain (`chainB`) +2. Updates (on `chainB`) the IBC client for `chainA` to a certain height `H` where `H >= h+1`. +3. Create IBC datagram at height `H-1`. +4. Submit the datagram from stage (2) to `chainB`. Note that an IBC event at height `h` corresponds to the modifications to the data store made as part of executing block at height `h`. The corresponding proof (that data is indeed written to the data store) can be verified using the data store root hash that is part of the header at height `h+1`. -Once stage 1 finishes correctly, stage 2 should succeed assuming that `chainB` has not already processed the event. The -interface between stage 1 and stage 2 is just the height `H`. Once stage 2 finishes correctly, stage 3 should -succeed. The interface between stage 2 and stage 3 is an IBC datagram. +Once stage 2 finishes correctly, stage 3 should succeed assuming that `chainB` has not already processed the event. The +interface between stage 2 and stage 3 is just the height `H`. Once stage 3 finishes correctly, stage 4 should +succeed. The interface between stage 3 and stage 4 is an IBC datagram. We assume that the corresponding light client is correctly installed on each chain. +The semantic of the returned error is to signal to the caller whether there was an error in the event processing that +could be transient (for example not able to establish connection to a correct full node), in which case +the event can be reprocessed later (`Error.RETRY`), or the event should be dropped as it has already been +received by the destination chain due to activity of a concurrent relayer (`Error.DROP`). + +Data structures and helper function definitions are provided +[here](https://github.com/informalsystems/ibc-rs/blob/master/docs/spec/relayer/Definitions.md). + ```golang enum Error { RETRY, DROP, } -``` - -The semantic of the returned error is to signal to the caller whether there was an error in the event processing that -could be transient (for example not able to establish connection to a correct full node), in which case -the event can be reprocessed later, or event should be dropped as it is already been received by the destination -chain (due to activity of concurrent relayer). +``` ```golang func handleEvent(ev, chainA) Error { // NOTE: we don't verify if event data are valid at this point. We trust full node we are connected to - // until some verification fails. Otherwise, we can have Stage 2 (datagram creation being done first). + // until some verification fails. // Stage 1. // Determine destination chain - chainB = GetDestinationInfo(ev, chainA) + chainB = getDestinationInfo(ev, chainA) if chainB == nil { return Error.RETRY } // Stage 2. @@ -105,22 +128,39 @@ func handleEvent(ev, chainA) Error { // Stage 3. // Create the IBC datagrams including `ev` & verify them. - datagram, error = createDatagram(ev, chainA, chainB, installedHeight) + datagram, error = CreateDatagram(ev, chainA, chainB, installedHeight) if error != nil { return Error.RETRY } // Stage 4. // Submit datagrams. - error = chainB.submit(datagram) + error = Submit(chainB, datagram) if error != nil { return Error.RETRY } } +func getDestinationInfo(ev IBCEvent, chain Chain) Chain { + switch ev.type { + case SendPacketEvent: + channel, proof = GetChannel(chain, ev.sourcePort, ev.sourceChannel, ev.Height) + if proof == nil { return nil } + + connectionId = channel.connectionHops[0] + connection, proof = GetConnection(chain, connectionId, ev.Height) + if proof == nil { return nil } + + clientState, proof = GetClientState(chain, connection.clientIdentifier, ev.Height) + if proof == nil { return nil } + + return GetChain(clientState.chainID) + ... + } +} // Perform an update on `dest` chain for the IBC client for `src` chain. // Preconditions: // - `src` chain has height greater or equal to `targetHeight` // Postconditions: // - returns the installedHeight >= targetHeight -// - return error if verification of client state fails +// - return error if some of verification steps fail func updateIBCClient(dest Chain, src Chain, targetHeight Height) -> (Height, Error) { clientState, proof = GetClientState(dest, dest.clientId, LATEST_HEIGHT) @@ -145,57 +185,6 @@ func updateIBCClient(dest Chain, src Chain, targetHeight Height) -> (Height, Err } ``` -## System model - -We assume that a correct relayer operates in the following model: - -### Connected chains - -Relayer transfers data between two chains: chainA and chainB. For simplicity, we assume Tendermint chains. -Each chain operates under Tendermint security model: -- given a block b at height h committed at time t = b.Header.Time, +2/3 of voting power behaves correctly -at least before t + UNBONDING_PERIOD, where UNBONDING_PERIOD is a system parameter (typically order of weeks). -Validators sets can be changed in every block, and we don't assume any constraint on the way validators are changed -(application specific logic). - -Furthermore, we assume that blockchain applications that operate on top of chainA and chainB writes -relevant data into Merkleised data store (for example IBC packets), and that parts of the store are publicly -available (so relayers can access it). - -In order to access IBC relevant data, a relayer needs to establish connections with full nodes (correct) from -both chains. Note that there is no constrain on number of faulty full nodes: we can only assume that a correct relayer -will eventually have access to a correct full node. - -### Data availability - -Note that data written to a store at height *h* as part of executing block *b* (`b.Height = h`) is effectively committed by -the next block (at height h+1). The reason is the fact that the data store root hash as an effect of executing block at -height h is part of the block header at height h+1. Therefore, data read at height h is available until time -`t = b.Header.Time + UNBONDING_PERIOD`, where `b.Header.Height = h+1`. After time *t* we cannot trust that data anymore. -Note that data present in the store are re-validated by each new block: data added/modified at block *h* are still -valid even if not altered after, as they are still "covered" by the root hash of the store. - -Therefore UNBONDING_PERIOD gives absolute time bound during which relayer needs to transfer data read at source chain -to the destination chain. As we will explain below, due to fork detection and accountability protocols, the effective -data availability period will be shorter than UNBONDING_PERIOD. - -### Data verification - -As connected chains in IBC do not blindly trust each other, data coming from the opposite chain must be verified at -the destination before being acted upon. Data verification in IBC is implemented by relying on the concept of light client. -Light client is a process that by relying on an initial trusted header (subjective initialisation), verifies and maintains -set of trusted headers. Note that a light client does not maintain full blockchain and does not execute (verify) application -transitions. It operates by relying on the Tendermint security model, and by applying header verification logic that operates -only on signed headers (header + corresponding commit). - -More details about light client assumptions and protocols can be found -[here](https://github.com/tendermint/spec/tree/master/rust-spec/lightclient). For the purpose of this document, we assume -that a relayer has access to the light client node that provides trusted headers. -Given a data d read at a given path at height h with a proof p, we assume existence of a function -`verifyMembership(header.AppHash, h, proof, path, d)` that returns `true` if data was committed by the corresponding -chain at height *h*. The trusted header is provided by the corresponding light client. - - From 441e3bbb70daa9a339d127ed3acf80134c73824a Mon Sep 17 00:00:00 2001 From: Zarko Milosevic Date: Thu, 17 Sep 2020 14:31:34 +0200 Subject: [PATCH 4/9] Fix off by one height --- docs/spec/relayer/Packets.md | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/docs/spec/relayer/Packets.md b/docs/spec/relayer/Packets.md index 2144a4d98d..6d9cd609af 100644 --- a/docs/spec/relayer/Packets.md +++ b/docs/spec/relayer/Packets.md @@ -26,12 +26,11 @@ type SendPacketEvent { func CreateDatagram(ev SendPacketEvent, chainA Chain, chainB Chain, - installedHeight Height) (PacketRecv, Error) { + proofHeight Height) (PacketRecv, Error) { // Stage 1 // Verify if packet is committed to chain A and it is still pending (commitment exists) - proofHeight = installedHeight - 1 packetCommitment, packetCommitmentProof = GetPacketCommitment(chainA, ev.sourcePort, ev.sourceChannel, ev.sequence, proofHeight) if packetCommitmentProof == nil { return (nil, Error.RETRY) } From 1809f1cee6af4555f3e583baa8f3640d8fdd51cc Mon Sep 17 00:00:00 2001 From: Zarko Milosevic Date: Fri, 18 Sep 2020 12:28:28 +0200 Subject: [PATCH 5/9] Add WriteAcknowledgementEvent handler --- docs/spec/relayer/Definitions.md | 26 +++++ docs/spec/relayer/Packets.md | 180 +++++++++++++++++++++++++++++-- docs/spec/relayer/Relayer.md | 35 ++++-- 3 files changed, 220 insertions(+), 21 deletions(-) diff --git a/docs/spec/relayer/Definitions.md b/docs/spec/relayer/Definitions.md index 64625779ad..6f29900419 100644 --- a/docs/spec/relayer/Definitions.md +++ b/docs/spec/relayer/Definitions.md @@ -124,6 +124,15 @@ type PacketRecv { } ``` +```go +type PacketAcknowledgement { + packet Packet + acknowledgement byte[] + proof CommitmentProof + proofHeight Height +} +``` + ## Helper functions We assume the existence of the following helper functions: @@ -166,12 +175,29 @@ GetNextSequenceRecv(chain Chain, channelId Identifier, proofHeight Height) (uint64, CommitmentProof) + +// Returns next recv sequence number with a commitment proof. +GetNextSequenceAck(chain Chain, + portId Identifier, + channelId Identifier, + proofHeight Height) (uint64, CommitmentProof) + + // Returns packet acknowledgment with a commitment proof. GetPacketAcknowledgement(chain Chain, portId Identifier, channelId Identifier, sequence uint64, proofHeight Height) (bytes, CommitmentProof) + + +// Returns packet receipt with a commitment proof. +GetPacketReceipt(chain Chain, + portId Identifier, + channelId Identifier, + sequence uint64, + proofHeight Height) (String, CommitmentProof) + // Returns estimate of the consensus height on the given chain. GetConsensusHeight(chain Chain) Height diff --git a/docs/spec/relayer/Packets.md b/docs/spec/relayer/Packets.md index 6d9cd609af..43740e9c98 100644 --- a/docs/spec/relayer/Packets.md +++ b/docs/spec/relayer/Packets.md @@ -14,13 +14,85 @@ type SendPacketEvent { sourceChannel Identifier destPort Identifier destChannel Identifier - data bytes + data []byte } ``` -## Datagram creation logic +```go +type ReceivePacket { + height Height + sourcePort Identifier + sourceChannel Identifier + destPort Identifier + destChannel Identifier + sequence uint64 + timeoutHeight Height + timeoutTimestamp uint64 + data []byte +} +``` + +```go +type WriteAcknowledgementEvent { + height Height + port Identifier + channel Identifier + sequence uint64 + timeoutHeight Height + timeoutTimestamp uint64 + data []byte + acknowledgement []byte +} +``` + +```go +type AcknowledgePacket { + height Height + sourcePort Identifier + sourceChannel Identifier + destPort Identifier + destChannel Identifier + sequence uint64 + timeoutHeight Height + timeoutTimestamp uint64 +} +``` -### PacketRecv datagram creation +```go +type CleanupPacket { + height Height + sourcePort Identifier + sourceChannel Identifier + destPort Identifier + destChannel Identifier + sequence uint64 + timeoutHeight Height + timeoutTimestamp uint64 +} +``` + +```go +type TimeoutPacket { + height Height + sourcePort Identifier + sourceChannel Identifier + destPort Identifier + destChannel Identifier + sequence uint64 + timeoutHeight Height + timeoutTimestamp uint64 +} +``` +TODO: Figure out more concise way to define these types as most of them are the same + +## Event handlers + +### SendPacketEvent handler + +Successful handling of *SendPacketEvent* leads to *PacketRecv* datagram creation. + +// NOTE: Stateful relayer might keep packet that are not acked in the state so the following logic +// can be a bit simpler. ```golang func CreateDatagram(ev SendPacketEvent, @@ -66,19 +138,19 @@ func CreateDatagram(ev SendPacketEvent, if ev.timeoutTimestamp != 0 AND GetCurrentTimestamp(chainB) >= ev.timeoutTimestamp { return (nil, Error.DROP) } // we now check if this packet is already received by the destination chain - if (channel.ordering === ORDERED) { - nextSequenceRecv, proof = GetNextSequenceRecv(chainB, ev.destPort, ev.destChannel, LATEST_HEIGHT) - if proof == nil { return (nil, Error.RETRY) } + if channel.ordering === ORDERED { + nextSequenceRecv, proof = GetNextSequenceRecv(chainB, ev.destPort, ev.destChannel, LATEST_HEIGHT) + if proof == nil { return (nil, Error.RETRY) } - if ev.sequence != nextSequenceRecv { return (nil, Error.DROP) } // packet has already been delivered by another relayer + if ev.sequence != nextSequenceRecv { return (nil, Error.DROP) } // packet has already been delivered by another relayer } else { // Note that absence of ack (packetAcknowledgment == nil) is also proven also and we should be able to verify it. - packetAcknowledgement, proof = - GetPacketAcknowledgement(chainB, ev.destPort, ev.destChannel, ev.sequence, LATEST_HEIGHT) + packetReceipt, proof = + GetPacketReceipt(chainB, ev.destPort, ev.destChannel, ev.sequence, LATEST_HEIGHT) if proof == nil { return (nil, Error.RETRY) } - if packetAcknowledgement != nil { return (nil, Error.DROP) } // packet has already been delivered by another relayer + if packetReceipt != nil { return (nil, Error.DROP) } // packet has already been delivered by another relayer } // Stage 3 @@ -98,4 +170,92 @@ func CreateDatagram(ev SendPacketEvent, } ``` +### WriteAcknowledgementEvent handler + +Successful handling of *WriteAcknowledgementEvent* leads to *PacketAcknowledgement* datagram creation. + +```golang +func CreateDatagram(ev WriteAcknowledgementEvent, + chainA Chain, // source chain + chainB Chain, // destination chain + proofHeight Height) (PacketAcknowledgement, Error) { + + // Stage 1 + // Verify if acknowledment is committed to chain A and it is still pending + + // TODO: Figure out what should be the height at which this info is read! + packetAck, packetAckCommitmentProof = + GetPacketAcknowledgement(chainA, ev.port, ev.channel, ev.sequence, proofHeight) + if packetAckCommitmentProof == nil { return (nil, Error.RETRY) } + + if packetAck == nil OR + packetAck != hash(ev.acknowledgement) { + // invalid event; replace provider + ReplaceProvider(chainA) // TODO: We shouldn't replace provider here; it should be communicated with the error type + return (nil, Error.DROP) + } + + // Stage 2 + // Execute checks IBC handler on chainB will execute + channelA, proof = GetChannel(chainA, ev.port, ev.channel, LATEST_HEIGHT) + if proof == nil { return (nil, Error.RETRY) } + + channelB, proof = + GetChannel(chainB, channelA.counterpartyPortIdentifier, channelA.counterpartyChannelIdentifier, LATEST_HEIGHT) + if proof == nil { return (nil, Error.RETRY) } + + if channelB == nil OR channel.state != OPEN { (nil, Error.DROP) } + // Note that we checked implicitly above that counterparty identifiers match each other + + connectionId = channelB.connectionHops[0] + connection, proof = GetConnection(chainB, connectionId, LATEST_HEIGHT) + if proof == nil { return (nil, Error.RETRY) } + + if connection == nil OR connection.state != OPEN { return (nil, Error.DROP) } + + // verify the packet is sent by chainB and hasn't been cleared out yet + packetCommitment, packetCommitmentProof = + GetPacketCommitment(chainB, channelA.counterpartyPortIdentifier, + channelA.counterpartyChannelIdentifier, ev.sequence, LATEST_HEIGHT) + if packetCommitmentProof == nil { return (nil, Error.RETRY) } + + if packetCommitment == nil OR + packetCommitment != hash(concat(ev.data, ev.timeoutHeight, ev.timeoutTimestamp)) { + // invalid event; replace provider + ReplaceProvider(chainB) + return (nil, Error.DROP) + } + + // abort transaction unless acknowledgement is processed in order + if channelB.ordering === ORDERED { + nextSequenceAck, proof = + GetNextSequenceAck(chainB, channelA.counterpartyPortIdentifier, + channelA.counterpartyChannelIdentifier, ev.sequence, LATEST_HEIGHT) + if proof == nil { return (nil, Error.RETRY) } + + if ev.sequence != nextSequenceAck { return (nil, Error.DROP) } + } + + // Stage 3 + // Build datagram as all checks has passed + packet = Packet { + sequence: ev.sequence, + timeoutHeight: ev.timeoutHeight, + timeoutTimestamp: ev.timeoutTimestamp, + sourcePort: channelA.counterpartyPortIdentifier, + sourceChannel: channelA.counterpartyChannelIdentifier, + destPort: ev.port, + destChannel: ev.channel, + data: ev.data + } + + type PacketAcknowledgement { + packet Packet + acknowledgement byte[] + proof CommitmentProof + proofHeight Height + } + return (PacketAcknowledgement { packet, ev.acknowledgement, packetAckCommitmentProof, proofHeight }, nil) +} +``` diff --git a/docs/spec/relayer/Relayer.md b/docs/spec/relayer/Relayer.md index 17d07c2250..220b2258cb 100644 --- a/docs/spec/relayer/Relayer.md +++ b/docs/spec/relayer/Relayer.md @@ -140,21 +140,34 @@ func handleEvent(ev, chainA) Error { func getDestinationInfo(ev IBCEvent, chain Chain) Chain { switch ev.type { case SendPacketEvent: - channel, proof = GetChannel(chain, ev.sourcePort, ev.sourceChannel, ev.Height) - if proof == nil { return nil } - - connectionId = channel.connectionHops[0] - connection, proof = GetConnection(chain, connectionId, ev.Height) - if proof == nil { return nil } - - clientState, proof = GetClientState(chain, connection.clientIdentifier, ev.Height) - if proof == nil { return nil } + chainId = getChainId(chain, ev.sourcePort, ev.sourceChannel, ev.Height) + if chainId == nil { return nil } + + return GetChain(chainId) - return GetChain(clientState.chainID) - ... + case WriteAcknowledgementEvent: + chainId = getChainId(chain, ev.Port, ev.Channel, ev.Height) + if chainId == nil { return nil } + + return GetChain(chainId) } } +// Return chaindId of the destination chain based on port and channel info for the given chain +func getChainId(chain Chain, port Identifier, channel Identifier, height Height) String { + channel, proof = GetChannel(chain, port, channel, height) + if proof == nil { return nil } + + connectionId = channel.connectionHops[0] + connection, proof = GetConnection(chain, connectionId, height) + if proof == nil { return nil } + + clientState, proof = GetClientState(chain, connection.clientIdentifier, height) + if proof == nil { return nil } + + return clientState.chainID +} + // Perform an update on `dest` chain for the IBC client for `src` chain. // Preconditions: // - `src` chain has height greater or equal to `targetHeight` From 4904478bf9a34e3a54d3f181cecfcbaa4b009a87 Mon Sep 17 00:00:00 2001 From: Zarko Milosevic Date: Fri, 18 Sep 2020 13:21:29 +0200 Subject: [PATCH 6/9] Small cleaning --- docs/spec/relayer/Packets.md | 79 ++++-------------------------------- 1 file changed, 9 insertions(+), 70 deletions(-) diff --git a/docs/spec/relayer/Packets.md b/docs/spec/relayer/Packets.md index 43740e9c98..09c963c2f8 100644 --- a/docs/spec/relayer/Packets.md +++ b/docs/spec/relayer/Packets.md @@ -18,20 +18,6 @@ type SendPacketEvent { } ``` -```go -type ReceivePacket { - height Height - sourcePort Identifier - sourceChannel Identifier - destPort Identifier - destChannel Identifier - sequence uint64 - timeoutHeight Height - timeoutTimestamp uint64 - data []byte -} -``` - ```go type WriteAcknowledgementEvent { height Height @@ -45,46 +31,6 @@ type WriteAcknowledgementEvent { } ``` -```go -type AcknowledgePacket { - height Height - sourcePort Identifier - sourceChannel Identifier - destPort Identifier - destChannel Identifier - sequence uint64 - timeoutHeight Height - timeoutTimestamp uint64 -} -``` - -```go -type CleanupPacket { - height Height - sourcePort Identifier - sourceChannel Identifier - destPort Identifier - destChannel Identifier - sequence uint64 - timeoutHeight Height - timeoutTimestamp uint64 -} -``` - -```go -type TimeoutPacket { - height Height - sourcePort Identifier - sourceChannel Identifier - destPort Identifier - destChannel Identifier - sequence uint64 - timeoutHeight Height - timeoutTimestamp uint64 -} -``` -TODO: Figure out more concise way to define these types as most of them are the same - ## Event handlers ### SendPacketEvent handler @@ -96,8 +42,8 @@ Successful handling of *SendPacketEvent* leads to *PacketRecv* datagram creation ```golang func CreateDatagram(ev SendPacketEvent, - chainA Chain, - chainB Chain, + chainA Chain, // source chain + chainB Chain, // destination chain proofHeight Height) (PacketRecv, Error) { // Stage 1 @@ -145,7 +91,7 @@ func CreateDatagram(ev SendPacketEvent, if ev.sequence != nextSequenceRecv { return (nil, Error.DROP) } // packet has already been delivered by another relayer } else { - // Note that absence of ack (packetAcknowledgment == nil) is also proven also and we should be able to verify it. + // Note that absence of receipt (packetReceipt == nil) is also proven also and we should be able to verify it. packetReceipt, proof = GetPacketReceipt(chainB, ev.destPort, ev.destChannel, ev.sequence, LATEST_HEIGHT) if proof == nil { return (nil, Error.RETRY) } @@ -188,11 +134,10 @@ func CreateDatagram(ev WriteAcknowledgementEvent, GetPacketAcknowledgement(chainA, ev.port, ev.channel, ev.sequence, proofHeight) if packetAckCommitmentProof == nil { return (nil, Error.RETRY) } - if packetAck == nil OR - packetAck != hash(ev.acknowledgement) { - // invalid event; replace provider - ReplaceProvider(chainA) // TODO: We shouldn't replace provider here; it should be communicated with the error type - return (nil, Error.DROP) + if packetAck == nil OR packetAck != hash(ev.acknowledgement) { + // invalid event; replace provider + ReplaceProvider(chainA) // TODO: We shouldn't replace provider here; it should be communicated with the error type + return (nil, Error.DROP) } // Stage 2 @@ -248,14 +193,8 @@ func CreateDatagram(ev WriteAcknowledgementEvent, destPort: ev.port, destChannel: ev.channel, data: ev.data - } - - type PacketAcknowledgement { - packet Packet - acknowledgement byte[] - proof CommitmentProof - proofHeight Height - } + } + return (PacketAcknowledgement { packet, ev.acknowledgement, packetAckCommitmentProof, proofHeight }, nil) } ``` From 1a42436d3b44522de27666ed7add735a29cff73d Mon Sep 17 00:00:00 2001 From: Zarko Milosevic Date: Fri, 18 Sep 2020 13:41:51 +0200 Subject: [PATCH 7/9] Small cleaning --- docs/spec/relayer/Definitions.md | 2 +- docs/spec/relayer/Packets.md | 5 +++-- docs/spec/relayer/Relayer.md | 24 +++++++++++++++--------- 3 files changed, 19 insertions(+), 12 deletions(-) diff --git a/docs/spec/relayer/Definitions.md b/docs/spec/relayer/Definitions.md index 6f29900419..2fcb5eeea8 100644 --- a/docs/spec/relayer/Definitions.md +++ b/docs/spec/relayer/Definitions.md @@ -258,7 +258,7 @@ func GetChannel(chain Chain, while(true) { // Query provable store exposed by the full node of chain. // The path for the channel end is at channelEnds/ports/{portId}/channels/{channelId}". - // The membership proof returned is read at height proofHeight. + // The channel and the membership proof returned is read at height proofHeight - 1. channel, proof, error = QueryChannel(chain.provider, portId, channelId, proofHeight) if error != nil { // elect a new provider from the peer list diff --git a/docs/spec/relayer/Packets.md b/docs/spec/relayer/Packets.md index 09c963c2f8..a8b421c217 100644 --- a/docs/spec/relayer/Packets.md +++ b/docs/spec/relayer/Packets.md @@ -142,8 +142,9 @@ func CreateDatagram(ev WriteAcknowledgementEvent, // Stage 2 // Execute checks IBC handler on chainB will execute - - channelA, proof = GetChannel(chainA, ev.port, ev.channel, LATEST_HEIGHT) + + // Fetch channelEnd from the chainA to be able to compute port and chain ids on destination chain + channelA, proof = GetChannel(chainA, ev.port, ev.channel, ev.height) if proof == nil { return (nil, Error.RETRY) } channelB, proof = diff --git a/docs/spec/relayer/Relayer.md b/docs/spec/relayer/Relayer.md index 220b2258cb..8961045e94 100644 --- a/docs/spec/relayer/Relayer.md +++ b/docs/spec/relayer/Relayer.md @@ -116,19 +116,19 @@ func handleEvent(ev, chainA) Error { // Stage 1. // Determine destination chain - chainB = getDestinationInfo(ev, chainA) - if chainB == nil { return Error.RETRY } + chainB, error = getDestinationInfo(ev, chainA) + if error != nil { return error } // Stage 2. // Update on `chainB` the IBC client for `chainA` to height `>= targetHeight`. targetHeight = ev.height + 1 // See the code for `updateIBCClient` below. - installedHeight, error := updateIBCClient(chainB, chainA, targetHeight) + proofHeight, error := updateIBCClient(chainB, chainA, targetHeight) if error != nil { return Error.RETRY } // Stage 3. // Create the IBC datagrams including `ev` & verify them. - datagram, error = CreateDatagram(ev, chainA, chainB, installedHeight) + datagram, error = CreateDatagram(ev, chainA, chainB, proofHeight) if error != nil { return Error.RETRY } // Stage 4. @@ -137,19 +137,25 @@ func handleEvent(ev, chainA) Error { if error != nil { return Error.RETRY } } -func getDestinationInfo(ev IBCEvent, chain Chain) Chain { +func getDestinationInfo(ev IBCEvent, chain Chain) (Chain, Error) { switch ev.type { case SendPacketEvent: chainId = getChainId(chain, ev.sourcePort, ev.sourceChannel, ev.Height) - if chainId == nil { return nil } + if chainId == nil { return (nil, Error.RETRY) } - return GetChain(chainId) + chain = GetChain(chainId) + if chain == nil { return (nil, Error.DROP) } + + return (chain, nil) case WriteAcknowledgementEvent: chainId = getChainId(chain, ev.Port, ev.Channel, ev.Height) - if chainId == nil { return nil } + if chainId == nil { return nil, Error.RETRY } + + chain = GetChain(chainId) + if chain == nil { nil, Error.DROP } - return GetChain(chainId) + return (chain, nil) } } From d3ce6af15f9124b69d6d584564ebae01d4f94cf0 Mon Sep 17 00:00:00 2001 From: Zarko Milosevic Date: Tue, 22 Sep 2020 12:02:24 +0200 Subject: [PATCH 8/9] Improve error handling --- docs/spec/relayer/Definitions.md | 103 +++++++++++++++---------------- docs/spec/relayer/Packets.md | 61 +++++++++--------- docs/spec/relayer/Relayer.md | 54 +++++++--------- 3 files changed, 100 insertions(+), 118 deletions(-) diff --git a/docs/spec/relayer/Definitions.md b/docs/spec/relayer/Definitions.md index 2fcb5eeea8..2216e5a536 100644 --- a/docs/spec/relayer/Definitions.md +++ b/docs/spec/relayer/Definitions.md @@ -142,24 +142,24 @@ We assume the existence of the following helper functions: GetChannel(chain Chain, portId Identifier, channelId Identifier, - proofHeight Height) (ChannelEnd, CommitmentProof) + proofHeight Height) (ChannelEnd, CommitmentProof, Error) // Returns connection end with a commitment proof. GetConnection(chain Chain, connectionId Identifier, - proofHeight Height) (ConnectionEnd, CommitmentProof) + proofHeight Height) (ConnectionEnd, CommitmentProof, Error) // Returns client state with a commitment proof. GetClientState(chain Chain, clientId Identifier, - proofHeight Height) (ClientState, CommitmentProof) + proofHeight Height) (ClientState, CommitmentProof, Error) // Returns consensus state with a commitment proof. GetConsensusState(chain Chain, clientId Identifier, targetHeight Height, - proofHeight Height) (ConsensusState, CommitmentProof) + proofHeight Height) (ConsensusState, CommitmentProof, Error) // Returns packet commitment with a commitment proof. @@ -167,20 +167,20 @@ GetPacketCommitment(chain Chain, portId Identifier, channelId Identifier, sequence uint64, - proofHeight Height) (bytes, CommitmentProof) + proofHeight Height) (bytes, CommitmentProof, Error) // Returns next recv sequence number with a commitment proof. GetNextSequenceRecv(chain Chain, portId Identifier, channelId Identifier, - proofHeight Height) (uint64, CommitmentProof) + proofHeight Height) (uint64, CommitmentProof, Error) // Returns next recv sequence number with a commitment proof. GetNextSequenceAck(chain Chain, portId Identifier, channelId Identifier, - proofHeight Height) (uint64, CommitmentProof) + proofHeight Height) (uint64, CommitmentProof, Error) // Returns packet acknowledgment with a commitment proof. @@ -188,7 +188,7 @@ GetPacketAcknowledgement(chain Chain, portId Identifier, channelId Identifier, sequence uint64, - proofHeight Height) (bytes, CommitmentProof) + proofHeight Height) (bytes, CommitmentProof, Error) // Returns packet receipt with a commitment proof. @@ -196,7 +196,7 @@ GetPacketReceipt(chain Chain, portId Identifier, channelId Identifier, sequence uint64, - proofHeight Height) (String, CommitmentProof) + proofHeight Height) (String, CommitmentProof, Error) // Returns estimate of the consensus height on the given chain. @@ -229,13 +229,45 @@ Submit(chain Chain, datagram IBCDatagram) Error GetChain(chainID String) Chain ``` -For functions that return proof, if proof != nil, then the returned value is being verified. +For functions that return proof, if `error == nil`, then the returned value is being verified. The value is being verified using the header's app hash that is provided by the corresponding light client. +We now show the pseudocode for one of those functions: + +```go +func GetChannel(chain Chain, + portId Identifier, + channelId Identifier, + proofHeight Height) (ChannelEnd, CommitmentProof, Error) { + + // Query provable store exposed by the full node of chain. + // The path for the channel end is at channelEnds/ports/{portId}/channels/{channelId}". + // The channel and the membership proof returned is read at height proofHeight - 1. + channel, proof, error = QueryChannel(chain.provider, portId, channelId, proofHeight) + if error != nil { return (nil, nil, Error.BADPROVIDER) } + + header, error = GetHeader(chain.lc, proofHeight) // get header for height proofHeight using light client + if error != nil { return (nil, nil, Error.BADLIGHTCLIENT) } // return if light client can't provide header for the given height + + // verify membership of the channel at path channelEnds/ports/{portId}/channels/{channelId} using + // the root hash header.AppHash + if !VerifyMembership(header.AppHash, proofHeight, proof, channelPath(portId, channelId), channel) { + // membership check fails; therefore provider is faulty. Try to elect new provider + return (nil, nil, Error.BadProvider) + } + + return (channel, proof, nil) +} +``` + +If *LATEST_HEIGHT* is passed as a parameter, the data should be read (and the corresponding proof created) +at the most recent height. + + ### Error handling Helper functions listed above assume querying (parts of the) application state using Tendermint RPC. For example, -`GetChannel` relies on `QueryChannel`. RPC calls can fail as +`GetChannel` relies on `QueryChannel`. RPC calls can fail if: - no response is received within some timeout or - malformed response is received. @@ -247,47 +279,14 @@ simpler to blame the provider (assume implicitly network is always correct and r always respond timely with a correct response, while in case of errors we consider the provider node faulty, and then we replace it with a different node. -We now show the pseudocode for one of those functions that contains simplified error handling logic: - -```go -func GetChannel(chain Chain, - portId Identifier, - channelId Identifier, - proofHeight Height) (ChannelEnd, CommitmentProof) { - - while(true) { - // Query provable store exposed by the full node of chain. - // The path for the channel end is at channelEnds/ports/{portId}/channels/{channelId}". - // The channel and the membership proof returned is read at height proofHeight - 1. - channel, proof, error = QueryChannel(chain.provider, portId, channelId, proofHeight) - if error != nil { - // elect a new provider from the peer list - if !ReplaceProvider(chain) { return (nil, nil) } // return if fail to elect new provider - } - - header, error = GetHeader(chain.lc, proofHeight) // get header for height proofHeight using light client - if error != nil { return (nil, nil) } // return if light client can't provide header for the given height - - // verify membership of the channel at path channelEnds/ports/{portId}/channels/{channelId} using - // the root hash header.AppHash - if VerifyMembership(header.AppHash, proofHeight, proof, channelPath(portId, channelId), channel) { - return (channel, proof) - } - - // membership check fails; therefore provider is faulty. Try to elect new provider - if !ReplaceProvider(chain) { return (nil, nil) } // if fails to elect new provider return - } - panic // should never reach this line -} +We assume the following error types: -// Simplified version of logic for electing new provider. In reality it will probably involve opening a connection to -// a newply elected provider and closing connection with an old provider. -func ReplaceProvider(chain Chain) boolean { - if chain.peerList.IsEmpty() return false - chain.provider = Head(chain.peerList) - chain.peerList = Tail(chain.peerList) - return true +```golang +enum Error { + RETRY, // transient processing error (for example due to optimistic send); function can be retried later + DROP, // event has already been received by the destination chain so it should be dropped + BADPROVIDER, // provider does not reply timely or with a correct data; it normally leads to replacing provider + BADLIGHTCLIENT // light client does not reply timely or with a correct data } ``` -If *LATEST_HEIGHT* is passed as a parameter, the data should be read (and the corresponding proof created) -at the most recent height. + diff --git a/docs/spec/relayer/Packets.md b/docs/spec/relayer/Packets.md index a8b421c217..94e6db4bb7 100644 --- a/docs/spec/relayer/Packets.md +++ b/docs/spec/relayer/Packets.md @@ -49,22 +49,21 @@ func CreateDatagram(ev SendPacketEvent, // Stage 1 // Verify if packet is committed to chain A and it is still pending (commitment exists) - packetCommitment, packetCommitmentProof = + packetCommitment, packetCommitmentProof, error = GetPacketCommitment(chainA, ev.sourcePort, ev.sourceChannel, ev.sequence, proofHeight) - if packetCommitmentProof == nil { return (nil, Error.RETRY) } + if error != nil { return (nil, error) } if packetCommitment == nil OR packetCommitment != hash(concat(ev.data, ev.timeoutHeight, ev.timeoutTimestamp)) { - // invalid event; replace provider - ReplaceProvider(chainA) - return (nil, Error.DROP) + // invalid event; bad provider + return (nil, Error.BADPROVIDER) } // Stage 2 // Execute checks IBC handler on chainB will execute - channel, proof = GetChannel(chainB, ev.destPort, ev.destChannel, LATEST_HEIGHT) - if proof == nil { return (nil, Error.RETRY) } + channel, proof, error = GetChannel(chainB, ev.destPort, ev.destChannel, LATEST_HEIGHT) + if error != nil { return (nil, error) } if channel != nil AND (channel.state == CLOSED OR @@ -75,8 +74,8 @@ func CreateDatagram(ev SendPacketEvent, // TODO: Maybe we shouldn't even enter handle loop for packets if the corresponding channel is not open! connectionId = channel.connectionHops[0] - connection, proof = GetConnection(chainB, connectionId, LATEST_HEIGHT) - if proof == nil { return (nil, Error.RETRY) } + connection, proof, error = GetConnection(chainB, connectionId, LATEST_HEIGHT) + if error != nil { return (nil, error) } if connection == nil OR connection.state != OPEN { return (nil, Error.RETRY) } @@ -85,16 +84,16 @@ func CreateDatagram(ev SendPacketEvent, // we now check if this packet is already received by the destination chain if channel.ordering === ORDERED { - nextSequenceRecv, proof = GetNextSequenceRecv(chainB, ev.destPort, ev.destChannel, LATEST_HEIGHT) - if proof == nil { return (nil, Error.RETRY) } + nextSequenceRecv, proof, error = GetNextSequenceRecv(chainB, ev.destPort, ev.destChannel, LATEST_HEIGHT) + if error != nil { return (nil, error) } if ev.sequence != nextSequenceRecv { return (nil, Error.DROP) } // packet has already been delivered by another relayer } else { // Note that absence of receipt (packetReceipt == nil) is also proven also and we should be able to verify it. - packetReceipt, proof = + packetReceipt, proof, error = GetPacketReceipt(chainB, ev.destPort, ev.destChannel, ev.sequence, LATEST_HEIGHT) - if proof == nil { return (nil, Error.RETRY) } + if error != nil { return (nil, error) } if packetReceipt != nil { return (nil, Error.DROP) } // packet has already been delivered by another relayer } @@ -128,57 +127,53 @@ func CreateDatagram(ev WriteAcknowledgementEvent, // Stage 1 // Verify if acknowledment is committed to chain A and it is still pending - - // TODO: Figure out what should be the height at which this info is read! - packetAck, packetAckCommitmentProof = + packetAck, packetAckCommitmentProof, error = GetPacketAcknowledgement(chainA, ev.port, ev.channel, ev.sequence, proofHeight) - if packetAckCommitmentProof == nil { return (nil, Error.RETRY) } + if error != nil { return (nil, error) } if packetAck == nil OR packetAck != hash(ev.acknowledgement) { - // invalid event; replace provider - ReplaceProvider(chainA) // TODO: We shouldn't replace provider here; it should be communicated with the error type - return (nil, Error.DROP) + // invalid event; bad provider + return (nil, Error.BADPROVIDER) } // Stage 2 // Execute checks IBC handler on chainB will execute // Fetch channelEnd from the chainA to be able to compute port and chain ids on destination chain - channelA, proof = GetChannel(chainA, ev.port, ev.channel, ev.height) - if proof == nil { return (nil, Error.RETRY) } + channelA, proof, error = GetChannel(chainA, ev.port, ev.channel, ev.height) + if error != nil { return (nil, error) } - channelB, proof = + channelB, proof, error = GetChannel(chainB, channelA.counterpartyPortIdentifier, channelA.counterpartyChannelIdentifier, LATEST_HEIGHT) - if proof == nil { return (nil, Error.RETRY) } + if error != nil { return (nil, error) } if channelB == nil OR channel.state != OPEN { (nil, Error.DROP) } // Note that we checked implicitly above that counterparty identifiers match each other connectionId = channelB.connectionHops[0] - connection, proof = GetConnection(chainB, connectionId, LATEST_HEIGHT) - if proof == nil { return (nil, Error.RETRY) } + connection, proof, error = GetConnection(chainB, connectionId, LATEST_HEIGHT) + if error != nil { return (nil, error) } if connection == nil OR connection.state != OPEN { return (nil, Error.DROP) } // verify the packet is sent by chainB and hasn't been cleared out yet - packetCommitment, packetCommitmentProof = + packetCommitment, packetCommitmentProof, error = GetPacketCommitment(chainB, channelA.counterpartyPortIdentifier, channelA.counterpartyChannelIdentifier, ev.sequence, LATEST_HEIGHT) - if packetCommitmentProof == nil { return (nil, Error.RETRY) } + if error != nil { return (nil, error) } if packetCommitment == nil OR packetCommitment != hash(concat(ev.data, ev.timeoutHeight, ev.timeoutTimestamp)) { - // invalid event; replace provider - ReplaceProvider(chainB) - return (nil, Error.DROP) + // invalid event; bad provider + return (nil, Error.BADPROVIDER) } // abort transaction unless acknowledgement is processed in order if channelB.ordering === ORDERED { - nextSequenceAck, proof = + nextSequenceAck, proof, error = GetNextSequenceAck(chainB, channelA.counterpartyPortIdentifier, channelA.counterpartyChannelIdentifier, ev.sequence, LATEST_HEIGHT) - if proof == nil { return (nil, Error.RETRY) } + if error != nil { return (nil, error) } if ev.sequence != nextSequenceAck { return (nil, Error.DROP) } } diff --git a/docs/spec/relayer/Relayer.md b/docs/spec/relayer/Relayer.md index 8961045e94..88b8a33e02 100644 --- a/docs/spec/relayer/Relayer.md +++ b/docs/spec/relayer/Relayer.md @@ -94,21 +94,9 @@ succeed. The interface between stage 3 and stage 4 is an IBC datagram. We assume that the corresponding light client is correctly installed on each chain. -The semantic of the returned error is to signal to the caller whether there was an error in the event processing that -could be transient (for example not able to establish connection to a correct full node), in which case -the event can be reprocessed later (`Error.RETRY`), or the event should be dropped as it has already been -received by the destination chain due to activity of a concurrent relayer (`Error.DROP`). - Data structures and helper function definitions are provided [here](https://github.com/informalsystems/ibc-rs/blob/master/docs/spec/relayer/Definitions.md). -```golang -enum Error { - RETRY, - DROP, -} -``` - ```golang func handleEvent(ev, chainA) Error { // NOTE: we don't verify if event data are valid at this point. We trust full node we are connected to @@ -124,24 +112,24 @@ func handleEvent(ev, chainA) Error { targetHeight = ev.height + 1 // See the code for `updateIBCClient` below. proofHeight, error := updateIBCClient(chainB, chainA, targetHeight) - if error != nil { return Error.RETRY } + if error != nil { return error } // Stage 3. // Create the IBC datagrams including `ev` & verify them. datagram, error = CreateDatagram(ev, chainA, chainB, proofHeight) - if error != nil { return Error.RETRY } + if error != nil { return error } // Stage 4. // Submit datagrams. error = Submit(chainB, datagram) - if error != nil { return Error.RETRY } + if error != nil { return error } } func getDestinationInfo(ev IBCEvent, chain Chain) (Chain, Error) { switch ev.type { case SendPacketEvent: - chainId = getChainId(chain, ev.sourcePort, ev.sourceChannel, ev.Height) - if chainId == nil { return (nil, Error.RETRY) } + chainId, error = getChainId(chain, ev.sourcePort, ev.sourceChannel, ev.Height) + if error != nil { return (nil, error) } chain = GetChain(chainId) if chain == nil { return (nil, Error.DROP) } @@ -149,8 +137,8 @@ func getDestinationInfo(ev IBCEvent, chain Chain) (Chain, Error) { return (chain, nil) case WriteAcknowledgementEvent: - chainId = getChainId(chain, ev.Port, ev.Channel, ev.Height) - if chainId == nil { return nil, Error.RETRY } + chainId, error = getChainId(chain, ev.Port, ev.Channel, ev.Height) + if error != nil { return (nil, error) } chain = GetChain(chainId) if chain == nil { nil, Error.DROP } @@ -160,18 +148,18 @@ func getDestinationInfo(ev IBCEvent, chain Chain) (Chain, Error) { } // Return chaindId of the destination chain based on port and channel info for the given chain -func getChainId(chain Chain, port Identifier, channel Identifier, height Height) String { - channel, proof = GetChannel(chain, port, channel, height) - if proof == nil { return nil } +func getChainId(chain Chain, port Identifier, channel Identifier, height Height) (String, Error) { + channel, proof, error = GetChannel(chain, port, channel, height) + if error != nil { return (nil, error) } connectionId = channel.connectionHops[0] - connection, proof = GetConnection(chain, connectionId, height) - if proof == nil { return nil } + connection, proof, error = GetConnection(chain, connectionId, height) + if error != nil { return (nil, error) } - clientState, proof = GetClientState(chain, connection.clientIdentifier, height) - if proof == nil { return nil } + clientState, proof, error = GetClientState(chain, connection.clientIdentifier, height) + if error != nil { return (nil, error) } - return clientState.chainID + return (clientState.chainID, error) } // Perform an update on `dest` chain for the IBC client for `src` chain. @@ -182,21 +170,21 @@ func getChainId(chain Chain, port Identifier, channel Identifier, height Height) // - return error if some of verification steps fail func updateIBCClient(dest Chain, src Chain, targetHeight Height) -> (Height, Error) { - clientState, proof = GetClientState(dest, dest.clientId, LATEST_HEIGHT) - if proof == nil { return (nil, Error.RETRY) } + clientState, proof, error = GetClientState(dest, dest.clientId, LATEST_HEIGHT) + if error != nil { return (nil, error) } // NOTE: What if a full node we are connected to send us stale (but correct) information regarding targetHeight? // if installed height is smaller than the targetHeight, we need to update client with targetHeight while (clientState.latestHeight < targetHeight) { // Do an update to IBC client for `src` on `dest`. shs, error = src.lc.getMinimalSet(clientState.latestHeight, targetHeight) - if error != nil { return (nil, Error.RETRY) } + if error != nil { return (nil, error) } error = dest.submit(createUpdateClientDatagrams(shs)) - if error != nil { return (nil, Error.RETRY) } + if error != nil { return (nil, error) } - clientState, proof = GetClientState(dest, dest.clientId, LATEST_HEIGHT) - if proof == nil { return (nil, Error.RETRY) } + clientState, proof, error = GetClientState(dest, dest.clientId, LATEST_HEIGHT) + if error != nil { return (nil, error) } } // NOTE: semantic check of the installed header is done using fork detection component From 777957ed472e5b99164bb0d1dd3b7c569e320802 Mon Sep 17 00:00:00 2001 From: Zarko Milosevic Date: Tue, 22 Sep 2020 13:06:51 +0200 Subject: [PATCH 9/9] Improve error handling --- docs/spec/relayer/Definitions.md | 48 ++++++++++++++++---------------- docs/spec/relayer/Packets.md | 4 +-- docs/spec/relayer/Relayer.md | 6 ++-- 3 files changed, 29 insertions(+), 29 deletions(-) diff --git a/docs/spec/relayer/Definitions.md b/docs/spec/relayer/Definitions.md index 2216e5a536..5d8830a717 100644 --- a/docs/spec/relayer/Definitions.md +++ b/docs/spec/relayer/Definitions.md @@ -232,6 +232,30 @@ GetChain(chainID String) Chain For functions that return proof, if `error == nil`, then the returned value is being verified. The value is being verified using the header's app hash that is provided by the corresponding light client. +Helper functions listed above assume querying (parts of the) application state using Tendermint RPC. For example, +`GetChannel` relies on `QueryChannel`. RPC calls can fail if: + +- no response is received within some timeout or +- malformed response is received. + +In both cases, error handling logic should be defined by the caller. For example, in the former case, the caller might +retry sending the same request to a same provider (full node), while in the latter case the request might be sent to +some other provider node. Although these kinds of errors could be due to network infrastructure issues, it is normally +simpler to blame the provider (assume implicitly network is always correct and reliable). Therefore, correct provider +always respond timely with a correct response, while in case of errors we consider the provider node faulty, and then +we replace it with a different node. + +We assume the following error types: + +```golang +enum Error { + RETRY, // transient processing error (for example due to optimistic send); function can be retried later + DROP, // event has already been received by the destination chain so it should be dropped + BADPROVIDER, // provider does not reply timely or with a correct data; it normally leads to replacing provider + BADLIGHTCLIENT // light client does not reply timely or with a correct data +} +``` + We now show the pseudocode for one of those functions: ```go @@ -264,29 +288,5 @@ If *LATEST_HEIGHT* is passed as a parameter, the data should be read (and the co at the most recent height. -### Error handling -Helper functions listed above assume querying (parts of the) application state using Tendermint RPC. For example, -`GetChannel` relies on `QueryChannel`. RPC calls can fail if: - -- no response is received within some timeout or -- malformed response is received. - -In both cases, error handling logic should be defined by the caller. For example, in the former case, the caller might -retry sending the same request to a same provider (full node), while in the latter case the request might be sent to -some other provider node. Although these kinds of errors could be due to network infrastructure issues, it is normally -simpler to blame the provider (assume implicitly network is always correct and reliable). Therefore, correct provider -always respond timely with a correct response, while in case of errors we consider the provider node faulty, and then -we replace it with a different node. - -We assume the following error types: - -```golang -enum Error { - RETRY, // transient processing error (for example due to optimistic send); function can be retried later - DROP, // event has already been received by the destination chain so it should be dropped - BADPROVIDER, // provider does not reply timely or with a correct data; it normally leads to replacing provider - BADLIGHTCLIENT // light client does not reply timely or with a correct data -} -``` diff --git a/docs/spec/relayer/Packets.md b/docs/spec/relayer/Packets.md index 94e6db4bb7..b4374ac4c1 100644 --- a/docs/spec/relayer/Packets.md +++ b/docs/spec/relayer/Packets.md @@ -68,9 +68,9 @@ func CreateDatagram(ev SendPacketEvent, if channel != nil AND (channel.state == CLOSED OR ev.sourcePort != channel.counterpartyPortIdentifier OR - ev.sourceChannel != channel.counterpartyChannelIdentifier) { (nil, Error.DROP) } + ev.sourceChannel != channel.counterpartyChannelIdentifier) { return (nil, Error.DROP) } - if channel == nil OR channel.state != OPEN { (nil, Error.RETRY) } + if channel == nil OR channel.state != OPEN { return (nil, Error.RETRY) } // TODO: Maybe we shouldn't even enter handle loop for packets if the corresponding channel is not open! connectionId = channel.connectionHops[0] diff --git a/docs/spec/relayer/Relayer.md b/docs/spec/relayer/Relayer.md index 88b8a33e02..4158c38dfe 100644 --- a/docs/spec/relayer/Relayer.md +++ b/docs/spec/relayer/Relayer.md @@ -79,10 +79,10 @@ chain at height *h*. The trusted header is provided by the corresponding light c The main relayer event loop is a pipeline of four stages. Assuming some IBC event at height `h` on `chainA`, the relayer: -1. Determine destination chain (`chainB`) +1. Determines destination chain (`chainB`) 2. Updates (on `chainB`) the IBC client for `chainA` to a certain height `H` where `H >= h+1`. -3. Create IBC datagram at height `H-1`. -4. Submit the datagram from stage (2) to `chainB`. +3. Creates IBC datagram at height `H-1`. +4. Submits the datagram from stage (2) to `chainB`. Note that an IBC event at height `h` corresponds to the modifications to the data store made as part of executing block at height `h`. The corresponding proof (that data is indeed written to the data store) can be verified using