Merge pull request #10246 from filecoin-project/steb/fix-eth-get-code-none

fix: eth: correctly handle ethGetCode ege-cases

.circleci/config.yml

@@ -628,6 +628,11 @@ workflows:
           suite: itest-eth_block_hash
           target: "./itests/eth_block_hash_test.go"
       
+      - test:
+          name: test-itest-eth_bytecode
+          suite: itest-eth_bytecode
+          target: "./itests/eth_bytecode_test.go"
+      
       - test:
           name: test-itest-eth_config
           suite: itest-eth_config

itests/eth_bytecode_test.go

@@ -0,0 +1,74 @@
+package itests
+
+import (
+	"context"
+	"encoding/hex"
+	"os"
+	"testing"
+	"time"
+
+	"github.com/stretchr/testify/require"
+
+	"github.com/filecoin-project/lotus/chain/types"
+	"github.com/filecoin-project/lotus/itests/kit"
+)
+
+// TestGetCode ensures that GetCode returns the correct results for:
+// 1. Placeholders.
+// 2. Non-existent actors.
+// 3. Normal EVM actors.
+// 4. Self-destructed EVM actors.
+func TestGetCode(t *testing.T) {
+	kit.QuietMiningLogs()
+
+	blockTime := 100 * 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()
+
+	// Accounts should have empty code.
+	{
+		// A random eth address should have no code.
+		_, ethAddr, filAddr := client.EVM().NewAccount()
+		bytecode, err := client.EVM().EthGetCode(ctx, ethAddr, "latest")
+		require.NoError(t, err)
+		require.Empty(t, bytecode)
+
+		// send some funds to the account.
+		kit.SendFunds(ctx, t, client, filAddr, types.FromFil(10))
+
+		// The code should still be empty, target is now a placeholder.
+		bytecode, err = client.EVM().EthGetCode(ctx, ethAddr, "latest")
+		require.NoError(t, err)
+		require.Empty(t, bytecode)
+	}
+
+	// Check contract code.
+	{
+		// install a contract
+		contractHex, err := os.ReadFile("./contracts/SelfDestruct.hex")
+		require.NoError(t, err)
+		contract, err := hex.DecodeString(string(contractHex))
+		require.NoError(t, err)
+		createReturn := client.EVM().DeployContract(ctx, client.DefaultKey.Address, contract)
+		contractAddr := createReturn.EthAddress
+		contractFilAddr := *createReturn.RobustAddress
+
+		// The newly deployed contract should not be empty.
+		bytecode, err := client.EVM().EthGetCode(ctx, contractAddr, "latest")
+		require.NoError(t, err)
+		require.NotEmpty(t, bytecode)
+
+		// Destroy it.
+		_, _, err = client.EVM().InvokeContractByFuncName(ctx, client.DefaultKey.Address, contractFilAddr, "destroy()", nil)
+		require.NoError(t, err)
+
+		// The code should be empty again.
+		bytecode, err = client.EVM().EthGetCode(ctx, contractAddr, "latest")
+		require.NoError(t, err)
+		require.Empty(t, bytecode)
+	}
+
+}

itests/kit/evm.go

