feat(x/accounts)!: make address generation more robust and add predic…

…table address creation (#22776)

Co-authored-by: Julien Robert <[email protected]>

tests/integration/accounts/base_account_test.go

@@ -32,7 +32,7 @@ func TestBaseAccount(t *testing.T) {
 
 	_, baseAccountAddr, err := ak.Init(ctx, "base", accCreator, &baseaccountv1.MsgInit{
 		PubKey: toAnyPb(t, privKey.PubKey()),
-	}, nil)
+	}, nil, nil)
 	require.NoError(t, err)
 
 	// fund base account! this will also cause an auth base account to be created

tests/integration/accounts/fixture_test.go

@@ -185,7 +185,7 @@ func initFixture(t *testing.T, f func(ctx context.Context, msg *account_abstract
 	banktypes.RegisterMsgServer(router, bankkeeper.NewMsgServerImpl(bankKeeper))
 
 	// init account
-	_, addr, err := accountsKeeper.Init(integrationApp.Context(), "mock", []byte("system"), &gogotypes.Empty{}, nil)
+	_, addr, err := accountsKeeper.Init(integrationApp.Context(), "mock", []byte("system"), &gogotypes.Empty{}, nil, nil)
 	require.NoError(t, err)
 
 	fixture := &fixture{

tests/integration/accounts/lockup/continous_lockup_test_suite.go

@@ -34,7 +34,7 @@ func (s *IntegrationTestSuite) TestContinuousLockingAccount() {
 		StartTime: currentTime,
 		// end time in 1 minutes
 		EndTime: currentTime.Add(time.Minute),
-	}, sdk.Coins{sdk.NewCoin("stake", math.NewInt(1000))})
+	}, sdk.Coins{sdk.NewCoin("stake", math.NewInt(1000))}, nil)
 	require.NoError(t, err)
 
 	addr, err := app.AuthKeeper.AddressCodec().BytesToString(randAcc)

tests/integration/accounts/lockup/delayed_lockup_test_suite.go

@@ -33,7 +33,7 @@ func (s *IntegrationTestSuite) TestDelayedLockingAccount() {
 		Owner: ownerAddrStr,
 		// end time in 1 minutes
 		EndTime: currentTime.Add(time.Minute),
-	}, sdk.Coins{sdk.NewCoin("stake", math.NewInt(1000))})
+	}, sdk.Coins{sdk.NewCoin("stake", math.NewInt(1000))}, nil)
 	require.NoError(t, err)
 
 	addr, err := app.AuthKeeper.AddressCodec().BytesToString(randAcc)

tests/integration/accounts/lockup/periodic_lockup_test_suite.go

@@ -46,7 +46,7 @@ func (s *IntegrationTestSuite) TestPeriodicLockingAccount() {
 				Length: time.Minute,
 			},
 		},
-	}, sdk.Coins{sdk.NewCoin("stake", math.NewInt(1500))})
+	}, sdk.Coins{sdk.NewCoin("stake", math.NewInt(1500))}, nil)
 	require.NoError(t, err)
 
 	addr, err := app.AuthKeeper.AddressCodec().BytesToString(randAcc)

tests/integration/accounts/lockup/permanent_lockup_test_suite.go

@@ -30,7 +30,7 @@ func (s *IntegrationTestSuite) TestPermanentLockingAccount() {
 
 	_, accountAddr, err := app.AccountsKeeper.Init(ctx, lockupaccount.PERMANENT_LOCKING_ACCOUNT, accOwner, &types.MsgInitLockupAccount{
 		Owner: ownerAddrStr,
-	}, sdk.Coins{sdk.NewCoin("stake", math.NewInt(1000))})
+	}, sdk.Coins{sdk.NewCoin("stake", math.NewInt(1000))}, nil)
 	require.NoError(t, err)
 
 	addr, err := app.AuthKeeper.AddressCodec().BytesToString(randAcc)

