mock_dependencies - creating canonical_address with long strings + alternate canonical_length

[ezaanm]

Hey Cosmwasm squad!

I was recently testing some stuff on 0.14 where I ran into a bug in mock_dependencies and after discussing with @orkunkl am creating an issue for it.

• Long Strings

If you create mock_dependencies as normal create a CanonicalAddress from a string of regular Terra address length (44ish) you get a GenericErr { msg: "Invalid input: human address too long" }'.
Example:

let mut deps = mock_dependencies(&[]);
let canon = deps.api.addr_canonicalize(&"terra1qfqa2eu9wp272ha93lj4yhcenrc6ymng079nu8").unwrap()
^ `Result::unwrap()` on an `Err` value: GenericErr { msg: "Invalid input: human address too long" }'

This could be because in MockApi addr_canonicalize contains this check:

if human.len() > self.canonical_length {
            return Err(StdError::generic_err(
                "Invalid input: human address too long",
            ));
        }

where canonical_length is set to 24 by default.

• Alternate canonical_length

Although frowned upon if you want to create a MockApi with an alternate canonical length instead of the default 24, canonicalizing a string and then humanizing yields different results.
Example:

let mut deps = mock_dependencies(&[]);
deps.api = MockApi {canonical_length: 44};

let dog = deps.api.addr_canonicalize(&"terra1qfqa2eu9wp272ha93lj4yhcenrc6ymng079nu8").unwrap();
println!("{:?}", deps.api.addr_humanize(&dog).unwrap().to_string());
^ prints: t2rrhyq3ga49uc8pnr2619nqj7ehuwe7caamfl02yn9e

[lukerhoads]

Hey there - I am just getting into Cosmwasm and am also new to Rust - so forgive me for the questions

What do you mean by "yields different results?" And a constructor for a Terra address might be necessary

[ezaanm]

Hey @lukerhoads no worries!

By yields a different result I mean when a string, say X, is passed into addr_canonicalize it does not equal the string received when you addr_humanize the canonicalized X. You can see this in the example I posted above. The the humanized address should print “terra1qfqa2eu9wp272ha93lj4yhcenrc6ymng079nu8“ but it does not.

[lukerhoads]

Ahh wow, I can't believe I really didn't know what that meant haha. Probably got confused with the code 😩. After all, it is pretty confusing for me with all the rotating and stuff.

Anyways, from what I understand, the riffle shuffle in the humanize function is causing the difference. We need to check if the canon length is the 44ish from Terra, and then perform the correct amount of riffle shuffles for both methods I think

[lukerhoads]

So looking at the riffle shuffle method, it basically splits the address and alternates appending to a new vector. So would we need to change the humanize methods riffle loop to perform it 12 times (I brute forced it, don't have the math skills yet 😇.

[webmaster128]

Nice find. canonical_length in MockApi should not be configurable (i.e. not public) because for other values the shuffling does not work anymore.

I agree it should be possible to use production-like Bech32 addresses in the testing environment. We always used very short test addresses like "alice" or "bob".

I think we can avoid all the special case handling and use a canonical_length of 64 for everything. Contract developers should not make any assumption on the encoded length. The chain defines it.

[lukerhoads]

Whoops, my b. Thanks much for clarifying. One question I still have... Is 18 riffle shuffles just a constant or would we need to change that too? Thanks

[webmaster128]

Whoops, my b. Thanks much for clarifying.

No worries. Thise code evolved over time and had some flaws. Your request is very valid and it is a great opportunity to make everything nicer and cleaner.

Is 18 riffle shuffles just a constant or would we need to change that too? Thanks

The only thing that matters is that the sum of shuffles restores the original data. For 24 items (bytes) you need 20 rounds, for 44 you need 14 rounds (or 28 or any other n * 14). See https://blogs.sas.com/content/iml/files/2018/09/perfectshuffle3.png from https://blogs.sas.com/content/iml/2018/09/24/perfect-riffle-shuffles.html.

The split 20 = 18 + 2 is arbitrary. You could as well use 17 + 3 or 10 + 10. If I recall correctly I used 18 to destroy as much structure as possible in the encoding to make the bytes look random.

For the 64 bytes we can use 6 = 5 + 1 rounds.

[lukerhoads]

Sounds good, last thing though, I am probably overlooking something, but in my PR (#989) the canonicalize_and_humanize_restores_original test still fails. I do not know why. I tested the 5 and 1 riffles and it should work on the string "shorty" but for some reason it doesn't when testing.

[lukerhoads]

And last thing haha: after unwrapping it is still not a string. This is probably where my beginner rust experience shines :/
Thanks

[webmaster128]

Turns out the numbers on the table linked above are for out-shuffles, not in-shuffles. How the two relate is described in https://towardsdatascience.com/the-perfect-shuffle-aa388ad1ffd1. The values are plotted here: https://miro.medium.com/max/1400/1*fUwSh8bZGQd_S0KD9QGgbg.png.

Looking at it closely, we can see that the number of out-shuffles needed for N cards is the same as the number of in-shuffles needed for N-2 cards, effectively making an in-shuffle “one step” behind an out shuffle.

So what should work is:
24 items (look up 26): 20
44 items (look up 46): 12
64 items (look up 66): 12

[lukerhoads]

Damn, this topic is deep (for me at least). Going to have to explore it sometime just for self satisfaction...

When I do 10 and 2 so many errors are thrown. Probably beyond me to know why it is happening. Specifically a bunch of wasm backend and cache errors

[webmaster128]

In #991 I prepare this change by ensuring the canonical length and the number of shuffles always work well together.

[webmaster128]

With #991 being merged it should be easy to bump CANONICAL_LENGTH in both places to 40 or 54 without changing the number of shuffles. However, MAX_LENGTH_CANONICAL_ADDRESS needed to be increased as well (e.g. 64 buut does not matter much). Do you want to give it a try, @lukerhoads?

[lukerhoads]

On it

[lukerhoads]

#995 but failing so many tests

[webmaster128]

Done in #995