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

New Subkeys Proposal Spec by B-Harvest #4480

Closed
Hyung-bharvest opened this issue Jun 5, 2019 · 3 comments
Closed

New Subkeys Proposal Spec by B-Harvest #4480

Hyung-bharvest opened this issue Jun 5, 2019 · 3 comments

Comments

@Hyung-bharvest
Copy link
Contributor

Hyung-bharvest commented Jun 5, 2019

New Subkeys Proposal Spec by B-Harvest

Abstract

@sunnya97 created a great proposal spec(#4380) on SubKeys functionality. It allows users to create an account called SubKeyAccount which has multiple SubKey to sign a transaction generated from the account. Each SubKey can hold different list of permission among transaction type(PermissionedRoutes), and also hold maximum 24 hour transaction fee allowance amount(DailyFeeAllowance).

B-Harvest proposes an advanced version of SubKeys functionality with additional utilities and with some structural changes from the original proposal by @sunnya97. Additional utilities are below.

  • An option to creation of SubKeyAccount without original public/private keys
  • Expand the idea of DailyFeeAllowance to DailySpendableAllowance(daily maximum allowed spendable amount)
  • Required minimum deposit when users create a SubKeyAccount to prevent spam attacks
  • Limit the maximum number of SubKey for a SubKeyAccount to prevent spam attacks
  • Ability to modify the permission list of a SubKey

We expect above additional utilities will create good use-cases as below.

  • A SubKey for daily casual usage with daily maximum spendable amount, which will greatly reduce the security burden of users.
  • A completely/dynamically decentralized shared asset management scheme, which is possible via a SubKeyAccount generation without original public/private keys.
  • Minimum deposit and maximum number of SubKey will effectively prevent spam attacks by creating excessive number of SubKeyAccounts or SubKeys.

Construction of SubKeyAccount and SubKey

SubKeyAccount(a new Account type) stores basic account information(BaseAccount) with related SubKeys and Deposit amount when creation.

type BaseAccount struct {
    Address       sdk.AccAddress `json:"address"`
    Coins         sdk.Coins      `json:"coins"`
    PubKey        crypto.PubKey  `json:"public_key"`
    AccountNumber uint64         `json:"account_number"`
    Sequence      uint64         `json:"sequence"`
}

// ...

type SubKeyAccount struct {
    *BaseAccount
    SubKeys       []SubKey
    Deposit       sdk.Coins
}

SubKey type stores SubKeyNumber(index of a SubKey), PubKey, PermissionedRoutes(list of allowed transaction type), DailySpendableAllowance and DailySpent.

type SubKey struct {
    PubKey                     crypto.PubKey
    SubKeyNumber               uint64
    PermissionedRoutes         []string
    DailySpendableAllowance    sdk.Coins
    DailySpent                 sdk.Coins
}

Now, we want to present pseudo codes to create a SubKeyAccount. There exists two kinds of it, 1) "upgrading" plain Account to a SubKeyAccount and 2) "creating" new SubKeyAccount without original master pubkey/privatekey.

  • pseudo code of CreateSubKeyAccount(upgrading)
// CreateSubKeyAccount
func (ak AccountKeeper) MigrateSubKeyAccount(ctx sdk.Context, acc Account) Account {
    vacc, ok := acc.(VestingAccount)
        if ok {
            panic(sdk.ErrUnknownAddress(fmt.Sprintf("Vesting Account %s can't be SubKey Account", vacc.GetAddress())))
        }
        params := ak.GetParams(ctx)
        
        dailySpendableAllowance := acc.GetCoins()
        for i := range dailySpendableAllowance {
            dailySpendableAllowance[i].Amount = sdk.NewInt(int64(NoLimitAllowance)) // // -1, nolimit dailySpendableAllowance
        }
        
        masterSubKey := ak.NewSubKey(ctx, acc.GetPubKey(), RouterKeyList, dailySpendableAllowance)
        
        // Deduct MinDepositAmount
        deposit := sdk.Coins{sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(int64(params.MinDepositAmount)))}
        err := acc.SetCoins(acc.GetCoins().Sub(deposit))
        if err != nil {
            panic(err)
        }
        
        newSubKeyAcc := &SubKeyAccount{
            BaseAccount:     &BaseAccount{Address: acc.GetAddress()},
            Deposit:         deposit,
        }
        newSubKeyAcc.SubKeys = append(newSubKeyAcc.SubKeys, masterSubKey)
        ak.SetAccount(ctx, acc)
        
        return acc
}
  • pseudo code of CreateSubKeyAccountWithoutPrivateKey(creating without pubkey/privatekey)
