WIP: Subkeys Spec

[sunnya97]

Reopening of #3930

• Targeted PR against correct branch (see CONTRIBUTING.md)

• Linked to github-issue with discussion and accepted design OR link to spec that describes this work.

• Wrote tests

• Updated relevant documentation (docs/)

• Added a relevant changelog entry: clog add [section] [stanza] [message]

• rereviewed Files changed in the github PR explorer

---

For Admin Use:

• Added appropriate labels to PR (ex. wip, ready-for-review, docs)
• Reviewers Assigned
• Squashed all commits, uses message "Merge pull request #XYZ: [title]" (coding standards)

[codecov]

Codecov Report

Merging #4380 into master will increase coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #4380      +/-   ##
==========================================
+ Coverage   58.08%   58.09%   +0.01%     
==========================================
  Files         235      235              
  Lines       14881    14881              
==========================================
+ Hits         8643     8645       +2     
+ Misses       5608     5606       -2     
  Partials      630      630

[Hyung-bharvest]

1. Expand the idea of DailyFeeAllowance to DailySpendableAllowance(including fee/sending)

• It makes user able to generate everyday-use-casual-key with daily spendable limit amount. Then it is security-wise easier to control the private key with safer manner.

2. Allow multisig key to become a subkey(longer length of the key string)

• Decentralizing control authority of an address to set of users

3. Ability to modify(add/delete) the list of subkey by the MasterSubkey or OriginalPrivatekey

• Decentralized authority of an address should be able to be edited after generation of the address.
• Number of subkey cannot exceed N.

4. Option to generate a stateful address(address with subkeys) without OriginalPrivatekey

• Main purpose of this utility is to make a stateful address FULLY DECENTRALIZED without any ownership of single private key.
• Step1 : When creating a stateful address, user should generate new type of tx with MasterSubkey config
• Step2 : Blockchain receive the tx and then generate a new address by combining tx generator address and its sequence.(new address = hash[generator address + sequence of the account]). Then, there is no knowledgable original private key for the new address. It is the same method how ethereum smart contract address generated. It is reasonable approach since the stateful address is a kind of smart contract.
• Step3 : Blockchain return the generated new address to the stateful address generator. nobody knows the private key of the new stateful address. Only the MasterSubkey(which can be multisig) or subkeys of the address can control it.

5. Receive deposit when create the stateful address

