fix: delegated signatures: check every field of txs and roundtrip eth…

… <-> FIL

pkg/chain/signatures.go

@@ -1,7 +1,7 @@
 package chain
 
 import (
-	"golang.org/x/xerrors"
+	"fmt"
 
 	"github.com/filecoin-project/go-address"
 	"github.com/filecoin-project/go-state-types/network"
@@ -20,21 +20,30 @@ func AuthenticateMessage(msg *types.SignedMessage, signer address.Address) error
 	typ := msg.Signature.Type
 	switch typ {
 	case crypto.SigTypeDelegated:
-		txArgs, err := types.NewEthTxArgsFromMessage(&msg.Message)
+		txArgs, err := types.EthTxArgsFromMessage(&msg.Message)
 		if err != nil {
-			return xerrors.Errorf("failed to reconstruct eth transaction: %w", err)
+			return fmt.Errorf("failed to reconstruct eth transaction: %w", err)
 		}
-		msg, err := txArgs.ToRlpUnsignedMsg()
+		roundTripMsg, err := txArgs.ToUnsignedMessage(msg.Message.From)
 		if err != nil {
-			return xerrors.Errorf("failed to repack eth rlp message: %w", err)
+			return fmt.Errorf("failed to reconstruct filecoin msg: %w", err)
 		}
-		digest = msg
+
+		if !msg.Message.Equals(roundTripMsg) {
+			return fmt.Errorf("ethereum tx failed to roundtrip")
+		}
+
+		rlpEncodedMsg, err := txArgs.ToRlpUnsignedMsg()
+		if err != nil {
+			return fmt.Errorf("failed to repack eth rlp message: %w", err)
+		}
+		digest = rlpEncodedMsg
 	default:
 		digest = msg.Message.Cid().Bytes()
 	}
 
 	if err := crypto.Verify(&msg.Signature, signer, digest); err != nil {
-		return xerrors.Errorf("message %s has invalid signature (type %d): %w", msg.Cid(), typ, err)
+		return fmt.Errorf("message %s has invalid signature (type %d): %w", msg.Cid(), typ, err)
 	}
 	return nil
 }

venus-shared/types/eth_transactions.go

@@ -14,7 +14,6 @@ import (
 	"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"
-	"github.com/filecoin-project/go-state-types/builtin/v10/eam"
 	typescrypto "github.com/filecoin-project/go-state-types/crypto"
 	"github.com/filecoin-project/venus/venus-shared/actors"
 )
@@ -124,76 +123,83 @@ func EthTxArgsFromMessage(msg *Message) (EthTxArgs, error) {
 	}, nil
 }
 
-func (tx *EthTxArgs) ToSignedMessage() (*SignedMessage, error) {
-	from, err := tx.Sender()
-	if err != nil {
-		return nil, err
+func (tx *EthTxArgs) ToUnsignedMessage(from address.Address) (*Message, error) {
+	if tx.ChainID != Eip155ChainID {
+		return nil, fmt.Errorf("unsupported chain id: %d", tx.ChainID)
 	}
 
-	var to address.Address
+	var err error
+	method := builtintypes.MethodSend
 	var params []byte
-
-	if len(tx.To) == 0 && len(tx.Input) == 0 {
-		return nil, fmt.Errorf("to and input cannot both be empty")
-	}
-
-	var method abi.MethodNum
+	var to address.Address
+	// nil indicates the EAM, only CreateExternal is allowed
 	if tx.To == nil {
-		// TODO unify with applyEvmMsg
-
-		// this is a contract creation
 		to = builtintypes.EthereumAddressManagerActorAddr
-
-		params2, err := actors.SerializeParams(&eam.CreateParams{
-			Initcode: tx.Input,
-			Nonce:    uint64(tx.Nonce),
-		})
+		method = builtintypes.MethodsEAM.CreateExternal
+		if len(tx.Input) == 0 {
+			return nil, fmt.Errorf("cannot call CreateExternal without params")
+		}
+		inputParams := abi.CborBytes(tx.Input)
+		params, err = actors.SerializeParams(&inputParams)
 		if err != nil {
 			return nil, fmt.Errorf("failed to serialize Create params: %w", err)
 		}
-		params = params2
-		method = builtintypes.MethodsEAM.Create
+
 	} else {
-		addr, err := tx.To.ToFilecoinAddress()
+		to, err = tx.To.ToFilecoinAddress()
 		if err != nil {
-			return nil, err
+			return nil, fmt.Errorf("failed to convert To into filecoin addr: %w", err)
 		}
-		to = addr
-
-		if len(tx.Input) > 0 {
-			var buf bytes.Buffer
-			if err := cbg.WriteByteArray(&buf, tx.Input); err != nil {
-				return nil, fmt.Errorf("failed to encode tx input into a cbor byte-string")
+		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, fmt.Errorf("failed to write input args: %w", err)
 			}
+
 			params = buf.Bytes()
-			method = builtintypes.MethodsEVM.InvokeContract
-		} else {
-			method = builtintypes.MethodSend
 		}
 	}
 
-	msg := &Message{
-		Nonce:      uint64(tx.Nonce),
-		From:       from,
+	return &Message{
+		Version:    0,
 		To:         to,
+		From:       from,
+		Nonce:      uint64(tx.Nonce),
 		Value:      tx.Value,
-		Method:     method,
-		Params:     params,
 		GasLimit:   int64(tx.GasLimit),
 		GasFeeCap:  tx.MaxFeePerGas,
 		GasPremium: tx.MaxPriorityFeePerGas,
+		Method:     method,
+		Params:     params,
+	}, nil
+}
+
+func (tx *EthTxArgs) ToSignedMessage() (*SignedMessage, error) {
+	from, err := tx.Sender()
+	if err != nil {
+		return nil, fmt.Errorf("failed to calculate sender: %w", err)
 	}
 
-	sig, err := tx.Signature()
+	unsignedMsg, err := tx.ToUnsignedMessage(from)
 	if err != nil {
-		return nil, err
+		return nil, fmt.Errorf("failed to convert to unsigned msg: %w", err)
 	}
 
-	signedMsg := SignedMessage{
-		Message:   *msg,
-		Signature: *sig,
+	siggy, err := tx.Signature()
+	if err != nil {
+		return nil, fmt.Errorf("failed to calculate signature: %w", err)
 	}
-	return &signedMsg, nil
+
+	return &SignedMessage{
+		Message:   *unsignedMsg,
+		Signature: *siggy,
+	}, nil
 }
 
 func (tx *EthTxArgs) HashedOriginalRlpMsg() ([]byte, error) {