tests/integration/accounts/multisig/test_suite.go

@@ -85,7 +85,7 @@ func (s *IntegrationTestSuite) initAccount(ctx context.Context, sender []byte, m
 				Revote:         false,
 				EarlyExecution: true,
 			},
-		}, sdk.Coins{sdk.NewCoin("stake", math.NewInt(1000))})
+		}, sdk.Coins{sdk.NewCoin("stake", math.NewInt(1000))}, nil)
 	s.NoError(err)
 
 	accountAddrStr, err := s.app.AuthKeeper.AddressCodec().BytesToString(accountAddr)

tests/integration/accounts/wiring_test.go

@@ -28,7 +28,7 @@ func TestDependencies(t *testing.T) {
 
 	_, counterAddr, err := ak.Init(ctx, "counter", accCreator, &counterv1.MsgInit{
 		InitialValue: 0,
-	}, nil)
+	}, nil, nil)
 	require.NoError(t, err)
 	// test dependencies
 	creatorInitFunds := sdk.NewCoins(sdk.NewInt64Coin("stake", 100_000))

tests/integration/auth/keeper/accounts_retro_compatibility_test.go

@@ -74,7 +74,7 @@ func TestAuthToAccountsGRPCCompat(t *testing.T) {
 
 	// init three accounts
 	for n, a := range accs {
-		_, addr, err := f.accountsKeeper.Init(f.app.Context(), n, []byte("me"), &gogotypes.Empty{}, nil)
+		_, addr, err := f.accountsKeeper.Init(f.app.Context(), n, []byte("me"), &gogotypes.Empty{}, nil, nil)
 		require.NoError(t, err)
 		a.(*mockRetroCompatAccount).address = addr
 	}
@@ -132,10 +132,10 @@ func TestAccountsBaseAccountRetroCompat(t *testing.T) {
 	require.NoError(t, err)
 
 	// we init two accounts to have account num not be zero.
-	_, _, err = f.accountsKeeper.Init(f.app.Context(), "base", []byte("me"), &basev1.MsgInit{PubKey: anyPk}, nil)
+	_, _, err = f.accountsKeeper.Init(f.app.Context(), "base", []byte("me"), &basev1.MsgInit{PubKey: anyPk}, nil, nil)
 	require.NoError(t, err)
 
-	_, addr, err := f.accountsKeeper.Init(f.app.Context(), "base", []byte("me"), &basev1.MsgInit{PubKey: anyPk}, nil)
+	_, addr, err := f.accountsKeeper.Init(f.app.Context(), "base", []byte("me"), &basev1.MsgInit{PubKey: anyPk}, nil, nil)
 	require.NoError(t, err)
 
 	// try to query it via auth

tests/integration/v2/auth/accounts_retro_compatibility_test.go

@@ -99,7 +99,7 @@ func TestAuthToAccountsGRPCCompat(t *testing.T) {
 
 	// init three accounts
 	for n, a := range accs {
-		_, addr, err := f.accountsKeeper.Init(f.ctx, n, []byte("me"), &gogotypes.Empty{}, nil)
+		_, addr, err := f.accountsKeeper.Init(f.ctx, n, []byte("me"), &gogotypes.Empty{}, nil, nil)
 		require.NoError(t, err)
 		a.(*mockRetroCompatAccount).address = addr
 	}
@@ -159,10 +159,10 @@ func TestAccountsBaseAccountRetroCompat(t *testing.T) {
 	// we init two accounts. Account number should start with 4
 	// since the first three accounts are fee_collector, bonded_tokens_pool, not_bonded_tokens_pool
 	// generated by init genesis plus one more genesis account, which make the current account number 4.
-	_, _, err = f.accountsKeeper.Init(f.ctx, "base", []byte("me"), &basev1.MsgInit{PubKey: anyPk}, nil)
+	_, _, err = f.accountsKeeper.Init(f.ctx, "base", []byte("me"), &basev1.MsgInit{PubKey: anyPk}, nil, nil)
 	require.NoError(t, err)
 
-	_, addr, err := f.accountsKeeper.Init(f.ctx, "base", []byte("me"), &basev1.MsgInit{PubKey: anyPk}, nil)
+	_, addr, err := f.accountsKeeper.Init(f.ctx, "base", []byte("me"), &basev1.MsgInit{PubKey: anyPk}, nil, nil)
 	require.NoError(t, err)
 
 	// try to query it via auth

x/accounts/README.md

@@ -496,6 +496,32 @@ func (a Account) AuthRetroCompatibility(ctx context.Context, _ *authtypes.QueryL
 * Implement this handler only for account types you want to expose via x/auth gRPC methods.
 * The `info` field in the response can be nil if your account doesn't fit the `BaseAccount` structure.
 
+## Address Derivation
+
+The x/accounts module offers two methods for deriving addresses, both ensuring non-squattability. This means each address is uniquely tied to its creator, preventing address collisions between different creators (e.g., Alice cannot create addresses that would conflict with Bob's addresses).
+
+### Method 1: Using Address Seeds
+
+When creating an account via `MsgInit`, you can provide an `address_seed`. The address is derived using:
+
+```bash
+address = sha256(ModuleName || address_seed || creator_address)
+```
+
+### Method 2: Using Account Numbers
+If no address seed is provided, the address is derived using:
+
+```
+address = sha256(ModuleName || creator_address || next_account_number)
+```
+
+### Address Seed Best Practices
+
+1. Address seeds must be unique per creator (not globally unique)
+2. Reusing an address seed will cause account creation to fail
+3. For programmatic account creation, use an incrementing sequence number as the address seed
+4. This is particularly useful for contracts or modules that need deterministic address generation
+
 ## Genesis
 
 ### Creating accounts on genesis

x/accounts/errors.go

@@ -10,4 +10,6 @@ var (
 	ErrBundlerPayment = errors.New(ModuleName, 2, "bundler payment failed")
 	// ErrExecution is returned when the execution fails.
 	ErrExecution = errors.New(ModuleName, 3, "execution failed")
+	// ErrAccountAlreadyExists is returned when the account already exists in state.
+	ErrAccountAlreadyExists = errors.New(ModuleName, 4, "account already exists")
 )

x/accounts/genesis_test.go

@@ -19,13 +19,13 @@ func TestGenesis(t *testing.T) {
 	// we init two accounts of the same type
 
 	// we set counter to 10
-	_, addr1, err := k.Init(ctx, testAccountType, []byte("sender"), &types.Empty{}, nil)
+	_, addr1, err := k.Init(ctx, testAccountType, []byte("sender"), &types.Empty{}, nil, nil)
 	require.NoError(t, err)
 	_, err = k.Execute(ctx, addr1, []byte("sender"), &types.UInt64Value{Value: 10}, nil)
 	require.NoError(t, err)
 
 	// we set counter to 20
-	_, addr2, err := k.Init(ctx, testAccountType, []byte("sender"), &types.Empty{}, nil)
+	_, addr2, err := k.Init(ctx, testAccountType, []byte("sender"), &types.Empty{}, nil, nil)
 	require.NoError(t, err)
 	_, err = k.Execute(ctx, addr2, []byte("sender"), &types.UInt64Value{Value: 20}, nil)
 	require.NoError(t, err)
@@ -62,7 +62,7 @@ func TestGenesis(t *testing.T) {
 	require.Equal(t, &types.UInt64Value{Value: 20}, resp)
 
 	// check initted on genesis account
-	addr3, err := k.makeAddress(2)
+	addr3, err := k.makeAddress([]byte("sender-2"), 2, nil)
 	require.NoError(t, err)
 	resp, err = k.Query(ctx, addr3, &types.DoubleValue{})
 	require.NoError(t, err)

x/accounts/keeper.go

@@ -148,14 +148,15 @@ func (k Keeper) Init(
 	creator []byte,
 	initRequest transaction.Msg,
 	funds sdk.Coins,
+	addressSeed []byte,
 ) (transaction.Msg, []byte, error) {
 	// get the next account number
 	num, err := k.AccountNumber.Next(ctx)
 	if err != nil {
 		return nil, nil, err
 	}
 	// create address
-	accountAddr, err := k.makeAddress(num)
+	accountAddr, err := k.makeAddress(creator, num, addressSeed)
 	if err != nil {
 		return nil, nil, err
 	}
@@ -180,7 +181,7 @@ func (k Keeper) initFromMsg(ctx context.Context, initMsg *v1.MsgInit) (transacti
 	}
 
 	// run account creation logic
-	return k.Init(ctx, initMsg.AccountType, creator, msg, initMsg.Funds)
+	return k.Init(ctx, initMsg.AccountType, creator, msg, initMsg.Funds, initMsg.AddressSeed)
 }
 
 // init initializes the account, given the type, the creator the newly created account number, its address and the
@@ -199,8 +200,17 @@ func (k Keeper) init(
 		return nil, fmt.Errorf("%w: not found %s", errAccountTypeNotFound, accountType)
 	}
 
+	// check if account exists
+	alreadyExists, err := k.AccountsByType.Has(ctx, accountAddr)
+	if err != nil {
+		return nil, err
+	}
+	if alreadyExists {
+		return nil, ErrAccountAlreadyExists
+	}
+
 	// send funds, if provided
-	err := k.maybeSendFunds(ctx, creator, accountAddr, funds)
+	err = k.maybeSendFunds(ctx, creator, accountAddr, funds)
 	if err != nil {
 		return nil, fmt.Errorf("unable to transfer funds: %w", err)
 	}
@@ -307,9 +317,26 @@ func (k Keeper) getImplementation(ctx context.Context, addr []byte) (implementat
 	return impl, nil
 }
 
-func (k Keeper) makeAddress(accNum uint64) ([]byte, error) {
-	// TODO: better address scheme, ref: https://github.com/cosmos/cosmos-sdk/issues/17516
-	addr := sha256.Sum256(append([]byte("x/accounts"), binary.BigEndian.AppendUint64(nil, accNum)...))
+// makeAddress creates an address for the given account.
+// It uses the creator address to ensure address squatting cannot happen, for example
+// assuming creator sends funds to a new account X nobody can front-run that address instantiation
+// unless the creator itself sends the tx.
+// AddressSeed can be used to create predictable addresses, security guarantees of the above are retained.
+// If address seed is not provided, the address is created using the creator and account number.
+func (k Keeper) makeAddress(creator []byte, accNum uint64, addressSeed []byte) ([]byte, error) {
+	// in case an address seed is provided, we use it to create the address.
+	var seed []byte
+	if len(addressSeed) > 0 {
+		seed = append(creator, addressSeed...)
+	} else {
+		// otherwise we use the creator and account number to create the address.
+		seed = append(creator, binary.BigEndian.AppendUint64(nil, accNum)...)
+	}
+
+	moduleAndSeed := append([]byte(ModuleName), seed...)
+
+	addr := sha256.Sum256(moduleAndSeed)
+
 	return addr[:], nil
 }
 

x/accounts/keeper_test.go

@@ -17,7 +17,7 @@ func TestKeeper_Init(t *testing.T) {
 	t.Run("ok", func(t *testing.T) {
 		sender := []byte("sender")
 
-		resp, addr, err := m.Init(ctx, "test", sender, &types.Empty{}, nil)
+		resp, addr, err := m.Init(ctx, "test", sender, &types.Empty{}, nil, nil)
 		require.NoError(t, err)
 		require.Equal(t, &types.Empty{}, resp)
 		require.NotNil(t, addr)
@@ -34,7 +34,7 @@ func TestKeeper_Init(t *testing.T) {
 	})
 
 	t.Run("unknown account type", func(t *testing.T) {
-		_, _, err := m.Init(ctx, "unknown", []byte("sender"), &types.Empty{}, nil)
+		_, _, err := m.Init(ctx, "unknown", []byte("sender"), &types.Empty{}, nil, nil)
 		require.ErrorIs(t, err, errAccountTypeNotFound)
 	})
 }
@@ -44,7 +44,7 @@ func TestKeeper_Execute(t *testing.T) {
 
 	// create account
 	sender := []byte("sender")
-	_, accAddr, err := m.Init(ctx, "test", sender, &types.Empty{}, nil)
+	_, accAddr, err := m.Init(ctx, "test", sender, &types.Empty{}, nil, nil)
 	require.NoError(t, err)
 
 	t.Run("ok", func(t *testing.T) {
@@ -70,7 +70,7 @@ func TestKeeper_Query(t *testing.T) {
 
 	// create account
 	sender := []byte("sender")
-	_, accAddr, err := m.Init(ctx, "test", sender, &types.Empty{}, nil)
+	_, accAddr, err := m.Init(ctx, "test", sender, &types.Empty{}, nil, nil)
 	require.NoError(t, err)
 
 	t.Run("ok", func(t *testing.T) {

x/accounts/msg_server_test.go

@@ -24,24 +24,46 @@ func TestMsgServer(t *testing.T) {
 		Sender:      "sender",
 		AccountType: "test",
 		Message:     initMsg,
+		AddressSeed: []byte("seed"),
 	})
 	require.NoError(t, err)
 	require.NotNil(t, initResp)
 
-	// execute
-	executeMsg := &wrapperspb.StringValue{
-		Value: "10",
-	}
-	executeMsgAny, err := implementation.PackAny(executeMsg)
-	require.NoError(t, err)
+	t.Run("success", func(t *testing.T) {
+		// execute
+		executeMsg := &wrapperspb.StringValue{
+			Value: "10",
+		}
+		executeMsgAny, err := implementation.PackAny(executeMsg)
+		require.NoError(t, err)
 
-	execResp, err := s.Execute(ctx, &v1.MsgExecute{
-		Sender:  "sender",
-		Target:  initResp.AccountAddress,
-		Message: executeMsgAny,
+		execResp, err := s.Execute(ctx, &v1.MsgExecute{
+			Sender:  "sender",
+			Target:  initResp.AccountAddress,
+			Message: executeMsgAny,
+		})
+		require.NoError(t, err)
+		require.NotNil(t, execResp)
+	})
+
+	t.Run("fail initting same account twice", func(t *testing.T) {
+		_, err := s.Init(ctx, &v1.MsgInit{
+			Sender:      "sender",
+			AccountType: "test",
+			Message:     initMsg,
+			AddressSeed: []byte("seed"),
+		})
+		require.ErrorIs(t, err, ErrAccountAlreadyExists)
+	})
+
+	t.Run("initting without seed", func(t *testing.T) {
+		_, err := s.Init(ctx, &v1.MsgInit{
+			Sender:      "sender",
+			AccountType: "test",
+			Message:     initMsg,
+		})
+		require.NoError(t, err)
 	})
-	require.NoError(t, err)
-	require.NotNil(t, execResp)
 }
 
 func TestMsgServer_BundlingDisabled(t *testing.T) {

x/accounts/proto/cosmos/accounts/v1/tx.proto

@@ -39,6 +39,9 @@ message MsgInit {
   // send alongside the request.
   repeated cosmos.base.v1beta1.Coin funds = 4
       [(gogoproto.castrepeated) = "github.com/cosmos/cosmos-sdk/types.Coins", (gogoproto.nullable) = false];
+  // address_seed can be used to deterministically create the address of the account.
+  // If not present the address will be generated based on its associated account number.
+  bytes address_seed = 5;
 }
 
 // MsgInitResponse defines the Create response type for the Msg/Create RPC method.