Skip to content

fix: cleanup close init after close confirmed #1326

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Nov 11, 2023
Merged

fix: cleanup close init after close confirmed #1326

merged 2 commits into from
Nov 11, 2023

Conversation

mmsqe
Copy link
Contributor

@mmsqe mmsqe commented Nov 10, 2023

Closes: #1323

mmsqe
mmsqe commented Nov 10, 2023
View reviewed changes
jtieri
jtieri approved these changes Nov 11, 2023
View reviewed changes
Copy link
Contributor

@jtieri jtieri left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch. it's interesting that our e2e tests did not catch this issue given we have tests here and here where we let an ICA channel close due to a packet timeout and then proceed to reopen a channel

@jtieri jtieri merged commit 27cf122 into cosmos:main Nov 11, 2023
@mmsqe
Copy link
Contributor Author

mmsqe commented Nov 12, 2023

good catch. it's interesting that our e2e tests did not catch this issue given we have tests here and here where we let an ICA channel close due to a packet timeout and then proceed to reopen a channel

I did see some retry error after successful tx of channel close in close test before.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jackzampolin jackzampolin Awaiting requested review from jackzampolin

@boojamya boojamya Awaiting requested review from boojamya

@mark-rushakoff mark-rushakoff Awaiting requested review from mark-rushakoff

1 more reviewer

@jtieri jtieri jtieri approved these changes

Reviewers whose approvals may not affect merge requirements
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

keep sending channel_close_confirm to dst chain after channel close successfully
2 participants
@mmsqe @jtieri