`pkg/cosmosanalysis`: support discovery of modules defined in a variable #2818

jeronimoalbi · 2022-09-08T15:03:23Z

Description
The cosmosanalysis package should be able to discover registered modules also when the list of modules is defined as a variable.

This behaviour is desired to better support code generation in different blockchain applications.

Solution

Change the module discovery to recognise when the list of modules is defined as a local variable:

module.NewBasicManager(appModules...)

Or a variable defined in a different package:

module.NewBasicManager(keepers.AppModules...)

The text was updated successfully, but these errors were encountered:

clockworkgr · 2022-09-15T08:22:30Z

Using: #2820

Building clients on latest repo:
SPN -> Works
Gaia -> Works
Crescent -> Fails (due to unrelated issue however)
Akash -> Fails (modules missing)
Irishub -> Fails (probably due to unrelated issue)
Juno -> Fails (modules missing)
Regen -> Fails (modules missing)
Osmosis -> Fails (modules missing...but better than without the PR)

jeronimoalbi · 2022-09-19T10:54:12Z

In the specific case of Akash they are using a way of declaring the modules that we currently aren't able to detect. We can add such a case but if we do I would like to have a second opinion on it because I am not sure that case would be a common one.

What do you think @ilgooz, should we also handle that way of declaring the modules?

jeronimoalbi · 2022-09-21T15:57:54Z

I checked some of the blockchains where the generation failed or didn't fully generated all of the modules.

Osmosis's twap module and the Regen modules are not being generated because each of the modules are defined in a folder that doesn't contain the required RPC service implementation to be recognised as a module, their RPC implementation is located in different directory branches and we expect to locate them at the module's definition level or within a child directory. Some example modules:

Osmosis's twap module
Regen's ecocredit module

As described in the previous commend Akash also have the issue with the module being defined at a different level than the RPC service implementation.

The issue with Juno is that the Go module definition is using versioning and the proto files are not using it in the go_package options and because of that we are not able to properly identify the custom modules. The Go import paths in the go_package options don't have the version suffix.

Crescent in the other hand have an issue that makes protoc fail because one of its dependencies have a proto file that is importing the same package twice, but it also uses Go module versioning which is not being used in the proto files.

One possible solution to recognise the custom modules is to look for the RPC implementation in all of the files defined in the x/custom_module directory instead of inside the package that defines the module, but it could make the generation considerably slower, so I would like to have a second opinion before making such a change.

Regarding the solution for the versioning in the Go modules and the go_package proto option I still need to figure it out if it's actually possible but I would appreciate any opinion on that too. Cosmos's ibc-go is using the version suffix in the proto files.

jeronimoalbi · 2022-09-23T11:38:12Z

Regarding the last two solutions described in the last comment, both were implemented.

Custom modules now handle the case where the RPC service implementation is defined in a different tree branch within the module.

Versioning in the Go import paths is now handled too so discovery in versioned modules works when the Go import paths in the proto are not versioned.

The generation results:

SPN (works)
Osmosis (works)
Juno (works)
Gaia (works)
Akash (fails)
Regen (fails)
Crescent (fails)

Further explanation can be found here.

@clockworkgr it would be great if you could take another look to the TS client generation result for the ones that are working. Thank you!

queencre · 2023-01-02T05:24:57Z

Thanks for testing Crescent core and writing this up @jeronimoalbi. I know this issue is already closed and there was an issue with Crescent Core. I would like to let you know that it has been resolved in this crescent-network/crescent#101.

tbruyelle · 2023-01-02T17:16:28Z

Thanks for testing Crescent core and writing this up @jeronimoalbi. I know this issue is already closed and there was an issue with Crescent Core. I would like to let you know that it has been resolved in this crescent-network/crescent#101.

