Should Ics20Context implement Module?

[kevinji]

Summary

Should Ics20Context implement Module so ChannelKeeper and ChannelReader don't need to be implemented?

Problem Definition

The current ICS20 logic looks a bit like a standalone IBC module, which means it doesn't interop nicely with the Module system proposed by ICS26. It also leads to a lot of extra boilerplate code due to having to implement ChannelKeeper and ChannelReader that could be delegated to a routing module that implements ICS26.

Proposal

Have Ics20Context implement Module, and update the interface of Ics20Context so only the BankKeeper and Ics20Reader functions need to be implemented.

Acceptance Criteria

The changes proposed are made to Ics20Context and the other related traits.

---

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate milestone (priority) applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

[hu55a1n1]

Hi @kevinji, the reason Ics20Context doesn't implement Module is that we didn't want to assume that the struct/component implementing Ics20Context is the same as the one implementing Module. Both of these are traits, so we could have had Module be a supertrait of Ics20Context but that would assume that they're the same type, or we could provide a blanket impl (i.e. impl<ICS20: Ics20Context> Module for ICS20 { ... }) but that would be quite a rigid design. The current impl just provides building blocks so people have more flexibility and can implement ICS20 with additional validation if they like. Happy to revisit the design if you think this can be done in a better way.

[plafer]

@kevinji can we close this, or do you still see an issue with the current design?

[kevinji]

Hey sorry for the late response. I agree that implementing Modulewould require the component implementing Ics20Context to implement Module, which seems less than ideal.

The use case I was thinking of is that I could separate out the ICS-20 logic into a separate module (possibly separate smart contract). Would this essentially require some way to generate an Ics20Context given a BankKeeper and Module implementation—something like impl<T: BankKeeper + Module> Ics20Context for T?

Or, phrased a different way, is there some way to get ChannelKeeper and ChannelReader for "free" given a Module and a Router?

[hu55a1n1]

Hi @kevinji, I just opened PR #244 - can you take a look and tell us if that addresses your issue?

[kevinji]

@hu55a1n1 This looks good, I'll try it out once I get some time to revisit this.