From 48287574d82c81ffc50f259134c6596add32a2f5 Mon Sep 17 00:00:00 2001 From: Aarsh Shah Date: Tue, 11 Jun 2024 19:07:17 +0400 Subject: [PATCH] fix: ETH RPC API: ETH Call should use the parent state root of the subsequent tipset (#11905) * fix eth call * tests * changes as per review * changes as per review * Update node/impl/full/eth.go Co-authored-by: Rod Vagg * fix as per review --------- Co-authored-by: Rod Vagg --- chain/stmgr/call.go | 92 +++++++++++++++++++++++++++---------------- chain/stmgr/forks.go | 2 +- node/impl/full/eth.go | 29 +++++++------- 3 files changed, 74 insertions(+), 49 deletions(-) diff --git a/chain/stmgr/call.go b/chain/stmgr/call.go index 61056528f11..7f2a57a6112 100644 --- a/chain/stmgr/call.go +++ b/chain/stmgr/call.go @@ -25,6 +25,14 @@ import ( "github.com/filecoin-project/lotus/chain/vm" ) +type execMessageStrategy int + +const ( + execNoMessages execMessageStrategy = iota // apply no prior or current tipset messages + execAllMessages // apply all prior and current tipset messages + execSameSenderMessages // apply all prior messages and any current tipset messages from the same sender +) + var ErrExpensiveFork = errors.New("refusing explicit call due to state fork at epoch") // Call applies the given message to the given tipset's parent state, at the epoch following the @@ -48,12 +56,24 @@ func (sm *StateManager) Call(ctx context.Context, msg *types.Message, ts *types. msg.Value = types.NewInt(0) } - return sm.callInternal(ctx, msg, nil, ts, cid.Undef, sm.GetNetworkVersion, false, false) + return sm.callInternal(ctx, msg, nil, ts, cid.Undef, sm.GetNetworkVersion, false, execSameSenderMessages) +} + +// ApplyOnStateWithGas applies the given message on top of the given state root with gas tracing enabled +func (sm *StateManager) ApplyOnStateWithGas(ctx context.Context, stateCid cid.Cid, msg *types.Message, ts *types.TipSet) (*api.InvocResult, error) { + return sm.callInternal(ctx, msg, nil, ts, stateCid, sm.GetNetworkVersion, true, execNoMessages) } // CallWithGas calculates the state for a given tipset, and then applies the given message on top of that state. func (sm *StateManager) CallWithGas(ctx context.Context, msg *types.Message, priorMsgs []types.ChainMsg, ts *types.TipSet, applyTsMessages bool) (*api.InvocResult, error) { - return sm.callInternal(ctx, msg, priorMsgs, ts, cid.Undef, sm.GetNetworkVersion, true, applyTsMessages) + var strategy execMessageStrategy + if applyTsMessages { + strategy = execAllMessages + } else { + strategy = execSameSenderMessages + } + + return sm.callInternal(ctx, msg, priorMsgs, ts, cid.Undef, sm.GetNetworkVersion, true, strategy) } // CallAtStateAndVersion allows you to specify a message to execute on the given stateCid and network version. @@ -64,14 +84,14 @@ func (sm *StateManager) CallAtStateAndVersion(ctx context.Context, msg *types.Me nvGetter := func(context.Context, abi.ChainEpoch) network.Version { return v } - - return sm.callInternal(ctx, msg, nil, nil, stateCid, nvGetter, true, false) + return sm.callInternal(ctx, msg, nil, nil, stateCid, nvGetter, true, execSameSenderMessages) } // - If no tipset is specified, the first tipset without an expensive migration or one in its parent is used. // - If executing a message at a given tipset or its parent would trigger an expensive migration, the call will // fail with ErrExpensiveFork. -func (sm *StateManager) callInternal(ctx context.Context, msg *types.Message, priorMsgs []types.ChainMsg, ts *types.TipSet, stateCid cid.Cid, nvGetter rand.NetworkVersionGetter, checkGas, applyTsMessages bool) (*api.InvocResult, error) { +func (sm *StateManager) callInternal(ctx context.Context, msg *types.Message, priorMsgs []types.ChainMsg, ts *types.TipSet, stateCid cid.Cid, + nvGetter rand.NetworkVersionGetter, checkGas bool, strategy execMessageStrategy) (*api.InvocResult, error) { ctx, span := trace.StartSpan(ctx, "statemanager.callInternal") defer span.End() @@ -95,7 +115,7 @@ func (sm *StateManager) callInternal(ctx context.Context, msg *types.Message, pr return nil, xerrors.Errorf("failed to find a non-forking epoch: %w", err) } // Checks for expensive forks from the parents to the tipset, including nil tipsets - if !sm.hasExpensiveForkBetween(pts.Height(), ts.Height()+1) { + if !sm.HasExpensiveForkBetween(pts.Height(), ts.Height()+1) { break } @@ -106,7 +126,7 @@ func (sm *StateManager) callInternal(ctx context.Context, msg *types.Message, pr if err != nil { return nil, xerrors.Errorf("failed to find a non-forking epoch: %w", err) } - if sm.hasExpensiveForkBetween(pts.Height(), ts.Height()+1) { + if sm.HasExpensiveForkBetween(pts.Height(), ts.Height()+1) { return nil, ErrExpensiveFork } } @@ -117,24 +137,6 @@ func (sm *StateManager) callInternal(ctx context.Context, msg *types.Message, pr if stateCid == cid.Undef { stateCid = ts.ParentState() } - tsMsgs, err := sm.cs.MessagesForTipset(ctx, ts) - if err != nil { - return nil, xerrors.Errorf("failed to lookup messages for parent tipset: %w", err) - } - - if applyTsMessages { - priorMsgs = append(tsMsgs, priorMsgs...) - } else { - var filteredTsMsgs []types.ChainMsg - for _, tsMsg := range tsMsgs { - //TODO we should technically be normalizing the filecoin address of from when we compare here - if tsMsg.VMMessage().From == msg.VMMessage().From { - filteredTsMsgs = append(filteredTsMsgs, tsMsg) - } - } - priorMsgs = append(filteredTsMsgs, priorMsgs...) - } - // Technically, the tipset we're passing in here should be ts+1, but that may not exist. stateCid, err = sm.HandleStateForks(ctx, stateCid, ts.Height(), nil, ts) if err != nil { @@ -169,18 +171,40 @@ func (sm *StateManager) callInternal(ctx context.Context, msg *types.Message, pr if err != nil { return nil, xerrors.Errorf("failed to set up vm: %w", err) } - for i, m := range priorMsgs { - _, err = vmi.ApplyMessage(ctx, m) + + switch strategy { + case execNoMessages: + // Do nothing + case execAllMessages, execSameSenderMessages: + tsMsgs, err := sm.cs.MessagesForTipset(ctx, ts) if err != nil { - return nil, xerrors.Errorf("applying prior message (%d, %s): %w", i, m.Cid(), err) + return nil, xerrors.Errorf("failed to lookup messages for parent tipset: %w", err) + } + if strategy == execAllMessages { + priorMsgs = append(tsMsgs, priorMsgs...) + } else if strategy == execSameSenderMessages { + var filteredTsMsgs []types.ChainMsg + for _, tsMsg := range tsMsgs { + //TODO we should technically be normalizing the filecoin address of from when we compare here + if tsMsg.VMMessage().From == msg.VMMessage().From { + filteredTsMsgs = append(filteredTsMsgs, tsMsg) + } + } + priorMsgs = append(filteredTsMsgs, priorMsgs...) + } + for i, m := range priorMsgs { + _, err = vmi.ApplyMessage(ctx, m) + if err != nil { + return nil, xerrors.Errorf("applying prior message (%d, %s): %w", i, m.Cid(), err) + } } - } - // We flush to get the VM's view of the state tree after applying the above messages - // This is needed to get the correct nonce from the actor state to match the VM - stateCid, err = vmi.Flush(ctx) - if err != nil { - return nil, xerrors.Errorf("flushing vm: %w", err) + // We flush to get the VM's view of the state tree after applying the above messages + // This is needed to get the correct nonce from the actor state to match the VM + stateCid, err = vmi.Flush(ctx) + if err != nil { + return nil, xerrors.Errorf("flushing vm: %w", err) + } } stTree, err := state.LoadStateTree(cbor.NewCborStore(buffStore), stateCid) diff --git a/chain/stmgr/forks.go b/chain/stmgr/forks.go index 9a236196187..c6c513e3a26 100644 --- a/chain/stmgr/forks.go +++ b/chain/stmgr/forks.go @@ -227,7 +227,7 @@ func (sm *StateManager) HandleStateForks(ctx context.Context, root cid.Cid, heig // Returns true executing tipsets between the specified heights would trigger an expensive // migration. NOTE: migrations occurring _at_ the target height are not included, as they're // executed _after_ the target height. -func (sm *StateManager) hasExpensiveForkBetween(parent, height abi.ChainEpoch) bool { +func (sm *StateManager) HasExpensiveForkBetween(parent, height abi.ChainEpoch) bool { for h := parent; h < height; h++ { if _, ok := sm.expensiveUpgrades[h]; ok { return true diff --git a/node/impl/full/eth.go b/node/impl/full/eth.go index 3b6bb408fb5..82f272c6cff 100644 --- a/node/impl/full/eth.go +++ b/node/impl/full/eth.go @@ -1028,25 +1028,26 @@ func (a *EthModule) applyMessage(ctx context.Context, msg *types.Message, tsk ty return nil, xerrors.Errorf("cannot get tipset: %w", err) } - applyTsMessages := true - if os.Getenv("LOTUS_SKIP_APPLY_TS_MESSAGE_CALL_WITH_GAS") == "1" { - applyTsMessages = false - } - - // Try calling until we find a height with no migration. - for { - res, err = a.StateManager.CallWithGas(ctx, msg, []types.ChainMsg{}, ts, applyTsMessages) - if err != stmgr.ErrExpensiveFork { - break - } - ts, err = a.Chain.GetTipSetFromKey(ctx, ts.Parents()) + if ts.Height() > 0 { + pts, err := a.Chain.GetTipSetFromKey(ctx, ts.Parents()) if err != nil { - return nil, xerrors.Errorf("getting parent tipset: %w", err) + return nil, xerrors.Errorf("failed to find a non-forking epoch: %w", err) + } + // Check for expensive forks from the parents to the tipset, including nil tipsets + if a.StateManager.HasExpensiveForkBetween(pts.Height(), ts.Height()+1) { + return nil, stmgr.ErrExpensiveFork } } + + st, _, err := a.StateManager.TipSetState(ctx, ts) + if err != nil { + return nil, xerrors.Errorf("cannot get tipset state: %w", err) + } + res, err = a.StateManager.ApplyOnStateWithGas(ctx, st, msg, ts) if err != nil { - return nil, xerrors.Errorf("CallWithGas failed: %w", err) + return nil, xerrors.Errorf("ApplyWithGasOnState failed: %w", err) } + if res.MsgRct.ExitCode.IsError() { reason := parseEthRevert(res.MsgRct.Return) return nil, xerrors.Errorf("message execution failed: exit %s, revert reason: %s, vm error: %s", res.MsgRct.ExitCode, reason, res.Error)