From 2b65076b06e106591638d5b98d0cdf2f16b1f672 Mon Sep 17 00:00:00 2001
From: beer-1 <147697694+beer-1@users.noreply.github.com>
Date: Thu, 28 Mar 2024 13:21:11 +0900
Subject: [PATCH] fix address collision check logic

---
 x/evm/keeper/address.go      | 31 ++++++++++++++++++++++---------
 x/evm/keeper/address_test.go |  7 +++++++
 x/evm/keeper/context.go      |  8 +++-----
 x/evm/types/auth.go          | 11 +++++++++++
 4 files changed, 43 insertions(+), 14 deletions(-)

diff --git a/x/evm/keeper/address.go b/x/evm/keeper/address.go
index 0e5e649..141dbb8 100644
--- a/x/evm/keeper/address.go
+++ b/x/evm/keeper/address.go
@@ -18,19 +18,32 @@ func (k Keeper) convertToEVMAddress(ctx context.Context, addr sdk.AccAddress) (c
 
 	shorthandAddr := common.BytesToAddress(addr.Bytes())
 	if found := k.accountKeeper.HasAccount(ctx, shorthandAddr.Bytes()); found {
-		existingAccount := k.accountKeeper.GetAccount(ctx, shorthandAddr.Bytes())
-		shorthandAccount, isShorthandAccount := existingAccount.(types.ShorthandAccountI)
-		if !isShorthandAccount {
-			return common.Address{}, types.ErrAddressAlreadyExists.Wrapf("failed to create shorthand account: %s", shorthandAddr)
+		account := k.accountKeeper.GetAccount(ctx, shorthandAddr.Bytes())
+
+		// if the account is empty account, convert it to shorthand account
+		if types.IsEmptyAccount(account) {
+			shorthandAccount, err := types.NewShorthandAccountWithAddress(k.ac, addr)
+			if err != nil {
+				return common.Address{}, err
+			}
+
+			shorthandAccount.AccountNumber = account.GetAccountNumber()
+			k.accountKeeper.SetAccount(ctx, shorthandAccount)
+
+			return shorthandAddr, nil
 		}
 
-		if originAddr, err := shorthandAccount.GetOriginalAddress(k.ac); err != nil {
-			return common.Address{}, err
-		} else if !originAddr.Equals(addr) {
-			return common.Address{}, types.ErrAddressAlreadyExists.Wrapf("failed to create shorthand account: %s", shorthandAddr)
+		// check if the account is shorthand account, and if so, check if the original address is the same
+		shorthandAccount, isShorthandAccount := account.(types.ShorthandAccountI)
+		if isShorthandAccount {
+			if originAddr, err := shorthandAccount.GetOriginalAddress(k.ac); err != nil {
+				return common.Address{}, err
+			} else if originAddr.Equals(addr) {
+				return shorthandAddr, nil
+			}
 		}
 
-		return shorthandAddr, nil
+		return common.Address{}, types.ErrAddressAlreadyExists.Wrapf("failed to create shorthand account of `%s`: `%s`", addr, shorthandAddr)
 	}
 
 	// create shorthand account
diff --git a/x/evm/keeper/address_test.go b/x/evm/keeper/address_test.go
index 5f2860e..b1f4095 100644
--- a/x/evm/keeper/address_test.go
+++ b/x/evm/keeper/address_test.go
@@ -51,4 +51,11 @@ func Test_AllowLongCosmosAddress(t *testing.T) {
 		sdk.NewCoin(fooDenom, math.NewInt(50)),
 	))
 	require.ErrorContains(t, err, types.ErrAddressAlreadyExists.Error())
+
+	// but still can send to the same address
+	err = erc20Keeper.SendCoins(ctx, addr, addr3, sdk.NewCoins(
+		sdk.NewCoin("bar", math.NewInt(100)),
+		sdk.NewCoin(fooDenom, math.NewInt(50)),
+	))
+	require.NoError(t, err)
 }
diff --git a/x/evm/keeper/context.go b/x/evm/keeper/context.go
index a778e9d..ffa08ef 100644
--- a/x/evm/keeper/context.go
+++ b/x/evm/keeper/context.go
@@ -114,15 +114,13 @@ func (k Keeper) contractCreatedHook(ctx context.Context) vm.ContractCreatedHook
 	return func(contractAddr common.Address) error {
 		if k.accountKeeper.HasAccount(ctx, sdk.AccAddress(contractAddr.Bytes())) {
 			account := k.accountKeeper.GetAccount(ctx, sdk.AccAddress(contractAddr.Bytes()))
-			_, isModuleAccount := account.(sdk.ModuleAccountI)
-			_, isShorthandAccount := account.(types.ShorthandAccountI)
 
-			// contract account collision should be check in evm side.
-			if isModuleAccount || isShorthandAccount || account.GetPubKey() != nil {
+			// check the account is empty or not
+			if !types.IsEmptyAccount(account) {
 				return types.ErrAddressAlreadyExists.Wrap(contractAddr.String())
 			}
 
-			// convert normal account to contract account only if this account is empty
+			// convert base account to contract account only if this account is empty
 			contractAccount := types.NewContractAccountWithAddress(contractAddr.Bytes())
 			contractAccount.AccountNumber = account.GetAccountNumber()
 			k.accountKeeper.SetAccount(ctx, contractAccount)
diff --git a/x/evm/types/auth.go b/x/evm/types/auth.go
index b216540..3fab9a6 100644
--- a/x/evm/types/auth.go
+++ b/x/evm/types/auth.go
@@ -32,6 +32,7 @@ func (ca ContractAccount) SetPubKey(pubKey cryptotypes.PubKey) error {
 	return fmt.Errorf("not supported for contract accounts")
 }
 
+// ShorthandAccountI is an interface for shorthand account.
 type ShorthandAccountI interface {
 	GetOriginalAddress(ac address.Codec) (sdk.AccAddress, error)
 }
@@ -55,6 +56,16 @@ func (sa ShorthandAccount) SetPubKey(pubKey cryptotypes.PubKey) error {
 	return fmt.Errorf("not supported for shorthand accounts")
 }
 
+// GetOriginalAddress returns the original address.
 func (sa ShorthandAccount) GetOriginalAddress(ac address.Codec) (sdk.AccAddress, error) {
 	return ac.StringToBytes(sa.OriginalAddress)
 }
+
+// IsEmptyAccount checks if the account is empty.
+func IsEmptyAccount(account sdk.AccountI) bool {
+	_, isModuleAccount := account.(sdk.ModuleAccountI)
+	_, isShorthandAccount := account.(ShorthandAccountI)
+	_, isContractAccount := account.(*ContractAccount)
+
+	return !isModuleAccount && !isShorthandAccount && !isContractAccount && account.GetPubKey() == nil
+}