chore: simplify ADR-028 and address.Module

docs/architecture/adr-028-public-key-addresses.md

@@ -97,7 +97,7 @@ As in other parts of the Cosmos SDK, we will use `sha256`.
 
 ### Basic Address
 
-We start with defining a base hash algorithm for generating addresses. Notably, it's used for accounts represented by a single key pair. For each public key schema we have to have an associated `typ` string, which we discuss in a section below. `hash` is the cryptographic hash function defined in the previous section.
+We start with defining a base algorithm for generating addresses which we will call `Hash`. Notably, it's used for accounts represented by a single key pair. For each public key schema we have to have an associated `typ` string, explained in the next section. `hash` is the cryptographic hash function defined in the previous section.
 
 ```go
 const A_LEN = 32
@@ -114,16 +114,13 @@ Motivation: this algorithm keeps the address relatively small (length of the `ty
 and it's more secure than [post-hash-prefix-proposal] (which uses the first 20 bytes of a pubkey hash, significantly reducing the address space).
 Moreover the cryptographer motivated the choice of adding `typ` in the hash to protect against a switch table attack.
 
-We use the `address.Hash` function for generating addresses for all accounts represented by a single key:
+`address.Hash` is a low level function to generate _base_ addresses for a new key types. Example:
 
-* simple public keys: `address.Hash(keyType, pubkey)`
-
-* aggregated keys (eg: BLS): `address.Hash(keyType, aggregatedPubKey)`
-* modules: `address.Hash("module", moduleName)`
+* BLS: `address.Hash("bls", pubkey)`
 
 ### Composed Addresses
 
-For simple composed accounts (like new naive multisig), we generalize the `address.Hash`. The address is constructed by recursively creating addresses for the sub accounts, sorting the addresses and composing them into a single address. It ensures that the ordering of keys doesn't impact the resulting address.
+For simple composed accounts (like a new naive multisig) we generalize the `address.Hash`. The address is constructed by recursively creating addresses for the sub accounts, sorting the addresses and composing them into a single address. It ensures that the ordering of keys doesn't impact the resulting address.
 
 ```go
 // We don't need a PubKey interface - we need anything which is addressable.
@@ -140,13 +137,13 @@ func Composed(typ string, subaccounts []Addressable) []byte {
 
 The `typ` parameter should be a schema descriptor, containing all significant attributes with deterministic serialization (eg: utf8 string).
 `LengthPrefix` is a function which prepends 1 byte to the address. The value of that byte is the length of the address bits before prepending. The address must be at most 255 bits long.
-We are using `LengthPrefix` to eliminate conflicts - it assures, that for 2 lists of addresses: `as = {a1, a2, ..., an}` and `bs = {b1, b2, ..., bm}` such that every `bi` and `ai` is at most 255 long, `concatenate(map(as, \a -> LengthPrefix(a))) = map(bs, \b -> LengthPrefix(b))` iff `as = bs`.
+We are using `LengthPrefix` to eliminate conflicts - it assures, that for 2 lists of addresses: `as = {a1, a2, ..., an}` and `bs = {b1, b2, ..., bm}` such that every `bi` and `ai` is at most 255 long, `concatenate(map(as, (a) => LengthPrefix(a))) = map(bs, (b) => LengthPrefix(b))` iff `as = bs`.
 
 Implementation Tip: account implementations should cache addresses.
 
 #### Multisig Addresses
 
-For new multisig public keys, we define the `typ` parameter not based on any encoding scheme (amino or protobuf). This avoids issues with non-determinism in the encoding scheme.
+For a new multisig public keys, we define the `typ` parameter not based on any encoding scheme (amino or protobuf). This avoids issues with non-determinism in the encoding scheme.
 
 Example:
 
@@ -175,53 +172,61 @@ func (multisig PubKey) Address() {
 }
 ```
 
-#### Module Account Addresses
 
-NOTE: this section is not finalize and it's in active discussion.
+### Derived Addresses
 
-In Basic Address section we defined a module account address as:
+We must be able to cryptographically derive one address from another one. The derivation process must guarantee hash properties, hence we use the already defined `Hash` function:
 
 ```go
-address.Hash("module", moduleName)
+func Derive(address []byte, derivationKey []byte) []byte {
+    return Hash(addres, derivationKey)
+}
 ```
 
