fix(cosmosclient): fix account prefix config

[lumtis]

account prefix config not correct when broadcasting a tx from a newly created default account

Fixes #2738

There is a weird bug from 0.46

When broadcasting a tx without mentioning an account with the --from flag, an account default is used. If default doesn't exist, the account will be created on the fly and will request tokens with the faucet.

In the case, default is created on the fly, the address prefix update in the SDK global config:

cli/ignite/pkg/cosmosclient/cosmosclient.go

Line 338 in 4eaa73a

config.SetBech32PrefixForAccount(c.addressPrefix, c.addressPrefix+"pub")

will not update the prefix, from cosmos to spn for example. This will make the method fails because cosmos prefix will be used and the error in the related issue will occur using network commands for example.
If the command is rerun, default will exist at this point and the command will then succeed with updated prefix.

This may be caused by a race condition since SDK config is global?

This fix is more a workaround rather than a long term fix. We also update the config when create cosmosclient in New, when doing this update in this method, the prefix will be effectively updated in BroadcastTx solving the attached issue.

We keep the call in BroadcastTx in case several instance of cosmosclient exists in the program.

In the long, term we should find another way to manage prefixes, which may require refactoring in the SDK as well, I will create an issue for this

[lumtis]

Might solves #2128

[tbruyelle]

@lubtd In the node command PR, @bjaanes has also refactored this part with a similar method.
The main difference is the mutex is not unlocked at the end of the method but at the end of the caller of that method, maybe it's worth a try?

[lumtis]

@lubtd In the node command PR, @bjaanes has also refactored this part with a similar method. The main difference is the mutex is not unlocked at the end of the method but at the end of the caller of that method, maybe it's worth a try?

I tested this and it didn't work. I really needed to set the config in New

Also, I don't think lockBech32Prefix name is adapted as the config is not locked but can be modified at anytime

[lumtis]

@lubtd In the node command PR, @bjaanes has also refactored this part with a similar method.
The main difference is the mutex is not unlocked at the end of the method but at the end of the caller of that method, maybe it's worth a try?

@tbruyelle do you have a link to the related PR?

[tbruyelle]

@tbruyelle do you have a link to the related PR?

Sure, #2669

[lumtis]

Thanks!
I think we can move forward with both PR and see when the conflict will arise, it's a small conflict so it's fine. As said I would tend to use the naming I set here for the method

[tbruyelle]

Thanks! I think we can move forward with both PR and see when the conflict will arise, it's a small conflict so it's fine. As said I would tend to use the naming I set here for the method

I investigated and found out that the error comes from the AccountRetriever :

func prepareFactory(clientCtx client.Context, txf tx.Factory) (tx.Factory, error) {
	from := clientCtx.GetFromAddress()
	fmt.Println("prefix", sdktypes.GetConfig().GetBech32AccountAddrPrefix())
	fmt.Println("from", from.String())

	if err := txf.AccountRetriever().EnsureExists(clientCtx, from); err != nil {
		return txf, err              // <---- HERE
	}

When I reproduce the error, the debug output gives

prefix spn
from cosmos1kzwujdkez5y5ahfywhfl3txzxrmkrvwgc2gxmz

from is a sdktypes.Address, where the String() method should be impacted by the configured prefix, but it also uses a cache mechanism, and since there's a hit (I suppose String() has been called before we change the prefix in the config), the cached hit is returned :/

That's a shame the cache doesn't use the prefix as part of the key!

That's why when you set the prefix in the config at the very beginning (in cosmosclient.New()), you don't have the problem.