ADR-028 extensible address format: breaking changes investigation

[robert-zaremba]

Summary

ADR-028 is very important because current address format is not future safe if we want to expand to more different account and pubkey schemes .

Discussion about moving ADR-028 is in #5694

Problem Definition

Changing address format is a breaking change for many apps.

Proposal

List and document the dev and client issues with changing the address format.

[robert-zaremba]

[EDIT]

This things should work because we don't change addresses for existing keys.

• SDKs and any library which are generating a pubkey for secp256k1
• all wallets which generate a pubkey (mobile, browser, hardware) for secp256k1 keys
• block explorers and indexers which relay on current pubkey -> address relationship for secp256k1 keys

[aaronc]

So just to be clear, I am not aware of any proposal to change current addresses. What I have proposed is that new addresses be a different length so that 20 byte addresses are grandfathered as legacy and all non-20 byte addresses are new.

What will break then is the assumption that addresses are always 20 bytes. First and foremost there is a lot of SDK code relying on this assumption - just do a find usages on the address length constant.

For clients, again let's be clear on what will actually break and as far as I know it is simply that new addresses will not be 20 bytes. What you're outlining @robert-zaremba are not things that will break AFAIK (except things affected by ADR 034 but that's a separate concern). Nobody has proposed rewriting secp256k1 addresses to some other format.

[robert-zaremba]

How about ed25519 addresses?

[robert-zaremba]

BTW - I have reformulated my initial list above

[aaronc]

Ed25519 has never been officially enabled so as far as the SDK is concerned that address format is still up discussion

[robert-zaremba]

Maybe other SDK users have a custom accounts and would be good to hear their concerns.

The goal of this issue is to investigate all problems we may face both with existing software and potential adoption path.

[aaronc]

Maybe other SDK users have a custom accounts and would be good to hear their concerns.

The goal of this issue is to investigate all problems we may face both with existing software and potential adoption path.

Maybe, but I think that's mostly outside of our scope of concern. The whole time we've had this discussion, everyone has emphasized that the only things we need to treat as legacy are secp256k1 and amino multisigs. If people have forked the SDK and added other keys, they should have known what they were getting into because it's really not that easy.

[aaronc]

So currently within the SDK, the sdk.AddrLen constant is used only when creating or parsing KVStore keys in the following modules:

• bank
• distribution
• gov
• slashing
• staking

I see 3 ways to refactor this code to accommodate a new address format:

(1) The mostly painless way

Allow variable length addresses but set a MaxAddrLen constant. Then all store keys which are currently use fixed length address arrays would instead use arrays that are MaxAddrLen + 1 bytes long with the first byte being a length prefix.

So if we had 40 byte addresses, store key functions would encode existing 20 byte addresses in a 41 byte array with the first byte being the byte 20, the next 20 bytes being the address, and the rest of the array being filled with zeros. Store key functions could do this using a helper function and the refactor would likely be relatively painless.

(2) The more efficient way

We could allow variable length addresses with no MaxAddrLen required and use a surrogate uint64 key (ex. account_number) instead of actual addresses in store keys. Then we'd always need 8 bytes per address in store keys, instead of 20 or 41 or whatever. The downside is that this would require converting to and from the surrogate key when forming addresses. It wouldn't be crazy hard, but it's probably a significantly bigger refactor than 1.

(3) The probably bad way

Set a larger fixed AddrLen and re-encode all 20 byte addresses in the new AddrLen with some sort of prefix. We could set things up so that all methods still accept 20 byte bech32 strings, but then the re-encoding happens behind the scenes. But this would be weird for query responses and probably a bunch of other things. Clients would probably hate this and we probably shouldn't even think about it.

[amaury1093]

Thanks @aaronc, that's helpful. Some immediate questions:

a. When you say (2) is efficient, you mean space-efficient, right? Is it that bad to have 41-byte keys? Maybe having slightly larger store-key sizes wins over doing an additional store read (to retrieve the address mapped to an account_number) in most keeper functions.

b. x/auth's state is a map address->account, where account contains the account_number. Afaik we don't have a map account_number->address or account_number->account. (2) would require such a map in x/auth's state, right?

c. It seems that variable-length addresses (without MaxAddrLen and padding) is not considered. Is that because it requires the introduction of some sort ORM to handle indexing, and that would be too big a refactor?

Also, I'm a bit confused why there are two issues #5694 and this one.

[aaronc]

Thanks @aaronc, that's helpful. Some immediate questions:

a. When you say (2) is efficient, you mean space-efficient, right? Is it that bad to have 41-byte keys? Maybe having slightly larger store-key sizes wins over doing an additional store read (to retrieve the address mapped to an account_number) in most keeper functions.

Right

b. x/auth's state is a map address->account, where account contains the account_number. Afaik we don't have a map account_number->address or account_number->account. (2) would require such a map in x/auth's state, right?

Right

c. It seems that variable-length addresses (without MaxAddrLen and padding) is not considered. Is that because it requires the introduction of some sort ORM to handle indexing, and that would be too big a refactor?

I'm confused. That's (2). Am I missing something?

Also, I'm a bit confused why there are two issues #5694 and this one.

🤷

[amaury1093]

I'm confused. That's (2). Am I missing something?

I was referring to variable-length addresses inside the store key. Same as (1), but without MaxAddrLen and padding. That won't use account numbers at all.

[aaronc]

I was referring to variable-length addresses inside the store key. Same as (1), but without MaxAddrLen and padding. That won't use account numbers at all.

Oh, well you need some sort of way of splitting components in store keys. It's hard to do that with binary data without fixed length keys. We could serialize addresses as strings and use the null terminator to separate but that's even more storage space. I guess we can give that as a fourth alternative.

[amaury1093]

Oh, well you need some sort of way of splitting components in store keys. It's hard to do that with binary data without fixed length keys. We could serialize addresses as strings and use the null terminator to separate but that's even more storage space. I guess we can give that as a fourth alternative.

Maybe I'm missing something, but afaiu you would just need a length prefix: store-key = [components-before][addr-length-prefix-byte][variable-length-addr][components-after], as presented in (1), but without filling in with zeroes.