adr-028 address generation (#8415)

* Optimize secp256k1 hashing

* Add ADR-028 related functions

* Update ed25519

* fix errors/handle

* fix build

* fix build

* Add tests and update function names

* wip

* Use LengthPrefix for composed addresses

* add tests for NewComposed

* add module hash function

* fix append

* rollback ed25519 ADR-28 update

* rollback ed25519 ADR-28 test

* Adding Module tests and convert tests to test suite

* convert store_key_test.go to test suite

* rollback test check comment

* Rename assert.Panic and add comment

* add note to ed25519 about SDK support with regards to ADR-28

* Update ed25519 TestAddress

* Adding Deprecated notes for ed25519.PrivKey

* Update crypto/keys/ed25519/ed25519.go

Co-authored-by: Marie Gauthier <marie.gauthier63@gmail.com>

* Update types/address/hash_test.go

Co-authored-by: Marie Gauthier <marie.gauthier63@gmail.com>

* solve linter issues

* linter: remove gocritic

Co-authored-by: Marie Gauthier <marie.gauthier63@gmail.com>

codec/amino.go

@@ -13,8 +13,8 @@ import (
 	"github.com/cosmos/cosmos-sdk/codec/types"
 )
 
-// deprecated: LegacyAmino defines a wrapper for an Amino codec that properly handles protobuf
-// types with Any's
+// LegacyAmino defines a wrapper for an Amino codec that properly
+// handles protobuf types with Any's. Deprecated.
 type LegacyAmino struct {
 	Amino *amino.Codec
 }

crypto/codec/amino.go

@@ -26,7 +26,7 @@ func RegisterCrypto(cdc *codec.LegacyAmino) {
 	cdc.RegisterInterface((*cryptotypes.PrivKey)(nil), nil)
 	cdc.RegisterConcrete(sr25519.PrivKey{},
 		sr25519.PrivKeyName, nil)
-	cdc.RegisterConcrete(&ed25519.PrivKey{},
+	cdc.RegisterConcrete(&ed25519.PrivKey{}, //nolint:staticcheck
 		ed25519.PrivKeyName, nil)
 	cdc.RegisterConcrete(&secp256k1.PrivKey{},
 		secp256k1.PrivKeyName, nil)

crypto/keyring/legacy.go

@@ -42,7 +42,7 @@ var _ LegacyKeybase = dbKeybase{}
 // dbKeybase combines encryption and storage implementation to provide a
 // full-featured key manager.
 //
-// NOTE: dbKeybase will be deprecated in favor of keyringKeybase.
+// Deprecated: dbKeybase will be removed in favor of keyringKeybase.
 type dbKeybase struct {
 	db dbm.DB
 }

crypto/keys/ed25519/ed25519.go

@@ -116,7 +116,8 @@ func (privKey *PrivKey) UnmarshalAminoJSON(bz []byte) error {
 	return privKey.UnmarshalAmino(bz)
 }
 
-// GenPrivKey generates a new ed25519 private key.
+// GenPrivKey generates a new ed25519 private key. These ed25519 keys must not
+// be used in SDK apps except in a tendermint validator context.
 // It uses OS randomness in conjunction with the current global random seed
 // in tendermint/libs/common to generate the private key.
 func GenPrivKey() *PrivKey {
@@ -137,6 +138,7 @@ func genPrivKey(rand io.Reader) *PrivKey {
 
 // GenPrivKeyFromSecret hashes the secret with SHA2, and uses
 // that 32 byte output to create the private key.
+// NOTE: ed25519 keys must not be used in SDK apps except in a tendermint validator context.
 // NOTE: secret should be the output of a KDF like bcrypt,
 // if it's derived from user input.
 func GenPrivKeyFromSecret(secret []byte) *PrivKey {
@@ -151,10 +153,14 @@ var _ cryptotypes.PubKey = &PubKey{}
 var _ codec.AminoMarshaler = &PubKey{}
 
 // Address is the SHA256-20 of the raw pubkey bytes.
+// It doesn't implement ADR-28 addresses and it must not be used
+// in SDK except in a tendermint validator context.
 func (pubKey *PubKey) Address() crypto.Address {
 	if len(pubKey.Key) != PubKeySize {
 		panic("pubkey is incorrect size")
 	}
+	// For ADR-28 compatible address we would need to
+	// return address.Hash(proto.MessageName(pubKey), pubKey.Key)
 	return crypto.Address(tmhash.SumTruncated(pubKey.Key))
 }
 

crypto/keys/ed25519/ed25519_test.go

@@ -84,6 +84,12 @@ func TestPubKeyEquals(t *testing.T) {
 	}
 }
 
+func TestAddressEd25519(t *testing.T) {
+	pk := ed25519.PubKey{[]byte{125, 80, 29, 208, 159, 53, 119, 198, 73, 53, 187, 33, 199, 144, 62, 255, 1, 235, 117, 96, 128, 211, 17, 45, 34, 64, 189, 165, 33, 182, 54, 206}}
+	addr := pk.Address()
+	require.Len(t, addr, 20, "Address must be 20 bytes long")
+}
+
 func TestPrivKeyEquals(t *testing.T) {
 	ed25519PrivKey := ed25519.GenPrivKey()
 

crypto/keys/ed25519/keys.pb.go

@@ -24,11 +24,11 @@ var _ = math.Inf
 // proto package needs to be updated.
 const _ = proto.GoGoProtoPackageIsVersion3 // please upgrade the proto package
 
-// PubKey defines a ed25519 public key
-// Key is the compressed form of the pubkey. The first byte depends is a 0x02 byte
-// if the y-coordinate is the lexicographically largest of the two associated with
-// the x-coordinate. Otherwise the first byte is a 0x03.
-// This prefix is followed with the x-coordinate.
+// PubKey is an ed25519 public key for handling Tendermint keys in SDK.
+// It's needed for Any serialization and SDK compatibility.
+// It must not be used in a non Tendermint key context because it doesn't implement
+// ADR-28. Nevertheless, you will like to use ed25519 in app user level
+// then you must create a new proto message and follow ADR-28 for Address construction.
 type PubKey struct {
 	Key crypto_ed25519.PublicKey `protobuf:"bytes,1,opt,name=key,proto3,casttype=crypto/ed25519.PublicKey" json:"key,omitempty"`
 }
@@ -72,7 +72,8 @@ func (m *PubKey) GetKey() crypto_ed25519.PublicKey {
 	return nil
 }
 
-// PrivKey defines a ed25519 private key.
+// Deprecated: PrivKey defines a ed25519 private key.
+// NOTE: ed25519 keys must not be used in SDK apps except in a tendermint validator context.
 type PrivKey struct {
 	Key crypto_ed25519.PrivateKey `protobuf:"bytes,1,opt,name=key,proto3,casttype=crypto/ed25519.PrivateKey" json:"key,omitempty"`
 }

crypto/keys/secp256k1/secp256k1.go

@@ -151,12 +151,9 @@ func (pubKey *PubKey) Address() crypto.Address {
 		panic("length of pubkey is incorrect")
 	}
 
-	hasherSHA256 := sha256.New()
-	hasherSHA256.Write(pubKey.Key) // does not error
-	sha := hasherSHA256.Sum(nil)
-
+	sha := sha256.Sum256(pubKey.Key)
 	hasherRIPEMD160 := ripemd160.New()
-	hasherRIPEMD160.Write(sha) // does not error
+	hasherRIPEMD160.Write(sha[:]) // does not error
 	return crypto.Address(hasherRIPEMD160.Sum(nil))
 }
 

docs/core/proto-docs.md

@@ -2885,7 +2885,8 @@ Msg defines the bank Msg service.
 <a name="cosmos.crypto.ed25519.PrivKey"></a>
 
 ### PrivKey
-PrivKey defines a ed25519 private key.
+Deprecated: PrivKey defines a ed25519 private key.
+NOTE: ed25519 keys must not be used in SDK apps except in a tendermint validator context.
 
 
 | Field | Type | Label | Description |
@@ -2900,11 +2901,11 @@ PrivKey defines a ed25519 private key.
 <a name="cosmos.crypto.ed25519.PubKey"></a>
 
 ### PubKey
-PubKey defines a ed25519 public key
-Key is the compressed form of the pubkey. The first byte depends is a 0x02 byte
-if the y-coordinate is the lexicographically largest of the two associated with
-the x-coordinate. Otherwise the first byte is a 0x03.
-This prefix is followed with the x-coordinate.
+PubKey is an ed25519 public key for handling Tendermint keys in SDK.
+It's needed for Any serialization and SDK compatibility.
+It must not be used in a non Tendermint key context because it doesn't implement
+ADR-28. Nevertheless, you will like to use ed25519 in app user level
+then you must create a new proto message and follow ADR-28 for Address construction.
 
 
 | Field | Type | Label | Description |

proto/cosmos/crypto/ed25519/keys.proto

@@ -5,18 +5,19 @@ import "gogoproto/gogo.proto";
 
 option go_package = "github.com/cosmos/cosmos-sdk/crypto/keys/ed25519";
 
-// PubKey defines a ed25519 public key
-// Key is the compressed form of the pubkey. The first byte depends is a 0x02 byte
-// if the y-coordinate is the lexicographically largest of the two associated with
-// the x-coordinate. Otherwise the first byte is a 0x03.
-// This prefix is followed with the x-coordinate.
+// PubKey is an ed25519 public key for handling Tendermint keys in SDK.
+// It's needed for Any serialization and SDK compatibility.
+// It must not be used in a non Tendermint key context because it doesn't implement
+// ADR-28. Nevertheless, you will like to use ed25519 in app user level
+// then you must create a new proto message and follow ADR-28 for Address construction.
 message PubKey {
   option (gogoproto.goproto_stringer) = false;
 
   bytes key = 1 [(gogoproto.casttype) = "crypto/ed25519.PublicKey"];
 }
 
-// PrivKey defines a ed25519 private key.
+// Deprecated: PrivKey defines a ed25519 private key.
+// NOTE: ed25519 keys must not be used in SDK apps except in a tendermint validator context.
 message PrivKey {
   bytes key = 1 [(gogoproto.casttype) = "crypto/ed25519.PrivateKey"];
 }

types/address/README.md

@@ -0,0 +1,7 @@
+# Account
+
+This package defines Cosmos SDK address related functions.
+
+## References
+
++ [ADR-028](../../docs/architecture/adr-028-public-key-addresses.md)

types/address/hash.go

@@ -0,0 +1,74 @@
+package address
+
+import (
+	"bytes"
+	"crypto/sha256"
+	"fmt"
+	"reflect"
+	"sort"
+	"unsafe"
+
+	"github.com/cosmos/cosmos-sdk/types/errors"
+)
+
+// Len is the length of base addresses
+const Len = sha256.Size
+
+type Addressable interface {
+	Address() []byte
+}
+
+// Hash creates a new address from address type and key
+func Hash(typ string, key []byte) []byte {
+	hasher := sha256.New()
+	hasher.Write(unsafeStrToByteArray(typ))
+	th := hasher.Sum(nil)
+
+	hasher.Reset()
+	_, err := hasher.Write(th)
+	// the error always nil, it's here only to satisfy the io.Writer interface
+	errors.AssertNil(err)
+	_, err = hasher.Write(key)
+	errors.AssertNil(err)
+	return hasher.Sum(nil)
+}
+
+// NewComposed creates a new address based on sub addresses.
+func NewComposed(typ string, subAddresses []Addressable) ([]byte, error) {
+	as := make([][]byte, len(subAddresses))
+	totalLen := 0
+	var err error
+	for i := range subAddresses {
+		a := subAddresses[i].Address()
+		as[i], err = LengthPrefix(a)
+		if err != nil {
+			return nil, fmt.Errorf("not compatible sub-adddress=%v at index=%d [%w]", a, i, err)
+		}
+		totalLen += len(as[i])
+	}
+
+	sort.Slice(as, func(i, j int) bool { return bytes.Compare(as[i], as[j]) <= 0 })
+	key := make([]byte, totalLen)
+	offset := 0
+	for i := range as {
+		copy(key[offset:], as[i])
+		offset += len(as[i])
+	}
+	return Hash(typ, key), nil
+}
+
+// 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 {
+	mKey := append([]byte(moduleName), 0)
+	return Hash("module", append(mKey, key...))
+}
+
+// unsafeStrToByteArray uses unsafe to convert string into byte array. Returned array
+// cannot be altered after this functions is called
+func unsafeStrToByteArray(s string) []byte {
+	sh := *(*reflect.SliceHeader)(unsafe.Pointer(&s))
+	sh.Cap = sh.Len
+	bs := *(*[]byte)(unsafe.Pointer(&sh))
+	return bs
+}

types/address/hash_test.go

@@ -0,0 +1,90 @@
+package address
+
+import (
+	"crypto/sha256"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/suite"
+)
+
+func TestAddressSuite(t *testing.T) {
+	suite.Run(t, new(AddressSuite))
+}
+
+type AddressSuite struct{ suite.Suite }
+
+func (suite *AddressSuite) TestHash() {
+	assert := suite.Assert()
+	typ := "1"
+	key := []byte{1}
+	part1 := sha256.Sum256([]byte(typ))
+	expected := sha256.Sum256(append(part1[:], key...))
+	received := Hash(typ, key)
+	assert.Equal(expected[:], received, "must create a correct address")
+
+	received = Hash("other", key)
+	assert.NotEqual(expected[:], received, "must create a correct address")
+	assert.Len(received, Len, "must have correct length")
+}
+
+func (suite *AddressSuite) TestComposed() {
+	assert := suite.Assert()
+	a1 := addrMock{[]byte{11, 12}}
+	a2 := addrMock{[]byte{21, 22}}
+
+	typ := "multisig"
+	ac, err := NewComposed(typ, []Addressable{a1, a2})
+	assert.NoError(err)
+	assert.Len(ac, Len)
+
+	// check if optimizations work
+	checkingKey := append([]byte{}, a1.AddressWithLen(suite.T())...)
+	checkingKey = append(checkingKey, a2.AddressWithLen(suite.T())...)
+	ac2 := Hash(typ, checkingKey)
+	assert.Equal(ac, ac2, "NewComposed works correctly")
+
+	// changing order of addresses shouldn't impact a composed address
+	ac2, err = NewComposed(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})
+	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)}})
+	assert.Error(err)
+	assert.Contains(err.Error(), "should be max 255 bytes, got 300")
+}
+
+func (suite *AddressSuite) TestModule() {
+	assert := suite.Assert()
+	var modName, key = "myModule", []byte{1, 2}
+	addr := Module(modName, key)
+	assert.Len(addr, Len, "must have address length")
+
+	addr2 := Module("myModule2", key)
+	assert.NotEqual(addr, addr2, "changing module name must change address")
+
+	addr3 := Module(modName, []byte{1, 2, 3})
+	assert.NotEqual(addr, addr3, "changing key must change address")
+	assert.NotEqual(addr2, addr3, "changing key must change address")
+}
+
+type addrMock struct {
+	Addr []byte
+}
+
+func (a addrMock) Address() []byte {
+	return a.Addr
+}
+
+func (a addrMock) AddressWithLen(t *testing.T) []byte {
+	addr, err := LengthPrefix(a.Addr)
+	assert.NoError(t, err)
+	return addr
+}

