# Regen Cosmos SDK Architecture Review

**Date:** August 28, 2020
**Attendees:**
**Zoom link:** https://zoom.us/j/489240122

## Agenda Items
- [x] Protobuf Pub Keys / Extensible Address Format ([#6928](https://github.com/cosmos/cosmos-sdk/pull/6928), [#5694](https://github.com/cosmos/cosmos-sdk/issues/5694))
- [x] Discuss level of support for Amino compatibility (it seams to create constant issues and takes time)
- [x] gRPC Swagger files [#7075](https://github.com/cosmos/cosmos-sdk/pull/7075)
- [x] Structural Config [7178](https://github.com/cosmos/cosmos-sdk/issues/7178)
- [x] What to do about IAVL? [#7100](https://github.com/cosmos/cosmos-sdk/issues/7100)
- [ ] ...

## Notes
- Protobuf pub keys
- Marko: on my side, we removed secp in tendermint, so when aaron wanted to move to it
- If we want backwards compatibility, the key names can't change (for amino serialization)
- Somewhere in IBC was using a tendermint mock, which got updated
- Tendermint migrated to using ed25519 keys in tests...
- Aaron: could we move the ed25519 keys if we didn't change the names?
- Marko: Sahith said he tried that out but saw some issues
- Sahith: when we use ed25519 from cosmos, the in process cli tests are causing issues from staking. I closed one issue we had w/ reverting
- To close the PR we had to use ed25519, even with keeping the same amino type name
- Aaron: Stepping back there's a couple things we're trying to tackle here:
- Change the address format for colission resistance (including all keys and ed25519)
- An alternative to bringing all the keys in, the SDK could use a different method for computing hte address
- Maybe a top level global config that involves the bech32 prefixes, and generates an address
- Maybe we don't work on bringing all keys in from tendermint, and also maybe we don't even do all protobuf key types
- Robert: i suggested a new format for computing addresses, but can we go over what hte whole goal is of computing them?
- Aaron: In the linked issue (5694), we're trying to reduce colission likelihood without having to change current address formats.
- Robert: So its ok to have 2 different schemas?
- My proposal involves in the name also having a checksum
- Aaron: the bech32 prefix is the human readable wrapper around arbitrary address bytes, and would not go away for new addresses
- Aaron: I do think it would be good for the SDK to support atleat one of tehse new keys.. We do have the ability to pass in an address function?
- Robert: Should we encode in the address from which scheme the address comes from? If it comes from one algorithm, how do we know where it comes from?
- Aaron: That's only done in the pre-image of the hash. You theoretically could have someone generate an address that is not generated from the same chain, but it would be a valid address still and nobody could retreive those coins
- Marie: So this would mean only one key was supported?
- Aaron: Any chain could actually implement their set of supported keys, as defined in the function.
- Aaron: Maybe we should put this to 0.41 ?
- Marko: The inflating of migrating keys from tendermint to SDK, this is why i was advocating for just using secp for now, and doing the rest later... Especially if extensible address format is turning into a bigger thing.
- Aaron: Does anyone else have opinions about the multi-sig? Do we need to be adding a secondary non-amino based multi-sig ?
- Zaki: I don't think its a huge deal. In fact, it probably less UX confusion to not introduce it.
- When we add new functionality, and do a bigger story around multisig, it would be a better time to adding protobuf based multi sig support
- Cory: So what's left for stargate now?
- Aaron: we should use existing pubkey encoding (oneof with pubkey codec). We don't move forward with creating protobuf pubkeys.. We'll have to add a workaround step / amino compatibility layer.
- Marie: I've already started to work on it.
- Cory: Is it ok that sahith's PR got merged?
- Aaron: That should be fine. We shouldj ust tag the issue & ADR as 0.41.
- Marko: Minor clarification - in the tm pubkey reference PR, should i be removing keys, or just leaving it as is right now?
- If i were to remove something right now, it should be ed25519 and sr key.
- Discuss level of support with amino compatibility?
- Robert: At this point is it ok to just scrap amino compatibility?
- Aaron: as a starting point, we can't drop it in this release. The question is more abt whether or not we keep it in future releases.
- Aaron: There are 2 kinds of level of support:
- we could allow folks to run transactions & api calls using amino
- we could allow for folks to use amino for state encoding onchain
- Aaron: This is a directional conversation for the community.
- Jack: It's been my understanding that in 0.41 we rip out support for amino entirely.
- gRPC swagger files
- Bez approved hte PR
- We will work with a nodejs script in the meantime, and move towards a go script in a follow up PR later
- Anil: What do we do about gRPC gatway use base64 for encoding instead of bech32?
- Aaron: hmm.. that could be an issue.
- Zaki: We are trying to nudge the ecosystem towards bech32 encoding. Ethereum hit this problem by using raw hashes in addresses, assuming ecosystem around UX would figure it all out. We're trying to avoid recreating that mistake.
- Aaron: then the easiest way to handle this is to encode all protobuf messages as strings instead of bytes. Anil- is there a way on the gPRC gateway level to chang things to bech32?
- Anil: Yes, we can write a small function / fork grpc gateway, mark all types that hsould be bech32 addresses as such.
- Aaron: Another option would be to switch all addresses in proto type to strings instead of bytes
- Anil: My suggestion would be to fork gRPC gateway.
- Cory: And how does it get handled on the CLI?
- Aaron: Actually this is an issue with jsonpb
- Aaron: Long term we could have our own custom JSON encoding ontop of protobuf
- Marko: I do believe Libra was using protobuf, with their own canonical encoding implementation, but now they're actually migrating away from gRPC to JSON RPC.

## Follow-ups
- Next direction on pubkey stuff:
- Base account will continue with current pubkey `any`
- Two types, with an equals method
- necesssary for antehandlers which compare amino tx type with proto based state type
- converstion of legacy rest endpoints (encode, decode, broadcast) to swap between amino and proto
- Update to ADR028 (Cosmos native pubkeys, that don't define their own address method)
- Cory to write up issue