fix(evm): improve safety of ERC20 transfers, accounting for boolean success return values and recipient balance changes that don't match the ERC20 transfer amount.

CHANGELOG.md

@@ -135,6 +135,7 @@ reflected on all occurences of "base fee" in the codebase. This fixes [#2059](ht
 and the [related comments from @Unique-Divine and @berndartmueller](https://github.com/NibiruChain/nibiru/issues/2059#issuecomment-2408625724).
 - [#2084](https://github.com/NibiruChain/nibiru/pull/2084) - feat(evm-forge): foundry support and template for Nibiru EVM develoment
 - [#2088](https://github.com/NibiruChain/nibiru/pull/2088) - refactor(evm): remove outdated comment and improper error message text
+- [#2090](https://github.com/NibiruChain/nibiru/pull/2090) - fix(evm): transfer for native to erc20 for specific tokens
 
 
 #### Dapp modules: perp, spot, oracle, etc

x/evm/evmmodule/genesis_test.go

@@ -54,11 +54,11 @@ func (s *Suite) TestExportInitGenesis() {
 	s.Require().NoError(err)
 
 	// Transfer ERC-20 tokens to user A
-	_, err = deps.EvmKeeper.ERC20().Transfer(erc20Addr, fromUser, toUserA, amountToSendA, deps.Ctx)
+	_, err, _ = deps.EvmKeeper.ERC20().Transfer(erc20Addr, fromUser, toUserA, amountToSendA, deps.Ctx)
 	s.Require().NoError(err)
 
 	// Transfer ERC-20 tokens to user B
-	_, err = deps.EvmKeeper.ERC20().Transfer(erc20Addr, fromUser, toUserB, amountToSendB, deps.Ctx)
+	_, err, _ = deps.EvmKeeper.ERC20().Transfer(erc20Addr, fromUser, toUserB, amountToSendB, deps.Ctx)
 	s.Require().NoError(err)
 
 	// Create fungible token from bank coin

x/evm/keeper/erc20.go

@@ -73,23 +73,53 @@ Transfer implements "ERC20.transfer"
 func (e erc20Calls) Transfer(
 	contract, from, to gethcommon.Address, amount *big.Int,
 	ctx sdk.Context,
-) (out bool, err error) {
+) (success bool, err error, received *big.Int) {
+	received = big.NewInt(0)
+
+	recipientBalanceBefore, err := e.BalanceOf(contract, to, ctx)
+	if err != nil {
+		return false, errors.Wrap(err, "failed to retrieve recipient balance"), received
+	}
+
 	input, err := e.ABI.Pack("transfer", to, amount)
 	if err != nil {
-		return false, fmt.Errorf("failed to pack ABI args: %w", err)
+		return false, fmt.Errorf("failed to pack ABI args: %w", err), received
 	}
 	resp, err := e.CallContractWithInput(ctx, from, &contract, true, input)
 	if err != nil {
-		return false, err
+		return false, err, received
+	}
+
+	recipientBalanceAfter, err := e.BalanceOf(contract, to, ctx)
+	if err != nil {
+		return false, errors.Wrap(err, "failed to retrieve recipient balance"), received
+	}
+
+	received = new(big.Int).Sub(recipientBalanceAfter, recipientBalanceBefore)
+
+	// we can't check that received = amount because the recipient could have
+	// a transfer fee or other deductions. We can only check that the recipient
+	// received some tokens
+	if received.Sign() <= 0 {
+		return false, fmt.Errorf("no (or negative) ERC20 tokens were received by the recipient"), received
 	}
 
 	var erc20Bool ERC20Bool
 	err = e.ABI.UnpackIntoInterface(&erc20Bool, "transfer", resp.Ret)
-	if err != nil {
-		return false, err
+
+	// per erc20 standard, the transfer function should return a boolean value
+	// indicating whether the operation succeeded. If the unpacking failed, we
+	// need to check the recipient balance to determine if the transfer was successful.
+	if err == nil {
+		// should be true if the transfer was successful but we do it anyway
+		// to respect the contract's return value
+		success = erc20Bool.Value
+
+		return success, nil, received
 	}
 
-	return erc20Bool.Value, nil
+	success = true
+	return
 }
 
 // BalanceOf retrieves the balance of an ERC20 token for a specific account.

x/evm/keeper/erc20_test.go

@@ -31,19 +31,21 @@ func (s *Suite) TestERC20Calls() {
 
 	s.T().Log("Transfer - Not enough funds")
 	{
-		_, err := deps.EvmKeeper.ERC20().Transfer(contract, deps.Sender.EthAddr, evm.EVM_MODULE_ADDRESS, big.NewInt(9_420), deps.Ctx)
+		_, err, received := deps.EvmKeeper.ERC20().Transfer(contract, deps.Sender.EthAddr, evm.EVM_MODULE_ADDRESS, big.NewInt(9_420), deps.Ctx)
 		s.ErrorContains(err, "ERC20: transfer amount exceeds balance")
 		// balances unchanged
 		evmtest.AssertERC20BalanceEqual(s.T(), deps, contract, deps.Sender.EthAddr, big.NewInt(0))
 		evmtest.AssertERC20BalanceEqual(s.T(), deps, contract, evm.EVM_MODULE_ADDRESS, big.NewInt(69_420))
+		s.Require().Equal(received, big.NewInt(0))
 	}
 
 	s.T().Log("Transfer - Success (sanity check)")
 	{
-		_, err := deps.EvmKeeper.ERC20().Transfer(contract, evm.EVM_MODULE_ADDRESS, deps.Sender.EthAddr, big.NewInt(9_420), deps.Ctx)
+		_, err, received := deps.EvmKeeper.ERC20().Transfer(contract, evm.EVM_MODULE_ADDRESS, deps.Sender.EthAddr, big.NewInt(9_420), deps.Ctx)
 		s.Require().NoError(err)
 		evmtest.AssertERC20BalanceEqual(s.T(), deps, contract, deps.Sender.EthAddr, big.NewInt(9_420))
 		evmtest.AssertERC20BalanceEqual(s.T(), deps, contract, evm.EVM_MODULE_ADDRESS, big.NewInt(60_000))
+		s.Require().Equal(received, big.NewInt(9_420))
 	}
 
 	s.T().Log("Burn tokens - Allowed as non-owner")

x/evm/keeper/msg_server.go

@@ -590,14 +590,6 @@ func (k Keeper) convertCoinNativeERC20(
 ) (*evm.MsgConvertCoinToEvmResponse, error) {
 	erc20Addr := funTokenMapping.Erc20Addr.Address
 
-	recipientBalanceBefore, err := k.ERC20().BalanceOf(erc20Addr, recipient, ctx)
-	if err != nil {
-		return nil, errors.Wrap(err, "failed to retrieve balance")
-	}
-	if recipientBalanceBefore == nil {
-		return nil, fmt.Errorf("failed to retrieve balance, balance is nil")
-	}
-
 	// Escrow Coins on module account
 	if err := k.bankKeeper.SendCoinsFromAccountToModule(
 		ctx,
@@ -619,52 +611,36 @@ func (k Keeper) convertCoinNativeERC20(
 		return nil, errors.Wrap(err, "failed to retrieve balance")
 	}
 	if evmModuleBalance == nil {
-		return nil, fmt.Errorf("failed to retrieve balance, balance is nil")
+		return nil, fmt.Errorf("failed to retrieve EVM module account balance, balance is nil")
 	}
 	if evmModuleBalance.Cmp(coin.Amount.BigInt()) < 0 {
 		return nil, fmt.Errorf("insufficient balance in EVM module account")
 	}
 
 	// unescrow ERC-20 tokens from EVM module address
-	res, err := k.ERC20().Transfer(
+	success, err, actualReceivedAmount := k.ERC20().Transfer(
 		erc20Addr,
 		evm.EVM_MODULE_ADDRESS,
 		recipient,
 		coin.Amount.BigInt(),
 		ctx,
 	)
-	if err != nil {
-		return nil, errors.Wrap(err, "failed to transfer ERC20 tokens")
-	}
-	if !res {
-		return nil, fmt.Errorf("failed to transfer ERC20 tokens")
+	if err != nil || !success {
+		return nil, errors.Wrap(err, "failed to transfer ERC-20 tokens")
 	}
 
-	// Check expected Receiver balance after transfer execution
-	recipientBalanceAfter, err := k.ERC20().BalanceOf(erc20Addr, recipient, ctx)
-	if err != nil {
-		return nil, errors.Wrap(err, "failed to retrieve balance")
-	}
-	if recipientBalanceAfter == nil {
-		return nil, fmt.Errorf("failed to retrieve balance, balance is nil")
-	}
-
-	expectedFinalBalance := big.NewInt(0).Add(recipientBalanceBefore, coin.Amount.BigInt())
-	if r := recipientBalanceAfter.Cmp(expectedFinalBalance); r != 0 {
-		return nil, fmt.Errorf("expected balance after transfer to be %s, got %s", expectedFinalBalance, recipientBalanceAfter)
-	}
-
-	// Burn escrowed Coins
-	err = k.bankKeeper.BurnCoins(ctx, evm.ModuleName, sdk.NewCoins(coin))
+	burnCoin := sdk.NewCoin(coin.Denom, sdk.NewIntFromBigInt(actualReceivedAmount))
+	err = k.bankKeeper.BurnCoins(ctx, evm.ModuleName, sdk.NewCoins(burnCoin))
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to burn coins")
 	}
 
+	// Emit event with the actual amount received
 	_ = ctx.EventManager().EmitTypedEvent(&evm.EventConvertCoinToEvm{
 		Sender:               sender.String(),
 		Erc20ContractAddress: funTokenMapping.Erc20Addr.String(),
 		ToEthAddr:            recipient.String(),
-		BankCoin:             coin,
+		BankCoin:             burnCoin,
 	})
 
 	return &evm.MsgConvertCoinToEvmResponse{}, nil

x/evm/precompile/funtoken.go

@@ -159,7 +159,7 @@ func (p precompileFunToken) bankSend(
 
 	// Caller transfers ERC20 to the EVM account
 	transferTo := evm.EVM_MODULE_ADDRESS
-	_, err = p.evmKeeper.ERC20().Transfer(erc20, caller, transferTo, amount, ctx)
+	_, err, _ = p.evmKeeper.ERC20().Transfer(erc20, caller, transferTo, amount, ctx)
 	if err != nil {
 		return nil, fmt.Errorf("failed to send from caller to the EVM account: %w", err)
 	}