Merge PR #2853: Write bank module specification, check spec/code cons…

…istency * Update PENDING.md * New structure * Start transactions section * Remove MsgIssue * Update keepers.md * Add state.md * Update keepers.md, discovered #2887 * Move inputOutputCoins to BaseKeeper * Remove no-loner-applicable tests * More spec updates * Tiny cleanup * Clarify storage rationale * Warn the user * Remove extra newline
cosmos · Dec 4, 2018 · 8254c33 · 8254c33
1 parent ae4fccb
commit 8254c33
Show file tree

Hide file tree

Showing 9 changed files with 203 additions and 58 deletions.
diff --git a/PENDING.md b/PENDING.md
@@ -36,7 +36,8 @@ IMPROVEMENTS
 * Gaia
 
 * SDK
-
+ - \#1277 Complete bank module specification
+
 * Tendermint
 
 

diff --git a/docs/spec/bank/README.md b/docs/spec/bank/README.md
@@ -0,0 +1,26 @@
+# Bank module specification
+
+## Abstract
+
+This document specifies the bank module of the Cosmos SDK.
+
+The bank module is responsible for handling multi-asset coin transfers between
+accounts and tracking special-case pseudo-transfers which must work differently
+with particular kinds of accounts (notably delegating/undelegating for vesting
+accounts). It exposes several interfaces with varying capabilities for secure
+interaction with other modules which must alter user balances.
+
+This module will be used in the Cosmos Hub.
+
+## Contents
+
+1. **[State](state.md)**
+1. **[Keepers](keepers.md)**
+    1. [Common Types](keepers.md#common-types)
+        1. [Input](keepers.md#input)
+        1. [Output](keepers.md#output)
+    1. [BaseKeeper](keepers.md#basekeeper)
+    1. [SendKeeper](keepers.md#sendkeeper)
+    1. [ViewKeeper](keepers.md#viewkeeper) 
+1. **[Transactions](transactions.md)**
+    1. [MsgSend](transactions.md#msgsend)
diff --git a/docs/spec/bank/WIP_keeper.md b/docs/spec/bank/WIP_keeper.md
diff --git a/docs/spec/bank/keepers.md b/docs/spec/bank/keepers.md
@@ -0,0 +1,131 @@
+## Keepers
+
+The bank module provides three different exported keeper interfaces which can be passed to other modules which need to read or update account balances. Modules should use the least-permissive interface which provides the functionality they require.
+
+Note that you should always review the `bank` module code to ensure that permissions are limited in the way that you expect.
+
+### Common Types
+
+#### Input
+
+An input of a multiparty transfer
+
+```golang
+type Input struct {
+  Address AccAddress
+  Coins   Coins
+}
+```
+
+#### Output
+
+An output of a multiparty transfer.
+
+```golang
+type Output struct {
+  Address AccAddress
+  Coins   Coins
+}
+```
+
+### BaseKeeper
+
+The base keeper provides full-permission access: the ability to arbitrary modify any account's balance and mint or burn coins.
+
+```golang
+type BaseKeeper interface {
+  SetCoins(addr AccAddress, amt Coins)
+  SubtractCoins(addr AccAddress, amt Coins)
+  AddCoins(addr AccAddress, amt Coins)
+  InputOutputCoins(inputs []Input, outputs []Output)
+}
+```
+
+`setCoins` fetches an account by address, sets the coins on the account, and saves the account.
+
+```
+setCoins(addr AccAddress, amt Coins)
+  account = accountKeeper.getAccount(addr)
+  if account == nil
+    fail with "no account found"
+  account.Coins = amt
+  accountKeeper.setAccount(account)
+```
+
+`subtractCoins` fetches the coins of an account, subtracts the provided amount, and saves the account. This decreases the total supply.
+
+```
+subtractCoins(addr AccAddress, amt Coins)
+  oldCoins = getCoins(addr)
+  newCoins = oldCoins - amt
+  if newCoins < 0
+    fail with "cannot end up with negative coins"
+  setCoins(addr, newCoins)
+```
+
+`addCoins` fetches the coins of an account, adds the provided amount, and saves the account. This increases the total supply.
+
+```
+addCoins(addr AccAddress, amt Coins)
+  oldCoins = getCoins(addr)
+  newCoins = oldCoins + amt
+  setCoins(addr, newCoins)
+```
+
+`inputOutputCoins` transfers coins from any number of input accounts to any number of output accounts.
+
+```
+inputOutputCoins(inputs []Input, outputs []Output)
+  for input in inputs
+    subtractCoins(input.Address, input.Coins)
+  for output in outputs
+    addCoins(output.Address, output.Coins)
+```
+
+### SendKeeper
+
+The send keeper provides access to account balances and the ability to transfer coins between accounts, but not to alter the total supply (mint or burn coins).
+
+```golang
+type SendKeeper interface {
+  SendCoins(from AccAddress, to AccAddress, amt Coins)
+}
+```
+
+`sendCoins` transfers coins from one account to another.
+
+```
+sendCoins(from AccAddress, to AccAddress, amt Coins)
+  subtractCoins(from, amt)
+  addCoins(to, amt)
+```
+
+### ViewKeeper
+
+The view keeper provides read-only access to account balances but no balance alteration functionality. All balance lookups are `O(1)`.
+
+```golang
+type ViewKeeper interface {
+  GetCoins(addr AccAddress) Coins
+  HasCoins(addr AccAddress, amt Coins) bool
+}
+```
+
+`getCoins` returns the coins associated with an account.
+
+```
+getCoins(addr AccAddress)
+  account = accountKeeper.getAccount(addr)
+  if account == nil
+    return Coins{}
+  return account.Coins
+```
+
+`hasCoins` returns whether or not an account has at least the provided amount of coins.
+
+```
+hasCoins(addr AccAddress, amt Coins)
+  account = accountKeeper.getAccount(addr)
+  coins = getCoins(addr)
+  return coins >= amt 
+```
diff --git a/docs/spec/bank/state.md b/docs/spec/bank/state.md
@@ -0,0 +1,5 @@
+## State
+
+Presently, the bank module has no inherent state — it simply reads and writes accounts using the `AccountKeeper` from the `auth` module.
+
+This implementation choice is intended to minimize necessary state reads/writes, since we expect most transactions to involve coin amounts (for fees), so storing coin data in the account saves reading it separately.
diff --git a/docs/spec/bank/transactions.md b/docs/spec/bank/transactions.md
@@ -0,0 +1,26 @@
+## Transactions
+
+### MsgSend
+
+```golang
+type MsgSend struct {
+  Inputs  []Input
+  Outputs []Output
+}
+```
+
+`handleMsgSend` just runs `inputOutputCoins`.
+
+```
+handleMsgSend(msg MsgSend)
+  inputSum = 0
+  for input in inputs
+    inputSum += input.Amount
+  outputSum = 0
+  for output in outputs
+    outputSum += output.Amount
+  if inputSum != outputSum:
+    fail with "input/output amount mismatch"
+
+  return inputOutputCoins(msg.Inputs, msg.Outputs)
+```
diff --git a/x/bank/keeper.go b/x/bank/keeper.go
@@ -28,6 +28,7 @@ type Keeper interface {
 	SetCoins(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins) sdk.Error
 	SubtractCoins(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins) (sdk.Coins, sdk.Tags, sdk.Error)
 	AddCoins(ctx sdk.Context, addr sdk.AccAddress, amt sdk.Coins) (sdk.Coins, sdk.Tags, sdk.Error)
+	InputOutputCoins(ctx sdk.Context, inputs []Input, outputs []Output) (sdk.Tags, sdk.Error)
 }
 
 // BaseKeeper manages transfers between accounts. It implements the Keeper
@@ -67,6 +68,14 @@ func (keeper BaseKeeper) AddCoins(
 	return addCoins(ctx, keeper.ak, addr, amt)
 }
 
+// InputOutputCoins handles a list of inputs and outputs
+func (keeper BaseKeeper) InputOutputCoins(
+	ctx sdk.Context, inputs []Input, outputs []Output,
+) (sdk.Tags, sdk.Error) {
+
+	return inputOutputCoins(ctx, keeper.ak, inputs, outputs)
+}
+
 //-----------------------------------------------------------------------------
 // Send Keeper
 
@@ -76,7 +85,6 @@ type SendKeeper interface {
 	ViewKeeper
 
 	SendCoins(ctx sdk.Context, fromAddr sdk.AccAddress, toAddr sdk.AccAddress, amt sdk.Coins) (sdk.Tags, sdk.Error)
-	InputOutputCoins(ctx sdk.Context, inputs []Input, outputs []Output) (sdk.Tags, sdk.Error)
 }
 
 var _ SendKeeper = (*BaseSendKeeper)(nil)
@@ -105,14 +113,6 @@ func (keeper BaseSendKeeper) SendCoins(
 	return sendCoins(ctx, keeper.ak, fromAddr, toAddr, amt)
 }
 
-// InputOutputCoins handles a list of inputs and outputs
-func (keeper BaseSendKeeper) InputOutputCoins(
-	ctx sdk.Context, inputs []Input, outputs []Output,
-) (sdk.Tags, sdk.Error) {
-
-	return inputOutputCoins(ctx, keeper.ak, inputs, outputs)
-}
-
 //-----------------------------------------------------------------------------
 // View Keeper
 

diff --git a/x/bank/keeper_test.go b/x/bank/keeper_test.go
@@ -124,7 +124,6 @@ func TestSendKeeper(t *testing.T) {
 
 	addr := sdk.AccAddress([]byte("addr1"))
 	addr2 := sdk.AccAddress([]byte("addr2"))
-	addr3 := sdk.AccAddress([]byte("addr3"))
 	acc := accountKeeper.NewAccountWithAddress(ctx, addr)
 
 	// Test GetCoins/SetCoins
@@ -157,27 +156,6 @@ func TestSendKeeper(t *testing.T) {
 	require.True(t, sendKeeper.GetCoins(ctx, addr).IsEqual(sdk.Coins{sdk.NewInt64Coin("barcoin", 20), sdk.NewInt64Coin("foocoin", 5)}))
 	require.True(t, sendKeeper.GetCoins(ctx, addr2).IsEqual(sdk.Coins{sdk.NewInt64Coin("barcoin", 10), sdk.NewInt64Coin("foocoin", 10)}))
 
-	// Test InputOutputCoins
-	input1 := NewInput(addr2, sdk.Coins{sdk.NewInt64Coin("foocoin", 2)})
-	output1 := NewOutput(addr, sdk.Coins{sdk.NewInt64Coin("foocoin", 2)})
-	sendKeeper.InputOutputCoins(ctx, []Input{input1}, []Output{output1})
-	require.True(t, sendKeeper.GetCoins(ctx, addr).IsEqual(sdk.Coins{sdk.NewInt64Coin("barcoin", 20), sdk.NewInt64Coin("foocoin", 7)}))
-	require.True(t, sendKeeper.GetCoins(ctx, addr2).IsEqual(sdk.Coins{sdk.NewInt64Coin("barcoin", 10), sdk.NewInt64Coin("foocoin", 8)}))
-
-	inputs := []Input{
-		NewInput(addr, sdk.Coins{sdk.NewInt64Coin("foocoin", 3)}),
-		NewInput(addr2, sdk.Coins{sdk.NewInt64Coin("barcoin", 3), sdk.NewInt64Coin("foocoin", 2)}),
-	}
-
-	outputs := []Output{
-		NewOutput(addr, sdk.Coins{sdk.NewInt64Coin("barcoin", 1)}),
-		NewOutput(addr3, sdk.Coins{sdk.NewInt64Coin("barcoin", 2), sdk.NewInt64Coin("foocoin", 5)}),
-	}
-	sendKeeper.InputOutputCoins(ctx, inputs, outputs)
-	require.True(t, sendKeeper.GetCoins(ctx, addr).IsEqual(sdk.Coins{sdk.NewInt64Coin("barcoin", 21), sdk.NewInt64Coin("foocoin", 4)}))
-	require.True(t, sendKeeper.GetCoins(ctx, addr2).IsEqual(sdk.Coins{sdk.NewInt64Coin("barcoin", 7), sdk.NewInt64Coin("foocoin", 6)}))
-	require.True(t, sendKeeper.GetCoins(ctx, addr3).IsEqual(sdk.Coins{sdk.NewInt64Coin("barcoin", 2), sdk.NewInt64Coin("foocoin", 5)}))
-
 }
 
 func TestViewKeeper(t *testing.T) {

diff --git a/x/bank/msgs.go b/x/bank/msgs.go
@@ -6,6 +6,9 @@ import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
 )
 
+// name to identify transaction routes
+const MsgRoute = "bank"
+
 // MsgSend - high level transaction of the coin module
 type MsgSend struct {
 	Inputs  []Input  `json:"inputs"`
@@ -21,7 +24,7 @@ func NewMsgSend(in []Input, out []Output) MsgSend {
 
 // Implements Msg.
 // nolint
-func (msg MsgSend) Route() string { return "bank" } // TODO: "bank/send"
+func (msg MsgSend) Route() string { return MsgRoute }
 func (msg MsgSend) Type() string  { return "send" }
 
 // Implements Msg.