-
Notifications
You must be signed in to change notification settings - Fork 3.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
tests(integration): port x/bank tests to server/v2 app #21912
base: main
Are you sure you want to change the base?
Changes from 58 commits
da84451
eb347f6
2e9835c
b0cc5a5
14686ba
74358ca
ba91038
c4d0834
a080b2e
f09a8fe
ce51d9e
441423d
7e508c3
99c2a5c
a6e0c1f
e0c9e3d
8c4d79d
2d28d64
0e71ce3
3ad2a6e
d8a6ed7
de2a723
a47a286
c8ea058
2410c04
8d3698f
57040e2
995974c
69598d4
29b0ebe
a977338
30814ac
ef50b4f
177a0da
25f0244
45ff83a
16a0e99
8d1014e
9fe1020
80db042
57ec239
c20649d
ea930ac
c508350
8b9b66c
e0eb0ef
b6d6f75
d8fc366
00dd82a
74e8eed
710a1aa
ae35d88
eb1f49e
e37ff14
7e6b4db
d3e474c
8292c98
de81b74
706a591
9e42ec8
db41984
869a492
a8f7d8e
6a9e803
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,6 +16,7 @@ import ( | |
reflectionv1 "cosmossdk.io/api/cosmos/reflection/v1" | ||
appmodulev2 "cosmossdk.io/core/appmodule/v2" | ||
"cosmossdk.io/core/comet" | ||
"cosmossdk.io/core/event" | ||
"cosmossdk.io/core/header" | ||
"cosmossdk.io/core/registry" | ||
"cosmossdk.io/core/server" | ||
|
@@ -26,6 +27,7 @@ import ( | |
"cosmossdk.io/log" | ||
"cosmossdk.io/runtime/v2/services" | ||
"cosmossdk.io/server/v2/stf" | ||
rootstore "cosmossdk.io/store/v2/root" | ||
) | ||
|
||
var ( | ||
|
@@ -40,19 +42,19 @@ type appModule[T transaction.Tx] struct { | |
func (m appModule[T]) IsOnePerModuleType() {} | ||
func (m appModule[T]) IsAppModule() {} | ||
|
||
func (m appModule[T]) RegisterServices(registar grpc.ServiceRegistrar) error { | ||
func (m appModule[T]) RegisterServices(registrar grpc.ServiceRegistrar) error { | ||
autoCliQueryService, err := services.NewAutoCLIQueryService(m.app.moduleManager.modules) | ||
if err != nil { | ||
return err | ||
} | ||
|
||
autocliv1.RegisterQueryServer(registar, autoCliQueryService) | ||
autocliv1.RegisterQueryServer(registrar, autoCliQueryService) | ||
|
||
reflectionSvc, err := services.NewReflectionService() | ||
if err != nil { | ||
return err | ||
} | ||
reflectionv1.RegisterReflectionServiceServer(registar, reflectionSvc) | ||
reflectionv1.RegisterReflectionServiceServer(registrar, reflectionSvc) | ||
|
||
return nil | ||
} | ||
|
@@ -97,6 +99,7 @@ func init() { | |
ProvideAppBuilder[transaction.Tx], | ||
ProvideEnvironment[transaction.Tx], | ||
ProvideModuleManager[transaction.Tx], | ||
ProvideStoreBuilder, | ||
), | ||
appconfig.Invoke(SetupAppBuilder), | ||
) | ||
|
@@ -146,7 +149,9 @@ type AppInputs struct { | |
InterfaceRegistrar registry.InterfaceRegistrar | ||
LegacyAmino registry.AminoRegistrar | ||
Logger log.Logger | ||
StoreBuilder *StoreBuilder | ||
DynamicConfig server.DynamicConfig `optional:"true"` // can be nil in client wiring | ||
StoreOptions *rootstore.Options `optional:"true"` // if unset defaults will be used | ||
Comment on lines
+152
to
+154
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Improve struct field comments for clarity In the Consider updating the comment to: type AppInputs struct {
...
StoreOptions *rootstore.Options `optional:"true"` // StoreOptions specifies the store options; if unset, defaults will be used.
}
|
||
} | ||
|
||
func SetupAppBuilder(inputs AppInputs) { | ||
|
@@ -157,8 +162,22 @@ func SetupAppBuilder(inputs AppInputs) { | |
app.moduleManager.RegisterInterfaces(inputs.InterfaceRegistrar) | ||
app.moduleManager.RegisterLegacyAminoCodec(inputs.LegacyAmino) | ||
|
||
if inputs.DynamicConfig != nil { | ||
inputs.AppBuilder.config = inputs.DynamicConfig | ||
if inputs.DynamicConfig == nil { | ||
return | ||
} | ||
storeOptions := rootstore.DefaultStoreOptions() | ||
if inputs.StoreOptions != nil { | ||
storeOptions = *inputs.StoreOptions | ||
} | ||
var err error | ||
app.db, err = inputs.StoreBuilder.Build( | ||
inputs.Logger, | ||
app.storeKeys, | ||
inputs.DynamicConfig, | ||
storeOptions, | ||
) | ||
if err != nil { | ||
panic(err) | ||
Comment on lines
+165
to
+180
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Handle errors without panicking In the Consider modifying the error handling: -func SetupAppBuilder(inputs AppInputs) {
+func SetupAppBuilder(inputs AppInputs) error {
...
if err != nil {
- panic(err)
+ return err
}
} Then, handle the returned error where
|
||
} | ||
} | ||
|
||
|
@@ -178,6 +197,7 @@ func ProvideEnvironment[T transaction.Tx]( | |
appBuilder *AppBuilder[T], | ||
kvFactory store.KVStoreServiceFactory, | ||
headerService header.Service, | ||
eventService event.Service, | ||
) ( | ||
appmodulev2.Environment, | ||
store.KVStoreService, | ||
|
@@ -209,7 +229,7 @@ func ProvideEnvironment[T transaction.Tx]( | |
env := appmodulev2.Environment{ | ||
Logger: logger, | ||
BranchService: stf.BranchService{}, | ||
EventService: stf.NewEventService(), | ||
EventService: eventService, | ||
GasService: stf.NewGasMeterService(), | ||
HeaderService: headerService, | ||
QueryRouterService: stf.NewQueryRouterService(), | ||
|
@@ -254,10 +274,12 @@ func DefaultServiceBindings() depinject.Config { | |
} | ||
headerService header.Service = services.NewGenesisHeaderService(stf.HeaderService{}) | ||
cometService comet.Service = &services.ContextAwareCometInfoService{} | ||
eventService = stf.NewEventService() | ||
) | ||
return depinject.Supply( | ||
kvServiceFactory, | ||
headerService, | ||
cometService, | ||
eventService, | ||
) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,11 +3,16 @@ package runtime | |
import ( | ||
"errors" | ||
"fmt" | ||
"path/filepath" | ||
|
||
"cosmossdk.io/core/server" | ||
"cosmossdk.io/core/store" | ||
"cosmossdk.io/log" | ||
"cosmossdk.io/server/v2/stf" | ||
storev2 "cosmossdk.io/store/v2" | ||
"cosmossdk.io/store/v2/db" | ||
"cosmossdk.io/store/v2/proof" | ||
"cosmossdk.io/store/v2/root" | ||
) | ||
|
||
// NewKVStoreService creates a new KVStoreService. | ||
|
@@ -59,6 +64,56 @@ type Store interface { | |
LastCommitID() (proof.CommitID, error) | ||
} | ||
|
||
// StoreBuilder is a builder for a store/v2 RootStore satisfying the Store interface. | ||
type StoreBuilder struct { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's smart, I like the abstraction! Makes it more readble too. |
||
store Store | ||
} | ||
|
||
// Build creates a new store/v2 RootStore. | ||
func (sb *StoreBuilder) Build( | ||
logger log.Logger, | ||
storeKeys []string, | ||
config server.DynamicConfig, | ||
options root.Options, | ||
) (Store, error) { | ||
if sb.store != nil { | ||
return sb.store, nil | ||
} | ||
home := config.GetString(FlagHome) | ||
scRawDb, err := db.NewDB( | ||
db.DBType(config.GetString("store.app-db-backend")), | ||
"application", | ||
filepath.Join(home, "data"), | ||
nil, | ||
) | ||
if err != nil { | ||
return nil, fmt.Errorf("failed to create SCRawDB: %w", err) | ||
} | ||
|
||
factoryOptions := &root.FactoryOptions{ | ||
Logger: logger, | ||
RootDir: home, | ||
Options: options, | ||
StoreKeys: append(storeKeys, "stf"), | ||
SCRawDB: scRawDb, | ||
} | ||
|
||
rs, err := root.CreateRootStore(factoryOptions) | ||
if err != nil { | ||
return nil, fmt.Errorf("failed to create root store: %w", err) | ||
} | ||
sb.store = rs | ||
return sb.store, nil | ||
} | ||
Comment on lines
+73
to
+107
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ensure In line 82, home := config.GetString(FlagHome) Please verify that If +const FlagHome = "home" Alternatively, import |
||
|
||
func (sb *StoreBuilder) Get() Store { | ||
return sb.store | ||
} | ||
Comment on lines
+109
to
+111
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Handle uninitialized The Consider adding a check or documenting that |
||
|
||
func ProvideStoreBuilder() *StoreBuilder { | ||
return &StoreBuilder{} | ||
} | ||
Comment on lines
+113
to
+115
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Consider returning an interface in In func ProvideStoreBuilder() *StoreBuilder {
return &StoreBuilder{}
} Consider returning the If returning the concrete type is intentional and necessary for your design, ensure this aligns with your overall architectural goals. |
||
|
||
// StoreLoader allows for custom loading of the store, this is useful when upgrading the store from a previous version | ||
type StoreLoader func(store Store) error | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -61,10 +61,9 @@ func NewSimApp[T transaction.Tx]( | |
viper *viper.Viper, | ||
) *SimApp[T] { | ||
var ( | ||
app = &SimApp[T]{} | ||
appBuilder *runtime.AppBuilder[T] | ||
err error | ||
storeOptions = &root.Options{} | ||
app = &SimApp[T]{} | ||
appBuilder *runtime.AppBuilder[T] | ||
err error | ||
|
||
// merge the AppConfig and other configuration in one config | ||
appConfig = depinject.Configs( | ||
|
@@ -127,6 +126,15 @@ func NewSimApp[T transaction.Tx]( | |
) | ||
) | ||
|
||
if sub := viper.Sub("store.options"); sub != nil { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we add short comment on this |
||
storeOptions := &root.Options{} | ||
err := sub.Unmarshal(storeOptions) | ||
if err != nil { | ||
panic(err) | ||
} | ||
appConfig = depinject.Configs(appConfig, depinject.Supply(storeOptions)) | ||
} | ||
|
||
if err := depinject.Inject(appConfig, | ||
&appBuilder, | ||
&app.appCodec, | ||
|
@@ -138,15 +146,7 @@ func NewSimApp[T transaction.Tx]( | |
panic(err) | ||
} | ||
|
||
var builderOpts []runtime.AppBuilderOption[T] | ||
if sub := viper.Sub("store.options"); sub != nil { | ||
err = sub.Unmarshal(storeOptions) | ||
if err != nil { | ||
panic(err) | ||
} | ||
builderOpts = append(builderOpts, runtime.AppBuilderWithStoreOptions[T](storeOptions)) | ||
} | ||
app.App, err = appBuilder.Build(builderOpts...) | ||
app.App, err = appBuilder.Build() | ||
if err != nil { | ||
panic(err) | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shal we use those in app_di (v1 & v2) to simplify things?