• Purpose of deposit is to prevent spam generation so that we can protect stateful address from pruning.([Multisig] Upgrade multisig function to mitigate dynamic key-set of multisig #4352 (comment))
• Deposit amount should be sent when create the stateful address.(10atom for instance)
• Deposit amount decided by governance and recorded in genesis.json.
• Deposit amount can be refunded when the stateful address is destroyed.
• Codebase methodology of deposit can be used from the similar functionality of governance deposit.

6. Destroying stateful address

• MasterSubkey or OriginalPrivatekey can destroy the stateful address.
• When destroy, all balance including deposit should be sent to designated address from destroy tx.

@sunnya97 @zmanian

If Sunny agrees, B-Harvest wants to propose a governance proposal to spend 2k atom from community fund to develop above functions by B-Harvest, also with gaiacli commands.

[sunnya97]

Expand the idea of DailyFeeAllowance to DailySpendableAllowance

While I think a DailySpendableAllowance is really good idea, so that you can limit SubKeys to only spending a certain amount. However, I think it is a way more complex thing, because "spends" can happen in many different ways across different modules (bank sends, gov deposit, etc).

Nice thing about checking just DailyFeeAllowance is it is only checked in one spot in the ante handler, and that's it, doesn't affect any other module.

I think DailyFeeAllowances is a good starting spot. We can do the more general SpendAllowance in the future; I think it needs a more complex feature called "SubAccounts".

Allow multisig key to become a subkey

A multisig implements crypto.PubKey so it can already become a subkey according to the current spec.

Ability to modify(add/delete) the list of subkey by the MasterSubkey or OriginalPrivatekey

This is included in the current spec.

Number of subkey cannot exceed N.

Agreed, good idea. If N is too large, it could become a bottleneck to Amino.

Option to generate a stateful address(address with subkeys) without OriginalPrivatekey

In my personal opinion, I think it's better to start without the ability to delete the Master Key because of the additional complexity it adds around things like MinDeposit and just focus on plain SubKeys for now as an MVP feature that is in urgent need imo.

But if you think its worth doing the masterkey-less account concurrently with this, how about instead of having a new type of tx type to create a masterkey-less account, just have a Msg type that deletes the MasterKey. That way an account can be created as normal right now, but then the masterkey can just self destruct itself. This seems simpler and less radical of a change.

I want to kick the can down the road on how to do "smart contract account" generation. For example, maybe Ethereum's new CREATE2 opcode is a better design (for example, it allows for submarine sends which is cool).

If Sunny agrees, B-Harvest wants to propose a governance proposal to spend 2k atom from community fund to develop above functions by B-Harvest, also with gaiacli commands.

For the proposal, I think it might be good to do two parts to it, one to get agreement on the feature and spec design from the community, and the second one to actually request funds from the Community Pool for implementation. Sikka would love to collaborate with you on part 1 😁

For the implementation, there is already a pretty good implementation started here (https://github.com/Abdelrahmansameh/cosmos-sdk/) off of this current spec by some people at the Paris Blockchain Week hackathon last month. I suggest maybe taking a look at that and seeing if you can build off of it instead of starting from scratch.

Also, the gaiacli commands here would include the changes needed to the keybase to support signing using subkeys and whatnot, right?

[Hyung-bharvest]

However, I think is a more complex thing, because "spends" can happen in many different ways across different modules (bank sends, gov deposit, etc).

We are investigating how "complex" it is to check the limit on bank sends, gov deposit and fee(I don't think there exists any more than 3 ways currently). We will share the exact spot where we fit in the checking logic. We can discuss the complexity with those information.

It is definitely little bit more complex than just cotrolling fee limit. But we think controlling spendable amount has enormous use-cases. Especially we are thinking about auto-tx-generation(auto refill, auto crypto-exchange, periodical payment, etc) services by 3rd party. If the security risk is minimized with this feature, it is easy for users to give away their subkey(which can be eliminated anytime owner wants) to service providers for more convenient asset usage. We think the use-case is so huge that some degree of complexity is happily bareable.

We can do the more general SpendAllowance in the future; I think it needs a more complex feature called "SubAccounts"

Basically our suggestions are from traditional financial services. I cannot clearly find the definition of "SubAccounts" in the context of traditional financial services. Could you elaborate more on this? Possibly in the words of traditional financial services. If we think it is very necessary thing, then we want to consider the possible structure of it too.

How about instead of having a new type of tx type to do this, just have a Msg type that deletes the MasterKey.

I think deleting OriginalPrivatekey will make huge confusions on/off chain. We actually cannot delete it because creator already knew it. Therefore we should disable the OriginalPrivatekey, but it looks like quite complex and risky to deploy such thing. And I think the way how ethereum create smart-contract addresses makes good sense to me therefore we want to use that convention.

Also, in general, I think it's better to start without the ability to delete the Master Key because of the additional complexity it adds around things like MinDeposit and just focus on plain SubKeys for now as an MVP feature that is in urgent need imo.

We reviewed some codebase and, deploying Deposit is not very complex, adding only several more lines. We will share the methodology with pseudo-code to explain how we can deploy it. Our deployment will need to create one more authorization category(PermissionedRoute) called "auth", which has rights to create/change/destroy the authority configuration of a stateful address.

No OriginalPrivatekey from the first place is very crucial for making the stateful address into a FULLY DECENTRALIZED asset container. It is one of the main reason why we want to build it with this specific utility. It has a great use-case for many organization even including ICF because the members can be changed anytime. Also it is the basic utility to support the "pegzone" which is the opposite direction of bitcoin-pegzone in Cosmos Network. This kind of shared control of asset on Cosmos Hub will bring many use-cases on different kinds of decentralized asset authority control methodologies.

Also, the gaiacli commands here would include the changes needed to the keybase to support signing using subkeys and whatnot, right?

We currently think that from every transaction generated by subkeys, gaiacli needs new flag "--fromaddress" which define the origin stateful address(we call it mother address of subkey) because a subkey might have several different mother addresses.

We have another option to disable any key to work as subkey of more than one mother address. In this case, the blockchain can find the mother address of given subkey in the backend.

Those two options have pros and cons. We will describe one of these when we finally conclude the best method. Please feel free to give opinion on the options.

[mdyring]

If Sunny agrees, B-Harvest wants to propose a governance proposal to spend 2k atom from community fund to develop above functions by B-Harvest, also with gaiacli commands

Overall, it will make it easier to get through governance if functionality is split up into two components:

1. Extended permissioning scheme and
2. Deposits for stateful addresses (to remove the need for account pruning)

Would be great to solve both and I generally agree with your thinking, but other people might see it differently. Decoupling increases probability of success. :-)

1. Expand the idea of DailyFeeAllowance to DailySpendableAllowance(including fee/sending)

This makes a lot of sense, especially for lower security modes (such as mobile wallet for everyday payments). Would love to see this implemented, even if it adds some complexity.

I am guessing Sunnys idea of SubAccounts has to do with creating sub-accounts that has some daily credit limit towards a main account. Would also be a nice way to do it.. Not really sure which is ideal, AFAICS SubAccounts would not give protection against revealing main account balance anyway.

6. Destroying stateful address

• MasterSubkey or OriginalPrivatekey can destroy the stateful address.
• When destroy, all balance including deposit should be sent to designated address from destroy tx.

Why not allow any SubKey with proper permission (path)? Don't see why "MasterSubKey or OriginalPrivateKey" should be required, as long as the SubKey has the proper permission (which ultimately derives from the Master key).

We currently think that from every transaction generated by subkeys, gaiacli needs new flag "--fromaddress" which define the origin stateful address(we call it mother address of subkey) because a subkey might have several different mother addresses.

I think it makes more sense to reference the account itself (--account flag perhaps, although --account-number is already in use), introducing "mother address" seems confusing as the purpose is to identify the account? See #3863 (comment) for suggestion.

[Hyung-bharvest]

Why not allow any SubKey with proper permission (path)? Don't see why "MasterSubKey or OriginalPrivateKey" should be required, as long as the SubKey has the proper permission (which ultimately derives from the Master key).

You are right. We already removed MasterSubkey concept in our doc because it has no unique definition. So, all subkeys are just subkeys. But when user wants to create a stateful account with their existing normal address, the original privatekey will still work as a MasterKey(with full authority) of the newly upgraded stateful account. But it will not in the list of Subkey or any other struct.

I think it makes more sense to reference the account itself (--account flag perhaps, although --account-number is already in use), introducing "mother address" seems confusing as the purpose is to identify the account? See #3863 (comment) for suggestion.

Yes, "mother address" is just a nickname and I agree we should use something like "--account" as a flag name. Below is our gaiacli command draft. All namings are subject to be properly renamed.

[mdyring]

@dlguddus can you clarify the purpose of "authkey" above? Do you see that as a master key of some kind?

Also: Unless there is good reason, I don't think a new account type should be introduced just for this, as it greatly increases support burden for eternity. The permission scheme seems like an extension where existing accounts could trivially be migrated.

In my mind, below naming make better sense:

gaiacli tx account create --subkey <subkey which receives master permissions>
gaiacli tx account add-permission --account <account-id> --pubkey <pk> --permission <path> ...
gaiacli tx account remove-permission --account <account-id> --pubkey <pk> --permission <path> ... // Should specify which permission to remove
gaiacli tx account destroy --account <account-id> --refund-address <account-id>

[Hyung-bharvest]

Authkey was a bad naming. We will come up with better one in final doc. Also thanks for advice on naming. Looks much better. We will consider it.

For splitting Gov, yes it looks better that way. We will first propose subkey function without deposit and then we will raise another proposal for deposit.

[jaekwon]

DailyFeeUsed shouldn't be in SubkeyMetadata, which should remain static as meta-data should.

[Hyung-bharvest]

We will review this and consider your advice. Thanks!!

[dongsam]

type SubKeyMetadata struct {
  ParentAccount        Account
  SubKey               SubKey
}

type SubKey struct {
  PubKey                     sdk.AccPubKey
  PermissionedRoutes         []string
  DailyFeeAllowance          sdk.Coins
  DailyFeeUsed               sdk.Coins
  Revoked                    bool
}

type StatefulAccount struct {
  *BaseAccount
  SubKeys       []SubKeyMetadata
  Deposit       sdk.Coins
}

How about this structure?

[Hyung-bharvest]

For the proposal, I think it might be good to do two parts to it, one to get agreement on the feature and spec design from the community, and the second one to actually request funds from the Community Pool for implementation. Sikka would love to collaborate with you on part 1 😁

What if 1 passes and 2 rejected, then what is that mean? I think it should be in the same proposal because two are correlated each other.(2 is the condition of 1) Not sure what is the purpose of splitting those two?

[Hyung-bharvest]

In my mind, below naming make better sense:

gaiacli tx account create --subkey <subkey which receives master permissions>
gaiacli tx account add-permission --account <account-id> --pubkey <pk> --permission <path> ...
gaiacli tx account remove-permission --account <account-id> --pubkey <pk> --permission <path> ... // Should specify which permission to remove
gaiacli tx account destroy --account <account-id> --refund-address <account-id>

We are fixing the naming as @mdyring 's suggested.
Also added command to modify permission config.

[fedekunze]

I'm going to close this as I believe there's an independent working group that's going to tackle this and bc there haven't been any updates in almost 2 mo.

[devrandom]

I recommend that the existing multisig functionality, where the multisig address hash covers the individual keys (and weights, whenever they are implemented) be preserved. This allows a relying party to verify that a certain account is controlled by certain keys.

For example, a cold storage system can verify that assets are being sent to an address controlled by certain keys. Without this hash coverage (e.g. by allowing controlling keys to change), the cold storage system can no longer verify this without relying on an online system that has access to blockchain state.

Prior art: this functionality is currently provided by the Bitcoin scripting language and by the Ethereum CREATE2 contract creation primitive.