Move AppModule.BeginBlock and AppModule.EndBlock to extension interfaces

[ValarDragon]

Summary

Most modules right now have a no-op for AppModule.BeginBlock and AppModule.EndBlock. We should move these methods off the AppModule interface so we have less deadcode, and instead move them to extension interfaces. This should not be a noticeable client breaking change, as users of the SDK don't really consume the AppModule interface, and thats moreso something for the ModuleManager.

This would mean removing:

    // ABCI
    BeginBlock(sdk.Context, abci.RequestBeginBlock)
    EndBlock(sdk.Context, abci.RequestEndBlock) []abci.ValidatorUpdate

from the AppModule interface, and making two new extension interfaces:

type BeginBlockAppModule interface {
    AppModule
    BeginBlock(sdk.Context, abci.RequestBeginBlock)
}
type EndBlockAppModule interface {
    AppModule
    EndBlock(sdk.Context, abci.RequestEndBlock) []abci.ValidatorUpdate
}

Then we retain having ModuleManager take in AppModules, so there is no client breaking changes. The module manager should type cast internally to get the BeginBlock and EndBlock app module lists. (We may waste some memory in the struct, but this is negligible)

As a secondary feature, (likely for a second PR), its total ordering constraint, can be relaxed to only requiring a specified order for modules that have the relevant method.

Problem Definition

Delete more methods from every module.go, improving the signal to noise ratio of cosmos code, and the accuracy of interfaces we work with.

(From discussion with @aaronc @kocubinski @alexanderbez )

---

For Admin Use

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

[stackman27]

Hi @ValarDragon is this being worked on right now? Can i give it a shot, it seems interesting?

[ValarDragon]

I don't think its being worked on (cc @xBalbinus to double check)

Would be really cool to get this in! Check out this blog post for some more details on extension interfaces: https://medium.com/swlh/what-is-the-extension-interface-pattern-in-golang-ce852dcecaec

[xBalbinus]

Heyo @stackman27 !! Thanks for the help / offer! I was wrapping up some last things with some doc edits and was going to get to this real soon but if you have more availability, definitely feel free! Don't want to block you.

[ValarDragon]

btw made a related PR in osmosis that shows how to do the typecast if it helps: https://github.com/osmosis-labs/osmosis/pull/2081

[stackman27]

@ValarDragon Gotchu i will look into it & try it out

[stackman27]

@xBalbinus for sure! i can give it a shot, i'm fairly free rn. Will reach out to you if i get stuck

[stackman27]

@ValarDragon

1. Is the idea to created a shared extension interfaces across all the modules or every module is going to have their own extension interface? Should i be creating a manger.go file?
2. Also since most modules right now have a no-op for AppModule.BeginBlock and AppModule.EndBlock why can't we define our extension interface like this;

   type BeginEndBlockAppModule interface {
     AppModule
     BeginBlock(sdk.Context, abci.RequestBeginBlock)
     EndBlock(sdk.Context, abci.RequestEndBlock) []abci.ValidatorUpdate
   }

[alexanderbez]

Is the idea to created a shared extension interfaces across all the modules or every module is going to have their own extension interface? Should i be creating a manger.go file?

There will only be a single interface, it's defined the same place where AppModule is defined. There is no need to create any new files -- just add the interface to where AppModule is defined (I believe that's module.go).

Also since most modules right now have a no-op for AppModule.BeginBlock and AppModule.EndBlock why can't we define our extension interface like this;

Not sure I follow? You want two interfaces -- BeginBlockAppModule and EndBlockAppModule.

1. Define these two interfaces
2. Remove the dead-code from modules that do no implement them
3. Add type casting in the the module code here and here

That should be it :)

[stackman27]

Aah i see, this makes sense i was checking the individual module.go files for each modules and got confused. Thank you very much!