cosmos / cosmos-sdk

:chains: A Framework for Building High Value Public Blockchains :sparkles:
https://cosmos.network/
Apache License 2.0
6.24k stars 3.6k forks source link

Add bank module escrows instead of account balances/blocked addresses #13212

Closed aaronc closed 1 year ago

aaronc commented 2 years ago

Summary

Problem Definition

The current bank design allows modules to have account balances that shouldn't be able to receive coins from transactions but the way for configuring this is to set some blocked addresses in app.go. This has proved fragile in a number of scenarios and discussed many times (see #12825).

Proposal

I've proposed this in a number of places but it doesn’t seem to have been tracked in its own issue.

Basically I think bank should have a separate set of balances which aren’t connected to account addresses but rather are sort of “module internal” balances. (Note that these are distinct from ADR 028 module accounts which are actually real accounts like group or cosmwasm accounts).

This would involve creating a part of the bank store which maps “module account name”/denom => balance. This would simply be the string name of the module account without any conversion to an actual address.

As an extension to this design, we could allow for ADR 033 internal module messages which do some authentication about which module is moving/minting which coins. (We'd want to rework this design to use module names instead of account addresses).

This would allow us to get rid of blocked addresses altogether and I think also make a dent in some of the tight coupling of x/auth and x/bank if we can deal with the permissions better with something like ADR 033.

alexanderbez commented 2 years ago

But module account are still accounts though, right? In which case they would still have a balance. How do you suggest we treat module accounts now? Also, there are many situations in which these accounts can and should receive funds, so keep that in mind.

tac0turtle commented 1 year ago

referenced this in a bank epic issue, closing this but it will be used for future discussions of a bank rewrite