-We use `"module"` as a schema type for all module derived addresses. Module accounts can have sub accounts. The derivation process has a defined order: module name, submodule key, subsubmodule key.
-Module account addresses are heavily used in the Cosmos SDK so it makes sense to optimize the derivation process: instead of using of using `LengthPrefix` for the module name, we use a null byte (`'\x00'`) as a separator. This works, because null byte is not a part of a valid module name.
+### Module Account Addresses
+
+A module account will have `"module"` type. Module accounts can have sub accounts. The submodule account will be created based on module name, and sequence of derivation keys. Typically, the first derivation key should be a class of the derived accounts. The derivation process has a defined order: module name, submodule key, subsubmodule key... An example module account is created using:
 
 ```go
-func Module(moduleName string, key []byte) []byte{
-	return Hash("module", []byte(moduleName) + 0 + key)
-}
+address.Module(moduleName, key)
 ```
 
-**Example**  A lending BTC pool address would be:
+An example sub-module account is created using:
 
 ```go
-btcPool := address.Module("lending", btc.Address()})
+groupPolicyAddresses := []byte{1}
+address.Module (moduleName, groupPolicyAddresses, policyID)
 ```
 
-If we want to create an address for a module account depending on more than one key, we can concatenate them:
+The `address.Module` function is using `addres.Hash` with `"module"` as the type argument, and byte representation of the module name concatenated with submodule key. The two last component must be uniquely separated to avoid potential clashes (example: modulename="ab" & submodulekey="bc" will have the same derivation key as modulename="a" & submodulekey="bbc").
+We use a null byte (`'\x00'`) to separate module name from the submodule key. This works, because null byte is not a part of a valid module name. Finally, the sub-submodule accounts are created by applying the `Derive` function recursively.
+We could use `Derive` function also in the first step (rather than concatenating module name with zero byte and the submodule key). We decided to do concatenation to avoid one level of derivation and speed up computation.
 
 ```go
-btcAtomAMM := address.Module("amm", btc.Address() + atom.Address()})
+func Module(moduleName string, key []byte, subsubKeys ...[]byte) []byte{
+	submoduleAddress := Hash("module", []byte(moduleName) + 0 + key)
+  return fold((a, k) => Derive(a, k), subsubKeys, submoduleAddress)
+}
 ```
 
-#### Derived Addresses
-
-We must be able to cryptographically derive one address from another one. The derivation process must guarantee hash properties, hence we use the already defined `Hash` function:
+**Example 1**  A lending BTC pool address would be:
 
 ```go
-func Derive(address []byte, derivationKey []byte) []byte {
-    return Hash(addres, derivationKey)
-}
+btcPool := address.Module("lending", btc.Address()})
 ```
 
-Note: `Module` is a special case of the more general _derived_ address, where we set the `"module"` string for the _from address_.
+If we want to create an address for a module account depending on more than one key, we can concatenate them:
+
+```go
+btcAtomAMM := address.Module("amm", btc.Address() + atom.Address()})
+```
 
-**Example**  For a cosmwasm smart-contract address we could use the following construction:
+**Example 2**  cosmwasm smart-contract address could be constructed by:
 
 ```go
-smartContractAddr := Derived(Module("cosmwasm", smartContractsNamespace), []{smartContractKey})
+smartContractAddr := Module("cosmwasm", smartContractsNamespace, smartContractKey})
+// wich equals to:
+    Derived(Module("cosmwasm", smartContractsNamespace), []{smartContractKey})
 ```
 
 ### Schema Types

types/address/hash.go

@@ -18,7 +18,9 @@ type Addressable interface {
 	Address() []byte
 }
 
