fix(precompile): Proper handling of fallback and receive function…

…s in common precompile (#2064)

* fix(precompile): Proper handling of `fallback` and `receive` functions

* fix: apply changes from code reivew

* fix: apply switch statements instead of if checks

* CHANGELOG

* fix: make short call data to be strictly more than 0

* fix: abstract away the switches in 3 different helper functions

* apply changes from code review

* apply changes from code review

* fix CHANGELOG

* fix: incorrect method in return

* Update precompiles/werc20/werc20.go

---------

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>

CHANGELOG.md

@@ -97,7 +97,8 @@ Ref: https://keepachangelog.com/en/1.0.0/
 - (staking) [#2053](https://github.com/evmos/evmos/pull/2053) Change the validator address in the events from string type to address type.
 - (werc20) [#2057](https://github.com/evmos/evmos/pull/2057) WERC20 refactors and handling of fallback and receive functions.
 - (werc20) [#2062](https://github.com/evmos/evmos/pull/2062) Remove name checking for `fallback` and `receive` functions.
-- (osmosis-outpost) [#202063](https://github.com/evmos/evmos/pull/2063) Check that receiver address is bech32 with "evmos" as human readable part.
+- (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.
 
 ### Bug Fixes

precompiles/common/precompile.go

@@ -53,20 +53,37 @@ func (p Precompile) RunSetup(
 
 	// NOTE: This is a special case where the calling transaction does not specify a function name.
 	// In this case we default to a `fallback` or `receive` function on the contract.
-	if len(contract.Input) != 0 {
-		methodID := contract.Input[:4]
-		// NOTE: this function iterates over the method map and returns
-		// the method with the given ID
-		method, err = p.MethodById(methodID)
-		if err != nil {
-			return sdk.Context{}, nil, nil, uint64(0), nil, err
-		}
 
-		// return error if trying to write to state during a read-only call
-		if readOnly && isTransaction(method.Name) {
-			return sdk.Context{}, nil, nil, uint64(0), nil, vm.ErrWriteProtection
-		}
+	// Simplify the calldata checks
+	isEmptyCallData := len(contract.Input) == 0
+	isShortCallData := len(contract.Input) > 0 && len(contract.Input) < 4
+	isStandardCallData := len(contract.Input) >= 4
 
+	switch {
+	// Case 1: Calldata is empty
+	case isEmptyCallData:
+		method, err = p.emptyCallData(contract)
+
+	// Case 2: calldata is non-empty but less than 4 bytes needed for a method
+	case isShortCallData:
+		method, err = p.methodIDCallData()
+
+	// Case 3: calldata is non-empty and contains the minimum 4 bytes needed for a method
+	case isStandardCallData:
+		method, err = p.standardCallData(contract)
+	}
+
+	if err != nil {
+		return sdk.Context{}, nil, nil, uint64(0), nil, err
+	}
+
+	// return error if trying to write to state during a read-only call
+	if readOnly && isTransaction(method.Name) {
+		return sdk.Context{}, nil, nil, uint64(0), nil, vm.ErrWriteProtection
+	}
+
+	// if the method type is `function` continue looking for arguments
+	if method.Type == abi.Function {
 		argsBz := contract.Input[4:]
 		args, err = method.Inputs.Unpack(argsBz)
 		if err != nil {
@@ -110,3 +127,48 @@ func HandleGasError(ctx sdk.Context, contract *vm.Contract, initialGas sdk.Gas,
 		}
 	}
 }
+
+// emptyCallData is a helper function that returns the method to be called when the calldata is empty.
+func (p Precompile) emptyCallData(contract *vm.Contract) (method *abi.Method, err error) {
+	switch {
+	// Case 1.1: Send call or transfer tx - 'receive' is called if present and value is transferred
+	case contract.Value().Sign() > 0 && p.HasReceive():
+		return &p.Receive, nil
+	// Case 1.2: Either 'receive' is not present, or no value is transferred - call 'fallback' if present
+	case p.HasFallback():
+		return &p.Fallback, nil
+	// Case 1.3: Neither 'receive' nor 'fallback' are present - return error
+	default:
+		return nil, vm.ErrExecutionReverted
+	}
+}
+
+// methodIDCallData is a helper function that returns the method to be called when the calldata is less than 4 bytes.
+func (p Precompile) methodIDCallData() (method *abi.Method, err error) {
+	// Case 2.2: calldata contains less than 4 bytes needed for a method and 'fallback' is not present - return error
+	if !p.HasFallback() {
+		return nil, vm.ErrExecutionReverted
+	}
+	// Case 2.1: calldata contains less than 4 bytes needed for a method - 'fallback' is called if present
+	return &p.Fallback, nil
+}
+
+// standardCallData is a helper function that returns the method to be called when the calldata is 4 bytes or more.
+func (p Precompile) standardCallData(contract *vm.Contract) (method *abi.Method, err error) {
+	methodID := contract.Input[:4]
+	// NOTE: this function iterates over the method map and returns
+	// the method with the given ID
+	method, err = p.MethodById(methodID)
+
+	// Case 3.1 calldata contains a non-existing method ID, and `fallback` is not present - return error
+	if err != nil && !p.HasFallback() {
+		return nil, err
+	}
+
+	// Case 3.2: calldata contains a non-existing method ID - 'fallback' is called if present
+	if err != nil && p.HasFallback() {
+		return &p.Fallback, nil
+	}
+
+	return method, nil
+}

precompiles/werc20/werc20.go

@@ -6,6 +6,7 @@ package werc20
 import (
 	"embed"
 
+	"github.com/ethereum/go-ethereum/accounts/abi"
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/core/vm"
 
@@ -77,7 +78,7 @@ func (p Precompile) RequiredGas(input []byte) uint64 {
 	// to ensure parity in the values.
 
 	// If there is no method ID, then it's the fallback or receive case
-	if len(input) == 0 {
+	if len(input) < 4 {
 		return DepositRequiredGas
 	}
 
@@ -109,7 +110,8 @@ func (p Precompile) Run(evm *vm.EVM, contract *vm.Contract, readOnly bool) (bz [
 	defer cmn.HandleGasError(ctx, contract, initialGas, &err)()
 
 	switch {
-	case method == nil,
+	case method.Type == abi.Fallback,
+		method.Type == abi.Receive,
 		method.Name == DepositMethod:
 		// WERC20 transactions
 		bz, err = p.Deposit(ctx, contract, stateDB, method, args)