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.

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

ICA: OnChanCloseInit should return error #416

Closed
3 tasks
colin-axner opened this issue Sep 16, 2021 · 4 comments
Closed
3 tasks

ICA: OnChanCloseInit should return error #416

colin-axner opened this issue Sep 16, 2021 · 4 comments
Labels
27-interchain-accounts
Milestone
Interchain Accounts

Comments

@colin-axner
Copy link
Contributor

Summary

The current code allows users to close channels. The interchain accounts callback should return an error

For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged/assigned
@colin-axner colin-axner added this to the Interchain Accounts milestone Sep 16, 2021
@seantking
Copy link
Contributor

Users are blocked from closing channels, see: https://github.com/cosmos/ibc-go/blob/interchain-accounts/modules/apps/27-interchain-accounts/module.go#L184

I left some commented-out code in handshake.go that be confusing.

@colin-axner
Copy link
Contributor Author

Returning nil allows users to close channels

A user sends CloseChanInit msg, if the app callback doesn't return an error, core IBC will close the channel. The comment is incorrect

@colin-axner
Copy link
Contributor Author

The close confirm is also incorrect, if the counterparty closes the channel, we should probably allow the channel to be closed

@seantking
Copy link
Contributor

seantking commented Sep 17, 2021 •
edited
Loading

Gotcha. Good catch. 👍 Updating this as part of a PR here - > #420

@seantking seantking mentioned this issue Sep 17, 2021
Merged
9 tasks
@crodriguezvega crodriguezvega moved this to Done in ibc-go Dec 30, 2021
faddat pushed a commit to notional-labs/ibc-go that referenced this issue Mar 1, 2022
@colin-axner
Refactor light client handling, remove reliance on histroical info (c…
872dc2e 
…osmos#416)

* use light client not chain to get trusted headers

* refactor light client handling

Remove SyncHeaders entirely. Reduce light client API. We can update off chain light clients. Then we can get creation or update headers to handle on chain light clients. No other functions necessary

* continue fixing cmd and self review suggestions

* fix build

* update error message and add todo

* fix lgtm

* use uint64 in latest height return

* fix lint

* more self review fixes

* apply @AdityaSripal review suggestions

* fix lint

* add - 1 notes cc @fedekunze

* add more -1 notes

* fix golang ci
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
27-interchain-accounts
Projects
ibc-go
Archived in project
Milestone
Interchain Accounts
Development

No branches or pull requests
3 participants
@crodriguezvega @seantking @colin-axner