fix: eth: correctly encode and simplify native input/output encoding

When generating eth traces, we encode "native" message inputs/outputs
to "solidity ABI" by formatting the inputs/outputs the same way we do in
FEVM's "handle_native_method". However, we had quite a few bugs with the
implementation:

1. We were right-aligning 64bit values in 256bit words, instead of
left-aligning (as we should given that these values are big-endian).
2. The return-value encoding wasn't correctly handling lengths.

This patch:

1. Fixes those bugs.
2. Deduplicates the logic (we're doing _basically_ the same thing in
both cases).
3. Removes all error paths (these functions can't fail).

node/impl/full/eth.go

@@ -948,10 +948,7 @@ func (a *EthModule) EthTraceReplayBlockTransactions(ctx context.Context, blkNum
 				return nil, xerrors.Errorf("failed to decode payload: %w", err)
 			}
 		} else {
-			output, err = encodeFilecoinReturnAsABI(ir.ExecutionTrace.MsgRct.ExitCode, ir.ExecutionTrace.MsgRct.ReturnCodec, ir.ExecutionTrace.MsgRct.Return)
-			if err != nil {
-				return nil, xerrors.Errorf("could not convert output: %w", err)
-			}
+			output = encodeFilecoinReturnAsABI(ir.ExecutionTrace.MsgRct.ExitCode, ir.ExecutionTrace.MsgRct.ReturnCodec, ir.ExecutionTrace.MsgRct.Return)
 		}
 
 		t := ethtypes.EthTraceReplayBlockTransaction{

node/impl/full/eth_test.go

@@ -1,6 +1,7 @@
 package full
 
 import (
+	"encoding/hex"
 	"testing"
 
 	"github.com/ipfs/go-cid"
@@ -162,3 +163,17 @@ func TestRewardPercentiles(t *testing.T) {
 		require.Equal(t, ans, rewards)
 	}
 }
+
+func TestABIEncoding(t *testing.T) {
+	// Generated from https://abi.hashex.org/
+	const expected = "000000000000000000000000000000000000000000000000000000000000001600000000000000000000000000000000000000000000000000000000000000510000000000000000000000000000000000000000000000000000000000000060000000000000000000000000000000000000000000000000000000000000001b1111111111111111111020200301000000044444444444444444010000000000"
+	const data = "111111111111111111102020030100000004444444444444444401"
+
+	expectedBytes, err := hex.DecodeString(expected)
+	require.NoError(t, err)
+
+	dataBytes, err := hex.DecodeString(data)
+	require.NoError(t, err)
+
+	require.Equal(t, expectedBytes, encodeAsABIHelper(22, 81, dataBytes))
+}

node/impl/full/eth_trace.go

@@ -3,18 +3,13 @@ package full
 import (
 	"bytes"
 	"context"
-	"encoding/binary"
-	"fmt"
-	"io"
 
 	"github.com/multiformats/go-multicodec"
 	cbg "github.com/whyrusleeping/cbor-gen"
 	"golang.org/x/xerrors"
 
-	"github.com/filecoin-project/go-state-types/abi"
 	"github.com/filecoin-project/go-state-types/builtin"
 	"github.com/filecoin-project/go-state-types/builtin/v10/evm"
-	"github.com/filecoin-project/go-state-types/exitcode"
 
 	builtinactors "github.com/filecoin-project/lotus/chain/actors/builtin"
 	"github.com/filecoin-project/lotus/chain/types"
@@ -124,14 +119,8 @@ func buildTraces(ctx context.Context, traces *[]*ethtypes.EthTrace, parent *etht
 	} else {
 		// we are going to assume a native method, but we may change it in one of the edge cases below
 		// TODO: only do this if we know it's a native method (optimization)
-		trace.Action.Input, err = encodeFilecoinParamsAsABI(et.Msg.Method, et.Msg.ParamsCodec, et.Msg.Params)
-		if err != nil {
-			return xerrors.Errorf("buildTraces: %w", err)
-		}
-		trace.Result.Output, err = encodeFilecoinReturnAsABI(et.MsgRct.ExitCode, et.MsgRct.ReturnCodec, et.MsgRct.Return)
-		if err != nil {
-			return xerrors.Errorf("buildTraces: %w", err)
-		}
+		trace.Action.Input = encodeFilecoinParamsAsABI(et.Msg.Method, et.Msg.ParamsCodec, et.Msg.Params)
+		trace.Result.Output = encodeFilecoinReturnAsABI(et.MsgRct.ExitCode, et.MsgRct.ReturnCodec, et.MsgRct.Return)
 	}
 
 	// TODO: is it OK to check this here or is this only specific to certain edge case (evm to evm)?
@@ -258,34 +247,3 @@ func buildTraces(ctx context.Context, traces *[]*ethtypes.EthTrace, parent *etht
 
 	return nil
 }
-
-func writePadded(w io.Writer, data any, size int) error {
-	tmp := &bytes.Buffer{}
-
-	// first write data to tmp buffer to get the size
-	err := binary.Write(tmp, binary.BigEndian, data)
-	if err != nil {
-		return fmt.Errorf("writePadded: failed writing tmp data to buffer: %w", err)
-	}
-
-	if tmp.Len() > size {
-		return fmt.Errorf("writePadded: data is larger than size")
-	}
-
-	// write tailing zeros to pad up to size
-	cnt := size - tmp.Len()
-	for i := 0; i < cnt; i++ {
-		err = binary.Write(w, binary.BigEndian, uint8(0))
-		if err != nil {
-			return fmt.Errorf("writePadded: failed writing tailing zeros to buffer: %w", err)
-		}
-	}
-
-	// finally write the actual value
-	err = binary.Write(w, binary.BigEndian, tmp.Bytes())
-	if err != nil {
-		return fmt.Errorf("writePadded: failed writing data to buffer: %w", err)
-	}
-
-	return nil
-}

node/impl/full/eth_utils.go

@@ -697,64 +697,35 @@ func newEthTxReceipt(ctx context.Context, tx ethtypes.EthTx, lookup *api.MsgLook
 	return receipt, nil
 }
 
-func encodeFilecoinParamsAsABI(method abi.MethodNum, codec uint64, params []byte) ([]byte, error) {
-	NATIVE_METHOD_SELECTOR := []byte{0x86, 0x8e, 0x10, 0xc4}
-	EVM_WORD_SIZE := 32
-
-	staticArgs := []uint64{
-		uint64(method),
-		codec,
-		uint64(EVM_WORD_SIZE) * 3,
-		uint64(len(params)),
-	}
-	totalWords := len(staticArgs) + (len(params) / EVM_WORD_SIZE)
-	if len(params)%EVM_WORD_SIZE != 0 {
-		totalWords++
-	}
-	len := 4 + totalWords*EVM_WORD_SIZE
-
-	w := &bytes.Buffer{}
-	err := binary.Write(w, binary.BigEndian, NATIVE_METHOD_SELECTOR)
-	if err != nil {
-		return nil, fmt.Errorf("handleFilecoinMethodInput: failed writing method selector: %w", err)
-	}
-
-	for _, arg := range staticArgs {
-		err := writePadded(w, arg, 32)
-		if err != nil {
-			return nil, fmt.Errorf("handleFilecoinMethodInput: %w", err)
-		}
-	}
-	err = binary.Write(w, binary.BigEndian, params)
-	if err != nil {
-		return nil, fmt.Errorf("handleFilecoinMethodInput: failed writing params: %w", err)
-	}
-	remain := len - w.Len()
-	for i := 0; i < remain; i++ {
-		err = binary.Write(w, binary.BigEndian, uint8(0))
-		if err != nil {
-			return nil, fmt.Errorf("handleFilecoinMethodInput: failed writing tailing zeros: %w", err)
-		}
-	}
+func encodeFilecoinParamsAsABI(method abi.MethodNum, codec uint64, params []byte) []byte {
+	buf := []byte{0x86, 0x8e, 0x10, 0xc4} // Native method selector.
+	return append(buf, encodeAsABIHelper(uint64(method), codec, params)...)
+}
 
-	return w.Bytes(), nil
+func encodeFilecoinReturnAsABI(exitCode exitcode.ExitCode, codec uint64, data []byte) []byte {
+	return encodeAsABIHelper(uint64(exitCode), codec, data)
 }
 
-func encodeFilecoinReturnAsABI(exitCode exitcode.ExitCode, codec uint64, data []byte) ([]byte, error) {
-	w := &bytes.Buffer{}
+// Format 2 numbers followed by an arbitrary byte array as solidity ABI. Both our native
+// inputs/outputs follow the same pattern, so we can reuse this code.
+func encodeAsABIHelper(param1 uint64, param2 uint64, data []byte) []byte {
+	const EVM_WORD_SIZE = 32
 
-	values := []interface{}{uint32(exitCode), codec, uint32(w.Len()), uint32(len(data))}
-	for _, v := range values {
-		err := writePadded(w, v, 32)
-		if err != nil {
-			return nil, fmt.Errorf("handleFilecoinMethodOutput: %w", err)
-		}
+	staticArgs := []uint64{param1, param2, EVM_WORD_SIZE * 3, uint64(len(data))}
+	totalWords := len(staticArgs) + (len(data) / EVM_WORD_SIZE)
+	if len(data)%EVM_WORD_SIZE != 0 {
+		totalWords++
 	}
-
-	err := binary.Write(w, binary.BigEndian, data)
-	if err != nil {
-		return nil, fmt.Errorf("handleFilecoinMethodOutput: failed writing data: %w", err)
+	len := totalWords * EVM_WORD_SIZE
+	buf := make([]byte, len)
+	offset := 0
+	for _, arg := range staticArgs {
+		offset += EVM_WORD_SIZE
+		start := offset - 8
+		binary.BigEndian.PutUint64(buf[start:offset], arg)
 	}
 
-	return w.Bytes(), nil
+	copy(buf[offset:], data)
+
+	return buf
 }