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

[Router] Missing parts of ICS05 & ICS26 that deal with the Module management #519

Open
Tracked by #554
Farhad-Shabani opened this issue Mar 10, 2023 · 1 comment · May be fixed by #1258
Open
Tracked by #554

[Router] Missing parts of ICS05 & ICS26 that deal with the Module management #519

Farhad-Shabani opened this issue Mar 10, 2023 · 1 comment · May be fixed by #1258
Assignees
Labels
A: breaking Admin: breaking change that may impact operators O: usability Objective: aims to enhance user experience (UX) and streamline product usability S: specs Scope: related to IBC protocol specifications

Comments

@Farhad-Shabani
Copy link
Member

Farhad-Shabani commented Mar 10, 2023

Summary

Missing parts of ICS05 & ICS26 that deal with the Module management

Problem Statement

We currently only offer one interface for host chains to inspect the port, which is the get_port method available through TokenTransferValidationContext, and missed any other portKeeper/Manager interfaces that should deal with getting/setting/binding/releasing ports that are done by taking the PortPath. (which isn't in use anywhere at the moment)

It seems that relocating the ports’ interface under the core context (better to say under the Router trait as our module manager) would enable any application to access this facility without the need for defining an additional app-specific get_port separately.

In addition, we are also missing the following parts of the spec that enforces module management:

Missed Specs

  • Port Allocation: The IBC handler MUST implement bindPortbindPort binds to an unallocated port

  • Exclusive Binding: Once a module has bound to a port, no other modules can use that port until the module releases it

  • Storing Ports: portPath takes an Identifier and returns the store path under which the object-capability reference or owner module identifier (in our case ModuleId) associated with a port should be stored.

  • Port Management: A module can, on its option, release a port or transfer it to another module

  • Port naming: we conventionally hardcode certain ports such as "transfer". Though this can be used as the default/reference value, the specification does not make any assumptions about exact naming and it can be customized by end users. So forth, the router should be able to allocate a custom-named port to a transfer module

@Farhad-Shabani Farhad-Shabani added A: breaking Admin: breaking change that may impact operators O: usability Objective: aims to enhance user experience (UX) and streamline product usability S: specs Scope: related to IBC protocol specifications labels Mar 10, 2023
@Farhad-Shabani Farhad-Shabani moved this to 📥 To Do in ibc-rs Mar 10, 2023
@Farhad-Shabani Farhad-Shabani changed the title [Router] Missing parts of ICS05 & ICS26 that deals with the Module management [Router] Missing parts of ICS05 & ICS26 that deal with the Module management Mar 10, 2023
@plafer
Copy link
Contributor

plafer commented Mar 10, 2023

We currently only offer one interface for host chains to inspect the port, which is the get_port method available through TokenTransferValidationContext

TokenTransferValidationContext::get_port is not a means for the host chain to inspect the port; it's the other way around. It a means for the host chain to tell our implementation of the ICS20 what port it is bound to.

It seems that relocating the ports’ interface under the core context (better to say under the Router trait as our module manager) would enable any application to access this facility without the need for defining an additional app-specific get_port separately.

If you put that functionality in the Router, then apps will need to have a reference to the Router to call that get_port. So this implies that we'd need to add a router: &Router field to all Module methods, right? But you'd still need to know either the ModuleId, or something in the TokenTransferValidationContext to query the Router. So, in this example, we'd need a get_module_id() method instead of get_port, which brings us back to square one.

Note also that TokenTransferValidationContext::get_port is not a universal interface; it's just an implementation detail of our implementation for ICS-20 app. Other apps could just implement their logic directly, without any "get_port" method.

Also see the list of features we don't support on purpose (port management being one of them).

@Farhad-Shabani Farhad-Shabani self-assigned this Jun 21, 2023
@rnbguy rnbguy linked a pull request Jun 15, 2024 that will close this issue
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A: breaking Admin: breaking change that may impact operators O: usability Objective: aims to enhance user experience (UX) and streamline product usability S: specs Scope: related to IBC protocol specifications
Projects
Status: 📥 To Do
Development

Successfully merging a pull request may close this issue.

2 participants