imp(erc20): minor alignments between ERC20 extension and contracts

CHANGELOG.md

@@ -101,6 +101,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 - (osmosis-outpost) [#2063](https://github.com/evmos/evmos/pull/2063) Check that receiver address is bech32 with "evmos" as human readable part.
 - (cmn-precompile) [#2064](https://github.com/evmos/evmos/pull/2064) Handle all `fallback` and `receive` function cases
 - (erc20) [#2066](https://github.com/evmos/evmos/pull/2066) Adjust ERC20 EVM extension allowance behavior to align with standard ERC20 smart contracts.
+- (erc20) [#2067](https://github.com/evmos/evmos/pull/2067) Further alignments between ERC20 smart contracts and EVM extension.
 
 ### Bug Fixes
 

precompiles/erc20/approve.go

@@ -26,10 +26,11 @@ import (
 // operation succeeded and emits the Approval event on success.
 //
 // The Approve method handles 4 cases:
-//  1. no authorization, amount 0 or negative -> return error
+//  1. no authorization, amount negative -> return error
 //  2. no authorization, amount positive -> create a new authorization
 //  3. authorization exists, amount 0 or negative -> delete authorization
 //  4. authorization exists, amount positive -> update authorization
+//  5. no authorizaiton, amount 0 -> no-op but still emit Approval event
 func (p Precompile) Approve(
 	ctx sdk.Context,
 	contract *vm.Contract,
@@ -49,11 +50,9 @@ func (p Precompile) Approve(
 	authorization, expiration, _ := auth.CheckAuthzExists(ctx, p.AuthzKeeper, grantee, granter, SendMsgURL) //#nosec:G703 -- we are handling the error case (authorization == nil) in the switch statement below
 
 	switch {
-	case authorization == nil && amount != nil && amount.Sign() <= 0:
+	case authorization == nil && amount != nil && amount.Sign() < 0:
 		// case 1: no authorization, amount 0 or negative -> error
-		// TODO: (@fedekunze) check if this is correct by comparing behavior with
-		// regular ERC20
-		err = errors.New("cannot approve non-positive values")
+		err = errors.New("cannot approve negative values")
 	case authorization == nil && amount != nil && amount.Sign() > 0:
 		// case 2: no authorization, amount positive -> create a new authorization
 		err = p.createAuthorization(ctx, grantee, granter, amount)

precompiles/erc20/errors.go

@@ -0,0 +1,64 @@
+// Copyright Tharsis Labs Ltd.(Evmos)
+// SPDX-License-Identifier:ENCL-1.0(https://github.com/evmos/evmos/blob/main/LICENSE)
+
+package erc20
+
+import (
+	"errors"
+	"strings"
+
+	"github.com/cosmos/cosmos-sdk/x/authz"
+	"github.com/ethereum/go-ethereum/accounts/abi"
+	"github.com/ethereum/go-ethereum/crypto"
+	evmtypes "github.com/evmos/evmos/v15/x/evm/types"
+)
+
+var (
+	ErrInsufficientAllowance        = errors.New("ERC20: insufficient allowance")
+	ErrTransferAmountExceedsBalance = errors.New("ERC20: transfer amount exceeds balance")
+)
+
+// BuildExecRevertedErr returns a mocked error that should align with the
+// behavior of the original ERC20 Solidity implementation.
+//
+// FIXME: This is not yet producing the correct reason bytes.
+func BuildExecRevertedErr(reason string) (error, error) {
+	// The reason bytes are prefixed with this byte array -> see UnpackRevert in ABI implementation in Geth.
+	prefixBytes := crypto.Keccak256([]byte("Error(string)"))
+
+	// This is reverse-engineering the ABI encoding of the revert reason.
+	typ, err := abi.NewType("string", "", nil)
+	if err != nil {
+		return nil, err
+	}
+
+	packedReason, err := (abi.Arguments{{Type: typ}}).Pack(reason)
+	if err != nil {
+		return nil, errors.New("failed to pack revert reason")
+	}
+
+	var reasonBytes []byte
+	reasonBytes = append(reasonBytes, prefixBytes...)
+	reasonBytes = append(reasonBytes, packedReason...)
+
+	return evmtypes.NewExecErrorWithReason(reasonBytes), nil
+}
+
+// convertErrToERC20Error is a helper function which maps errors raised by the Cosmos SDK stack
+// to the corresponding errors which are raised by an ERC20 contract.
+//
+// TODO: Create the full RevertError types instead of just the standard error type.
+//
+// TODO: Return ERC-6093 compliant errors.
+func convertErrToERC20Error(err error) error {
+	switch {
+	case strings.Contains(err.Error(), "spendable balance"):
+		return ErrTransferAmountExceedsBalance
+	case strings.Contains(err.Error(), "requested amount is more than spend limit"):
+		return ErrInsufficientAllowance
+	case strings.Contains(err.Error(), authz.ErrNoAuthorizationFound.Error()):
+		return ErrInsufficientAllowance
+	default:
+		return err
+	}
+}

precompiles/erc20/errors_test.go

@@ -0,0 +1,25 @@
+package erc20_test
+
+import (
+	"github.com/evmos/evmos/v15/precompiles/erc20"
+	evmtypes "github.com/evmos/evmos/v15/x/evm/types"
+)
+
+// TODO: This is not yet producing the correct reason bytes so we skip this test for now,
+// until that's correctly implemented.
+func (s *PrecompileTestSuite) TestBuildExecRevertedError() {
+	s.T().Skip("skipping until correctly implemented")
+
+	reason := "ERC20: transfer amount exceeds balance"
+	revErr, err := erc20.BuildExecRevertedErr(reason)
+	s.Require().NoError(err, "should not error when building revert error")
+
+	revertErr, ok := revErr.(*evmtypes.RevertError)
+	s.Require().True(ok, "error should be a revert error")
+
+	// Here we expect the correct revert reason that's returned by an ERC20 Solidity contract.
+	s.Require().Equal(
+		"0x08c379a00000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000002645524332303a207472616e7366657220616d6f756e7420657863656564732062616c616e63650000000000000000000000000000000000000000000000000000",
+		revertErr.ErrorData(),
+		"error data should be the revert reason")
+}

precompiles/erc20/tx.go

@@ -90,6 +90,7 @@ func (p Precompile) transfer(
 	}
 
 	if err != nil {
+		err = convertErrToERC20Error(err)
 		// This should return an error to avoid the contract from being executed and an event being emitted
 		return nil, err
 	}