Can we make AccAddressFromHex safer?

[ebuchman]

Juno seems to have sent funds to a tx hash: https://www.coindesk.com/tech/2022/05/05/typo-moves-36m-in-seized-juno-tokens-to-wrong-wallet/

It looks like they converted a 64-byte tx hash to an address: https://github.com/CosmosContracts/juno/blob/v4.0.0/app/upgrade/upgrade_handler.go#L17-L21

Addresses are 20 bytes, while this thing is 32 bytes. Can we get AccAddressFromHex to panic if the address is not the right length?

I know there is some work to allow longer addresses, but maybe it can still be configured somehow to prevent errors like this ?

[webmaster128]

32 byte addresses are perfectly valid these days. Yes, this is the same length as tx hashes. But 32 is common for a lot of things.

Isn't the issue more there source of the data? I'm not familiar with what junod keys parse juno1nz96hjc926e6a74gyvkwvtt0qhu22wx049c6ph6f4q8kp3ffm9xq5938mr does, but it does not seem to be bech32 decoding.

Using CosmJS to decode the input, you get a different 32 byte data:

$ npx @cosmjs/cli@0.28.0 --code 'import { fromBech32, toHex } from "@cosmjs/encoding"; const { prefix, data } = fromBech32("juno1nz96hjc926e6a74gyvkwvtt0qhu22wx049c6ph6f4q8kp3ffm9xq5938mr"); console.log(toHex(data))'
>> 988babcb0556b3aefaa8232ce62d6f05f8a538cfa971a0df49a80f60c529d94c

Same result in the Go code:

$ ~/go/bin/wasmd keys parse juno1nz96hjc926e6a74gyvkwvtt0qhu22wx049c6ph6f4q8kp3ffm9xq5938mr
human: juno
bytes: 988BABCB0556B3AEFAA8232CE62D6F05F8A538CFA971A0DF49A80F60C529D94C

[ebuchman]

Right thanks, good to know.

What if we just get rid of AccAddressFromHex? Or at least call it Unsafe or something? Seems like it's hardly used.

[webmaster128]

Yeah. Not sure why AccAddressFromBech32 was not used. Maybe the global config causes issues in some contexts? In this case it would be good to have an bech32 to bytes converter that ignores the prefix.

[giansalex]

Yeah. Not sure why AccAddressFromBech32 was not used. Maybe the global config causes issues in some contexts? In this case it would be good to have an bech32 to bytes converter that ignores the prefix.

bech32 could be used perfectly, there was no need to use hex address