Height in update_client is always greater than proofHeights in the sending messages/acknowledgements

[songwongtp]

Hi all,

Sometimes when I am sending a message between my two chains, I would get an error like the below:

failed to execute message; message index: 1: acknowledge packet verification failed: packet acknowledgement verification failed: failed packet acknowledgement verification for client (07-tendermint-0): consensus state does not exist for height 0-82: consensus state not found: invalid request

So my friend and I did some digging into the code and find out that packet commit responses are generated with FetchCommitResponse at the beginning of sendTxFromEventPackets method and never get updated again. So in a case when updateMsg somehow has its height higher than the packet commit responses created previously. The height in updateMsg is always greater than proofHeight in the commit responses, since each retry.Do in sendTxFromEventPackets only updates updateMsg with the lastest height, not the commit responses. With this, light client can never verify those old proofHeight in the commit responses of messages/acknowledgements.

Then we did some quick fix with the following steps

1. We moved FetchCommitResponse into rlyPackets loop in retry.Do right before calling rp.Msg(src, dst) as the following

for _, rp := range rlyPackets {
	rp.FetchCommitResponse(src, dst)
	msg, err := rp.Msg(src, dst)
	if err != nil {
		if src.debug {
			src.Log(fmt.Sprintf("- [%s] failed to create relay packet message bound for %s of type %T, retrying: %s",
				src.ChainID, dst.ChainID, rp, err))
		}
		return err
	}
	txs.Src = append(txs.Src, msg)
}

2. Moving FetchCommitResponse did help with the error, but there is still a chance that updateClient height is not always equal to proofHeight in commit responses after multiple retries. So we modified FetchCommitResponse to take targetHeight as the third argument and passed the latest height from updateMsg which is created before rlyPackets loop.

There might be a better way to do this, but I just want to let you know what we did to fix the error.

[colin-axner]

Hello! Thanks for the nice writeup! Very good conclusions. This issue is also discussed in #425. I like your solution added in (2).

Do you think you could upstream the fix?

[songwongtp]

Sure I have created a new PR here #490

[colin-axner]

closed by #490