-// Hash creates a new address from address type and key
+// Hash creates a new address from address type and key.
+// The functions should only be used by new types defining thier own address function
+// (eg public keys).
 func Hash(typ string, key []byte) []byte {
 	hasher := sha256.New()
 	_, err := hasher.Write(conv.UnsafeStrToBytes(typ))
@@ -59,10 +61,15 @@ func Compose(typ string, subAddresses []Addressable) ([]byte, error) {
 }
 
 // Module is a specialized version of a composed address for modules. Each module account
-// is constructed from a module name and module account key.
-func Module(moduleName string, key []byte) []byte {
+// is constructed from a module name and a sequence of derivation keys (at least one
+// derivation key must be provided).
+func Module(moduleName string, derivationKey []byte, derivationKeys ...[]byte) []byte {
 	mKey := append([]byte(moduleName), 0)
-	return Hash("module", append(mKey, key...))
+	addr := Hash("module", append(mKey, derivationKey...))
+	for _, k := range derivationKeys {
+		addr = Derive(addr, k)
+	}
+	return addr
 }
 
 // Derive derives a new address from the main `address` and a derivation `key`.

types/address/hash_test.go

@@ -70,9 +70,18 @@ func (suite *AddressSuite) TestModule() {
 	addr2 := Module("myModule2", key)
 	assert.NotEqual(addr, addr2, "changing module name must change address")
 
-	addr3 := Module(modName, []byte{1, 2, 3})
+	k1 := []byte{1, 2, 3}
+	addr3 := Module(modName, k1)
 	assert.NotEqual(addr, addr3, "changing key must change address")
 	assert.NotEqual(addr2, addr3, "changing key must change address")
+
+	addr4 := Module(modName, k1, k1)
+	assert.Equal(Derive(addr3, k1), addr4)
+
+	k2 := []byte{0, 0, 7}
+	addr5 := Module(modName, k1, k1, k2)
+	assert.Equal(Derive(addr4, k2), addr5)
+
 }
 
 func (suite *AddressSuite) TestDerive() {

x/auth/types/credentials.go

@@ -31,25 +31,20 @@ const ModuleCredentialType = "ModuleCredential"
 
 var _ cryptotypes.PubKey = &ModuleCredential{}
 
+// NewModuleCredential creates new module credential key.
+// At least one derivation key must be provided. Panics otherwise.
 func NewModuleCredential(moduleName string, derivationKeys [][]byte) *ModuleCredential {
+	if len(derivationKeys) == 0 {
+		panic("can't create ModuleCredential withouth derivation key")
+	}
 	return &ModuleCredential{
 		ModuleName:     moduleName,
 		DerivationKeys: derivationKeys,
 	}
 }
 
 func (m *ModuleCredential) Address() cryptotypes.Address {
-	var addr []byte
-	for i, dk := range m.DerivationKeys {
-		if i == 0 {
-			addr = address.Module(m.ModuleName, dk)
-			continue
-		}
-
-		addr = address.Derive(addr, dk)
-	}
-
-	return addr
+	return address.Module(m.ModuleName, m.DerivationKeys[0], m.DerivationKeys[1:]...)
 }
 
 func (m *ModuleCredential) Bytes() []byte {

x/group/migrations/v2/migrate.go

@@ -21,7 +21,7 @@ const (
 )
 
 // Migrate migrates the x/group module state from the consensus version 1 to version 2.
-// Specifically, it changes the group policy account to from module account to base account.
+// Specifically, it changes the group policy account from module account to base account.
 func Migrate(
 	ctx sdk.Context,
 	storeKey storetypes.StoreKey,
@@ -32,11 +32,11 @@ func Migrate(
 	store := ctx.KVStore(storeKey)
 	curAccVal := groupPolicySeq.CurVal(store)
 	groupPolicyAccountDerivationKey := make(map[string][]byte, 0)
+	policyKey := []byte{GroupPolicyTablePrefix}
 	for i := uint64(0); i <= curAccVal; i++ {
 		derivationKey := make([]byte, 8)
 		binary.BigEndian.PutUint64(derivationKey, i)
-		parentAcc := address.Module(group.ModuleName, []byte{GroupPolicyTablePrefix})
-		groupPolicyAcc := sdk.AccAddress(address.Derive(parentAcc, derivationKey))
+		groupPolicyAcc := sdk.AccAddress(address.Module(group.ModuleName, policyKey, derivationKey))
 		groupPolicyAccountDerivationKey[groupPolicyAcc.String()] = derivationKey
 	}