Improved address handling

[webmaster128]

Over the time it became clear that HumanAddr and CanonicalAddr are not equally important for contract developers. The developer friendlyness and JSON friendlyness made HumanAddr the primary choice for messages and APIs (such as ContractInfo::address from Env).

The goal number 1 here is to reflect that by having a primary Addr type that is more or less HumanAddr and keep CanonicalAddr for use cases where it is desired to use a compact fixed length address representation, such as the embedded addresses in the keys of the balances/allowances in erc20.

There is also a desire to store such human readable addresses in the contract's state directly, which for JSON encoded state is similarly efficient as canonicalizing and base64 encoding the address. It has been concluded that the change of address prefixes during a chain upgrade is unlikely. So goal number 2 is to make this possible. The main problem here is the validation of an address before it is written to state permanently. In #802 and address validation API is added, but nothing prevents the contract developer from forgetting it.

In order to make it very hard to forget address validation, it would be good if Addr was immutable and could only be created via Addr::unchecked(input) or let checked: Addr = deps.api.addr_validate(input)?. This also means messages sent to the contract would use String instead of Addr and Addr does not implement Deserialize because this would implicitly create unchecked addresses. This leads to the problem: how to differentiate between untrusted JSON deserialization in messages from users and trusted JSON deserialization from state? Right now this works because state and messages use different types and nobody uses CanonicalAddr for messages to the contract.

Here is a rough roadmap

• Provide new address validation API (Thoughts on addresses #802)
• Find a safe way to ensure contract developers do not forget address validation
• Create new immutable, safe Addr type
• Deprecate HumanAddr, which is more or less a String alias
• Encourage the use of Addr after validating String inputs in state

[ethanfrey]

In order to make it very hard to forget address validation, it would be good if Addr was immutable and could only be created via Addr::unchecked(input) or let checked: Addr = deps.api.addr_validate(input)?. This also means messages sent to the contract would use String instead of Addr and Addr does not implement Deserialize because this would implicitly create unchecked addresses. This leads to the problem: how to differentiate between untrusted JSON deserialization in messages from users and trusted JSON deserialization from state? Right now this works because state and messages use different types and nobody uses CanonicalAddr for messages to the contract.

I like the idea of two ways of creating address: Addr::unchecked<T: Into<String>>(input: T) mainly designed for test code. And let checked: Addr = deps.api.addr_validate(input)? to parse and validate user input.

I think the Serialization/Deserialization issue is overkill, we want to serialize/deserialize it in state. The migration guide should say more or less "change HumanAddr -> String, change CannonicalAddr -> Addr, and then update the conversion function to be a validate function".

We cannot prevent devs from making mistakes. Maybe a simple script to check for Addr in msgs.rs file and flag those and recommend that in people's CI scripts? But I am against making nearly unusable types in the name of developer safety. If they don't follow API guidelines, there are 100s of other mistakes they can make in their code.

[ethanfrey]

One way to provide warnings to user is to not implement JsonSchema for Addr. Since the standard practice is to output all user-visible messages, decorating with JsonSchema, while not providing that for the internal state.rs types, having Addr not derive JsonSchema will produce compile errors for people that follow general patterns and accidentally leave it in msgs.rs. This is probably a better check than the script I described above.

[webmaster128]

Good, thank you. Both are good ways to mitigate the problem.