@@ -64,7 +64,7 @@ func (e *EVM) DeployContractWithValue(ctx context.Context, sender address.Addres
 	require.NoError(err)
 
 	e.t.Log("waiting for message to execute")
-	wait, err := e.StateWaitMsg(ctx, smsg.Cid(), 0, 0, false)
+	wait, err := e.StateWaitMsg(ctx, smsg.Cid(), 3, 0, false)
 	require.NoError(err)
 
 	require.True(wait.Receipt.ExitCode.IsSuccess(), "contract installation failed")
@@ -128,7 +128,7 @@ func (e *EVM) InvokeSolidity(ctx context.Context, sender address.Address, target
 	}
 
 	e.t.Log("waiting for message to execute")
-	wait, err := e.StateWaitMsg(ctx, smsg.Cid(), 0, 0, false)
+	wait, err := e.StateWaitMsg(ctx, smsg.Cid(), 3, 0, false)
 	if err != nil {
 		return nil, err
 	}

node/impl/full/eth.go

@@ -426,22 +426,6 @@ func (a *EthModule) EthGetCode(ctx context.Context, ethAddr ethtypes.EthAddress,
 		return nil, xerrors.Errorf("cannot get Filecoin address: %w", err)
 	}
 
-	// use the system actor as the caller
-	from, err := address.NewIDAddress(0)
-	if err != nil {
-		return nil, fmt.Errorf("failed to construct system sender address: %w", err)
-	}
-	msg := &types.Message{
-		From:       from,
-		To:         to,
-		Value:      big.Zero(),
-		Method:     builtintypes.MethodsEVM.GetBytecode,
-		Params:     nil,
-		GasLimit:   build.BlockGasLimit,
-		GasFeeCap:  big.Zero(),
-		GasPremium: big.Zero(),
-	}
-
 	ts, err := a.parseBlkParam(ctx, blkParam)
 	if err != nil {
 		return nil, xerrors.Errorf("cannot parse block param: %s", blkParam)
@@ -452,6 +436,31 @@ func (a *EthModule) EthGetCode(ctx context.Context, ethAddr ethtypes.EthAddress,
 		return nil, xerrors.Errorf("block param must not specify genesis block")
 	}
 
+	actor, err := a.StateManager.LoadActor(ctx, to, ts)
+	if err != nil {
+		if xerrors.Is(err, types.ErrActorNotFound) {
+			return nil, nil
+		}
+		return nil, xerrors.Errorf("failed to lookup contract %s: %w", ethAddr, err)
+	}
+
+	// Not a contract. We could try to distinguish between accounts and "native" contracts here,
+	// but it's not worth it.
+	if !builtinactors.IsEvmActor(actor.Code) {
+		return nil, nil
+	}
+
+	msg := &types.Message{
+		From:       builtinactors.SystemActorAddr,
+		To:         to,
+		Value:      big.Zero(),
+		Method:     builtintypes.MethodsEVM.GetBytecode,
+		Params:     nil,
+		GasLimit:   build.BlockGasLimit,
+		GasFeeCap:  big.Zero(),
+		GasPremium: big.Zero(),
+	}
+
 	// Try calling until we find a height with no migration.
 	var res *api.InvocResult
 	for {
@@ -466,9 +475,7 @@ func (a *EthModule) EthGetCode(ctx context.Context, ethAddr ethtypes.EthAddress,
 	}
 
 	if err != nil {
-		// if the call resulted in error, this is not an EVM smart contract;
-		// return no bytecode.
-		return nil, nil
+		return nil, xerrors.Errorf("failed to call GetBytecode: %w", err)
 	}
 
 	if res.MsgRct == nil {
@@ -476,15 +483,20 @@ func (a *EthModule) EthGetCode(ctx context.Context, ethAddr ethtypes.EthAddress,
 	}
 
 	if res.MsgRct.ExitCode.IsError() {
-		return nil, xerrors.Errorf("message execution failed: exit %s, reason: %s", res.MsgRct.ExitCode, res.Error)
+		return nil, xerrors.Errorf("GetBytecode failed: %s", res.Error)
 	}
 
-	var bytecodeCid cbg.CborCid
-	if err := bytecodeCid.UnmarshalCBOR(bytes.NewReader(res.MsgRct.Return)); err != nil {
+	var getBytecodeReturn evm.GetBytecodeReturn
+	if err := getBytecodeReturn.UnmarshalCBOR(bytes.NewReader(res.MsgRct.Return)); err != nil {
 		return nil, fmt.Errorf("failed to decode EVM bytecode CID: %w", err)
 	}
 
-	blk, err := a.Chain.StateBlockstore().Get(ctx, cid.Cid(bytecodeCid))
+	// The contract has selfdestructed, so the code is "empty".
+	if getBytecodeReturn.Cid == nil {
+		return nil, nil
+	}
+
+	blk, err := a.Chain.StateBlockstore().Get(ctx, *getBytecodeReturn.Cid)
 	if err != nil {
 		return nil, fmt.Errorf("failed to get EVM bytecode: %w", err)
 	}