fix: make the FunToken precompile return the sent amount

CHANGELOG.md

@@ -162,7 +162,6 @@ about the expected resulting balance for the transfer recipient.
 - [#2060](https://github.com/NibiruChain/nibiru/pull/2060) - fix(evm-precompiles): add assertNumArgs validation
 - [#2056](https://github.com/NibiruChain/nibiru/pull/2056) - feat(evm): add oracle precompile
 - [#2065](https://github.com/NibiruChain/nibiru/pull/2065) - refactor(evm)!: Refactor out dead code from the evm.Params
-- [#2073](https://github.com/NibiruChain/nibiru/pull/2073) - fix(evm-keeper): better utilize ERC20 metadata during FunToken creation
 
 ### State Machine Breaking (Other)
 

x/evm/embeds/artifacts/contracts/IFunToken.sol/IFunToken.json

@@ -22,7 +22,13 @@
         }
       ],
       "name": "bankSend",
-      "outputs": [],
+      "outputs": [
+        {
+          "internalType": "uint256",
+          "name": "sentAmount",
+          "type": "uint256"
+        }
+      ],
       "stateMutability": "nonpayable",
       "type": "function"
     }

x/evm/embeds/contracts/IFunToken.sol

@@ -5,11 +5,18 @@ pragma solidity >=0.8.19;
 /// coins to a Nibiru bech32 address using the "FunToken" mapping between the
 /// ERC20 and bank.
 interface IFunToken {
-  /// @dev bankSend sends ERC20 tokens as coins to a Nibiru base account
-  /// @param erc20 the address of the ERC20 token contract
-  /// @param amount the amount of tokens to send
-  /// @param to the receiving Nibiru base account address as a string
-  function bankSend(address erc20, uint256 amount, string memory to) external;
+    /// @dev bankSend sends ERC20 tokens as coins to a Nibiru base account
+    /// @param erc20 - the address of the ERC20 token contract
+    /// @param amount - the amount of tokens to send
+    /// @param to - the receiving Nibiru base account address as a string
+    /// @return sentAmount - amount of tokens received by the recipient. This may
+    /// not be equal to `amount` if the corresponding ERC20 contract has a fee or
+    /// deduction on transfer.
+    function bankSend(
+        address erc20,
+        uint256 amount,
+        string memory to
+    ) external returns (uint256 sentAmount);
 }
 
 address constant FUNTOKEN_PRECOMPILE_ADDRESS = 0x0000000000000000000000000000000000000800;

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,53 +73,53 @@ Transfer implements "ERC20.transfer"
 func (e erc20Calls) Transfer(
 	contract, from, to gethcommon.Address, amount *big.Int,
 	ctx sdk.Context,
-) (success bool, err error, received *big.Int) {
-	received = big.NewInt(0)
-
+) (balanceIncrease *big.Int, err error) {
 	recipientBalanceBefore, err := e.BalanceOf(contract, to, ctx)
 	if err != nil {
-		return false, errors.Wrap(err, "failed to retrieve recipient balance"), received
+		return balanceIncrease, errors.Wrap(err, "failed to retrieve recipient balance")
 	}
 
 	input, err := e.ABI.Pack("transfer", to, amount)
 	if err != nil {
-		return false, fmt.Errorf("failed to pack ABI args: %w", err), received
+		return balanceIncrease, fmt.Errorf("failed to pack ABI args: %w", err)
 	}
+
 	resp, _, err := e.CallContractWithInput(ctx, from, &contract, true, input)
 	if err != nil {
-		return false, err, received
+		return balanceIncrease, err
 	}
 
-	recipientBalanceAfter, err := e.BalanceOf(contract, to, ctx)
+	var erc20Bool ERC20Bool
+	err = e.ABI.UnpackIntoInterface(&erc20Bool, "transfer", resp.Ret)
 	if err != nil {
-		return false, errors.Wrap(err, "failed to retrieve recipient balance"), received
+		return balanceIncrease, err
 	}
 
-	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
+	// Handle the case of success=false: https://github.com/NibiruChain/nibiru/issues/2080
+	success := erc20Bool.Value
+	if !success {
+		return balanceIncrease, fmt.Errorf("transfer executed but returned success=false")
 	}
 
-	var erc20Bool ERC20Bool
-	err = e.ABI.UnpackIntoInterface(&erc20Bool, "transfer", resp.Ret)
-
-	// 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
+	recipientBalanceAfter, err := e.BalanceOf(contract, to, ctx)
+	if err != nil {
+		return balanceIncrease, errors.Wrap(err, "failed to retrieve recipient balance")
+	}
 
-		return success, nil, received
+	balanceIncrease = new(big.Int).Sub(recipientBalanceAfter, recipientBalanceBefore)
+
+	// For flexibility with fee on transfer tokens and other types of deductions,
+	// we cannot assume that the amount received by the recipient is equal to
+	// the call "amount". Instead, verify that the recipient got tokens and
+	// return the amount.
+	if balanceIncrease.Sign() <= 0 {
+		return balanceIncrease, fmt.Errorf(
+			"amount of ERC20 tokens received MUST be positive: the balance of recipient %s would've changed by %v for token %s",
+			to.Hex(), balanceIncrease.String(), contract.Hex(),
+		)
 	}
 
-	success = true
-	return
+	return balanceIncrease, err
 }
 
 // BalanceOf retrieves the balance of an ERC20 token for a specific account.

x/evm/keeper/erc20_test.go

@@ -34,21 +34,26 @@ func (s *Suite) TestERC20Calls() {
 
 	s.T().Log("Transfer - Not enough funds")
 	{
-		_, err, received := deps.EvmKeeper.ERC20().Transfer(contract, deps.Sender.EthAddr, evm.EVM_MODULE_ADDRESS, big.NewInt(9_420), deps.Ctx)
+		amt := big.NewInt(9_420)
+		_, err := deps.EvmKeeper.ERC20().Transfer(contract, deps.Sender.EthAddr, evm.EVM_MODULE_ADDRESS, amt, 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, received := deps.EvmKeeper.ERC20().Transfer(contract, evm.EVM_MODULE_ADDRESS, deps.Sender.EthAddr, big.NewInt(9_420), deps.Ctx)
+		amt := big.NewInt(9_420)
+		sentAmt, err := deps.EvmKeeper.ERC20().Transfer(
+			contract, evm.EVM_MODULE_ADDRESS, deps.Sender.EthAddr, amt, 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))
+		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(sentAmt.String(), amt.String())
 	}
 
 	s.T().Log("Burn tokens - Allowed as non-owner")

x/evm/keeper/msg_server.go

@@ -491,30 +491,31 @@ func (k *Keeper) ConvertCoinToEvm(
 	fungibleTokenMapping := funTokens[0]
 
 	if fungibleTokenMapping.IsMadeFromCoin {
-		return k.convertCoinNativeCoin(ctx, sender, msg.ToEthAddr.Address, msg.BankCoin, fungibleTokenMapping)
+		return k.convertCoinToEvmBornCoin(ctx, sender, msg.ToEthAddr.Address, msg.BankCoin, fungibleTokenMapping)
 	} else {
-		return k.convertCoinNativeERC20(ctx, sender, msg.ToEthAddr.Address, msg.BankCoin, fungibleTokenMapping)
+		return k.convertCoinToEvmBornERC20(ctx, sender, msg.ToEthAddr.Address, msg.BankCoin, fungibleTokenMapping)
 	}
 }
 
-// Converts a native coin to an ERC20 token.
-// EVM module owns the ERC-20 contract and can mint the ERC-20 tokens.
-func (k Keeper) convertCoinNativeCoin(
+// Converts Bank Coins for FunToken mapping that was born from a coin
+// (IsMadeFromCoin=true) into the ERC20 tokens. EVM module owns the ERC-20
+// contract and can mint the ERC-20 tokens.
+func (k Keeper) convertCoinToEvmBornCoin(
 	ctx sdk.Context,
 	sender sdk.AccAddress,
 	recipient gethcommon.Address,
 	coin sdk.Coin,
 	funTokenMapping evm.FunToken,
 ) (*evm.MsgConvertCoinToEvmResponse, error) {
-	// Step 1: Escrow bank coins with EVM module account
+	// Step 1: Send Bank Coins to the EVM module
 	err := k.bankKeeper.SendCoinsFromAccountToModule(ctx, sender, evm.ModuleName, sdk.NewCoins(coin))
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to send coins to module account")
 	}
 
 	erc20Addr := funTokenMapping.Erc20Addr.Address
 
-	// Step 2: mint ERC-20 tokens for recipient
+	// Step 2: Mint ERC20 tokens to the recipient
 	evmResp, err := k.CallContract(
 		ctx,
 		embeds.SmartContract_ERC20Minter.ABI,
@@ -542,10 +543,11 @@ func (k Keeper) convertCoinNativeCoin(
 	return &evm.MsgConvertCoinToEvmResponse{}, nil
 }
 
-// Converts a coin that was originally an ERC20 token, and that was converted to a bank coin, back to an ERC20 token.
-// EVM module does not own the ERC-20 contract and cannot mint the ERC-20 tokens.
-// EVM module has escrowed tokens in the first conversion from ERC-20 to bank coin.
-func (k Keeper) convertCoinNativeERC20(
+// Converts a coin that was originally an ERC20 token, and that was converted to
+// a bank coin, back to an ERC20 token. EVM module does not own the ERC-20
+// contract and cannot mint the ERC-20 tokens. EVM module has escrowed tokens in
+// the first conversion from ERC-20 to bank coin.
+func (k Keeper) convertCoinToEvmBornERC20(
 	ctx sdk.Context,
 	sender sdk.AccAddress,
 	recipient gethcommon.Address,
@@ -581,19 +583,19 @@ func (k Keeper) convertCoinNativeERC20(
 		return nil, fmt.Errorf("insufficient balance in EVM module account")
 	}
 
-	// unescrow ERC-20 tokens from EVM module address
-	success, err, actualReceivedAmount := k.ERC20().Transfer(
+	// Supply ERC-20 tokens from EVM module address
+	actualSentAmount, err := k.ERC20().Transfer(
 		erc20Addr,
 		evm.EVM_MODULE_ADDRESS,
 		recipient,
 		coin.Amount.BigInt(),
 		ctx,
 	)
-	if err != nil || !success {
+	if err != nil {
 		return nil, errors.Wrap(err, "failed to transfer ERC-20 tokens")
 	}
 
-	burnCoin := sdk.NewCoin(coin.Denom, sdk.NewIntFromBigInt(actualReceivedAmount))
+	burnCoin := sdk.NewCoin(coin.Denom, sdk.NewIntFromBigInt(actualSentAmount))
 	err = k.bankKeeper.BurnCoins(ctx, evm.ModuleName, sdk.NewCoins(burnCoin))
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to burn coins")

x/evm/precompile/funtoken.go

@@ -143,20 +143,19 @@ 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)
+	gotAmount, 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)
+		return nil, fmt.Errorf("error in ERC20.transfer from caller to EVM account: %w", err)
 	}
 
 	// EVM account mints FunToken.BankDenom to module account
-	amt := math.NewIntFromBigInt(amount)
-	coinToSend := sdk.NewCoin(funtoken.BankDenom, amt)
+	coinToSend := sdk.NewCoin(funtoken.BankDenom, math.NewIntFromBigInt(gotAmount))
 	if funtoken.IsMadeFromCoin {
 		// If the FunToken mapping was created from a bank coin, then the EVM account
 		// owns the ERC20 contract and was the original minter of the ERC20 tokens.
 		// Since we're sending them away and want accurate total supply tracking, the
 		// tokens need to be burned.
-		_, err = p.evmKeeper.ERC20().Burn(erc20, evm.EVM_MODULE_ADDRESS, amount, ctx)
+		_, err = p.evmKeeper.ERC20().Burn(erc20, evm.EVM_MODULE_ADDRESS, gotAmount, ctx)
 		if err != nil {
 			err = fmt.Errorf("ERC20.Burn: %w", err)
 			return
@@ -187,7 +186,7 @@ func (p precompileFunToken) bankSend(
 
 	// TODO: UD-DEBUG: feat: Emit EVM events
 
-	return method.Outputs.Pack()
+	return method.Outputs.Pack(gotAmount)
 }
 
 func SafeMintCoins(

x/evm/precompile/funtoken_test.go

@@ -126,18 +126,32 @@ func (s *FuntokenSuite) TestHappyPath() {
 	input, err := embeds.SmartContract_FunToken.ABI.Pack(string(precompile.FunTokenMethod_BankSend), callArgs...)
 	s.NoError(err)
 
-	_, resp, err := evmtest.CallContractTx(
+	_, ethTxResp, err := evmtest.CallContractTx(
 		&deps,
 		precompile.PrecompileAddr_FunToken,
 		input,
 		deps.Sender,
 	)
 	s.Require().NoError(err)
-	s.Require().Empty(resp.VmError)
+	s.Require().Empty(ethTxResp.VmError)
 
-	evmtest.AssertERC20BalanceEqual(s.T(), deps, erc20, deps.Sender.EthAddr, big.NewInt(69_000))
-	evmtest.AssertERC20BalanceEqual(s.T(), deps, erc20, evm.EVM_MODULE_ADDRESS, big.NewInt(0))
+	evmtest.AssertERC20BalanceEqual(
+		s.T(), deps, erc20, deps.Sender.EthAddr, big.NewInt(69_000),
+	)
+	evmtest.AssertERC20BalanceEqual(
+		s.T(), deps, erc20, evm.EVM_MODULE_ADDRESS, big.NewInt(0),
+	)
 	s.Equal(sdk.NewInt(420).String(),
 		deps.App.BankKeeper.GetBalance(deps.Ctx, randomAcc, funtoken.BankDenom).Amount.String(),
 	)
+
+	s.T().Log("Parse the response contract addr and response bytes")
+	var sentAmt *big.Int
+	err = embeds.SmartContract_FunToken.ABI.UnpackIntoInterface(
+		&sentAmt,
+		string(precompile.FunTokenMethod_BankSend),
+		ethTxResp.Ret,
+	)
+	s.NoError(err)
+	s.Require().Equal("420", sentAmt.String())
 }