Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refactor update client and packet relay some more #425

Closed
colin-axner opened this issue Feb 12, 2021 · 6 comments · Fixed by #488
Closed

Refactor update client and packet relay some more #425

colin-axner opened this issue Feb 12, 2021 · 6 comments · Fixed by #488
Assignees
Labels
T: Refactor TYPE: Refactor

Comments

@colin-axner
Copy link
Contributor

I'm fairly pleased with the recent refactor of update clients, it reduced the code quite a bit and improved readability. It did however also reveal a sort of brittleness built into the relayer. Anytime we need to provide proof on chain, a consensus state must exist on chain. In #424, I restructured the updates to occur once at the beginning, however this convention is not stable when retries are done by simply updating the light clients and not reconstructing the update client message. For handshakes this is fine, but relaying packets and acknowledgements relies on only retrying by updating the light client.

We should construct a helper function that wraps relayer messages with an update client function. It should switch on the type of message, if the message contains proof, it should construct the update client message based on that proof height otherwise it should use the latest client available.

@colin-axner
Copy link
Contributor Author

The function that wraps a message with an update client should take in a proof height, 0 indicates use the latest ie consensus state height doesn't matter

@colin-axner colin-axner self-assigned this Feb 18, 2021
@colin-axner
Copy link
Contributor Author

colin-axner commented Feb 18, 2021

Note to self:

$ rly tx relay-packets demo -d
failed to execute message; message index: 1: timeout packet verification failed: clientID (07-tendermint-0), height (1-1722): consensus state not found: invalid request
failed to execute message; message index: 1: receive packet verification failed: couldn't verify counterparty packet commitment: failed packet commitment verification for client (07-tendermint-0): consensus state does not exist for height 0-1730: consensus state not found: invalid request

Going to fix today

Edit: false alarm, issue was related to a broken rly paths generate cmd

@hacheigriega
Copy link
Contributor

Hi @colin-axner. Can I bring your attention to the following error I am getting in relaying acknowledgement?

$ rly tx relay-acknowledgements demo -d
I[2021-03-31|16:24:26.048] - [yulei-1]@{67} - try(1/5) query packet acknowledgement: %!s(<nil>)
I[2021-03-31|16:24:27.088] - [yulei-1]@{68} - try(2/5) query packet acknowledgement: %!s(<nil>)
I[2021-03-31|16:24:28.441] - [yulei-1]@{69} - try(3/5) query packet acknowledgement: %!s(<nil>)
I[2021-03-31|16:24:30.681] - [yulei-1]@{71} - try(4/5) query packet acknowledgement: %!s(<nil>)
I[2021-03-31|16:24:34.415] - [yulei-1]@{74} - try(5/5) query packet acknowledgement: %!s(<nil>)
E[2021-03-31|16:07:14.867] yulei-1: err(portID (transfer), channelID (channel-0), sequence (1): invalid acknowledgement)
Error: portID (transfer), channelID (channel-0), sequence (1): invalid acknowledgement

This issue disappeared after #424 but has reemerged since #431. Do you think it's plausible that #431 contained some errors? Thanks.

@colin-axner
Copy link
Contributor Author

colin-axner commented Mar 31, 2021

Thanks for reporting this. Yes it is possible #431 contained some errors, but #424 shouldn't have fixed this issue. Can you show me the full logs? What steps did you take before getting this error?

@hacheigriega
Copy link
Contributor

hacheigriega commented Apr 1, 2021

I am just trying to test a transfer transaction between two chains. I tried the following on #431:

$ rly tx transfer yulei-1 regen-1 333333uctk $(rly chains address regen-1) --path demo
I[2021-04-01|13:43:06.687] ✔ [yulei-1]@{40} - msg(0:transfer) hash(B5DF4BD45ED2B004A4E65C26B6543678F94B983158254D602A4CC854DCF63F80)

$ rly query unrelayed-packets demo
{"src":[1],"dst":null}

$ rly tx relay-packets demo -d
I[2021-04-01|13:43:19.027] ✔ [regen-1]@{50} - msg(0:update_client,1:recv_packet) hash(380A354A0ADD7339945D94F813184A55AB2883C7A2041A1842B45CC35D2C0F2B)
I[2021-04-01|13:43:19.027] ★ Relayed 1 packets: [yulei-1]port{transfer}->[regen-1]port{transfer}

$ rly query unrelayed-acknowledgements demo
{"src":null,"dst":[1]}

$ rly tx relay-acknowledgements demo -d
I[2021-04-01|13:43:32.368] - [yulei-1]@{60} - try(1/5) query packet acknowledgement: %!s(<nil>)
I[2021-04-01|13:43:33.737] - [yulei-1]@{61} - try(2/5) query packet acknowledgement: %!s(<nil>)
I[2021-04-01|13:43:35.143] - [yulei-1]@{62} - try(3/5) query packet acknowledgement: %!s(<nil>)
I[2021-04-01|13:43:37.374] - [yulei-1]@{64} - try(4/5) query packet acknowledgement: %!s(<nil>)
I[2021-04-01|13:43:40.995] - [yulei-1]@{67} - try(5/5) query packet acknowledgement: %!s(<nil>)
E[2021-04-01|13:43:40.995] yulei-1: err(portID (transfer), channelID (channel-0), sequence (1): invalid acknowledgement)
Error: portID (transfer), channelID (channel-0), sequence (1): invalid acknowledgement

These steps work fine on #424. Thanks again.

@colin-axner
Copy link
Contributor Author

@hacheigriega Sorry for the late reply. Thanks for the logs! That helped me identify the issue really fast. I will open up a pr to fix the issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T: Refactor TYPE: Refactor
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants