###### tags: `Cosmos SDK`

# SDK Architecture Review - 2021-07-16

The **SDK Architecture Review** meeting is a bi-weekly meeting focused on architecture discussions of ADRs and features currently in development (any issues tagged with the label "Status: Needs Architecture Review").

## Agenda

- [x] Working Group Check In
- https://github.com/cosmos/cosmos-sdk/wiki/Architecture-Design-Process
- [x] ADR 40 (multistore)
- https://github.com/cosmos/ibc-go/discussions/256
- [x] x/gov Deposit queries
- https://github.com/cosmos/cosmos-sdk/pull/9519#discussion_r668537465
- [x] bech32 prefixes move to x/auth
- https://github.com/cosmos/cosmos-sdk/issues/9690
- [x] Separate go.mod's For Modules
- https://github.com/cosmos/cosmos-sdk/pull/9631
- https://github.com/cosmos/cosmos-sdk/pull/9631#issuecomment-876698347
- [x] Interoperability (discuss interoperability for x/bank and x/nft)
- https://github.com/cosmos/cosmos-sdk/pull/9709
- [x] golang proto v2 codegen update and breaking changes review
- https://github.com/cosmos/cosmos-sdk/issues/7488
- [x] 0.43 Release

...

- [ ] Decimals
- [ ] Extendable GRPC routers in BaseApp
- https://github.com/cosmos/cosmos-sdk/issues/9641
- [ ] localnet failing on non linux/amd64 platforms
- https://github.com/cosmos/cosmos-sdk/issues/9635
- [ ] ADR 044: Guidelines for Updating Protobuf Definitions
- https://github.com/cosmos/cosmos-sdk/issues/9477
- https://github.com/cosmos/cosmos-sdk/pull/9613

## Notes

### Working Group Check In

**Consensus Fees**
- No update

**Gov/Group Module Alignment**
- Being worked on. No updates to the architecture
- Calum starting with implementation, and will do an ADR once he gets a bit more progress

**Module Wiring**
- There's rough alignemnt on a design (similar to the dig based design)
- Aaron has started some small PRs to start working on pieces for merging
- Hoping for some pieces ready for review next week
- Robert: Yesterday we aligned on the dynamic approach for module configuration (e.g parameters for hte modules will be defined in a config file)

**x/auth, x/bank and vesting re-design**
- Decided to move forward w/ the first thing being:
- refactoring bech32 prefixes in the auth module
- There's an issue, and possibly worth some discussion of the side-effects

**Protobuf golang v2 migration and codegen**
- Now we have Frodji & Ru supporting on this work
- Next steps is getting fast path methods implemented on v2 message interface

**Offline signatures**
- In the last sprint planning, decided that it will get merged this sprint

**NFT support for the SDK**
- Last week week, ADR was merged
- Still some active discussions on github discussion page
- Proposal is already implemented
- Billy: Efforts for ICS will happen independently and after the implementation of the NFT itself. We should be looking forward to anticipate, but not blocking on waiting for that spec to be completed for any ADRs to be used by chains

