Skip to content
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

[Bug]: Module not Registered but still used on client #17888

Closed
1 task done
cloud-j-luna opened this issue Sep 26, 2023 · 6 comments
Closed
1 task done

[Bug]: Module not Registered but still used on client #17888

cloud-j-luna opened this issue Sep 26, 2023 · 6 comments

Comments

@cloud-j-luna
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

What happened?

I want to be able to pass the modules to register in my client. So that I can at runtime decide whether it will be a cosmos client or other chain.

I have the following variable:

var AkashModuleBasics = []module.AppModuleBasic{
	agov.AppModuleBasic{}, // This comes from Akash's governance module.
}

And this method that adds the modules together:

func NewBasicManager(modules ...[]module.AppModuleBasic) module.BasicManager {
	all := []module.AppModuleBasic{
		// accounts, fees.
		auth.AppModuleBasic{},
		// tokens, token balance.
		bank.AppModuleBasic{},
	}

	for _, appModule := range modules {
		all = append(all, appModule...)
	}

	return module.NewBasicManager(all...)
}
encodingConfig := MakeEncodingConfig()

std.RegisterLegacyAminoCodec(encodingConfig.Amino)
std.RegisterInterfaces(encodingConfig.InterfaceRegistry)
NewBasicManager(AkashModuleBasics).RegisterLegacyAminoCodec(encodingConfig.Amino)
NewBasicManager(AkashModuleBasics).RegisterInterfaces(encodingConfig.InterfaceRegistry)

This works if I try to send tokens between Akash addresses.
If I pass an empty []module.AppModuleBasic{} like this:

encodingConfig := MakeEncodingConfig()

std.RegisterLegacyAminoCodec(encodingConfig.Amino)
std.RegisterInterfaces(encodingConfig.InterfaceRegistry)
NewBasicManager([]module.AppModuleBasic{}).RegisterLegacyAminoCodec(encodingConfig.Amino)
NewBasicManager([]module.AppModuleBasic{}).RegisterInterfaces(encodingConfig.InterfaceRegistry)

I get the error invalid Bech32 prefix; expected akash, got cosmos which is weird, since I did not include any reference to AkashModuleBasics.

Now, if I remove all the modules from inside the AkashModuleBasics so that the slice is empty, the code works.
It leads me to believe that there is some kind of reflection in use that finds my AkashModuleBasics even if I do not specifically register it.

Cosmos SDK Version

v0.45.16

How to reproduce?

No response

@julienrbrt
Copy link
Member

Where exactly do you get this error?
There is no reflection in the basic manager.

@cloud-j-luna
Copy link
Author

On this line addr, err := sdk.AccAddressFromBech32(source).

@troian
Copy link
Contributor

troian commented Sep 26, 2023

cosmos-sdk config is a singleton, whenever akash module included it will call for this and seal the config which includes bech type

@cloud-j-luna
Copy link
Author

cosmos-sdk config is a singleton, whenever akash module included it will call for this and seal the config which includes bech type

@julienrbrt Any chance this can be discussed? This limits SDK applications to manage different chains, or is there a known workaround?

@julienrbrt
Copy link
Member

cosmos-sdk config is a singleton, whenever akash module included it will call for this and seal the config which includes bech type

@julienrbrt Any chance this can be discussed? This limits SDK applications to manage different chains, or is there a known workaround?

That's not due to the SDK. Apparently importing Akash modules has side effects (that init function will be called). There is nothing the SDK can do about it.

@troian
Copy link
Contributor

troian commented Sep 26, 2023

Yeah, it's kind of wanky solution from our side, coming from the fact the sdk.GetConfig which returns singleton is still in use within cosmos-sdk. We had a few cases when config instantiation was missing from API standpoint and decided to put that in init so it get's initialized every time akash SDK is included.

@julienrbrt julienrbrt closed this as not planned Won't fix, can't repro, duplicate, stale Sep 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants