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

MockApi's addr_humanize incompatible with instantiate2_address #1648

Closed
webmaster128 opened this issue Mar 25, 2023 · 3 comments · Fixed by #1914
Closed

MockApi's addr_humanize incompatible with instantiate2_address #1648

webmaster128 opened this issue Mar 25, 2023 · 3 comments · Fixed by #1914

Comments

@webmaster128
Copy link
Member

instantiate2_address returns a CanonicalAddr of 32 bytes. However, this check prevents the use of

        let address = deps
            .api
            .addr_humanize(&instantiate2_address(&checksum, &creator, &salt)?)?;

In the virus contract we already have this pattern. However, it is not unit tested there, so this was missed.

@webmaster128 webmaster128 added this to the 1.3.0 milestone May 4, 2023
@chipshort
Copy link
Collaborator

Just changing the length won't work, since we don't always get valid utf8 from the bech32 data. So, we would have to do some kind of different encoding that covers string input (to allow addresses like "alice"), but that also decodes the bech32 data (not necessarily to the real bech32 human readable version, but some valid string)

One option would be to just try to encode bech32 in addr_humanize, but that feels a bit heavy.
Another thing we could do is just base64 encode it if it's bech32 (that will give us a valid string, but obviously not the actual bech32 one, but I think that's probably fine for testing).

One thing to keep in mind is that with this kind of special handling, they won't be inverse functions anymore (at least for the addr from instantiate2_address): addr_canonicalize(addr_humanize(bech32)) != bech32

@webmaster128
Copy link
Member Author

See also CosmWasm/cw-multi-test#39.

Removing this from the milestone for now since it is unclear how to best solve it.

@webmaster128
Copy link
Member Author

This is a workaround I use in one project (https://github.com/noislabs/nois-contracts/blob/v0.15.1/contracts/nois-gateway/src/contract.rs#L272-L283):

    #[allow(unused)]
    let address = instantiate2_address(&checksum, &creator, &salt)
        .map_err(|e| StdError::generic_err(format!("Cound not generate address: {}", e)))?;

    let address = {
        #[cfg(not(test))]
        {
            deps.api.addr_humanize(&address)?
        }
        #[cfg(test)]
        cosmwasm_std::Addr::unchecked("some payment address")
    };

The idea is pretty much that in the test case, instantiate2_address+addr_humanize does not work together. So we don't use addr_humanize in #[cfg(test)] but create an address differently. Here you could also use a bech32 encoder or create a hex address from the address bytes address.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants