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

# SDK Architecture Review - 2021-07-02

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] Implement ADR 032: Typed Events
- https://github.com/cosmos/cosmos-sdk/issues/7563
- [x] NFT Module: Use or not use x/bank
- https://github.com/cosmos/cosmos-sdk/discussions/9065
- https://github.com/cosmos/cosmos-sdk/issues/9619
- https://github.com/cosmos/cosmos-sdk/pull/9620
- [x] ADR 40 PRs:
+ https://github.com/cosmos/cosmos-sdk/pull/9451 (update ADR-40 by adding low level access)
+ https://github.com/cosmos/cosmos-sdk/pull/9355 ( adr-40: use prefix store instead of multistore)
- [x] App Module Wiring Update/Presentation
- https://github.com/cosmos/cosmos-sdk/discussions/9182

...

- [ ] Decide about future of sdk.Dec
- https://github.com/cosmos/cosmos-sdk/issues/7773
- [ ] Use decimals in x/bank
- https://github.com/cosmos/cosmos-sdk/issues/7113

## Notes

### Working Group Check In

**Consensus Fees**
- Robert: There was no meeting in the past few months, currently

**Gov/Group Module Alignment**
- Aaron: Calum is moving forward w/ teh minimal gov refactoring, and planning to work on an ADR for a larger refactoring
- We might start doing some lightweight design work with a designer
- Introduction for John Kemp -- Long history of doing key mgmt systems (more traditional nature), cryptosigning with hardware of map-tile images for mapping companies. Key rotation for encryption/decryption keys. Don't have a lot of blockchain context so learning a lot in first week.
- Cory: would be great to have Keplr further involved in this group
- Aaron: I'll reach out to Sam on this

**Module Wiring**
- Let's leave for the update/presentation at the end

**x/auth, x/bank and vesting re-design**
- Amaury: Didn't meet this week. Next step is for me to schedule a call around use cases for vesting accounts

**Protobuf golang v2 migration and codegen**
- Amaury: Tyler's now implemented fast marshaling on proto methods as a separate method, and will have a call next week with Aaron to put this in the API

**Offline signatures**
- Robert: No meeting. Frodji or Jonathan from AiB said they would try to merge it, but have not found the time.

**NFT support for the SDK**
- ADR043, have 2 implementations already. In the discussion they were proposing both hte x/bank based approach and one not using x/bank

### Implement ADR 032: Typed Events

