search

cosmos / cosmos-sdk

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

[Epic]: Bank/v2 #17579

Open tac0turtle opened 1 year ago

tac0turtle commented 1 year ago

Summary

The current bank module is heavily overloaded with jobs it needs to maintain. To name a few jobs it has: handle sends, handle account restrictions, handle delegation counting, minting and burning of coins.

Some of the jobs are fine and are part of a bank and some are not. We should write a new bank module that is extendable via hooks and reduce the size of the bank module significantly.

Secondly, we should strive to make bank sends as fast as possible with minimal amount of gas needed. Through this, we can easily define the execution model of bank within other VMs for provability. If we can write bank in a way that compiles simply to vms this would make it even better.

Problem Definition

Bank is too verbose and handles too many things, we should strive to reduce the scope of the module, make it efficient and performant.

Work Breakdown

ref related issues:

https://github.com/cosmos/cosmos-sdk/issues/14453 https://github.com/cosmos/cosmos-sdk/issues/14701 https://github.com/cosmos/cosmos-sdk/issues/13212 https://github.com/cosmos/cosmos-sdk/issues/12026 https://github.com/cosmos/cosmos-sdk/issues/11388 https://github.com/cosmos/cosmos-sdk/issues/9619 https://github.com/cosmos/cosmos-sdk/issues/7113 https://github.com/cosmos/cosmos-sdk/issues/3689 https://github.com/cosmos/cosmos-sdk/issues/12404

alexanderbez commented 1 year ago

Honestly, most of those jobs you listed sounds like its right in the wheelhouse of "banking" minus a few of them which could be done via hooks or "extensions". Why write a new module instead of improving the existing one?

tac0turtle commented 1 year ago

Well to fix the current one I'd propose we delete 50-70% of it. At that point it's basically a rewrite. Happy to do it either way

tac0turtle commented 11 months ago

I propose splitting the bank module into two:

  1. The bank module as we know today handles sends between addresses. It holds information about all user funds, handles send restrictions and/or token restrictions. The api is reduced from the current design. Notably removing the burn functionality and multisend. All functions will be private and the way that other modules need to conduct msg sends is via a router. 

    service Msg {
option (cosmos.msg.v1.service) = true;

rpc Send(MsgSend) returns (MsgSendResponse);
rpc UpdateParams(MsgUpdateParams) returns (MsgUpdateParamsResponse);
rpc SetSendEnabled(MsgSetSendEnabled) returns (MsgSetSendEnabledResponse);
rpc ForceTransfer(MsgForceTransfer) returns (MsgForceTransferResponse);
}

  2. A new reserve module is created. This module acts similar to the token factory the ecosystem knows. The reserves module is responsible for minting, burning, denom metadata and keeping track of supply. The below api is copied from the tokenfactory module owned by osmosis.

We would need to coordinate on if current users of token factory would like migrate to this module or have two modules with similar functionality in their binary. 

service Msg {
  option (cosmos.msg.v1.service) = true;

  rpc CreateDenom(MsgCreateDenom) returns (MsgCreateDenomResponse);
  rpc Mint(MsgMint) returns (MsgMintResponse);
  rpc Burn(MsgBurn) returns (MsgBurnResponse);
  rpc ChangeAdmin(MsgChangeAdmin) returns (MsgChangeAdminResponse);
  rpc SetDenomMetadata(MsgSetDenomMetadata)
      returns (MsgSetDenomMetadataResponse);
}

A key motivation for the this is to allow anyone to create tokens on a chain, we should require a state rent for creating a token. The "rent" should be high enough to deter users from spamming many denoms as this could be a dos factor.

Another item we should keep in mind is to enable liquid staking through this module. This means staking should be able to mint a token for the stake delegated to a validator if this feature is enabled.

Im not sure if we should support aggregation of tokens into one. Since staking will mint a unique token for every validator, in order to produce something like stATOM, there will be a need to aggregate the tokens. We can do this via a simple module that handles it all for the user if they decide to liquid stake.

alexanderbez commented 11 months ago

I'm in favor of splitting x/bank into two discrete domains of responsibility -- banking and reserve. I have a few questions though:

  1. Is state rent for denomination creation a single fixed fee or is rent charged over time to the denom creator? The former right?
  2. Could you expand upon token aggregation a bit more? What does it mean to aggregate tokens? You mean pool balances from multiple holders into a single place?

EDIT: I also feel like there's a case to be made for consolidating x/mint and x/reserve, as the reserve is responsible for minting and honestly could be responsible for housing actual monetary policy control.

tac0turtle commented 11 months ago
  1. Is state rent for denomination creation a single fixed fee or is rent charged over time to the denom creator? The former right?

it would be a certain amount of tokens in order to mint the tokens. In order to get the rent back you would need to burn all the tokens. Maybe this isnt needed, was thinking how to avoid spam

Could you expand upon token aggregation a bit more? What does it mean to aggregate tokens? You mean pool balances from multiple holders into a single place?

aggregate many tokens into one, this would be needed for liquid staking. Im not sure if we should include it in the reserve module though.

I also feel like there's a case to be made for consolidating x/mint and x/reserver, as the reserve is responsible for minting and honestly should be responsible for the actual inflation/mint logic as well.

most of the logic lives in reserve. The only thing we may want to consider is the overlapping of responsibilities. Reserve mints tokens to an account, but doesnt know when to print, while mint knows when mint

tac0turtle commented 8 months ago

After further thought we should avoid creating a new module and have everything under one module. We want to avoid the need for as many migrations as possible.

The main changes that will appear in bank/v2 is that the module will be slimmed down significantly. We will have a single api to move modules. Denoms will have much more granular defined permissions to allow for more complex usecases.

yihuang commented 8 months ago

Lower Level Bank API

Provide lower level bank API to do deduct and credit separately.

Use Cases

how the events should change is TBD

tac0turtle commented 8 months ago
  • EVM integration, evm calls add/sub balances directly to lower level statedb, we have to trust evm implementation for the numbers and blindly update the balances, right now we translate that to mint and burn which is not efficient because it has to go through a module account, an alternative solution it to provide MintToAccont/BurnFromAccount.

yes we want to avoid this separation, allow mint and burn for everyone, but this requires denom permissions. which is also part of the plan

fee collection, fee collection is more efficient if we first deduct from tx sender, and only credit to fee collector at once at the end of the block, it's also a requirement for parallel tx execution, but this also needs some lower level APIs from bank module.

im a fan of this, thank you for the suggestions