Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

EPIC: Accounts #14900

Closed
1 of 2 tasks
tac0turtle opened this issue Feb 3, 2023 · 10 comments
Closed
1 of 2 tasks

EPIC: Accounts #14900

tac0turtle opened this issue Feb 3, 2023 · 10 comments
Labels

Comments

@tac0turtle
Copy link
Member

tac0turtle commented Feb 3, 2023

Summary

This issue is meant to facilitate discussion, feature requests and proposals. There have been many discussions on the auth module and how it could be improved. The current design is 5+ years old and there have been countless learnings on the limitations of the current system and how the account module has evolved in the greater cosmos ecosystem.

ref #7091

Work Breakdown

TBD

cc @testinginprod

We would love to hear from others on designs you have in mind for auth and accounts.

@alexanderbez
Copy link
Contributor

alexanderbez commented Feb 3, 2023

With EPICs such as these, it's a good idea to explain the (1) rationale/why and (2) what are the high-level goals we're trying to solve for.

E.g. for auth, based on various discussions, it seems that we want to:

These are all the general ideas I can think of.

@testinginprod
Copy link
Contributor

testinginprod commented Feb 6, 2023

My general idea around auth would be to have it be divided in 2 different modules (3 if we count vesting).

The division would be authv2 and accounts.

authv2

Domain

  • asserts if a state transition coming from outside (the tx) should be executed.
  • given a tx returns which identity is trying to execute which message.
  • maintains only the authentication related state: eg pubkey, sequence; acc number/chain ID information IS NOT part of authv2 state.

Consequences

cosmos-sdk are abstracted in such a way that authentication can be handled by something else which is not elliptic curves cryptography.

This brings us to say that: cosmos-sdk chains share the same tx anatomy, represented by tx.TxRaw, but the ways for which a chain can interpret that information depends on auth.

  • signing belongs to auth and not to cosmos-sdk.
  • Tx, which is a way to interpret TxRaw belongs to auth too.
  • tx decoding belongs to auth, a TX needs to be decoded into []struct StateTransition { signer: Identity, transition: Msg }.

This distinction might allow us to plug in different accounts types (accounts as defined by the x/accounts module) with pub keys, for example a CosmWasm contract might be born as an externally owned account.

accounts

Domain

  • handles identities in the chain
  • exposes information regarding the identities (not authentication information).

features

  • accounts can have state in the same way as modules do.
  • they don't necessarily have authv2 credentials to back them.
  • intercept state transitions (Msg), and approve/reject them (whilst also being able to modify state during the state transition admission stage). Note: a state transition can be incepted by a TX, by a module, or another account.

x/accounts MUST NOT handle the relationship between different identities, authorization and capabilities should be handled by the identities themselves.

modules vs accounts

I'd like x/accounts to also bring cohesion between what accounts and modules are in the system, I am leaning on to thinking accounts and modules are just the same actor, they both incept state transitions in the chain and they differentiate from each other only at permission level and capabilities over the state machine.

I see modules as a super-set of accounts.

What they share:

  • both modules and accounts have an identity (represented through AccAddress).
  • modules and accounts send messages (considering modules are moving to adr-033)
  • modules and accounts hold state.
  • modules treat other modules and EOAs as the same entity:
    • staking allows a module to stake in the same way as it allows an externally owned account.
    • bank allows another module to send money, makes differentiation at permission level only eg: SendCoinsFromModule*

What they don't share:

  • A module has business logic that can be deployed only once (there cannot be multiple instances of bank living in the chain), an account has business logic that can be deployed multiple times (eg: BaseAccount).
  • An account does not work with begin/end block.

towards a more extendable and safer state transition execution model

This section justifies why we want accounts to be able to be made aware of state transitions whose subject is the account itself, we make distinction between subject and sender. In the context of a bank.MsgSend, the subject is bank.MsgSend.from_address but the sender might be staking itself, which is trying to move money on behalf of from_address.

There has been desire to enhance and extend limitations and features of accounts, the proposed implementations try to address the limitations and capabilities by extending module's functionality, when in reality they belong to the accounts themselves. For example: #14224 wants to allow developers to inject restrictions on how an account can spend its money, but is this bank's domain or is this the specific account's concern? (It would be bank's domain, most likely, only if the limitation is on the denom rather than the account)

What happens when tomorrow developers will want to impose limitations on governance votes, staking rewards, etc.

In my opinion these limitations should be handled by accounts themselves and not by modules.

ADR-033

