refactor!: migrate to core appmodule.AppModule extension interfaces #13794
Conversation
github-actions
bot
added
C:x/auth
C:x/authz
C:x/bank
C:x/capability
C:x/crisis
C:x/distribution
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
types/module/module.go
Outdated
|
|
||
| // RegisterServices allows a module to register services | ||
| // LegacyRegisterServices is the legacy method for modules to register services. | ||
| type LegacyRegisterServices interface { |
There was a problem hiding this comment.
This is legacy because of the new core api?
There was a problem hiding this comment.
Yes. I have the idea to push the new core API version relatively quickly after this. Or we could do the rename to legacy once the core API version is ready. Don't have a strong preference just thought this was clearer.
There was a problem hiding this comment.
the import will change to use the new api, correct?
There was a problem hiding this comment.
Well with extension interfaces imports are actually optional, but yes it might
|
I've addressed a bunch of review comments and in particular got rid of |
| for name, mod := range inputs.Modules { | ||
| if basicMod, ok := mod.(module.AppModuleBasic); ok { | ||
| app.basicManager[name] = basicMod | ||
| basicMod.RegisterInterfaces(inputs.InterfaceRegistry) | ||
| basicMod.RegisterLegacyAminoCodec(inputs.LegacyAmino) | ||
| } | ||
| } |
Check warning
Code scanning / CodeQL
Iteration over map
| @@ -81,15 +81,12 @@ type AppInputs struct { | |||
| AppConfig *appv1alpha1.Config | |||
| Config *runtimev1alpha1.Module | |||
| AppBuilder *AppBuilder | |||
| Modules map[string]AppModuleWrapper | |||
There was a problem hiding this comment.
Yes, I think that's a good idea. Totally fine to do in a follow-up too, as this already has approvals. It's not breaking anyways, right?
Yes |
aaronc
added
the
backport/v0.47.x
|
Should we be indicating somehow on the extension interfaces ( |
|
Also, most of the mock tests for modules no longer work because we are using extension interfaces which the mock app module doesn't have defined. Any thoughts on what to do? I can either:
|
I ended up taking the second approach and created a type |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #13794 +/- ##
==========================================
- Coverage 56.87% 56.44% -0.43%
==========================================
Files 646 648 +2
Lines 55145 56221 +1076
==========================================
+ Hits 31364 31735 +371
- Misses 21226 21924 +698
- Partials 2555 2562 +7
|
|
|
||
| // BeginBlock mocks base method. | ||
| func (m *MockAppModuleWithAllExtensions) BeginBlock(arg0 types0.Context, arg1 types1.RequestBeginBlock) { | ||
| m.ctrl.T.Helper() |
Check warning
Code scanning / CodeQL
Panic in BeginBock or EndBlock consensus methods
| // BeginBlock mocks base method. | ||
| func (m *MockAppModuleWithAllExtensions) BeginBlock(arg0 types0.Context, arg1 types1.RequestBeginBlock) { | ||
| m.ctrl.T.Helper() | ||
| m.ctrl.Call(m, "BeginBlock", arg0, arg1) |
Check warning
Code scanning / CodeQL
Panic in BeginBock or EndBlock consensus methods
|
|
||
| // BeginBlock indicates an expected call of BeginBlock. | ||
| func (mr *MockAppModuleWithAllExtensionsMockRecorder) BeginBlock(arg0, arg1 interface{}) *gomock.Call { | ||
| mr.mock.ctrl.T.Helper() |
Check warning
Code scanning / CodeQL
Panic in BeginBock or EndBlock consensus methods
| // BeginBlock indicates an expected call of BeginBlock. | ||
| func (mr *MockAppModuleWithAllExtensionsMockRecorder) BeginBlock(arg0, arg1 interface{}) *gomock.Call { | ||
| mr.mock.ctrl.T.Helper() | ||
| return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "BeginBlock", reflect.TypeOf((*MockAppModuleWithAllExtensions)(nil).BeginBlock), arg0, arg1) |
Check warning
Code scanning / CodeQL
Panic in BeginBock or EndBlock consensus methods
| // BeginBlock indicates an expected call of BeginBlock. | ||
| func (mr *MockAppModuleWithAllExtensionsMockRecorder) BeginBlock(arg0, arg1 interface{}) *gomock.Call { | ||
| mr.mock.ctrl.T.Helper() | ||
| return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "BeginBlock", reflect.TypeOf((*MockAppModuleWithAllExtensions)(nil).BeginBlock), arg0, arg1) |
Check warning
Code scanning / CodeQL
Panic in BeginBock or EndBlock consensus methods
|
|
||
| // EndBlock mocks base method. | ||
| func (m *MockAppModuleWithAllExtensions) EndBlock(arg0 types0.Context, arg1 types1.RequestEndBlock) []types1.ValidatorUpdate { | ||
| m.ctrl.T.Helper() |
Check warning
Code scanning / CodeQL
Panic in BeginBock or EndBlock consensus methods
| // EndBlock mocks base method. | ||
| func (m *MockAppModuleWithAllExtensions) EndBlock(arg0 types0.Context, arg1 types1.RequestEndBlock) []types1.ValidatorUpdate { | ||
| m.ctrl.T.Helper() | ||
| ret := m.ctrl.Call(m, "EndBlock", arg0, arg1) |
Check warning
Code scanning / CodeQL
Panic in BeginBock or EndBlock consensus methods
|
|
||
| // EndBlock indicates an expected call of EndBlock. | ||
| func (mr *MockAppModuleWithAllExtensionsMockRecorder) EndBlock(arg0, arg1 interface{}) *gomock.Call { | ||
| mr.mock.ctrl.T.Helper() |
Check warning
Code scanning / CodeQL
Panic in BeginBock or EndBlock consensus methods
| // EndBlock indicates an expected call of EndBlock. | ||
| func (mr *MockAppModuleWithAllExtensionsMockRecorder) EndBlock(arg0, arg1 interface{}) *gomock.Call { | ||
| mr.mock.ctrl.T.Helper() | ||
| return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "EndBlock", reflect.TypeOf((*MockAppModuleWithAllExtensions)(nil).EndBlock), arg0, arg1) |
Check warning
Code scanning / CodeQL
Panic in BeginBock or EndBlock consensus methods
| // EndBlock indicates an expected call of EndBlock. | ||
| func (mr *MockAppModuleWithAllExtensionsMockRecorder) EndBlock(arg0, arg1 interface{}) *gomock.Call { | ||
| mr.mock.ctrl.T.Helper() | ||
| return mr.mock.ctrl.RecordCallWithMethodType(mr.mock, "EndBlock", reflect.TypeOf((*MockAppModuleWithAllExtensions)(nil).EndBlock), arg0, arg1) |
Check warning
Code scanning / CodeQL
Panic in BeginBock or EndBlock consensus methods
…aaronc/appmodule-refactor
Description
This migrates all modules and the module manager to use the
cosmossdk.io/core/appmodule.AppModuleinterface. It does this in a mostly backwards compatible way. The main breaking change that is required for modules is to implement the tag methods inappmodule.AppModule:module.AppModuleandmodule.NewManagerhave been deprecated in favor ofappmodule.AppModuleandmodule.NewManagerFromMap.This refactoring changes the way
runtimeconsumes modules usingdepinjectand now modules should provideappmodule.AppModule.runtime.AppModuleWrapperandruntime.AppModuleBasicWrapperhave been deleted.Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!to the type prefix if API or client breaking changeCHANGELOG.mdReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
!in the type prefix if API or client breaking change