Hi @queencre, if I run our package cosmosanalysis on your app/app.go (main branch), I get this list of modules :

	                        "github.com/cosmos/cosmos-sdk/x/auth",
				"github.com/cosmos/cosmos-sdk/x/genutil",
				"github.com/cosmos/cosmos-sdk/x/bank",
				"github.com/cosmos/cosmos-sdk/x/capability",
				"github.com/cosmos/cosmos-sdk/x/staking",
				"github.com/crescent-network/crescent/v4/x/mint",
				"github.com/cosmos/cosmos-sdk/x/distribution",
				"github.com/cosmos/cosmos-sdk/x/gov",
				"github.com/cosmos/cosmos-sdk/x/params",
				"github.com/cosmos/cosmos-sdk/x/crisis",
				"github.com/cosmos/cosmos-sdk/x/slashing",
				"github.com/cosmos/cosmos-sdk/x/feegrant/module",
				"github.com/cosmos/cosmos-sdk/x/authz/module",
				"github.com/cosmos/ibc-go/v3/modules/core",
				"github.com/cosmos/ibc-go/v3/modules/apps/transfer",
				"github.com/cosmos/cosmos-sdk/x/upgrade",
				"github.com/cosmos/cosmos-sdk/x/evidence",
				"github.com/cosmos/cosmos-sdk/x/auth/vesting",
				"github.com/tendermint/budget/x/budget",
				"github.com/crescent-network/crescent/v4/x/farming",
				"github.com/crescent-network/crescent/v4/x/liquidity",
				"github.com/crescent-network/crescent/v4/x/liquidstaking",
				"github.com/crescent-network/crescent/v4/x/liquidfarming",
				"github.com/crescent-network/crescent/v4/x/claim",
				"github.com/crescent-network/crescent/v4/x/marketmaker",
				"github.com/crescent-network/crescent/v4/x/lpfarm",
				"github.com/cosmos/ibc-go/v3/modules/apps/27-interchain-accounts",
				"github.com/cosmos/cosmos-sdk/x/auth/tx",
				"github.com/cosmos/cosmos-sdk/client/grpc/tmservice",

Can you tell me what's missing ?

queencre · 2023-01-03T11:59:57Z

@tbruyelle I don't think you are missing anything. Those are the exact custom modules that we have. FYI, Crescent Mainnet is currently running with v3.0.0 and the main branch is the latest development branch.

tbruyelle · 2023-01-03T13:00:44Z

OK I ran the same algorithm on v3.0.0, here is the diff with what I got from main. Is there anything wrong this time ?

 				"github.com/cosmos/cosmos-sdk/x/bank",
 				"github.com/cosmos/cosmos-sdk/x/capability",
 				"github.com/cosmos/cosmos-sdk/x/staking",
-				"github.com/crescent-network/crescent/v4/x/mint",
+				"github.com/crescent-network/crescent/v3/x/mint",
 				"github.com/cosmos/cosmos-sdk/x/distribution",
 				"github.com/cosmos/cosmos-sdk/x/gov",
 				"github.com/cosmos/cosmos-sdk/x/params",
	                        "github.com/cosmos/cosmos-sdk/x/crisis",
 				"github.com/cosmos/cosmos-sdk/x/slashing",
 				"github.com/cosmos/cosmos-sdk/x/feegrant/module",
 				"github.com/cosmos/cosmos-sdk/x/authz/module",
-				"github.com/cosmos/ibc-go/v3/modules/core",
-				"github.com/cosmos/ibc-go/v3/modules/apps/transfer",
+				"github.com/cosmos/ibc-go/v2/modules/core",
 				"github.com/cosmos/cosmos-sdk/x/upgrade",
 				"github.com/cosmos/cosmos-sdk/x/evidence",
+				"github.com/cosmos/ibc-go/v2/modules/apps/transfer",
 				"github.com/cosmos/cosmos-sdk/x/auth/vesting",
 				"github.com/tendermint/budget/x/budget",
-				"github.com/crescent-network/crescent/v4/x/farming",
-				"github.com/crescent-network/crescent/v4/x/liquidity",
-				"github.com/crescent-network/crescent/v4/x/liquidstaking",
-				"github.com/crescent-network/crescent/v4/x/liquidfarming",
-				"github.com/crescent-network/crescent/v4/x/claim",
-				"github.com/crescent-network/crescent/v4/x/marketmaker",
-				"github.com/crescent-network/crescent/v4/x/lpfarm",
-				"github.com/cosmos/ibc-go/v3/modules/apps/27-interchain-accounts",
+				"github.com/crescent-network/crescent/v3/x/farming",
+				"github.com/crescent-network/crescent/v3/x/liquidity",
+				"github.com/crescent-network/crescent/v3/x/liquidstaking",
+				"github.com/crescent-network/crescent/v3/x/liquidfarming",
+				"github.com/crescent-network/crescent/v3/x/claim",
+				"github.com/crescent-network/crescent/v3/x/marketmaker",
+				"github.com/crescent-network/crescent/v3/x/lpfarm",
 				"github.com/cosmos/cosmos-sdk/x/auth/tx",
 				"github.com/cosmos/cosmos-sdk/client/grpc/tmservice",