Generally speaking, ADR-033 talks about how to safely allow modules to interact with other modules, this is mainly motivated for two reasons:

  • define clear boundaries between modules
  • ensure that modules using other modules cannot break certain invariances (EG: bank.SetBalance used to break supply invariance).

A problem which ADR-033 does not address, which x/accounts is trying to provide solution for in a COMPLEMENTARY WAY, is the fact that even when handling a state transition a module can act on the behalf of another entity, for example, in an ADR-033 world staking might send bank a MsgSend which moves the money on behalf of a user, but if this user holds an account that has some specific limitations on how money can be spent, then this limitation would be bypassed.

@aaronc
Copy link
Member

aaronc commented Feb 7, 2023

Thanks for writing this up @testinginprod. I'm having a bit of trouble understanding the distinction betweeen x/accounts and x/auth v2. I wonder if you could describe the state that is managed by these two modules and also the API that they expose in a bit more detail?


I can share what I've had in mind for auth v2 (or authn as it probably should be called to distinguish it from authz/authorization). Basically, authn would have two tables:

  • Account: a mapping from acc id -> address & credential/pubkey, with a unique index on address
  • AccountSeq: a mapping from acc id -> seq

I think the auto-generated uint64 acc id is somewhat useful as a short identifier for an account because new addresses are getting longer and take 32 bytes by default. Also, I'm proposing to separate Account and AccountSeq because seq is the only thing that gets updated frequently and it's a very small value compared to the address + pubkey so it's fewer storage bytes being rewritten.

As for credential/pubkey, this has been discussed in other places, but I think we can start with a credential abstraction which is a type that defines an address, and then pubkey extends credential by being able to verify signatures. Credentials that aren't pubkeys could be used to represent module owned addresses and used to authenticate ADR 033 intermodule messages. Credentials which also implement pubkey can verify transactions. In go this would be something like:

type Credential interface {
   Address() []byte
}

type PubKey interface
  Credential
  VerifySignature(msg, sig []byte) bool
}

@alexanderbez
Copy link
Contributor

Yeah @aaronc this is what I had in mind. I think separating out accounts is pretty straight forward and non-contentious. What we really need to explore is the capability layer.

@SpicyLemon
Copy link
Collaborator

I'd like to address this part as I feel it's missing a point:

There has been desire to enhance and extend limitations and features of accounts, the proposed implementations try to address the limitations and capabilities by extending module's functionality, when in reality they belong to the accounts themselves. For example: #14224 wants to allow developers to inject restrictions on how an account can spend its money, but is this bank's domain or is this the specific account's concern? (It would be bank's domain, most likely, only if the limitation is on the denom rather than the account)

What happens when tomorrow developers will want to impose limitations on governance votes, staking rewards, etc.

In my opinion these limitations should be handled by accounts themselves and not by modules.

The bank module is responsible for managing balances, including the transfer of funds from one account to another. The need to transfer funds is common both for users and for other modules, and is one of the core functionalities in a blockchain. What is needed is that the same set of restrictions are applied to a transfer whether it's coming directly from a Tx (e.g. MsgSend), or at the request of another module (e.g. SendCoins). These restrictions can come from multiple places, e.g. x/vesting, x/sanction, x/quarantine, and/or x/lien, etc.. Regardless of how a transfer is initiated, all those restrictions need to be applied every time. Additionally, each of those restrictions shouldn't have to care about the others.

Take a look at the troubles that the SendEnabled checking has caused in the past. There has been several instances of other modules/processes accidentally allowing the transfer of SendEnabled = false coins because the author didn't know they needed to make that extra check themselves before calling SendCoins. That's the type of thing I don't want to happen. The thing attempting the transfer shouldn't be required to know of all things that might possibly restrict a transfer.

As the creator/maintainer of a module, I need to be able to define a restriction that is applied every time funds are being transferred, regardless of the source of that request (e.g. MsgSend vs SendCoins). Furthermore, I MUST NOT be required to have knowledge of all other restrictions that might be needed.

As the creator/maintainer of a blockchain, I want the addition of those restrictions as simple as adding the module(s) to my app.


I guess I'm having trouble picturing how restrictions-as-accounts would work. A specific complex example would be useful. Something like this:

  • The blockchain I'm using has vesting, quarantine, and sanction capabilities.
  • I have an account that I can sign for that has some funds received from a friend.
  • That account also has two different vesting schedules.
  • I have opted into quarantine (i.e. transfers from untrusted sources to my account go into a general holding account instead).
  • I am not sanctioned (i.e. my account has not been frozen).