types/address/store_key_test.go

@@ -3,12 +3,19 @@ package address_test
 import (
 	"testing"
 
-	"github.com/stretchr/testify/require"
+	"github.com/stretchr/testify/suite"
 
 	"github.com/cosmos/cosmos-sdk/types/address"
 )
 
-func TestLengthPrefixedAddressStoreKey(t *testing.T) {
+func TestStoreKeySuite(t *testing.T) {
+	suite.Run(t, new(StoreKeySuite))
+}
+
+type StoreKeySuite struct{ suite.Suite }
+
+func (suite *StoreKeySuite) TestLengthPrefix() {
+	require := suite.Require()
 	addr10byte := []byte{0, 1, 2, 3, 4, 5, 6, 7, 8, 9}
 	addr20byte := []byte{0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19}
 	addr256byte := make([]byte, 256)
@@ -23,15 +30,16 @@ func TestLengthPrefixedAddressStoreKey(t *testing.T) {
 		{"20-byte address", addr20byte, append([]byte{byte(20)}, addr20byte...), false},
 		{"256-byte address (too long)", addr256byte, nil, true},
 	}
+
 	for _, tt := range tests {
 		tt := tt
-		t.Run(tt.name, func(t *testing.T) {
+		suite.Run(tt.name, func() {
 			storeKey, err := address.LengthPrefix(tt.addr)
 			if tt.expErr {
-				require.Error(t, err)
+				require.Error(err)
 			} else {
-				require.NoError(t, err)
-				require.Equal(t, tt.expStoreKey, storeKey)
+				require.NoError(err)
+				require.Equal(tt.expStoreKey, storeKey)
 			}
 		})
 	}

types/errors/handle.go

@@ -0,0 +1,12 @@
+package errors
+
+import "fmt"
+
+// AssertNil panics on error
+// Should be only used with interface methods, which require return error, but the
+// error is always nil
+func AssertNil(err error) {
+	if err != nil {
+		panic(fmt.Errorf("logic error - this should never happen. %w", err))
+	}
+}

x/auth/types/account.go

@@ -48,6 +48,7 @@ func ProtoBaseAccount() AccountI {
 }
 
 // NewBaseAccountWithAddress - returns a new base account with a given address
+// leaving AccountNumber and Sequence to zero.
 func NewBaseAccountWithAddress(addr sdk.AccAddress) *BaseAccount {
 	return &BaseAccount{
 		Address: addr.String(),

x/staking/types/validator.go

@@ -103,9 +103,7 @@ func (v Validators) Less(i, j int) bool {
 
 // Implements sort interface
 func (v Validators) Swap(i, j int) {
-	it := v[i]
-	v[i] = v[j]
-	v[j] = it
+	v[i], v[j] = v[j], v[i]
 }
 
 // ValidatorsByVotingPower implements sort.Interface for []Validator based on