Skip to content

Commit

Permalink
fix address collision check logic
Browse files Browse the repository at this point in the history
  • Loading branch information
beer-1 committed Mar 28, 2024
1 parent 3c204bc commit 2b65076
Show file tree
Hide file tree
Showing 4 changed files with 43 additions and 14 deletions.
31 changes: 22 additions & 9 deletions x/evm/keeper/address.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 7 additions & 0 deletions x/evm/keeper/address_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
8 changes: 3 additions & 5 deletions x/evm/keeper/context.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
11 changes: 11 additions & 0 deletions x/evm/types/auth.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
Expand All @@ -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
}

0 comments on commit 2b65076

Please sign in to comment.