Questions:

  • What are all the accounts?
  • When my new x/bonanza module attempts to transfer funds, how exactly are the vesting, quarantine, and sanction restrictions (and any others I don't know about) applied?
  • When I add the x/liens module to my blockchain, how does this picture change?

@testinginprod
Copy link
Contributor

testinginprod commented Mar 15, 2023

Thank you @SpicyLemon for the insightful comment.

The bank module is responsible for managing balances, including the transfer of funds from one account to another.

Bank serves the sole purpose of being a bookkeeper. It keeps track of balance changes (with respect to denoms), and enforces some simple invariances: eg a balance cannot hold amounts smaller than zero.

What is needed is that the same set of restrictions are applied to a transfer whether it's coming directly from a Tx (e.g. MsgSend), or at the request of another module (e.g. SendCoins).

This is where the fallacies of the current execution model come in the transfer scope:

  1. The fact that multiple paths (MsgSend, SendCoins) yielding the same action.
  2. Modules can transfer money in an unbounded way. If we were working in a sound system, concretely, the account itself allows a module to transfer funds on its behalf (eg: account gives staking an allowance to spend money on its behalf to stake).

Regardless of how a transfer is initiated, all those restrictions need to be applied every time. Additionally, each of those restrictions shouldn't have to care about the others.

It will eventually need to: you cannot pile restrictions and be confident that some will not conflict with the other. There needs to be awareness of restrictions applied.

Take a look at the troubles that the SendEnabled checking has caused in the past.

But this is orthogonal to the accounts issue (unless you're saying that denoms should be implemented as accounts and implement an ERC20-esque interface), this is bank documentation issue?

I want the addition of those restrictions as simple as adding the module(s) to my app.

But who/what/when/why are you restricting? Will you be implementing a SendRestriction function that each time a transfer happens will run all the possible checks, for all the denoms (even the ones which are not meant to be restricted), and for all the modules requiring a restriction? and maybe also apply a state change there for each middleware in the stack?

I guess I'm having trouble picturing how restrictions-as-accounts would work

In the same way as Ethereum implements vesting (as smart contracts) without the need of modifying anything of the core.
You create an account which is not externally owned (which means no pub key attached to it), and an externally owned account can interact with it, it can ask the restricted-account to send-coins/stake.

Old model:

graph LR
A[MyExternallyOwnedAccount] -->|MsgDelegate| B[StakingModule]
Loading

Possible model:

graph LR
A[MyExternallyOwnedAccount] -->|ExecuteMsgDelegate| B[MyVestingAccount]
B -->|MsgDelegate| C[StakingModule]
Loading

When MyVestingAccount receives a request to stake from MyExternallyOwnedAccount it is going to check if there are restrictions by querying x/sanctions, and then send a MsgDelegate to staking. This is where you get sound composability.

OFC the proposed accounts model is fully backwards compatible and even if I am strongly against continuing to extend and implement things as we currently do (hooks/middlewares into modules) you'd still be free to implement things as you did. As it was mentioned in the call, the scope of work does not consider only vesting and limitations on transfer, we want to get more from accounts, like recoverability capabilities, group management of accounts, creating more extendable authorisation accounts, etc.

@tac0turtle
Copy link
Member Author

When accounts lands, we should disable vesting account creation in the current vesting module.

@tac0turtle tac0turtle changed the title EPIC: Auth EPIC: Accounts Apr 26, 2023
@tac0turtle tac0turtle mentioned this issue May 3, 2023
3 tasks
@robert-zaremba
Copy link
Collaborator

#12597 was closed in favor of this epic.
Let's add a checklist in this epic to keep it in the loop. Let me add it quickly

@robert-zaremba
Copy link
Collaborator

robert-zaremba commented May 8, 2023

2 years ago we were researching with @aaronc how to model authentication and addresses with respect to DIDs / IIDs, Group Accounts and Internal Accounts.
I was doing a presentation about it which discussed the Credential type (as @aaronc mentioned above), but also Addressable and generic Verification. We had an issue about it where, but I can't find it now. The key diagram is:
image

@github-project-automation github-project-automation bot moved this to 👀 To Do in Cosmos-SDK Nov 17, 2023
@tac0turtle tac0turtle moved this from 👀 To Do to ✍ In Progress in Cosmos-SDK Nov 17, 2023
@tac0turtle
Copy link
Member Author

closing in favour of #17786

@github-project-automation github-project-automation bot moved this from ✍ In Progress to 🥳 Done in Cosmos-SDK Nov 17, 2023
@tac0turtle tac0turtle removed this from Cosmos-SDK Nov 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants