From 013ab53a7ddff19c890207538e38fd7457cda323 Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Thu, 8 Apr 2021 20:44:59 +0200 Subject: [PATCH 1/7] update Derive functions --- types/address/hash.go | 25 +++++++++++++++++++++++-- types/address/hash_test.go | 38 ++++++++++++++++++++++++++++++++++---- 2 files changed, 57 insertions(+), 6 deletions(-) diff --git a/types/address/hash.go b/types/address/hash.go index a6c9fe94c51c..9ad53d937682 100644 --- a/types/address/hash.go +++ b/types/address/hash.go @@ -32,8 +32,8 @@ func Hash(typ string, key []byte) []byte { return hasher.Sum(nil) } -// NewComposed creates a new address based on sub addresses. -func NewComposed(typ string, subAddresses []Addressable) ([]byte, error) { +// Compose creates a new address based on sub addresses. +func Compose(typ string, subAddresses []Addressable) ([]byte, error) { as := make([][]byte, len(subAddresses)) totalLen := 0 var err error @@ -62,3 +62,24 @@ func Module(moduleName string, key []byte) []byte { mKey := append([]byte(moduleName), 0) return Hash("module", append(mKey, key...)) } + +// Derive derives a new address from the main `address` and a derivation `key`. +func Derive(address []byte, key []byte) []byte { + return Hash(conv.UnsafeBytesToStr(address), key) +} + +// DeriveMulti generalizes `Derive` function for multiple keys - `path`. The keys are order +// sensitive. Changing an order of the elements in the path will create a different key. +// NOTE: DeriveMulti(addr, [k1, k2]) != Derive(Derive(addr, k1), k2) +// != Derive(Derive(addr, k2), k1) +func DeriveMulti(address []byte, path [][]byte) ([]byte, error) { + key := []byte{} + for i, p := range path { + a, err := LengthPrefix(p) + if err != nil { + return nil, fmt.Errorf("a path key=%v at index=%d is not compatible [%w]", p, i, err) + } + key = append(key, a...) + } + return Hash(conv.UnsafeBytesToStr(address), key), nil +} diff --git a/types/address/hash_test.go b/types/address/hash_test.go index be096c357fec..09993b733367 100644 --- a/types/address/hash_test.go +++ b/types/address/hash_test.go @@ -34,7 +34,7 @@ func (suite *AddressSuite) TestComposed() { a2 := addrMock{[]byte{21, 22}} typ := "multisig" - ac, err := NewComposed(typ, []Addressable{a1, a2}) + ac, err := Compose(typ, []Addressable{a1, a2}) assert.NoError(err) assert.Len(ac, Len) @@ -45,18 +45,18 @@ func (suite *AddressSuite) TestComposed() { assert.Equal(ac, ac2, "NewComposed works correctly") // changing order of addresses shouldn't impact a composed address - ac2, err = NewComposed(typ, []Addressable{a2, a1}) + ac2, err = Compose(typ, []Addressable{a2, a1}) assert.NoError(err) assert.Len(ac2, Len) assert.Equal(ac, ac2, "NewComposed is not sensitive for order") // changing a type should change composed address - ac2, err = NewComposed(typ+"other", []Addressable{a2, a1}) + ac2, err = Compose(typ+"other", []Addressable{a2, a1}) assert.NoError(err) assert.NotEqual(ac, ac2, "NewComposed must be sensitive to type") // changing order of addresses shouldn't impact a composed address - ac2, err = NewComposed(typ, []Addressable{a1, addrMock{make([]byte, 300, 300)}}) + ac2, err = Compose(typ, []Addressable{a1, addrMock{make([]byte, 300, 300)}}) assert.Error(err) assert.Contains(err.Error(), "should be max 255 bytes, got 300") } @@ -75,6 +75,36 @@ func (suite *AddressSuite) TestModule() { assert.NotEqual(addr2, addr3, "changing key must change address") } +func (suite *AddressSuite) TestDerive() { + assert := suite.Assert() + var addr, key1, key2 = []byte{1, 2}, []byte{3, 4}, []byte{1, 2} + d1 := Derive(addr, key1) + d2 := Derive(addr, key2) + d3 := Derive(key1, key2) + assert.Len(d1, Len) + assert.Len(d2, Len) + assert.Len(d3, Len) + + assert.NotEqual(d1, d2) + assert.NotEqual(d1, d3) + assert.NotEqual(d2, d3) +} + +func (suite *AddressSuite) TestDeriveMulti() { + assert := suite.Assert() + var addr, key1, key2 = []byte{1, 2}, []byte{3, 4}, []byte{1, 2} + d1, err := DeriveMulti(addr, [][]byte{key1, key2}) + assert.NoError(err) + d2, err := DeriveMulti(addr, [][]byte{key2, key1}) + assert.NoError(err) + + assert.NotEqual(d1, d2) + d3 := Derive(Derive(addr, key1), key2) + assert.NotEqual(d1, d3) + d3 = Derive(Derive(addr, key2), key1) + assert.NotEqual(d1, d3) +} + type addrMock struct { Addr []byte } From c547f26e71972324559884b2befa9e2edc7aa5f8 Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Thu, 8 Apr 2021 20:45:29 +0200 Subject: [PATCH 2/7] update adr-028 --- .../adr-028-public-key-addresses.md | 27 ++++++++++++------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/docs/architecture/adr-028-public-key-addresses.md b/docs/architecture/adr-028-public-key-addresses.md index 91c52e810d5e..c0fe270455e9 100644 --- a/docs/architecture/adr-028-public-key-addresses.md +++ b/docs/architecture/adr-028-public-key-addresses.md @@ -185,14 +185,14 @@ NOTE: this section is not finalize and it's in active discussion. In Basic Address section we defined a module account address as: -``` +```go address.Hash("module", moduleName) ``` 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 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. -``` +```go func Module(moduleName string, key []byte) []byte{ return Hash("module", []byte(moduleName) + 0 + key) } @@ -208,20 +208,29 @@ If we want to create an address for a module account depending on more than one btcAtomAMM := address.Module("amm", btc.Addrress() + atom.Address()}) ``` -We can continue the derivation process and can create an address for a submodule account. +#### Derived Addresses -``` -func Submodule(address []byte, derivationKey []byte) { - return Hash("module", address + derivationKey) +Module address is a special case of more general _derived_ address defined by the `Derive` function: + +```go +func Derive(address []byte, derivationKey []byte) { + return Hash(addres, derivationKey) } ``` -NOTE: if `address` is not a hash based address (with `LEN` length) then we should use `LengthPrefix`. An alternative would be to use one `Module` function, which takes a slice of keys and mapped with `LengthPrefix`. For final version we need to validate what's the most common use. - **Example** For a cosmwasm smart-contract address we could use the following construction: ``` -smartContractAddr := Submodule(Module("cosmwasm", smartContractsNamespace), smartContractKey) +smartContractAddr := Derived(Module("cosmwasm", smartContractsNamespace), []{smartContractKey}) +``` + +We can also define a function which will derive an address based on multiple keys (path). The function is similar to the `NewComposed`, however it doesn't sort the derivation keys: + +```go +func DeriveMulti(address []byte, derivationKeys [][]byte) { + keys = map(derivationKeys, \k -> LengthPrefix(k)) + return Hash(LengthPrefix(address), keys[0] + ... + keys[n]) +} ``` From 89a895b96ee695f474a053e84af668e7ff4107d2 Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Fri, 9 Apr 2021 11:50:21 +0200 Subject: [PATCH 3/7] changelog update --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7de63a77f6b1..14f7273cf827 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,6 +42,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * [\#8786](https://github.com/cosmos/cosmos-sdk/pull/8786) Enabled secp256r1 in x/auth. * (rosetta) [\#8729](https://github.com/cosmos/cosmos-sdk/pull/8729) Data API fully supports balance tracking. Construction API can now construct any message supported by the application. * [\#8754](https://github.com/cosmos/cosmos-sdk/pull/8875) Added support for reverse iteration to pagination. +* [#9088](https://github.com/cosmos/cosmos-sdk/pull/9088) Added implementation to ADR-28 Derived Addresses. ### Client Breaking Changes @@ -78,6 +79,9 @@ Ref: https://keepachangelog.com/en/1.0.0/ * `MsgCreateValidator.Pubkey` type changed from `string` to `codectypes.Any`. * (client) [\#8926](https://github.com/cosmos/cosmos-sdk/pull/8926) `client/tx.PrepareFactory` has been converted to a private function, as it's only used internally. * (auth/tx) [\#8926](https://github.com/cosmos/cosmos-sdk/pull/8926) The `ProtoTxProvider` interface used as a workaround for transaction simulation has been removed. +* [#9088](https://github.com/cosmos/cosmos-sdk/pull/9088) Unify naming of the ADR-28 related functions: Renames: `address.NewCompose` -> `address.Compose`, `address.NewCompose` + + ### State Machine Breaking From dd0478d0d14d055bd7fecdbd20322711257338cd Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Fri, 9 Apr 2021 14:23:03 +0200 Subject: [PATCH 4/7] Apply suggestions from code review Co-authored-by: Marie Gauthier --- docs/architecture/adr-028-public-key-addresses.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/architecture/adr-028-public-key-addresses.md b/docs/architecture/adr-028-public-key-addresses.md index c0fe270455e9..f12645d82480 100644 --- a/docs/architecture/adr-028-public-key-addresses.md +++ b/docs/architecture/adr-028-public-key-addresses.md @@ -213,7 +213,7 @@ btcAtomAMM := address.Module("amm", btc.Addrress() + atom.Address()}) Module address is a special case of more general _derived_ address defined by the `Derive` function: ```go -func Derive(address []byte, derivationKey []byte) { +func Derive(address []byte, derivationKey []byte) []byte { return Hash(addres, derivationKey) } ``` @@ -227,7 +227,7 @@ smartContractAddr := Derived(Module("cosmwasm", smartContractsNamespace), []{sma We can also define a function which will derive an address based on multiple keys (path). The function is similar to the `NewComposed`, however it doesn't sort the derivation keys: ```go -func DeriveMulti(address []byte, derivationKeys [][]byte) { +func DeriveMulti(address []byte, derivationKeys [][]byte) []byte { keys = map(derivationKeys, \k -> LengthPrefix(k)) return Hash(LengthPrefix(address), keys[0] + ... + keys[n]) } From 9d415ad3c4a5debe2298318691c2022d8acea2d3 Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Fri, 9 Apr 2021 14:32:21 +0200 Subject: [PATCH 5/7] review updates --- docs/architecture/adr-028-public-key-addresses.md | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/docs/architecture/adr-028-public-key-addresses.md b/docs/architecture/adr-028-public-key-addresses.md index f12645d82480..d737f8b2a5c7 100644 --- a/docs/architecture/adr-028-public-key-addresses.md +++ b/docs/architecture/adr-028-public-key-addresses.md @@ -135,7 +135,7 @@ type Addressable interface { Address() []byte } -func NewComposed(typ string, subaccounts []Addressable) []byte { +func Composed(typ string, subaccounts []Addressable) []byte { addresses = map(subaccounts, \a -> LengthPrefix(a.Address())) addresses = sort(addresses) return address.Hash(typ, addresses[0] + ... + addresses[n]) @@ -175,7 +175,7 @@ func (multisig PubKey) Address() { prefix := fmt.Sprintf("%s/%d", proto.MessageName(multisig), multisig.Threshold) // use the Composed function defined above - return address.NewComposed(prefix, keys) + return address.Composed(prefix, keys) } ``` @@ -210,7 +210,7 @@ btcAtomAMM := address.Module("amm", btc.Addrress() + atom.Address()}) #### Derived Addresses -Module address is a special case of more general _derived_ address defined by the `Derive` function: +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 func Derive(address []byte, derivationKey []byte) []byte { @@ -218,13 +218,14 @@ func Derive(address []byte, derivationKey []byte) []byte { } ``` +Note: `Module` is a special case of the more general _derived_ address, where we set the `"module"` string for the _from address_. **Example** For a cosmwasm smart-contract address we could use the following construction: ``` smartContractAddr := Derived(Module("cosmwasm", smartContractsNamespace), []{smartContractKey}) ``` -We can also define a function which will derive an address based on multiple keys (path). The function is similar to the `NewComposed`, however it doesn't sort the derivation keys: +We can also define a function which will derive an address based on multiple keys (path). The function is similar to the `Composed`, however it doesn't sort the derivation keys: ```go func DeriveMulti(address []byte, derivationKeys [][]byte) []byte { From ac53c6e0aa371eb2d82d30cdd396e9d36fc0b79f Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Wed, 14 Apr 2021 16:11:39 +0200 Subject: [PATCH 6/7] remove DeriveMulti and rollback some changes in CHANGELOG --- CHANGELOG.md | 1 - .../architecture/adr-028-public-key-addresses.md | 10 ---------- types/address/hash.go | 16 ---------------- types/address/hash_test.go | 15 --------------- 4 files changed, 42 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 14f7273cf827..b6cdeb465fef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -79,7 +79,6 @@ Ref: https://keepachangelog.com/en/1.0.0/ * `MsgCreateValidator.Pubkey` type changed from `string` to `codectypes.Any`. * (client) [\#8926](https://github.com/cosmos/cosmos-sdk/pull/8926) `client/tx.PrepareFactory` has been converted to a private function, as it's only used internally. * (auth/tx) [\#8926](https://github.com/cosmos/cosmos-sdk/pull/8926) The `ProtoTxProvider` interface used as a workaround for transaction simulation has been removed. -* [#9088](https://github.com/cosmos/cosmos-sdk/pull/9088) Unify naming of the ADR-28 related functions: Renames: `address.NewCompose` -> `address.Compose`, `address.NewCompose` diff --git a/docs/architecture/adr-028-public-key-addresses.md b/docs/architecture/adr-028-public-key-addresses.md index d737f8b2a5c7..d426e9969ff7 100644 --- a/docs/architecture/adr-028-public-key-addresses.md +++ b/docs/architecture/adr-028-public-key-addresses.md @@ -225,16 +225,6 @@ Note: `Module` is a special case of the more general _derived_ address, where we smartContractAddr := Derived(Module("cosmwasm", smartContractsNamespace), []{smartContractKey}) ``` -We can also define a function which will derive an address based on multiple keys (path). The function is similar to the `Composed`, however it doesn't sort the derivation keys: - -```go -func DeriveMulti(address []byte, derivationKeys [][]byte) []byte { - keys = map(derivationKeys, \k -> LengthPrefix(k)) - return Hash(LengthPrefix(address), keys[0] + ... + keys[n]) -} -``` - - ### Schema Types diff --git a/types/address/hash.go b/types/address/hash.go index 9ad53d937682..912b831fc2da 100644 --- a/types/address/hash.go +++ b/types/address/hash.go @@ -67,19 +67,3 @@ func Module(moduleName string, key []byte) []byte { func Derive(address []byte, key []byte) []byte { return Hash(conv.UnsafeBytesToStr(address), key) } - -// DeriveMulti generalizes `Derive` function for multiple keys - `path`. The keys are order -// sensitive. Changing an order of the elements in the path will create a different key. -// NOTE: DeriveMulti(addr, [k1, k2]) != Derive(Derive(addr, k1), k2) -// != Derive(Derive(addr, k2), k1) -func DeriveMulti(address []byte, path [][]byte) ([]byte, error) { - key := []byte{} - for i, p := range path { - a, err := LengthPrefix(p) - if err != nil { - return nil, fmt.Errorf("a path key=%v at index=%d is not compatible [%w]", p, i, err) - } - key = append(key, a...) - } - return Hash(conv.UnsafeBytesToStr(address), key), nil -} diff --git a/types/address/hash_test.go b/types/address/hash_test.go index 09993b733367..ae712c18dd4a 100644 --- a/types/address/hash_test.go +++ b/types/address/hash_test.go @@ -90,21 +90,6 @@ func (suite *AddressSuite) TestDerive() { assert.NotEqual(d2, d3) } -func (suite *AddressSuite) TestDeriveMulti() { - assert := suite.Assert() - var addr, key1, key2 = []byte{1, 2}, []byte{3, 4}, []byte{1, 2} - d1, err := DeriveMulti(addr, [][]byte{key1, key2}) - assert.NoError(err) - d2, err := DeriveMulti(addr, [][]byte{key2, key1}) - assert.NoError(err) - - assert.NotEqual(d1, d2) - d3 := Derive(Derive(addr, key1), key2) - assert.NotEqual(d1, d3) - d3 = Derive(Derive(addr, key2), key1) - assert.NotEqual(d1, d3) -} - type addrMock struct { Addr []byte } From 7ee294b329febf3138fa4aa09b7cecacb247dbea Mon Sep 17 00:00:00 2001 From: Robert Zaremba Date: Thu, 15 Apr 2021 00:20:42 +0200 Subject: [PATCH 7/7] add noop error check --- types/address/hash.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/types/address/hash.go b/types/address/hash.go index 912b831fc2da..8d9c5b39e11b 100644 --- a/types/address/hash.go +++ b/types/address/hash.go @@ -20,12 +20,13 @@ type Addressable interface { // Hash creates a new address from address type and key func Hash(typ string, key []byte) []byte { hasher := sha256.New() - hasher.Write(conv.UnsafeStrToBytes(typ)) + _, err := hasher.Write(conv.UnsafeStrToBytes(typ)) + // the error always nil, it's here only to satisfy the io.Writer interface + errors.AssertNil(err) th := hasher.Sum(nil) hasher.Reset() - _, err := hasher.Write(th) - // the error always nil, it's here only to satisfy the io.Writer interface + _, err = hasher.Write(th) errors.AssertNil(err) _, err = hasher.Write(key) errors.AssertNil(err)