Rollback ABCI use for cross module communication

[robert-zaremba]

Summary

The "Write back account changes after tracking delegation/undelegation for vesting accounts" #8865 Pull Request introduced a heavy weight machinery to communicate x/auth with x/bank.

Problem Definition

x/auth can't import keeper x/bank - that cause circular dependency. In the pull request, author used ABCI and App gRPC Query Server to get information from x/auth. This causes few potential problems:

• obscures the code
• makes it very hard to test
• add unnecessary protobuf roundtrip (significant one)
• is more error prone because we need to use paths rather than RPC calls.

This is only used for migration, but adding it to master already signals an example and could lead to reuse this in other places by other developers.

Proposal

Standard way to communicate between modules is to use keepers. As noted above, we can't do it in a direct way. What we could do, however, is to:

• in x/auth create a bank keeper interface without importing x/bank (limited to the functions we need).
• or even have a proxy object
• inject a bank keeper instance to the x/auth module
• use it in the migration without the ceremony with querier.

A new way of solving this would be ADR-33 (which is not done yet for master)

References:

• https://github.com/cosmos/cosmos-sdk/pull/8865/files#r599983821
• Write back account changes after tracking delegation/undelegation for vesting accounts #8865 (comment)

---

For Admin Use

• Not duplicate issue
• Appropriate labels applied
• Appropriate contributors tagged
• Contributor assigned/self-assigned

[aaronc]

The simple way that @AmauryM and I had looked at may be reusing QueryServiceTestHelper.

[fdymylja]

Maybe we can address this when ADR-033 is ready? As @AmauryM has stated in #8865 that would probably be the cleanest way of handling this.

[aaronc]

From my perspective I think we could consider this nice-to-have for 0.43. Maybe we can just put in code comments to indicate this isn't a pattern others should imitate for now.

[robert-zaremba]

Do we have any ETA for ADR-033?

[amaury1093]

I'm adding a comment in #9132 for other people not to replicate this pattern.

Proposing to remove the 0.43 milestone once #9132 is merged, but to keep this issue open after.

Edit: merged

[clevinson]

Let's check in on this on next Friday's architecture call. It'd be great to get ADR33 intermodule wiring finalized so other chains can start making use of the new way of doing inter module rpc, etc.

[tac0turtle]

closing as we will begin working on adr33 soon, finally :). Modules using the query router to get information will be a way for modules to communicate if needed. There isnt an inherent issue with this.