// CreateSubKeyAccountWithoutPrivateKey
func (ak AccountKeeper) NewSubKeyAccount(ctx sdk.Context, acc Account, pubKey crypto.PubKey) Account {
    newSubKeyAccAddress := sdk.AccAddress(crypto.AddressHash([]byte(fmt.Sprintf("%s %d", acc.GetAddress(), acc.GetSequence()))))
    newBaseAcc := NewBaseAccountWithAddress(newSubKeyAccAddress)
    err := newBaseAcc.SetAccountNumber(ak.GetNextAccountNumber(ctx))
    if err != nil {
        panic(err)
    }
    params := ak.GetParams(ctx)
    
    dailySpendableAllowance := acc.GetCoins()
    for i := range dailySpendableAllowance {
        dailySpendableAllowance[i].Amount = sdk.NewInt(int64(NoLimitAllowance)) // // -1, nolimit dailySpendableAllowance
    }
    
    masterSubKey := ak.NewSubKey(ctx, pubKey, RouterKeyList, dailySpendableAllowance)
    
    // Deduct MinDepositAmount
    deposit := sdk.Coins{sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(int64(params.MinDepositAmount)))}
    err = acc.SetCoins(acc.GetCoins().Sub(deposit))
    if err != nil {
        panic(err)
    }
    
    newSubKeyAcc := &SubKeyAccount{
        BaseAccount:     &newBaseAcc,
        Deposit:         deposit,
    }
    newSubKeyAcc.SubKeys = append(newSubKeyAcc.SubKeys, masterSubKey)
    ak.SetAccount(ctx, acc)
    
    return newSubKeyAcc
}

Parameters for SubKeyAccount and SubKey

We will add several parameters to use it for additional utilities as below.

  • SubKeyLimit : Maximum number of SubKey for a SubKeyAccount
  • MinDeposit : Minimum required deposit to create and maintain a SubKeyAccount. Deposit will be vested until the SubKeyAccount is destroyed.
  • DefaultSubKeyLimit : Default value of SubKeyLimit
  • DefaultMinDepositAmount : Default value of MinDeposit
  • We will add auth in RouterKeyList to properly handle the authority to manage a SubKeyAccount
  • SubKeyNumber = NoSubKey = 0 when user signs a transaction from a SubKeyAccount with original master key
  • DailySpendableAllowance = NoLimitAllowance = -1 implies the SubKey has infinite daily spendable allowance limit
// x/auth/params.go
type SubKeyAccountParams struct {
    SubKeyLimit          uint64
    MinDeposit           sdk.Coins       
}

const (
    //...
    DefaultSubKeyLimit       uint64 = 30
    DefaultMinDepositAmount  uint64 = 1000000 // NewCoin(sdk.DefaultBondDenom, sdk.NewInt(DefaultMinDepositAmount))
)

// x/auth/keeper.go
var (
    // ...
    RouterKeyList = []string{"bank", "crisis", "distr", "gov", "slashing", "staking", "auth"}
)

// x/auth/account.go
const (
    NoSubKey = uint64(0)
    NoLimitAllowance = int64(-1)
)

Checking and Tracking DailySpendableAllowance

1) Checking DailySpendableAllowance

Cosmos-SDK uses SpendableCoins and subtractCoins to check whether the balance of the account is enough. When a user signs a tx with a SubKey, the SDK should additionally check whether given tx will spend more than allowed. The checking is possible by using SpendableCoins, substractCoins and additionally DailySpendableAllowance, DailySpent from the given SubKey information.

Below is describing a new context called WithSubKey, which is holding context information of SubKey when a transaction is signed by a SubKey. It will allow SDK to use DailySpendableAllowance and DailySpent from the new context.

func (c Context) WithSubKey(subKeyNumber uint64) Context {
    return c.withValue(contextKeySubKey, subKeyNumber) 
}

Also we need to add SubKeyNumber item in SpendableCoins to inform which pair of SubKeyAccount and SubKey is related to the given stdTx as below.

type Account interface {
    // ...
    
    // Calculates the amount of coins that can be sent to other accounts given
    // the current time.
    - SpendableCoins(blockTime time.Time) sdk.Coins
    + SpendableCoins(blockTime time.Time, subKeyNumber uint64) sdk.Coins
    
    // ...
}

Below explains how we add SubKey adjustment into the SpendableCoins function.

func (sa SubKeyAccount) SpendableCoins(blockTime time.Time, subKeyNumber uint64) sdk.Coins {
    var spendableCoins sdk.Coins
    
    for _, subKey:= range sa.SubKeys {
        if subKey.SubKeyNumber == subKeyNumber {
            saCoins := sa.GetCoins().Sub(sa.Deposit)  // exclude Deposited coins
            dailyRemainingCoins := subKey.DailySpendableAllowance.Sub(subKey.DailySpent)
    
            for _, coin := range saCoins {
                baseAmt := coin.Amount
    
                remainingAmt := dailyRemainingCoins.AmountOf(coin.Denom)
    
                min := sdk.MinInt(remainingAmt, baseAmt)
                spendableCoin := sdk.NewCoin(coin.Denom, min)
    
                if !spendableCoin.IsZero() {
                    spendableCoins = spendableCoins.Add(sdk.Coins{spendableCoin})
                }
            }
            return spendableCoins
        }
    }
    return spendableCoins
}

