From b7651128969997fc0fecd0151b397f896d483f5e Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Sun, 12 Feb 2023 19:43:25 -0800 Subject: [PATCH] fix: correctly handle ethGetCode edge-cases 1. If the actor is a non-evm actor, return "nothing". 2. Correctly handle missing actors. 3. Handle self-destructed actors (they'll return "null"). fixes https://github.com/filecoin-project/ref-fvm/issues/1641 --- itests/eth_bytecode_test.go | 73 +++++++++++++++++++++++++++++++++++++ itests/kit/evm.go | 4 +- node/impl/full/eth.go | 48 ++++++++++++++---------- 3 files changed, 103 insertions(+), 22 deletions(-) create mode 100644 itests/eth_bytecode_test.go diff --git a/itests/eth_bytecode_test.go b/itests/eth_bytecode_test.go new file mode 100644 index 000000000..fa1a3fa35 --- /dev/null +++ b/itests/eth_bytecode_test.go @@ -0,0 +1,73 @@ +package itests + +import ( + "context" + "encoding/hex" + "os" + "testing" + "time" + + "github.com/filecoin-project/lotus/chain/types" + "github.com/filecoin-project/lotus/itests/kit" + "github.com/stretchr/testify/require" +) + +// 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) + } + +} diff --git a/itests/kit/evm.go b/itests/kit/evm.go index 216a7f1f3..3cbfcd5f2 100644 --- a/itests/kit/evm.go +++ b/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 } diff --git a/node/impl/full/eth.go b/node/impl/full/eth.go index 49e976cdf..9576dee40 100644 --- a/node/impl/full/eth.go +++ b/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,13 +483,14 @@ 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 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) } + // The contract has selfdestructed, so the code is "empty". if getBytecodeReturn.Cid == nil { return nil, nil