English spec for SendPacketEvent handling #230
Conversation
There was a problem hiding this comment.
Looks good in general.
I'm missing a bit of context and explanations on why it is necessary for the relayer to do verification before creating and submitting the datagrams. Also, it seems that the last paragraph of Relayer.md is incomplete.
docs/spec/relayer/Packets.md
Outdated
| proofHeight Height) (ConnectionEnd, CommitmentProof) | ||
|
|
||
|
|
||
| // Returns client connection with a commitment proof. If proof != nil, then it is being verified with the corresponding light client. |
There was a problem hiding this comment.
| // Returns client connection with a commitment proof. If proof != nil, then it is being verified with the corresponding light client. | |
| // Returns client state with a commitment proof. If proof != nil, then it is being verified with the corresponding light client. |
docs/spec/relayer/Packets.md
Outdated
| sequence uint64, | ||
| proofHeight Height) (bytes, CommitmentProof) | ||
|
|
||
| // Returns next recv sequence number a commitment proof. If proof != nil, then it is being verified with the corresponding |
There was a problem hiding this comment.
| // Returns next recv sequence number a commitment proof. If proof != nil, then it is being verified with the corresponding | |
| // Returns next recv sequence number with a commitment proof. If proof != nil, then it is being verified with the corresponding |
docs/spec/relayer/Packets.md
Outdated
| if proof == nil return nil | ||
|
|
||
| clientState = GetClientState(chainA, connection.clientIdentifier, ev.Height) | ||
| return getHostInfo(clientStateA.chainID) |
There was a problem hiding this comment.
| return getHostInfo(clientStateA.chainID) | |
| return getHostInfo(clientState.chainID) |
docs/spec/relayer/Packets.md
Outdated
| ### PacketRecv datagram creation | ||
|
|
||
| ```golang | ||
| func createDatagram(ev SendPacketEvent, chainA Chain, chainB Chain, installedHeight Height) PacketRecv { |
There was a problem hiding this comment.
Maybe this function should be called createPacketRecvDatagram.
docs/spec/relayer/Packets.md
Outdated
| connection, proof = GetConnection(chainB, connectionId, LATEST_HEIGHT) | ||
| if proof != nil { return nil } | ||
|
|
||
| if connectionB == null OR connectionB.state != OPEN { return nil } |
There was a problem hiding this comment.
| if connectionB == null OR connectionB.state != OPEN { return nil } | |
| if connection == null OR connection.state != OPEN { return nil } |
| // 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 } |
There was a problem hiding this comment.
Move this check below the sequence matching check and change it to:
| if proof != nil { return nil } | |
| if proof == nil { return nil } |
i.e. first we check if the chain expects exactly the ev.sequence. Not sure what to do if it doesn't though. Maybe the packet was delivered or maybe a full rpc node is lying?
Then if we ask for a proof and we don't get one something is wrong -> change the rpc full node?
Then do the proof verification, if it fails -> change the rpc full node?
I don't know the safest sequence here...and I think we trust the full node here? So maybe ignore.
There was a problem hiding this comment.
We trust light client, so everything checked by the light client (via header's app hash) we also trust. I will add more on error handling in the next version, but in general we should probably switch to different rpc node within GetX functions, but handler ideally don't need to care about that. But this need to be clarified.
Add spec for SendPacketEvent handling and PacketRecv creation logic
Addresses #229.