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

[Module] Differentiate module-specific contexts #520

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

[Module] Differentiate module-specific contexts #520

Farhad-Shabani opened this issue Mar 10, 2023 · 1 comment
Assignees
Labels
A: breaking Admin: breaking change that may impact operators A: critical Admin: critical or Important O: decoupling Objective: aims to separate concerns and cause to independent, reusable components O: logic Objective: aims for better implementation logic

Comments

@Farhad-Shabani
Copy link
Member

Problem Statement

The recent refactoring of TokenTransferReader/Keeper to TokenTransferValidation/ExecutionContext revealed that there is a potential for host chain objects implementing core contexts to expand their capabilities by also implementing token transfer contexts.

However, this approach is incorrect (which already has been applied to the MockContext). This mixes distinct concerns under the core context and limits IBC implementation to only support one transfer module. Considering that the IBC core only operates applications by calling on__*__validate/execute methods under the Module trait, the token transferring or any other apps should be a capability of a module object! and for the core, it does not matter what's the content of a packet's data!

To prevent incorrect implementation, improve modularity, and clarify the role of each, It is suggested to bind TokenTransferValidation/ExecutionContext with a newly defined ModuleContext (As we execute issue #519, some methods might be gather-able under ModuleContext). This also can be bounded by the current Module trait. Additionally, the Module trait can be split into two: ModuleOnValidation and ModuleOnExecution, further clarifying their respective roles.

This is an initial proposal, and as we work on issue #519, better ideas may arise. Nonetheless, the ultimate goal is to clearly differentiate module-specific contexts.

@Farhad-Shabani Farhad-Shabani added A: critical Admin: critical or Important A: breaking Admin: breaking change that may impact operators O: logic Objective: aims for better implementation logic 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 [Module] differentiate module-specific contexts [Module] Differentiate module-specific contexts Mar 10, 2023
@plafer
Copy link
Contributor

plafer commented Mar 10, 2023

The recent refactoring of TokenTransferReader/Keeper to TokenTransferValidation/ExecutionContext revealed that there is a potential for host chain objects implementing core contexts to expand their capabilities by also implementing token transfer contexts.

They can but they don't have to. Both basecoin-rs and Namada implement Moduleon a separate object from the one that implements Validation/ExecutionContext.

This mixes distinct concerns under the core context and limits IBC implementation to only support one transfer module.

Our current architecture allows chains to have as many transfer modules as they want; only one can be bound to the transfer port though (as mandated by the protocol).

and for the core, it does not matter what's the content of a packet's data!
that's right; our implementation of ibc-core currently is agnostic to the contents of packets. Can you expand on why you think that is not the case?

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 A: critical Admin: critical or Important O: decoupling Objective: aims to separate concerns and cause to independent, reusable components O: logic Objective: aims for better implementation logic
Projects
Status: 📥 To Do
Development

No branches or pull requests

2 participants