### ADR-40 Multistore
- Billy: I wrote a comment a few hours ago, seems like there's light agreement that multistore is bad and we'd like to get rid of it. How do we prioritze this?
- While we all want to get rid of this, its really just a nice to have, and not worth putting in ADR040 as it changes a lot also from the IBC team that it wouldn't really alleviate much stress
- Robert: WHy do we want to remove the multistore? bc it complicates the module wiring
- Alex Bez: Also the main reason being that its not atomic
- Aaron: At the level of storage we want to have one atomic commit
- Dev: As a module, do i still have flexibility to use an alternative merklized data structure? and an alternative backend?
- You can still have your own custom merkliziation scheme in your module or database, and a way to commit to it.
- Dev: So its that every commit method doesn't terminate until all constituent database finishes committing?
- Aaron: There's a pathway to do this, but its not something that we have explicit plans to support just yet
- Aaron: and to jump back to the IBC topic, i think what we want to do is first do the changes on the IBC side (do something that allows for this proof format that doesn't allow a multistore) and then provide an upgrade that removes the multistore
- Billy: How does that change the workload on hte IBC team itself? Is there still upfront work necessary?
- Aaron: If we want to make it so that hte multistore is not part of the picture in the SDK, the IBC team first needs to get that work done, and in an IBC upgrade ensure that zones dont become incompatible w/ IBC by removing hte multistore
- Billy: I'm suggesting that the work replacing IAVL w/ SMT happen first. Can't we separate these things and address them with separate timelines?
- Aaron: I think that's already the plan. and there's a short term solution as well. We could have a multistore with one store, could potentially not break IBC in the same way.
- Robert: I think it will... The way IBC is constructed, you need to veirfy the IBC module proofs themself. The commitment in breaking the 2 things... The store prefix & the key..
- Aaron: Say we take the design that Frodji put forth. In his design we would have a protobuf message w/ forms hte prefix key for a given substore. In that case, we could put all messages that are being serialized in one root for IBC. I don't know to what extent IBC needs to be aware of hte other modules... but just thorwing that out there as an idea.
- Robert: Just to answer your question, in the github discussion what you propose is the first solution. It's possible, but will require still work from the IBC team.
- Aaron: As a follow-up we should probably establish a bit more formally the ADR-40 group.

### x/gov Deposit queries
- Do we need to query deposits which have been refunded because a proposal was passed?
- Robert: When proposal exits voting, we delete the proposal
- In a test, we want to query the deposit of a deleted proposal. We decided that we should just not remove the proposals so that we keep the record on-chain. However, this still seems bad.
- Why should i be able to query a deposit which was refunded to me?
- Billy: You'd want to be able to query proposals which have passed when they're done. Seems like this is similar info to the metadata on a proposal

### bech32 prefixes move to x/auth
- We are moving forward w/ removing the global bech32 prefixes
- For code that is in keepers will need to use the auth keeper to encode & decode bech32 prefixes
- This is a pretty big breaking change. We've already done the refactor to return strings in GetSigners()
- We would probably also need a parameter in the CLI context that would have the bech32 parameters
- We would probably do similar things w/ the vlaidator operator & consensus addresses, as the staking keeper needs these as well

### Separate go.mod's For Modules
- The idea is we could try this with group module & NFT module with smaller release teams
- Billy: How would this work w/ larger dependencies & versioning? Each module would require a set of verisons for the SDK or versions itself? It's own go.mod would ensure that?
- Aaron: You wouldn't be able to have circular dependencies btw modules (which we still have in some cases)
- Each go.mod would refer to a version of hte SDK that its compatible with. We would eventually break simapp out with its own go.mod as well
- Part of the motivation here is that its hard in a large codebase to not create dependencies that cut across in a large cyclical way
- Billy: Should we wait for v1.0 of SDK core?
- Aaron: It makes sense to start with these two modules if we want to have a quicker release process for those new modules (NFT & Group)
- There's a lot of refactoring work that we're starting to do, and if all of these thigns can move forward at a reasonable pace, then we're moving towards a 1.0 at a reasonable frame
- It might be valuable to have group & NFT not depend on a 1.0
- Marko: i think it makes sense to use group & NFT for now for this...

### Interoperability for x/bank & NFT
- Robert: It's super important that if modules want to interact w/ IBC, they will need to use x/NFT as a common repository, so they will need to proxy all the ojbect management (create/update/store) to the NFT module itself
- I just created a short PR to clarify this. Looking for more comments & feedback. I just want to make sure that everyone understands what's the risk, what's needed, and what's required to make these assets interoperable
- Billy: Can you speak a bit more- the IBC needs to have a common understanding of what hte asset type is
- If someone wanted to write their own version of bank, they could still interact w/ IBC but would need to manage their own way that that plays out
- Robert: It might not be hard, but it would be difficult for hte ecosystem. If i wanted a custom bank i could modify it..
- Billy: Do you think its worth making a distinction btw BaseNFT, so that modules wanting to use NFT do leverage the common infra?
- Robert: I think its a good idea. Probably good to also make a documentation page about interoperability. I'll create a short pull request.
- Aaron: I'd be in favor of adding this language to the ADR as well
- Billy: I can comment on your PR and see if there's a line change that makes that more present.

### Golang Protov2 update
- Aaron: A new codegenerator is coming for protobuf. We are going to be migrating off of gogoproto
- This is going to be (in addition to some of the refactoring we're doing for globals), one of the biggest breaking changes that we have coming in the next few months
- Some of the types will change and lose hteir base gogoproto annotations
- Currently we're thinking of not supporting things like nullable, so everything would be pointer types instead of embedded struct types
- Our goal is a minimal featureset:
- fast marshalling
- some scalers (not go specific annotations, maybe something like Cosmos.Int which maps to SDK's Int type)
- Transparent serialization to interfaces (as a replacement of Anys)

### v0.43 Release Update
- Robert: Wednesday we released a 2nd RC. There was one issue discovered. The grpc server is blocking. There is already a PR for that. We may just jump in

## Follow Up

- [ ] Setup working group for ADR040, SMT, any interactions on storage layer. Possible members: Vulcanize, Robert, Frodji, more?
- [ ] Bez to look more into x/gov deposit query, Robert to provide more context