search

cosmos / ibc-rs

Rust implementation of the Inter-Blockchain Communication (IBC) protocol.
Apache License 2.0
193 stars 80 forks source link

[Module] Differentiate module-specific contexts #520

Open Farhad-Shabani opened 1 year ago

Farhad-Shabani commented 1 year ago

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.

plafer commented 1 year ago

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?