- We don't have Akhil or Anil on this call, so may be difficult to discuss
- Aaron: Robert and I had a short conversation. We still want to do this and its related to a mechanism for wiring hooks generically throughout the SDK. We do see this as relatively high priority, but we should probably wait until Akhil and Anil are here and have a more thought out proposal of that system
- Amaury: Could we already start creating the proto files for these events? And wire up the hook system at the end
- Aaron: The blocker is not as much around the hook system, as it is about how the tendermint event indexing system works. Actually mabye what we want to do with ADR032 is a custom event indexing system within teh SDK that's separate from Tendermint
- Sunny: what's the benefit of this?
- Aaron: When you're emitting events and listening it makes it more structured & well defined
- Sunny: The biggest concern would be that its hard to read events if they're in binary
- Marko: We found that events are not merklized / deterministic. The result transaction is merklized, but its a subset of the field in that struct, and events it not one of them
- Aaron: I just shared an events.proto file from our repo as an example [regen-ledger](https://github.com/regen-network/regen-ledger/blob/master/proto/regen/ecocredit/v1alpha1/events.proto)
- Denis: Its important to think of users of events as well. For front end apps they likely want to subscribe to certain events and listen for new updates. Right now when we need to update a balance we listen for events and then have to query the balance again. It would be great to know what part of the state an event corresponds to.
- Marko: That's actaully going to e addressed in a different ADR
- Sunny: I dont think thats what he's talking about. With events you can subscribe to an event "sends to this address"
- Aaron: This is indeed being worked on in separate ADR
- Sunny: Doesn't events give us more context around why it happens? Why do we have separate architectures for this?
- Bez: You could do that, and it may work in most cases, but it would benefit from having a model where there are events that aren't just state mutations.
- Aaron: I think just subscribing to events means you would lose that context when it comes from events. KVStore indexing is a separate topic. There isn't any merkliziation of events, and state does have it. The idea of state indexing is that you could have blockchain state stored in a relational database, which is a different use case

### NFT Module: Use or not use x/bank

- Robert: There's been a discussion & multiple implementations (one based on the ADR, one with extended bank capabilities)
- I would really like anyone whose working at all on NFTs to comment on this x/bank issue.
- Pros of x/bank approach:
- unifiied interface for all token related apps
- IBC ready
- similar logic defined in one place
- Cons
- If you want something special you have to implement it in your own module
- If you want to deal w/ asset management, you need to interface with another module
- Initially NFT was proposed to have standaline w/ line-by-line replication of functionality of ERC721
- There was a x/bank approach that got kind of approved, and then a rollback from that to move towards the non-bank approach
- Ira: A concern that provenance has (and NFT is just one way to express this)... we expect to have a lot of denoms and lots of different kinds of coins. We expect to have liquidity coming in from other chains. In our view, it would be really nice if Bank as a general asset tracking system of accounts was able to handle lots of different coins with lots of different ways of tracking things
- We were hopeful to see the bank used as a standard asset tracking system
- Aaron: I feel like my role here is to wear two hats = looking out for architecture hat of SDK, and also the hat of taking in stakeholder input
- Having a singular asset manager is likely the best long term strategy (both for FT & NFTs). I've heard some technical concerns about that approach. There are solutions to these technical concerns. I don't see many of the technical concerns that don't have very straight forward solutions
- On the other hand: Folks who are wanting to move foward w/ NFTs are worried that they will be inhibited by the bank approach
- I don't want to shove that approach down other people's throats, and so I'm trusting the NFT folks with their feedback that they don't want to use bank now
- They are open to bank as a unified asset manager possibly meeting their needs in the future
- Robert: What will be the decision process?
- Ryan: I think that we may want to bring this conversation more out into the open if we want further feedback
- Aaron: Given the current setup this is a call that I need to make, but also in this current setup I think that the call that i'm making is a call that i dont agree with technically, but one that I support because I want to support the representatives of the NFT space to be free to experiment
- I'm expressing now that the non-global asset manager appraoch can move forward, but we will need to earn teh buy-in of the NFT community in order for that appraoch to be accepted
- Ira: The cosmos SDK that's a core should have a focus on satisfying the needs that come across several different chains. If there was a process for allowing those things to develop in a way that doesnt involve them coming into the core Cosmos SDK, then that would be preferable from my perspective
- Aaron: I'd still like to see a place to convene all stakeholders from the NFT community and review the architecture design in 2-4 weeks.

### ADR040 PRs

- Robert: One concern that came up. What if a module wants to use a different store (something related to zero knowledge, etc.). We could still deal with that, and it could use a different store not used by the core. The module would only commit that store as a single record at the end blocker. Looking for more approvals or comments / disagreements.
- Second piece - the state commitments store
- In state commitments we only commit to the values. Here the idea was to open the low level interface to the state commitment store, and let hte modules commit to somethign without storing it. During the discussion, i'm personally rather against this because I think it will add more complexity to the API side. If someone would like to commit to a value without storing, they can commit to the commitment.
- We need to decide about it soon bc the ADR040 implementation is ongoing
- Aaron: So there's a question around the use case of exposing both state commitment & main store. I was the one who proposed that distinction. If we think of the ORM or the object store (the way AiB has presented the object store), the prefix of the key is actually teh fully qualified name of the protobuf Msg type. That to me makes sense to put in a state commitment. But we should find a way to keep single byte prefix keys in the state storage.
- I think it would be good for the ORM or Object Store to hook into both of those separately
- Aaron: Maybe we get together with AiB to talk about their object store and refine the use cases?

### Decide about future of sdk.Dec

- Aaron: I would like to come to a conclusion on this, but maybe we put these at the end of the queue.

### Use decimals in x/bank

...
## Follow Up

- [ ] ...