fix(vlocalchain): refuse to use existing account (#10247)

closes: #10246

## Description

Changes `VLOCALCHAIN_ALLOCATE_ADDRESS` to continue using pseudorandom inputs to generate a likely-unused account address, but increments the sequence number until the address does not correspond to an existing account.  Then, atomically, create a new account with that address.

This prevents localchain account addresses from clashing with preexisting accounts or ones that were created after the address was known but before any deposit activity.

### Security Considerations

Removes a race condition.

### Scaling Considerations

Scanning for existing accounts should not be expensive, because in order for it to be repeated indefinitely, the account addresses would need to be predicted, and the prior messages to create those clashing accounts would still require gas payments.

### Documentation Considerations

n/a

### Testing Considerations

Added tests for collision detection and account creation via mock AccountKeeper.

### Upgrade Considerations

n/a

golang/cosmos/app/app.go

@@ -655,6 +655,7 @@ func NewAgoricApp(
 		keys[vlocalchain.StoreKey],
 		app.BaseApp.MsgServiceRouter(),
 		app.BaseApp.GRPCQueryRouter(),
+		app.AccountKeeper,
 	)
 	app.vlocalchainPort = app.AgdServer.MustRegisterPortHandler(
 		"vlocalchain",

golang/cosmos/vm/action.go

@@ -86,7 +86,7 @@ func PopulateAction(ctx sdk.Context, action Action) Action {
 		var headerPtr *ActionHeader
 		if fieldType.Type == actionHeaderType {
 			headerPtr = field.Addr().Interface().(*ActionHeader)
-		} else if fieldType.Type == reflect.PtrTo(actionHeaderType) {
+		} else if fieldType.Type == reflect.PointerTo(actionHeaderType) {
 			if field.IsNil() {
 				headerPtr = &ActionHeader{}
 			} else {

golang/cosmos/x/vlocalchain/keeper/keeper.go

@@ -28,6 +28,7 @@ type Keeper struct {
 	cdc         codec.Codec
 	msgRouter   types.MsgRouter
 	queryRouter types.GRPCQueryRouter
+	acctKeeper  types.AccountKeeper
 }
 
 // NewKeeper creates a new dIBC Keeper instance
@@ -36,12 +37,14 @@ func NewKeeper(
 	key storetypes.StoreKey,
 	msgRouter types.MsgRouter,
 	queryRouter types.GRPCQueryRouter,
+	acctKeeper types.AccountKeeper,
 ) Keeper {
 	return Keeper{
 		key:         key,
 		cdc:         cdc,
 		msgRouter:   msgRouter,
 		queryRouter: queryRouter,
+		acctKeeper:  acctKeeper,
 	}
 }
 
@@ -266,14 +269,27 @@ func (k Keeper) AllocateAddress(cctx context.Context) sdk.AccAddress {
 	localchainModuleAcc := sdkaddress.Module(types.ModuleName, []byte("localchain"))
 	header := ctx.BlockHeader()
 
-	// Increment our sequence number.
-	seq := store.Get(types.KeyLastSequence)
-	seq = types.NextSequence(seq)
-	store.Set(types.KeyLastSequence, seq)
+	// Loop until we find an unused address.
+	for {
+		// Increment our sequence number.
+		seq := store.Get(types.KeyLastSequence)
+		seq = types.NextSequence(seq)
+		store.Set(types.KeyLastSequence, seq)
 
-	buf := seq
-	buf = append(buf, header.AppHash...)
-	buf = append(buf, header.DataHash...)
+		buf := seq
+		buf = append(buf, header.AppHash...)
+		buf = append(buf, header.DataHash...)
 
-	return sdkaddress.Derive(localchainModuleAcc, buf)
+		addr := sdkaddress.Derive(localchainModuleAcc, buf)
+		if k.acctKeeper.HasAccount(ctx, addr) {
+			continue
+		}
+
+		// We found an unused address, so create an account for it.
+		acct := k.acctKeeper.NewAccountWithAddress(ctx, addr)
+		k.acctKeeper.SetAccount(ctx, acct)
+
+		// All good, return the address.
+		return addr
+	}
 }

golang/cosmos/x/vlocalchain/keeper/keeper_test.go

@@ -10,7 +10,6 @@ import (
 	banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
 )
 
-
 func TestKeeper_ParseRequestTypeURL(t *testing.T) {
 	testCases := []struct {
 		name        string
@@ -46,7 +45,7 @@ func TestKeeper_DeserializeTxMessages(t *testing.T) {
 
 	banktypes.RegisterInterfaces(encodingConfig.InterfaceRegistry)
 
-	keeper := NewKeeper(cdc, nil, nil, nil)
+	keeper := NewKeeper(cdc, nil, nil, nil, nil)
 
 	expectedMsgSend := []sdk.Msg{
 		&banktypes.MsgSend{
@@ -63,22 +62,22 @@ func TestKeeper_DeserializeTxMessages(t *testing.T) {
 		wantErr  bool
 	}{
 		{
-			name: "camelCase keys",
-			json: `{"messages":[{"@type":"/cosmos.bank.v1beta1.MsgSend","fromAddress":"cosmos1abc","toAddress":"cosmos1xyz","amount":[{"denom":"stake","amount":"100"}]}]}`,
+			name:     "camelCase keys",
+			json:     `{"messages":[{"@type":"/cosmos.bank.v1beta1.MsgSend","fromAddress":"cosmos1abc","toAddress":"cosmos1xyz","amount":[{"denom":"stake","amount":"100"}]}]}`,
 			expected: expectedMsgSend,
-			wantErr: false,
+			wantErr:  false,
 		},
 		{
-			name: "snake_case keys",
-			json: `{"messages":[{"@type":"/cosmos.bank.v1beta1.MsgSend","from_address":"cosmos1abc","to_address":"cosmos1xyz","amount":[{"denom":"stake","amount":"100"}]}]}`,
+			name:     "snake_case keys",
+			json:     `{"messages":[{"@type":"/cosmos.bank.v1beta1.MsgSend","from_address":"cosmos1abc","to_address":"cosmos1xyz","amount":[{"denom":"stake","amount":"100"}]}]}`,
 			expected: expectedMsgSend,
-			wantErr: false,
+			wantErr:  false,
 		},
 		{
-			name: "misspelled key",
-			json: `{"messages":[{"@type":"/cosmos.bank.v1beta1.MsgSend","from_addresss":"cosmos1abc","to_address":"cosmos1xyz","amount":[{"denom":"stake","amount":"100"}]}]}`,
+			name:     "misspelled key",
+			json:     `{"messages":[{"@type":"/cosmos.bank.v1beta1.MsgSend","from_addresss":"cosmos1abc","to_address":"cosmos1xyz","amount":[{"denom":"stake","amount":"100"}]}]}`,
 			expected: expectedMsgSend,
-			wantErr: true,
+			wantErr:  true,
 		},
 	}
 

golang/cosmos/x/vlocalchain/types/expected_keepers.go

@@ -0,0 +1,12 @@
+package types
+
+import (
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
+)
+
+type AccountKeeper interface {
+	NewAccountWithAddress(ctx sdk.Context, addr sdk.AccAddress) authtypes.AccountI
+	HasAccount(ctx sdk.Context, addr sdk.AccAddress) bool
+	SetAccount(ctx sdk.Context, acc authtypes.AccountI)
+}

golang/cosmos/x/vlocalchain/vlocalchain_test.go

@@ -16,6 +16,7 @@ import (
 	"github.com/cosmos/cosmos-sdk/store"
 	storetypes "github.com/cosmos/cosmos-sdk/store/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
 	banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
 	stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"
 	transfertypes "github.com/cosmos/ibc-go/v6/modules/apps/transfer/types"
@@ -36,6 +37,25 @@ const (
 	msgAllocateAddress = `{"type":"VLOCALCHAIN_ALLOCATE_ADDRESS"}`
 )
 
+type mockAccounts struct {
+	existing map[string]bool
+}
+
+var _ types.AccountKeeper = (*mockAccounts)(nil)
+
+func (a *mockAccounts) NewAccountWithAddress(ctx sdk.Context, addr sdk.AccAddress) authtypes.AccountI {
+	return authtypes.NewBaseAccountWithAddress(addr)
+}
+
+func (a *mockAccounts) HasAccount(ctx sdk.Context, addr sdk.AccAddress) bool {
+	existing := a.existing[addr.String()]
+	return existing
+}
+
+func (a *mockAccounts) SetAccount(ctx sdk.Context, acc authtypes.AccountI) {
+	a.existing[acc.GetAddress().String()] = true
+}
+
 type mockBank struct {
 	banktypes.UnimplementedQueryServer
 	banktypes.UnimplementedMsgServer
@@ -108,7 +128,7 @@ func (s *mockStaking) UnbondingDelegation(cctx context.Context, req *stakingtype
 }
 
 // makeTestKit creates a minimal Keeper and Context for use in testing.
-func makeTestKit(bank *mockBank, transfer *mockTransfer, staking *mockStaking) (vm.PortHandler, context.Context) {
+func makeTestKit(bank *mockBank, transfer *mockTransfer, staking *mockStaking, accts *mockAccounts) (vm.PortHandler, context.Context) {
 	encodingConfig := params.MakeEncodingConfig()
 	cdc := encodingConfig.Marshaler
 
@@ -127,9 +147,8 @@ func makeTestKit(bank *mockBank, transfer *mockTransfer, staking *mockStaking) (
 	stakingtypes.RegisterMsgServer(txRouter, staking)
 	stakingtypes.RegisterQueryServer(queryRouter, staking)
 
-
 	// create a new Keeper
-	keeper := vlocalchain.NewKeeper(cdc, vlocalchainStoreKey, txRouter, queryRouter)
+	keeper := vlocalchain.NewKeeper(cdc, vlocalchainStoreKey, txRouter, queryRouter, accts)
 
 	db := dbm.NewMemDB()
 	ms := store.NewCommitMultiStore(db)
@@ -153,13 +172,15 @@ func TestAllocateAddress(t *testing.T) {
 	bank := &mockBank{}
 	transfer := &mockTransfer{}
 	staking := &mockStaking{}
-	handler, cctx := makeTestKit(bank, transfer, staking)
+	acct := &mockAccounts{existing: map[string]bool{
+		firstAddr: true,
+		"cosmos1c5hplwyxk5jr2dsygjqepzfqvfukwduq9c4660aah76krf99m6gs0k7hvl": true,
+	}}
+	handler, cctx := makeTestKit(bank, transfer, staking, acct)
 
 	addrs := map[string]bool{
-		firstAddr: false,
 		"cosmos1yj40fakym8kf4wvgz9tky7k9f3v9msm3t7frscrmkjsdkxkpsfkqgeczkg": false,
 		"cosmos1s76vryj7m8k8nm9le65a4plhf5rym5sumtt2n0vwnk5l6k4cwuhsj56ujj": false,
-		"cosmos1c5hplwyxk5jr2dsygjqepzfqvfukwduq9c4660aah76krf99m6gs0k7hvl": false,
 		"cosmos1ys3a7mtna3cad0wxcs4ddukn37stexjdvns8jfdn4uerlr95y4xqnrypf6": false,
 	}
 	numToTest := len(addrs)
@@ -185,6 +206,9 @@ func TestAllocateAddress(t *testing.T) {
 			t.Fatalf("unexpected duplicate address[%d]: %v", i, addr)
 		}
 		addrs[addr] = true
+		if !acct.existing[addr] {
+			t.Fatalf("expected address[%d]: %v to be added to accounts", i, addr)
+		}
 	}
 }
 
@@ -197,7 +221,8 @@ func TestQuery(t *testing.T) {
 	}}
 	transfer := &mockTransfer{}
 	staking := &mockStaking{}
-	handler, cctx := makeTestKit(bank, transfer, staking)
+	accts := &mockAccounts{existing: map[string]bool{}}
+	handler, cctx := makeTestKit(bank, transfer, staking, accts)
 
 	// get balances
 	testCases := []struct {
@@ -338,7 +363,8 @@ func TestExecuteTx(t *testing.T) {
 	}}
 	transfer := &mockTransfer{}
 	staking := &mockStaking{}
-	handler, cctx := makeTestKit(bank, transfer, staking)
+	accts := &mockAccounts{existing: map[string]bool{}}
+	handler, cctx := makeTestKit(bank, transfer, staking, accts)
 
 	// create a new message
 	msg := `{"type":"VLOCALCHAIN_ALLOCATE_ADDRESS"}`
@@ -426,7 +452,7 @@ func TestExecuteTx(t *testing.T) {
 		if len(resp) != 1 {
 			t.Fatalf("expected 1 response, got %d", len(resp))
 		}
-		
+
 		if _, ok := resp[0]["completionTime"]; !ok {
 			t.Error("expected 'completionTime' field in response")
 		}