cosmwasm.GoAPI will not work on sdk.ValAddress

[yun-yeo]

In staking query binding, we pass sdk.ValAddress as HumanAddr for Validators and Delegation queries. Humanize and Canonicalize API only accept AccAddress. I recommend to fix cosmwasm.GoAPI to accept the purpose of address with default AccAddress.

wasmd/x/wasm/internal/keeper/query_plugins.go

Lines 132 to 138 in b650fa7

	if request.Validators != nil {
	validators := keeper.GetBondedValidatorsByPower(ctx)
	//validators := keeper.GetAllValidators(ctx)
	wasmVals := make([]wasmTypes.Validator, len(validators))
	for i, v := range validators {
	wasmVals[i] = wasmTypes.Validator{
	Address: v.OperatorAddress.String(),

wasmd/x/wasm/internal/keeper/query_plugins.go

Lines 206 to 210 in b650fa7

	result[i] = wasmTypes.Delegation{
	Delegator: d.DelegatorAddress.String(),
	Validator: d.ValidatorAddress.String(),
	Amount: convertSdkCoinToWasmCoin(amount),
	}

wasmd/x/wasm/internal/keeper/api.go

Lines 14 to 25 in b650fa7

	func humanAddress(canon []byte) (string, uint64, error) {
	if len(canon) != sdk.AddrLen {
	return "", CostHumanize, fmt.Errorf("Expected %d byte address", sdk.AddrLen)
	}
	return sdk.AccAddress(canon).String(), CostHumanize, nil
	}
	func canonicalAddress(human string) ([]byte, uint64, error) {
	bz, err := sdk.AccAddressFromBech32(human)
	return bz, CostCanonical, err
	}

[alpe]

Some additional context from @YunSuk-Yeo on discord:

When we want to store that HumanAddress(sdk.ValAddress), we usually convert HumanAddress to CanonicalAddress.
But current canonicalAddress function has no feature to convert sdk.ValAddress to bytes format

[webmaster128]

human address -> canonical address is trivial to add because you strip off information (bech32 prefix and checksum). The other direction however (canonical address -> human address) is a real problem. Here we need to know additional context and right now the whole API is only considering AccAddresses.

[ethanfrey]

I will be fixing all the other staking-related issues this next week as part of the drive for a 0.11 release.

I have thought about this and really start wondering why there are these multiple types in the CosmosSDK in the first place.

For example, the validator operator address is shown as cosmosvaloper123...., while normal AccAddresss are shown as cosmos1234.... However, the valoper is actually a normal account, it can send funds with a normal cosmos-sdk AccAddress. It just seems to be using a different encoding format when displaying them for the staking subsystem. This has caused me a headache outside of CosmWasm when attempting to send tokens to the valoper on a testnet, so it could self-stake more.

Rather than pull this complexity into the CosmWasm side, I would remove it. There are only two different pubkeys that matter - the secp256k1 sdk key, which produces an sdk.AccAddress. This can sign sdk.Tx and actually move funds and trigger contracts. And the ed25519 tendermint pubkey, which signs block. This is listed in the validator info, but not used in any transactions on the sdk.

My proposal is simply to treat Validator as sdk.AccAddress on the wasmd <-> CosmWasm boundary. You will get the normal cosmos1234... keys here and if we need to encode them differently for submitting to the staking subsystem, the handler/querier_plugins in wasmd will take care of that transition.

[ethanfrey]

@YunSuk-Yeo do you agree to my proposal to turn the sdk.ValAddress in sdk.AccAddress bech32 format for the contracts? I think it is the simplest way to handle it and honestly, I have not heard a good argument for the different encoding for the validators... to make sure you don't stake to an account that is not a validator? But that is checked with logic anyway

[ethanfrey]

In the absence of a response, I would just leave it as is, and say the best "work around" is to leave Validator addresses as HumanAddr in rust and never convert to/from CanonicalAddr. We can revisit this for 0.12

[webmaster128]

I have thought about this and really start wondering why there are these multiple types in the CosmosSDK in the first place.

Excellent question. The two validator addresses shown at e.g. https://bigdipper.coralnet.cosmwasm.com/validator/coralvaloper10zn0d2eeust0495crtr3zqz7t688hg0sm7wr4f contain the exact same data in the bech32 address.

coralvaloper10zn0d2eeust0495crtr3zqz7t688hg0sm7wr4f
Prefix: coralvaloper
Data: 78a6f6ab39e416fa96981ac711005e5e8e7ba1f0

coral10zn0d2eeust0495crtr3zqz7t688hg0s53afrh
Prefix: coral
Data: 78a6f6ab39e416fa96981ac711005e5e8e7ba1f0

That means you can freely convert between the two.

[ethanfrey]

Any idea how to proceed here?

Currently if the user provides the coralvaloper type addresses, we just keep them as HumanAddr in the rust code, which is fine with me. In fact, the cosmos-sdk is using more and more bech32 strings instead of binary, so maybe we even want to just promote HumanAddr as a storage mechanism???

In any case, we can modify HumanAddr -> CanonicalAddr to accept both types of addresses, but then Canonical->Human will always go to the coral prefix so we lose info. Some ideas how to close this issue

• Provide a valdiate_addr(human: &HumanAddr) -> StdResult<()> which ensures this is a valid address for this chain. It may accept both coral and coralvaloper addresses. Then we still can store the validator addresses in bech32, but at least ensure they are valid.
• Add two more methods to the api. humanize_validator and canonicalize_validator that work like the existing functions, but expect coralvaloper style addresses.

Other ideas? Preferences for one of the two approaches above?

[yun-yeo]

Sorry delayed response, hmm we have not a good option for validator address now.

I'm personally prefer your suggestion. Cosmos has more bech32 prefixes, we cannot ensure which is the proper prefix.

• if the user provides the coralvaloper type addresses, we just keep them as HumanAddr in the rust code