From 72730d304d0dca335444fb0268d6998b6b5c6513 Mon Sep 17 00:00:00 2001 From: Steven Allen Date: Wed, 29 Nov 2023 16:51:24 +0400 Subject: [PATCH] [WIP] Improve the correctness of Eth's trace_block - Improve encoding/decoding of parameters and return values: - Encode "native" parameters and return values with Solidity ABI. - Correctly decode parameters to "create" calls. - Use the correct (ish) output for "create" calls. - Handle all forms of "create". - Make robust with respect to reverts: - Use the actor ID/address from the trace instead of looking it up in the state-tree (may not exist in the state-tree due to a revert). - Gracefully handle failed actor/contract creation. - Improve performance: - We avoid looking anything up in the state-tree when translating the trace, which should significantly improve performance. - Improve code readability: - Remove all "backtracking" logic. - Use an "environment" struct to store temporary state instead of attaching it to the trace. - Fix random bugs: - Fix an allocation bug in the "address" logic (need to set the capacity before modifying the slice). - Improved error checking/handling. --- chain/types/ethtypes/eth_types.go | 38 +-- node/impl/full/eth.go | 56 ++- node/impl/full/eth_trace.go | 549 +++++++++++++++++++++--------- node/impl/full/eth_utils.go | 31 +- 4 files changed, 447 insertions(+), 227 deletions(-) diff --git a/chain/types/ethtypes/eth_types.go b/chain/types/ethtypes/eth_types.go index bff15ed24ad..a7341aac40f 100644 --- a/chain/types/ethtypes/eth_types.go +++ b/chain/types/ethtypes/eth_types.go @@ -349,6 +349,13 @@ func IsEthAddress(addr address.Address) bool { return namespace == builtintypes.EthereumAddressManagerActorID && len(payload) == 20 && !bytes.HasPrefix(payload, maskedIDPrefix[:]) } +func EthAddressFromActorID(id abi.ActorID) EthAddress { + var ethaddr EthAddress + ethaddr[0] = 0xff + binary.BigEndian.PutUint64(ethaddr[12:], uint64(id)) + return ethaddr +} + func EthAddressFromFilecoinAddress(addr address.Address) (EthAddress, error) { switch addr.Protocol() { case address.ID: @@ -356,10 +363,7 @@ func EthAddressFromFilecoinAddress(addr address.Address) (EthAddress, error) { if err != nil { return EthAddress{}, err } - var ethaddr EthAddress - ethaddr[0] = 0xff - binary.BigEndian.PutUint64(ethaddr[12:], id) - return ethaddr, nil + return EthAddressFromActorID(abi.ActorID(id)), nil case address.Delegated: payload := addr.Payload() namespace, n, err := varint.FromUvarint(payload) @@ -987,13 +991,8 @@ type EthTrace struct { Result EthTraceResult `json:"result"` Subtraces int `json:"subtraces"` TraceAddress []int `json:"traceAddress"` - Type string `json:"Type"` - - Parent *EthTrace `json:"-"` - - // if a subtrace makes a call to GetBytecode, we store a pointer to that subtrace here - // which we then lookup when checking for delegatecall (InvokeContractDelegate) - LastByteCode *EthTrace `json:"-"` + Type string `json:"type"` + Error string `json:"error,omitempty"` } func (t *EthTrace) SetCallType(callType string) { @@ -1018,17 +1017,12 @@ type EthTraceReplayBlockTransaction struct { } type EthTraceAction struct { - CallType string `json:"callType"` - From EthAddress `json:"from"` - To EthAddress `json:"to"` - Gas EthUint64 `json:"gas"` - Input EthBytes `json:"input"` - Value EthBigInt `json:"value"` - - FilecoinMethod abi.MethodNum `json:"-"` - FilecoinCodeCid cid.Cid `json:"-"` - FilecoinFrom address.Address `json:"-"` - FilecoinTo address.Address `json:"-"` + CallType string `json:"callType"` + From EthAddress `json:"from"` + To *EthAddress `json:"to"` + Gas EthUint64 `json:"gas"` + Input EthBytes `json:"input"` + Value EthBigInt `json:"value"` } type EthTraceResult struct { diff --git a/node/impl/full/eth.go b/node/impl/full/eth.go index cbe491afe1c..5d5633365eb 100644 --- a/node/impl/full/eth.go +++ b/node/impl/full/eth.go @@ -20,7 +20,6 @@ import ( "github.com/filecoin-project/go-jsonrpc" "github.com/filecoin-project/go-state-types/abi" "github.com/filecoin-project/go-state-types/big" - "github.com/filecoin-project/go-state-types/builtin" builtintypes "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" @@ -875,19 +874,21 @@ func (a *EthModule) EthTraceBlock(ctx context.Context, blkNum string) ([]*ethtyp return nil, xerrors.Errorf("failed to get transaction hash by cid: %w", err) } if txHash == nil { - log.Warnf("cannot find transaction hash for cid %s", ir.MsgCid) - continue + return nil, xerrors.Errorf("cannot find transaction hash for cid %s", ir.MsgCid) + } + + env, err := baseEnvironment(st, ir.Msg.From) + if err != nil { + return nil, xerrors.Errorf("when processing message %s: %w", err) } - traces := []*ethtypes.EthTrace{} - err = buildTraces(&traces, nil, []int{}, ir.ExecutionTrace, int64(ts.Height()), st) + err = buildTraces(env, nil, &ir.ExecutionTrace) if err != nil { return nil, xerrors.Errorf("failed building traces: %w", err) } - traceBlocks := make([]*ethtypes.EthTraceBlock, 0, len(traces)) - for _, trace := range traces { - traceBlocks = append(traceBlocks, ðtypes.EthTraceBlock{ + for _, trace := range env.traces { + allTraces = append(allTraces, ðtypes.EthTraceBlock{ EthTrace: trace, BlockHash: blkHash, BlockNumber: int64(ts.Height()), @@ -895,8 +896,6 @@ func (a *EthModule) EthTraceBlock(ctx context.Context, blkNum string) ([]*ethtyp TransactionPosition: msgIdx, }) } - - allTraces = append(allTraces, traceBlocks...) } return allTraces, nil @@ -934,34 +933,31 @@ func (a *EthModule) EthTraceReplayBlockTransactions(ctx context.Context, blkNum return nil, xerrors.Errorf("failed to get transaction hash by cid: %w", err) } if txHash == nil { - log.Warnf("cannot find transaction hash for cid %s", ir.MsgCid) - continue - } - - var output ethtypes.EthBytes - invokeCreateOnEAM := ir.Msg.To == builtin.EthereumAddressManagerActorAddr && (ir.Msg.Method == builtin.MethodsEAM.Create || ir.Msg.Method == builtin.MethodsEAM.Create2) - if ir.Msg.Method == builtin.MethodsEVM.InvokeContract || invokeCreateOnEAM { - output, err = decodePayload(ir.ExecutionTrace.MsgRct.Return, ir.ExecutionTrace.MsgRct.ReturnCodec) - if err != nil { - return nil, xerrors.Errorf("failed to decode payload: %w", err) - } - } else { - output = encodeFilecoinReturnAsABI(ir.ExecutionTrace.MsgRct.ExitCode, ir.ExecutionTrace.MsgRct.ReturnCodec, ir.ExecutionTrace.MsgRct.Return) + return nil, xerrors.Errorf("cannot find transaction hash for cid %s", ir.MsgCid) } - t := ethtypes.EthTraceReplayBlockTransaction{ - Output: output, - TransactionHash: *txHash, - StateDiff: nil, - VmTrace: nil, + env, err := baseEnvironment(st, ir.Msg.From) + if err != nil { + return nil, xerrors.Errorf("when processing message %s: %w", err) } - err = buildTraces(&t.Trace, nil, []int{}, ir.ExecutionTrace, int64(ts.Height()), st) + err = buildTraces(env, nil, &ir.ExecutionTrace) if err != nil { return nil, xerrors.Errorf("failed building traces: %w", err) } - allTraces = append(allTraces, &t) + var output []byte + if len(env.traces) > 0 { + output = env.traces[0].Result.Output + } + + allTraces = append(allTraces, ðtypes.EthTraceReplayBlockTransaction{ + Output: output, + TransactionHash: *txHash, + Trace: env.traces, + StateDiff: nil, + VmTrace: nil, + }) } return allTraces, nil diff --git a/node/impl/full/eth_trace.go b/node/impl/full/eth_trace.go index 986de034a1d..ac8c7011744 100644 --- a/node/impl/full/eth_trace.go +++ b/node/impl/full/eth_trace.go @@ -7,8 +7,11 @@ import ( cbg "github.com/whyrusleeping/cbor-gen" "golang.org/x/xerrors" + "github.com/filecoin-project/go-address" "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/builtin/v12/eam" + "github.com/filecoin-project/go-state-types/builtin/v12/evm" + "github.com/filecoin-project/go-state-types/exitcode" builtinactors "github.com/filecoin-project/lotus/chain/actors/builtin" "github.com/filecoin-project/lotus/chain/state" @@ -38,212 +41,438 @@ func decodePayload(payload []byte, codec uint64) (ethtypes.EthBytes, error) { return nil, xerrors.Errorf("decodePayload: unsupported codec: %d", codec) } -// buildTraces recursively builds the traces for a given ExecutionTrace by walking the subcalls -func buildTraces(traces *[]*ethtypes.EthTrace, parent *ethtypes.EthTrace, addr []int, et types.ExecutionTrace, height int64, st *state.StateTree) error { - // lookup the eth address from the from/to addresses. Note that this may fail but to support - // this we need to include the ActorID in the trace. For now, just log a warning and skip - // this trace. - // - // TODO: Add ActorID in trace, see https://github.com/filecoin-project/lotus/pull/11100#discussion_r1302442288 - from, err := lookupEthAddress(et.Msg.From, st) +func decodeParams[P any, T interface { + *P + cbg.CBORUnmarshaler +}](msg *types.MessageTrace) (T, error) { + var params T = new(P) + switch msg.ParamsCodec { + case uint64(multicodec.DagCbor), uint64(multicodec.Cbor): + default: + return nil, xerrors.Errorf("Method called with unexpected codec %d", msg.ParamsCodec) + } + + if err := params.UnmarshalCBOR(bytes.NewReader(msg.Params)); err != nil { + return nil, xerrors.Errorf("failed to decode params: %w", err) + } + + return params, nil +} + +func find[T any](values []T, cb func(t *T) *T) *T { + for i := range values { + if o := cb(&values[i]); o != nil { + return o + } + } + return nil +} + +type environment struct { + caller ethtypes.EthAddress + isEVM bool + subtraceCount int + traces []*ethtypes.EthTrace + lastByteCode *ethtypes.EthAddress +} + +func baseEnvironment(st *state.StateTree, from address.Address) (*environment, error) { + sender, err := lookupEthAddress(from, st) if err != nil { - log.Warnf("buildTraces: failed to lookup from address %s: %v", et.Msg.From, err) - return nil + return nil, xerrors.Errorf("top-level message sender %s s could not be found: %w", from, err) } - to, err := lookupEthAddress(et.Msg.To, st) + return &environment{caller: sender}, nil +} + +func traceToAddress(act *types.ActorTrace) ethtypes.EthAddress { + if act.State.Address != nil { + if addr, err := ethtypes.EthAddressFromFilecoinAddress(*act.State.Address); err != nil { + return addr + } + } + return ethtypes.EthAddressFromActorID(act.Id) +} + +// buildTraces recursively builds the traces for a given ExecutionTrace by walking the subcalls +func buildTraces(env *environment, addr []int, et *types.ExecutionTrace) error { + trace, recurseInto, err := buildTrace(env, addr, et) if err != nil { - log.Warnf("buildTraces: failed to lookup to address %s: %w", et.Msg.To, err) + return xerrors.Errorf("at trace %v: %w", addr, err) + } + + if trace != nil { + env.traces = append(env.traces, trace) + env.subtraceCount++ + } + + // Skip if there's nothing more to do and/or `buildTrace` told us to skip this one. + if recurseInto == nil || recurseInto.InvokedActor == nil || len(recurseInto.Subcalls) == 0 { return nil } + subEnv := &environment{ + caller: traceToAddress(recurseInto.InvokedActor), + isEVM: builtinactors.IsEvmActor(recurseInto.InvokedActor.State.Code), + } + for i := range recurseInto.Subcalls { + err := buildTraces(subEnv, append(addr[:len(addr):len(addr)], i), &recurseInto.Subcalls[i]) + if err != nil { + return err + } + } + trace.Subtraces = subEnv.subtraceCount + + return nil +} + +func decodeCreate(msg *types.MessageTrace) (callType string, initcode []byte, err error) { + switch msg.Method { + case builtin.MethodsEAM.Create: + params, err := decodeParams[eam.CreateParams](msg) + if err != nil { + return "", nil, err + } + return "create", params.Initcode, nil + case builtin.MethodsEAM.Create2: + params, err := decodeParams[eam.Create2Params](msg) + if err != nil { + return "", nil, err + } + return "create2", params.Initcode, nil + case builtin.MethodsEAM.CreateExternal: + input, err := decodePayload(msg.Params, msg.ParamsCodec) + if err != nil { + return "", nil, err + } + return "create", input, nil + default: + return "", nil, xerrors.Errorf("unexpected CREATE method %d", msg.Method) + } +} + +// buildTrace processes the passed execution trace and updates the environment, if necessary. +// +// On success, it returns a trace to add (or nil to skip) and the trace recurse into (or nil to skip). +func buildTrace(env *environment, addr []int, et *types.ExecutionTrace) (*ethtypes.EthTrace, *types.ExecutionTrace, error) { + // This function first assumes that the call is a "native" call, then handles all the "not + // native" cases. If we get any unexpected results in any of these special cases, we just + // keep the "native" interpretation and move on. + // + // 1. If we're invoking a contract (even if the caller is a native account/actor), we + // attempt to decode the params/return value as a contract invocation. + // 2. If we're calling the EAM and/or init actor, we try to treat the call as a CREATE. + // 3. Finally, if the caller is an EVM smart contract and it's calling a "private" (1-1023) + // method, we know something special is going on. We look for calls related to + // DELEGATECALL and drop everything else (everything else includes calls triggered by, + // e.g., EXTCODEHASH). + + // If we don't have sufficient funds, or we have a fatal error, or we have some + // other syscall error: skip the entire trace to mimic Ethereum (Ethereum records + // traces _after_ checking things like this). + // + // NOTE: The FFI currently folds all unknown syscall errors into "sys assertion + // failed" which is turned into SysErrFatal. + if len(addr) > 0 { + switch et.MsgRct.ExitCode { + case exitcode.SysErrInsufficientFunds, exitcode.SysErrFatal: + return nil, nil, nil + } + } + + // We may fail before we can even invoke the actor. In that case, we have no 100% reliable + // way of getting its address (e.g., due to reverts) so we're just going to drop the entire + // trace. This is OK (ish) because the call never really "happened". + if et.InvokedActor == nil { + return nil, nil, nil + } + + ///////////////////////////////////// + // Step 1: Decode as a native call // + ///////////////////////////////////// + + to := traceToAddress(et.InvokedActor) + var errMsg string + if et.MsgRct.ExitCode.IsError() { + errMsg = et.MsgRct.ExitCode.Error() + } trace := ðtypes.EthTrace{ Action: ethtypes.EthTraceAction{ - From: from, - To: to, + From: env.caller, + To: &to, Gas: ethtypes.EthUint64(et.Msg.GasLimit), - Input: nil, Value: ethtypes.EthBigInt(et.Msg.Value), - - FilecoinFrom: et.Msg.From, - FilecoinTo: et.Msg.To, - FilecoinMethod: et.Msg.Method, - FilecoinCodeCid: et.InvokedActor.State.Code, + Input: encodeFilecoinParamsAsABI(et.Msg.Method, et.Msg.ParamsCodec, et.Msg.Params), }, Result: ethtypes.EthTraceResult{ GasUsed: ethtypes.EthUint64(et.SumGas().TotalGas), - Output: nil, + Output: encodeFilecoinReturnAsABI(et.MsgRct.ExitCode, et.MsgRct.ReturnCodec, et.MsgRct.Return), }, - Subtraces: 0, // will be updated by the children once they are added to the trace TraceAddress: addr, + Error: errMsg, + } - Parent: parent, - LastByteCode: nil, + // Set the assumed call mode. We'll override this if the call ends up looking like a create, + // delegatecall, etc. + if et.Msg.ReadOnly { + trace.SetCallType("staticcall") + } else { + trace.SetCallType("call") } - trace.SetCallType("call") + ///////////////////////////////////////////// + // Step 2: Decode as a contract invocation // + ///////////////////////////////////////////// + // Normal EVM calls. We don't care if the caller/receiver are actually EVM actors, we only + // care if the call _looks_ like an EVM call. If we fail to decode it as an EVM call, we + // fallback on interpreting it as a native call. if et.Msg.Method == builtin.MethodsEVM.InvokeContract { - log.Debugf("COND1 found InvokeContract call at height: %d", height) - - // TODO: ignore return errors since actors can send gibberish and we don't want - // to fail the whole trace in that case - trace.Action.Input, err = decodePayload(et.Msg.Params, et.Msg.ParamsCodec) - if err != nil { - return xerrors.Errorf("buildTraces: %w", err) - } - trace.Result.Output, err = decodePayload(et.MsgRct.Return, et.MsgRct.ReturnCodec) + input, err := decodePayload(et.Msg.Params, et.Msg.ParamsCodec) if err != nil { - return xerrors.Errorf("buildTraces: %w", err) + log.Debugf("failed to decode contract invocation payload: %w", err) + return trace, et, nil } - } else if et.Msg.To == builtin.EthereumAddressManagerActorAddr && - et.Msg.Method == builtin.MethodsEAM.CreateExternal { - log.Debugf("COND2 found CreateExternal call at height: %d", height) - trace.Action.Input, err = decodePayload(et.Msg.Params, et.Msg.ParamsCodec) + output, err := decodePayload(et.MsgRct.Return, et.MsgRct.ReturnCodec) if err != nil { - return xerrors.Errorf("buildTraces: %w", err) + log.Debugf("failed to decode contract invocation return: %w", err) + return trace, et, nil } + trace.Action.Input = input + trace.Result.Output = output + return trace, et, nil + } - if et.MsgRct.ExitCode.IsSuccess() { - // ignore return value - trace.Result.Output = nil - } else { - // return value is the error message - trace.Result.Output, err = decodePayload(et.MsgRct.Return, et.MsgRct.ReturnCodec) - if err != nil { - return xerrors.Errorf("buildTraces: %w", err) + ///////////////////////////////////////////// + // Step 3: Decode as a contract deployment // + ///////////////////////////////////////////// + + switch et.Msg.To { + // NOTE: this will only catch _direct_ calls to the init actor. Calls through the EAM will + // be caught and _skipped_ below in the next case. + case builtin.InitActorAddr: + switch et.Msg.Method { + case builtin.MethodsInit.Exec, builtin.MethodsInit.Exec4: + if et.Msg.ReadOnly { + // "create" isn't valid in a staticcall, so we just skip this trace + // (couldn't have created an actor anyways). + // This mimic's the EVM: it doesn't trace CREATE calls when in + // read-only mode. + return nil, nil, nil } - } - // treat this as a contract creation - trace.SetCallType("create") - } 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 = 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)? - if et.Msg.ReadOnly { - trace.SetCallType("staticcall") - } + subTrace := find(et.Subcalls, func(c *types.ExecutionTrace) *types.ExecutionTrace { + if c.Msg.Method == builtin.MethodConstructor { + return c + } + return nil + }) + if subTrace == nil { + // If we succeed in calling Exec/Exec4 but don't even try to construct + // something, we have a bug in our tracing logic or a mismatch between our + // tracing logic and the actors. + if et.MsgRct.ExitCode.IsSuccess() { + return nil, nil, xerrors.Errorf("successful Exec/Exec4 call failed to call a constructor") + } + // Otherwise, this can happen if creation fails early (bad params, + // out of gas, contract already exists, etc.). The EVM wouldn't + // trace such cases, so we don't either. + // + // NOTE: It's actually impossible to run out of gas before calling + // initcode in the EVM (without running out of gas in the calling + // contract), but this is an equivalent edge-case to InvokedActor + // being nil, so we treat it the same way and skip the entire + // operation. + return nil, nil, nil + } - // there are several edge cases that require special handling when displaying the traces. Note that while iterating over - // the traces we update the trace backwards (through the parent pointer) - if parent != nil { - // Handle Native actor creation - // - // Actor A calls to the init actor on method 2 and The init actor creates the target actor B then calls it on method 1 - if parent.Action.FilecoinTo == builtin.InitActorAddr && - parent.Action.FilecoinMethod == builtin.MethodsInit.Exec && - et.Msg.Method == builtin.MethodConstructor { - log.Debugf("COND3 Native actor creation! method:%d, code:%s, height:%d", et.Msg.Method, et.InvokedActor.State.Code.String(), height) - parent.SetCallType("create") - parent.Action.To = to - parent.Action.Input = []byte{0xFE} - parent.Result.Output = nil - - // there should never be any subcalls when creating a native actor - // - // TODO: add support for native actors calling another when created - return nil - } + // Native actors that aren't the EAM can attempt to call Exec4, but such + // call should fail immediately without ever attempting to construct an + // actor. I'm catching this here because it likely means that there's a bug + // in our trace-conversion logic. + if et.Msg.Method == builtin.MethodsInit.Exec4 { + return nil, nil, xerrors.Errorf("direct call to Exec4 successfully called a constructor!") + } - // Handle EVM contract creation - // - // To detect EVM contract creation we need to check for the following sequence of events: - // - // 1) EVM contract A calls the EAM (Ethereum Address Manager) on method 2 (create) or 3 (create2). - // 2) The EAM calls the init actor on method 3 (Exec4). - // 3) The init actor creates the target actor B then calls it on method 1. - if parent.Parent != nil { - calledCreateOnEAM := parent.Parent.Action.FilecoinTo == builtin.EthereumAddressManagerActorAddr && - (parent.Parent.Action.FilecoinMethod == builtin.MethodsEAM.Create || parent.Parent.Action.FilecoinMethod == builtin.MethodsEAM.Create2) - eamCalledInitOnExec4 := parent.Action.FilecoinTo == builtin.InitActorAddr && - parent.Action.FilecoinMethod == builtin.MethodsInit.Exec4 - initCreatedActor := trace.Action.FilecoinMethod == builtin.MethodConstructor - - // TODO: We need to handle failures in contract creations and support resurrections on an existing but dead EVM actor) - if calledCreateOnEAM && eamCalledInitOnExec4 && initCreatedActor { - log.Debugf("COND4 EVM contract creation method:%d, code:%s, height:%d", et.Msg.Method, et.InvokedActor.State.Code.String(), height) - - if parent.Parent.Action.FilecoinMethod == builtin.MethodsEAM.Create { - parent.Parent.SetCallType("create") + // Contract creation has no "to". + trace.Action.To = nil + // If we get here, this isn't a native EVM create. Those always go through + // the EAM. So we have no "real" initcode and must use the sentinel value + // for "invalid" initcode. + trace.Action.Input = []byte{0xFE} + trace.SetCallType("create") + // Handle the output. + if et.MsgRct.ExitCode.IsError() { + // If the sub-call fails, record the reason. It's possible for the + // call to the init actor to fail, but for the construction to + // succeed, so we need to check the exit code here. + if subTrace.MsgRct.ExitCode.IsError() { + trace.Result.Output = encodeFilecoinReturnAsABI( + subTrace.MsgRct.ExitCode, + subTrace.MsgRct.ReturnCodec, + subTrace.MsgRct.Return, + ) } else { - parent.Parent.SetCallType("create2") + // Otherwise, output nothing. + trace.Result.Output = nil } + } else { + // We're supposed to put the "installed bytecode" here. But this + // isn't an EVM actor, so we just put some invalid bytecode (this is + // the answer you'd get if you called EXTCODECOPY on a native + // non-account actor, anyways). + trace.Result.Output = []byte{0xFE} + } + return trace, subTrace, nil + } + case builtin.EthereumAddressManagerActorAddr: + switch et.Msg.Method { + case builtin.MethodsEAM.Create, builtin.MethodsEAM.Create2, builtin.MethodsEAM.CreateExternal: + // Same as the Init actor case above, see the comment there. + if et.Msg.ReadOnly { + return nil, nil, nil + } - // update the parent.parent to make this - parent.Parent.Action.To = trace.Action.To - parent.Parent.Subtraces = 0 - - // delete the parent (the EAM) and skip the current trace (init) - *traces = (*traces)[:len(*traces)-1] - + // Look for a call to either a constructor or the EVM's resurrect method. + subTrace := find(et.Subcalls, func(et *types.ExecutionTrace) *types.ExecutionTrace { + if et.Msg.To == builtinactors.InitActorAddr { + return find(et.Subcalls, func(et *types.ExecutionTrace) *types.ExecutionTrace { + if et.Msg.Method == builtinactors.MethodConstructor { + return et + } + return nil + }) + } + if et.Msg.Method == builtin.MethodsEVM.Resurrect { + return et + } return nil + }) + + // Same as the Init actor case above, see the comment there. + if subTrace == nil { + if et.MsgRct.ExitCode.IsSuccess() { + return nil, nil, xerrors.Errorf("successful Create/Create2 call failed to call a constructor") + } + return nil, nil, nil } - } - if builtinactors.IsEvmActor(parent.Action.FilecoinCodeCid) { - // Handle delegate calls - // - // 1) Look for trace from an EVM actor to itself on InvokeContractDelegate, method 6. - // 2) Check that the previous trace calls another actor on method 3 (GetByteCode) and they are at the same level (same parent) - // 3) Treat this as a delegate call to actor A. - if parent.LastByteCode != nil && trace.Action.From == trace.Action.To && - trace.Action.FilecoinMethod == builtin.MethodsEVM.InvokeContractDelegate { - log.Debugf("COND7 found delegate call, height: %d", height) - prev := parent.LastByteCode - if prev.Action.From == trace.Action.From && prev.Action.FilecoinMethod == builtin.MethodsEVM.GetBytecode && prev.Parent == trace.Parent { - trace.SetCallType("delegatecall") - trace.Action.To = prev.Action.To - - var dp evm.DelegateCallParams - err := dp.UnmarshalCBOR(bytes.NewReader(et.Msg.Params)) - if err != nil { - return xerrors.Errorf("failed UnmarshalCBOR: %w", err) - } - trace.Action.Input = dp.Input + // Contract creation has no "to". + trace.Action.To = nil - trace.Result.Output, err = decodePayload(et.MsgRct.Return, et.MsgRct.ReturnCodec) + // Decode inputs & determine create type. + method, initcode, err := decodeCreate(&et.Msg) + if err != nil { + return nil, nil, xerrors.Errorf("EAM called with invalid params, but it still tried to construct the contract: %w", err) + } + trace.Action.Input = initcode + trace.SetCallType(method) + + // Handle the output. + if et.MsgRct.ExitCode.IsError() { + if subTrace.MsgRct.ExitCode.IsError() { + // If we managed to call the constructor, parse/return its + // revert message. + output, err := decodePayload(subTrace.MsgRct.Return, subTrace.MsgRct.ReturnCodec) if err != nil { - return xerrors.Errorf("failed decodePayload: %w", err) + log.Debugf("EVM actor returned indecipherable create error: %w", err) + output = encodeFilecoinReturnAsABI( + subTrace.MsgRct.ExitCode, + subTrace.MsgRct.ReturnCodec, + subTrace.MsgRct.Return, + ) } + trace.Result.Output = output + } else { + // Otherwise, if we failed before that, we have no revert + // message. + trace.Result.Output = nil } } else { - // Handle EVM call special casing - // - // Any outbound call from an EVM actor on methods 1-1023 are side-effects from EVM instructions - // and should be dropped from the trace. - if et.Msg.Method > 0 && - et.Msg.Method <= 1023 { - log.Debugf("Infof found outbound call from an EVM actor on method 1-1023 method:%d, code:%s, height:%d", et.Msg.Method, parent.Action.FilecoinCodeCid.String(), height) - - if et.Msg.Method == builtin.MethodsEVM.GetBytecode { - // save the last bytecode trace to handle delegate calls - parent.LastByteCode = trace - } - - return nil - } + // We're _supposed_ to include the contracts bytecode here, but we + // can't do that reliably (e.g., if some part of the trace reverts). + // So we don't try and include a sentinel "impossible bytecode" + // value (the value specified by EIP-3541). + trace.Result.Output = []byte{0xFE} } + return trace, subTrace, nil } - } - // we are adding trace to the traces so update the parent subtraces count as it was originally set to zero - if parent != nil { - parent.Subtraces++ - } + ///////////////////////////////// + // Step 4: Handle DELEGATECALL // + ///////////////////////////////// - *traces = append(*traces, trace) + // EVM contracts cannot call methods in the range 1-1023, only the EVM itself can. So, if we + // see a call in this range, we know it's an implementation detail of the EVM and not an + // explicit call by the user. + // + // While the EVM calls several methods in this range (some we've already handled above with + // respect to the EAM), we only care about the ones relevant DELEGATECALL and can _ignore_ + // all the others. + if env.isEVM && et.Msg.Method > 0 && et.Msg.Method < 1024 { + // The EVM actor implements DELEGATECALL by: + // + // 1. Asking the callee for its bytecode by calling it on the GetBytecode method. + // 2. Recursively invoking the currently executing contract on the + // InvokeContractDelegate method. + // + // The code below "reconstructs" that delegate call by: + // + // 1. Remembering the last contract on which we called GetBytecode. + // 2. Treating the contract invoked in step 1 as the DELEGATECALL receiver. + // + // Note, however: GetBytecode will be called, e.g., if the user invokes the + // EXTCODECOPY instruction. It's not an error to see multiple GetBytecode calls + // before we see an InvokeContractDelegate. + switch et.Msg.Method { + case builtin.MethodsEVM.GetBytecode: + // NOTE: I'm not checking anything about the receiver here. The EVM won't + // DELEGATECALL any non-EVM actor, but there's no need to encode that fact + // here in case we decide to loosen this up in the future. + if et.MsgRct.ExitCode.IsSuccess() { + env.lastByteCode = &to + } else { + env.lastByteCode = nil + } + return nil, nil, nil + case builtin.MethodsEVM.InvokeContractDelegate: + // NOTE: We return errors in all the failure cases below instead of trying + // to continue because the caller is an EVM actor. If something goes wrong + // here, there's a bug in our EVM implementation. - for i, call := range et.Subcalls { - err := buildTraces(traces, trace, append(addr, i), call, height, st) - if err != nil { - return err + // Handle delegate calls + // + // 1) Look for trace from an EVM actor to itself on InvokeContractDelegate, + // method 6. + // 2) Check that the previous trace calls another actor on method 3 + // (GetByteCode) and they are at the same level (same parent) + // 3) Treat this as a delegate call to actor A. + if env.lastByteCode == nil { + return nil, nil, xerrors.Errorf("unknown bytecode for delegate call") + } + if &trace.Action.From != trace.Action.To { + return nil, nil, xerrors.Errorf("delegate-call not from & to self") + } + + trace.SetCallType("delegatecall") + trace.Action.To = env.lastByteCode + + dp, err := decodeParams[evm.DelegateCallParams](&et.Msg) + if err != nil { + return nil, nil, xerrors.Errorf("failed to decode delegate-call params: %w", err) + } + trace.Action.Input = dp.Input + + trace.Result.Output, err = decodePayload(et.MsgRct.Return, et.MsgRct.ReturnCodec) + if err != nil { + return nil, nil, xerrors.Errorf("failed to decode delegate-call return: %w", err) + } + + return trace, et, nil } + // We drop all other "private" calls from FEVM. We _forbid_ explicit calls between 0 and 1024 (exclusive), so any calls in this range must be implementation details. + return nil, nil, nil } - return nil + return trace, et, nil } diff --git a/node/impl/full/eth_utils.go b/node/impl/full/eth_utils.go index 90e6d054a9c..6186c2645f9 100644 --- a/node/impl/full/eth_utils.go +++ b/node/impl/full/eth_utils.go @@ -382,34 +382,35 @@ func parseEthRevert(ret []byte) string { // 3. Otherwise, we fall back to returning a masked ID Ethereum address. If the supplied address is an f0 address, we // use that ID to form the masked ID address. // 4. Otherwise, we fetch the actor's ID from the state tree and form the masked ID with it. +// +// If the actor doesn't exist in the state-tree but we have its ID, we use a masked ID address. It could have been deleted. func lookupEthAddress(addr address.Address, st *state.StateTree) (ethtypes.EthAddress, error) { - // BLOCK A: We are trying to get an actual Ethereum address from an f410 address. // Attempt to convert directly, if it's an f4 address. ethAddr, err := ethtypes.EthAddressFromFilecoinAddress(addr) if err == nil && !ethAddr.IsMaskedID() { return ethAddr, nil } - // Lookup on the target actor and try to get an f410 address. - if actor, err := st.GetActor(addr); err != nil { + // Otherwise, resolve the ID addr. + idAddr, err := st.LookupID(addr) + if err != nil { return ethtypes.EthAddress{}, err - } else if actor.Address != nil { - if ethAddr, err := ethtypes.EthAddressFromFilecoinAddress(*actor.Address); err == nil && !ethAddr.IsMaskedID() { - return ethAddr, nil - } } - // BLOCK B: We gave up on getting an actual Ethereum address and are falling back to a Masked ID address. - // Check if we already have an ID addr, and use it if possible. - if err == nil && ethAddr.IsMaskedID() { + // Lookup on the target actor and try to get an f410 address. + if actor, err := st.GetActor(idAddr); errors.Is(err, types.ErrActorNotFound) { + // Not found -> use a masked ID address + } else if err != nil { + // Any other error -> fail. + return ethtypes.EthAddress{}, err + } else if actor.Address == nil { + // No delegated address -> use masked ID address. + } else if ethAddr, err := ethtypes.EthAddressFromFilecoinAddress(*actor.Address); err == nil && !ethAddr.IsMaskedID() { + // Conversable into an eth address, use it. return ethAddr, nil } - // Otherwise, resolve the ID addr. - idAddr, err := st.LookupID(addr) - if err != nil { - return ethtypes.EthAddress{}, err - } + // Otherwise, use the masked address. return ethtypes.EthAddressFromFilecoinAddress(idAddr) }