2) Tracking DailySpent

The definition of DailySpent is the accumulated amount spent by a SubKey of a SubKeyAccount in recent 24 hours window. Therefore, we need to track the 24 hours window accumulation.

type DailySpent struct {
    Address              sdk.AccAddress
    SubKeyNumber         uint64
    Spent                sdk.Coins
}

But now we need a way to decrease the DailySpent field once transactions are past the 24 hour window. To do this we us a time based iterator similar to the ones used in the governance and staking queues.

We start by defining a new struct called a DailySpent, which is what will be inserted into the Queue.

When a transaction using a SubKey has spent, we insert a new entry into this queue's store.

  • The key is sdk.FormatTimeBytes(ctx.BlockTime.Add(time.Day * 1)
  • The value is the Amino marshalled DailySpent

Finally in the EndBlocker we iterate over the all the DailySpent in the queue which are expired (more than a day old). We prune them from state, and deduct their Spent amount from the corresponding SubKey.

store := ctx.Store(spentQueueStoreKey)
iterator := sdk.Iterator(nil, sdk.FormatTimeBytes(ctx.BlockTime)) // pseudocode abstract this as a slice of DailySpent

for _, dailySpent := range iterator {
    sacc := accountKeeper.GetAccount(dailySpent.Address)
    for i := range sacc.SubKeys {
        if sacc.SubKeys[i].SubKeyNumber == dailySpent.SubKeyNumber {
            sacc.SubKeys[i].DailySpent.Sub(dailySpent.Spent)
            store.Delete(dailySpent)
          break
        }
    }
}

Signature and Verification

When users sign a transaction with a SubKey, we need several changes in SDK so that it can properly verify the signature.

First, we introduce a new function called SubKey which finds the SubKey object using PubKey of SubKey/SubKeyAccount.

func (sa SubKeyAccount) SubKey(subKeyPubKey crypto.PubKey) *SubKey {
    for _, subKey:= range sa.SubKeys {
        if subKey.PubKey == subKeyPubKey {
            return &subKey
        }
    }
    return nil
}

In NewAnteHandle, if account type is SubKeyAccount, check the pubkey of the given signature is included as a SubKey of the SubKeyAccount.

// ...
    
    stdSigs := stdTx.GetSignatures()
    // flag for checking only one subKey in one tx,
    // because ctx need one subKeyNumber for checking Allowance
    subKeyFlag := false
    for i := 0; i < len(stdSigs); i++ {
        // skip the fee payer, account is cached and fees were deducted already
        if i != 0 {
            signerAccs[i], res = GetSignerAcc(newCtx, ak, signerAddrs[i])
            if !res.IsOK() {
                return newCtx, res, true
            }
        }
        sacc, ok := signerAccs[i].(SubKeyAccount)
        if ok {
            if subKeyFlag {
                return newCtx, sdk.ErrInvalidPubKey("Allowed Only one subKey in one tx").Result(), true
            }
            subKey := sacc.SubKey(stdSigs[i].PubKey)
            if subKey == nil {
                return newCtx, sdk.ErrInvalidPubKey("Invalid subKey").Result(), true
            }
            subKeyFlag = true
            newCtx.WithSubKey(subKey.SubKeyNumber)
        }
        // check signature, return account with incremented nonce
        signBytes := GetSignBytes(newCtx.ChainID(), stdTx, signerAccs[i], isGenesis)
        signerAccs[i], res = processSig(newCtx, signerAccs[i], stdSigs[i], signBytes, simulate, params)
        if !res.IsOK() {
            return newCtx, res, true
        }
        ak.SetAccount(newCtx, signerAccs[i])
    }
    
    // TODO: tx tags (?)
    return newCtx, sdk.Result{GasWanted: stdTx.Fee.Gas}, false // continue...
    }
}

In processSig , when SubKeyNumber exists on ctx(when the signature is done by a SubKey of a SubKeyAccount), change the pubKey to SubKey's pubKey to properly verify the SubKey signature with VerifyBytes function.

