From 34e8692ccebed0a65f89ae02436cf9db88998051 Mon Sep 17 00:00:00 2001
From: Colin Axner <colinaxner@berkeley.edu>
Date: Wed, 17 Jul 2019 10:43:51 -0700
Subject: [PATCH 01/19] update proposal to have interfaces

---
 docs/architecture/adr-003-subaccounts.md | 154 +++++++++++++++++++++++
 1 file changed, 154 insertions(+)
 create mode 100644 docs/architecture/adr-003-subaccounts.md

diff --git a/docs/architecture/adr-003-subaccounts.md b/docs/architecture/adr-003-subaccounts.md
new file mode 100644
index 000000000000..23773fe80965
--- /dev/null
+++ b/docs/architecture/adr-003-subaccounts.md
@@ -0,0 +1,154 @@
+# ADR 3: Subaccounts
+
+## Changelog
+
+## Context
+
+Currently module accounts must be declared upon supply keeper initialization. Furthermore, they don't allow for separation of fungible coins within an account.
+
+The account structure should be modified so a `ModuleAccount` can dynamically add accounts.
+
+## Decision
+
+Add the following interfaces and permissions into `x/auth`.
+
+MultiAccounts and SubAccounts could be subkey accounts by having subkey account implement the interface functions.
+
+### Implementation Changes
+
+Introduce two new interfaces:
+
+* `MultiAccount`
+* `SubAccount`
+
+MultiAccount implements the `Account` interface.
+It maintains a list of its subaccounts as well as a list of permissions defined upon initialization of supply keeper (permAddrs).
+`SetCoins` returns an error. This prevents MultiAccount address from having a balance.
+MultiAccount has no pubkey for ModuleAccounts. MultiAccount for non ModuleAccounts could perhaps be a subkey account. MultiAccount pubkey would be master pubkey.
+Upon initialization of a MultiAccounts, a limit can be set on the max number of sub accounts can be set. There should also be the option to set the max number of sub accounts as unbonded.
+MultiAccount Constructor returns a MultiAccount with no sub accounts.
+MultiAccount `GetCoins` returns sum of sub account balances.
+SubAccounts can only be appended. To invalidate an account we would add `SetAccountDisabled` which sets the `disabled` field to true. 
+A disabled account could allow for withdraws, but cannot recieve any coins.
+Passively tracks the sum of all account balances.
+
+```
+type MultiAccount interface {
+    // MultiAccount interface functions
+    CreateSubAccount(pubkey, address) int // returns id of subaccount
+    GetSubAccount(id int) SubAccount
+   
+    // account interface functions
+    GetAddress() sdk.AccAddress
+	SetAddress(sdk.AccAddress) error 
+
+	GetPubKey() crypto.PubKey 
+	SetPubKey(crypto.PubKey) error
+
+	GetAccountNumber() uint64
+	SetAccountNumber(uint64) error
+
+	GetSequence() uint64
+	SetSequence(uint64) error
+
+	GetCoins() sdk.Coins
+	SetCoins(sdk.Coins) error
+
+	SpendableCoins(blockTime time.Time) sdk.Coins
+
+	String() string
+
+        
+}
+```
+
+```
+type SubAccount interface {
+    // SubAccount interface functions    
+    SetAccountDisabled()
+    SetAccountEnabled()
+    
+    AddPermissions(perms ...string)
+    RemovePermissions(perms ...string)
+
+    GetPermissions() []string
+
+    // account interface functions
+    GetAddress() sdk.AccAddress
+	SetAddress(sdk.AccAddress) error 
+
+	GetPubKey() crypto.PubKey 
+	SetPubKey(crypto.PubKey) error
+
+	GetAccountNumber() uint64
+	SetAccountNumber(uint64) error
+
+	GetSequence() uint64
+	SetSequence(uint64) error
+
+	GetCoins() sdk.Coins
+	SetCoins(sdk.Coins) error
+
+	SpendableCoins(blockTime time.Time) sdk.Coins
+
+	String() string
+
+     
+}
+```
+
+// possible implementation of MultiAccount
+```
+type ModuleMultiAccount struct {
+    subaccs []SubAccount
+    permissions []string
+    maxNumSubAccs uint
+    coins sdk.Coins // passively track all sub account balances
+    disabled bool
+}
+```
+
+
+SubAccount implements the `Account` interface.
+SubAccount's address is the multi account address with the id appended
+SubAccount permissions must be a sub set of its multi account's permissions.
+
+// possible implementation of SubAccount
+```
+type SubAccount struct {
+    address sdk.AccAddress // MultiAccount (parent) address with index appended
+    pubkey
+    id uint // index of subaccount
+    permissions []string
+}
+```
+
+**Other changes**
+
+Add an invariant check for MultiAccount `GetCoins`, which iterates over all subaccs to see if the sum of the subacc balances equals the passive tracking which is returned in `GetCoins`
+
+Update BankKeepers SetCoins function to return an error instead of calling panic on the account's SetCoins error.
+
+## Status
+
+Proposed
+
+## Consequences
+
+### Positive
+
+* Accounts can now separate fungible coins
+* Accounts can distribute permissions to sub accounts.
+
+### Negative
+
+* Brings permissions into `x/auth`
+
+### Neutral
+
+* Adds a new Account types
+
+## References
+
+Issues: [4657] (https://github.com/cosmos/cosmos-sdk/issues/4657)
+

From 708cba6ef1df40e9a8289fe9c946034de3292245 Mon Sep 17 00:00:00 2001
From: Colin Axner <colinaxner@berkeley.edu>
Date: Wed, 17 Jul 2019 11:51:37 -0700
Subject: [PATCH 02/19] formatting fixes

---
 docs/architecture/adr-003-subaccounts.md | 29 +++++++++++-------------
 1 file changed, 13 insertions(+), 16 deletions(-)

diff --git a/docs/architecture/adr-003-subaccounts.md b/docs/architecture/adr-003-subaccounts.md
index 23773fe80965..b10c405f6bd9 100644
--- a/docs/architecture/adr-003-subaccounts.md
+++ b/docs/architecture/adr-003-subaccounts.md
@@ -21,18 +21,17 @@ Introduce two new interfaces:
 * `MultiAccount`
 * `SubAccount`
 
-MultiAccount implements the `Account` interface.
-It maintains a list of its subaccounts as well as a list of permissions defined upon initialization of supply keeper (permAddrs).
-`SetCoins` returns an error. This prevents MultiAccount address from having a balance.
-MultiAccount has no pubkey for ModuleAccounts. MultiAccount for non ModuleAccounts could perhaps be a subkey account. MultiAccount pubkey would be master pubkey.
+MultiAccount maintains a list of its subaccounts as well as a list of permissions defined upon initialization of supply keeper (permAddrs).
+MultiAccount has no pubkey for ModuleAccounts and for non ModuleAccounts MultiAccount could be a subkey account. In this case, MultiAccount pubkey would be master pubkey.
 Upon initialization of a MultiAccounts, a limit can be set on the max number of sub accounts can be set. There should also be the option to set the max number of sub accounts as unbonded.
 MultiAccount Constructor returns a MultiAccount with no sub accounts.
-MultiAccount `GetCoins` returns sum of sub account balances.
-SubAccounts can only be appended. To invalidate an account we would add `SetAccountDisabled` which sets the `disabled` field to true. 
-A disabled account could allow for withdraws, but cannot recieve any coins.
-Passively tracks the sum of all account balances.
+To invalidate an account we would add `SetAccountDisabled` which sets the `disabled` field to true. 
 
-```
+```go
+// Implements the `Account` interface. SetCoins returns an error to prevent MultiAccount address from having a balance.
+// GetCoins returns sum of sub account balances. SubAccounts can only be appended. 
+// A disabled account can do withdraws, but cannot recieve any coins.
+// Passively tracks the sum of all account balances.
 type MultiAccount interface {
     // MultiAccount interface functions
     CreateSubAccount(pubkey, address) int // returns id of subaccount
@@ -62,7 +61,9 @@ type MultiAccount interface {
 }
 ```
 
-```
+```go
+// Implements the `Account` interface. Address is the multi account address with the id appended.
+// Permissions must be a subset of its multi account's permissions.
 type SubAccount interface {
     // SubAccount interface functions    
     SetAccountDisabled()
@@ -97,8 +98,8 @@ type SubAccount interface {
 }
 ```
 
+```go
 // possible implementation of MultiAccount
-```
 type ModuleMultiAccount struct {
     subaccs []SubAccount
     permissions []string
@@ -109,12 +110,8 @@ type ModuleMultiAccount struct {
 ```
 
 
-SubAccount implements the `Account` interface.
-SubAccount's address is the multi account address with the id appended
-SubAccount permissions must be a sub set of its multi account's permissions.
-
+```go
 // possible implementation of SubAccount
-```
 type SubAccount struct {
     address sdk.AccAddress // MultiAccount (parent) address with index appended
     pubkey

From 7f92cafc8492940af1ae74a54202ccc49e216830 Mon Sep 17 00:00:00 2001
From: Colin Axner <colinaxner@berkeley.edu>
Date: Wed, 17 Jul 2019 11:56:10 -0700
Subject: [PATCH 03/19] small format fixes

---
 docs/architecture/adr-003-subaccounts.md | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/docs/architecture/adr-003-subaccounts.md b/docs/architecture/adr-003-subaccounts.md
index b10c405f6bd9..9aae03b57b94 100644
--- a/docs/architecture/adr-003-subaccounts.md
+++ b/docs/architecture/adr-003-subaccounts.md
@@ -22,13 +22,13 @@ Introduce two new interfaces:
 * `SubAccount`
 
 MultiAccount maintains a list of its subaccounts as well as a list of permissions defined upon initialization of supply keeper (permAddrs).
-MultiAccount has no pubkey for ModuleAccounts and for non ModuleAccounts MultiAccount could be a subkey account. In this case, MultiAccount pubkey would be master pubkey.
+MultiAccount has no pubkey for ModuleAccounts. MultiAccount could be a subkey account. In this case, MultiAccount pubkey would be master pubkey.
 Upon initialization of a MultiAccounts, a limit can be set on the max number of sub accounts can be set. There should also be the option to set the max number of sub accounts as unbonded.
-MultiAccount Constructor returns a MultiAccount with no sub accounts.
+MultiAccount constructor returns a MultiAccount with no sub accounts.
 To invalidate an account we would add `SetAccountDisabled` which sets the `disabled` field to true. 
 
 ```go
-// Implements the `Account` interface. SetCoins returns an error to prevent MultiAccount address from having a balance.
+// Implements the Account interface. SetCoins returns an error to prevent MultiAccount address from having a balance.
 // GetCoins returns sum of sub account balances. SubAccounts can only be appended. 
 // A disabled account can do withdraws, but cannot recieve any coins.
 // Passively tracks the sum of all account balances.
@@ -39,7 +39,7 @@ type MultiAccount interface {
    
     // account interface functions
     GetAddress() sdk.AccAddress
-	SetAddress(sdk.AccAddress) error 
+    SetAddress(sdk.AccAddress) error 
 
 	GetPubKey() crypto.PubKey 
 	SetPubKey(crypto.PubKey) error
@@ -62,7 +62,7 @@ type MultiAccount interface {
 ```
 
 ```go
-// Implements the `Account` interface. Address is the multi account address with the id appended.
+// Implements the Account interface. Address is the multi account address with the id appended.
 // Permissions must be a subset of its multi account's permissions.
 type SubAccount interface {
     // SubAccount interface functions    

From b59cc63232434f49c77f684e0941395c15c26771 Mon Sep 17 00:00:00 2001
From: Colin Axner <colinaxner@berkeley.edu>
Date: Wed, 17 Jul 2019 11:57:37 -0700
Subject: [PATCH 04/19] fix tabbing issue

---
 docs/architecture/adr-003-subaccounts.md | 42 ++++++++++++------------
 1 file changed, 21 insertions(+), 21 deletions(-)

diff --git a/docs/architecture/adr-003-subaccounts.md b/docs/architecture/adr-003-subaccounts.md
index 9aae03b57b94..a3cca3d0e044 100644
--- a/docs/architecture/adr-003-subaccounts.md
+++ b/docs/architecture/adr-003-subaccounts.md
@@ -41,21 +41,21 @@ type MultiAccount interface {
     GetAddress() sdk.AccAddress
     SetAddress(sdk.AccAddress) error 
 
-	GetPubKey() crypto.PubKey 
-	SetPubKey(crypto.PubKey) error
+    GetPubKey() crypto.PubKey 
+    SetPubKey(crypto.PubKey) error
 
-	GetAccountNumber() uint64
-	SetAccountNumber(uint64) error
+    GetAccountNumber() uint64
+    SetAccountNumber(uint64) error
 
-	GetSequence() uint64
-	SetSequence(uint64) error
+    GetSequence() uint64
+    SetSequence(uint64) error
 
-	GetCoins() sdk.Coins
-	SetCoins(sdk.Coins) error
+    GetCoins() sdk.Coins
+    SetCoins(sdk.Coins) error
 
-	SpendableCoins(blockTime time.Time) sdk.Coins
+    SpendableCoins(blockTime time.Time) sdk.Coins
 
-	String() string
+    String() string
 
         
 }
@@ -76,23 +76,23 @@ type SubAccount interface {
 
     // account interface functions
     GetAddress() sdk.AccAddress
-	SetAddress(sdk.AccAddress) error 
+    SetAddress(sdk.AccAddress) error 
 
-	GetPubKey() crypto.PubKey 
-	SetPubKey(crypto.PubKey) error
+    GetPubKey() crypto.PubKey 
+    SetPubKey(crypto.PubKey) error
 
-	GetAccountNumber() uint64
-	SetAccountNumber(uint64) error
+    GetAccountNumber() uint64
+    SetAccountNumber(uint64) error
 
-	GetSequence() uint64
-	SetSequence(uint64) error
+    GetSequence() uint64
+    SetSequence(uint64) error
 
-	GetCoins() sdk.Coins
-	SetCoins(sdk.Coins) error
+    GetCoins() sdk.Coins
+    SetCoins(sdk.Coins) error
 
-	SpendableCoins(blockTime time.Time) sdk.Coins
+    SpendableCoins(blockTime time.Time) sdk.Coins
 
-	String() string
+    String() string
 
      
 }

From 60edc1a4f14cbbfdca67fc40157e54f3c0e623a4 Mon Sep 17 00:00:00 2001
From: Colin Axner <colinaxner@berkeley.edu>
Date: Thu, 18 Jul 2019 11:45:54 -0700
Subject: [PATCH 05/19] update adr with no modifications of auth

---
 docs/architecture/adr-003-subaccounts.md | 131 ++++++-----------------
 1 file changed, 34 insertions(+), 97 deletions(-)

diff --git a/docs/architecture/adr-003-subaccounts.md b/docs/architecture/adr-003-subaccounts.md
index a3cca3d0e044..26d6dfead0aa 100644
--- a/docs/architecture/adr-003-subaccounts.md
+++ b/docs/architecture/adr-003-subaccounts.md
@@ -1,128 +1,67 @@
-# ADR 3: Subaccounts
+# ADR 3: Module SubAccounts
 
 ## Changelog
 
 ## Context
 
-Currently module accounts must be declared upon supply keeper initialization. Furthermore, they don't allow for separation of fungible coins within an account.
+Currently `ModuleAccount`s must be declared upon supply keeper initialization. In addition to this they don't allow for separation of fungible coins within an account.
 
-The account structure should be modified so a `ModuleAccount` can dynamically add accounts.
+New structs should be added so a `ModuleAccount` can dynamically add accounts.
 
 ## Decision
 
-Add the following interfaces and permissions into `x/auth`.
-
-MultiAccounts and SubAccounts could be subkey accounts by having subkey account implement the interface functions.
+Add the following structs into `x/supply`.
 
 ### Implementation Changes
 
-Introduce two new interfaces:
+Introduce two new structs:
 
-* `MultiAccount`
+* `ModuleMultiAccount`
 * `SubAccount`
 
-MultiAccount maintains a list of its subaccounts as well as a list of permissions defined upon initialization of supply keeper (permAddrs).
-MultiAccount has no pubkey for ModuleAccounts. MultiAccount could be a subkey account. In this case, MultiAccount pubkey would be master pubkey.
-Upon initialization of a MultiAccounts, a limit can be set on the max number of sub accounts can be set. There should also be the option to set the max number of sub accounts as unbonded.
-MultiAccount constructor returns a MultiAccount with no sub accounts.
-To invalidate an account we would add `SetAccountDisabled` which sets the `disabled` field to true. 
+`ModuleMultiAccount` maintains an array of `SubAccount` as well as a list of permissions defined upon initialization of supply keeper (permAddrs).
+It has no pubkey and no limit on the number of `SubAccount`s that can be created.
+Its constructor returns a `ModuleMultiAccount` with no `SubAccount`s.
+`MultiModuleAccount` will implement the Account interface.
 
 ```go
-// Implements the Account interface. SetCoins returns an error to prevent MultiAccount address from having a balance.
-// GetCoins returns sum of sub account balances. SubAccounts can only be appended. 
-// A disabled account can do withdraws, but cannot recieve any coins.
-// Passively tracks the sum of all account balances.
-type MultiAccount interface {
-    // MultiAccount interface functions
+// SetCoins will return an error to prevent ModuleMultiAccount address from having a balance.
+// SubAccounts can only be appended to the SubAccount array.
+// Passively tracks the sum of all SubAccount balances.
+type ModuleMultiAccount struct {
+    Subaccs []SubAccount
+    Permissions []string
+    Coins sdk.Coins // passively track all sub account balances
+
     CreateSubAccount(pubkey, address) int // returns id of subaccount
     GetSubAccount(id int) SubAccount
-   
-    // account interface functions
-    GetAddress() sdk.AccAddress
-    SetAddress(sdk.AccAddress) error 
-
-    GetPubKey() crypto.PubKey 
-    SetPubKey(crypto.PubKey) error
-
-    GetAccountNumber() uint64
-    SetAccountNumber(uint64) error
-
-    GetSequence() uint64
-    SetSequence(uint64) error
-
-    GetCoins() sdk.Coins
-    SetCoins(sdk.Coins) error
-
-    SpendableCoins(blockTime time.Time) sdk.Coins
-
-    String() string
-
-        
 }
 ```
 
+To invalidate a `SubAccount` the `ModuleMultiAccount` calls `SetAccountDisabled` for a `SubAccount`
+
 ```go
-// Implements the Account interface. Address is the multi account address with the id appended.
-// Permissions must be a subset of its multi account's permissions.
-type SubAccount interface {
-    // SubAccount interface functions    
+// Implements the Account interface. Address is the ModuleMultiAccount address with the id appended.
+// Permissions must be a subset of its ModuleMultiAccount permissions.
+// A disabled account can do withdraws, but cannot recieve any coins.
+type SubAccount struct {
+    Address sdk.AccAddress // MultiAccount (parent) address with index appended
+    ID uint // index of subaccount
+    Permissions []string
+    Disabled bool
+
     SetAccountDisabled()
-    SetAccountEnabled()
-    
+
     AddPermissions(perms ...string)
     RemovePermissions(perms ...string)
 
     GetPermissions() []string
-
-    // account interface functions
-    GetAddress() sdk.AccAddress
-    SetAddress(sdk.AccAddress) error 
-
-    GetPubKey() crypto.PubKey 
-    SetPubKey(crypto.PubKey) error
-
-    GetAccountNumber() uint64
-    SetAccountNumber(uint64) error
-
-    GetSequence() uint64
-    SetSequence(uint64) error
-
-    GetCoins() sdk.Coins
-    SetCoins(sdk.Coins) error
-
-    SpendableCoins(blockTime time.Time) sdk.Coins
-
-    String() string
-
-     
-}
-```
-
-```go
-// possible implementation of MultiAccount
-type ModuleMultiAccount struct {
-    subaccs []SubAccount
-    permissions []string
-    maxNumSubAccs uint
-    coins sdk.Coins // passively track all sub account balances
-    disabled bool
-}
-```
-
-
-```go
-// possible implementation of SubAccount
-type SubAccount struct {
-    address sdk.AccAddress // MultiAccount (parent) address with index appended
-    pubkey
-    id uint // index of subaccount
-    permissions []string
 }
 ```
 
 **Other changes**
 
-Add an invariant check for MultiAccount `GetCoins`, which iterates over all subaccs to see if the sum of the subacc balances equals the passive tracking which is returned in `GetCoins`
+Add an invariant check for MultiAccount `GetCoins`, which iterates over all `SubAccount`s to see if the sum of the `SubAccount` balances equals the passive tracking which is returned in `GetCoins`
 
 Update BankKeepers SetCoins function to return an error instead of calling panic on the account's SetCoins error.
 
@@ -134,13 +73,12 @@ Proposed
 
 ### Positive
 
-* Accounts can now separate fungible coins
-* Accounts can distribute permissions to sub accounts.
+* ModuleAccount can separate fungible coins.
+* ModuleAccount can dynamically add accounts.
+* ModuleAccount can distribute permissions to SubAccounts.
 
 ### Negative
 
-* Brings permissions into `x/auth`
-
 ### Neutral
 
 * Adds a new Account types
@@ -148,4 +86,3 @@ Proposed
 ## References
 
 Issues: [4657] (https://github.com/cosmos/cosmos-sdk/issues/4657)
-

From c12697711bf945cb5999bc23f90ebdf4707bfd24 Mon Sep 17 00:00:00 2001
From: Colin Axner <colinaxner@berkeley.edu>
Date: Fri, 19 Jul 2019 15:47:40 -0700
Subject: [PATCH 06/19] address pr comments

---
 docs/architecture/adr-003-subaccounts.md | 88 ++++++++++++++----------
 1 file changed, 50 insertions(+), 38 deletions(-)

diff --git a/docs/architecture/adr-003-subaccounts.md b/docs/architecture/adr-003-subaccounts.md
index 26d6dfead0aa..b03962c45807 100644
--- a/docs/architecture/adr-003-subaccounts.md
+++ b/docs/architecture/adr-003-subaccounts.md
@@ -4,66 +4,74 @@
 
 ## Context
 
-Currently `ModuleAccount`s must be declared upon supply keeper initialization. In addition to this they don't allow for separation of fungible coins within an account.
+Currently `ModuleAccount`s must be declared upon Supply Keeper initialization. In addition to this they don't allow for separation of fungible coins within an account.
 
-New structs should be added so a `ModuleAccount` can dynamically add accounts.
+We want to support the ability to define and manage sub-module-accounts.
 
 ## Decision
 
-Add the following structs into `x/supply`.
+We will use the type `ModuleMultiAccount` to manage sub-accounts of type `ModuleAccount`.
+A `ModuleMultiAccount` may have zero or more `ModuleAccount`s.
+Each sub-account will utilize the existing permissioned properties of `ModuleAccount`.
+`ModuleMultiAccount` and `ModuleAccount` have no pubkeys.
+There is no limit on the number of `ModuleAccount`s that a `ModuleMultiAccount` can have.
+A `ModuleMultiAccount` has no permissions since it cannot hold any coins.
+Its constructor returns a `ModuleMultiAccount` with no `ModuleAccount`s.
+A sub-account can only be removed from a `MultiModuleAccount` if its balance is zero.
 
 ### Implementation Changes
 
-Introduce two new structs:
+Introduce a new type into `x/supply`:
 
 * `ModuleMultiAccount`
-* `SubAccount`
-
-`ModuleMultiAccount` maintains an array of `SubAccount` as well as a list of permissions defined upon initialization of supply keeper (permAddrs).
-It has no pubkey and no limit on the number of `SubAccount`s that can be created.
-Its constructor returns a `ModuleMultiAccount` with no `SubAccount`s.
-`MultiModuleAccount` will implement the Account interface.
 
 ```go
+// Implements the Account interface.
 // SetCoins will return an error to prevent ModuleMultiAccount address from having a balance.
-// SubAccounts can only be appended to the SubAccount array.
-// Passively tracks the sum of all SubAccount balances.
+// ModuleAccounts are appended to the SubAccounts array.
+// Passively tracks the sum of all ModuleAccount balances.
 type ModuleMultiAccount struct {
-    Subaccs []SubAccount
-    Permissions []string
+    SubAccounts []ModuleAccount
     Coins sdk.Coins // passively track all sub account balances
 
-    CreateSubAccount(pubkey, address) int // returns id of subaccount
-    GetSubAccount(id int) SubAccount
+    CreateSubAccount(address sdk.AccAddress) int // returns account number of sub-account
+    GetSubAccount(subAccNumber int64) SubAccount
 }
 ```
 
-To invalidate a `SubAccount` the `ModuleMultiAccount` calls `SetAccountDisabled` for a `SubAccount`
-
+The `ModuleAccount` implementation will remain unchanged, but we will add the following constructor function:
 ```go
-// Implements the Account interface. Address is the ModuleMultiAccount address with the id appended.
-// Permissions must be a subset of its ModuleMultiAccount permissions.
-// A disabled account can do withdraws, but cannot recieve any coins.
-type SubAccount struct {
-    Address sdk.AccAddress // MultiAccount (parent) address with index appended
-    ID uint // index of subaccount
-    Permissions []string
-    Disabled bool
+// NewEmptyModuleSubAccount creates an sub-account ModuleAccount which has an address created from
+// the hash of the module's name with the sub-account number appended.
+func NewEmptyModuleSubAccount(name string, subAccNumber uint64, permissions ...string) sdk.AccAddress {
+    bz := make([]byte, 8)
+   	binary.LittleEndian.PutUint64(bz, subAccNumber)
+    moduleAddress := append(NewModuleAddress(name), bz...)
+	baseAcc := authtypes.NewBaseAccountWithAddress(moduleAddress)
+
+	if err := validatePermissions(permissions...); err != nil {
+		panic(err)
+	}
+
+	return &ModuleAccount{
+		BaseAccount: &baseAcc,
+		Name:        name,
+		Permissions: permissions,
+	} 
+}
+```
 
-    SetAccountDisabled()
+**Permissions**:
 
-    AddPermissions(perms ...string)
-    RemovePermissions(perms ...string)
+A `ModuleMultiAccount` has no permissions.
 
-    GetPermissions() []string
-}
-```
+Since `ModuleAccount`s that are sub-accounts have the same name as its parent `ModuleMultiAccount`, a sub-account should only be granted a subset of the permissions registered with the Supply Keeper under its name.
 
 **Other changes**
 
-Add an invariant check for MultiAccount `GetCoins`, which iterates over all `SubAccount`s to see if the sum of the `SubAccount` balances equals the passive tracking which is returned in `GetCoins`
+We will add an invariant check for the `ModuleMultiAccount` `GetCoins()` function, which will iterate over all SubAccounts to see if the sum of the `ModuleAccount` balances equal the passive tracking which is returned in `GetCoins()`
 
-Update BankKeepers SetCoins function to return an error instead of calling panic on the account's SetCoins error.
+Bank Keepers `SetCoins()` function will be updated to return an error instead of calling panic on the account's SetCoins error.
 
 ## Status
 
@@ -73,16 +81,20 @@ Proposed
 
 ### Positive
 
-* ModuleAccount can separate fungible coins.
-* ModuleAccount can dynamically add accounts.
-* ModuleAccount can distribute permissions to SubAccounts.
+* ModuleMultiAccount can separate fungible coins.
+* ModuleMultiAccount can dynamically add accounts.
+* ModuleMultiAccount can distribute permissions to sub-accounts.
 
 ### Negative
 
+* sub-accounts cannot be removed from `ModuleMultiAccount`
+
 ### Neutral
 
-* Adds a new Account types
+* Use `ModuleAccount` type as a sub-account for `ModuleMultiAccount`
+* Adds a new Account type
 
 ## References
 
+Spec: [ModuleAccount](https://github.com/cosmos/cosmos-sdk/blob/master/docs/spec/supply/01_concepts.md#module-accounts)
 Issues: [4657] (https://github.com/cosmos/cosmos-sdk/issues/4657)

From c26f98ed2938d34dcb9a5ce2cd0aab8a3ba9a8e0 Mon Sep 17 00:00:00 2001
From: Colin Axner <colinaxner@berkeley.edu>
Date: Fri, 19 Jul 2019 15:52:11 -0700
Subject: [PATCH 07/19] fix markdown formatting and typos

---
 docs/architecture/adr-003-subaccounts.md | 27 ++++++++++++------------
 1 file changed, 14 insertions(+), 13 deletions(-)

diff --git a/docs/architecture/adr-003-subaccounts.md b/docs/architecture/adr-003-subaccounts.md
index b03962c45807..4e6f2d4ff1f8 100644
--- a/docs/architecture/adr-003-subaccounts.md
+++ b/docs/architecture/adr-003-subaccounts.md
@@ -17,7 +17,7 @@ Each sub-account will utilize the existing permissioned properties of `ModuleAcc
 There is no limit on the number of `ModuleAccount`s that a `ModuleMultiAccount` can have.
 A `ModuleMultiAccount` has no permissions since it cannot hold any coins.
 Its constructor returns a `ModuleMultiAccount` with no `ModuleAccount`s.
-A sub-account can only be removed from a `MultiModuleAccount` if its balance is zero.
+A sub-account cannot be removed from a `MultiModuleAccount`.
 
 ### Implementation Changes
 
@@ -45,19 +45,19 @@ The `ModuleAccount` implementation will remain unchanged, but we will add the fo
 // the hash of the module's name with the sub-account number appended.
 func NewEmptyModuleSubAccount(name string, subAccNumber uint64, permissions ...string) sdk.AccAddress {
     bz := make([]byte, 8)
-   	binary.LittleEndian.PutUint64(bz, subAccNumber)
+    binary.LittleEndian.PutUint64(bz, subAccNumber)
     moduleAddress := append(NewModuleAddress(name), bz...)
-	baseAcc := authtypes.NewBaseAccountWithAddress(moduleAddress)
+    baseAcc := authtypes.NewBaseAccountWithAddress(moduleAddress)
 
-	if err := validatePermissions(permissions...); err != nil {
-		panic(err)
-	}
+    if err := validatePermissions(permissions...); err != nil {
+        panic(err)
+    }
 
-	return &ModuleAccount{
-		BaseAccount: &baseAcc,
-		Name:        name,
-		Permissions: permissions,
-	} 
+    return &ModuleAccount{
+        BaseAccount: &baseAcc,
+        Name:        name,
+        Permissions: permissions,
+    } 
 }
 ```
 
@@ -69,7 +69,7 @@ Since `ModuleAccount`s that are sub-accounts have the same name as its parent `M
 
 **Other changes**
 
-We will add an invariant check for the `ModuleMultiAccount` `GetCoins()` function, which will iterate over all SubAccounts to see if the sum of the `ModuleAccount` balances equal the passive tracking which is returned in `GetCoins()`
+We will add an invariant check for the `ModuleMultiAccount` `GetCoins()` function, which will iterate over all SubAccounts to see if the sum of the `ModuleAccount` balances equals the passive tracking which is returned in `GetCoins()`
 
 Bank Keepers `SetCoins()` function will be updated to return an error instead of calling panic on the account's SetCoins error.
 
@@ -96,5 +96,6 @@ Proposed
 
 ## References
 
-Spec: [ModuleAccount](https://github.com/cosmos/cosmos-sdk/blob/master/docs/spec/supply/01_concepts.md#module-accounts)
+Specs: [ModuleAccount](https://github.com/cosmos/cosmos-sdk/blob/master/docs/spec/supply/01_concepts.md#module-accounts)
+
 Issues: [4657] (https://github.com/cosmos/cosmos-sdk/issues/4657)

From 96148f727c9d09593ddb30edd2b0d9bbb324abb4 Mon Sep 17 00:00:00 2001
From: Colin Axner <colinaxner@berkeley.edu>
Date: Fri, 19 Jul 2019 15:53:49 -0700
Subject: [PATCH 08/19] fix issue link

---
 docs/architecture/adr-003-subaccounts.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/architecture/adr-003-subaccounts.md b/docs/architecture/adr-003-subaccounts.md
index 4e6f2d4ff1f8..099878ebba1a 100644
--- a/docs/architecture/adr-003-subaccounts.md
+++ b/docs/architecture/adr-003-subaccounts.md
@@ -98,4 +98,4 @@ Proposed
 
 Specs: [ModuleAccount](https://github.com/cosmos/cosmos-sdk/blob/master/docs/spec/supply/01_concepts.md#module-accounts)
 
-Issues: [4657] (https://github.com/cosmos/cosmos-sdk/issues/4657)
+Issues: [4657](https://github.com/cosmos/cosmos-sdk/issues/4657)

From 44229b4e13ee16f356020ea6f5950a06fdf6be4f Mon Sep 17 00:00:00 2001
From: Federico Kunze <31522760+fedekunze@users.noreply.github.com>
Date: Mon, 22 Jul 2019 18:22:38 +0200
Subject: [PATCH 09/19] Update docs/architecture/adr-003-subaccounts.md

---
 docs/architecture/adr-003-subaccounts.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/architecture/adr-003-subaccounts.md b/docs/architecture/adr-003-subaccounts.md
index 099878ebba1a..2753955d658c 100644
--- a/docs/architecture/adr-003-subaccounts.md
+++ b/docs/architecture/adr-003-subaccounts.md
@@ -75,7 +75,7 @@ Bank Keepers `SetCoins()` function will be updated to return an error instead of
 
 ## Status
 
-Proposed
+Accepted
 
 ## Consequences
 

From 28fb6974e1dae99b1f86a20fc4d6d4b5c53a6e05 Mon Sep 17 00:00:00 2001
From: Colin Axner <colinaxner@berkeley.edu>
Date: Mon, 22 Jul 2019 11:24:32 -0700
Subject: [PATCH 10/19] add comment for auto incrementing acc number

---
 docs/architecture/adr-003-subaccounts.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/docs/architecture/adr-003-subaccounts.md b/docs/architecture/adr-003-subaccounts.md
index 099878ebba1a..bf21d44f567f 100644
--- a/docs/architecture/adr-003-subaccounts.md
+++ b/docs/architecture/adr-003-subaccounts.md
@@ -18,6 +18,7 @@ There is no limit on the number of `ModuleAccount`s that a `ModuleMultiAccount`
 A `ModuleMultiAccount` has no permissions since it cannot hold any coins.
 Its constructor returns a `ModuleMultiAccount` with no `ModuleAccount`s.
 A sub-account cannot be removed from a `MultiModuleAccount`.
+The account number assigned to sub-accounts will begin at 0 and be monotonically auto incrementing.
 
 ### Implementation Changes
 

From 6a6cfdd0152022623e180523f795f6290616c511 Mon Sep 17 00:00:00 2001
From: Colin Axner <colinaxner@berkeley.edu>
Date: Mon, 22 Jul 2019 12:07:17 -0700
Subject: [PATCH 11/19] update getsubaccount return value

---
 docs/architecture/adr-003-subaccounts.md | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/docs/architecture/adr-003-subaccounts.md b/docs/architecture/adr-003-subaccounts.md
index b1d2985a9e23..271f7b35b414 100644
--- a/docs/architecture/adr-003-subaccounts.md
+++ b/docs/architecture/adr-003-subaccounts.md
@@ -36,7 +36,7 @@ type ModuleMultiAccount struct {
     Coins sdk.Coins // passively track all sub account balances
 
     CreateSubAccount(address sdk.AccAddress) int // returns account number of sub-account
-    GetSubAccount(subAccNumber int64) SubAccount
+    GetSubAccount(subAccNumber int64) ModuleAccount
 }
 ```
 

From e434bb643f4c6fa8b03820c5e1364fce1880efda Mon Sep 17 00:00:00 2001
From: Colin Axner <colinaxner@berkeley.edu>
Date: Mon, 22 Jul 2019 12:15:24 -0700
Subject: [PATCH 12/19] small code fixes in adr

---
 docs/architecture/adr-003-subaccounts.md | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/docs/architecture/adr-003-subaccounts.md b/docs/architecture/adr-003-subaccounts.md
index 271f7b35b414..5ab900745203 100644
--- a/docs/architecture/adr-003-subaccounts.md
+++ b/docs/architecture/adr-003-subaccounts.md
@@ -35,16 +35,16 @@ type ModuleMultiAccount struct {
     SubAccounts []ModuleAccount
     Coins sdk.Coins // passively track all sub account balances
 
-    CreateSubAccount(address sdk.AccAddress) int // returns account number of sub-account
+    CreateSubAccount(name string, permissions ...string) int // returns account number of sub-account
     GetSubAccount(subAccNumber int64) ModuleAccount
 }
 ```
 
 The `ModuleAccount` implementation will remain unchanged, but we will add the following constructor function:
 ```go
-// NewEmptyModuleSubAccount creates an sub-account ModuleAccount which has an address created from
+// NewEmptyModuleSubAccount creates a sub-account ModuleAccount which has an address created from
 // the hash of the module's name with the sub-account number appended.
-func NewEmptyModuleSubAccount(name string, subAccNumber uint64, permissions ...string) sdk.AccAddress {
+func NewEmptyModuleSubAccount(name string, subAccNumber uint64, permissions ...string) ModuleAccount {
     bz := make([]byte, 8)
     binary.LittleEndian.PutUint64(bz, subAccNumber)
     moduleAddress := append(NewModuleAddress(name), bz...)

From abe610ff03d351799e75bd2a350d6ccf3c85b915 Mon Sep 17 00:00:00 2001
From: Colin Axner <colinaxner@berkeley.edu>
Date: Wed, 24 Jul 2019 10:51:33 -0700
Subject: [PATCH 13/19] add adr to TOC

---
 docs/architecture/README.md | 1 +
 1 file changed, 1 insertion(+)

diff --git a/docs/architecture/README.md b/docs/architecture/README.md
index 7c75babd58ba..cc910f52ec32 100644
--- a/docs/architecture/README.md
+++ b/docs/architecture/README.md
@@ -26,3 +26,4 @@ Please add a entry below in your Pull Request for an ADR.
 ### ADR Table of Contents
 
 - [ADR-002-Docs-Structure](./adr-002-docs-structure.md)
+- [ADR-003-Module-SubAccounts](./adr-003-subaccounts.md)

From 911ffc0cad5b5c574795b57c59af22e29428ab95 Mon Sep 17 00:00:00 2001
From: Colin Axner <colinaxner@berkeley.edu>
Date: Wed, 24 Jul 2019 11:27:58 -0700
Subject: [PATCH 14/19] reflect some pr comments

---
 docs/architecture/adr-003-subaccounts.md | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/docs/architecture/adr-003-subaccounts.md b/docs/architecture/adr-003-subaccounts.md
index 5ab900745203..747cbfb6f75b 100644
--- a/docs/architecture/adr-003-subaccounts.md
+++ b/docs/architecture/adr-003-subaccounts.md
@@ -35,8 +35,8 @@ type ModuleMultiAccount struct {
     SubAccounts []ModuleAccount
     Coins sdk.Coins // passively track all sub account balances
 
-    CreateSubAccount(name string, permissions ...string) int // returns account number of sub-account
-    GetSubAccount(subAccNumber int64) ModuleAccount
+    CreateSubAccount(name string, permissions ...string) (int, error) // returns account number of sub-account
+    GetSubAccount(subAccNumber int64) (ma ModuleAccount, found bool)
 }
 ```
 
@@ -64,7 +64,7 @@ func NewEmptyModuleSubAccount(name string, subAccNumber uint64, permissions ...s
 
 **Permissions**:
 
-A `ModuleMultiAccount` has no permissions.
+A `ModuleMultiAccount` has no permissions since it does not have any coins.
 
 Since `ModuleAccount`s that are sub-accounts have the same name as its parent `ModuleMultiAccount`, a sub-account should only be granted a subset of the permissions registered with the Supply Keeper under its name.
 
@@ -88,7 +88,7 @@ Accepted
 
 ### Negative
 
-* sub-accounts cannot be removed from `ModuleMultiAccount`
+* Sub-accounts cannot be removed from `ModuleMultiAccount`
 
 ### Neutral
 

From cc18af3ced34842edc728ff6c633ecb14e7e2183 Mon Sep 17 00:00:00 2001
From: Colin Axner <colinaxner@berkeley.edu>
Date: Fri, 26 Jul 2019 12:39:35 -0700
Subject: [PATCH 15/19] updated adr to reflect discussed ideas

---
 docs/architecture/adr-003-subaccounts.md | 108 ++++++++++++-----------
 1 file changed, 57 insertions(+), 51 deletions(-)

diff --git a/docs/architecture/adr-003-subaccounts.md b/docs/architecture/adr-003-subaccounts.md
index 747cbfb6f75b..420c49e0db7c 100644
--- a/docs/architecture/adr-003-subaccounts.md
+++ b/docs/architecture/adr-003-subaccounts.md
@@ -1,4 +1,4 @@
-# ADR 3: Module SubAccounts
+# ADR 3: Module Account Hierarchy
 
 ## Changelog
 
@@ -10,69 +10,75 @@ We want to support the ability to define and manage sub-module-accounts.
 
 ## Decision
 
-We will use the type `ModuleMultiAccount` to manage sub-accounts of type `ModuleAccount`.
-A `ModuleMultiAccount` may have zero or more `ModuleAccount`s.
-Each sub-account will utilize the existing permissioned properties of `ModuleAccount`.
-`ModuleMultiAccount` and `ModuleAccount` have no pubkeys.
-There is no limit on the number of `ModuleAccount`s that a `ModuleMultiAccount` can have.
-A `ModuleMultiAccount` has no permissions since it cannot hold any coins.
-Its constructor returns a `ModuleMultiAccount` with no `ModuleAccount`s.
-A sub-account cannot be removed from a `MultiModuleAccount`.
-The account number assigned to sub-accounts will begin at 0 and be monotonically auto incrementing.
+We will modify the existing `ModuleAccount` interface to support a heirarchical account structure.
+The `ModuleAccount`s defined upon initialization of Supply Keeper are the roots of family trees.
+Each `ModuleAccount` in a family tree may have zero or more children.
+A `ModuleAccount` with one or more children is considered a parent to each child.
+All `ModuleAccount`s have exactly one parent, unless they are the root of their family tree.
+`ModuleAccount` permissions will be renamed to `Attribute`.
+Each child's attributes must be a subset of their parent's attributes.
+There is no limit on the number of children a `ModuleAccount` can have.
+No `ModuleAccount`s can be removed from a family tree.
+A `ModuleAccount` name is the path of the `ModuleAccount` names used to reach the child.
+It starts with the root `ModuleAccount` name and is separated by a colon for each parent that follows until the child is reached.
+Example name: `root:parent:child`
+We will add a `TrackBalance` function which recursively updates the passive tracking of parent balances.
+A `ModuleAccount` address is the hash of its name.
+A `ModuleAccount` has no pubkeys.
+
 
 ### Implementation Changes
 
-Introduce a new type into `x/supply`:
+Modify `ModuleAccount` interface in `x/supply`:
 
-* `ModuleMultiAccount`
+```go
+type ModuleAccount interface {
+    GetName() string
+    GetAddress() string
+    GetAttribute() []string 
+    HasAttribute() bool
+    GetParent() string
+    GetChildren() []string
+    HasChild(string) bool 
+    GetChildCoins() sdk.Coins
+}
+```
 
 ```go
 // Implements the Account interface.
-// SetCoins will return an error to prevent ModuleMultiAccount address from having a balance.
-// ModuleAccounts are appended to the SubAccounts array.
-// Passively tracks the sum of all ModuleAccount balances.
-type ModuleMultiAccount struct {
-    SubAccounts []ModuleAccount
-    Coins sdk.Coins // passively track all sub account balances
-
-    CreateSubAccount(name string, permissions ...string) (int, error) // returns account number of sub-account
-    GetSubAccount(subAccNumber int64) (ma ModuleAccount, found bool)
+// ModuleAccount defines an account for modules that holds coins on a pool. A ModuleAccount
+// may have sub-accounts by having children.
+type ModuleAccount struct {
+	*authtypes.BaseAccount
+	Name        string    `json:"name" yaml:"name"`               // name of the module
+	Attributes  []string  `json:"attributes" yaml:"attributes"`   // permissions of module account
+    ChildCoins  sdk.Coins `json:"child_coins" yaml:"child_coins"` // passive tracking of sum of child balances
+    Children    []string  `json:"children" yaml:"children"`       // array of children names
+    Parent      string    `json:"parent" yaml:"parent"`           // parent name
 }
 ```
 
-The `ModuleAccount` implementation will remain unchanged, but we will add the following constructor function:
 ```go
-// NewEmptyModuleSubAccount creates a sub-account ModuleAccount which has an address created from
-// the hash of the module's name with the sub-account number appended.
-func NewEmptyModuleSubAccount(name string, subAccNumber uint64, permissions ...string) ModuleAccount {
-    bz := make([]byte, 8)
-    binary.LittleEndian.PutUint64(bz, subAccNumber)
-    moduleAddress := append(NewModuleAddress(name), bz...)
-    baseAcc := authtypes.NewBaseAccountWithAddress(moduleAddress)
-
-    if err := validatePermissions(permissions...); err != nil {
-        panic(err)
+// Pseudocode
+func TrackBalance(name string, delta sdk.Coins) {
+    if name == "" {
+        return
+    } else {
+        self.Balance += delta
     }
-
-    return &ModuleAccount{
-        BaseAccount: &baseAcc,
-        Name:        name,
-        Permissions: permissions,
-    } 
+    TrackBalance(chopString(name)) // chop off last name after last ":"
+    return
 }
 ```
 
-**Permissions**:
-
-A `ModuleMultiAccount` has no permissions since it does not have any coins.
+**Attributes**:
 
-Since `ModuleAccount`s that are sub-accounts have the same name as its parent `ModuleMultiAccount`, a sub-account should only be granted a subset of the permissions registered with the Supply Keeper under its name.
+Attributes for a root `ModuleAccount` are decalred upon initialization of the Supply Keeper.
+A child `ModuleAccount` must have a subset of its parents attributes.
 
 **Other changes**
 
-We will add an invariant check for the `ModuleMultiAccount` `GetCoins()` function, which will iterate over all SubAccounts to see if the sum of the `ModuleAccount` balances equals the passive tracking which is returned in `GetCoins()`
-
-Bank Keepers `SetCoins()` function will be updated to return an error instead of calling panic on the account's SetCoins error.
+We will add an invariant check for the `ModuleAccount` `GetCoins()` function, which will iterate over all `ModuleAccounts` to see if the sum of the `ModuleAccount` balances equals the passive tracking which is returned in `GetCoins()`
 
 ## Status
 
@@ -82,18 +88,18 @@ Accepted
 
 ### Positive
 
-* ModuleMultiAccount can separate fungible coins.
-* ModuleMultiAccount can dynamically add accounts.
-* ModuleMultiAccount can distribute permissions to sub-accounts.
+* ModuleAccount can separate fungible coins.
+* ModuleAccount can dynamically add accounts.
+* Children can have a subset of its parent's attributes.
 
 ### Negative
 
-* Sub-accounts cannot be removed from `ModuleMultiAccount`
+* `ModuleAccount` cannot be removed from a family tree.
+* `ModuleAccount` implementation has increased complexity.
 
 ### Neutral
 
-* Use `ModuleAccount` type as a sub-account for `ModuleMultiAccount`
-* Adds a new Account type
+* `ModuleAccount` passively tracks child balances.
 
 ## References
 

From e260a888aaf94b3aafb89b953d97e1b38a0558c8 Mon Sep 17 00:00:00 2001
From: Colin Axner <colinaxner@berkeley.edu>
Date: Fri, 26 Jul 2019 12:52:05 -0700
Subject: [PATCH 16/19] formatting

---
 docs/architecture/adr-003-subaccounts.md | 20 ++++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/docs/architecture/adr-003-subaccounts.md b/docs/architecture/adr-003-subaccounts.md
index 420c49e0db7c..e972a29d4586 100644
--- a/docs/architecture/adr-003-subaccounts.md
+++ b/docs/architecture/adr-003-subaccounts.md
@@ -15,17 +15,21 @@ The `ModuleAccount`s defined upon initialization of Supply Keeper are the roots
 Each `ModuleAccount` in a family tree may have zero or more children.
 A `ModuleAccount` with one or more children is considered a parent to each child.
 All `ModuleAccount`s have exactly one parent, unless they are the root of their family tree.
+
 `ModuleAccount` permissions will be renamed to `Attribute`.
 Each child's attributes must be a subset of their parent's attributes.
 There is no limit on the number of children a `ModuleAccount` can have.
 No `ModuleAccount`s can be removed from a family tree.
 A `ModuleAccount` name is the path of the `ModuleAccount` names used to reach the child.
 It starts with the root `ModuleAccount` name and is separated by a colon for each parent that follows until the child is reached.
+
 Example name: `root:parent:child`
+
 We will add a `TrackBalance` function which recursively updates the passive tracking of parent balances.
 A `ModuleAccount` address is the hash of its name.
 A `ModuleAccount` has no pubkeys.
-
+The function `AddChildToModuleAccount` will be added to Supply Keeper, 
+It will validate that the granted attributes are a subset of the parent and then register the child's name with the Supply Keeper.
 
 ### Implementation Changes
 
@@ -47,11 +51,11 @@ type ModuleAccount interface {
 ```go
 // Implements the Account interface.
 // ModuleAccount defines an account for modules that holds coins on a pool. A ModuleAccount
-// may have sub-accounts by having children.
+// may have sub-accounts known as children.
 type ModuleAccount struct {
-	*authtypes.BaseAccount
-	Name        string    `json:"name" yaml:"name"`               // name of the module
-	Attributes  []string  `json:"attributes" yaml:"attributes"`   // permissions of module account
+    *authtypes.BaseAccount
+    Name        string    `json:"name" yaml:"name"`               // name of the module
+    Attributes  []string  `json:"attributes" yaml:"attributes"`   // permissions of module account
     ChildCoins  sdk.Coins `json:"child_coins" yaml:"child_coins"` // passive tracking of sum of child balances
     Children    []string  `json:"children" yaml:"children"`       // array of children names
     Parent      string    `json:"parent" yaml:"parent"`           // parent name
@@ -78,7 +82,7 @@ A child `ModuleAccount` must have a subset of its parents attributes.
 
 **Other changes**
 
-We will add an invariant check for the `ModuleAccount` `GetCoins()` function, which will iterate over all `ModuleAccounts` to see if the sum of the `ModuleAccount` balances equals the passive tracking which is returned in `GetCoins()`
+We will add an invariant check for the `ModuleAccount` `GetChildCoins()` function, which will iterate over all `ModuleAccounts` to see if the sum of their child balances equals the passive tracking which is returned in `GetChildCoins()`
 
 ## Status
 
@@ -88,8 +92,8 @@ Accepted
 
 ### Positive
 
-* ModuleAccount can separate fungible coins.
-* ModuleAccount can dynamically add accounts.
+* `ModuleAccount` can separate fungible coins.
+* `ModuleAccount` can dynamically add accounts.
 * Children can have a subset of its parent's attributes.
 
 ### Negative

From 6a87e1610d4e7d101e2c211ce7756076321ce38d Mon Sep 17 00:00:00 2001
From: Colin Axner <colinaxner@berkeley.edu>
Date: Fri, 26 Jul 2019 13:02:42 -0700
Subject: [PATCH 17/19] update file naming

---
 docs/architecture/README.md                                     | 2 +-
 .../{adr-003-subaccounts.md => adr-003-module-sub-accounts.md}  | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)
 rename docs/architecture/{adr-003-subaccounts.md => adr-003-module-sub-accounts.md} (99%)

diff --git a/docs/architecture/README.md b/docs/architecture/README.md
index cc910f52ec32..ade04126cdf4 100644
--- a/docs/architecture/README.md
+++ b/docs/architecture/README.md
@@ -26,4 +26,4 @@ Please add a entry below in your Pull Request for an ADR.
 ### ADR Table of Contents
 
 - [ADR-002-Docs-Structure](./adr-002-docs-structure.md)
-- [ADR-003-Module-SubAccounts](./adr-003-subaccounts.md)
+- [ADR-003-Module-Sub-Accounts](./adr-003-module-sub-accounts.md)
diff --git a/docs/architecture/adr-003-subaccounts.md b/docs/architecture/adr-003-module-sub-accounts.md
similarity index 99%
rename from docs/architecture/adr-003-subaccounts.md
rename to docs/architecture/adr-003-module-sub-accounts.md
index e972a29d4586..38897cdf56f9 100644
--- a/docs/architecture/adr-003-subaccounts.md
+++ b/docs/architecture/adr-003-module-sub-accounts.md
@@ -1,4 +1,4 @@
-# ADR 3: Module Account Hierarchy
+# ADR 3: Module Sub-Accounts
 
 ## Changelog
 

From 9f9f14494f9dbb1afaa87c95e4662f30556046be Mon Sep 17 00:00:00 2001
From: Colin Axner <colinaxner@berkeley.edu>
Date: Mon, 29 Jul 2019 15:38:39 -0700
Subject: [PATCH 18/19] address @AdityaSripal comments

---
 docs/architecture/adr-003-module-sub-accounts.md | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/docs/architecture/adr-003-module-sub-accounts.md b/docs/architecture/adr-003-module-sub-accounts.md
index 38897cdf56f9..ebd5fbe8696d 100644
--- a/docs/architecture/adr-003-module-sub-accounts.md
+++ b/docs/architecture/adr-003-module-sub-accounts.md
@@ -20,13 +20,14 @@ All `ModuleAccount`s have exactly one parent, unless they are the root of their
 Each child's attributes must be a subset of their parent's attributes.
 There is no limit on the number of children a `ModuleAccount` can have.
 No `ModuleAccount`s can be removed from a family tree.
-A `ModuleAccount` name is the path of the `ModuleAccount` names used to reach the child.
+A `ModuleAccount` name must not contain ":".
+A `ModuleAccount` address is the hash of its full path.
+A `ModuleAccount`s full path is the path of the `ModuleAccount` names used to reach the child.
 It starts with the root `ModuleAccount` name and is separated by a colon for each parent that follows until the child is reached.
 
 Example name: `root:parent:child`
 
 We will add a `TrackBalance` function which recursively updates the passive tracking of parent balances.
-A `ModuleAccount` address is the hash of its name.
 A `ModuleAccount` has no pubkeys.
 The function `AddChildToModuleAccount` will be added to Supply Keeper, 
 It will validate that the granted attributes are a subset of the parent and then register the child's name with the Supply Keeper.
@@ -54,11 +55,11 @@ type ModuleAccount interface {
 // may have sub-accounts known as children.
 type ModuleAccount struct {
     *authtypes.BaseAccount
-    Name        string    `json:"name" yaml:"name"`               // name of the module
+    Name        string    `json:"name" yaml:"name"`               // name of the module without the full path
     Attributes  []string  `json:"attributes" yaml:"attributes"`   // permissions of module account
     ChildCoins  sdk.Coins `json:"child_coins" yaml:"child_coins"` // passive tracking of sum of child balances
     Children    []string  `json:"children" yaml:"children"`       // array of children names
-    Parent      string    `json:"parent" yaml:"parent"`           // parent name
+    Parent      string    `json:"parent" yaml:"parent"`           // full path of the parent name
 }
 ```
 
@@ -77,7 +78,7 @@ func TrackBalance(name string, delta sdk.Coins) {
 
 **Attributes**:
 
-Attributes for a root `ModuleAccount` are decalred upon initialization of the Supply Keeper.
+Attributes for a root `ModuleAccount` are declared upon initialization of the Supply Keeper.
 A child `ModuleAccount` must have a subset of its parents attributes.
 
 **Other changes**

From 3bd014ea1d2f87a5cb9031a2fe23c15c8c8c24e6 Mon Sep 17 00:00:00 2001
From: Colin Axner <colinaxner@berkeley.edu>
Date: Mon, 5 Aug 2019 10:59:48 -0700
Subject: [PATCH 19/19] update adr with problem example

---
 docs/architecture/adr-003-module-sub-accounts.md | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/docs/architecture/adr-003-module-sub-accounts.md b/docs/architecture/adr-003-module-sub-accounts.md
index ebd5fbe8696d..6da15dc04fdf 100644
--- a/docs/architecture/adr-003-module-sub-accounts.md
+++ b/docs/architecture/adr-003-module-sub-accounts.md
@@ -4,7 +4,10 @@
 
 ## Context
 
-Currently `ModuleAccount`s must be declared upon Supply Keeper initialization. In addition to this they don't allow for separation of fungible coins within an account.
+In the design of a simple decentralized exchange, it is likely that a `ModuleAccount` will be used to store the liquidity used for fulfilling trades. 
+In the case of the uniswap, trading pairs do not share liquidity pools. A reserve pool of atoms may only be allowed to be traded for denom A and not denom B. 
+This would require multiple accounts since the atom is fungible coin. It is likely that trading pairs will be added over time, but `ModuleAccount`s cannot be dynamically created.
+`ModuleAccount`s must be declared upon Supply Keeper initialization. In addition to this they don't allow for separation of fungible coins within an account.
 
 We want to support the ability to define and manage sub-module-accounts.