Skip to content
New issue

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

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

Already on GitHub? # to your account

Refactor Localhost #27

Closed
4 tasks
colin-axner opened this issue Nov 10, 2020 · 5 comments
Closed
4 tasks

Refactor Localhost #27

colin-axner opened this issue Nov 10, 2020 · 5 comments

Comments

@colin-axner
Copy link
Contributor

Summary

When creating connections and channels we do not store them under client prefixed stores. They are stored under the IBC prefix and then either channelEnds or connections. When verifying connection and channel states for light clients we pass the client prefixed store, so the client can access the consensus states if necessary. The localhost was using this client prefixed store to verify connections/channels. Unfortunately, connections and channels are not stored there, so this check would always fail.


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@cwgoes
Copy link
Contributor

cwgoes commented Nov 10, 2020

In principle, the localhost client should just have a read-only wrapper over the IBC store. Sounds like that might take awhile.

For now, I think it is reasonable to disable the localhost client.

@colin-axner
Copy link
Contributor Author

Also relevant to fixing localhost Localhost acknowledgement verification needs to use types.CommitAcknowledgement

@colin-axner
Copy link
Contributor Author

From cosmos/cosmos-sdk#7706

Localhost is just the chain ID and height of the current executing chain's context. We should support a dynamic query using the context. There is no need to store it and have custom logic in Create and Update client handling. We should refactor this out.

After further inquiry, we realized we cannot remove localhost from storage without injecting localhost logic into connection handshakes since the client won't always have access to the context. Instead we will remove localhost logic from ClientKeeper.CreateClient and ClientKeeper.UpdateClient and continue to store the localhost, since storage costs are tiny

We should just redesign the localhost's interaction with core IBC. cc @AdityaSripal for ideas on how to do this

@colin-axner colin-axner transferred this issue from cosmos/cosmos-sdk Mar 4, 2021
@colin-axner
Copy link
Contributor Author

ref #75

faddat referenced this issue in notional-labs/ibc-go Feb 23, 2022
@colin-axner
Copy link
Contributor Author

Localhost will be removed in v6 as there are no plans to fix it

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

No branches or pull requests

2 participants