func processSig(
    ctx sdk.Context, acc Account, sig StdSignature, signBytes []byte, simulate bool, params Params,
) (updatedAcc Account, res sdk.Result) {

    var pubKey crypto.PubKey
    if ctx.SubKeyNumber() != NoSubKey{ // already checked subKeyNumber in AnteHandler
        pubKey = sig.PubKey
    } else {
        pubKey, res := ProcessPubKey(acc, sig, simulate)
        if !res.IsOK() {
            return nil, res
        }
    
        err := acc.SetPubKey(pubKey)
        if err != nil {
            return nil, sdk.ErrInternal("setting PubKey on signer's account").Result()
        }
    }

// ...

    if !simulate && !pubKey.VerifyBytes(signBytes, sig.Signature) {
        return nil, sdk.ErrUnauthorized("signature verification failed; verify correct account sequence and chain-id").Result()
    }
    
    if err := acc.SetSequence(acc.GetSequence() + 1); err != nil {
        panic(err)
    }
    
    return acc, res
}

We need to add subKeyNumber to TxBuilder for offline gaiacli usage.

// TxBuilder implements a transaction context created in SDK modules.
type TxBuilder struct {
    txEncoder          sdk.TxEncoder
    keybase            crkeys.Keybase
    accountNumber      uint64
    sequence           uint64
+   subKeyNumber       uint64
    gas                uint64
    gasAdjustment      float64
    simulateAndExecute bool
    chainID            string
    memo               string
    fees               sdk.Coins
    gasPrices          sdk.DecCoins
}

Msgs

In order to create and destroy SubKeyAccount, we need three new Msg types.

  • CreateSubKeyAccount("upgrading")
type MsgCreateSubKeyAccount struct {
    CreatorAddress             sdk.AccAddress
}
  • CreateSubKeyAccountWithoutPrivateKey("creating")
type MsgCreateSubKeyAccountWithoutPrivateKey struct {
    CreatorAddress             sdk.AccAddress
    MasterPubKey               crypto.PubKey
}
  • DestroySubKeyAccount
type MsgDestroySubKeyAccount struct {
    SubKeyAccountAddress      sdk.AccAddress
    RefundAddress             sdk.AccAddress        
}

Also we need three new Msg to add/delete a SubKey and edit the permission setting of a SubKey.

  • AddSubKey
type MsgAddSubKey struct {
    SubKeyAccountAddress       sdk.AccAddress
    PubKey                     crypto.PubKey
    PermissionedRoutes         []string
    DailySpendableAllowance    sdk.Coins
}
  • DeleteSubKey
type MsgDeleteSubKey struct {
    SubKeyAccountAddress       sdk.AccAddress
    PubKey                     crypto.PubKey
}
  • EditSubKeyPermission
type MsgEditSubKeyPermission struct {
    SubKeyAccountAddress       sdk.AccAddress
    PubKey                     crypto.PubKey
    PermissionedRoutes         []string
    DailySpendableAllowance    sdk.Coins
}
@mdyring
Copy link

mdyring commented Jun 6, 2019

Great work on this @dlguddus. Following up from forum post.

I'll try to structure feedback:

  1. Can you explain the rationale for not using a single account representation?
    1.1. It seems BaseAccount could be extended with subkey functionality and a migration could be done (see 2.4. below).
    1.2. Complexity can be reduced by only supporting a single account representation, there seems little value in supporting both.

  2. I think a global Minimum Balance setting, instead of per-account Deposit is preferable:
    2.1. Naming: Deposit does not describe the intended use, Minimum Balance better describes the requirement.
    2.2. Transactions that leave account balance below this minimum will be rejected, with the notable exception of MsgDestroyAccount.
    2.3. Provides better user experience as they see actual available balance of their account, though obviously it needs to be destroyed to claim what is below Minimum Balance.
    2.4. Allows for smooth migration of existing accounts with an existing balance below the minimum. These migrated accounts are essentially frozen until they are destroyed or receive additional funds.
    2.5. A global setting makes (IMO) better sense to honor governance decision, otherwise we have inconsistent behavior for older accounts with different Minimum Balance.
    2.5. Less state keeping / storage overhead.

  3. Seems like the "master" PubKey can be removed from BaseAccount if a single account representation is used.

A single (unified) account representation incorporating the above changes could look like this:

type BaseAccount struct {
    Address       sdk.AccAddress `json:"address"`
    Coins         sdk.Coins      `json:"coins"`
    AccountNumber uint64         `json:"account_number"`
    Sequence      uint64         `json:"sequence"`
    SubKeys       []SubKey       `json:"subkeys"`
}

Hope this is useful feedback. I haven't reviewed everything yet, but I guess this is a good starting point. :-)

@Hyung-bharvest
Copy link
Contributor Author

B-Harvest closes this Subkey feature issue due to a similar feature being developed by gaian team, a winner of Berlin hackatom.

@mdyring
Copy link

mdyring commented Jun 21, 2019

Forum post with some thoughts on subkey process:
https://forum.cosmos.network/t/process-for-cosmos-sdk-changes/2345

@aaronc aaronc mentioned this issue Aug 20, 2020
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants