Skip to content
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
b4e4d53
feat(depinject/appconfig): make it easier to define module configs
aaronc Mar 28, 2024
e784156
go.sum
aaronc Mar 29, 2024
d7d96dc
WIP
aaronc Mar 29, 2024
bd58e40
WIP
aaronc Apr 1, 2024
8f668c2
Merge branch 'main' of github.com:cosmos/cosmos-sdk into aaronc/appco…
aaronc Jun 4, 2024
365842e
updates to support gogo
aaronc Jun 4, 2024
4371065
updates to support gogo
aaronc Jun 4, 2024
cbb40bd
updates to support gogo
aaronc Jun 4, 2024
2c30eb4
WIP on tests
aaronc Jun 4, 2024
4404794
WIP
aaronc Jun 4, 2024
6cfcb8b
tests pass
aaronc Jun 4, 2024
088a792
refactory dynamic resolver
aaronc Jun 4, 2024
7380623
update to gogoproto v1.5.0 and simplify logic
aaronc Jun 5, 2024
563dfed
Merge branch 'main' into aaronc/appconfig-gogo
aaronc Jun 5, 2024
360b40f
Merge branch 'main' of github.com:cosmos/cosmos-sdk into aaronc/appco…
aaronc Jun 13, 2024
c4a56c6
go.sum
aaronc Jun 13, 2024
84efdb5
Merge branch 'main' into aaronc/appconfig-gogo
aaronc Jun 19, 2024
6fc9b2c
add CHANGELOG.md
aaronc Jun 20, 2024
e543c0a
Merge remote-tracking branch 'origin/aaronc/appconfig-gogo' into aaro…
aaronc Jun 20, 2024
e057097
Merge branch 'main' of github.com:cosmos/cosmos-sdk into aaronc/appco…
aaronc Jun 20, 2024
217ecc1
Merge branch 'main' into aaronc/appconfig-gogo
tac0turtle Jun 24, 2024
9211087
go mod tidy
tac0turtle Jun 24, 2024
ab8811c
Merge branch 'main' into aaronc/appconfig-gogo
aaronc Jun 24, 2024
886a1d9
Merge branch 'main' of github.com:cosmos/cosmos-sdk into aaronc/appco…
aaronc Jun 24, 2024
1067839
Merge branch 'aaronc/appconfig-gogo' of github.com:cosmos/cosmos-sdk …
aaronc Jun 24, 2024
f370a1c
fix gogo proto version causing fix test failures
aaronc Jun 24, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion client/v2/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ require (
github.com/cosmos/crypto v0.0.0-20240309083813-82ed2537802e // indirect
github.com/cosmos/go-bip39 v1.0.0 // indirect
github.com/cosmos/gogogateway v1.2.0 // indirect
github.com/cosmos/gogoproto v1.4.12
github.com/cosmos/gogoproto v1.5.0
github.com/cosmos/iavl v1.1.4 // indirect
github.com/cosmos/ics23/go v0.10.0 // indirect
github.com/cosmos/ledger-cosmos-go v0.13.3 // indirect
Expand Down
1 change: 1 addition & 0 deletions client/v2/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ github.com/cosmos/gogogateway v1.2.0/go.mod h1:iQpLkGWxYcnCdz5iAdLcRBSw3h7NXeOkZ
github.com/cosmos/gogoproto v1.4.2/go.mod h1:cLxOsn1ljAHSV527CHOtaIP91kK6cCrZETRBrkzItWU=
github.com/cosmos/gogoproto v1.4.12 h1:vB6Lbe/rtnYGjQuFxkPiPYiCybqFT8QvLipDZP8JpFE=
github.com/cosmos/gogoproto v1.4.12/go.mod h1:LnZob1bXRdUoqMMtwYlcR3wjiElmlC+FkjaZRv1/eLY=
github.com/cosmos/gogoproto v1.5.0/go.mod h1:iUM31aofn3ymidYG6bUR5ZFrk+Om8p5s754eMUcyp8I=
github.com/cosmos/iavl v1.1.4 h1:Z0cVVjeQqOUp78/nWt/uhQy83vYluWlAMGQ4zbH9G34=
github.com/cosmos/iavl v1.1.4/go.mod h1:vCYmRQUJU1wwj0oRD3wMEtOM9sJNDP+GFMaXmIxZ/rU=
github.com/cosmos/ics23/go v0.10.0 h1:iXqLLgp2Lp+EdpIuwXTYIQU+AiHj9mOC2X9ab++bZDM=
Expand Down
59 changes: 38 additions & 21 deletions depinject/appconfig/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,25 +2,29 @@ package appconfig

import (
"fmt"
"reflect"

Check notice

Code scanning / CodeQL

Sensitive package import

Certain system packages contain functions which may be a possible source of non-determinism
"strings"

"github.com/cosmos/cosmos-proto/anyutil"
gogoproto "github.com/cosmos/gogoproto/proto"
"google.golang.org/protobuf/encoding/protojson"
"google.golang.org/protobuf/proto"
protov2 "google.golang.org/protobuf/proto"
"google.golang.org/protobuf/reflect/protoreflect"
"google.golang.org/protobuf/reflect/protoregistry"
"google.golang.org/protobuf/types/known/anypb"
"sigs.k8s.io/yaml"

appv1alpha1 "cosmossdk.io/api/cosmos/app/v1alpha1"

"cosmossdk.io/depinject"
internal "cosmossdk.io/depinject/internal/appconfig"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usage of internal package not allowed.

The use of cosmossdk.io/depinject/internal/appconfig may lead to dependency issues or restrict the reusability of the package.

Tools
golangci-lint

19-19: use of internal package cosmossdk.io/depinject/internal/appconfig not allowed (typecheck)

)

// LoadJSON loads an app config in JSON format.
func LoadJSON(bz []byte) depinject.Config {
config := &appv1alpha1.Config{}
err := protojson.Unmarshal(bz, config)
err := protojson.UnmarshalOptions{
Resolver: dynamicTypeResolver{resolver: gogoproto.HybridResolver},
}.Unmarshal(bz, config)
if err != nil {
return depinject.Error(err)
}
Expand Down Expand Up @@ -55,6 +59,11 @@ func Compose(appConfig *appv1alpha1.Config) depinject.Config {
depinject.Supply(appConfig),
}

modules, err := internal.ModulesByModuleTypeName()
if err != nil {
return depinject.Error(err)
}

for _, module := range appConfig.Modules {
if module.Name == "" {
return depinject.Error(fmt.Errorf("module is missing name"))
Expand All @@ -64,30 +73,38 @@ func Compose(appConfig *appv1alpha1.Config) depinject.Config {
return depinject.Error(fmt.Errorf("module %q is missing a config object", module.Name))
}

msgType, err := protoregistry.GlobalTypes.FindMessageByURL(module.Config.TypeUrl)
if err != nil {
return depinject.Error(err)
msgName := module.Config.TypeUrl
// strip type URL prefix
if slashIdx := strings.LastIndex(msgName, "/"); slashIdx >= 0 {
msgName = msgName[slashIdx+1:]
}

modules, err := internal.ModulesByProtoMessageName()
if err != nil {
return depinject.Error(err)
if msgName == "" {
return depinject.Error(fmt.Errorf("module %q is missing a type URL", module.Name))
}

init, ok := modules[msgType.Descriptor().FullName()]
init, ok := modules[msgName]
if !ok {
modDesc := proto.GetExtension(msgType.Descriptor().Options(), appv1alpha1.E_Module).(*appv1alpha1.ModuleDescriptor)
if modDesc == nil {
return depinject.Error(fmt.Errorf("no module registered for type URL %s and that protobuf type does not have the option %s\n\n%s",
module.Config.TypeUrl, appv1alpha1.E_Module.TypeDescriptor().FullName(), dumpRegisteredModules(modules)))
if msgDesc, err := gogoproto.HybridResolver.FindDescriptorByName(protoreflect.FullName(msgName)); err == nil {
modDesc := protov2.GetExtension(msgDesc.Options(), appv1alpha1.E_Module).(*appv1alpha1.ModuleDescriptor)
if modDesc == nil {
return depinject.Error(fmt.Errorf("no module registered for type URL %s and that protobuf type does not have the option %s\n\n%s",
module.Config.TypeUrl, appv1alpha1.E_Module.TypeDescriptor().FullName(), dumpRegisteredModules(modules)))
}

return depinject.Error(fmt.Errorf("no module registered for type URL %s, did you forget to import %s: find more information on how to make a module ready for app wiring: https://docs.cosmos.network/main/building-modules/depinject\n\n%s",
module.Config.TypeUrl, modDesc.GoImport, dumpRegisteredModules(modules)))
}

return depinject.Error(fmt.Errorf("no module registered for type URL %s, did you forget to import %s: find more information on how to make a module ready for app wiring: https://docs.cosmos.network/main/building-modules/depinject\n\n%s",
module.Config.TypeUrl, modDesc.GoImport, dumpRegisteredModules(modules)))
}

config := init.ConfigProtoMessage.ProtoReflect().Type().New().Interface()
err = anypb.UnmarshalTo(module.Config, config, proto.UnmarshalOptions{})
var config gogoproto.Message
if configInit, ok := init.ConfigProtoMessage.(protov2.Message); ok {
config = configInit.ProtoReflect().Type().New().Interface().(gogoproto.Message)
} else {
config = reflect.New(init.ConfigGoType.Elem()).Interface().(gogoproto.Message)
}
// as of gogo v1.5.0 this should work with either gogoproto or golang v2 proto
err = gogoproto.Unmarshal(module.Config.Value, config)
if err != nil {
return depinject.Error(err)
}
Expand All @@ -114,10 +131,10 @@ func Compose(appConfig *appv1alpha1.Config) depinject.Config {
return depinject.Configs(opts...)
}

func dumpRegisteredModules(modules map[protoreflect.FullName]*internal.ModuleInitializer) string {
func dumpRegisteredModules(modules map[string]*internal.ModuleInitializer) string {
var mods []string
for name := range modules {
mods = append(mods, " "+string(name))
mods = append(mods, " "+name)
}
return fmt.Sprintf("registered modules are:\n%s", strings.Join(mods, "\n"))
}
25 changes: 24 additions & 1 deletion depinject/appconfig/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"cosmossdk.io/depinject/appconfig"
internal "cosmossdk.io/depinject/internal/appconfig"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usage of internal package not allowed.

The use of cosmossdk.io/depinject/internal/appconfig may lead to dependency issues or restrict the reusability of the package.

Tools
golangci-lint

15-15: use of internal package cosmossdk.io/depinject/internal/appconfig not allowed (typecheck)

"cosmossdk.io/depinject/internal/appconfig/testpb"
testpbgogo "cosmossdk.io/depinject/internal/appconfiggogo/testpb"
)

func expectContainerErrorContains(t *testing.T, option depinject.Config, contains string) {
Expand Down Expand Up @@ -68,7 +69,10 @@ modules:
"@type": testpb.TestModuleA
- name: b
config:
"@type": testpb.TestModuleB
"@type": /testpb.TestModuleB
- name: c
config:
"@type": /testpb.TestModuleGogo
`))
assert.NilError(t, depinject.Inject(opt, &app))
buf := &bytes.Buffer{}
Expand Down Expand Up @@ -137,6 +141,10 @@ func init() {
appconfig.RegisterModule(&testpb.TestModuleB{},
appconfig.Provide(ProvideModuleB),
)

appconfig.RegisterModule(&testpbgogo.TestModuleGogo{},
appconfig.Provide(ProvideModuleC),
)
}

func ProvideRuntimeState() *RuntimeState {
Expand Down Expand Up @@ -220,3 +228,18 @@ type KeeperB interface {
}

func (k keeperB) isKeeperB() {}

func ProvideModuleC(key StoreKey, b KeeperB) KeeperC {
return keeperC{key: key}
}

type keeperC struct {
key StoreKey
b KeeperB
}

type KeeperC interface {
isKeeperC()
}

func (k keeperC) isKeeperC() {}
80 changes: 80 additions & 0 deletions depinject/appconfig/dynamic_resolver.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
package appconfig

import (
"strings"

"google.golang.org/protobuf/reflect/protodesc"
"google.golang.org/protobuf/reflect/protoreflect"
"google.golang.org/protobuf/reflect/protoregistry"
"google.golang.org/protobuf/types/dynamicpb"
)

// dynamic resolver allows marshaling gogo proto messages from the gogoproto.HybridResolver as long as those
// files have been imported before calling LoadJSON. There is similar code in autocli, this should probably
// eventually be moved into a library.
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to refactor this code and the almost identical implementation in autocli to cosmos-proto possibly?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could also move it to gogoproto because it uses the hybrid resolver and then call it here: https://github.com/cosmos/gogoproto/blob/6eec9731781bf8ec4feb0e4098ffb19ee74afb50/jsonpb/jsonpb.go#L792. So that gogoproto jsonpb.Unmarshal can be used to unmarshal v2 messages from json that contain some gogo proto messages in Any's.

type dynamicTypeResolver struct {
resolver protodesc.Resolver
}

func (r dynamicTypeResolver) FindExtensionByName(field protoreflect.FullName) (protoreflect.ExtensionType, error) {
ext, err := protoregistry.GlobalTypes.FindExtensionByName(field)
if err == nil {
return ext, nil
}

desc, err := r.resolver.FindDescriptorByName(field)
if err != nil {
return nil, err
}

return dynamicpb.NewExtensionType(desc.(protoreflect.ExtensionTypeDescriptor)), nil
}

func (r dynamicTypeResolver) FindExtensionByNumber(message protoreflect.FullName, field protoreflect.FieldNumber) (protoreflect.ExtensionType, error) {
ext, err := protoregistry.GlobalTypes.FindExtensionByNumber(message, field)
if err == nil {
return ext, nil
}

desc, err := r.resolver.FindDescriptorByName(message)
if err != nil {
return nil, err
}

messageDesc := desc.(protoreflect.MessageDescriptor)
exts := messageDesc.Extensions()
n := exts.Len()
for i := 0; i < n; i++ {
ext := exts.Get(i)
if ext.Number() == field {
return dynamicpb.NewExtensionType(ext), nil
}
}

return nil, protoregistry.NotFound
}

func (r dynamicTypeResolver) FindMessageByName(message protoreflect.FullName) (protoreflect.MessageType, error) {
typ, err := protoregistry.GlobalTypes.FindMessageByName(message)
if err == nil {
return typ, nil
}

desc, err := r.resolver.FindDescriptorByName(message)
if err != nil {
return nil, err
}

return dynamicpb.NewMessageType(desc.(protoreflect.MessageDescriptor)), nil
}

func (r dynamicTypeResolver) FindMessageByURL(url string) (protoreflect.MessageType, error) {
if i := strings.LastIndexByte(url, '/'); i >= 0 {
url = url[i+1:]
}

return r.FindMessageByName(protoreflect.FullName(url))
}

var _ protoregistry.MessageTypeResolver = dynamicTypeResolver{}
var _ protoregistry.ExtensionTypeResolver = dynamicTypeResolver{}
14 changes: 10 additions & 4 deletions depinject/appconfig/module.go
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
package appconfig

import (
"fmt"
"reflect"

"google.golang.org/protobuf/proto"
"github.com/cosmos/gogoproto/proto"

internal "cosmossdk.io/depinject/internal/appconfig"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Usage of internal package not allowed.

The use of cosmossdk.io/depinject/internal/appconfig may lead to dependency issues or restrict the reusability of the package.

Tools
golangci-lint

9-9: use of internal package cosmossdk.io/depinject/internal/appconfig not allowed (typecheck)

)
Expand All @@ -19,10 +20,15 @@ var Register = RegisterModule
// Protobuf message types used for module configuration should define the
// cosmos.app.v1alpha.module option and must explicitly specify go_package
// to make debugging easier for users.
func RegisterModule(msg proto.Message, options ...Option) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why using any of we do a type check/cast just below. Cannot we just use the gogoproto.Message interface?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The logic I had was that we may want to support plan old go structs (not protobuf) eventually so one breaking change is better than two. When we discussed in architecture @tac0turtle said gogo is enough, although personally if people are annoyed with too much proto we should lower the barrier

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We okay to leave as is and merge this?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense 👍

ty := reflect.TypeOf(msg)
func RegisterModule(config any, options ...Option) {
protoConfig, ok := config.(proto.Message)
if !ok {
panic(fmt.Errorf("expected config to be a proto.Message, got %T", config))
}

ty := reflect.TypeOf(config)
init := &internal.ModuleInitializer{
ConfigProtoMessage: msg,
ConfigProtoMessage: protoConfig,
ConfigGoType: ty,
}
internal.ModuleRegistry[ty] = init
Expand Down
3 changes: 2 additions & 1 deletion depinject/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@ go 1.20
require (
cosmossdk.io/api v0.7.5
github.com/cosmos/cosmos-proto v1.0.0-beta.5
github.com/cosmos/gogoproto v1.5.0
github.com/stretchr/testify v1.9.0
golang.org/x/exp v0.0.0-20230811145659-89c5cff77bcb
golang.org/x/exp v0.0.0-20231006140011-7918f672742d
google.golang.org/protobuf v1.34.1
gotest.tools/v3 v3.5.1
sigs.k8s.io/yaml v1.4.0
Expand Down
7 changes: 5 additions & 2 deletions depinject/go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,12 @@ cosmossdk.io/api v0.7.5 h1:eMPTReoNmGUm8DeiQL9DyM8sYDjEhWzL1+nLbI9DqtQ=
cosmossdk.io/api v0.7.5/go.mod h1:IcxpYS5fMemZGqyYtErK7OqvdM0C8kdW3dq8Q/XIG38=
github.com/cosmos/cosmos-proto v1.0.0-beta.5 h1:eNcayDLpip+zVLRLYafhzLvQlSmyab+RC5W7ZfmxJLA=
github.com/cosmos/cosmos-proto v1.0.0-beta.5/go.mod h1:hQGLpiIUloJBMdQMMWb/4wRApmI9hjHH05nefC0Ojec=
github.com/cosmos/gogoproto v1.5.0 h1:SDVwzEqZDDBoslaeZg+dGE55hdzHfgUA40pEanMh52o=
github.com/cosmos/gogoproto v1.5.0/go.mod h1:iUM31aofn3ymidYG6bUR5ZFrk+Om8p5s754eMUcyp8I=
github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E=
github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
github.com/golang/protobuf v1.5.4 h1:i7eJL8qZTpSEXOPTxNKhASYpMn+8e5Q6AdndVa1dWek=
github.com/google/go-cmp v0.5.9/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
github.com/google/go-cmp v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI=
github.com/google/go-cmp v0.6.0/go.mod h1:17dUlkBOakJ0+DkrSSNjCkIjxS6bF9zb3elmeNGIjoY=
Expand All @@ -23,8 +26,8 @@ github.com/rogpeppe/go-internal v1.11.0 h1:cWPaGQEPrBb5/AsnsZesgZZ9yb1OQ+GOISoDN
github.com/rogpeppe/go-internal v1.11.0/go.mod h1:ddIwULY96R17DhadqLgMfk9H9tvdUzkipdSkR5nkCZA=
github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg=
github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY=
golang.org/x/exp v0.0.0-20230811145659-89c5cff77bcb h1:mIKbk8weKhSeLH2GmUTrvx8CjkyJmnU1wFmg59CUjFA=
golang.org/x/exp v0.0.0-20230811145659-89c5cff77bcb/go.mod h1:FXUEEKJgO7OQYeo8N01OfiKP8RXMtf6e8aTskBGqWdc=
golang.org/x/exp v0.0.0-20231006140011-7918f672742d h1:jtJma62tbqLibJ5sFQz8bKtEM8rJBtfilJ2qTU199MI=
golang.org/x/exp v0.0.0-20231006140011-7918f672742d/go.mod h1:ldy0pHrwJyGW56pPQzzkH36rKxoZW1tw7ZJpeKx+hdo=
golang.org/x/net v0.25.0 h1:d/OCCoBEUq33pjydKrGQhw7IlUPI2Oylr+8qLx49kac=
golang.org/x/net v0.25.0/go.mod h1:JkAGAh7GEvH74S6FOH42FLoXpXbE/aqXSrIQjXgsiwM=
golang.org/x/sys v0.20.0 h1:Od9JTbYCk261bKm4M/mw7AklTlFYIa0bIp9BgSm1S8Y=
Expand Down
2 changes: 1 addition & 1 deletion depinject/internal/appconfig/buf.gen.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ version: v1
managed:
enabled: true
go_package_prefix:
default: cosmossdk.io/depinject/internal
default: cosmossdk.io/depinject/internal/appconfig
override:
buf.build/cosmos/cosmos-sdk: cosmossdk.io/api
plugins:
Expand Down
Loading