From ad99ddb8dd6a648eef8d24bc4ebe3b746c8c1653 Mon Sep 17 00:00:00 2001
From: Steven Allen <steven@stebalien.com>
Date: Thu, 26 Jan 2023 15:44:55 -0800
Subject: [PATCH] evm: feat: always call contracts on InvokeContract

This:

1. Always uses InvokeContract, never reducing to a method-0 value transfer.
2. Always encodes the parameters as a cbor byte array for simplicity.

This removes _many_ edge cases and makes it possible to run contract
logic with no parameters.

Depends on https://github.com/filecoin-project/ref-fvm/issues/1567

Fixes the issues outlined in https://github.com/filecoin-project/ref-fvm/issues/1302
---
 chain/types/ethtypes/eth_transactions.go | 94 +++++++-----------------
 1 file changed, 28 insertions(+), 66 deletions(-)

diff --git a/chain/types/ethtypes/eth_transactions.go b/chain/types/ethtypes/eth_transactions.go
index 18b92223488..78ed26db1fe 100644
--- a/chain/types/ethtypes/eth_transactions.go
+++ b/chain/types/ethtypes/eth_transactions.go
@@ -12,6 +12,7 @@ import (
 
 	"github.com/filecoin-project/go-address"
 	gocrypto "github.com/filecoin-project/go-crypto"
+	"github.com/filecoin-project/go-state-types/abi"
 	"github.com/filecoin-project/go-state-types/big"
 	builtintypes "github.com/filecoin-project/go-state-types/builtin"
 	typescrypto "github.com/filecoin-project/go-state-types/crypto"
@@ -58,59 +59,38 @@ type EthTxArgs struct {
 
 func EthTxArgsFromMessage(msg *types.Message) (EthTxArgs, error) {
 	var (
-		to           *EthAddress
-		params       []byte
-		paramsReader = bytes.NewReader(msg.Params)
-		err          error
+		to     *EthAddress
+		params []byte
 	)
 
 	if msg.Version != 0 {
 		return EthTxArgs{}, xerrors.Errorf("unsupported msg version: %d", msg.Version)
 	}
 
+	paramsReader := bytes.NewReader(msg.Params)
+	params, err := cbg.ReadByteArray(paramsReader, uint64(len(msg.Params)))
+	if err != nil {
+		return EthTxArgs{}, xerrors.Errorf("failed to read params byte array: %w", err)
+	}
+
+	if paramsReader.Len() != 0 {
+		return EthTxArgs{}, xerrors.Errorf("extra data found in params")
+	}
+
 	if msg.To == builtintypes.EthereumAddressManagerActorAddr {
-		switch msg.Method {
-		case builtintypes.MethodsEAM.CreateExternal:
-			params, err = cbg.ReadByteArray(paramsReader, uint64(len(msg.Params)))
-			if err != nil {
-				return EthTxArgs{}, xerrors.Errorf("failed to read params byte array: %w", err)
-			}
-		default:
+		if msg.Method != builtintypes.MethodsEAM.CreateExternal {
 			return EthTxArgs{}, fmt.Errorf("unsupported EAM method")
 		}
 	} else {
+		if msg.Method != builtintypes.MethodsEVM.InvokeContract {
+			return EthTxArgs{}, xerrors.Errorf("invalid methodnum %d: only allowed non-send method is InvokeContract(%d)", msg.Method, builtintypes.MethodsEVM.InvokeContract)
+		}
+
 		addr, err := EthAddressFromFilecoinAddress(msg.To)
 		if err != nil {
 			return EthTxArgs{}, err
 		}
 		to = &addr
-
-		if len(msg.Params) == 0 {
-			if msg.Method != builtintypes.MethodSend {
-				return EthTxArgs{}, xerrors.Errorf("cannot invoke method %d on non-EAM actor without params", msg.Method)
-			}
-		} else {
-			if msg.Method != builtintypes.MethodsEVM.InvokeContract {
-				return EthTxArgs{},
-					xerrors.Errorf("invalid methodnum %d: only allowed non-send method is InvokeContract(%d)",
-						msg.Method,
-						builtintypes.MethodsEVM.InvokeContract)
-			}
-
-			params, err = cbg.ReadByteArray(paramsReader, uint64(len(msg.Params)))
-			if err != nil {
-				return EthTxArgs{}, xerrors.Errorf("failed to read params byte array: %w", err)
-			}
-		}
-	}
-
-	if paramsReader.Len() != 0 {
-		return EthTxArgs{}, xerrors.Errorf("extra data found in params")
-	}
-
-	if len(params) == 0 && msg.Method != builtintypes.MethodSend {
-		// Otherwise, we don't get a guaranteed round-trip.
-		return EthTxArgs{}, xerrors.Errorf("msgs with empty parameters from an eth-account must be Sends (MethodNum: %d)", msg.Method)
 	}
 
 	return EthTxArgs{
@@ -130,43 +110,25 @@ func (tx *EthTxArgs) ToUnsignedMessage(from address.Address) (*types.Message, er
 		return nil, xerrors.Errorf("unsupported chain id: %d", tx.ChainID)
 	}
 
-	var err error
-	method := builtintypes.MethodSend
-	var params []byte
+	buf := new(bytes.Buffer)
+	if err := cbg.WriteByteArray(buf, tx.Input); err != nil {
+		return nil, xerrors.Errorf("failed to write input args: %w", err)
+	}
+	params := buf.Bytes()
+
 	var to address.Address
+	var method abi.MethodNum
 	// nil indicates the EAM, only CreateExternal is allowed
 	if tx.To == nil {
-		to = builtintypes.EthereumAddressManagerActorAddr
 		method = builtintypes.MethodsEAM.CreateExternal
-		if len(tx.Input) == 0 {
-			return nil, xerrors.New("cannot call CreateExternal without params")
-		}
-
-		buf := new(bytes.Buffer)
-		if err = cbg.WriteByteArray(buf, tx.Input); err != nil {
-			return nil, xerrors.Errorf("failed to serialize Create params: %w", err)
-		}
-
-		params = buf.Bytes()
+		to = builtintypes.EthereumAddressManagerActorAddr
 	} else {
+		method = builtintypes.MethodsEVM.InvokeContract
+		var err error
 		to, err = tx.To.ToFilecoinAddress()
 		if err != nil {
 			return nil, xerrors.Errorf("failed to convert To into filecoin addr: %w", err)
 		}
-		if len(tx.Input) == 0 {
-			// Yes, this is redundant, but let's be sure what we're doing
-			method = builtintypes.MethodSend
-			params = make([]byte, 0)
-		} else {
-			// must be InvokeContract
-			method = builtintypes.MethodsEVM.InvokeContract
-			buf := new(bytes.Buffer)
-			if err = cbg.WriteByteArray(buf, tx.Input); err != nil {
-				return nil, xerrors.Errorf("failed to write input args: %w", err)
-			}
-
-			params = buf.Bytes()
-		}
 	}
 
 	return &types.Message{