If needed this is the complete module list for v3.0.0 :

	                        "github.com/cosmos/cosmos-sdk/x/auth",
				"github.com/cosmos/cosmos-sdk/x/genutil",
				"github.com/cosmos/cosmos-sdk/x/bank",
				"github.com/cosmos/cosmos-sdk/x/capability",
				"github.com/cosmos/cosmos-sdk/x/staking",
				"github.com/crescent-network/crescent/v3/x/mint",
				"github.com/cosmos/cosmos-sdk/x/distribution",
				"github.com/cosmos/cosmos-sdk/x/gov",
				"github.com/cosmos/cosmos-sdk/x/params",
				"github.com/cosmos/cosmos-sdk/x/crisis",
				"github.com/cosmos/cosmos-sdk/x/slashing",
				"github.com/cosmos/cosmos-sdk/x/feegrant/module",
				"github.com/cosmos/cosmos-sdk/x/authz/module",
				"github.com/cosmos/ibc-go/v2/modules/core",
				"github.com/cosmos/cosmos-sdk/x/upgrade",
				"github.com/cosmos/cosmos-sdk/x/evidence",
				"github.com/cosmos/ibc-go/v2/modules/apps/transfer",
				"github.com/cosmos/cosmos-sdk/x/auth/vesting",
				"github.com/tendermint/budget/x/budget",
				"github.com/crescent-network/crescent/v3/x/farming",
				"github.com/crescent-network/crescent/v3/x/liquidity",
				"github.com/crescent-network/crescent/v3/x/liquidstaking",
				"github.com/crescent-network/crescent/v3/x/liquidfarming",
				"github.com/crescent-network/crescent/v3/x/claim",
				"github.com/crescent-network/crescent/v3/x/marketmaker",
				"github.com/crescent-network/crescent/v3/x/lpfarm",
				"github.com/cosmos/cosmos-sdk/x/auth/tx",
				"github.com/cosmos/cosmos-sdk/client/grpc/tmservice",

jeronimoalbi · 2023-01-04T09:44:58Z

Thanks for testing Crescent core and writing this up @jeronimoalbi. I know this issue is already closed and there was an issue with Crescent Core. I would like to let you know that it has been resolved in this crescent-network/crescent#101.

Thank you very much for the feedback @queencre!

Just generated the Typescript code and I can see that it works, all the basic modules that were registered in the app are discovered 🎉 .

queencre · 2023-01-05T06:12:45Z

@tbruyelle It looks fine to me.

jeronimoalbi added the type:feature To implement new feature. label Sep 8, 2022

jeronimoalbi self-assigned this Sep 8, 2022

jeronimoalbi mentioned this issue Sep 9, 2022

feat(pkg/cosmosanalysis): support discovery of modules defined in a variable #2820

Merged

aljo242 closed this as completed in #2820 Sep 30, 2022

queencre mentioned this issue Nov 9, 2022

Fix TypeScript client code generation using Ignite CLI crescent-network/crescent#100

Closed

4 tasks

tbruyelle mentioned this issue Jan 5, 2023

test: cosmosanalysis crescent #3370

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

`pkg/cosmosanalysis`: support discovery of modules defined in a variable #2818

`pkg/cosmosanalysis`: support discovery of modules defined in a variable #2818

jeronimoalbi commented Sep 8, 2022

clockworkgr commented Sep 15, 2022

jeronimoalbi commented Sep 19, 2022 •

edited

jeronimoalbi commented Sep 21, 2022

jeronimoalbi commented Sep 23, 2022

queencre commented Jan 2, 2023 •

edited

tbruyelle commented Jan 2, 2023

queencre commented Jan 3, 2023

tbruyelle commented Jan 3, 2023 •

edited

jeronimoalbi commented Jan 4, 2023

queencre commented Jan 5, 2023

pkg/cosmosanalysis: support discovery of modules defined in a variable #2818

pkg/cosmosanalysis: support discovery of modules defined in a variable #2818

Comments

jeronimoalbi commented Sep 8, 2022

clockworkgr commented Sep 15, 2022

jeronimoalbi commented Sep 19, 2022 • edited

jeronimoalbi commented Sep 21, 2022

jeronimoalbi commented Sep 23, 2022

queencre commented Jan 2, 2023 • edited

tbruyelle commented Jan 2, 2023

queencre commented Jan 3, 2023

tbruyelle commented Jan 3, 2023 • edited

jeronimoalbi commented Jan 4, 2023

queencre commented Jan 5, 2023

`pkg/cosmosanalysis`: support discovery of modules defined in a variable #2818

`pkg/cosmosanalysis`: support discovery of modules defined in a variable #2818

jeronimoalbi commented Sep 19, 2022 •

edited

queencre commented Jan 2, 2023 •

edited

tbruyelle commented Jan 3, 2023 •

edited