fix: eth: handle unresolvable addresses (#11433)

Correctly handle "unresolvable" to/from addresses in top-level messages in the Ethereum API. Specifically:

1. Fail if we can't resolve the from address. As far as I can tell, this should be impossible (the message statically couldn't have been included in the block if the sender didn't exist).
2. If we can't resolve the "to" address to an ID, use "max uint64" as the ID (`0xff0000000000000000000000ffffffffffffffff`). This will only happen if the transaction was reverted. It'll be a little confusing, but the alternative is to (a) use an empty address (will look like a contract creation, which is definitely wrong) or (b) use a random/hashed address which will likely be more confusing as it won't be "obviously weird".

CHANGELOG.md

@@ -14,6 +14,7 @@
 - feat: Unambiguously translate native messages to Ethereum transactions by:
   - Detecting native messages that "look" like Ethereum transactions (creating smart contracts, invoking a smart contract, etc.), and decoding them as such.
   - Otherwise, ABI-encoding the inputs as if they were calls to a `handle_filecoin_method` Solidity method.
+- fix: ensure that the Ethereum API never returns "empty" addresses for native messages. When a "to" address cannot be resolved to a 0x-style address, it will be re-written to `0xff0000000000000000000000ffffffffffffffff`. This can only happen when the native transaction _reverted_ (failing to create an account at the specified "to" address).
 
 # v 1.25.0 / 2023-11-22
 

itests/eth_transactions_test.go

@@ -9,11 +9,14 @@ import (
 
 	"github.com/stretchr/testify/require"
 
+	"github.com/filecoin-project/go-address"
 	"github.com/filecoin-project/go-state-types/abi"
 	"github.com/filecoin-project/go-state-types/big"
 	builtin2 "github.com/filecoin-project/go-state-types/builtin"
+	"github.com/filecoin-project/go-state-types/exitcode"
 	"github.com/filecoin-project/go-state-types/manifest"
 
+	"github.com/filecoin-project/lotus/api"
 	"github.com/filecoin-project/lotus/build"
 	"github.com/filecoin-project/lotus/chain/actors"
 	"github.com/filecoin-project/lotus/chain/store"
@@ -378,7 +381,8 @@ func deployContractTx(ctx context.Context, client *kit.TestFullNode, ethAddr eth
 	}, nil
 }
 
-func TestEthTxFromNativeAccount(t *testing.T) {
+// Invoke a contract with empty input.
+func TestEthTxFromNativeAccount_EmptyInput(t *testing.T) {
 	blockTime := 10 * time.Millisecond
 	client, _, ens := kit.EnsembleMinimal(t, kit.MockProofs(), kit.ThroughRPC())
 
@@ -387,15 +391,16 @@ func TestEthTxFromNativeAccount(t *testing.T) {
 	ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
 	defer cancel()
 
+	secpAddr, err := address.NewSecp256k1Address([]byte("foobar"))
+	require.NoError(t, err)
+
 	msg := &types.Message{
 		From:   client.DefaultKey.Address,
-		To:     client.DefaultKey.Address,
+		To:     secpAddr,
 		Value:  abi.TokenAmount(types.MustParseFIL("100")),
 		Method: builtin2.MethodsEVM.InvokeContract,
 	}
 
-	// Send a message with no input.
-
 	sMsg, err := client.MpoolPushMessage(ctx, msg, nil)
 	require.NoError(t, err)
 	client.WaitMsg(ctx, sMsg.Cid())
@@ -408,35 +413,79 @@ func TestEthTxFromNativeAccount(t *testing.T) {
 	// Expect empty input params given that we "invoked" the contract (well, invoked ourselves).
 	require.Equal(t, ethtypes.EthBytes{}, tx.Input)
 
-	// Send a message with some input.
+	// Validate the to/from addresses.
+	toId, err := client.StateLookupID(ctx, msg.To, types.EmptyTSK)
+	require.NoError(t, err)
+	fromId, err := client.StateLookupID(ctx, msg.From, types.EmptyTSK)
+	require.NoError(t, err)
+
+	expectedTo, err := ethtypes.EthAddressFromFilecoinAddress(toId)
+	require.NoError(t, err)
+	expectedFrom, err := ethtypes.EthAddressFromFilecoinAddress(fromId)
+	require.NoError(t, err)
+	require.Equal(t, &expectedTo, tx.To)
+	require.Equal(t, expectedFrom, tx.From)
+}
 
+// Invoke a contract with non-empty input.
+func TestEthTxFromNativeAccount_NonEmptyInput(t *testing.T) {
+	blockTime := 10 * time.Millisecond
+	client, _, ens := kit.EnsembleMinimal(t, kit.MockProofs(), kit.ThroughRPC())
+
+	ens.InterconnectAll().BeginMining(blockTime)
+
+	ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
+	defer cancel()
+
+	msg := &types.Message{
+		From:   client.DefaultKey.Address,
+		To:     client.DefaultKey.Address,
+		Value:  abi.TokenAmount(types.MustParseFIL("100")),
+		Method: builtin2.MethodsEVM.InvokeContract,
+	}
+
+	var err error
 	input := abi.CborBytes([]byte{0x1, 0x2, 0x3, 0x4})
 	msg.Params, err = actors.SerializeParams(&input)
 	require.NoError(t, err)
 
-	sMsg, err = client.MpoolPushMessage(ctx, msg, nil)
+	sMsg, err := client.MpoolPushMessage(ctx, msg, nil)
 	require.NoError(t, err)
 	client.WaitMsg(ctx, sMsg.Cid())
-	hash, err = client.EthGetTransactionHashByCid(ctx, sMsg.Cid())
+	hash, err := client.EthGetTransactionHashByCid(ctx, sMsg.Cid())
 	require.NoError(t, err)
-	tx, err = client.EthGetTransactionByHash(ctx, hash)
+	tx, err := client.EthGetTransactionByHash(ctx, hash)
 	require.NoError(t, err)
 
 	// Expect the decoded input.
 	require.EqualValues(t, input, tx.Input)
+}
 
-	// Invoke the contract, but with incorrectly encoded input. We expect this to be abi-encoded
-	// as if it were any other method call.
+// Invoke a contract, but with incorrectly encoded input. We expect this to be abi-encoded as if it
+// were any other method call.
+func TestEthTxFromNativeAccount_BadInput(t *testing.T) {
+	blockTime := 10 * time.Millisecond
+	client, _, ens := kit.EnsembleMinimal(t, kit.MockProofs(), kit.ThroughRPC())
 
-	msg.Params = input
-	require.NoError(t, err)
+	ens.InterconnectAll().BeginMining(blockTime)
+
+	ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
+	defer cancel()
 
-	sMsg, err = client.MpoolPushMessage(ctx, msg, nil)
+	msg := &types.Message{
+		From:   client.DefaultKey.Address,
+		To:     client.DefaultKey.Address,
+		Value:  abi.TokenAmount(types.MustParseFIL("100")),
+		Method: builtin2.MethodsEVM.InvokeContract,
+		Params: []byte{0x1, 0x2, 0x3, 0x4},
+	}
+
+	sMsg, err := client.MpoolPushMessage(ctx, msg, nil)
 	require.NoError(t, err)
 	client.WaitMsg(ctx, sMsg.Cid())
-	hash, err = client.EthGetTransactionHashByCid(ctx, sMsg.Cid())
+	hash, err := client.EthGetTransactionHashByCid(ctx, sMsg.Cid())
 	require.NoError(t, err)
-	tx, err = client.EthGetTransactionByHash(ctx, hash)
+	tx, err := client.EthGetTransactionByHash(ctx, hash)
 	require.NoError(t, err)
 
 	const expectedHex1 = "868e10c4" + // "handle filecoin method" function selector
@@ -451,24 +500,39 @@ func TestEthTxFromNativeAccount(t *testing.T) {
 		// Input: 1, 2, 3, 4
 		"0102030400000000000000000000000000000000000000000000000000000000"
 
-	input, err = hex.DecodeString(expectedHex1)
+	input, err := hex.DecodeString(expectedHex1)
 	require.NoError(t, err)
 	require.EqualValues(t, input, tx.Input)
 
-	// Invoke a random method with the same input. We expect the same result as above, but with
-	// a different method number.
+}
+
+// Invoke a native method.
+func TestEthTxFromNativeAccount_NativeMethod(t *testing.T) {
+	blockTime := 10 * time.Millisecond
+	client, _, ens := kit.EnsembleMinimal(t, kit.MockProofs(), kit.ThroughRPC())
 
-	msg.Method++
+	ens.InterconnectAll().BeginMining(blockTime)
+
+	ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
+	defer cancel()
+
+	msg := &types.Message{
+		From:   client.DefaultKey.Address,
+		To:     client.DefaultKey.Address,
+		Value:  abi.TokenAmount(types.MustParseFIL("100")),
+		Method: builtin2.MethodsEVM.InvokeContract + 1,
+		Params: []byte{0x1, 0x2, 0x3, 0x4},
+	}
 
-	sMsg, err = client.MpoolPushMessage(ctx, msg, nil)
+	sMsg, err := client.MpoolPushMessage(ctx, msg, nil)
 	require.NoError(t, err)
 	client.WaitMsg(ctx, sMsg.Cid())
-	hash, err = client.EthGetTransactionHashByCid(ctx, sMsg.Cid())
+	hash, err := client.EthGetTransactionHashByCid(ctx, sMsg.Cid())
 	require.NoError(t, err)
-	tx, err = client.EthGetTransactionByHash(ctx, hash)
+	tx, err := client.EthGetTransactionByHash(ctx, hash)
 	require.NoError(t, err)
 
-	const expectedHex2 = "868e10c4" + // "handle filecoin method" function selector
+	const expectedHex = "868e10c4" + // "handle filecoin method" function selector
 		// InvokeEVM+1
 		"00000000000000000000000000000000000000000000000000000000e525aa16" +
 		// CBOR multicodec (0x51)
@@ -479,7 +543,51 @@ func TestEthTxFromNativeAccount(t *testing.T) {
 		"0000000000000000000000000000000000000000000000000000000000000004" +
 		// Input: 1, 2, 3, 4
 		"0102030400000000000000000000000000000000000000000000000000000000"
-	input, err = hex.DecodeString(expectedHex2)
+	input, err := hex.DecodeString(expectedHex)
 	require.NoError(t, err)
 	require.EqualValues(t, input, tx.Input)
 }
+
+// Send to an invalid receiver. We're checking to make sure we correctly set `txn.To` to the special
+// "reverted" eth addr.
+func TestEthTxFromNativeAccount_InvalidReceiver(t *testing.T) {
+	blockTime := 10 * time.Millisecond
+	client, _, ens := kit.EnsembleMinimal(t, kit.MockProofs(), kit.ThroughRPC())
+
+	ens.InterconnectAll().BeginMining(blockTime)
+
+	ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
+	defer cancel()
+
+	to, err := address.NewActorAddress([]byte("foobar"))
+	require.NoError(t, err)
+
+	msg := &types.Message{
+		From:   client.DefaultKey.Address,
+		To:     to,
+		Value:  abi.TokenAmount(types.MustParseFIL("100")),
+		Method: builtin2.MethodsEVM.InvokeContract + 1,
+		Params: []byte{0x1, 0x2, 0x3, 0x4},
+		// We can't estimate gas for a failed message, so we hard-code these values.
+		GasLimit:  10_000_000,
+		GasFeeCap: abi.NewTokenAmount(10000),
+	}
+
+	// We expect the "to" address to be the special "reverted" eth address.
+	expectedTo, err := ethtypes.ParseEthAddress("ff0000000000000000000000ffffffffffffffff")
+	require.NoError(t, err)
+
+	sMsg, err := client.WalletSignMessage(ctx, client.DefaultKey.Address, msg)
+	require.NoError(t, err)
+	k, err := client.MpoolPush(ctx, sMsg)
+	require.NoError(t, err)
+	res, err := client.StateWaitMsg(ctx, k, 3, api.LookbackNoLimit, true)
+	require.NoError(t, err)
+	require.Equal(t, res.Receipt.ExitCode, exitcode.SysErrInvalidReceiver)
+
+	hash, err := client.EthGetTransactionHashByCid(ctx, k)
+	require.NoError(t, err)
+	tx, err := client.EthGetTransactionByHash(ctx, hash)
+	require.NoError(t, err)
+	require.EqualValues(t, &expectedTo, tx.To)
+}

node/impl/full/eth_utils.go

@@ -29,6 +29,18 @@ import (
 	"github.com/filecoin-project/lotus/chain/vm"
 )
 
+// The address used in messages to actors that have since been deleted.
+//
+// 0xff0000000000000000000000ffffffffffffffff
+var revertedEthAddress ethtypes.EthAddress
+
+func init() {
+	revertedEthAddress[0] = 0xff
+	for i := 20 - 8; i < 20; i++ {
+		revertedEthAddress[i] = 0xff
+	}
+}
+
 func getTipsetByBlockNumber(ctx context.Context, chain *store.ChainStore, blkParam string, strict bool) (*types.TipSet, error) {
 	if blkParam == "earliest" {
 		return nil, fmt.Errorf("block param \"earliest\" is not supported")
@@ -463,13 +475,19 @@ func newEthTxFromSignedMessage(smsg *types.SignedMessage, st *state.StateTree) (
 			return ethtypes.EthTx{}, xerrors.Errorf("failed to calculate hash for ethTx: %w", err)
 		}
 	} else if smsg.Signature.Type == crypto.SigTypeSecp256k1 { // Secp Filecoin Message
-		tx = ethTxFromNativeMessage(smsg.VMMessage(), st)
+		tx, err = ethTxFromNativeMessage(smsg.VMMessage(), st)
+		if err != nil {
+			return ethtypes.EthTx{}, err
+		}
 		tx.Hash, err = ethtypes.EthHashFromCid(smsg.Cid())
 		if err != nil {
 			return ethtypes.EthTx{}, err
 		}
 	} else { // BLS Filecoin message
-		tx = ethTxFromNativeMessage(smsg.VMMessage(), st)
+		tx, err = ethTxFromNativeMessage(smsg.VMMessage(), st)
+		if err != nil {
+			return ethtypes.EthTx{}, err
+		}
 		tx.Hash, err = ethtypes.EthHashFromCid(smsg.Message.Cid())
 		if err != nil {
 			return ethtypes.EthTx{}, err
@@ -482,19 +500,33 @@ func newEthTxFromSignedMessage(smsg *types.SignedMessage, st *state.StateTree) (
 // Convert a native message to an eth transaction.
 //
 //   - The state-tree must be from after the message was applied (ideally the following tipset).
+//   - In some cases, the "to" address may be `0xff0000000000000000000000ffffffffffffffff`. This
+//     means that the "to" address has not been assigned in the passed state-tree and can only
+//     happen if the transaction reverted.
 //
 // ethTxFromNativeMessage does NOT populate:
 // - BlockHash
 // - BlockNumber
 // - TransactionIndex
 // - Hash
-func ethTxFromNativeMessage(msg *types.Message, st *state.StateTree) ethtypes.EthTx {
-	// We don't care if we error here, conversion is best effort for non-eth transactions
-	from, _ := lookupEthAddress(msg.From, st)
-	to, _ := lookupEthAddress(msg.To, st)
+func ethTxFromNativeMessage(msg *types.Message, st *state.StateTree) (ethtypes.EthTx, error) {
+	// Lookup the from address. This must succeed.
+	from, err := lookupEthAddress(msg.From, st)
+	if err != nil {
+		return ethtypes.EthTx{}, xerrors.Errorf("failed to lookup sender address %s when converting a native message to an eth txn: %w", msg.From, err)
+	}
+	// Lookup the to address. If the recipient doesn't exist, we replace the address with a
+	// known sentinel address.
+	to, err := lookupEthAddress(msg.To, st)
+	if err != nil {
+		if !errors.Is(err, types.ErrActorNotFound) {
+			return ethtypes.EthTx{}, xerrors.Errorf("failed to lookup receiver address %s when converting a native message to an eth txn: %w", msg.To, err)
+		}
+		to = revertedEthAddress
+	}
 	toPtr := &to
 
-	// Convert the input parameters to "solidity ABI".
+	// Finally, convert the input parameters to "solidity ABI".
 
 	// For empty, we use "0" as the codec. Otherwise, we use CBOR for message
 	// parameters.
@@ -536,7 +568,7 @@ func ethTxFromNativeMessage(msg *types.Message, st *state.StateTree) ethtypes.Et
 		MaxFeePerGas:         ethtypes.EthBigInt(msg.GasFeeCap),
 		MaxPriorityFeePerGas: ethtypes.EthBigInt(msg.GasPremium),
 		AccessList:           []ethtypes.EthHash{},
-	}
+	}, nil
 }
 
 func getSignedMessage(ctx context.Context, cs *store.ChainStore, msgCid cid.Cid) (*types.SignedMessage, error) {